[PATCH] iio: core: Cast info mask to 'unsigned long *' in bit-ops

[PATCH] iio: core: Cast info mask to 'unsigned long *' in bit-ops

[Junjie Cao]

for_each_set_bit()/find_*_bit() expect 'const unsigned long *' (see
include/linux/find.h), but industrialio-core.c passes 'const long *'
in iio_device_add_info_mask_type{,_avail}(). Sparse flags a signedness
mismatch.

The masks are used purely as bit arrays. Cast them to 'const unsigned
long *' at the call sites to match the helpers' prototypes, without
changing memory layout or behavior. Changing the field types would cause
unnecessary churn across IIO.

Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
 drivers/iio/industrialio-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 159d6c5ca3ce..e9491a999ac0 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1248,7 +1248,7 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
 	int i, ret, attrcount = 0;
 
-	for_each_set_bit(i, infomask, sizeof(*infomask)*8) {
+	for_each_set_bit(i, (const unsigned long *)infomask, sizeof(*infomask) * 8) {
 		if (i >= ARRAY_SIZE(iio_chan_info_postfix))
 			return -EINVAL;
 		ret = __iio_add_chan_devattr(iio_chan_info_postfix[i],
@@ -1279,7 +1279,7 @@ static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
 	int i, ret, attrcount = 0;
 	char *avail_postfix;
 
-	for_each_set_bit(i, infomask, sizeof(*infomask) * 8) {
+	for_each_set_bit(i, (const unsigned long *)infomask, sizeof(*infomask) * 8) {
 		if (i >= ARRAY_SIZE(iio_chan_info_postfix))
 			return -EINVAL;
 		avail_postfix = kasprintf(GFP_KERNEL,
-- 
2.43.0

Re: [PATCH] iio: core: Cast info mask to 'unsigned long *' in bit-ops

[Jonathan Cameron]

On Fri, 15 Aug 2025 10:25:28 +0800
Junjie Cao <junjie.cao@intel.com> wrote:

> for_each_set_bit()/find_*_bit() expect 'const unsigned long *' (see
> include/linux/find.h), but industrialio-core.c passes 'const long *'
> in iio_device_add_info_mask_type{,_avail}(). Sparse flags a signedness
> mismatch.

How do you get that warning?  I'm not seeing it.

> 
> The masks are used purely as bit arrays. Cast them to 'const unsigned
> long *' at the call sites to match the helpers' prototypes, without
> changing memory layout or behavior. Changing the field types would cause
> unnecessary churn across IIO.

Did you have a go?  Superficially feels like we should fix this up and
that it should be almost entirely confined to relatively few core functions
and the types of the various elements of struct iio_dev.

Might just be (builds and I'm not seeing new warning but then I couldn't trigger
the one you saw!)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 159d6c5ca3ce..9125d466118d 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1243,7 +1243,7 @@ static int iio_device_add_channel_label(struct iio_dev *indio_dev,
 static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
                                         struct iio_chan_spec const *chan,
                                         enum iio_shared_by shared_by,
-                                        const long *infomask)
+                                        const unsigned long *infomask)
 {
        struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
        int i, ret, attrcount = 0;
@@ -1273,7 +1273,7 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
 static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
                                               struct iio_chan_spec const *chan,
                                               enum iio_shared_by shared_by,
-                                              const long *infomask)
+                                              const unsigned long *infomask)
 {
        struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
        int i, ret, attrcount = 0;
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 2f5560646ee4..e45306821e25 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -271,14 +271,14 @@ struct iio_chan_spec {
                        unsigned int num_ext_scan_type;
                };
        };
-       long                    info_mask_separate;
-       long                    info_mask_separate_available;
-       long                    info_mask_shared_by_type;
-       long                    info_mask_shared_by_type_available;
-       long                    info_mask_shared_by_dir;
-       long                    info_mask_shared_by_dir_available;
-       long                    info_mask_shared_by_all;
-       long                    info_mask_shared_by_all_available;
+       unsigned long           info_mask_separate;
+       unsigned long           info_mask_separate_available;
+       unsigned long           info_mask_shared_by_type;
+       unsigned long           info_mask_shared_by_type_available;
+       unsigned long           info_mask_shared_by_dir;
+       unsigned long           info_mask_shared_by_dir_available;
+       unsigned long           info_mask_shared_by_all;
+       unsigned long           info_mask_shared_by_all_available;
        const struct iio_event_spec *event_spec;
        unsigned int            num_event_specs;
        const struct iio_chan_spec_ext_info *ext_info;

I don't think there are problems with how these are set as that is always
BIT()

Jonathan

> 
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
> ---
>  drivers/iio/industrialio-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 159d6c5ca3ce..e9491a999ac0 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1248,7 +1248,7 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
>  	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
>  	int i, ret, attrcount = 0;
>  
> -	for_each_set_bit(i, infomask, sizeof(*infomask)*8) {
> +	for_each_set_bit(i, (const unsigned long *)infomask, sizeof(*infomask) * 8) {
>  		if (i >= ARRAY_SIZE(iio_chan_info_postfix))
>  			return -EINVAL;
>  		ret = __iio_add_chan_devattr(iio_chan_info_postfix[i],
> @@ -1279,7 +1279,7 @@ static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
>  	int i, ret, attrcount = 0;
>  	char *avail_postfix;
>  
> -	for_each_set_bit(i, infomask, sizeof(*infomask) * 8) {
> +	for_each_set_bit(i, (const unsigned long *)infomask, sizeof(*infomask) * 8) {
>  		if (i >= ARRAY_SIZE(iio_chan_info_postfix))
>  			return -EINVAL;
>  		avail_postfix = kasprintf(GFP_KERNEL,

Re: [PATCH] iio: core: Cast info mask to 'unsigned long *' in bit-ops

[Andy Shevchenko]

On Sat, Aug 16, 2025 at 2:11 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 15 Aug 2025 10:25:28 +0800
> Junjie Cao <junjie.cao@intel.com> wrote:
>
> > for_each_set_bit()/find_*_bit() expect 'const unsigned long *' (see
> > include/linux/find.h), but industrialio-core.c passes 'const long *'
> > in iio_device_add_info_mask_type{,_avail}(). Sparse flags a signedness
> > mismatch.
>
> How do you get that warning?  I'm not seeing it.

With or without any warning the casting is a very bad practice for
bitmaps. NAK for any patch like this to any place in the kernel. I
already fixed some of such castings in the past (one may browse the
Git log with my name for that). Yes, I understand that in this case
it's not critical as the sizeof(long) == sizeof(unsigned long), but in
general this is not a good style leading to 3 potential issues
(out-of-boundary accesses or endianess).

> > The masks are used purely as bit arrays. Cast them to 'const unsigned
> > long *' at the call sites to match the helpers' prototypes, without
> > changing memory layout or behavior. Changing the field types would cause
> > unnecessary churn across IIO.
>
> Did you have a go?  Superficially feels like we should fix this up and
> that it should be almost entirely confined to relatively few core functions
> and the types of the various elements of struct iio_dev.
>
> Might just be (builds and I'm not seeing new warning but then I couldn't trigger
> the one you saw!)

Jonathan, the patch you provided LGTM as I have in mind to do
something like this a long time ago. Please, proceed with it. It would
be better to have unsigned long * for bitmaps to make it uniform and
follow the API.



-- 
With Best Regards,
Andy Shevchenko