2022-03-25 08:11:55

by Stephen Boyd

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

Quoting Johan Jonker (2022-03-24 12:51:36)
> 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'

'clocks' is not a dependency of 'assigned-clocks'. The dt-schema should
be fixed to remove that requirement.


2022-03-25 19:06:27

by Krzysztof Kozlowski

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

On 25/03/2022 01:51, Stephen Boyd wrote:
> Quoting Johan Jonker (2022-03-24 12:51:36)
>> 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'
>
> 'clocks' is not a dependency of 'assigned-clocks'. The dt-schema should
> be fixed to remove that requirement.

If the driver does not have any clock inputs ("clocks" property), why
does it care about some clock frequencies and parents?

The clocks is the logical dependency of assigned-clocks, because
otherwise hardware description is not complete.

What should be here for Rockhip? We had similar cases like this for many
drivers, I was fixing some of Exynos as well. In my case usually the
root/external clock was missing, so I supplied is as input clock to the
clock controller.


Best regards,
Krzysztof

2022-04-01 15:14:01

by Stephen Boyd

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

Quoting Krzysztof Kozlowski (2022-03-25 00:31:25)
> On 25/03/2022 01:51, Stephen Boyd wrote:
> > Quoting Johan Jonker (2022-03-24 12:51:36)
> >> 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'
> >
> > 'clocks' is not a dependency of 'assigned-clocks'. The dt-schema should
> > be fixed to remove that requirement.
>
> If the driver does not have any clock inputs ("clocks" property), why
> does it care about some clock frequencies and parents?

Because it's a clock provider itself. In this case I suspect because
this is a clock-controller node it was skipping describing some crystal
input though. Maybe it wants to configure the various PLLs in the
clock-controller for a particular board. I can imagine some node with
#clock-cells may want to configure the frequency of the clock outputs or
configure the clk parents for a certain board/SoC. In that case there
may not be any clocks property, but we still want to configure things.

>
> The clocks is the logical dependency of assigned-clocks, because
> otherwise hardware description is not complete.

Sure, but also #clock-cells indicating that this is a clock-controller
itself means something. The existing bindings are what they are so
forcing bindings to be updated to comply with having a 'clocks' property
doesn't seem very nice.

>
> What should be here for Rockhip? We had similar cases like this for many
> drivers, I was fixing some of Exynos as well. In my case usually the
> root/external clock was missing, so I supplied is as input clock to the
> clock controller.
>

Can the schema consider either #clock-cells or clocks? I think that will
work for most cases. It would also be good to have a comment in the
schema or more detail around the definition of assigned-clocks in
bindings/clock/clock-bindings.txt that clocks or #clock-cells are
required. It would be super cool if assigned-clocks only applied if
#clock-cells was present, otherwise clocks property applies, but I doubt
we can do that anymore given how long the binding has been around.

2022-04-14 10:20:02

by Rob Herring

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

On Thu, Mar 31, 2022 at 5:38 PM Stephen Boyd <[email protected]> wrote:
>
> Quoting Krzysztof Kozlowski (2022-03-25 00:31:25)
> > On 25/03/2022 01:51, Stephen Boyd wrote:
> > > Quoting Johan Jonker (2022-03-24 12:51:36)
> > >> 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'
> > >
> > > 'clocks' is not a dependency of 'assigned-clocks'. The dt-schema should
> > > be fixed to remove that requirement.
> >
> > If the driver does not have any clock inputs ("clocks" property), why
> > does it care about some clock frequencies and parents?
>
> Because it's a clock provider itself. In this case I suspect because
> this is a clock-controller node it was skipping describing some crystal
> input though. Maybe it wants to configure the various PLLs in the
> clock-controller for a particular board. I can imagine some node with
> #clock-cells may want to configure the frequency of the clock outputs or
> configure the clk parents for a certain board/SoC. In that case there
> may not be any clocks property, but we still want to configure things.
>
> >
> > The clocks is the logical dependency of assigned-clocks, because
> > otherwise hardware description is not complete.
>
> Sure, but also #clock-cells indicating that this is a clock-controller
> itself means something. The existing bindings are what they are so
> forcing bindings to be updated to comply with having a 'clocks' property
> doesn't seem very nice.
>
> >
> > What should be here for Rockhip? We had similar cases like this for many
> > drivers, I was fixing some of Exynos as well. In my case usually the
> > root/external clock was missing, so I supplied is as input clock to the
> > clock controller.
> >
>
> Can the schema consider either #clock-cells or clocks? I think that will
> work for most cases. It would also be good to have a comment in the
> schema or more detail around the definition of assigned-clocks in
> bindings/clock/clock-bindings.txt that clocks or #clock-cells are
> required. It would be super cool if assigned-clocks only applied if
> #clock-cells was present, otherwise clocks property applies, but I doubt
> we can do that anymore given how long the binding has been around.

I've just made a schema change to allow this. This will still require
'assigned-clocks' to be explicitly listed in the clock controller
schemas if 'clocks' is not present.

Please don't add to clock-bindings.txt, but improve the schema
instead. I need Samsung's permission to relicense all the
assigned-clocks text though.

Rob