[PATCH v3 0/2] iio: frequency: ad9832: cleanups

[PATCH v3 0/2] iio: frequency: ad9832: cleanups

[Joshua Crofts]

This series cleans up issues in the AD9832 driver,
including proxy header removal and bit math
simplification.

v2:
 - PATCH 1: removed redundant bits.h include as it
   is implied in bitops.h
 - PATCH 2: changed ull to u64 type
v3:
 - PATCH 2: remove rogue vim typo

Joshua Crofts (2):
  iio: frequency: ad9832: remove kernel.h proxy header.
  iio: frequency: ad9832: simplify bitwise math

 drivers/staging/iio/frequency/ad9832.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH v3 1/2] iio: frequency: ad9832: remove kernel.h proxy header.

[Joshua Crofts]

Remove kernel.h proxy header and add bitops.h for
better dependency control and code clarity.

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
v2:
 - removed redundant bits.h header

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

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index b87ea1781b..8873a6d11e 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -8,11 +8,10 @@
 #include <asm/div64.h>
 
 #include <linux/bitfield.h>
-#include <linux/bits.h>
+#include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/err.h>
-#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
-- 
2.34.1

Re: [PATCH v3 1/2] iio: frequency: ad9832: remove kernel.h proxy header.

[Andy Shevchenko]

On Sat, Apr 11, 2026 at 02:35:11PM +0200, Joshua Crofts wrote:
> Remove kernel.h proxy header and add bitops.h for
> better dependency control and code clarity.

...

>  #include <asm/div64.h>

While at it, you can move asm/* to go after generic linux/* ones as asm is more
custom than linux.

>  #include <linux/bitfield.h>
> -#include <linux/bits.h>
> +#include <linux/bitops.h>
>  #include <linux/clk.h>

>  #include <linux/device.h>

What about this one? Is it used?

Many drivers just need dev_printk.h + device/devres.h. Some indeed require
device.h. Please, double check.

>  #include <linux/err.h>
> -#include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/2] iio: frequency: ad9832: remove kernel.h proxy header.

[Joshua Crofts]

On Tue, 14 Apr 2026 at 11:38, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> >  #include <asm/div64.h>
>
> While at it, you can move asm/* to go after generic linux/* ones as asm is more
> custom than linux.

Sure.

> >  #include <linux/device.h>
>
> What about this one? Is it used?
>
> Many drivers just need dev_printk.h + device/devres.h. Some indeed require
> device.h. Please, double check.

I compiled it with the include combination above and it passed, but looking into
the generated .i file I can see a full struct device definition, so I
assume there's
some sort of transitive include behavior (spi.h or something)

-- 
Kind regards

CJD

Re: [PATCH v3 1/2] iio: frequency: ad9832: remove kernel.h proxy header.

[Andy Shevchenko]

On Tue, Apr 14, 2026 at 12:12:02PM +0200, Joshua Crofts wrote:
> On Tue, 14 Apr 2026 at 11:38, Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:

...

> > >  #include <linux/device.h>
> >
> > What about this one? Is it used?
> >
> > Many drivers just need dev_printk.h + device/devres.h. Some indeed require
> > device.h. Please, double check.
> 
> I compiled it with the include combination above and it passed, but looking into
> the generated .i file I can see a full struct device definition, so I
> assume there's
> some sort of transitive include behavior (spi.h or something)

Please, avoid following the suggestions blindly. This work needs a careful
check of what's exactly is being used and which header should provide the
necessary information. While it may compile it might be still wrong. You
need to (briefly) read the code in full. I don't use iwyu tool with tweaks
by Jonathan Cameron, old school guy, but you can also try it [1].

[1]: https://lore.kernel.org/linux-iio/20250629183649.184479-1-jic23@kernel.org/
Read the whole thread, it's not big.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/2] iio: frequency: ad9832: remove kernel.h proxy header.

[Joshua Crofts]

On Tue, 14 Apr 2026 at 13:43, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> Please, avoid following the suggestions blindly. This work needs a careful
> check of what's exactly is being used and which header should provide the
> necessary information. While it may compile it might be still wrong. You
> need to (briefly) read the code in full. I don't use iwyu tool with tweaks
> by Jonathan Cameron, old school guy, but you can also try it [1].

Don't worry, I wasn't blindly following your suggestions. After some tweaking,
IWYU recommends keeping device.h and removing slab.h. My question is
whether the swap from asm/div64.h to linux/math64.h is valid? The asm
implementation would be platform specific, so swapping to math64 seems
like a good idea.

-- 
Kind regards

CJD

Re: [PATCH v3 1/2] iio: frequency: ad9832: remove kernel.h proxy header.

[Andy Shevchenko]

On Tue, Apr 14, 2026 at 03:27:39PM +0200, Joshua Crofts wrote:
> On Tue, 14 Apr 2026 at 13:43, Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > Please, avoid following the suggestions blindly. This work needs a careful
> > check of what's exactly is being used and which header should provide the
> > necessary information. While it may compile it might be still wrong. You
> > need to (briefly) read the code in full. I don't use iwyu tool with tweaks
> > by Jonathan Cameron, old school guy, but you can also try it [1].
> 
> Don't worry, I wasn't blindly following your suggestions. After some tweaking,
> IWYU recommends keeping device.h and removing slab.h. My question is
> whether the swap from asm/div64.h to linux/math64.h is valid? The asm
> implementation would be platform specific, so swapping to math64 seems
> like a good idea.

Up to you, but that would need to be a separate patch, because it does
the opposite to what your patch is doing. And justification is most likely
that it's preferred to use linux/ generic over asm/ in the drivers. Or
something in those lines...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/2] iio: frequency: ad9832: remove kernel.h proxy header.

[Joshua Crofts]

On Tue, 14 Apr 2026 at 19:03, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> Up to you, but that would need to be a separate patch, because it does
> the opposite to what your patch is doing. And justification is most likely
> that it's preferred to use linux/ generic over asm/ in the drivers. Or
> something in those lines...

Okay, I'll send it once I get this patch series done.

-- 
Kind regards

CJD

[PATCH v3 2/2] iio: frequency: ad9832: simplify bitwise math

[Joshua Crofts]

Refactor the ad9832_calc_freqreg by adding a BIT_ULL()
macro instead of manual bit shifting for better
readability.

Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
v2:
 - changed ull to u64 type
v3:
 - removed rogue vim typo

 drivers/staging/iio/frequency/ad9832.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 8873a6d11e..778aaa8747 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -112,8 +112,8 @@ struct ad9832_state {
 
 static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
-	unsigned long long freqreg = (u64)fout *
-				     (u64)((u64)1L << AD9832_FREQ_BITS);
+	u64 freqreg = (u64)fout * BIT_ULL(AD9832_FREQ_BITS);
+
 	do_div(freqreg, mclk);
 	return freqreg;
 }
-- 
2.34.1

Re: [PATCH v3 2/2] iio: frequency: ad9832: simplify bitwise math

[Andy Shevchenko]

On Sat, Apr 11, 2026 at 02:35:12PM +0200, Joshua Crofts wrote:
> Refactor the ad9832_calc_freqreg by adding a BIT_ULL()
> macro instead of manual bit shifting for better
> readability.

"While at it, switch to u64 type for the local variable
 that may hold big values."

...

> -	unsigned long long freqreg = (u64)fout *
> -				     (u64)((u64)1L << AD9832_FREQ_BITS);
> +	u64 freqreg = (u64)fout * BIT_ULL(AD9832_FREQ_BITS);

It can even be just like this:

	u64 freqreg = fout * BIT_ULL(AD9832_FREQ_BITS);

if I understand the integer promotion for unsigned types here
and the * operator correctly.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] iio: frequency: ad9832: simplify bitwise math

[Joshua Crofts]

On Tue, 14 Apr 2026 at 11:44, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> It can even be just like this:
>
>         u64 freqreg = fout * BIT_ULL(AD9832_FREQ_BITS);
>
> if I understand the integer promotion for unsigned types here
> and the * operator correctly.

Sure, at least I'll ensure that the reviewed-by tag is incorporated
correctly.

-- 
Kind regards

CJD

Re: [PATCH v3 2/2] iio: frequency: ad9832: simplify bitwise math

[Andy Shevchenko]

On Tue, Apr 14, 2026 at 11:51:03AM +0200, Joshua Crofts wrote:
> On Tue, 14 Apr 2026 at 11:44, Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > It can even be just like this:
> >
> >         u64 freqreg = fout * BIT_ULL(AD9832_FREQ_BITS);
> >
> > if I understand the integer promotion for unsigned types here
> > and the * operator correctly.
> 
> Sure, at least I'll ensure that the reviewed-by tag is incorporated
> correctly.

It's up to you. (u64) casting here is not bad per se. And it's more robust
against changes in the second operand the type of which is hidden currently.
(Reading again what I just wrote, it seems I objecting my own suggestion!)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] iio: frequency: ad9832: simplify bitwise math

[Joshua Crofts]

On Tue, 14 Apr 2026 at 11:57, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> It's up to you. (u64) casting here is not bad per se. And it's more robust
> against changes in the second operand the type of which is hidden currently.
> (Reading again what I just wrote, it seems I objecting my own suggestion!)

If I'm not mistaken, the compiler would always do a type promotion of the
"smaller" operand (in this case fout). By casting at this point we're just doing
the work for it, so I guess it doesn't matter.

-- 
Kind regards

CJD

Re: [PATCH v3 2/2] iio: frequency: ad9832: simplify bitwise math

[Andy Shevchenko]

On Tue, Apr 14, 2026 at 12:45:31PM +0200, Joshua Crofts wrote:
> On Tue, 14 Apr 2026 at 11:57, Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > It's up to you. (u64) casting here is not bad per se. And it's more robust
> > against changes in the second operand the type of which is hidden currently.
> > (Reading again what I just wrote, it seems I objecting my own suggestion!)

> If I'm not mistaken, the compiler would always do a type promotion of the
> "smaller" operand (in this case fout). By casting at this point we're just doing
> the work for it, so I guess it doesn't matter.

Yes, and the problem here that BIT_ULL() is (semi)hidden on what it returns for
smaller values. Explicit casting helps with that (while not needed). The problem
that might appear in the future if on some circumstances the both operands become
of 32-bit types... You won't get 64-bit value out of that without casting.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] iio: frequency: ad9832: simplify bitwise math

[Dan Carpenter]

On Tue, Apr 14, 2026 at 03:33:52PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 14, 2026 at 12:45:31PM +0200, Joshua Crofts wrote:
> > On Tue, 14 Apr 2026 at 11:57, Andy Shevchenko
> > <andriy.shevchenko@intel.com> wrote:
> > > It's up to you. (u64) casting here is not bad per se. And it's more robust
> > > against changes in the second operand the type of which is hidden currently.
> > > (Reading again what I just wrote, it seems I objecting my own suggestion!)
> 
> > If I'm not mistaken, the compiler would always do a type promotion of the
> > "smaller" operand (in this case fout). By casting at this point we're just doing
> > the work for it, so I guess it doesn't matter.
> 
> Yes, and the problem here that BIT_ULL() is (semi)hidden on what it returns for
> smaller values.

There is no way we'd implement a BIT_ULL() that doesn't give you a ULL.

regards,
dan carpenter

Re: [PATCH v3 2/2] iio: frequency: ad9832: simplify bitwise math

[Andy Shevchenko]

On Tue, Apr 14, 2026 at 04:02:30PM +0300, Dan Carpenter wrote:
> On Tue, Apr 14, 2026 at 03:33:52PM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 14, 2026 at 12:45:31PM +0200, Joshua Crofts wrote:
> > > On Tue, 14 Apr 2026 at 11:57, Andy Shevchenko
> > > <andriy.shevchenko@intel.com> wrote:
> > > > It's up to you. (u64) casting here is not bad per se. And it's more robust
> > > > against changes in the second operand the type of which is hidden currently.
> > > > (Reading again what I just wrote, it seems I objecting my own suggestion!)
> > 
> > > If I'm not mistaken, the compiler would always do a type promotion of the
> > > "smaller" operand (in this case fout). By casting at this point we're just doing
> > > the work for it, so I guess it doesn't matter.
> > 
> > Yes, and the problem here that BIT_ULL() is (semi)hidden on what it returns for
> > smaller values.
> 
> There is no way we'd implement a BIT_ULL() that doesn't give you a ULL.

I meant that it's not crystal clear, one needs to know the naming convention of
BIT_ULL() and/or check the implementation. Reading (u64) is much clearer in
that sense. Also in case of BIT_ULL() parameter being < 32 it may be replaced
with BIT().

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/2] iio: frequency: ad9832: cleanups

[Joshua Crofts]

On Sat, 11 Apr 2026 at 14:35, Joshua Crofts <joshua.crofts1@gmail.com> wrote:
>
> This series cleans up issues in the AD9832 driver,
> including proxy header removal and bit math
> simplification.
>
> v2:
>  - PATCH 1: removed redundant bits.h include as it
>    is implied in bitops.h
>  - PATCH 2: changed ull to u64 type
> v3:
>  - PATCH 2: remove rogue vim typo
>

FYI, adding Nuno's reviewed-by tag from v2 as v3 is just a typo fix.

Reviewed-by: Nuno Sá <nuno.sa@analog.com>