Message ID | 20230426095805.15338-6-ddrokosov@sberdevices.ru (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | add Amlogic A1 clock controller drivers | expand |
Hi Dmitry, On Wed, Apr 26, 2023 at 11:58 AM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote: > > Add the documentation for Amlogic A1 Peripherals clock driver, > and A1 Peripherals clock controller bindings. Maybe a native English speaker can comment on whether it's "peripheral" or "peripherals". [...] > Signed-off-by: Jian Hu <jian.hu@amlogic.com> > Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru> > Reviewed-by: Rob Herring <robh@kernel.org> > --- > .../bindings/clock/amlogic,a1-clkc.yaml | 73 +++++++++++ > .../bindings/clock/amlogic,a1-pll-clkc.yaml | 5 +- > include/dt-bindings/clock/amlogic,a1-clkc.h | 114 ++++++++++++++++++ I have seen that Yu Tu named the S4 peripheral clock controller binding and driver "s4-peripherals-clkc" [0]. Does it make sense to apply the same naming here as well? Best regards, Martin [0] https://lore.kernel.org/linux-amlogic/20230417065005.24967-3-yu.tu@amlogic.com/
> On 1 May 2023, at 7:51 pm, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > Hi Dmitry, > > On Wed, Apr 26, 2023 at 11:58 AM Dmitry Rokosov > <ddrokosov@sberdevices.ru> wrote: >> >> Add the documentation for Amlogic A1 Peripherals clock driver, >> and A1 Peripherals clock controller bindings. > Maybe a native English speaker can comment on whether it's > "peripheral" or "peripherals". I’m not a grammar specialist, but I would write: “Add documentation and bindings for the Amlogic A1 SoC peripherals clock driver” Peripherals is the correct plural but reads better when you add context on the type of peripherals. Christian > [...] >> Signed-off-by: Jian Hu <jian.hu@amlogic.com> >> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru> >> Reviewed-by: Rob Herring <robh@kernel.org> >> --- >> .../bindings/clock/amlogic,a1-clkc.yaml | 73 +++++++++++ >> .../bindings/clock/amlogic,a1-pll-clkc.yaml | 5 +- >> include/dt-bindings/clock/amlogic,a1-clkc.h | 114 ++++++++++++++++++ > I have seen that Yu Tu named the S4 peripheral clock controller > binding and driver "s4-peripherals-clkc" [0]. > Does it make sense to apply the same naming here as well? > > > Best regards, > Martin > > > [0] https://lore.kernel.org/linux-amlogic/20230417065005.24967-3-yu.tu@amlogic.com/ > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-amlogic
On 02/05/2023 03:38, Christian Hewitt wrote: >> On 1 May 2023, at 7:51 pm, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: >> >> Hi Dmitry, >> >> On Wed, Apr 26, 2023 at 11:58 AM Dmitry Rokosov >> <ddrokosov@sberdevices.ru> wrote: >>> >>> Add the documentation for Amlogic A1 Peripherals clock driver, >>> and A1 Peripherals clock controller bindings. >> Maybe a native English speaker can comment on whether it's >> "peripheral" or "peripherals". > > I’m not a grammar specialist, but I would write: > > “Add documentation and bindings for the Amlogic A1 SoC peripherals > clock driver” > > Peripherals is the correct plural but reads better when you add > context on the type of peripherals. Drop the "driver" references - from the binding itself and from commit msg. The bindings are for hardware, not for the driver, so: "for the Amlogic A1 SoC peripherals clock controller.". Best regards, Krzysztof
Hello Martin, Sorry for the delayed response as I was on vacation without laptop. On Mon, May 01, 2023 at 08:51:30PM +0200, Martin Blumenstingl wrote: > Hi Dmitry, > > On Wed, Apr 26, 2023 at 11:58 AM Dmitry Rokosov > <ddrokosov@sberdevices.ru> wrote: > > > > Add the documentation for Amlogic A1 Peripherals clock driver, > > and A1 Peripherals clock controller bindings. > Maybe a native English speaker can comment on whether it's > "peripheral" or "peripherals". > Ok > [...] > > Signed-off-by: Jian Hu <jian.hu@amlogic.com> > > Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru> > > Reviewed-by: Rob Herring <robh@kernel.org> > > --- > > .../bindings/clock/amlogic,a1-clkc.yaml | 73 +++++++++++ > > .../bindings/clock/amlogic,a1-pll-clkc.yaml | 5 +- > > include/dt-bindings/clock/amlogic,a1-clkc.h | 114 ++++++++++++++++++ > I have seen that Yu Tu named the S4 peripheral clock controller > binding and driver "s4-peripherals-clkc" [0]. > Does it make sense to apply the same naming here as well? Yes, it makes sense. I will prepare a new version with the necessary renaming. > > > Best regards, > Martin > > > [0] https://lore.kernel.org/linux-amlogic/20230417065005.24967-3-yu.tu@amlogic.com/
Hello Christian, On Tue, May 02, 2023 at 02:38:20AM +0100, Christian Hewitt wrote: > > On 1 May 2023, at 7:51 pm, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > > > Hi Dmitry, > > > > On Wed, Apr 26, 2023 at 11:58 AM Dmitry Rokosov > > <ddrokosov@sberdevices.ru> wrote: > >> > >> Add the documentation for Amlogic A1 Peripherals clock driver, > >> and A1 Peripherals clock controller bindings. > > Maybe a native English speaker can comment on whether it's > > "peripheral" or "peripherals". > > I’m not a grammar specialist, but I would write: > > “Add documentation and bindings for the Amlogic A1 SoC peripherals > clock driver” > > Peripherals is the correct plural but reads better when you add > context on the type of peripherals. Thank you for your suggestion! I will make the change in the v15. [...]
Hello Krzysztof, Thank you for the review! On Tue, May 02, 2023 at 09:39:12AM +0200, Krzysztof Kozlowski wrote: > On 02/05/2023 03:38, Christian Hewitt wrote: > >> On 1 May 2023, at 7:51 pm, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > >> > >> Hi Dmitry, > >> > >> On Wed, Apr 26, 2023 at 11:58 AM Dmitry Rokosov > >> <ddrokosov@sberdevices.ru> wrote: > >>> > >>> Add the documentation for Amlogic A1 Peripherals clock driver, > >>> and A1 Peripherals clock controller bindings. > >> Maybe a native English speaker can comment on whether it's > >> "peripheral" or "peripherals". > > > > I’m not a grammar specialist, but I would write: > > > > “Add documentation and bindings for the Amlogic A1 SoC peripherals > > clock driver” > > > > Peripherals is the correct plural but reads better when you add > > context on the type of peripherals. > > Drop the "driver" references - from the binding itself and from commit > msg. The bindings are for hardware, not for the driver, so: "for the > Amlogic A1 SoC peripherals clock controller.". Okay, thank you for the suggestion! I will remove it in the next version.
diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml new file mode 100644 index 000000000000..37dd89b43d9d --- /dev/null +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Amlogic A1 Peripheral Clock Control Unit + +maintainers: + - Neil Armstrong <neil.armstrong@linaro.org> + - Jerome Brunet <jbrunet@baylibre.com> + - Jian Hu <jian.hu@jian.hu.com> + - Dmitry Rokosov <ddrokosov@sberdevices.ru> + +properties: + compatible: + const: amlogic,a1-clkc + + '#clock-cells': + const: 1 + + reg: + maxItems: 1 + + clocks: + items: + - description: input fixed pll div2 + - description: input fixed pll div3 + - description: input fixed pll div5 + - description: input fixed pll div7 + - description: input hifi pll + - description: input oscillator (usually at 24MHz) + + clock-names: + items: + - const: fclk_div2 + - const: fclk_div3 + - const: fclk_div5 + - const: fclk_div7 + - const: hifi_pll + - const: xtal + +required: + - compatible + - '#clock-cells' + - reg + - clocks + - clock-names + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/amlogic,a1-pll-clkc.h> + apb { + #address-cells = <2>; + #size-cells = <2>; + + clock-controller@800 { + compatible = "amlogic,a1-clkc"; + reg = <0 0x800 0 0x104>; + #clock-cells = <1>; + clocks = <&clkc_pll CLKID_FCLK_DIV2>, + <&clkc_pll CLKID_FCLK_DIV3>, + <&clkc_pll CLKID_FCLK_DIV5>, + <&clkc_pll CLKID_FCLK_DIV7>, + <&clkc_pll CLKID_HIFI_PLL>, + <&xtal>; + clock-names = "fclk_div2", "fclk_div3", + "fclk_div5", "fclk_div7", + "hifi_pll", "xtal"; + }; + }; diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml index 5c6fa620a63c..3d8490ebcaa6 100644 --- a/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml @@ -43,6 +43,7 @@ additionalProperties: false examples: - | + #include <dt-bindings/clock/amlogic,a1-clkc.h> apb { #address-cells = <2>; #size-cells = <2>; @@ -51,8 +52,8 @@ examples: compatible = "amlogic,a1-pll-clkc"; reg = <0 0x7c80 0 0x18c>; #clock-cells = <1>; - clocks = <&clkc_periphs_fixpll_in>, - <&clkc_periphs_hifipll_in>; + clocks = <&clkc_periphs CLKID_FIXPLL_IN>, + <&clkc_periphs CLKID_HIFIPLL_IN>; clock-names = "fixpll_in", "hifipll_in"; }; }; diff --git a/include/dt-bindings/clock/amlogic,a1-clkc.h b/include/dt-bindings/clock/amlogic,a1-clkc.h new file mode 100644 index 000000000000..2c6221f2dc6b --- /dev/null +++ b/include/dt-bindings/clock/amlogic,a1-clkc.h @@ -0,0 +1,114 @@ +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */ +/* + * Copyright (c) 2019 Amlogic, Inc. All rights reserved. + * Author: Jian Hu <jian.hu@amlogic.com> + * + * Copyright (c) 2023, SberDevices. All Rights Reserved. + * Author: Dmitry Rokosov <ddrokosov@sberdevices.ru> + */ + +#ifndef __A1_CLKC_H +#define __A1_CLKC_H + +#define CLKID_FIXPLL_IN 1 +#define CLKID_USB_PHY_IN 2 +#define CLKID_USB_CTRL_IN 3 +#define CLKID_HIFIPLL_IN 4 +#define CLKID_SYSPLL_IN 5 +#define CLKID_DDS_IN 6 +#define CLKID_SYS 7 +#define CLKID_CLKTREE 8 +#define CLKID_RESET_CTRL 9 +#define CLKID_ANALOG_CTRL 10 +#define CLKID_PWR_CTRL 11 +#define CLKID_PAD_CTRL 12 +#define CLKID_SYS_CTRL 13 +#define CLKID_TEMP_SENSOR 14 +#define CLKID_AM2AXI_DIV 15 +#define CLKID_SPICC_B 16 +#define CLKID_SPICC_A 17 +#define CLKID_MSR 18 +#define CLKID_AUDIO 19 +#define CLKID_JTAG_CTRL 20 +#define CLKID_SARADC_EN 21 +#define CLKID_PWM_EF 22 +#define CLKID_PWM_CD 23 +#define CLKID_PWM_AB 24 +#define CLKID_CEC 25 +#define CLKID_I2C_S 26 +#define CLKID_IR_CTRL 27 +#define CLKID_I2C_M_D 28 +#define CLKID_I2C_M_C 29 +#define CLKID_I2C_M_B 30 +#define CLKID_I2C_M_A 31 +#define CLKID_ACODEC 32 +#define CLKID_OTP 33 +#define CLKID_SD_EMMC_A 34 +#define CLKID_USB_PHY 35 +#define CLKID_USB_CTRL 36 +#define CLKID_SYS_DSPB 37 +#define CLKID_SYS_DSPA 38 +#define CLKID_DMA 39 +#define CLKID_IRQ_CTRL 40 +#define CLKID_NIC 41 +#define CLKID_GIC 42 +#define CLKID_UART_C 43 +#define CLKID_UART_B 44 +#define CLKID_UART_A 45 +#define CLKID_SYS_PSRAM 46 +#define CLKID_RSA 47 +#define CLKID_CORESIGHT 48 +#define CLKID_AM2AXI_VAD 49 +#define CLKID_AUDIO_VAD 50 +#define CLKID_AXI_DMC 51 +#define CLKID_AXI_PSRAM 52 +#define CLKID_RAMB 53 +#define CLKID_RAMA 54 +#define CLKID_AXI_SPIFC 55 +#define CLKID_AXI_NIC 56 +#define CLKID_AXI_DMA 57 +#define CLKID_CPU_CTRL 58 +#define CLKID_ROM 59 +#define CLKID_PROC_I2C 60 +#define CLKID_DSPA_EN 63 +#define CLKID_DSPA_EN_NIC 64 +#define CLKID_DSPB_EN 65 +#define CLKID_DSPB_EN_NIC 66 +#define CLKID_RTC 67 +#define CLKID_CECA_32K 68 +#define CLKID_CECB_32K 69 +#define CLKID_24M 70 +#define CLKID_12M 71 +#define CLKID_FCLK_DIV2_DIVN 72 +#define CLKID_GEN 73 +#define CLKID_SARADC 75 +#define CLKID_PWM_A 76 +#define CLKID_PWM_B 77 +#define CLKID_PWM_C 78 +#define CLKID_PWM_D 79 +#define CLKID_PWM_E 80 +#define CLKID_PWM_F 81 +#define CLKID_SPICC 82 +#define CLKID_TS 83 +#define CLKID_SPIFC 84 +#define CLKID_USB_BUS 85 +#define CLKID_SD_EMMC 86 +#define CLKID_PSRAM 87 +#define CLKID_DMC 88 +#define CLKID_DSPA_A_SEL 95 +#define CLKID_DSPA_B_SEL 98 +#define CLKID_DSPB_A_SEL 101 +#define CLKID_DSPB_B_SEL 104 +#define CLKID_CECB_32K_SEL_PRE 113 +#define CLKID_CECB_32K_SEL 114 +#define CLKID_CECA_32K_SEL_PRE 117 +#define CLKID_CECA_32K_SEL 118 +#define CLKID_GEN_SEL 121 +#define CLKID_PWM_A_SEL 124 +#define CLKID_PWM_B_SEL 126 +#define CLKID_PWM_C_SEL 128 +#define CLKID_PWM_D_SEL 130 +#define CLKID_PWM_E_SEL 132 +#define CLKID_PWM_F_SEL 134 + +#endif /* __A1_CLKC_H */