On Wed, Aug 12, 2020 at 09:20:02AM +0100, Lee Jones wrote:
> On Wed, 12 Aug 2020, Uwe Kleine-König wrote:
> 
> > Add an error message for failure points that currently lack a message
> > and convert dev_err to dev_err_probe() which does the right thing for
> > -EPROBE_DEFER. Also slightly simplify the error handling.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/pwm/pwm-atmel.c | 24 +++++++++++-------------
> >  1 file changed, 11 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> > index 6161e7e3e9ac..aa0b36695dc7 100644
> > --- a/drivers/pwm/pwm-atmel.c
> > +++ b/drivers/pwm/pwm-atmel.c
> > @@ -415,17 +415,18 @@ static int atmel_pwm_probe(struct platform_device *pdev)
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	atmel_pwm->base = devm_ioremap_resource(&pdev->dev, res);
> >  	if (IS_ERR(atmel_pwm->base))
> > -		return PTR_ERR(atmel_pwm->base);
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(atmel_pwm->base),
> > +				     "Failed to remap register space\n");
> 
> This is a regression.
> 
> devm_ioremap_resource() already emits and error message for !-ENOMEM.
> 
> -ENOMEM cases should fail silently.

ah right. Maybe dev_err_probe() should do this right, too?

> >  	atmel_pwm->clk = devm_clk_get(&pdev->dev, NULL);
> >  	if (IS_ERR(atmel_pwm->clk))
> > -		return PTR_ERR(atmel_pwm->clk);
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(atmel_pwm->clk),
> > +				     "Failed to get clock\n");
> 
> Isn't dev_err_probe() only useful for drivers handling -EPROBE_DEFER?

devm_clk_get() might return -EPROBE_DEFER.

> >  	ret = clk_prepare(atmel_pwm->clk);
> > -	if (ret) {
> > -		dev_err(&pdev->dev, "failed to prepare PWM clock\n");
> > -		return ret;
> > -	}
> > +	if (ret)
> > +		return dev_err_probe(&pdev->dev, ret,
> > +				     "Failed to prepare PWM clock\n");
> 
> As above.

I only checked quickly and didn't find an instance where clk_prepare can
return -EPROBE_DEFER, but even if it doesn't it works fine.

Best regards
Uwe

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