Re: [PATCH 1/3] clk: ignore of_device_ensure_probed error in clock lookup

[Lucas Stach <dev@lynxeye.de>]

Hi Ahmad,

Am Montag, dem 17.01.2022 um 06:12 +0100 schrieb Ahmad Fatoum:
> Hello Lucas,
> 
> On 16.01.22 22:32, Lucas Stach wrote:
> > For CLK_OF_DECLARE clocks there is no driver that is bound to the device,
> > so the lookup fails before even trying to find the provider, breaking
> > the parent_ready() logic used when initializing the declared providers.
> > 
> > Ignore the return code from of_device_ensure_probed to allow the lookup
> > to proceed as usual. If of_device_ensure_probed the lookup will also fail,
> > as no provider will be found.
> 
> This seems to be an alternate fix for the issue addressed by:
> bd516e38dd14 ("clk: handle CLK_OF_DECLARE in deep probe")
> 
> What do you think?

Yes, it seems to be aimed at the same issue. However, your fix in
bd516e38dd14 doesn't fix my specific issue, as we still don't get the
correct -EPROBE_DEFER error code for the logic in parent_ready() in
drivers/clk/clk.c to work.

I guess we could even revert your fix if we agree that ignoring the
return code here is the right thing to do? Attaching a dummy driver 
doesn't seem to be totally correct, as I think it isn't prohibited to
have both a CLK_OF_DECLARE clock controller and a real platform driver
for other uses attached to the same OF node. While I'm not aware of any
clock controllers where this would be the case, I have seen such
constructs with IRQ controllers.

Regards,
Lucas

> 
> > 
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> >  drivers/clk/clk.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 189c9c62df5c..a1d1d7f1a467 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -643,11 +643,9 @@ struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
> >  {
> >  	struct of_clk_provider *provider;
> >  	struct clk *clk = ERR_PTR(-EPROBE_DEFER);
> > -	int ret;
> >  
> > -	ret = of_device_ensure_probed(clkspec->np);
> > -	if (ret)
> > -		return ERR_PTR(ret);
> > +	/* Ignore error, as CLK_OF_DECLARE clocks have no proper driver. */
> > +	of_device_ensure_probed(clkspec->np);
> >  
> >  	/* Check if we have such a provider in our array */
> >  	list_for_each_entry(provider, &of_clk_providers, link) {
> > 
> 
> 



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox