diff mbox series

[1/2] dt-bindings: clock: meson: add A1 clock controller bindings

Message ID 1569411888-98116-2-git-send-email-jian.hu@amlogic.com (mailing list archive)
State Changes Requested, archived
Headers show
Series add Amlogic A1 clock controller driver | expand

Commit Message

Jian Hu Sept. 25, 2019, 11:44 a.m. UTC 
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

Comments

Jerome Brunet Sept. 25, 2019, 2:29 p.m. UTC | #1 
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>;
Jian Hu Sept. 27, 2019, 6:08 a.m. UTC | #2 
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>;
> 
> .
>
Rob Herring Sept. 27, 2019, 8:10 p.m. UTC | #3 
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
Jian Hu Dec. 18, 2019, 7:30 a.m. UTC | #4 
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 mbox series

Patch

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 */