* [PATCH] staging: pi433: using div64_u64() instead of do_div()
@ 2022-01-21 11:49 Jiapeng Chong
2022-01-21 13:34 ` David Laight
0 siblings, 1 reply; 6+ messages in thread
From: Jiapeng Chong @ 2022-01-21 11:49 UTC (permalink / raw)
To: gregkh; +Cc: linux-staging, linux-kernel, Jiapeng Chong, Abaci Robot
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] staging: pi433: using div64_u64() instead of do_div()
2022-01-21 11:49 [PATCH] staging: pi433: using div64_u64() instead of do_div() Jiapeng Chong
@ 2022-01-21 13:34 ` David Laight
2022-02-09 19:15 ` Christophe JAILLET
0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2022-01-21 13:34 UTC (permalink / raw)
To: 'Jiapeng Chong', gregkh@linuxfoundation.org
Cc: linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
Abaci Robot
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: pi433: using div64_u64() instead of do_div()
2022-01-21 13:34 ` David Laight
@ 2022-02-09 19:15 ` Christophe JAILLET
2022-02-10 8:06 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Christophe JAILLET @ 2022-02-09 19:15 UTC (permalink / raw)
To: David Laight, 'Jiapeng Chong', gregkh@linuxfoundation.org
Cc: linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
Abaci Robot
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/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: pi433: using div64_u64() instead of do_div()
2022-02-09 19:15 ` Christophe JAILLET
@ 2022-02-10 8:06 ` Dan Carpenter
2022-02-10 9:21 ` David Laight
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2022-02-10 8:06 UTC (permalink / raw)
To: Christophe JAILLET
Cc: David Laight, 'Jiapeng Chong', gregkh@linuxfoundation.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
Abaci Robot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] staging: pi433: using div64_u64() instead of do_div()
2022-02-10 8:06 ` Dan Carpenter
@ 2022-02-10 9:21 ` David Laight
2022-02-10 10:42 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2022-02-10 9:21 UTC (permalink / raw)
To: 'Dan Carpenter', Christophe JAILLET
Cc: 'Jiapeng Chong', gregkh@linuxfoundation.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
Abaci Robot
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: pi433: using div64_u64() instead of do_div()
2022-02-10 9:21 ` David Laight
@ 2022-02-10 10:42 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2022-02-10 10:42 UTC (permalink / raw)
To: David Laight
Cc: Christophe JAILLET, 'Jiapeng Chong',
gregkh@linuxfoundation.org, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org, Abaci Robot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-10 10:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-21 11:49 [PATCH] staging: pi433: using div64_u64() instead of do_div() Jiapeng Chong
2022-01-21 13:34 ` David Laight
2022-02-09 19:15 ` Christophe JAILLET
2022-02-10 8:06 ` Dan Carpenter
2022-02-10 9:21 ` David Laight
2022-02-10 10:42 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox