diff mbox series

pwm: meson: Simplify probe function with dev_err_probe()

Message ID 20220818072234.9640-1-zhaoxiao@uniontech.com (mailing list archive)
State New, archived
Headers show
Series pwm: meson: Simplify probe function with dev_err_probe() | expand

Commit Message

zhaoxiao Aug. 18, 2022, 7:22 a.m. UTC
Switch to dev_err_probe() to remove all dev_err() -> return repeated
patterns, simplifying and shortening the probe function.

Signed-off-by: zhaoxiao <zhaoxiao@uniontech.com>
---
 drivers/pwm/pwm-meson.c | 53 ++++++++++++++---------------------------
 1 file changed, 18 insertions(+), 35 deletions(-)

Comments

Uwe Kleine-König Aug. 18, 2022, 7:45 a.m. UTC | #1
On Thu, Aug 18, 2022 at 03:22:34PM +0800, zhaoxiao wrote:
> Switch to dev_err_probe() to remove all dev_err() -> return repeated
> patterns, simplifying and shortening the probe function.
> 
> Signed-off-by: zhaoxiao <zhaoxiao@uniontech.com>
> ---
>  drivers/pwm/pwm-meson.c | 53 ++++++++++++++---------------------------
>  1 file changed, 18 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index 57112f438c6d..43ce8d9a33d2 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -126,20 +126,16 @@ static int meson_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
>  
>  	if (channel->clk_parent) {
>  		err = clk_set_parent(channel->clk, channel->clk_parent);
> -		if (err < 0) {
> -			dev_err(dev, "failed to set parent %s for %s: %d\n",
> +		if (err < 0)
> +			return dev_err_probe(dev, err, "failed to set parent %s for %s\n",
>  				__clk_get_name(channel->clk_parent),
> -				__clk_get_name(channel->clk), err);
> -			return err;
> -		}
> +				__clk_get_name(channel->clk));
>  	}
>  
>  	err = clk_prepare_enable(channel->clk);
> -	if (err < 0) {
> -		dev_err(dev, "failed to enable clock %s: %d\n",
> -			__clk_get_name(channel->clk), err);
> -		return err;
> -	}
> +	if (err < 0)
> +		return dev_err_probe(dev, err, "failed to enable clock %s\n",
> +			__clk_get_name(channel->clk));

It's wrong to use dev_err_probe in .request(). The function is only
supposed to be used in the .probe() call.

>  	return 0;
>  }
> @@ -166,24 +162,18 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
>  		duty = period - duty;
>  
>  	fin_freq = clk_get_rate(channel->clk);
> -	if (fin_freq == 0) {
> -		dev_err(meson->chip.dev, "invalid source clock frequency\n");
> -		return -EINVAL;
> -	}
> +	if (fin_freq == 0)
> +		return dev_err_probe(meson->chip.dev, -EINVAL, "invalid source clock frequency\n");
>  
>  	dev_dbg(meson->chip.dev, "fin_freq: %lu Hz\n", fin_freq);
>  
>  	pre_div = div64_u64(fin_freq * (u64)period, NSEC_PER_SEC * 0xffffLL);
> -	if (pre_div > MISC_CLK_DIV_MASK) {
> -		dev_err(meson->chip.dev, "unable to get period pre_div\n");
> -		return -EINVAL;
> -	}
> +	if (pre_div > MISC_CLK_DIV_MASK)
> +		return dev_err_probe(meson->chip.dev, -EINVAL, "unable to get period pre_div\n");
>  
>  	cnt = div64_u64(fin_freq * (u64)period, NSEC_PER_SEC * (pre_div + 1));
> -	if (cnt > 0xffff) {
> -		dev_err(meson->chip.dev, "unable to get period cnt\n");
> -		return -EINVAL;
> -	}
> +	if (cnt > 0xffff)
> +		return dev_err_probe(meson->chip.dev, -EINVAL, "unable to get period cnt\n");
>  
>  	dev_dbg(meson->chip.dev, "period=%u pre_div=%u cnt=%u\n", period,
>  		pre_div, cnt);
> @@ -200,10 +190,8 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
>  		/* Then check is we can have the duty with the same pre_div */
>  		duty_cnt = div64_u64(fin_freq * (u64)duty,
>  				     NSEC_PER_SEC * (pre_div + 1));
> -		if (duty_cnt > 0xffff) {
> -			dev_err(meson->chip.dev, "unable to get duty cycle\n");
> -			return -EINVAL;
> -		}
> +		if (duty_cnt > 0xffff)
> +			return dev_err_probe(meson->chip.dev, -EINVAL, "unable to get duty cycle\n");
>  
>  		dev_dbg(meson->chip.dev, "duty=%u pre_div=%u duty_cnt=%u\n",
>  			duty, pre_div, duty_cnt);
> @@ -509,11 +497,8 @@ static int meson_pwm_init_channels(struct meson_pwm *meson)
>  		channel->mux.hw.init = &init;
>  
>  		channel->clk = devm_clk_register(dev, &channel->mux.hw);
> -		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);

This is the first conversion that is OK, as meson_pwm_init_channels() is
(only) called by meson_pwm_probe().

>  
>  		snprintf(name, sizeof(name), "clkin%u", i);

Here follows another return that could benefit for a conversion to
dev_err_probe. Currently it only returns an error code and doesn't emit
a message, but a message would be good.

> @@ -550,10 +535,8 @@ 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 hunk is fine.

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 57112f438c6d..43ce8d9a33d2 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -126,20 +126,16 @@  static int meson_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 
 	if (channel->clk_parent) {
 		err = clk_set_parent(channel->clk, channel->clk_parent);
-		if (err < 0) {
-			dev_err(dev, "failed to set parent %s for %s: %d\n",
+		if (err < 0)
+			return dev_err_probe(dev, err, "failed to set parent %s for %s\n",
 				__clk_get_name(channel->clk_parent),
-				__clk_get_name(channel->clk), err);
-			return err;
-		}
+				__clk_get_name(channel->clk));
 	}
 
 	err = clk_prepare_enable(channel->clk);
-	if (err < 0) {
-		dev_err(dev, "failed to enable clock %s: %d\n",
-			__clk_get_name(channel->clk), err);
-		return err;
-	}
+	if (err < 0)
+		return dev_err_probe(dev, err, "failed to enable clock %s\n",
+			__clk_get_name(channel->clk));
 
 	return 0;
 }
@@ -166,24 +162,18 @@  static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
 		duty = period - duty;
 
 	fin_freq = clk_get_rate(channel->clk);
-	if (fin_freq == 0) {
-		dev_err(meson->chip.dev, "invalid source clock frequency\n");
-		return -EINVAL;
-	}
+	if (fin_freq == 0)
+		return dev_err_probe(meson->chip.dev, -EINVAL, "invalid source clock frequency\n");
 
 	dev_dbg(meson->chip.dev, "fin_freq: %lu Hz\n", fin_freq);
 
 	pre_div = div64_u64(fin_freq * (u64)period, NSEC_PER_SEC * 0xffffLL);
-	if (pre_div > MISC_CLK_DIV_MASK) {
-		dev_err(meson->chip.dev, "unable to get period pre_div\n");
-		return -EINVAL;
-	}
+	if (pre_div > MISC_CLK_DIV_MASK)
+		return dev_err_probe(meson->chip.dev, -EINVAL, "unable to get period pre_div\n");
 
 	cnt = div64_u64(fin_freq * (u64)period, NSEC_PER_SEC * (pre_div + 1));
-	if (cnt > 0xffff) {
-		dev_err(meson->chip.dev, "unable to get period cnt\n");
-		return -EINVAL;
-	}
+	if (cnt > 0xffff)
+		return dev_err_probe(meson->chip.dev, -EINVAL, "unable to get period cnt\n");
 
 	dev_dbg(meson->chip.dev, "period=%u pre_div=%u cnt=%u\n", period,
 		pre_div, cnt);
@@ -200,10 +190,8 @@  static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
 		/* Then check is we can have the duty with the same pre_div */
 		duty_cnt = div64_u64(fin_freq * (u64)duty,
 				     NSEC_PER_SEC * (pre_div + 1));
-		if (duty_cnt > 0xffff) {
-			dev_err(meson->chip.dev, "unable to get duty cycle\n");
-			return -EINVAL;
-		}
+		if (duty_cnt > 0xffff)
+			return dev_err_probe(meson->chip.dev, -EINVAL, "unable to get duty cycle\n");
 
 		dev_dbg(meson->chip.dev, "duty=%u pre_div=%u duty_cnt=%u\n",
 			duty, pre_div, duty_cnt);
@@ -509,11 +497,8 @@  static int meson_pwm_init_channels(struct meson_pwm *meson)
 		channel->mux.hw.init = &init;
 
 		channel->clk = devm_clk_register(dev, &channel->mux.hw);
-		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);
 
 		snprintf(name, sizeof(name), "clkin%u", i);
 
@@ -550,10 +535,8 @@  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;
 }