Re: [PATCH v2 13/14] soc/tegra: pmc: Add core power domain

Dmitry Osipenko
2021-06-01T15:48:40Z

01.06.2021 13:19, Ulf Hansson пишет:
...
>> This is not sufficient for Tegra because we have individual OPP tables for the root PLLs, system clocks and device clocks. The device clocks could be muxed to a different PLLs, depending on clk requirements of a particular board.
>
> Are you saying that the clock providers for the "root PLLs" and
> "system clocks" have OPP tables themselves? If so, would you mind
> posting a patch for an updated DT binding for these changes, so it can
> be discussed separately?

I will post all those patches soon, thank you.

...
>> The device drivers don't manage the parent clocks directly and OPP core doesn't support this use-case where OPP needs to be applied to a generic/parent PLL clock. Moving the OPP management to the clk driver is the easy solution which works good in practice for Tegra, it also removes a need to switch each driver to dev_pm_opp_set_rate() usage.
>
> I admit, if clock consumer drivers could avoid calling
> dev_pm_opp_set_rate|opp(), that would be nice. But, as I stated, it's
> a fragile path from locking point of view, to call
> dev_pm_opp_set_rate|opp() from a clock provider driver. Personally, I
> think it's better to avoid it.
>
> More importantly, you also need to convince the clock subsystem
> maintainers, that setting an OPP internally from the clock provider
> driver is a good idea. As far as I can tell, they have said *no* to
> this, since the common clock framework was invented, I believe for
> good reasons.

Pushing the OPP into a CCF driver is indeed not ideal. I'm open to new
ideas. I will post those patches where we could discuss this in a more
details.

...
>> For example please see clock@60006000 and pmc@7000e400 nodes of [1].
>>
>> [1] https://github.com/grate-driver/linux/blob/master/arch/arm/boot/dts/tegra30.dtsi
>
> Thanks, that certainly helped me understand better!
>
> I see that you want to add OPP tables to clock provider nodes. As I
> said above, an updated DT binding is probably a good idea to discuss
> separately.
...
>
> Okay, to not stall things from moving forward, may I suggest that you
> simply drop the call to lockdep_set_class() (and the corresponding
> comment) for now.
>
> Then you can continue to post the next parts - and if it turns out
> that lockdep_set_class() becomes needed, you can always add it back
> then.

Thank you very much for helping with reviewing this all. I'll drop the
lockdep_set_class() and post the v7 shortly. Afterwards, I'll send the
rest of clk, device-tree and etc related patches targeting 5.15.