diff mbox series

[v3,1/2] dt-bindings: clock: convert rockchip, rk3188-cru.txt to YAML

Message ID 20220329111323.3569-1-jbx6244@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/2] dt-bindings: clock: convert rockchip, rk3188-cru.txt to YAML | expand

Commit Message

Johan Jonker March 29, 2022, 11:13 a.m. UTC
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

Comments

Krzysztof Kozlowski March 29, 2022, 11:43 a.m. UTC | #1
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
Stephen Boyd March 31, 2022, 10:51 p.m. UTC | #2
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?
Heiko Stübner April 1, 2022, 7:55 a.m. UTC | #3
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
Stephen Boyd April 1, 2022, 6:24 p.m. UTC | #4
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?
Krzysztof Kozlowski April 2, 2022, 11:41 a.m. UTC | #5
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
Johan Jonker April 2, 2022, 11:45 a.m. UTC | #6
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
Krzysztof Kozlowski April 2, 2022, 12:16 p.m. UTC | #7
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
Johan Jonker April 2, 2022, 12:20 p.m. UTC | #8
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
Krzysztof Kozlowski April 2, 2022, 4:01 p.m. UTC | #9
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
Heiko Stübner April 6, 2022, 4:18 p.m. UTC | #10
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.
Heiko Stübner May 17, 2022, 7:11 p.m. UTC | #11
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 mbox series

Patch

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>;
+    };