Re: [PATCH 4/4] drivers: clk: renesas: enable all clocks which is assinged to non Linux system

[Stephen Boyd <sboyd@kernel.org>]

Quoting Geert Uytterhoeven (2023-11-16 13:08:46)
> On Thu, Nov 16, 2023 at 8:23 PM Rob Herring <robh@kernel.org> wrote:
> > On Tue, Nov 14, 2023 at 12:01:14AM +0000, Kuninori Morimoto wrote:
> > > Some board might use Linux and another OS in the same time. In such
> > > case, current Linux will stop necessary module clock when booting
> > > which is not used on Linux side, but is used on another OS side.
> > >
> > > To avoid such situation, renesas-cpg-mssr try to find
> > > status = "reserved" devices (A), and add CLK_IGNORE_UNUSED flag to its
> > > <&cgp CPG_MOD xxx> clock (B).
> >
> > See Stephen's presentation from Plumbers this week. The default behavior
> > for unused clocks may be changing soon.
> 
> Thank you!
> 
> ou mean "Make sync_state()/handoff work for the common clk
> framework"[1]? IIUIC, that presentation didn't cover the problem we are
> facing, except for the big "Kconfig for clk_ignore_unused=true" hammer.

:)

> 
> > > Table 2.4: Values for status property
> > > https://github.com/devicetree-org/devicetree-specification/releases/download/v0.4/devicetree-specification-v0.4.pdf
> > >
> > > "reserved"
> > >       Indicates that the device is operational, but should not be
> > >       used. Typically this is used for devices that are controlled
> > >       by another software component, such as platform firmware.
> > >
> > > ex)
> > >       scif5: serial@e6f30000 {
> > >               ...
> > > (B)           clocks = <&cpg CPG_MOD 202>,
> > >                        <&cpg CPG_CORE R8A7795_CLK_S3D1>,
> > >                        <&scif_clk>;
> > >               ...
> > > (A)           status = "reserved";
> > >       };
> >
> > I have some reservations about whether a reserved node should be touched
> > at all by Linux. I suppose since it is platform specific, it's okay. I
> > don't think we could apply such behavior globally.
> 
> That's an interesting comment, as the issue is that currently Linux
> does touch (resources belonging to) reserved nodes, and this patch
> would prevent doing that for module clock resources;-)

I think I get it.

> 
> The core issue is that Linux distinguishes only between two cases:
>   1. "device is used by Linux" (if a driver is available),
>      as indicated by 'status = "okay"' in DT, or
>   2. "device is unused by Linux".
> On a heterogenous system, the latter actually comprises two cases:
>   2a. "device is unused", or
>   2b. "device is used by another OS running on another CPU core".
> 
> Looking for 'status = "reserved"' allows us to distinguish between 2a
> and 2b, and can prevent disabling clocks that are used by another OS.
> Probably we need a similar solution for power domains.
> 
> Do you have a better or alternative suggestion?

Does the protected-clocks property work? That basically says "don't use
these clks in the OS". The driver implementation would not register
those clks and then the framework would be unaware of their existence,
leading to them never being disabled during late init.

This approach also looks OK to me, basically programmatically creating
the protected-clocks list by parsing DT for reserved consumer nodes and
then figuring out that no consumer exists so we can skip registering the
clk entirely, or add the flag. I'm not sure we want to implement that
policy globally, because maybe someone really wants to disable the clk
still to clean up bootloader state and then let a remoteproc use the clk
later.

Do you want to keep those clks registered with the framework? Is there
any benefit to keeping clks around if linux can't do anything with them?