[PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[Archit Anant]

Replace do_div() with div64_ul() since the remainder is not used.
div64_ul() is the preferred API for 64-bit by 32-bit division when
only the quotient is needed, as it returns the result directly rather
than modifying the dividend in-place.

Issue identified by coccicheck using do_div.cocci.

Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/staging/iio/impedance-analyzer/ad5933.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 85a4223295cd..772267f6f093 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -194,8 +194,8 @@ static int ad5933_set_freq(struct ad5933_state *st,
 		u8 d8[4];
 	} dat;
 
-	freqreg = (u64)freq * (u64)(1 << 27);
-	do_div(freqreg, st->mclk_hz / 4);
+	freqreg = div64_ul((u64)freq * (u64)(1 << 27),
+			   st->mclk_hz / 4);
 
 	switch (reg) {
 	case AD5933_REG_FREQ_START:
-- 
2.39.5

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[Andy Shevchenko]

On Thu, Jan 22, 2026 at 08:26:33PM +0530, Archit Anant wrote:
> Replace do_div() with div64_ul() since the remainder is not used.
> div64_ul() is the preferred API for 64-bit by 32-bit division when
> only the quotient is needed, as it returns the result directly rather
> than modifying the dividend in-place.
> 
> Issue identified by coccicheck using do_div.cocci.

...

> -	freqreg = (u64)freq * (u64)(1 << 27);
> -	do_div(freqreg, st->mclk_hz / 4);
> +	freqreg = div64_ul((u64)freq * (u64)(1 << 27),
> +			   st->mclk_hz / 4);

It can be one line to begin with.
Then drop that ugly castings and explicit big shifts.

	freqreg = div64_ul(BIT_ULL(27) * freq, st->mclk_hz / 4);

Now you can see That 4 is only 2 bits, so this can be written in
simpler way:

	freqreg = div64_ul(BIT_ULL(29) * freq, st->mclk_hz);

which may give a better precision at the end of the day.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[Andy Shevchenko]

On Thu, Jan 22, 2026 at 05:12:42PM +0200, Andy Shevchenko wrote:
> On Thu, Jan 22, 2026 at 08:26:33PM +0530, Archit Anant wrote:

...

> > -	freqreg = (u64)freq * (u64)(1 << 27);
> > -	do_div(freqreg, st->mclk_hz / 4);
> > +	freqreg = div64_ul((u64)freq * (u64)(1 << 27),
> > +			   st->mclk_hz / 4);
> 
> It can be one line to begin with.
> Then drop that ugly castings and explicit big shifts.
> 
> 	freqreg = div64_ul(BIT_ULL(27) * freq, st->mclk_hz / 4);
> 
> Now you can see That 4 is only 2 bits, so this can be written in
> simpler way:
> 
> 	freqreg = div64_ul(BIT_ULL(29) * freq, st->mclk_hz);
> 
> which may give a better precision at the end of the day.

It also might be worth to add a comment on top to explain (with given context
I don't know if there is already one on top of the function, though).

And I think we want AD people to comment on this and maybe explain better
the calculations done (and why the original code drops precision, was it
deliberate?).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[Andy Shevchenko]

On Thu, Jan 22, 2026 at 10:27:52PM +0530, Archit Anant wrote:
> (Resending to the list, apologies for the private reply earlier.)

First of all, do no top post!

> Thanks for the review and the detailed math breakdown.
> 
> I agree that `(freq * BIT_ULL(29)) / mclk` is algebraically superior and
> improves precision.
> 
> However, regarding your concern about the original code:
> > "why the original code drops precision, was it deliberate?"
> 
> Since I do not have the AD5933 hardware to test if the register expects the
> truncated value from `(mclk / 4)`, I am hesitant to change the logic in
> this patch.
> 
> Would you prefer I:
> 1. Stick to a purely mechanical change (keep the logic equivalent, just use
> `div64_ul` and `BIT_ULL(27)` for readability)?
> 2. Or proceed with the `BIT_ULL(29)` simplification assuming the precision
> loss was unintentional?

I definitely prefer #2. That's why I plead to AD people to confirm.

> I'm leaning towards #1 for safety, but happy to do #2 if the maintainers
> (Lars/Michael) think it's safe.

> On Thu, Jan 22, 2026 at 8:45 PM Andy Shevchenko <andriy.shevchenko@intel.com>
> wrote:
> 
> > On Thu, Jan 22, 2026 at 05:12:42PM +0200, Andy Shevchenko wrote:
> > > On Thu, Jan 22, 2026 at 08:26:33PM +0530, Archit Anant wrote:
> >
> > ...
> >
> > > > -   freqreg = (u64)freq * (u64)(1 << 27);
> > > > -   do_div(freqreg, st->mclk_hz / 4);
> > > > +   freqreg = div64_ul((u64)freq * (u64)(1 << 27),
> > > > +                      st->mclk_hz / 4);
> > >
> > > It can be one line to begin with.
> > > Then drop that ugly castings and explicit big shifts.
> > >
> > >       freqreg = div64_ul(BIT_ULL(27) * freq, st->mclk_hz / 4);
> > >
> > > Now you can see That 4 is only 2 bits, so this can be written in
> > > simpler way:
> > >
> > >       freqreg = div64_ul(BIT_ULL(29) * freq, st->mclk_hz);
> > >
> > > which may give a better precision at the end of the day.
> >
> > It also might be worth to add a comment on top to explain (with given
> > context
> > I don't know if there is already one on top of the function, though).
> >
> > And I think we want AD people to comment on this and maybe explain better
> > the calculations done (and why the original code drops precision, was it
> > deliberate?).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[Archit Anant]

Gentle ping on this.

Has anyone from AD had a chance to confirm whether the original
truncation via (mclk / 4) was intentional?

If there are no objections, I can prepare a v2 using the BIT_ULL(29)
form as suggested.

Thanks,
Archit

On Thu, Jan 22, 2026 at 11:33 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Thu, Jan 22, 2026 at 10:27:52PM +0530, Archit Anant wrote:
> > (Resending to the list, apologies for the private reply earlier.)
>
> First of all, do no top post!
>
> > Thanks for the review and the detailed math breakdown.
> >
> > I agree that `(freq * BIT_ULL(29)) / mclk` is algebraically superior and
> > improves precision.
> >
> > However, regarding your concern about the original code:
> > > "why the original code drops precision, was it deliberate?"
> >
> > Since I do not have the AD5933 hardware to test if the register expects the
> > truncated value from `(mclk / 4)`, I am hesitant to change the logic in
> > this patch.
> >
> > Would you prefer I:
> > 1. Stick to a purely mechanical change (keep the logic equivalent, just use
> > `div64_ul` and `BIT_ULL(27)` for readability)?
> > 2. Or proceed with the `BIT_ULL(29)` simplification assuming the precision
> > loss was unintentional?
>
> I definitely prefer #2. That's why I plead to AD people to confirm.
>
> > I'm leaning towards #1 for safety, but happy to do #2 if the maintainers
> > (Lars/Michael) think it's safe.
>
> > On Thu, Jan 22, 2026 at 8:45 PM Andy Shevchenko <andriy.shevchenko@intel.com>
> > wrote:
> >
> > > On Thu, Jan 22, 2026 at 05:12:42PM +0200, Andy Shevchenko wrote:
> > > > On Thu, Jan 22, 2026 at 08:26:33PM +0530, Archit Anant wrote:
> > >
> > > ...
> > >
> > > > > -   freqreg = (u64)freq * (u64)(1 << 27);
> > > > > -   do_div(freqreg, st->mclk_hz / 4);
> > > > > +   freqreg = div64_ul((u64)freq * (u64)(1 << 27),
> > > > > +                      st->mclk_hz / 4);
> > > >
> > > > It can be one line to begin with.
> > > > Then drop that ugly castings and explicit big shifts.
> > > >
> > > >       freqreg = div64_ul(BIT_ULL(27) * freq, st->mclk_hz / 4);
> > > >
> > > > Now you can see That 4 is only 2 bits, so this can be written in
> > > > simpler way:
> > > >
> > > >       freqreg = div64_ul(BIT_ULL(29) * freq, st->mclk_hz);
> > > >
> > > > which may give a better precision at the end of the day.
> > >
> > > It also might be worth to add a comment on top to explain (with given
> > > context
> > > I don't know if there is already one on top of the function, though).
> > >
> > > And I think we want AD people to comment on this and maybe explain better
> > > the calculations done (and why the original code drops precision, was it
> > > deliberate?).
>
> --
> With Best Regards,
> Andy Shevchenko
>
>


-- 
Sincerely,
Archit Anant

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[David Lechner]

On 2/16/26 9:52 AM, Archit Anant wrote:
> 
> On Thu, Jan 22, 2026 at 11:33 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
>>
>> On Thu, Jan 22, 2026 at 10:27:52PM +0530, Archit Anant wrote:
>>> (Resending to the list, apologies for the private reply earlier.)
>>
>> First of all, do no top post!
>>
>>> Thanks for the review and the detailed math breakdown.
>>>
>>> I agree that `(freq * BIT_ULL(29)) / mclk` is algebraically superior and
>>> improves precision.
>>>
>>> However, regarding your concern about the original code:
>>>> "why the original code drops precision, was it deliberate?"
>>>
>>> Since I do not have the AD5933 hardware to test if the register expects the
>>> truncated value from `(mclk / 4)`, I am hesitant to change the logic in
>>> this patch.
>>>
>>> Would you prefer I:
>>> 1. Stick to a purely mechanical change (keep the logic equivalent, just use
>>> `div64_ul` and `BIT_ULL(27)` for readability)?
>>> 2. Or proceed with the `BIT_ULL(29)` simplification assuming the precision
>>> loss was unintentional?
>>
>> I definitely prefer #2. That's why I plead to AD people to confirm.
>>
>>> I'm leaning towards #1 for safety, but happy to do #2 if the maintainers
>>> (Lars/Michael) think it's safe.

> Gentle ping on this.


As Andy said, please don't top-post. Put the reply in context to make it easy
to see what "this" is referring to.

I moved your reply to show what we mean.

> 
> Has anyone from AD had a chance to confirm whether the original
> truncation via (mclk / 4) was intentional?
> 
> If there are no objections, I can prepare a v2 using the BIT_ULL(29)
> form as suggested.
> 
> Thanks,
> Archit


Given how old this driver is, I would not expect an reply as the
author has likely long forgotten what they were thinking at the
time.

I would keep the (mclk / 4) as it is to be safe as you suggest. There
could be userspace depending on exact values currently being returned.

>>
>>> On Thu, Jan 22, 2026 at 8:45 PM Andy Shevchenko <andriy.shevchenko@intel.com>
>>> wrote:
>>>
>>>> On Thu, Jan 22, 2026 at 05:12:42PM +0200, Andy Shevchenko wrote:
>>>>> On Thu, Jan 22, 2026 at 08:26:33PM +0530, Archit Anant wrote:
>>>>
>>>> ...
>>>>
>>>>>> -   freqreg = (u64)freq * (u64)(1 << 27);
>>>>>> -   do_div(freqreg, st->mclk_hz / 4);
>>>>>> +   freqreg = div64_ul((u64)freq * (u64)(1 << 27),
>>>>>> +                      st->mclk_hz / 4);
>>>>>
>>>>> It can be one line to begin with.
>>>>> Then drop that ugly castings and explicit big shifts.
>>>>>
>>>>>       freqreg = div64_ul(BIT_ULL(27) * freq, st->mclk_hz / 4);
>>>>>
>>>>> Now you can see That 4 is only 2 bits, so this can be written in
>>>>> simpler way:
>>>>>
>>>>>       freqreg = div64_ul(BIT_ULL(29) * freq, st->mclk_hz);
>>>>>
>>>>> which may give a better precision at the end of the day.
>>>>
>>>> It also might be worth to add a comment on top to explain (with given
>>>> context
>>>> I don't know if there is already one on top of the function, though).
>>>>
>>>> And I think we want AD people to comment on this and maybe explain better
>>>> the calculations done (and why the original code drops precision, was it
>>>> deliberate?).
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>>
>>
> 
> 

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[David Lechner]

On 2/16/26 1:02 PM, David Lechner wrote:
> On 2/16/26 9:52 AM, Archit Anant wrote:
>>
>> On Thu, Jan 22, 2026 at 11:33 PM Andy Shevchenko
>> <andriy.shevchenko@intel.com> wrote:
>>>
>>> On Thu, Jan 22, 2026 at 10:27:52PM +0530, Archit Anant wrote:
>>>> (Resending to the list, apologies for the private reply earlier.)
>>>
>>> First of all, do no top post!
>>>
>>>> Thanks for the review and the detailed math breakdown.
>>>>
>>>> I agree that `(freq * BIT_ULL(29)) / mclk` is algebraically superior and
>>>> improves precision.
>>>>
>>>> However, regarding your concern about the original code:
>>>>> "why the original code drops precision, was it deliberate?"
>>>>
>>>> Since I do not have the AD5933 hardware to test if the register expects the
>>>> truncated value from `(mclk / 4)`, I am hesitant to change the logic in
>>>> this patch.
>>>>
>>>> Would you prefer I:
>>>> 1. Stick to a purely mechanical change (keep the logic equivalent, just use
>>>> `div64_ul` and `BIT_ULL(27)` for readability)?
>>>> 2. Or proceed with the `BIT_ULL(29)` simplification assuming the precision
>>>> loss was unintentional?
>>>
>>> I definitely prefer #2. That's why I plead to AD people to confirm.
>>>
>>>> I'm leaning towards #1 for safety, but happy to do #2 if the maintainers
>>>> (Lars/Michael) think it's safe.
> 
>> Gentle ping on this.
> 
> 
> As Andy said, please don't top-post. Put the reply in context to make it easy
> to see what "this" is referring to.
> 
> I moved your reply to show what we mean.
> 
>>
>> Has anyone from AD had a chance to confirm whether the original
>> truncation via (mclk / 4) was intentional?
>>
>> If there are no objections, I can prepare a v2 using the BIT_ULL(29)
>> form as suggested.
>>
>> Thanks,
>> Archit
> 
> 
> Given how old this driver is, I would not expect an reply as the
> author has likely long forgotten what they were thinking at the
> time.
> 
> I would keep the (mclk / 4) as it is to be safe as you suggest. There
> could be userspace depending on exact values currently being returned.
> 

Actually, I just saw this was in staging, so I guess not breaking userspace
isn't the top priority if there the other way is objectively better. I would
assume that testing/measurement could show which is actually correct.

>>>
>>>> On Thu, Jan 22, 2026 at 8:45 PM Andy Shevchenko <andriy.shevchenko@intel.com>
>>>> wrote:
>>>>
>>>>> On Thu, Jan 22, 2026 at 05:12:42PM +0200, Andy Shevchenko wrote:
>>>>>> On Thu, Jan 22, 2026 at 08:26:33PM +0530, Archit Anant wrote:
>>>>>
>>>>> ...
>>>>>
>>>>>>> -   freqreg = (u64)freq * (u64)(1 << 27);
>>>>>>> -   do_div(freqreg, st->mclk_hz / 4);
>>>>>>> +   freqreg = div64_ul((u64)freq * (u64)(1 << 27),
>>>>>>> +                      st->mclk_hz / 4);
>>>>>>
>>>>>> It can be one line to begin with.
>>>>>> Then drop that ugly castings and explicit big shifts.
>>>>>>
>>>>>>       freqreg = div64_ul(BIT_ULL(27) * freq, st->mclk_hz / 4);
>>>>>>
>>>>>> Now you can see That 4 is only 2 bits, so this can be written in
>>>>>> simpler way:
>>>>>>
>>>>>>       freqreg = div64_ul(BIT_ULL(29) * freq, st->mclk_hz);
>>>>>>
>>>>>> which may give a better precision at the end of the day.
>>>>>
>>>>> It also might be worth to add a comment on top to explain (with given
>>>>> context
>>>>> I don't know if there is already one on top of the function, though).
>>>>>
>>>>> And I think we want AD people to comment on this and maybe explain better
>>>>> the calculations done (and why the original code drops precision, was it
>>>>> deliberate?).
>>>
>>> --
>>> With Best Regards,
>>> Andy Shevchenko
>>>
>>>
>>
>>
> 

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[Andy Shevchenko]

On Mon, Feb 16, 2026 at 09:22:32PM +0530, Archit Anant wrote:
> Gentle ping on this.
> 
> Has anyone from AD had a chance to confirm whether the original
> truncation via (mclk / 4) was intentional?
> 
> If there are no objections, I can prepare a v2 using the BIT_ULL(29)
> form as suggested.

Gentle ping on reading and understanding the given comments, please!

> On Thu, Jan 22, 2026 at 11:33 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > On Thu, Jan 22, 2026 at 10:27:52PM +0530, Archit Anant wrote:
> > > (Resending to the list, apologies for the private reply earlier.)
> >
> > First of all, do no top post!

^^^ Have you understood this?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[Archit Anant]

On Tue, Feb 17, 2026 at 1:59 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Mon, Feb 16, 2026 at 09:22:32PM +0530, Archit Anant wrote:
> > Gentle ping on this.
> >
> > Has anyone from AD had a chance to confirm whether the original
> > truncation via (mclk / 4) was intentional?
> >
> > If there are no objections, I can prepare a v2 using the BIT_ULL(29)
> > form as suggested.
>
> Gentle ping on reading and understanding the given comments, please!
>
> > On Thu, Jan 22, 2026 at 11:33 PM Andy Shevchenko
> > <andriy.shevchenko@intel.com> wrote:
> > >
> > > On Thu, Jan 22, 2026 at 10:27:52PM +0530, Archit Anant wrote:
> > > > (Resending to the list, apologies for the private reply earlier.)
> > >
> > > First of all, do no top post!
>
> ^^^ Have you understood this?

Yes, understood. I’ll avoid top posting going forward.

>
> --
> With Best Regards,
> Andy Shevchenko
>
>

-- 
Sincerely,
Archit Anant

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[Archit Anant]

On Tue, Feb 17, 2026 at 12:34 AM David Lechner <dlechner@baylibre.com> wrote:
> Actually, I just saw this was in staging, so I guess not breaking userspace
> isn't the top priority if there the other way is objectively better. I would
> assume that testing/measurement could show which is actually correct.

Since I don't have the hardware to perform those measurements and confirm
if the precision improvement is valid or a regression, I will take the
conservative
approach for v2.

I will send a v2 that only replaces do_div() with div64_ul() and uses
BIT_ULL(27), preserving the original (mclk / 4) logic to guarantee no
behavioral change.

-- 
Sincerely,
Archit Anant

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[Jonathan Cameron]

On Tue, 17 Feb 2026 22:16:19 +0530
Archit Anant <architanant5@gmail.com> wrote:

> On Tue, Feb 17, 2026 at 12:34 AM David Lechner <dlechner@baylibre.com> wrote:
> > Actually, I just saw this was in staging, so I guess not breaking userspace
> > isn't the top priority if there the other way is objectively better. I would
> > assume that testing/measurement could show which is actually correct.  
> 
> Since I don't have the hardware to perform those measurements and confirm
> if the precision improvement is valid or a regression, I will take the
> conservative
> approach for v2.
> 
> I will send a v2 that only replaces do_div() with div64_ul() and uses
> BIT_ULL(27), preserving the original (mclk / 4) logic to guarantee no
> behavioral change.
> 

I started writing that I didn't care either way and given the chances of
not having to fix some ABI up on the way out of staging is near zero
(which would break any userspace anyway) but then I took at look at the
log.  We've had a fixes for this fairly recently!

Might imply Gabriel has hardware? (or a 'healthy' interest in
reading data sheets! :)

So +CC Gabriel who might have some insight.

Thanks,

Jonathan

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use div64_ul() instead of do_div()

[Archit Anant]

On Wed, Feb 18, 2026 at 11:49 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 17 Feb 2026 22:16:19 +0530
> Archit Anant <architanant5@gmail.com> wrote:
>
> > On Tue, Feb 17, 2026 at 12:34 AM David Lechner <dlechner@baylibre.com> wrote:
> > > Actually, I just saw this was in staging, so I guess not breaking userspace
> > > isn't the top priority if there the other way is objectively better. I would
> > > assume that testing/measurement could show which is actually correct.
> >
> > Since I don't have the hardware to perform those measurements and confirm
> > if the precision improvement is valid or a regression, I will take the
> > conservative
> > approach for v2.
> >
> > I will send a v2 that only replaces do_div() with div64_ul() and uses
> > BIT_ULL(27), preserving the original (mclk / 4) logic to guarantee no
> > behavioral change.
> >
>
> I started writing that I didn't care either way and given the chances of
> not having to fix some ABI up on the way out of staging is near zero
> (which would break any userspace anyway) but then I took at look at the
> log.  We've had a fixes for this fairly recently!
>
> Might imply Gabriel has hardware? (or a 'healthy' interest in
> reading data sheets! :)
>
> So +CC Gabriel who might have some insight.
>
> Thanks,
>
> Jonathan

Hi Jonathan,

Thanks for the lead.

Hi Gabriel,

I sent out a v2 of this patch earlier today [1] using BIT_ULL(27)
while preserving the original (mclk / 4) logic for safety.

If you have access to the hardware or documentation, I would
appreciate any insight into whether Andy's suggested simplification
to (mclk) instead of (mclk / 4) would be a valid precision improvement
or if the original truncation was intentional.

[1] https://lore.kernel.org/all/20260218125327.21467-1-architanant5@gmail.com/

-- 
Sincerely,
Archit Anant