Re: [PATCH] clk: fixed: handle failed clk setup

[Emil Abildgaard Svendsen <EMAS@bang-olufsen.dk>]

On Tue, Oct 24, 2023 at 11:41:05AM -0700, Stephen Boyd wrote:
> Quoting Emil Abildgaard Svendsen (2023-10-24 07:54:28)
> > On Mon, Oct 23, 2023 at 08:14:01PM -0700, Stephen Boyd wrote:
> > > Quoting Emil Abildgaard Svendsen (2023-04-14 05:52:16)
> > > > When initializing clock providers "of_clk_init" will try and init
> > > > parents first. But if parent clock is provided by a platform driver it
> > > > can't. Then clocks will be forced on and OF_POPULATED flag will be set
> > > > blindly. So if setup failes e.g. with -EPROBE_DEFER the clock will not
> > > > be probed later on.
> > > > 
> > > > This patch will clear the OF_POPULATED falg if clock setup failes.
> > > 
> > > Are you actually running into a problem with of_clk_init() failing to
> > > register a fixed rate of fixed factor clk from DT?
> 
> Can you answer this question?

I wouldn't call it a problem with of_clk_init() because how does it know
when fixed-factor-clk fails to register a clock when the setup function
returns void? 

> > > I don't understand
> > > why that is happening. I agree it's good hygiene to clear the flag, but
> > > I don't see why this is failing.
> > 
> > The problem is of_clk_init() will not wait for clock providers
> > registered by a platform driver that returns -EPROBE_DEFER. Which can
> > result in final probe to be delayed quite a bit. If this happens and
> > CLK_OF_DECLARE tries to setup a fixed-factor-clock with a deferred
> > clock parent, it will fail.
> 
> Doesn't the 'force' local variable get set to true in this case, and
> then the fixed factor clk is registered anyway? The clk framework tries
> hard to let clks be registered in any order so that parents don't need
> to be registered before children. From what I remember, we want to wait
> for parents because of_clk_set_defaults() may fail otherwise.

Yes, the 'force' local variable get set. But it's the setup function of
fixed-factor-clk that fails to register the clock.

> > Clearing the flag will allow the
> > fixed-factor-clock to be probed after the clock provider has been
> > successfully created.
> 
> Got it. By clock provider you mean the parent, right?

Yes, you're correct.