| Message ID | 1569411888-98116-2-git-send-email-jian.hu@amlogic.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | add Amlogic A1 clock controller driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
On Wed 25 Sep 2019 at 19:44, Jian Hu <jian.hu@amlogic.com> wrote: In addition to the comment expressed by Stephen on patch 2 > Add the documentation to support Amlogic A1 clock driver, > and add A1 clock controller bindings. > > Signed-off-by: Jian Hu <jian.hu@amlogic.com> > Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> > --- > .../devicetree/bindings/clock/amlogic,a1-clkc.yaml | 65 +++++++++++++ > include/dt-bindings/clock/a1-clkc.h | 102 +++++++++++++++++++++ > 2 files changed, 167 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml > create mode 100644 include/dt-bindings/clock/a1-clkc.h > > 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 0000000..f012eb2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml > @@ -0,0 +1,65 @@ > +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ > +/* > + * Copyright (c) 2019 Amlogic, Inc. All rights reserved. > + */ > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Amlogic Meson A1 Clock Control Unit Device Tree Bindings > + > +maintainers: > + - Neil Armstrong <narmstrong@baylibre.com> > + - Jerome Brunet <jbrunet@baylibre.com> > + - Jian Hu <jian.hu@jian.hu.com> > + > +properties: > + compatible: > + - enum: > + - amlogic,a1-clkc > + > + reg: > + minItems: 1 > + maxItems: 3 > + items: > + - description: peripheral registers > + - description: cpu registers > + - description: pll registers > + > + reg-names: > + items: > + - const: peripheral > + - const: pll > + - const: cpu > + > + clocks: > + maxItems: 1 > + items: > + - description: Input Oscillator (usually at 24MHz) > + > + clock-names: > + maxItems: 1 > + items: > + - const: xtal > + > +required: > + - compatible > + - reg > + - reg-names > + - clocks > + - clock-names > + - "#clock-cells" > + > +examples: > + - | > + clkc: clock-controller { > + compatible = "amlogic,a1-clkc"; > + reg = <0x0 0xfe000800 0x0 0x100>, > + <0x0 0xfe007c00 0x0 0x21c>, > + <0x0 0xfd000080 0x0 0x20>; > + reg-names = "peripheral", "pll", "cpu"; I'm sorry but I don't agree with this. You are trying to regroup several controllers into one with this, and it is not OK By the looks of it there are 3 different controllers, including one you did not implement in the driver. > + clocks = <&xtal; > + clock-names = "xtal"; > + #clock-cells = <1>;
Hi, Jerome Thank you for review. On 2019/9/25 22:29, Jerome Brunet wrote: > On Wed 25 Sep 2019 at 19:44, Jian Hu <jian.hu@amlogic.com> wrote: > > In addition to the comment expressed by Stephen on patch 2 > got it. >> Add the documentation to support Amlogic A1 clock driver, >> and add A1 clock controller bindings. >> >> Signed-off-by: Jian Hu <jian.hu@amlogic.com> >> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> >> --- >> .../devicetree/bindings/clock/amlogic,a1-clkc.yaml | 65 +++++++++++++ >> include/dt-bindings/clock/a1-clkc.h | 102 +++++++++++++++++++++ >> 2 files changed, 167 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml >> create mode 100644 include/dt-bindings/clock/a1-clkc.h >> >> 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 0000000..f012eb2 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml >> @@ -0,0 +1,65 @@ >> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ >> +/* >> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved. >> + */ >> +%YAML 1.2 >> +--- >> +$id: "http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml#" >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >> + >> +title: Amlogic Meson A1 Clock Control Unit Device Tree Bindings >> + >> +maintainers: >> + - Neil Armstrong <narmstrong@baylibre.com> >> + - Jerome Brunet <jbrunet@baylibre.com> >> + - Jian Hu <jian.hu@jian.hu.com> >> + >> +properties: >> + compatible: >> + - enum: >> + - amlogic,a1-clkc >> + >> + reg: >> + minItems: 1 >> + maxItems: 3 >> + items: >> + - description: peripheral registers >> + - description: cpu registers >> + - description: pll registers >> + >> + reg-names: >> + items: >> + - const: peripheral >> + - const: pll >> + - const: cpu >> + >> + clocks: >> + maxItems: 1 >> + items: >> + - description: Input Oscillator (usually at 24MHz) >> + >> + clock-names: >> + maxItems: 1 >> + items: >> + - const: xtal >> + >> +required: >> + - compatible >> + - reg >> + - reg-names >> + - clocks >> + - clock-names >> + - "#clock-cells" >> + >> +examples: >> + - | >> + clkc: clock-controller { >> + compatible = "amlogic,a1-clkc"; >> + reg = <0x0 0xfe000800 0x0 0x100>, >> + <0x0 0xfe007c00 0x0 0x21c>, >> + <0x0 0xfd000080 0x0 0x20>; >> + reg-names = "peripheral", "pll", "cpu"; > > I'm sorry but I don't agree with this. You are trying to regroup several > controllers into one with this, and it is not OK > > By the looks of it there are 3 different controllers, including one you > did not implement in the driver. > Yes, In A1, the clock registers include three regions. I agree with your opinion. I will implement the two clock drivers of peripheral and plls first in PATCH V2. And CPU clock driver will be sent after the patches are merged. >> + clocks = <&xtal; >> + clock-names = "xtal"; >> + #clock-cells = <1>; > > . >
On Wed, Sep 25, 2019 at 6:45 AM Jian Hu <jian.hu@amlogic.com> wrote: > > Add the documentation to support Amlogic A1 clock driver, > and add A1 clock controller bindings. > > Signed-off-by: Jian Hu <jian.hu@amlogic.com> > Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> > --- > .../devicetree/bindings/clock/amlogic,a1-clkc.yaml | 65 +++++++++++++ > include/dt-bindings/clock/a1-clkc.h | 102 +++++++++++++++++++++ > 2 files changed, 167 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml > create mode 100644 include/dt-bindings/clock/a1-clkc.h > > 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 0000000..f012eb2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml > @@ -0,0 +1,65 @@ > +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ (GPL-2.0-only OR BSD-2-Clause) please. Rob
Hi Rob Thanks for your review On 2019/9/28 4:10, Rob Herring wrote: > On Wed, Sep 25, 2019 at 6:45 AM Jian Hu <jian.hu@amlogic.com> wrote: >> >> Add the documentation to support Amlogic A1 clock driver, >> and add A1 clock controller bindings. >> >> Signed-off-by: Jian Hu <jian.hu@amlogic.com> >> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> >> --- >> .../devicetree/bindings/clock/amlogic,a1-clkc.yaml | 65 +++++++++++++ >> include/dt-bindings/clock/a1-clkc.h | 102 +++++++++++++++++++++ >> 2 files changed, 167 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml >> create mode 100644 include/dt-bindings/clock/a1-clkc.h >> >> 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 0000000..f012eb2 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml >> @@ -0,0 +1,65 @@ >> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ > > (GPL-2.0-only OR BSD-2-Clause) please. > Sorry, I missed your reply. I will change it in next version v5. > Rob > > . >
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 0000000..f012eb2 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml @@ -0,0 +1,65 @@ +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ +/* + * Copyright (c) 2019 Amlogic, Inc. All rights reserved. + */ +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Amlogic Meson A1 Clock Control Unit Device Tree Bindings + +maintainers: + - Neil Armstrong <narmstrong@baylibre.com> + - Jerome Brunet <jbrunet@baylibre.com> + - Jian Hu <jian.hu@jian.hu.com> + +properties: + compatible: + - enum: + - amlogic,a1-clkc + + reg: + minItems: 1 + maxItems: 3 + items: + - description: peripheral registers + - description: cpu registers + - description: pll registers + + reg-names: + items: + - const: peripheral + - const: pll + - const: cpu + + clocks: + maxItems: 1 + items: + - description: Input Oscillator (usually at 24MHz) + + clock-names: + maxItems: 1 + items: + - const: xtal + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - "#clock-cells" + +examples: + - | + clkc: clock-controller { + compatible = "amlogic,a1-clkc"; + reg = <0x0 0xfe000800 0x0 0x100>, + <0x0 0xfe007c00 0x0 0x21c>, + <0x0 0xfd000080 0x0 0x20>; + reg-names = "peripheral", "pll", "cpu"; + clocks = <&xtal; + clock-names = "xtal"; + #clock-cells = <1>; + }; diff --git a/include/dt-bindings/clock/a1-clkc.h b/include/dt-bindings/clock/a1-clkc.h new file mode 100644 index 0000000..69fbf37 --- /dev/null +++ b/include/dt-bindings/clock/a1-clkc.h @@ -0,0 +1,102 @@ +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ +/* + * Copyright (c) 2019 Amlogic, Inc. All rights reserved. + */ + +#ifndef __A1_CLKC_H +#define __A1_CLKC_H + +#define CLKID_FIXED_PLL 1 +#define CLKID_FCLK_DIV2 2 +#define CLKID_FCLK_DIV3 3 +#define CLKID_FCLK_DIV5 4 +#define CLKID_FCLK_DIV7 5 +#define CLKID_FCLK_DIV2_DIV 6 +#define CLKID_FCLK_DIV3_DIV 7 +#define CLKID_FCLK_DIV5_DIV 8 +#define CLKID_FCLK_DIV7_DIV 9 +#define CLKID_SYS_CLK 16 +#define CLKID_HIFI_PLL 17 +#define CLKID_CLKTREE 25 +#define CLKID_RESET_CTRL 26 +#define CLKID_ANALOG_CTRL 27 +#define CLKID_PWR_CTRL 28 +#define CLKID_PAD_CTRL 29 +#define CLKID_SYS_CTRL 30 +#define CLKID_TEMP_SENSOR 31 +#define CLKID_AM2AXI_DIV 32 +#define CLKID_SPICC_B 33 +#define CLKID_SPICC_A 34 +#define CLKID_CLK_MSR 35 +#define CLKID_AUDIO 36 +#define CLKID_JTAG_CTRL 37 +#define CLKID_SARADC 38 +#define CLKID_PWM_EF 39 +#define CLKID_PWM_CD 40 +#define CLKID_PWM_AB 41 +#define CLKID_CEC 42 +#define CLKID_I2C_S 43 +#define CLKID_IR_CTRL 44 +#define CLKID_I2C_M_D 45 +#define CLKID_I2C_M_C 46 +#define CLKID_I2C_M_B 47 +#define CLKID_I2C_M_A 48 +#define CLKID_ACODEC 49 +#define CLKID_OTP 50 +#define CLKID_SD_EMMC_A 51 +#define CLKID_USB_PHY 52 +#define CLKID_USB_CTRL 53 +#define CLKID_SYS_DSPB 54 +#define CLKID_SYS_DSPA 55 +#define CLKID_DMA 56 +#define CLKID_IRQ_CTRL 57 +#define CLKID_NIC 58 +#define CLKID_GIC 59 +#define CLKID_UART_C 60 +#define CLKID_UART_B 61 +#define CLKID_UART_A 62 +#define CLKID_SYS_PSRAM 63 +#define CLKID_RSA 64 +#define CLKID_CORESIGHT 65 +#define CLKID_AM2AXI_VAD 66 +#define CLKID_AUDIO_VAD 67 +#define CLKID_AXI_DMC 68 +#define CLKID_AXI_PSRAM 69 +#define CLKID_RAMB 70 +#define CLKID_RAMA 71 +#define CLKID_AXI_SPIFC 72 +#define CLKID_AXI_NIC 73 +#define CLKID_AXI_DMA 74 +#define CLKID_CPU_CTRL 75 +#define CLKID_ROM 76 +#define CLKID_PROC_I2C 77 +#define CLKID_DSPA_SEL 84 +#define CLKID_DSPB_SEL 91 +#define CLKID_DSPA_EN_DSPA 92 +#define CLKID_DSPA_EN_NIC 93 +#define CLKID_DSPB_EN_DSPB 94 +#define CLKID_DSPB_EN_NIC 95 +#define CLKID_RTC_CLK 100 +#define CLKID_CECA_32K 105 +#define CLKID_CECB_32K 110 +#define CLKID_24M 111 +#define CLKID_12M 112 +#define CLKID_FCLK_DIV2_DIVN 114 +#define CLKID_GEN 118 +#define CLKID_SARADC_SEL 119 +#define CLKID_SARADC_CLK 121 +#define CLKID_PWM_A 124 +#define CLKID_PWM_B 127 +#define CLKID_PWM_C 130 +#define CLKID_PWM_D 133 +#define CLKID_PWM_E 136 +#define CLKID_PWM_F 139 +#define CLKID_SPICC 143 +#define CLKID_TS 145 +#define CLKID_SPIFC 149 +#define CLKID_USB_BUS 152 +#define CLKID_SD_EMMC 156 +#define CLKID_PSRAM 160 +#define CLKID_DMC 164 + +#endif /* __A1_CLKC_H */