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

[Stephen Boyd <sboyd@kernel.org>]

Quoting Kuninori Morimoto (2023-11-27 23:48:04)
> 
> Hi Stephen
> 
> Thank you for your feedback
> 
> > 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?
> 
> Actually, this idea (= using "protected-clocks" property style) was our 1st
> idea, but I noticed that it is not enough. Because clock driver is possible
> to know which module was not used on Linux, but other driver can't, in this
> idea. For example, power-domain control driver might want to know about it.
> 
> In case of using "protected-clocks" property, we need to have same/similar
> settings on each driver, but in case of using "status = reserved", all
> driver is possible to know it. It has consistent, and no contradict like
> some module was listed as "protected-clocks" on clock / power driver,
> but has "status = ok" on its driver, etc.
> 
> It seems we have different opinions around here ?

I don't really have any opinion here.

> 
> Our other idea was having "unassigned" node instead of
> "status = reserved", like below.
> clock driver checks "unassigned" node's "devices" property, and
> ignore/disable listed device's "clocks".
> 
>         unassigned {
>                 devices = <&scif1>, <&hsusb>;
>         };

This approach looks like the chosen node. I'd say what you have done in
this patch series is better. The protected-clocks property is really
about clks that shouldn't be used by the OS on some board. In your case
it sounds like you want to still be able to read the clk hardware? Does
anything go wrong if you let some consumer get the clk and change
settings? Do you want to prevent that from happening? I'm mostly
thinking it may be useful to have this logic be common in the clk
framework by having the framework search through DT and figure out that
the only consumers are reserved and thus we should treat those clks as
read-only in the framework.