| 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 |
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 --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; }
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(-)