[PATCH v2] staging: iio: frequency: ad9834: Use div64_ul instead of do_div

Deepak R Varma posted 1 patch 3 years, 5 months ago
drivers/staging/iio/frequency/ad9834.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH v2] staging: iio: frequency: ad9834: Use div64_ul instead of do_div
Posted by Deepak R Varma 3 years, 5 months ago
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>
---

Changes in v2:
   1. No functional change.
      Include outreachy mailing list on the to list

 drivers/staging/iio/frequency/ad9834.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index 285df0e489a6..3917a76e7976 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -102,8 +102,7 @@ static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
 	unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);

-	do_div(freqreg, mclk);
-	return freqreg;
+	return div64_ul(freqreg, mclk);
 }

 static int ad9834_write_frequency(struct ad9834_state *st,
--
2.34.1
RE: [PATCH v2] staging: iio: frequency: ad9834: Use div64_ul instead of do_div
Posted by Sa, Nuno 3 years, 5 months ago

> -----Original Message-----
> From: Deepak R Varma <drv@mailo.com>
> Sent: Friday, October 28, 2022 12:00 AM
> 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 v2] staging: iio: frequency: ad9834: 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>
> ---

Clearly, I should have looked better and only reply to this one:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Re: [PATCH v2] staging: iio: frequency: ad9834: Use div64_ul instead of do_div
Posted by Jonathan Cameron 3 years, 5 months ago
On Fri, 28 Oct 2022 10:14:48 +0000
"Sa, Nuno" <Nuno.Sa@analog.com> wrote:

> > -----Original Message-----
> > From: Deepak R Varma <drv@mailo.com>
> > Sent: Friday, October 28, 2022 12:00 AM
> > 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 v2] staging: iio: frequency: ad9834: 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>
> > ---  
> 
> Clearly, I should have looked better and only reply to this one:
> 
> 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.