[PATCH] pwm: use div64_u64() instead of do_div()

[PATCH] pwm: use div64_u64() instead of do_div()

[Qing Wang]

From: Wang Qing <wangqing@vivo.com>

do_div() does a 64-by-32 division.
When the divisor is u64, do_div() truncates it to 32 bits, this means it
can test non-zero and be truncated to zero for division.

fix do_div.cocci warning:
do_div() does a 64-by-32 division, please consider using div64_u64 instead.

Signed-off-by: Wang Qing <wangqing@vivo.com>
---
 drivers/pwm/pwm-berlin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-berlin.c b/drivers/pwm/pwm-berlin.c
index e157273..15b10cb3
--- a/drivers/pwm/pwm-berlin.c
+++ b/drivers/pwm/pwm-berlin.c
@@ -109,7 +109,7 @@ static int berlin_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	period = cycles;
 	cycles *= duty_ns;
-	do_div(cycles, period_ns);
+	div64_u64(cycles, period_ns);
 	duty = cycles;
 
 	value = berlin_pwm_readl(bpc, pwm->hwpwm, BERLIN_PWM_CONTROL);
-- 
2.7.4

Re: [PATCH] pwm: use div64_u64() instead of do_div()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1313 bytes --]

Hello,

On Wed, Feb 09, 2022 at 12:39:58AM -0800, Qing Wang wrote:
> From: Wang Qing <wangqing@vivo.com>
> 
> do_div() does a 64-by-32 division.
> When the divisor is u64, do_div() truncates it to 32 bits, this means it
> can test non-zero and be truncated to zero for division.
> 
> fix do_div.cocci warning:
> do_div() does a 64-by-32 division, please consider using div64_u64 instead.
> 
> Signed-off-by: Wang Qing <wangqing@vivo.com>
> ---
>  drivers/pwm/pwm-berlin.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-berlin.c b/drivers/pwm/pwm-berlin.c
> index e157273..15b10cb3
> --- a/drivers/pwm/pwm-berlin.c
> +++ b/drivers/pwm/pwm-berlin.c
> @@ -109,7 +109,7 @@ static int berlin_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  
>  	period = cycles;
>  	cycles *= duty_ns;
> -	do_div(cycles, period_ns);
> +	div64_u64(cycles, period_ns);

This is wrong, div64_u64() has a different calling convention than do_div().

The issue however is real. Please add 

Fixes: 30dffb42fcd4 ("pwm: berlin: Implement .apply() callback")

to your v2.

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 --]

RE: [PATCH] pwm: use div64_u64() instead of do_div()

[David Laight]

From: Uwe Kleine-König
> Sent: 09 February 2022 15:26
...
> > -	do_div(cycles, period_ns);
> > +	div64_u64(cycles, period_ns);
> 
> This is wrong, div64_u64() has a different calling convention than do_div().
> 
> The issue however is real. Please add

Not really although I can't see a check I'd assume that period_ns
is expected to be much less than a second - so well under 32 bits
There is certainly a general expectation that multiplying by
other 'largish' values won't exceed 64 bits.

Plausible the pwm 'period' should actually be a u32.
But then care would be needed to ensure the multiplies have
64bit results.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] pwm: use div64_u64() instead of do_div()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1248 bytes --]

On Thu, Feb 10, 2022 at 09:48:44AM +0000, David Laight wrote:
> From: Uwe Kleine-König
> > Sent: 09 February 2022 15:26
> ...
> > > -	do_div(cycles, period_ns);
> > > +	div64_u64(cycles, period_ns);
> > 
> > This is wrong, div64_u64() has a different calling convention than do_div().
> > 
> > The issue however is real. Please add
> 
> Not really although I can't see a check I'd assume that period_ns
> is expected to be much less than a second - so well under 32 bits
> There is certainly a general expectation that multiplying by
> other 'largish' values won't exceed 64 bits.

I'd consider such expectations a bug and hope to catch this type of
problem for new drivers. However I'm not surprised if you can point out
several such problems in the code base. Please fix at will :-)

> Plausible the pwm 'period' should actually be a u32.
> But then care would be needed to ensure the multiplies have
> 64bit results.

There are definitely consumers expecting to be able to set bigger
periods, see a9d887dc1c60ed67f2271d66560cdcf864c4a578.

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 --]