2022-09-11 21:23:25

by Johan Jonker

[permalink] [raw]
Subject: [PATCH v1] dt-bindings: clock: convert rockchip,rk3128-cru.txt to YAML

Convert rockchip,rk3128-cru.txt to YAML.

Signed-off-by: Johan Jonker <[email protected]>
---
.../bindings/clock/rockchip,rk3128-cru.txt | 58 ---------------
.../bindings/clock/rockchip,rk3128-cru.yaml | 73 +++++++++++++++++++
2 files changed, 73 insertions(+), 58 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.txt
create mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml

diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.txt b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.txt
deleted file mode 100644
index 6f8744fd3..000000000
--- a/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.txt
+++ /dev/null
@@ -1,58 +0,0 @@
-* Rockchip RK3126/RK3128 Clock and Reset Unit
-
-The RK3126/RK3128 clock controller generates and supplies clock to various
-controllers within the SoC and also implements a reset controller for SoC
-peripherals.
-
-Required Properties:
-
-- compatible: should be "rockchip,rk3126-cru" or "rockchip,rk3128-cru"
- "rockchip,rk3126-cru" - controller compatible with RK3126 SoC.
- "rockchip,rk3128-cru" - controller compatible with RK3128 SoC.
-- reg: physical base address of the controller and length of memory mapped
- region.
-- #clock-cells: should be 1.
-- #reset-cells: should be 1.
-
-Optional Properties:
-
-- rockchip,grf: phandle to the syscon managing the "general register files"
- If missing pll rates are not changeable, due to the missing pll lock status.
-
-Each clock is assigned an identifier and client nodes can use this identifier
-to specify the clock which they consume. All available clocks are defined as
-preprocessor macros in the dt-bindings/clock/rk3128-cru.h headers and can be
-used in device tree sources. Similar macros exist for the reset sources in
-these files.
-
-External clocks:
-
-There are several clocks that are generated outside the SoC. It is expected
-that they are defined using standard clock bindings with following
-clock-output-names:
- - "xin24m" - crystal input - required,
- - "ext_i2s" - external I2S clock - optional,
- - "gmac_clkin" - external GMAC clock - optional
-
-Example: Clock controller node:
-
- cru: cru@20000000 {
- compatible = "rockchip,rk3128-cru";
- reg = <0x20000000 0x1000>;
- rockchip,grf = <&grf>;
-
- #clock-cells = <1>;
- #reset-cells = <1>;
- };
-
-Example: UART controller node that consumes the clock generated by the clock
- controller:
-
- uart2: serial@20068000 {
- compatible = "rockchip,serial";
- reg = <0x20068000 0x100>;
- interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
- clock-frequency = <24000000>;
- clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
- clock-names = "sclk_uart", "pclk_uart";
- };
diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
new file mode 100644
index 000000000..03e5d7f0e
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/rockchip,rk3128-cru.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip RK3126/RK3128 Clock and Reset Unit (CRU)
+
+maintainers:
+ - Elaine Zhang <[email protected]>
+ - Heiko Stuebner <[email protected]>
+
+description: |
+ The RK3126/RK3128 clock controller generates and supplies clock to various
+ controllers within the SoC and also implements a reset controller for SoC
+ peripherals.
+ Each clock is assigned an identifier and client nodes can use this identifier
+ to specify the clock which they consume. All available clocks are defined as
+ preprocessor macros in the dt-bindings/clock/rk3128-cru.h headers and can be
+ used in device tree sources. Similar macros exist for the reset sources in
+ these files.
+ There are several clocks that are generated outside the SoC. It is expected
+ that they are defined using standard clock bindings with following
+ clock-output-names:
+ - "xin24m" - crystal input - required
+ - "ext_i2s" - external I2S clock - optional
+ - "gmac_clkin" - external GMAC clock - optional
+
+properties:
+ compatible:
+ enum:
+ - rockchip,rk3126-cru
+ - rockchip,rk3128-cru
+
+ reg:
+ maxItems: 1
+
+ "#clock-cells":
+ const: 1
+
+ "#reset-cells":
+ const: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ const: xin24m
+
+ rockchip,grf:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the syscon managing the "general register files" (GRF),
+ if missing pll rates are not changeable, due to the missing pll
+ lock status.
+
+required:
+ - compatible
+ - reg
+ - "#clock-cells"
+ - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ cru: clock-controller@20000000 {
+ compatible = "rockchip,rk3128-cru";
+ reg = <0x20000000 0x1000>;
+ rockchip,grf = <&grf>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ };
--
2.20.1


2022-09-12 11:21:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1] dt-bindings: clock: convert rockchip,rk3128-cru.txt to YAML

On 11/09/2022 23:20, Johan Jonker wrote:
> Convert rockchip,rk3128-cru.txt to YAML.
>
> Signed-off-by: Johan Jonker <[email protected]>

Thank you for your patch. There is something to discuss/improve.

> diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
> new file mode 100644
> index 000000000..03e5d7f0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: GPL-2.0

Can't it be Dual licensed?

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/rockchip,rk3128-cru.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip RK3126/RK3128 Clock and Reset Unit (CRU)
> +
> +maintainers:
> + - Elaine Zhang <[email protected]>
> + - Heiko Stuebner <[email protected]>
> +
> +description: |
> + The RK3126/RK3128 clock controller generates and supplies clock to various
> + controllers within the SoC and also implements a reset controller for SoC
> + peripherals.
> + Each clock is assigned an identifier and client nodes can use this identifier
> + to specify the clock which they consume. All available clocks are defined as
> + preprocessor macros in the dt-bindings/clock/rk3128-cru.h headers and can be
> + used in device tree sources. Similar macros exist for the reset sources in
> + these files.
> + There are several clocks that are generated outside the SoC. It is expected
> + that they are defined using standard clock bindings with following
> + clock-output-names:
> + - "xin24m" - crystal input - required
> + - "ext_i2s" - external I2S clock - optional
> + - "gmac_clkin" - external GMAC clock - optional
> +
> +properties:
> + compatible:
> + enum:
> + - rockchip,rk3126-cru
> + - rockchip,rk3128-cru
> +
> + reg:
> + maxItems: 1
> +
> + "#clock-cells":
> + const: 1
> +
> + "#reset-cells":
> + const: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: xin24m

More clocks were mentioned in old binding.

> +
> + rockchip,grf:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the syscon managing the "general register files" (GRF),
> + if missing pll rates are not changeable, due to the missing pll
> + lock status.
> +
> +required:
> + - compatible
> + - reg
> + - "#clock-cells"
> + - "#reset-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + cru: clock-controller@20000000 {
> + compatible = "rockchip,rk3128-cru";
> + reg = <0x20000000 0x1000>;
> + rockchip,grf = <&grf>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };


Best regards,
Krzysztof

2022-09-13 17:29:36

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v1] dt-bindings: clock: convert rockchip,rk3128-cru.txt to YAML

On Sun, Sep 11, 2022 at 11:20:10PM +0200, Johan Jonker wrote:
> Convert rockchip,rk3128-cru.txt to YAML.
>
> Signed-off-by: Johan Jonker <[email protected]>
> ---
> .../bindings/clock/rockchip,rk3128-cru.txt | 58 ---------------
> .../bindings/clock/rockchip,rk3128-cru.yaml | 73 +++++++++++++++++++
> 2 files changed, 73 insertions(+), 58 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.txt
> create mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
>
> diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.txt b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.txt
> deleted file mode 100644
> index 6f8744fd3..000000000
> --- a/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.txt
> +++ /dev/null
> @@ -1,58 +0,0 @@
> -* Rockchip RK3126/RK3128 Clock and Reset Unit
> -
> -The RK3126/RK3128 clock controller generates and supplies clock to various
> -controllers within the SoC and also implements a reset controller for SoC
> -peripherals.
> -
> -Required Properties:
> -
> -- compatible: should be "rockchip,rk3126-cru" or "rockchip,rk3128-cru"
> - "rockchip,rk3126-cru" - controller compatible with RK3126 SoC.
> - "rockchip,rk3128-cru" - controller compatible with RK3128 SoC.
> -- reg: physical base address of the controller and length of memory mapped
> - region.
> -- #clock-cells: should be 1.
> -- #reset-cells: should be 1.
> -
> -Optional Properties:
> -
> -- rockchip,grf: phandle to the syscon managing the "general register files"
> - If missing pll rates are not changeable, due to the missing pll lock status.
> -
> -Each clock is assigned an identifier and client nodes can use this identifier
> -to specify the clock which they consume. All available clocks are defined as
> -preprocessor macros in the dt-bindings/clock/rk3128-cru.h headers and can be
> -used in device tree sources. Similar macros exist for the reset sources in
> -these files.
> -
> -External clocks:
> -
> -There are several clocks that are generated outside the SoC. It is expected
> -that they are defined using standard clock bindings with following
> -clock-output-names:
> - - "xin24m" - crystal input - required,
> - - "ext_i2s" - external I2S clock - optional,
> - - "gmac_clkin" - external GMAC clock - optional
> -
> -Example: Clock controller node:
> -
> - cru: cru@20000000 {
> - compatible = "rockchip,rk3128-cru";
> - reg = <0x20000000 0x1000>;
> - rockchip,grf = <&grf>;
> -
> - #clock-cells = <1>;
> - #reset-cells = <1>;
> - };
> -
> -Example: UART controller node that consumes the clock generated by the clock
> - controller:
> -
> - uart2: serial@20068000 {
> - compatible = "rockchip,serial";
> - reg = <0x20068000 0x100>;
> - interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> - clock-frequency = <24000000>;
> - clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
> - clock-names = "sclk_uart", "pclk_uart";
> - };
> diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
> new file mode 100644
> index 000000000..03e5d7f0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/rockchip,rk3128-cru.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip RK3126/RK3128 Clock and Reset Unit (CRU)
> +
> +maintainers:
> + - Elaine Zhang <[email protected]>
> + - Heiko Stuebner <[email protected]>
> +
> +description: |
> + The RK3126/RK3128 clock controller generates and supplies clock to various
> + controllers within the SoC and also implements a reset controller for SoC
> + peripherals.
> + Each clock is assigned an identifier and client nodes can use this identifier
> + to specify the clock which they consume. All available clocks are defined as
> + preprocessor macros in the dt-bindings/clock/rk3128-cru.h headers and can be
> + used in device tree sources. Similar macros exist for the reset sources in
> + these files.

> + There are several clocks that are generated outside the SoC. It is expected
> + that they are defined using standard clock bindings with following
> + clock-output-names:

Which node does clock-output-names live in?

From the description, it sounds more like this should be
clocks/clock-names.

> + - "xin24m" - crystal input - required
> + - "ext_i2s" - external I2S clock - optional
> + - "gmac_clkin" - external GMAC clock - optional

2022-09-14 10:03:13

by Johan Jonker

[permalink] [raw]
Subject: Re: [PATCH v1] dt-bindings: clock: convert rockchip,rk3128-cru.txt to YAML



On 9/12/22 12:51, Krzysztof Kozlowski wrote:
> On 11/09/2022 23:20, Johan Jonker wrote:
>> Convert rockchip,rk3128-cru.txt to YAML.
>>
>> Signed-off-by: Johan Jonker <[email protected]>
>
> Thank you for your patch. There is something to discuss/improve.
>
>> diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
>> new file mode 100644
>> index 000000000..03e5d7f0e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
>> @@ -0,0 +1,73 @@
>> +# SPDX-License-Identifier: GPL-2.0
>

> Can't it be Dual licensed?

That depends on Heiko and Rockchip.
I can produce the patch for it, but I'm not in control whether they reply or not.

===============

dt-bindings: add bindings for px30 clock controller
@mmind
Elaine Zhang authored and mmind committed on Jul 3, 2018

dt-bindings: add documentation for rk1108 cru
@shawn1221
@mmind
shawn1221 authored and mmind committed on Nov 16, 2016

dt-bindings: add documentation of rk3036 clock controller
@acgzx
@mmind
acgzx authored and mmind committed on Nov 23, 2015

dt-bindings: add documentation for rk3188 clock and reset unit
@mmind
mmind authored and Mike Turquette committed on Jul 13, 2014

dt-bindings: add documentation of rk3228 clock controller
@JeffyCN
@mmind
JeffyCN authored and mmind committed on Dec 12, 2015

dt-bindings: add documentation for rk3288 cru
@mmind
mmind authored and Mike Turquette committed on Jul 13, 2014

dt-bindings: add documentation of rk3668 clock controller
@mmind
@bebarino
mmind authored and bebarino committed on Jul 7, 2015

dt-bindings: Add bindings for rk3308 clock controller
@finley1226
@mmind
finley1226 authored and mmind committed on Sep 5, 2019

dt-bindings: add bindings for rk3328 clock controller
@mmind
Elaine Zhang authored and mmind committed on Jan 5, 2017

dt-bindings: add documentation of rk3668 clock controller
@mmind
@bebarino
mmind authored and bebarino committed on Jul 7, 2015

dt-bindings: add bindings for rk3399 clock controller
@acgzx
@mmind
acgzx authored and mmind committed on Mar 28, 2016


>
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/rockchip,rk3128-cru.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Rockchip RK3126/RK3128 Clock and Reset Unit (CRU)
>> +
>> +maintainers:
>> + - Elaine Zhang <[email protected]>
>> + - Heiko Stuebner <[email protected]>
>> +
>> +description: |
>> + The RK3126/RK3128 clock controller generates and supplies clock to various
>> + controllers within the SoC and also implements a reset controller for SoC
>> + peripherals.
>> + Each clock is assigned an identifier and client nodes can use this identifier
>> + to specify the clock which they consume. All available clocks are defined as
>> + preprocessor macros in the dt-bindings/clock/rk3128-cru.h headers and can be
>> + used in device tree sources. Similar macros exist for the reset sources in
>> + these files.
>> + There are several clocks that are generated outside the SoC. It is expected
>> + that they are defined using standard clock bindings with following
>> + clock-output-names:
>> + - "xin24m" - crystal input - required
>> + - "ext_i2s" - external I2S clock - optional
>> + - "gmac_clkin" - external GMAC clock - optional
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - rockchip,rk3126-cru
>> + - rockchip,rk3128-cru
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + "#clock-cells":
>> + const: 1
>> +
>> + "#reset-cells":
>> + const: 1
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + clock-names:
>> + const: xin24m
>

> More clocks were mentioned in old binding.

We go for a slow approach.
The man with the hammer syndrome thinks that every hardware needs to described with the DT method.
For clocks we do that in the clock driver with macros and structures.
This works fine for Rockchip. If the need arises further proposals will be submitted.
So thanks, but no thanks!


In U-boot we can't filter clocks with exception for the cru node, so DTOC adds them to dt-plat.c with tpl size increase.
When syncing DT changes must be made all over the place.

static struct dtd_fixed_clock dtv_oscillator = {
.clock_frequency = 0x16e3600,
.clock_output_names = "xin24m",
};
U_BOOT_DRVINFO(oscillator) = {
.name = "fixed_clock",
.plat = &dtv_oscillator,
.plat_size = sizeof(dtv_oscillator),
.parent_idx = -1,
};

static struct dtd_rockchip_rk3066a_cru dtv_clock_controller_at_20000000 = {
.clocks = {
{3, {}},},
.reg = {0x20000000, 0x1000},
.rockchip_grf = 0x9,
};
U_BOOT_DRVINFO(clock_controller_at_20000000) = {
.name = "rockchip_rk3066a_cru",
.plat = &dtv_clock_controller_at_20000000,
.plat_size = sizeof(dtv_clock_controller_at_20000000),
.parent_idx = -1,
};

Comment by Heiko:

In the discussion Stephen had comments about the optional clocks, that
have a circular dependency (xin24m -> cru -> i2c -> rtc -> xin32k -> cru) .
After everyone put in their argument, the discussion itself just stopped.

I've picked up the conversion patches anyway, as this is the status-quo
in terms of modelling clocks on Rockchip, so the yaml conversion doesn't
change anything in either direction and just transforms what is in the
kernel right now into the yaml format which will improve devicetree checks
a lot and also reduce the number of warnings emitted by the checker.

https://lore.kernel.org/linux-rockchip/5781991.QJadu78ljV@phil/

>
>> +
>> + rockchip,grf:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + Phandle to the syscon managing the "general register files" (GRF),
>> + if missing pll rates are not changeable, due to the missing pll
>> + lock status.
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - "#clock-cells"
>> + - "#reset-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + cru: clock-controller@20000000 {
>> + compatible = "rockchip,rk3128-cru";
>> + reg = <0x20000000 0x1000>;
>> + rockchip,grf = <&grf>;
>> + #clock-cells = <1>;
>> + #reset-cells = <1>;
>> + };
>
>
> Best regards,
> Krzysztof

2022-09-17 06:59:35

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH v1] dt-bindings: clock: convert rockchip,rk3128-cru.txt to YAML

Am Mittwoch, 14. September 2022, 11:25:48 CEST schrieb Johan Jonker:
>
> On 9/12/22 12:51, Krzysztof Kozlowski wrote:
> > On 11/09/2022 23:20, Johan Jonker wrote:
> >> Convert rockchip,rk3128-cru.txt to YAML.
> >>
> >> Signed-off-by: Johan Jonker <[email protected]>
> >
> > Thank you for your patch. There is something to discuss/improve.
> >
> >> diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
> >> new file mode 100644
> >> index 000000000..03e5d7f0e
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3128-cru.yaml
> >> @@ -0,0 +1,73 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> >
>
> > Can't it be Dual licensed?
>
> That depends on Heiko and Rockchip.
> I can produce the patch for it, but I'm not in control whether they reply or not.

Rockchip recently replied on other clock-patches to dual-license the
binding, so this will be ok [0] .

And for me, following in-kernel policies more, is always acceptable :-)

Heiko

[0] https://lore.kernel.org/all/[email protected]/
From Finley with an @rock-chips.com address, so this should be ok