* [PATCH] pwm: meson: Simplify probe function with dev_err_probe()
@ 2022-08-18 7:22 ` zhaoxiao
0 siblings, 0 replies; 6+ messages in thread
From: zhaoxiao @ 2022-08-18 7:22 UTC (permalink / raw)
To: thierry.reding, narmstrong, khilman
Cc: u.kleine-koenig, jbrunet, martin.blumenstingl, linux-pwm,
linux-arm-kernel, linux-amlogic, linux-kernel, zhaoxiao
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));
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;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] pwm: meson: Simplify probe function with dev_err_probe()
@ 2022-08-18 7:22 ` zhaoxiao
0 siblings, 0 replies; 6+ messages in thread
From: zhaoxiao @ 2022-08-18 7:22 UTC (permalink / raw)
To: thierry.reding, narmstrong, khilman
Cc: u.kleine-koenig, jbrunet, martin.blumenstingl, linux-pwm,
linux-arm-kernel, linux-amlogic, linux-kernel, zhaoxiao
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));
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;
}
--
2.20.1
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] pwm: meson: Simplify probe function with dev_err_probe()
@ 2022-08-18 7:22 ` zhaoxiao
0 siblings, 0 replies; 6+ messages in thread
From: zhaoxiao @ 2022-08-18 7:22 UTC (permalink / raw)
To: thierry.reding, narmstrong, khilman
Cc: u.kleine-koenig, jbrunet, martin.blumenstingl, linux-pwm,
linux-arm-kernel, linux-amlogic, linux-kernel, zhaoxiao
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));
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;
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] pwm: meson: Simplify probe function with dev_err_probe()
2022-08-18 7:22 ` zhaoxiao
(?)
@ 2022-08-18 7:45 ` Uwe Kleine-König
-1 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2022-08-18 7:45 UTC (permalink / raw)
To: zhaoxiao
Cc: thierry.reding, narmstrong, khilman, jbrunet,
martin.blumenstingl, linux-pwm, linux-arm-kernel, linux-amlogic,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4804 bytes --]
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
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pwm: meson: Simplify probe function with dev_err_probe()
@ 2022-08-18 7:45 ` Uwe Kleine-König
0 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2022-08-18 7:45 UTC (permalink / raw)
To: zhaoxiao
Cc: thierry.reding, narmstrong, khilman, jbrunet,
martin.blumenstingl, linux-pwm, linux-arm-kernel, linux-amlogic,
linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 4804 bytes --]
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
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pwm: meson: Simplify probe function with dev_err_probe()
@ 2022-08-18 7:45 ` Uwe Kleine-König
0 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2022-08-18 7:45 UTC (permalink / raw)
To: zhaoxiao
Cc: thierry.reding, narmstrong, khilman, jbrunet,
martin.blumenstingl, linux-pwm, linux-arm-kernel, linux-amlogic,
linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 4804 bytes --]
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
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-18 7:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-18 7:22 [PATCH] pwm: meson: Simplify probe function with dev_err_probe() zhaoxiao
2022-08-18 7:22 ` zhaoxiao
2022-08-18 7:22 ` zhaoxiao
2022-08-18 7:45 ` Uwe Kleine-König
2022-08-18 7:45 ` Uwe Kleine-König
2022-08-18 7:45 ` Uwe Kleine-König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.