Clean the following coccicheck warning:

./drivers/staging/pi433/rf69.c:286:1-7: WARNING: do_div() does a
64-by-32 division, please consider using div64_u64 instead.

./drivers/staging/pi433/rf69.c:332:1-7: WARNING: do_div() does a
64-by-32 division, please consider using div64_u64 instead.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
 drivers/staging/pi433/rf69.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index d64df072d8e8..ae4adeb00ce1 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -283,7 +283,7 @@ int rf69_set_deviation(struct spi_device *spi, u32 deviation)
 
 	// calculate register settings
 	f_reg = deviation * factor;
-	do_div(f_reg, f_step);
+	div64_u64(f_reg, f_step);
 
 	msb = (f_reg & 0xff00) >> 8;
 	lsb = (f_reg & 0xff);
@@ -329,7 +329,7 @@ int rf69_set_frequency(struct spi_device *spi, u32 frequency)
 
 	// calculate reg settings
 	f_reg = frequency * factor;
-	do_div(f_reg, f_step);
+	div64_u64(f_reg, f_step);
 
 	msb = (f_reg & 0xff0000) >> 16;
 	mid = (f_reg & 0xff00)   >>  8;
-- 
2.20.1.7.g153144c

From: Jiapeng Chong
> Sent: 21 January 2022 11:50
> Subject: [PATCH] staging: pi433: using div64_u64() instead of do_div()
> 
> Clean the following coccicheck warning:
> 
> ./drivers/staging/pi433/rf69.c:286:1-7: WARNING: do_div() does a
> 64-by-32 division, please consider using div64_u64 instead.

That is one of patchcheck's worse warnings.

You need to check the domain of the divisor, not its type.

do_div() exists to avoid expensive 64bit divides when the
divisor is small.

	David

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

Le 21/01/2022 à 14:34, David Laight a écrit :
> From: Jiapeng Chong
>> Sent: 21 January 2022 11:50
>> Subject: [PATCH] staging: pi433: using div64_u64() instead of do_div()
>>
>> Clean the following coccicheck warning:
>>
>> ./drivers/staging/pi433/rf69.c:286:1-7: WARNING: do_div() does a
>> 64-by-32 division, please consider using div64_u64 instead.
> 
> That is one of patchcheck's worse warnings.
> 
> You need to check the domain of the divisor, not its type.
> 
> do_div() exists to avoid expensive 64bit divides when the
> divisor is small.
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 
> 

Moreover, the patch is broken by itself.

See [1] were it was already reported that do_div() and div64_u64() don't 
have the same calling convention.

Looks that div64_u64() and div64_ul() works the same way.


CJ

[1]: 
https://lore.kernel.org/linux-kernel/20211117112559.jix3hmx7uwqmuryg@pengutronix.de/ 

On Wed, Feb 09, 2022 at 08:15:13PM +0100, Christophe JAILLET wrote:
> Le 21/01/2022 à 14:34, David Laight a écrit :
> > From: Jiapeng Chong
> > > Sent: 21 January 2022 11:50
> > > Subject: [PATCH] staging: pi433: using div64_u64() instead of do_div()
> > > 
> > > Clean the following coccicheck warning:
> > > 
> > > ./drivers/staging/pi433/rf69.c:286:1-7: WARNING: do_div() does a
> > > 64-by-32 division, please consider using div64_u64 instead.
> > 
> > That is one of patchcheck's worse warnings.
> > 
> > You need to check the domain of the divisor, not its type.
> > 
> > do_div() exists to avoid expensive 64bit divides when the
> > divisor is small.
> > 
> > 	David
> > 
> > -
> > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > Registration No: 1397386 (Wales)
> > 
> > 
> 
> Moreover, the patch is broken by itself.
> 
> See [1] were it was already reported that do_div() and div64_u64() don't
> have the same calling convention.
> 
> Looks that div64_u64() and div64_ul() works the same way.

We could mark those as __must_check functions.

regards,
dan carpenter

From: Dan Carpenter
> Sent: 10 February 2022 08:06
> 
> On Wed, Feb 09, 2022 at 08:15:13PM +0100, Christophe JAILLET wrote:
> > Le 21/01/2022 à 14:34, David Laight a écrit :
> > > From: Jiapeng Chong
> > > > Sent: 21 January 2022 11:50
> > > > Subject: [PATCH] staging: pi433: using div64_u64() instead of do_div()
> > > >
> > > > Clean the following coccicheck warning:
> > > >
> > > > ./drivers/staging/pi433/rf69.c:286:1-7: WARNING: do_div() does a
> > > > 64-by-32 division, please consider using div64_u64 instead.
> > >
> > > That is one of patchcheck's worse warnings.
> > >
> > > You need to check the domain of the divisor, not its type.
> > >
> > > do_div() exists to avoid expensive 64bit divides when the
> > > divisor is small.
> > >
> > > 	David
> > >
> > > -
> > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > Registration No: 1397386 (Wales)
> > >
> > >
> >
> > Moreover, the patch is broken by itself.
> >
> > See [1] were it was already reported that do_div() and div64_u64() don't
> > have the same calling convention.
> >
> > Looks that div64_u64() and div64_ul() works the same way.
> 
> We could mark those as __must_check functions.

That, and some kind of AI system to filter out untested patches
from (presumably) students who think that the output from these
tools 'must be right'.

Same for all the patches for using swap(), min() LIST_HEAD() etc.
They are at best churn and make the code harder to read.

	David

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

On Thu, Feb 10, 2022 at 09:21:08AM +0000, David Laight wrote:
> From: Dan Carpenter
> > Sent: 10 February 2022 08:06
> > 
> > On Wed, Feb 09, 2022 at 08:15:13PM +0100, Christophe JAILLET wrote:
> > > Le 21/01/2022 à 14:34, David Laight a écrit :
> > > > From: Jiapeng Chong
> > > > > Sent: 21 January 2022 11:50
> > > > > Subject: [PATCH] staging: pi433: using div64_u64() instead of do_div()
> > > > >
> > > > > Clean the following coccicheck warning:
> > > > >
> > > > > ./drivers/staging/pi433/rf69.c:286:1-7: WARNING: do_div() does a
> > > > > 64-by-32 division, please consider using div64_u64 instead.
> > > >
> > > > That is one of patchcheck's worse warnings.
> > > >
> > > > You need to check the domain of the divisor, not its type.
> > > >
> > > > do_div() exists to avoid expensive 64bit divides when the
> > > > divisor is small.
> > > >
> > > > 	David
> > > >
> > > > -
> > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > Registration No: 1397386 (Wales)
> > > >
> > > >
> > >
> > > Moreover, the patch is broken by itself.
> > >
> > > See [1] were it was already reported that do_div() and div64_u64() don't
> > > have the same calling convention.
> > >
> > > Looks that div64_u64() and div64_ul() works the same way.
> > 
> > We could mark those as __must_check functions.
> 
> That, and some kind of AI system to filter out untested patches
> from (presumably) students who think that the output from these
> tools 'must be right'.
> 
> Same for all the patches for using swap(), min() LIST_HEAD() etc.
> They are at best churn and make the code harder to read.

Churn is kind of the whole point of staging.  Generally, churn is a net
positive for any subsystem.  It's good to get eyes on the code.

The truth is that I looked at this patch and thought "I don't know
what do_div() does" so I moved on.  This is the first time we've ever
recieved a staging patch to convert do_div().  Next time we will all
know the issues with do_div() better.

Adding a __must_check is a good safety measure as well.

I've looked at adding a Smatch check for ignoring the returns for
functions which have no side effects but I've never completed that
work...  :(

regards,
dan carpenter