* [PATCH 1/2] pwm: lpc18xx: Convert to use dev_err_probe() @ 2022-07-12 16:15 ` Uwe Kleine-König 0 siblings, 0 replies; 6+ messages in thread From: Uwe Kleine-König @ 2022-07-12 16:15 UTC (permalink / raw) To: Thierry Reding, Lee Jones Cc: Vladimir Zapolskiy, Ariel D'Alessandro, linux-pwm, linux-arm-kernel, kernel This has various upsides: - It emits the symbolic name of the error code - It is silent in the EPROBE_DEFER case and properly sets the defer reason - It reduces the number of code lines slightly Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/pwm/pwm-lpc18xx-sct.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c index 272e0b5d01b8..9bb2693cece3 100644 --- a/drivers/pwm/pwm-lpc18xx-sct.c +++ b/drivers/pwm/pwm-lpc18xx-sct.c @@ -359,21 +359,19 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev) return PTR_ERR(lpc18xx_pwm->base); lpc18xx_pwm->pwm_clk = devm_clk_get(&pdev->dev, "pwm"); - if (IS_ERR(lpc18xx_pwm->pwm_clk)) { - dev_err(&pdev->dev, "failed to get pwm clock\n"); - return PTR_ERR(lpc18xx_pwm->pwm_clk); - } + if (IS_ERR(lpc18xx_pwm->pwm_clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(lpc18xx_pwm->pwm_clk), + "failed to get pwm clock\n"); ret = clk_prepare_enable(lpc18xx_pwm->pwm_clk); - if (ret < 0) { - dev_err(&pdev->dev, "could not prepare or enable pwm clock\n"); - return ret; - } + if (ret < 0) + return dev_err_probe(&pdev->dev, ret, + "could not prepare or enable pwm clock\n"); lpc18xx_pwm->clk_rate = clk_get_rate(lpc18xx_pwm->pwm_clk); if (!lpc18xx_pwm->clk_rate) { - dev_err(&pdev->dev, "pwm clock has no frequency\n"); - ret = -EINVAL; + ret = dev_err_probe(&pdev->dev, + -EINVAL, "pwm clock has no frequency\n"); goto disable_pwmclk; } @@ -423,7 +421,7 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev) ret = pwmchip_add(&lpc18xx_pwm->chip); if (ret < 0) { - dev_err(&pdev->dev, "pwmchip_add failed: %d\n", ret); + dev_err_probe(&pdev->dev, ret, "pwmchip_add failed\n"); goto disable_pwmclk; } base-commit: 394b517585da9fbb2eea2f2103ff47d37321e976 -- 2.36.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/2] pwm: lpc18xx: Convert to use dev_err_probe() @ 2022-07-12 16:15 ` Uwe Kleine-König 0 siblings, 0 replies; 6+ messages in thread From: Uwe Kleine-König @ 2022-07-12 16:15 UTC (permalink / raw) To: Thierry Reding, Lee Jones Cc: Vladimir Zapolskiy, Ariel D'Alessandro, linux-pwm, linux-arm-kernel, kernel This has various upsides: - It emits the symbolic name of the error code - It is silent in the EPROBE_DEFER case and properly sets the defer reason - It reduces the number of code lines slightly Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/pwm/pwm-lpc18xx-sct.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c index 272e0b5d01b8..9bb2693cece3 100644 --- a/drivers/pwm/pwm-lpc18xx-sct.c +++ b/drivers/pwm/pwm-lpc18xx-sct.c @@ -359,21 +359,19 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev) return PTR_ERR(lpc18xx_pwm->base); lpc18xx_pwm->pwm_clk = devm_clk_get(&pdev->dev, "pwm"); - if (IS_ERR(lpc18xx_pwm->pwm_clk)) { - dev_err(&pdev->dev, "failed to get pwm clock\n"); - return PTR_ERR(lpc18xx_pwm->pwm_clk); - } + if (IS_ERR(lpc18xx_pwm->pwm_clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(lpc18xx_pwm->pwm_clk), + "failed to get pwm clock\n"); ret = clk_prepare_enable(lpc18xx_pwm->pwm_clk); - if (ret < 0) { - dev_err(&pdev->dev, "could not prepare or enable pwm clock\n"); - return ret; - } + if (ret < 0) + return dev_err_probe(&pdev->dev, ret, + "could not prepare or enable pwm clock\n"); lpc18xx_pwm->clk_rate = clk_get_rate(lpc18xx_pwm->pwm_clk); if (!lpc18xx_pwm->clk_rate) { - dev_err(&pdev->dev, "pwm clock has no frequency\n"); - ret = -EINVAL; + ret = dev_err_probe(&pdev->dev, + -EINVAL, "pwm clock has no frequency\n"); goto disable_pwmclk; } @@ -423,7 +421,7 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev) ret = pwmchip_add(&lpc18xx_pwm->chip); if (ret < 0) { - dev_err(&pdev->dev, "pwmchip_add failed: %d\n", ret); + dev_err_probe(&pdev->dev, ret, "pwmchip_add failed\n"); goto disable_pwmclk; } base-commit: 394b517585da9fbb2eea2f2103ff47d37321e976 -- 2.36.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
* [PATCH 2/2] pwm: lpc18xx: Fix period handling 2022-07-12 16:15 ` Uwe Kleine-König @ 2022-07-12 16:15 ` Uwe Kleine-König -1 siblings, 0 replies; 6+ messages in thread From: Uwe Kleine-König @ 2022-07-12 16:15 UTC (permalink / raw) To: Thierry Reding, Lee Jones Cc: Vladimir Zapolskiy, Ariel D'Alessandro, linux-pwm, linux-arm-kernel, kernel The calculation: val = (u64)NSEC_PER_SEC * LPC18XX_PWM_TIMER_MAX; do_div(val, lpc18xx_pwm->clk_rate); lpc18xx_pwm->max_period_ns = val; is bogus because with NSEC_PER_SEC = 1000000000, LPC18XX_PWM_TIMER_MAX = 0xffffffff and clk_rate < NSEC_PER_SEC this overflows the (on lpc18xx (i.e. ARM32) 32 bit wide) unsigned int .max_period_ns. This results (dependant of the actual clk rate) in an arbitrary limitation of the maximal period. E.g. for clkrate = 333333333 (Hz) we get max_period_ns = 9 instead of 12884901897. So make .max_period_ns an u64 and pass period and duty as u64 to not discard relevant digits. And also make use of mul_u64_u64_div_u64() which prevents all overflows assuming clk_rate < NSEC_PER_SEC. Fixes: 841e6f90bb78 ("pwm: NXP LPC18xx PWM/SCT driver") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/pwm/pwm-lpc18xx-sct.c | 47 +++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c index 9bb2693cece3..763f2e3a146d 100644 --- a/drivers/pwm/pwm-lpc18xx-sct.c +++ b/drivers/pwm/pwm-lpc18xx-sct.c @@ -98,7 +98,7 @@ struct lpc18xx_pwm_chip { unsigned long clk_rate; unsigned int period_ns; unsigned int min_period_ns; - unsigned int max_period_ns; + u64 max_period_ns; unsigned int period_event; unsigned long event_map; struct mutex res_lock; @@ -145,40 +145,48 @@ static void lpc18xx_pwm_set_conflict_res(struct lpc18xx_pwm_chip *lpc18xx_pwm, mutex_unlock(&lpc18xx_pwm->res_lock); } -static void lpc18xx_pwm_config_period(struct pwm_chip *chip, int period_ns) +static void lpc18xx_pwm_config_period(struct pwm_chip *chip, u64 period_ns) { struct lpc18xx_pwm_chip *lpc18xx_pwm = to_lpc18xx_pwm_chip(chip); - u64 val; + u32 val; - val = (u64)period_ns * lpc18xx_pwm->clk_rate; - do_div(val, NSEC_PER_SEC); + /* + * With clk_rate < NSEC_PER_SEC this cannot overflow. + * With period_ns < max_period_ns this also fits into an u32. + * As period_ns >= min_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, lpc18xx_pwm->clk_rate); + * we have val >= 1. + */ + val = mul_u64_u64_div_u64(period_ns, lpc18xx_pwm->clk_rate, NSEC_PER_SEC); lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_MATCH(lpc18xx_pwm->period_event), - (u32)val - 1); + val - 1); lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_MATCHREL(lpc18xx_pwm->period_event), - (u32)val - 1); + val - 1); } static void lpc18xx_pwm_config_duty(struct pwm_chip *chip, - struct pwm_device *pwm, int duty_ns) + struct pwm_device *pwm, u64 duty_ns) { struct lpc18xx_pwm_chip *lpc18xx_pwm = to_lpc18xx_pwm_chip(chip); struct lpc18xx_pwm_data *lpc18xx_data = &lpc18xx_pwm->channeldata[pwm->hwpwm]; - u64 val; + u32 val; - val = (u64)duty_ns * lpc18xx_pwm->clk_rate; - do_div(val, NSEC_PER_SEC); + /* + * With clk_rate < NSEC_PER_SEC this cannot overflow. + * With duty_ns <= period_ns < max_period_ns this also fits into an u32. + */ + val = mul_u64_u64_div_u64(duty_ns, lpc18xx_pwm->clk_rate, NSEC_PER_SEC); lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_MATCH(lpc18xx_data->duty_event), - (u32)val); + val); lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_MATCHREL(lpc18xx_data->duty_event), - (u32)val); + val); } static int lpc18xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, @@ -375,12 +383,19 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev) goto disable_pwmclk; } + /* + * If clkrate is too fast, the calculations in .apply() might overflow. + */ + if (lpc18xx_pwm->clk_rate > NSEC_PER_SEC) { + ret = dev_err_probe(&pdev->dev, -EINVAL, "pwm clock to fast\n"); + goto disable_pwmclk; + } + mutex_init(&lpc18xx_pwm->res_lock); mutex_init(&lpc18xx_pwm->period_lock); - val = (u64)NSEC_PER_SEC * LPC18XX_PWM_TIMER_MAX; - do_div(val, lpc18xx_pwm->clk_rate); - lpc18xx_pwm->max_period_ns = val; + lpc18xx_pwm->max_period_ns = + mul_u64_u64_div_u64(NSEC_PER_SEC, LPC18XX_PWM_TIMER_MAX, lpc18xx_pwm->clk_rate); lpc18xx_pwm->min_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, lpc18xx_pwm->clk_rate); -- 2.36.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] pwm: lpc18xx: Fix period handling @ 2022-07-12 16:15 ` Uwe Kleine-König 0 siblings, 0 replies; 6+ messages in thread From: Uwe Kleine-König @ 2022-07-12 16:15 UTC (permalink / raw) To: Thierry Reding, Lee Jones Cc: Vladimir Zapolskiy, Ariel D'Alessandro, linux-pwm, linux-arm-kernel, kernel The calculation: val = (u64)NSEC_PER_SEC * LPC18XX_PWM_TIMER_MAX; do_div(val, lpc18xx_pwm->clk_rate); lpc18xx_pwm->max_period_ns = val; is bogus because with NSEC_PER_SEC = 1000000000, LPC18XX_PWM_TIMER_MAX = 0xffffffff and clk_rate < NSEC_PER_SEC this overflows the (on lpc18xx (i.e. ARM32) 32 bit wide) unsigned int .max_period_ns. This results (dependant of the actual clk rate) in an arbitrary limitation of the maximal period. E.g. for clkrate = 333333333 (Hz) we get max_period_ns = 9 instead of 12884901897. So make .max_period_ns an u64 and pass period and duty as u64 to not discard relevant digits. And also make use of mul_u64_u64_div_u64() which prevents all overflows assuming clk_rate < NSEC_PER_SEC. Fixes: 841e6f90bb78 ("pwm: NXP LPC18xx PWM/SCT driver") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/pwm/pwm-lpc18xx-sct.c | 47 +++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c index 9bb2693cece3..763f2e3a146d 100644 --- a/drivers/pwm/pwm-lpc18xx-sct.c +++ b/drivers/pwm/pwm-lpc18xx-sct.c @@ -98,7 +98,7 @@ struct lpc18xx_pwm_chip { unsigned long clk_rate; unsigned int period_ns; unsigned int min_period_ns; - unsigned int max_period_ns; + u64 max_period_ns; unsigned int period_event; unsigned long event_map; struct mutex res_lock; @@ -145,40 +145,48 @@ static void lpc18xx_pwm_set_conflict_res(struct lpc18xx_pwm_chip *lpc18xx_pwm, mutex_unlock(&lpc18xx_pwm->res_lock); } -static void lpc18xx_pwm_config_period(struct pwm_chip *chip, int period_ns) +static void lpc18xx_pwm_config_period(struct pwm_chip *chip, u64 period_ns) { struct lpc18xx_pwm_chip *lpc18xx_pwm = to_lpc18xx_pwm_chip(chip); - u64 val; + u32 val; - val = (u64)period_ns * lpc18xx_pwm->clk_rate; - do_div(val, NSEC_PER_SEC); + /* + * With clk_rate < NSEC_PER_SEC this cannot overflow. + * With period_ns < max_period_ns this also fits into an u32. + * As period_ns >= min_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, lpc18xx_pwm->clk_rate); + * we have val >= 1. + */ + val = mul_u64_u64_div_u64(period_ns, lpc18xx_pwm->clk_rate, NSEC_PER_SEC); lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_MATCH(lpc18xx_pwm->period_event), - (u32)val - 1); + val - 1); lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_MATCHREL(lpc18xx_pwm->period_event), - (u32)val - 1); + val - 1); } static void lpc18xx_pwm_config_duty(struct pwm_chip *chip, - struct pwm_device *pwm, int duty_ns) + struct pwm_device *pwm, u64 duty_ns) { struct lpc18xx_pwm_chip *lpc18xx_pwm = to_lpc18xx_pwm_chip(chip); struct lpc18xx_pwm_data *lpc18xx_data = &lpc18xx_pwm->channeldata[pwm->hwpwm]; - u64 val; + u32 val; - val = (u64)duty_ns * lpc18xx_pwm->clk_rate; - do_div(val, NSEC_PER_SEC); + /* + * With clk_rate < NSEC_PER_SEC this cannot overflow. + * With duty_ns <= period_ns < max_period_ns this also fits into an u32. + */ + val = mul_u64_u64_div_u64(duty_ns, lpc18xx_pwm->clk_rate, NSEC_PER_SEC); lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_MATCH(lpc18xx_data->duty_event), - (u32)val); + val); lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_MATCHREL(lpc18xx_data->duty_event), - (u32)val); + val); } static int lpc18xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, @@ -375,12 +383,19 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev) goto disable_pwmclk; } + /* + * If clkrate is too fast, the calculations in .apply() might overflow. + */ + if (lpc18xx_pwm->clk_rate > NSEC_PER_SEC) { + ret = dev_err_probe(&pdev->dev, -EINVAL, "pwm clock to fast\n"); + goto disable_pwmclk; + } + mutex_init(&lpc18xx_pwm->res_lock); mutex_init(&lpc18xx_pwm->period_lock); - val = (u64)NSEC_PER_SEC * LPC18XX_PWM_TIMER_MAX; - do_div(val, lpc18xx_pwm->clk_rate); - lpc18xx_pwm->max_period_ns = val; + lpc18xx_pwm->max_period_ns = + mul_u64_u64_div_u64(NSEC_PER_SEC, LPC18XX_PWM_TIMER_MAX, lpc18xx_pwm->clk_rate); lpc18xx_pwm->min_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, lpc18xx_pwm->clk_rate); -- 2.36.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 1/2] pwm: lpc18xx: Convert to use dev_err_probe() 2022-07-12 16:15 ` Uwe Kleine-König @ 2022-07-28 17:22 ` Thierry Reding -1 siblings, 0 replies; 6+ messages in thread From: Thierry Reding @ 2022-07-28 17:22 UTC (permalink / raw) To: Uwe Kleine-König Cc: Lee Jones, Vladimir Zapolskiy, Ariel D'Alessandro, linux-pwm, linux-arm-kernel, kernel [-- Attachment #1: Type: text/plain, Size: 515 bytes --] On Tue, Jul 12, 2022 at 06:15:18PM +0200, Uwe Kleine-König wrote: > This has various upsides: > - It emits the symbolic name of the error code > - It is silent in the EPROBE_DEFER case and properly sets the defer reason > - It reduces the number of code lines slightly > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/pwm/pwm-lpc18xx-sct.c | 20 +++++++++----------- > 1 file changed, 9 insertions(+), 11 deletions(-) Both patches applied, thanks. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] pwm: lpc18xx: Convert to use dev_err_probe() @ 2022-07-28 17:22 ` Thierry Reding 0 siblings, 0 replies; 6+ messages in thread From: Thierry Reding @ 2022-07-28 17:22 UTC (permalink / raw) To: Uwe Kleine-König Cc: Lee Jones, Vladimir Zapolskiy, Ariel D'Alessandro, linux-pwm, linux-arm-kernel, kernel [-- Attachment #1.1: Type: text/plain, Size: 515 bytes --] On Tue, Jul 12, 2022 at 06:15:18PM +0200, Uwe Kleine-König wrote: > This has various upsides: > - It emits the symbolic name of the error code > - It is silent in the EPROBE_DEFER case and properly sets the defer reason > - It reduces the number of code lines slightly > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/pwm/pwm-lpc18xx-sct.c | 20 +++++++++----------- > 1 file changed, 9 insertions(+), 11 deletions(-) Both patches applied, thanks. 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-28 17:24 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-07-12 16:15 [PATCH 1/2] pwm: lpc18xx: Convert to use dev_err_probe() Uwe Kleine-König 2022-07-12 16:15 ` Uwe Kleine-König 2022-07-12 16:15 ` [PATCH 2/2] pwm: lpc18xx: Fix period handling Uwe Kleine-König 2022-07-12 16:15 ` Uwe Kleine-König 2022-07-28 17:22 ` [PATCH 1/2] pwm: lpc18xx: Convert to use dev_err_probe() Thierry Reding 2022-07-28 17:22 ` Thierry Reding
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.