| Message ID | 20220329111323.3569-1-jbx6244@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v3,1/2] dt-bindings: clock: convert rockchip,rk3188-cru.txt to YAML | expand |
On 29/03/2022 13:13, 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 > clock-names > > Signed-off-by: Johan Jonker <jbx6244@gmail.com> > --- > It's the second patchset without my review. What's happening here? Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
Quoting Johan Jonker (2022-03-29 04:13:22) > 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..ddd7e46af > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml > @@ -0,0 +1,78 @@ > +# 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: > + - Elaine Zhang <zhangqing@rock-chips.com> > + - 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 I'd expect all these clks here to be inputs to this node. > + > +properties: > + compatible: > + enum: > + - rockchip,rk3066a-cru > + - rockchip,rk3188-cru > + - rockchip,rk3188a-cru > + > + reg: > + maxItems: 1 > + > + "#clock-cells": > + const: 1 > + > + "#reset-cells": > + const: 1 > + > + clocks: > + maxItems: 1 so that maxItems here should be larger and minItems would be 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,rk3188-cru"; > + reg = <0x20000000 0x1000>; > + rockchip,grf = <&grf>; > + #clock-cells = <1>; > + #reset-cells = <1>; Where is clocks in the example?
Hi Stephen, Am Freitag, 1. April 2022, 00:51:32 CEST schrieb Stephen Boyd: > Quoting Johan Jonker (2022-03-29 04:13:22) > > 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..ddd7e46af > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml > > @@ -0,0 +1,78 @@ > > +# 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: > > + - Elaine Zhang <zhangqing@rock-chips.com> > > + - 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 > > I'd expect all these clks here to be inputs to this node. The optional clocks are all part of a circular dependency. So for example xin32k normally is generated by the pmic and fed back into the system, so to get xin32k, we need the pmic to probe, which needs i2c, which in turn already needs the clock controller. Or optional clocks may not be available at all. So for the past years we already relied on the clock-system's self adaptation if a clock becomes available at some point during later boot and hence do not have those in a clocks-property. Heiko
Quoting Heiko Stübner (2022-04-01 00:55:55) > Hi Stephen, > > Am Freitag, 1. April 2022, 00:51:32 CEST schrieb Stephen Boyd: > > Quoting Johan Jonker (2022-03-29 04:13:22) > > > 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..ddd7e46af > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml > > > @@ -0,0 +1,78 @@ > > > +# 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: > > > + - Elaine Zhang <zhangqing@rock-chips.com> > > > + - 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 > > > > I'd expect all these clks here to be inputs to this node. > > The optional clocks are all part of a circular dependency. > > So for example xin32k normally is generated by the pmic and fed > back into the system, so to get xin32k, we need the pmic to probe, > which needs i2c, which in turn already needs the clock controller. > > Or optional clocks may not be available at all. > > So for the past years we already relied on the clock-system's > self adaptation if a clock becomes available at some point > during later boot and hence do not have those in a clocks-property. > Are you saying that we can't have these clks in the clocks property because there's a circular driver probe dependency? I don't see how that matters for the dt binding. What is broken if they're put in the binding and specified in the node?
On 01/04/2022 09:55, Heiko Stübner wrote: > Hi Stephen, > > Am Freitag, 1. April 2022, 00:51:32 CEST schrieb Stephen Boyd: >> Quoting Johan Jonker (2022-03-29 04:13:22) >>> 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..ddd7e46af >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml >>> @@ -0,0 +1,78 @@ >>> +# 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: >>> + - Elaine Zhang <zhangqing@rock-chips.com> >>> + - 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 >> >> I'd expect all these clks here to be inputs to this node. > > The optional clocks are all part of a circular dependency. > > So for example xin32k normally is generated by the pmic and fed > back into the system, so to get xin32k, we need the pmic to probe, > which needs i2c, which in turn already needs the clock controller. Are you sure that xin32k (RTC) clock should be input to the clock controller? I would expect it is the input to the SoC RTC block, so there is no circular dependency. Best regards, Krzysztof
On 4/2/22 13:41, Krzysztof Kozlowski wrote: > On 01/04/2022 09:55, Heiko Stübner wrote: >> Hi Stephen, >> >> Am Freitag, 1. April 2022, 00:51:32 CEST schrieb Stephen Boyd: >>> Quoting Johan Jonker (2022-03-29 04:13:22) >>>> 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..ddd7e46af >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml >>>> @@ -0,0 +1,78 @@ >>>> +# 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: >>>> + - Elaine Zhang <zhangqing@rock-chips.com> >>>> + - 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 >>> >>> I'd expect all these clks here to be inputs to this node. >> >> The optional clocks are all part of a circular dependency. >> >> So for example xin32k normally is generated by the pmic and fed >> back into the system, so to get xin32k, we need the pmic to probe, >> which needs i2c, which in turn already needs the clock controller. > > Are you sure that xin32k (RTC) clock should be input to the clock > controller? I would expect it is the input to the SoC RTC block, so > there is no circular dependency. clk-rk3188.c: PNAME(mux_pll_p) = { "xin24m", "xin32k" }; > > > Best regards, > Krzysztof
On 02/04/2022 13:45, Johan Jonker wrote: > > > On 4/2/22 13:41, Krzysztof Kozlowski wrote: >> On 01/04/2022 09:55, Heiko Stübner wrote: >>> Hi Stephen, >>> >>> Am Freitag, 1. April 2022, 00:51:32 CEST schrieb Stephen Boyd: >>>> Quoting Johan Jonker (2022-03-29 04:13:22) >>>>> 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..ddd7e46af >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml >>>>> @@ -0,0 +1,78 @@ >>>>> +# 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: >>>>> + - Elaine Zhang <zhangqing@rock-chips.com> >>>>> + - 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 >>>> >>>> I'd expect all these clks here to be inputs to this node. >>> >>> The optional clocks are all part of a circular dependency. >>> >>> So for example xin32k normally is generated by the pmic and fed >>> back into the system, so to get xin32k, we need the pmic to probe, >>> which needs i2c, which in turn already needs the clock controller. >> >> Are you sure that xin32k (RTC) clock should be input to the clock >> controller? I would expect it is the input to the SoC RTC block, so >> there is no circular dependency. > > clk-rk3188.c: > > PNAME(mux_pll_p) = { "xin24m", "xin32k" }; Thanks, but that's not the answer whether it is an input to the clock controller. It's the answer how the driver implements this. :) Best regards, Krzysztof
On 4/2/22 14:16, Krzysztof Kozlowski wrote: > On 02/04/2022 13:45, Johan Jonker wrote: >> >> >> On 4/2/22 13:41, Krzysztof Kozlowski wrote: >>> On 01/04/2022 09:55, Heiko Stübner wrote: >>>> Hi Stephen, >>>> >>>> Am Freitag, 1. April 2022, 00:51:32 CEST schrieb Stephen Boyd: >>>>> Quoting Johan Jonker (2022-03-29 04:13:22) >>>>>> 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..ddd7e46af >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml >>>>>> @@ -0,0 +1,78 @@ >>>>>> +# 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: >>>>>> + - Elaine Zhang <zhangqing@rock-chips.com> >>>>>> + - 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 >>>>> >>>>> I'd expect all these clks here to be inputs to this node. >>>> >>>> The optional clocks are all part of a circular dependency. >>>> >>>> So for example xin32k normally is generated by the pmic and fed >>>> back into the system, so to get xin32k, we need the pmic to probe, >>>> which needs i2c, which in turn already needs the clock controller. >>> >>> Are you sure that xin32k (RTC) clock should be input to the clock >>> controller? I would expect it is the input to the SoC RTC block, so >>> there is no circular dependency. >> >> clk-rk3188.c: >> >> PNAME(mux_pll_p) = { "xin24m", "xin32k" }; > > Thanks, but that's not the answer whether it is an input to the clock > controller. It's the answer how the driver implements this. :) PX2 == rk3066 Rockchip PX2 TRM V1.0.pdf page 30 Chip Clock Architecture Diagram 1 > > Best regards, > Krzysztof
On 02/04/2022 14:20, Johan Jonker wrote: > > > On 4/2/22 14:16, Krzysztof Kozlowski wrote: >> On 02/04/2022 13:45, Johan Jonker wrote: >>> >>> >>> On 4/2/22 13:41, Krzysztof Kozlowski wrote: >>>> On 01/04/2022 09:55, Heiko Stübner wrote: >>>>> Hi Stephen, >>>>> >>>>> Am Freitag, 1. April 2022, 00:51:32 CEST schrieb Stephen Boyd: >>>>>> Quoting Johan Jonker (2022-03-29 04:13:22) >>>>>>> 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..ddd7e46af >>>>>>> --- /dev/null >>>>>>> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml >>>>>>> @@ -0,0 +1,78 @@ >>>>>>> +# 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: >>>>>>> + - Elaine Zhang <zhangqing@rock-chips.com> >>>>>>> + - 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 >>>>>> >>>>>> I'd expect all these clks here to be inputs to this node. >>>>> >>>>> The optional clocks are all part of a circular dependency. >>>>> >>>>> So for example xin32k normally is generated by the pmic and fed >>>>> back into the system, so to get xin32k, we need the pmic to probe, >>>>> which needs i2c, which in turn already needs the clock controller. >>>> >>>> Are you sure that xin32k (RTC) clock should be input to the clock >>>> controller? I would expect it is the input to the SoC RTC block, so >>>> there is no circular dependency. >>> >>> clk-rk3188.c: >>> >>> PNAME(mux_pll_p) = { "xin24m", "xin32k" }; >> >> Thanks, but that's not the answer whether it is an input to the clock >> controller. It's the answer how the driver implements this. :) > > PX2 == rk3066 > > Rockchip PX2 TRM V1.0.pdf > > page 30 > > Chip Clock Architecture Diagram 1 I found it, thanks. That's the answer, so indeed this is an input. Answering partially to Heiko/Stephen, this could be still modeled in DTS as a fixed-frequency clock going as input to the clock-controller. The trouble with that approach is that it would actually duplicate the definition (another one coming from PMIC). Best regards, Krzysztof
Am Samstag, 2. April 2022, 18:01:11 CEST schrieb Krzysztof Kozlowski: > On 02/04/2022 14:20, Johan Jonker wrote: > > > > > > On 4/2/22 14:16, Krzysztof Kozlowski wrote: > >> On 02/04/2022 13:45, Johan Jonker wrote: > >>> > >>> > >>> On 4/2/22 13:41, Krzysztof Kozlowski wrote: > >>>> On 01/04/2022 09:55, Heiko Stübner wrote: > >>>>> Hi Stephen, > >>>>> > >>>>> Am Freitag, 1. April 2022, 00:51:32 CEST schrieb Stephen Boyd: > >>>>>> Quoting Johan Jonker (2022-03-29 04:13:22) > >>>>>>> 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..ddd7e46af > >>>>>>> --- /dev/null > >>>>>>> +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml > >>>>>>> @@ -0,0 +1,78 @@ > >>>>>>> +# 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: > >>>>>>> + - Elaine Zhang <zhangqing@rock-chips.com> > >>>>>>> + - 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 > >>>>>> > >>>>>> I'd expect all these clks here to be inputs to this node. > >>>>> > >>>>> The optional clocks are all part of a circular dependency. > >>>>> > >>>>> So for example xin32k normally is generated by the pmic and fed > >>>>> back into the system, so to get xin32k, we need the pmic to probe, > >>>>> which needs i2c, which in turn already needs the clock controller. > >>>> > >>>> Are you sure that xin32k (RTC) clock should be input to the clock > >>>> controller? I would expect it is the input to the SoC RTC block, so > >>>> there is no circular dependency. > >>> > >>> clk-rk3188.c: > >>> > >>> PNAME(mux_pll_p) = { "xin24m", "xin32k" }; > >> > >> Thanks, but that's not the answer whether it is an input to the clock > >> controller. It's the answer how the driver implements this. :) > > > > PX2 == rk3066 > > > > Rockchip PX2 TRM V1.0.pdf > > > > page 30 > > > > Chip Clock Architecture Diagram 1 > > I found it, thanks. That's the answer, so indeed this is an input. > > Answering partially to Heiko/Stephen, this could be still modeled in DTS > as a fixed-frequency clock going as input to the clock-controller. The > trouble with that approach is that it would actually duplicate the > definition (another one coming from PMIC). And it has the issue that the pmic doesn't necessarily has the clock enabled at boot. Also this would combat one issue by introducing a different one, as this would be modelling a clock in a way that is not reflected by hardware at all.
On Tue, 29 Mar 2022 13:13:22 +0200, 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 > clock-names Applied, thanks! [1/2] dt-bindings: clock: convert rockchip,rk3188-cru.txt to YAML commit: 43434c6681daa89c4c96b6c5ab82cdf894960ee0 [2/2] ARM: dts: rockchip: add clocks property to cru node rk3066a/rk3188 commit: 89b6d86ded950b385b4778e061f7fc8311fb007e Best regards,
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..ddd7e46af --- /dev/null +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3188-cru.yaml @@ -0,0 +1,78 @@ +# 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: + - Elaine Zhang <zhangqing@rock-chips.com> + - 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: + 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,rk3188-cru"; + reg = <0x20000000 0x1000>; + rockchip,grf = <&grf>; + #clock-cells = <1>; + #reset-cells = <1>; + };
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 clock-names Signed-off-by: Johan Jonker <jbx6244@gmail.com> --- Changed V3: add Rockchip maintainer on her request fix yamllint line too long Changed V2: change clocks maxItems add clock-names use clock-controller node name remove assigned-xxx --- .../bindings/clock/rockchip,rk3188-cru.txt | 61 --------------- .../bindings/clock/rockchip,rk3188-cru.yaml | 78 +++++++++++++++++++ 2 files changed, 78 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