Re: [PATCH v8 06/34] dt-bindings: clock: tegra-car: Document new tegra-clocks sub-node

Dmitry Osipenko
2021-08-18T16:57:21Z

18.08.2021 19:39, Thierry Reding пишет:
>> We don't have a platform device for CaR. I don't see how it's going to
>> work. We need to create a platform device for each RPM-capable clock
>> because that's how RPM works. The compatible string is required for
>> instantiating OF-devices from a node, otherwise we will have to
>> re-invent the OF core.
> I think we do have a platform device for CAR. It's just not bound
> against by the driver because these clock drivers are "special". But
> from other parts of the series you're already trying to fix that, at
> least partially.
>
> But it doesn't seem right to create a platform device for each RPM-
> capable clock. Why do they need to be devices? They aren't, so why
> pretend? Is it that some API that we want to use here requires the
> struct device?

The "device" representation is internal to the kernel. It's okay to me
to have PLLs represented by a device, it's a distinct h/w by itself.

CCF supports managing of clock's RPM and it requires to have clock to be
backed by a device. That's what we are using here.

Please see
https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/clk/clk.c#L109

>>> Also, I don't think the tegra- prefix is necessary here. The parent node
>>> is already identified as Tegra via the compatible string.
>>>
>>> In the case of CAR, I'd imagine something like:
>>>
>>> clocks {
>>> sclk {
>>> operating-points-v2 = <&opp_table>;
>>> power-domains = <&domain>;
>>> };
>>> };
>>>
>>> Now you've only got the bare minimum in here that you actually add. All
>>> the other data that you used to have is simply derived from the parent.
>> 'clocks' is already a generic keyword in DT. It's probably not okay to
>> redefine it.
> "clocks" is not a generic keyword. It's the name of a property and given
> that we're talking about the clock provider here, it doesn't need a
> clocks property of its own, so it should be fine to use that for the
> node.

I'm curious what Rob thinks about it. Rob, does this sound okay to you?