Re: [PATCH RFC 2/6] gpio: mvebu: honour EPROBE_DEFER for devm_clk_get()

[Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>]

Hello Russell,

On Sun, Mar 29, 2020 at 02:34:00PM +0100, Russell King - ARM Linux admin wrote:
> On Sun, Mar 29, 2020 at 03:16:59PM +0200, Uwe Kleine-König wrote:
> > On Sun, Mar 29, 2020 at 11:48:09AM +0100, Russell King wrote:
> > > diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> > > index fa5641615db6..ee13b11c5298 100644
> > > --- a/drivers/gpio/gpio-mvebu.c
> > > +++ b/drivers/gpio/gpio-mvebu.c
> > > @@ -1132,6 +1132,9 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
> > >  	}
> > >  
> > >  	mvchip->clk = devm_clk_get(&pdev->dev, NULL);
> > > +	if (mvchip->clk == ERR_PTR(-EPROBE_DEFER))
> > > +		return -EPROBE_DEFER;
> > > +
> > >  	/* Not all SoCs require a clock.*/
> > >  	if (!IS_ERR(mvchip->clk))
> > >  		clk_prepare_enable(mvchip->clk);
> > 
> > I'd say the following is the right thing to do here:
> > 
> > 	mvchip->clk = devm_clk_get_optional(...);
> > 	if (IS_ERR(mvchip->clk))
> > 		return ...
> 
> It's not that simple.  The clock is required for Armada 370, and is
> optional for Armada 8040.

I'd say it is still the right approach here. On Armada 370 the dtb then
has a clk and on Armada 8040 it doesn't. So if with
devm_clk_get_optional() something goes wrong that's because the dtb is
wrong. And in fact the handling is even better than with your suggested
patch as every error (but EPROBE_DEFER) is ignored instead of passed to
the caller with your (and the existing) approach.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |