[PATCH] staging: iio: frequency: ad9832/ad9834: use div64_ul instead of do_div

[PATCH] staging: iio: frequency: ad9832/ad9834: use div64_ul instead of do_div

[Mohamad Raizudeen]

Coccinelle warns that do_div() should not be used when the divisor is an unsigned long, because do_div() expects a 32-bit divisor.

In this case, the 'mclk' divisor is an 'unsigned long', which can be 64-bit on certain architectures. The correct function to use here is div64_ul().

Because do_div() modifies the dividend in place, while div64_ul() returns the result, the code was updated to directly return the result of div64_ul. This keeps the math exactly the same but fixes the warning and makes the code safer.

Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c | 3 +--
 drivers/staging/iio/frequency/ad9834.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index b87ea1781b27..91df7ee65bfc 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -115,8 +115,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,
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index bdb2580e29bf..70b310488237 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -101,8 +101,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.53.0

Re: [PATCH] staging: iio: frequency: ad9832/ad9834: use div64_ul instead of do_div

[Joshua Crofts]

On Tue, 30 Jun 2026 08:53:22 +0530
Mohamad Raizudeen <raizudeen.kerneldev@gmail.com> wrote:

> Coccinelle warns that do_div() should not be used when the divisor is an unsigned long, because do_div() expects a 32-bit divisor.
> 
> In this case, the 'mclk' divisor is an 'unsigned long', which can be 64-bit on certain architectures. The correct function to use here is div64_ul().
> 
> Because do_div() modifies the dividend in place, while div64_ul() returns the result, the code was updated to directly return the result of div64_ul. This keeps the math exactly the same but fixes the warning and makes the code safer.

Please try to wrap your lines to 72 characters.

> 
> Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
> ---
>  drivers/staging/iio/frequency/ad9832.c | 3 +--
>  drivers/staging/iio/frequency/ad9834.c | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index b87ea1781b27..91df7ee65bfc 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -115,8 +115,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);
>  }

I've actually sent a patch for this previously, however the
discussion ended with the fact that there really isn't a point in
doing this, since mclk will always be a value that can fit
into a 32-bit unsigned type, therefore truncation isn't possible.

See here:
https://lore.kernel.org/all/20260409182755.499a419c@pumpkin/

-- 
Kind regards

CJD

Re: [PATCH] staging: iio: frequency: ad9832/ad9834: use div64_ul instead of do_div

[Andy Shevchenko]

On Tue, Jun 30, 2026 at 09:39:31AM +0200, Joshua Crofts wrote:
> On Tue, 30 Jun 2026 08:53:22 +0530
> Mohamad Raizudeen <raizudeen.kerneldev@gmail.com> wrote:

...

> >  {
> >  	unsigned long long freqreg = (u64)fout *
> >  				     (u64)((u64)1L << AD9832_FREQ_BITS);
> > -	do_div(freqreg, mclk);
> > -	return freqreg;
> > +	return div64_ul(freqreg, mclk);
> >  }
> 
> I've actually sent a patch for this previously, however the
> discussion ended with the fact that there really isn't a point in
> doing this, since mclk will always be a value that can fit
> into a 32-bit unsigned type, therefore truncation isn't possible.

Since we are getting more "fixes" in the area, perhaps it's a time to add
a comment in the code summarizing the mentioned discussion?

> See here:
> https://lore.kernel.org/all/20260409182755.499a419c@pumpkin/

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: iio: frequency: ad9832/ad9834: use div64_ul instead of do_div

[Joshua Crofts]

On Tue, 30 Jun 2026 15:02:02 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> Since we are getting more "fixes" in the area, perhaps it's a time to add
> a comment in the code summarizing the mentioned discussion?
> 

Good idea. I'd also mention Coccinelle and its false positive in the comment
since patches trying to fix this always mention it (my attempt included).

-- 
Kind regards

CJD

Re: [PATCH] staging: iio: frequency: ad9832/ad9834: use div64_ul instead of do_div

[Dan Carpenter]

On Tue, Jun 30, 2026 at 03:07:55PM +0200, Joshua Crofts wrote:
> On Tue, 30 Jun 2026 15:02:02 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > Since we are getting more "fixes" in the area, perhaps it's a time to add
> > a comment in the code summarizing the mentioned discussion?
> > 
> 
> Good idea. I'd also mention Coccinelle and its false positive in the comment
> since patches trying to fix this always mention it (my attempt included).

Generally, if the code is old then someone has already looked at the
static checker warnings and decided the original code is fine.  You
should look up old static checker warnings on lore before sending a
patch.

regards,
dan carpenter

[PATCH v2] staging: iio: frequency: ad9832/ad9834: add comment explaining do_div usage

[Mohamad Raizudeen]

Previous attempts to change do_div() to div64_ul() were rejected because
mclk will always fit within 32 bits for this hardware, making do_div()
safe to use.

However, Coccinelle continues to flag this as a false positive. To
prevent future developers from submitting unnecessary fixes, add a
comment explaining why do_div() is intentionally kept.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
---
Changes in v2:
- Instead of changing do_div to div64_ul, added a comment explaining why
  do_div is kept, as requested by Andy Shevchenko and Joshua Crofts.

 drivers/staging/iio/frequency/ad9832.c | 7 +++++++
 drivers/staging/iio/frequency/ad9834.c | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 60c33e10c46f..6d046e9121e2 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -118,6 +118,13 @@ static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
 	u64 freqreg = (u64)fout << AD9832_FREQ_BITS;
 
+       /*
+	* mclk is an unsigned long, which triggers a Coccinelle false positive
+	* warning about using do_div() for 64-by-32 division. However, mclk
+	* for this hardware will always fit within 32 bits, so do_div() is
+	* safe to use here.
+	*/
+
 	do_div(freqreg, mclk);
 	return freqreg;
 }
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index 33dfd723923c..8722f3a01cde 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -103,6 +103,13 @@ static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
 	unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);
 
+       /*
+	* mclk is an unsigned long, which triggers a Coccinelle false positive
+	* warning about using do_div() for 64-by-32 division. However, mclk
+	* for this hardware will always fit within 32 bits, so do_div() is
+	* safe to use here.
+	*/
+
 	do_div(freqreg, mclk);
 	return freqreg;
 }
-- 
2.53.0

Re: [PATCH v2] staging: iio: frequency: ad9832/ad9834: add comment explaining do_div usage

[Joshua Crofts]

On Wed, 22 Jul 2026 13:52:27 +0530
Mohamad Raizudeen <raizudeen.kerneldev@gmail.com> wrote:
> Previous attempts to change do_div() to div64_ul() were rejected because
> mclk will always fit within 32 bits for this hardware, making do_div()
> safe to use.
> 
> However, Coccinelle continues to flag this as a false positive. To
> prevent future developers from submitting unnecessary fixes, add a
> comment explaining why do_div() is intentionally kept.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
> ---

Please don't send a new version as a reply to the previous version,
as it can break tooling such as b4 for patch review.

As for the comment, I think it's good, however I'm not sure whether
this should be split into two patches, given that this is a small
change. Andy, what do you think?

> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 60c33e10c46f..6d046e9121e2 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -118,6 +118,13 @@ static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
>  {
>  	u64 freqreg = (u64)fout << AD9832_FREQ_BITS;
>  
> +       /*
> +	* mclk is an unsigned long, which triggers a Coccinelle false positive
> +	* warning about using do_div() for 64-by-32 division. However, mclk
> +	* for this hardware will always fit within 32 bits, so do_div() is
> +	* safe to use here.
> +	*/
> +

Unnecessary blank line here, you want to couple the comment with the
line that it's describing.

>  	do_div(freqreg, mclk);
>  	return freqreg;
>  }
> diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> index 33dfd723923c..8722f3a01cde 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -103,6 +103,13 @@ static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
>  {
>  	unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);
>  
> +       /*
> +	* mclk is an unsigned long, which triggers a Coccinelle false positive
> +	* warning about using do_div() for 64-by-32 division. However, mclk
> +	* for this hardware will always fit within 32 bits, so do_div() is
> +	* safe to use here.
> +	*/
> +

Ditto.

>  	do_div(freqreg, mclk);
>  	return freqreg;
>  }



-- 
Kind regards,
Joshua Crofts

Re: [PATCH v2] staging: iio: frequency: ad9832/ad9834: add comment explaining do_div usage

[Mohamad Raizudeen]

On Wed, Jul 22, 2026 at 10:46:17AM +0200, Joshua Crofts wrote:
> On Wed, 22 Jul 2026 13:52:27 +0530
> Mohamad Raizudeen <raizudeen.kerneldev@gmail.com> wrote:
> > Previous attempts to change do_div() to div64_ul() were rejected because
> > mclk will always fit within 32 bits for this hardware, making do_div()
> > safe to use.
> > 
> > However, Coccinelle continues to flag this as a false positive. To
> > prevent future developers from submitting unnecessary fixes, add a
> > comment explaining why do_div() is intentionally kept.
> > 
> > Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
> > ---
> 
> Please don't send a new version as a reply to the previous version,
> as it can break tooling such as b4 for patch review.
> 
> As for the comment, I think it's good, however I'm not sure whether
> this should be split into two patches, given that this is a small
> change. Andy, what do you think?
> 
> > diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> > index 60c33e10c46f..6d046e9121e2 100644
> > --- a/drivers/staging/iio/frequency/ad9832.c
> > +++ b/drivers/staging/iio/frequency/ad9832.c
> > @@ -118,6 +118,13 @@ static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
> >  {
> >  	u64 freqreg = (u64)fout << AD9832_FREQ_BITS;
> >  
> > +       /*
> > +	* mclk is an unsigned long, which triggers a Coccinelle false positive
> > +	* warning about using do_div() for 64-by-32 division. However, mclk
> > +	* for this hardware will always fit within 32 bits, so do_div() is
> > +	* safe to use here.
> > +	*/
> > +
> 
> Unnecessary blank line here, you want to couple the comment with the
> line that it's describing.
> 
> >  	do_div(freqreg, mclk);
> >  	return freqreg;
> >  }
> > diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> > index 33dfd723923c..8722f3a01cde 100644
> > --- a/drivers/staging/iio/frequency/ad9834.c
> > +++ b/drivers/staging/iio/frequency/ad9834.c
> > @@ -103,6 +103,13 @@ static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
> >  {
> >  	unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);
> >  
> > +       /*
> > +	* mclk is an unsigned long, which triggers a Coccinelle false positive
> > +	* warning about using do_div() for 64-by-32 division. However, mclk
> > +	* for this hardware will always fit within 32 bits, so do_div() is
> > +	* safe to use here.
> > +	*/
> > +
> 
> Ditto.
> 
> >  	do_div(freqreg, mclk);
> >  	return freqreg;
> >  }
> 
> 
> 
> -- 
> Kind regards,
> Joshua Crofts

Hi Joshua,
Thanks you for reviewing the patch and for the clarifications.

I understood and will make sure v3 is sent as a new and separate email
thread without in-reply-to header so i doesn't break b4.

Sure, I will remove the unnecessary blank line.

I will hold off sending v3 until I hear from Andy regarding whether this should be split
into two patches or kept as one.

Thanks & regards,
Mohamad Raizudeen