Re: [PATCH] pwm: meson: Simplify using dev_err_probe()

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3429 bytes --]

On Wed, Dec 06, 2023 at 10:48:18PM +0100, Uwe Kleine-König wrote:
> Using dev_err_probe() emitting an error message mentioning a return
> value and returning that value can be done in a single statement. Make
> use of that to simplify the probe part of the driver. This has the
> additional advantage to emit the symbolic name for the error instead of
> the integer error value.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> while trying to understand Jerome's series[1] I noticed this patch
> opportunity.
> 
> Best regards
> Uwe
> 
> [1] https://lore.kernel.org/linux-pwm/20231129134004.3642121-1-jbrunet@baylibre.com
> 
>  drivers/pwm/pwm-meson.c | 35 ++++++++++++++---------------------
>  1 file changed, 14 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index 5bea53243ed2..2971bbf3b5e7 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -468,10 +468,9 @@ static int meson_pwm_init_channels(struct meson_pwm *meson)
>  		channel->mux.hw.init = &init;
>  
>  		err = devm_clk_hw_register(dev, &channel->mux.hw);
> -		if (err) {
> -			dev_err(dev, "failed to register %s: %d\n", name, err);
> -			return err;
> -		}
> +		if (err)
> +			return dev_err_probe(dev, err,
> +					     "failed to register %s\n", name);
>  
>  		snprintf(name, sizeof(name), "%s#div%u", dev_name(dev), i);
>  
> @@ -491,10 +490,9 @@ static int meson_pwm_init_channels(struct meson_pwm *meson)
>  		channel->div.lock = &meson->lock;
>  
>  		err = devm_clk_hw_register(dev, &channel->div.hw);
> -		if (err) {
> -			dev_err(dev, "failed to register %s: %d\n", name, err);
> -			return err;
> -		}
> +		if (err)
> +			return dev_err_probe(dev, err,
> +					     "failed to register %s\n", name);
>  
>  		snprintf(name, sizeof(name), "%s#gate%u", dev_name(dev), i);
>  
> @@ -513,17 +511,13 @@ static int meson_pwm_init_channels(struct meson_pwm *meson)
>  		channel->gate.lock = &meson->lock;
>  
>  		err = devm_clk_hw_register(dev, &channel->gate.hw);
> -		if (err) {
> -			dev_err(dev, "failed to register %s: %d\n", name, err);
> -			return err;
> -		}
> +		if (err)
> +			return dev_err_probe(dev, err, "failed to register %s\n", name);
>  
>  		channel->clk = devm_clk_hw_get_clk(dev, &channel->gate.hw, NULL);
> -		if (IS_ERR(channel->clk)) {
> -			err = PTR_ERR(channel->clk);
> -			dev_err(dev, "failed to register %s: %d\n", name, err);
> -			return err;
> -		}
> +		if (IS_ERR(channel->clk))
> +			return dev_err_probe(dev, PTR_ERR(channel->clk),
> +					     "failed to register %s\n", name);
>  	}
>  
>  	return 0;
> @@ -554,10 +548,9 @@ static int meson_pwm_probe(struct platform_device *pdev)
>  		return err;
>  
>  	err = devm_pwmchip_add(&pdev->dev, &meson->chip);
> -	if (err < 0) {
> -		dev_err(&pdev->dev, "failed to register PWM chip: %d\n", err);
> -		return err;
> -	}
> +	if (err < 0)
> +		return dev_err_probe(&pdev->dev, err,
> +				     "failed to register PWM chip\n");
>  
>  	return 0;
>  }

This is a lot of churn for very little gain. None of these functions are
ever going to return -EPROBE_DEFER. And yes, I know that function's doc
says that it is "deemed acceptable to use" elsewhere. However, the
existing error messages are just fine, no need to churn just for the
sake of it.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel