Re: [PATCH v1] dt-bindings: clock: convert rockchip,rk3188-cru.txt to YAML

Johan Jonker
2022-03-24T19:52:33Z

Hi Heiko, Krzysztof,

Question for the Rockchip clock maintainer:
What clock should be used here and other SoCs with several clock parents
in the tree?

The clock.yaml produces a lot off notifications like:

/arch/arm/boot/dts/rk3036-evb.dtb: clock-controller@20000000: 'clocks'
is a dependency of 'assigned-clocks'

Same for power-controller:

/arch/arm/boot/dts/rk3288-evb-rk808.dtb: power-controller: 'clocks' is a
dependency of 'assigned-clocks'

Just followed rk3399.yaml that got approved recent.
The current CRU nodes are in use for a long time this way.
Converting a simple .txt comes with a lot of extra work each time. :\
Please advise.

Johan

===
dependencies:
assigned-clocks: [clocks]

ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make dtbs_check
DT_SCHEMA_FILES=~/.local/lib/python3.7/site-packages/dtschema/schemas/clock/clock.yaml

===

See /Documentation/devicetree/bindings/clock/ grep for "assigned":

From cirrus,lochnagar.yaml:

clocks: true
assigned-clocks: true
assigned-clock-parents: true

Is this a possible solution?
So we we don't have to be specific.

On 3/24/22 15:28, Krzysztof Kozlowski wrote:
> On 24/03/2022 14:32, Johan Jonker wrote:
>> Current dts files with RK3188/RK3066 'cru' nodes are manually verified.
>> In order to automate this process rockchip,rk3188-cru.txt has to be
>> converted to YAML.
>>
>> Changed:
>> Add properties to fix notifications by clocks.yaml for example:
>> clocks
>> assigned-clock-rates
>> assigned-clocks
>>
>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>> ---
>> .../bindings/clock/rockchip,rk3188-cru.txt | 61 --------------
>> .../bindings/clock/rockchip,rk3188-cru.yaml | 81 +++++++++++++++++++
>> 2 files changed, 81 insertions(+), 61 deletions(-)
>> delete mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.txt
>> create mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.txt b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.txt
>> deleted file mode 100644
>> index 7f368530a..000000000
>> --- a/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.txt
>> +++ /dev/null
>> @@ -1,61 +0,0 @@
>> -* Rockchip RK3188/RK3066 Clock and Reset Unit
>> -
>> -The RK3188/RK3066 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,rk3188-cru", "rockchip,rk3188a-cru" or
>> - "rockchip,rk3066a-cru"
>> -- 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/rk3188-cru.h and
>> -dt-bindings/clock/rk3066-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,
>> - - "xin32k" - rtc clock - optional,
>> - - "xin27m" - 27mhz crystal input on rk3066 - optional,
>> - - "ext_hsadc" - external HSADC clock - optional,
>> - - "ext_cif0" - external camera clock - optional,
>> - - "ext_rmii" - external RMII clock - optional,
>> - - "ext_jtag" - externalJTAG clock - optional
>> -
>> -Example: Clock controller node:
>> -
>> - cru: cru@20000000 {
>> - compatible = "rockchip,rk3188-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:
>> -
>> - uart0: serial@10124000 {
>> - compatible = "snps,dw-apb-uart";
>> - reg = <0x10124000 0x400>;
>> - interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
>> - reg-shift = <2>;
>> - reg-io-width = <1>;
>> - clocks = <&cru SCLK_UART0>;
>> - };
>> diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml
>> new file mode 100644
>> index 000000000..136a9771e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml
>> @@ -0,0 +1,81 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/rockchip,rk3188-cru.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Rockchip RK3188/RK3066 Clock and Reset Unit (CRU)
>> +
>> +maintainers:
>> + - Heiko Stuebner <heiko@sntech.de>
>> +
>> +description: |
>> + The RK3188/RK3066 clock controller generates and supplies clocks 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/rk3188-cru.h and
>> + dt-bindings/clock/rk3066-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
>> + - "xin32k" - RTC clock - optional
>> + - "xin27m" - 27mhz crystal input on RK3066 - optional
>> + - "ext_hsadc" - external HSADC clock - optional
>> + - "ext_cif0" - external camera clock - optional
>> + - "ext_rmii" - external RMII clock - optional
>> + - "ext_jtag" - external JTAG clock - optional
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - rockchip,rk3066a-cru
>> + - rockchip,rk3188-cru
>> + - rockchip,rk3188a-cru
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + "#clock-cells":
>> + const: 1
>> +
>> + "#reset-cells":
>> + const: 1
>> +
>> + clocks:
>> + minItems: 1
>

> You need maxItems. Would be also nice to describe what should be here as
> input

Just followed rockchip,rk3399-cru.yaml that got rob+dt's ack.

Is this dependency only for simple clock consumers suitable for complex
clock trees with pll's, mux, etc? Shouldn't that be relaxed a bit?

>
>
>> +
>> + assigned-clock-rates:
>> + minItems: 1
>> + maxItems: 64
>> +
>> + assigned-clocks:
>> + minItems: 1
>> + maxItems: 64
>
> Both assigned-xxx should not be necessary. Did you try validate the dtbs
> without these?

That's how it's currently done for Rockchip cru's.

ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml

>
> I guess you added "clocks" above because of these, so you need to
> correct the DTS because asigned-xxx depend on clocks property.

See comment above.

>
>> +
>> + 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: cru@20000000 {
>
> Node name: clock-controller

Which schema?

>
>> + compatible = "rockchip,rk3188-cru";
>> + reg = <0x20000000 0x1000>;
>> + rockchip,grf = <&grf>;
>> + #clock-cells = <1>;
>> + #reset-cells = <1>;
>> + };
>
>
> Best regards,
> Krzysztof