linux-kernel - Re: [PATCH v1] staging: iio: frequency: ad9832: replace long/short with u32/u16

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251206154250.7d5dfa74@jic23-huawei>
Date: Sat, 6 Dec 2025 15:42:50 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Dharanitharan R <dharanitharan725@...il.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, David Lechner <dlechner@...libre.com>, Nuno
 Sá <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-iio@...r.kernel.org,
 linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] staging: iio: frequency: ad9832: replace long/short
 with u32/u16

On Sat,  6 Dec 2025 05:48:31 +0000
Dharanitharan R <dharanitharan725@...il.com> wrote:

> Cleanup the AD9832 header by explicitly including <linux/types.h> and
> replacing ambiguous integer types with fixed-width kernel types:
> 
> - unsigned long → u32
> - unsigned short → u16
> 
> This improves type clarity and ensures consistent behavior across
> architectures.
> 
> Signed-off-by: Dharanitharan R <dharanitharan725@...il.com>

Hi Dharanitharan,

Thanks for sending this.

A few comments.  Firstly there is a reasonably high chance we'll be shortly
dropping this driver from staging because it's had no significant work to
bring it up to date in a very long time.  Unfortunately unless you were
following that discussion deep in an unrelated looking thread there was
no way for you to know that :(

Following up on this particular patch, it doesn't make sense to just
change the types where they are stored here.  These are passed into
various functions that continue to take unsigned long / short parameters.
For example, ad9832_write_frequency() and the calls under that.
If we were going to tighten the types up it should be consistent.
Such a patch would then show how these updated types interact with the
other values in the driver for example the output of clk_get_rate().

So in general this sort of cleanup might makes sense only if done
right across all use of the values for which the types are being changed
otherwise it just ends up adding to the confusion.

Third, platform data like this is a nasty bit of legacy from the pre
device tree days (this is a very old driver!) and so if we didn't fix
up this driver it would likely go away as part of that effort.

As noted though I wouldn't advise continuing with this particular
driver on simple basis it will probably go away (unless someone shouts
that they still need it when I send the removal patch!)

Jonathan

> ---
>  drivers/staging/iio/frequency/ad9832.h | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
> index d0d840edb8d2..d59ad0627a9b 100644
> --- a/drivers/staging/iio/frequency/ad9832.h
> +++ b/drivers/staging/iio/frequency/ad9832.h
> @@ -6,6 +6,7 @@
>   */
>  #ifndef IIO_DDS_AD9832_H_
>  #define IIO_DDS_AD9832_H_
> +#include <linux/types.h>
>  
>  /*
>   * TODO: struct ad9832_platform_data needs to go into include/linux/iio
> @@ -22,12 +23,12 @@
>   */
>  
>  struct ad9832_platform_data {
> -	unsigned long		freq0;
> -	unsigned long		freq1;
> -	unsigned short		phase0;
> -	unsigned short		phase1;
> -	unsigned short		phase2;
> -	unsigned short		phase3;
> +	u32 freq0;
> +	u32 freq1;
> +	u16 phase0;
> +	u16 phase1;
> +	u16 phase2;
> +	u16 phase3;
>  };
>  
>  #endif /* IIO_DDS_AD9832_H_ */