18.08.2021 19:39, Thierry Reding пишет:

quoted

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

quoted

quoted

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?