staging: iio: frequency: ad9832: Use div64_ul instead of do_div

Message ID Y1r+EHqpVz/HYrIm@ubunlion
State New
Headers
Series staging: iio: frequency: ad9832: Use div64_ul instead of do_div |

Commit Message

Deepak R Varma Oct. 27, 2022, 9:54 p.m. UTC
  do_div() does a 64-by-32 division. Here the divisor is an unsigned long
which on some platforms is 64 bit wide. So use div64_ul instead of do_div
to avoid a possible truncation. Issue identified using the
coccicheck tool.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 drivers/staging/iio/frequency/ad9832.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--
2.34.1
  

Comments

Sa, Nuno Oct. 28, 2022, 10:11 a.m. UTC | #1
> -----Original Message-----
> From: Deepak R Varma <drv@mailo.com>
> Sent: Thursday, October 27, 2022 11:54 PM
> To: outreachy@lists.linux.dev; Lars-Peter Clausen <lars@metafoo.de>;
> Hennerich, Michael <Michael.Hennerich@analog.com>; Jonathan Cameron
> <jic23@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> linux-iio@vger.kernel.org; linux-staging@lists.linux.dev; linux-
> kernel@vger.kernel.org
> Subject: [PATCH] staging: iio: frequency: ad9832: Use div64_ul instead of
> do_div
> 
> [External]
> 
> do_div() does a 64-by-32 division. Here the divisor is an unsigned long
> which on some platforms is 64 bit wide. So use div64_ul instead of do_div
> to avoid a possible truncation. Issue identified using the
> coccicheck tool.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>
  
Jonathan Cameron Oct. 29, 2022, 12:11 p.m. UTC | #2
On Fri, 28 Oct 2022 10:11:56 +0000
"Sa, Nuno" <Nuno.Sa@analog.com> wrote:

> > -----Original Message-----
> > From: Deepak R Varma <drv@mailo.com>
> > Sent: Thursday, October 27, 2022 11:54 PM
> > To: outreachy@lists.linux.dev; Lars-Peter Clausen <lars@metafoo.de>;
> > Hennerich, Michael <Michael.Hennerich@analog.com>; Jonathan Cameron
> > <jic23@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> > linux-iio@vger.kernel.org; linux-staging@lists.linux.dev; linux-
> > kernel@vger.kernel.org
> > Subject: [PATCH] staging: iio: frequency: ad9832: Use div64_ul instead of
> > do_div
> > 
> > [External]
> > 
> > do_div() does a 64-by-32 division. Here the divisor is an unsigned long
> > which on some platforms is 64 bit wide. So use div64_ul instead of do_div
> > to avoid a possible truncation. Issue identified using the
> > coccicheck tool.
> > 
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---  
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>

As per the email Greg linked to, please take a close look at the surround code
and include analysis of whether the value can actually be greater than 32 bits.
Note that in most cases that would actually mean the code was broken on 32 bit
platforms.
  

Patch

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 6f9eebd6c7ee..a8409f6b1c4c 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -122,8 +122,7 @@  static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
 	unsigned long long freqreg = (u64)fout *
 				     (u64)((u64)1L << AD9832_FREQ_BITS);
-	do_div(freqreg, mclk);
-	return freqreg;
+	return div64_ul(freqreg, mclk);
 }

 static int ad9832_write_frequency(struct ad9832_state *st,