Re: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2919 bytes --]

On Mon, Nov 22, 2021 at 11:33:27AM +0800, Peng Fan (OSS) wrote:
> + Rob
> 
> On 2021/11/20 20:57, Tom Rini wrote:
> > On Sat, Nov 20, 2021 at 12:10:54PM +0000, Peng Fan (OSS) wrote:
> > > > Subject: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults
> > > > 
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > > 
> > > > Current code has a force clk_set_defaults in multiple stages, U-Boot reuse the
> > > > same device tree and Linux Kernel device tree, but we not register all the clks
> > > > as Linux Kernel, so clk_set_defaults will fail and cause the clk provider
> > > > registeration fail.
> > > > 
> > > > So introduce a new property to ignore the default settings which could be
> > > > used by any node that wanna ignore default settings.
> > > > 
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > > 
> > > > V2:
> > > >   Add R-b tag
> > > >   Tom, Simon
> > > >     After a thought, I think still put it as a u-boot thing. assigned-clock-x is
> > > >     actually Linux specific, however I could not add the new property to Linux,
> > > >     because we are supporting SystemReady-IR, we need the
> > > > assigned-clock-x property
> > > >     in linux working and ignore it in U-Boot.
> > > 
> > > Any more thoughts?
> > 
> > Just my continued request that you treat this as generic and submit the
> > binding upstream so it can be in the device tree for the platform.
> > 
> 
> As Sean said, this is to serve cast that linux and U-Boot use the same
> device tree, I mean U-Boot runtime export device tree to linux for SR-IR
> (system-ready IR) booting.
> 
> Linux needs assigned-clocks to some reason, but U-Boot not need that because
> the driver not added the support or not a must to have that.
> 
> Because assigned-clocks failure in U-Boot will cause probe fail now,
> the device driver will report failure.
> 
> You mean rename this to "ignore-clk-defaults" or keep
> "u-boot,ignore-clk-defauls" or "firmware,ignore-clk-defaults" to linux
> device tree binding?
> 
> I could try to send to linux kernel with "firmware" as a prefix.

What I mean is that first I'm not seeing the description of the property
as being clear enough, either in commit message or the binding itself.
That's why in my mind I keep seeing this as "we set the properties
linux,assigned-clocks and u-boot,ignore-clk-defaults" and I don't know why we
need both.  Is there not something we can do based on seeing
linux,assigned-clocks ?  Showing something that makes use of the property
you're wishing to add would be helpful.  In fact, some specific dts
snippets would be helpful to understand what's going on here.

Second, if we need a new property here then yes, it goes upstream and
you put u-boot,has-not-clk-foo or firmware,has-not-clk-foo or whatever
in the boards dts file.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]