[PATCH] iio: adc: ti-ads8688: use read_avail for available attributes

[PATCH] iio: adc: ti-ads8688: use read_avail for available attributes

[Gabriel Rondon]

Convert the in_voltage_scale_available and in_voltage_offset_available
attributes from legacy IIO_DEVICE_ATTR with custom show functions to the
IIO framework's read_avail callback. This uses the framework's built-in
support for _available attributes, removing the need for manual sysfs
formatting.

Precompute the available scale values at probe time since they depend on
the reference voltage which does not change after initialization.

Signed-off-by: Gabriel Rondon <grondon@gmail.com>
---
 drivers/iio/adc/ti-ads8688.c | 73 +++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/adc/ti-ads8688.c b/drivers/iio/adc/ti-ads8688.c
index b0bf46cae..f37098db9 100644
--- a/drivers/iio/adc/ti-ads8688.c
+++ b/drivers/iio/adc/ti-ads8688.c
@@ -6,7 +6,6 @@
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
-#include <linux/sysfs.h>
 #include <linux/spi/spi.h>
 #include <linux/regulator/consumer.h>
 #include <linux/err.h>
@@ -17,7 +16,6 @@
 #include <linux/iio/buffer.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
-#include <linux/iio/sysfs.h>
 
 #define ADS8688_CMD_REG(x)		(x << 8)
 #define ADS8688_CMD_REG_NOOP		0x00
@@ -66,6 +64,7 @@ struct ads8688_state {
 	const struct ads8688_chip_info	*chip_info;
 	struct spi_device		*spi;
 	unsigned int			vref_mv;
+	int				scale_avail[3][2];
 	enum ads8688_range		range[8];
 	union {
 		__be32 d32;
@@ -114,37 +113,9 @@ static const struct ads8688_ranges ads8688_range_def[5] = {
 	}
 };
 
-static ssize_t ads8688_show_scales(struct device *dev,
-				   struct device_attribute *attr, char *buf)
-{
-	struct ads8688_state *st = iio_priv(dev_to_iio_dev(dev));
-
-	return sprintf(buf, "0.%09u 0.%09u 0.%09u\n",
-		       ads8688_range_def[0].scale * st->vref_mv,
-		       ads8688_range_def[1].scale * st->vref_mv,
-		       ads8688_range_def[2].scale * st->vref_mv);
-}
-
-static ssize_t ads8688_show_offsets(struct device *dev,
-				    struct device_attribute *attr, char *buf)
-{
-	return sprintf(buf, "%d %d\n", ads8688_range_def[0].offset,
-		       ads8688_range_def[3].offset);
-}
-
-static IIO_DEVICE_ATTR(in_voltage_scale_available, S_IRUGO,
-		       ads8688_show_scales, NULL, 0);
-static IIO_DEVICE_ATTR(in_voltage_offset_available, S_IRUGO,
-		       ads8688_show_offsets, NULL, 0);
-
-static struct attribute *ads8688_attributes[] = {
-	&iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
-	&iio_dev_attr_in_voltage_offset_available.dev_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group ads8688_attribute_group = {
-	.attrs = ads8688_attributes,
+static const int ads8688_offset_avail[] = {
+	-(1 << (ADS8688_REALBITS - 1)),
+	0,
 };
 
 #define ADS8688_CHAN(index)					\
@@ -155,6 +126,9 @@ static const struct attribute_group ads8688_attribute_group = {
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)		\
 			      | BIT(IIO_CHAN_INFO_SCALE)	\
 			      | BIT(IIO_CHAN_INFO_OFFSET),	\
+	.info_mask_shared_by_type_available =			\
+			      BIT(IIO_CHAN_INFO_SCALE)		\
+			      | BIT(IIO_CHAN_INFO_OFFSET),	\
 	.scan_index = index,					\
 	.scan_type = {						\
 		.sign = 'u',					\
@@ -369,11 +343,34 @@ static int ads8688_write_raw_get_fmt(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static int ads8688_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	struct ads8688_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (const int *)st->scale_avail;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		*length = ARRAY_SIZE(st->scale_avail) * 2;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_OFFSET:
+		*vals = ads8688_offset_avail;
+		*type = IIO_VAL_INT;
+		*length = ARRAY_SIZE(ads8688_offset_avail);
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
 static const struct iio_info ads8688_info = {
 	.read_raw = &ads8688_read_raw,
+	.read_avail = &ads8688_read_avail,
 	.write_raw = &ads8688_write_raw,
 	.write_raw_get_fmt = &ads8688_write_raw_get_fmt,
-	.attrs = &ads8688_attribute_group,
 };
 
 static irqreturn_t ads8688_trigger_handler(int irq, void *p)
@@ -412,7 +409,7 @@ static int ads8688_probe(struct spi_device *spi)
 {
 	struct ads8688_state *st;
 	struct iio_dev *indio_dev;
-	int ret;
+	int ret, i;
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
 	if (indio_dev == NULL)
@@ -426,6 +423,12 @@ static int ads8688_probe(struct spi_device *spi)
 
 	st->vref_mv = ret == -ENODEV ? ADS8688_VREF_MV : ret / 1000;
 
+	for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) {
+		st->scale_avail[i][0] = 0;
+		st->scale_avail[i][1] = ads8688_range_def[i].scale *
+					st->vref_mv;
+	}
+
 	st->chip_info =	&ads8688_chip_info_tbl[spi_get_device_id(spi)->driver_data];
 
 	spi->mode = SPI_MODE_1;
-- 
2.33.0

Re: [PATCH] iio: adc: ti-ads8688: use read_avail for available attributes

[Nuno Sá]

On Mon, 2026-03-23 at 21:56 +0000, Gabriel Rondon wrote:
> Convert the in_voltage_scale_available and in_voltage_offset_available
> attributes from legacy IIO_DEVICE_ATTR with custom show functions to the
> IIO framework's read_avail callback. This uses the framework's built-in
> support for _available attributes, removing the need for manual sysfs
> formatting.
> 
> Precompute the available scale values at probe time since they depend on
> the reference voltage which does not change after initialization.
> 
> Signed-off-by: Gabriel Rondon <grondon@gmail.com>
> ---

Hi Grabriel,

Thanks for your patch, just some minor nits from me. Anyways:

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

>  drivers/iio/adc/ti-ads8688.c | 73 +++++++++++++++++++-----------------
>  1 file changed, 38 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads8688.c b/drivers/iio/adc/ti-ads8688.c
> index b0bf46cae..f37098db9 100644
> --- a/drivers/iio/adc/ti-ads8688.c
> +++ b/drivers/iio/adc/ti-ads8688.c
> @@ -6,7 +6,6 @@
>  #include <linux/device.h>
>  #include <linux/kernel.h>
>  #include <linux/slab.h>
> -#include <linux/sysfs.h>
>  #include <linux/spi/spi.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/err.h>
> @@ -17,7 +16,6 @@
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/triggered_buffer.h>
> -#include <linux/iio/sysfs.h>
>  
>  #define ADS8688_CMD_REG(x)		(x << 8)
>  #define ADS8688_CMD_REG_NOOP		0x00
> @@ -66,6 +64,7 @@ struct ads8688_state {
>  	const struct ads8688_chip_info	*chip_info;
>  	struct spi_device		*spi;
>  	unsigned int			vref_mv;
> +	int				scale_avail[3][2];
>  	enum ads8688_range		range[8];
>  	union {
>  		__be32 d32;
> @@ -114,37 +113,9 @@ static const struct ads8688_ranges ads8688_range_def[5] = {
>  	}
>  };
>  
> -static ssize_t ads8688_show_scales(struct device *dev,
> -				   struct device_attribute *attr, char *buf)
> -{
> -	struct ads8688_state *st = iio_priv(dev_to_iio_dev(dev));
> -
> -	return sprintf(buf, "0.%09u 0.%09u 0.%09u\n",
> -		       ads8688_range_def[0].scale * st->vref_mv,
> -		       ads8688_range_def[1].scale * st->vref_mv,
> -		       ads8688_range_def[2].scale * st->vref_mv);
> -}
> -
> -static ssize_t ads8688_show_offsets(struct device *dev,
> -				    struct device_attribute *attr, char *buf)
> -{
> -	return sprintf(buf, "%d %d\n", ads8688_range_def[0].offset,
> -		       ads8688_range_def[3].offset);
> -}
> -
> -static IIO_DEVICE_ATTR(in_voltage_scale_available, S_IRUGO,
> -		       ads8688_show_scales, NULL, 0);
> -static IIO_DEVICE_ATTR(in_voltage_offset_available, S_IRUGO,
> -		       ads8688_show_offsets, NULL, 0);
> -
> -static struct attribute *ads8688_attributes[] = {
> -	&iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
> -	&iio_dev_attr_in_voltage_offset_available.dev_attr.attr,
> -	NULL,
> -};
> -
> -static const struct attribute_group ads8688_attribute_group = {
> -	.attrs = ads8688_attributes,
> +static const int ads8688_offset_avail[] = {
> +	-(1 << (ADS8688_REALBITS - 1)),
> +	0,
>  };
>  
>  #define ADS8688_CHAN(index)					\
> @@ -155,6 +126,9 @@ static const struct attribute_group ads8688_attribute_group = {
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)		\
>  			      | BIT(IIO_CHAN_INFO_SCALE)	\
>  			      | BIT(IIO_CHAN_INFO_OFFSET),	\
> +	.info_mask_shared_by_type_available =			\
> +			      BIT(IIO_CHAN_INFO_SCALE)		\
> +			      | BIT(IIO_CHAN_INFO_OFFSET),	\
>  	.scan_index = index,					\
>  	.scan_type = {						\
>  		.sign = 'u',					\
> @@ -369,11 +343,34 @@ static int ads8688_write_raw_get_fmt(struct iio_dev *indio_dev,
>  	return -EINVAL;
>  }
>  
> +static int ads8688_read_avail(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      const int **vals, int *type, int *length,
> +			      long mask)
> +{
> +	struct ads8688_state *st = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		*vals = (const int *)st->scale_avail;
> +		*type = IIO_VAL_INT_PLUS_NANO;
> +		*length = ARRAY_SIZE(st->scale_avail) * 2;
> +		return IIO_AVAIL_LIST;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*vals = ads8688_offset_avail;
> +		*type = IIO_VAL_INT;
> +		*length = ARRAY_SIZE(ads8688_offset_avail);
> +		return IIO_AVAIL_LIST;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static const struct iio_info ads8688_info = {
>  	.read_raw = &ads8688_read_raw,
> +	.read_avail = &ads8688_read_avail,
>  	.write_raw = &ads8688_write_raw,
>  	.write_raw_get_fmt = &ads8688_write_raw_get_fmt,
> -	.attrs = &ads8688_attribute_group,
>  };
>  
>  static irqreturn_t ads8688_trigger_handler(int irq, void *p)
> @@ -412,7 +409,7 @@ static int ads8688_probe(struct spi_device *spi)
>  {
>  	struct ads8688_state *st;
>  	struct iio_dev *indio_dev;
> -	int ret;
> +	int ret, i;

These days you could just declare i in the for() loop and make it
unsigned int.

>  
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>  	if (indio_dev == NULL)
> @@ -426,6 +423,12 @@ static int ads8688_probe(struct spi_device *spi)
>  
>  	st->vref_mv = ret == -ENODEV ? ADS8688_VREF_MV : ret / 1000;
>  
> +	for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) {
> +		st->scale_avail[i][0] = 0;
> +		st->scale_avail[i][1] = ads8688_range_def[i].scale *
> +					st->vref_mv;
> +	}

Even though the above crosses the column preferred limit, I don't think
the line break hurts readability. 

- Nuno Sá
> 

Re: [PATCH] iio: adc: ti-ads8688: use read_avail for available attributes

[Gabriel Rondon]

On Tue, 24 Mar 2026 11:42:36 +0000, Nuno Sá wrote:
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>

Thanks for the review.

> These days you could just declare i in the for() loop and make it
> unsigned int.

Will do.

Gabriel

Re: [PATCH] iio: adc: ti-ads8688: use read_avail for available attributes

[Nuno Sá]

On Tue, 2026-03-24 at 11:42 +0000, Nuno Sá wrote:
> On Mon, 2026-03-23 at 21:56 +0000, Gabriel Rondon wrote:
> > Convert the in_voltage_scale_available and in_voltage_offset_available
> > attributes from legacy IIO_DEVICE_ATTR with custom show functions to the
> > IIO framework's read_avail callback. This uses the framework's built-in
> > support for _available attributes, removing the need for manual sysfs
> > formatting.
> > 
> > Precompute the available scale values at probe time since they depend on
> > the reference voltage which does not change after initialization.
> > 
> > Signed-off-by: Gabriel Rondon <grondon@gmail.com>
> > ---
> 
> Hi Grabriel,
> 
> Thanks for your patch, just some minor nits from me. Anyways:
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> 
> >  drivers/iio/adc/ti-ads8688.c | 73 +++++++++++++++++++-----------------
> >  1 file changed, 38 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ti-ads8688.c b/drivers/iio/adc/ti-ads8688.c
> > index b0bf46cae..f37098db9 100644
> > --- a/drivers/iio/adc/ti-ads8688.c
> > +++ b/drivers/iio/adc/ti-ads8688.c
> > @@ -6,7 +6,6 @@
> >  #include <linux/device.h>
> >  #include <linux/kernel.h>
> >  #include <linux/slab.h>
> > -#include <linux/sysfs.h>
> >  #include <linux/spi/spi.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/err.h>
> > @@ -17,7 +16,6 @@
> >  #include <linux/iio/buffer.h>
> >  #include <linux/iio/trigger_consumer.h>
> >  #include <linux/iio/triggered_buffer.h>
> > -#include <linux/iio/sysfs.h>
> >  
> >  #define ADS8688_CMD_REG(x)		(x << 8)
> >  #define ADS8688_CMD_REG_NOOP		0x00
> > @@ -66,6 +64,7 @@ struct ads8688_state {
> >  	const struct ads8688_chip_info	*chip_info;
> >  	struct spi_device		*spi;
> >  	unsigned int			vref_mv;
> > +	int				scale_avail[3][2];
> >  	enum ads8688_range		range[8];
> >  	union {
> >  		__be32 d32;
> > @@ -114,37 +113,9 @@ static const struct ads8688_ranges ads8688_range_def[5] = {
> >  	}
> >  };
> >  
> > -static ssize_t ads8688_show_scales(struct device *dev,
> > -				   struct device_attribute *attr, char *buf)
> > -{
> > -	struct ads8688_state *st = iio_priv(dev_to_iio_dev(dev));
> > -
> > -	return sprintf(buf, "0.%09u 0.%09u 0.%09u\n",
> > -		       ads8688_range_def[0].scale * st->vref_mv,
> > -		       ads8688_range_def[1].scale * st->vref_mv,
> > -		       ads8688_range_def[2].scale * st->vref_mv);
> > -}
> > -
> > -static ssize_t ads8688_show_offsets(struct device *dev,
> > -				    struct device_attribute *attr, char *buf)
> > -{
> > -	return sprintf(buf, "%d %d\n", ads8688_range_def[0].offset,
> > -		       ads8688_range_def[3].offset);
> > -}
> > -
> > -static IIO_DEVICE_ATTR(in_voltage_scale_available, S_IRUGO,
> > -		       ads8688_show_scales, NULL, 0);
> > -static IIO_DEVICE_ATTR(in_voltage_offset_available, S_IRUGO,
> > -		       ads8688_show_offsets, NULL, 0);
> > -
> > -static struct attribute *ads8688_attributes[] = {
> > -	&iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
> > -	&iio_dev_attr_in_voltage_offset_available.dev_attr.attr,
> > -	NULL,
> > -};
> > -
> > -static const struct attribute_group ads8688_attribute_group = {
> > -	.attrs = ads8688_attributes,
> > +static const int ads8688_offset_avail[] = {
> > +	-(1 << (ADS8688_REALBITS - 1)),
> > +	0,
> >  };
> >  
> >  #define ADS8688_CHAN(index)					\
> > @@ -155,6 +126,9 @@ static const struct attribute_group ads8688_attribute_group = {
> >  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)		\
> >  			      | BIT(IIO_CHAN_INFO_SCALE)	\
> >  			      | BIT(IIO_CHAN_INFO_OFFSET),	\
> > +	.info_mask_shared_by_type_available =			\
> > +			      BIT(IIO_CHAN_INFO_SCALE)		\
> > +			      | BIT(IIO_CHAN_INFO_OFFSET),	\
> >  	.scan_index = index,					\
> >  	.scan_type = {						\
> >  		.sign = 'u',					\
> > @@ -369,11 +343,34 @@ static int ads8688_write_raw_get_fmt(struct iio_dev *indio_dev,
> >  	return -EINVAL;
> >  }
> >  
> > +static int ads8688_read_avail(struct iio_dev *indio_dev,
> > +			      struct iio_chan_spec const *chan,
> > +			      const int **vals, int *type, int *length,
> > +			      long mask)
> > +{
> > +	struct ads8688_state *st = iio_priv(indio_dev);
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_SCALE:
> > +		*vals = (const int *)st->scale_avail;
> > +		*type = IIO_VAL_INT_PLUS_NANO;
> > +		*length = ARRAY_SIZE(st->scale_avail) * 2;
> > +		return IIO_AVAIL_LIST;
> > +	case IIO_CHAN_INFO_OFFSET:
> > +		*vals = ads8688_offset_avail;
> > +		*type = IIO_VAL_INT;
> > +		*length = ARRAY_SIZE(ads8688_offset_avail);
> > +		return IIO_AVAIL_LIST;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> >  static const struct iio_info ads8688_info = {
> >  	.read_raw = &ads8688_read_raw,
> > +	.read_avail = &ads8688_read_avail,
> >  	.write_raw = &ads8688_write_raw,
> >  	.write_raw_get_fmt = &ads8688_write_raw_get_fmt,
> > -	.attrs = &ads8688_attribute_group,
> >  };
> >  
> >  static irqreturn_t ads8688_trigger_handler(int irq, void *p)
> > @@ -412,7 +409,7 @@ static int ads8688_probe(struct spi_device *spi)
> >  {
> >  	struct ads8688_state *st;
> >  	struct iio_dev *indio_dev;
> > -	int ret;
> > +	int ret, i;
> 
> These days you could just declare i in the for() loop and make it
> unsigned int.
> 
> >  
> >  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> >  	if (indio_dev == NULL)
> > @@ -426,6 +423,12 @@ static int ads8688_probe(struct spi_device *spi)
> >  
> >  	st->vref_mv = ret == -ENODEV ? ADS8688_VREF_MV : ret / 1000;
> >  
> > +	for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) {
> > +		st->scale_avail[i][0] = 0;
> > +		st->scale_avail[i][1] = ads8688_range_def[i].scale *
> > +					st->vref_mv;
> > +	}
> 
> Even though the above crosses the column preferred limit, I don't think
> the line break hurts readability. 

Naturally I meant that the line break __hurts__ (to me at least :))

- Nuno Sá
> 
> - Nuno Sá
> > 

Re: [PATCH] iio: adc: ti-ads8688: use read_avail for available attributes

[Andy Shevchenko]

On Mon, Mar 23, 2026 at 09:56:33PM +0000, Gabriel Rondon wrote:
> Convert the in_voltage_scale_available and in_voltage_offset_available
> attributes from legacy IIO_DEVICE_ATTR with custom show functions to the
> IIO framework's read_avail callback. This uses the framework's built-in
> support for _available attributes, removing the need for manual sysfs
> formatting.
> 
> Precompute the available scale values at probe time since they depend on
> the reference voltage which does not change after initialization.

...

> +static const int ads8688_offset_avail[] = {
> +	-(1 << (ADS8688_REALBITS - 1)),

This is fragile code (however it probably won't be exposed IRL)
if ADS8688_REALBITS == 32 this is UB in accordance with C standard.

Also the trick with -BIT(x) is harder to read than simple GENMASK().
If ADS8688_REALBITS == 1, this becomes -1 (all ones), is it correct?

> +	0,

Do not add trailing comma to a terminator.

>  };

...

> static const struct attribute_group ads8688_attribute_group = {

>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)		\
>  			      | BIT(IIO_CHAN_INFO_SCALE)	\
>  			      | BIT(IIO_CHAN_INFO_OFFSET),	\
> +	.info_mask_shared_by_type_available =			\
> +			      BIT(IIO_CHAN_INFO_SCALE)		\
> +			      | BIT(IIO_CHAN_INFO_OFFSET),	\

This is actually slightly different style to what is put in the above.
Ideally should be unified, but probably matter for another change
(if even needed).

>  	.scan_index = index,					\
>  	.scan_type = {						\
>  		.sign = 'u',					\

>  }

...

> -	int ret;
> +	int ret, i;

Why is 'i' signed?

...


> +	for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++) {

	for (unsigned int i = 0; i < ARRAY_SIZE(st->scale_avail); i++) {

should be good.

> +		st->scale_avail[i][0] = 0;

> +		st->scale_avail[i][1] = ads8688_range_def[i].scale *
> +					st->vref_mv;

I would leave this on a single line (it's only 81 characters).

> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: adc: ti-ads8688: use read_avail for available attributes

[Gabriel Rondon]

On Tue, 24 Mar 2026 13:42:40 +0200, Andy Shevchenko wrote:
> This is fragile code (however it probably won't be exposed IRL)
> if ADS8688_REALBITS == 32 this is UB in accordance with C standard.
>
> Also the trick with -BIT(x) is harder to read than simple GENMASK().

Good point. Will rework the expression in v2.

> Do not add trailing comma to a terminator.

Will fix.

> This is actually slightly different style to what is put in the above.
> Ideally should be unified, but probably matter for another change

Agreed, will leave for a separate change.

> Why is 'i' signed?

Will declare as unsigned int in the for loop.

> I would leave this on a single line (it's only 81 characters).

Will do.

Thanks for the review.

Gabriel

Re: [PATCH] iio: adc: ti-ads8688: use read_avail for available attributes

[Jonathan Cameron]

On Tue, 24 Mar 2026 13:42:40 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, Mar 23, 2026 at 09:56:33PM +0000, Gabriel Rondon wrote:
> > Convert the in_voltage_scale_available and in_voltage_offset_available
> > attributes from legacy IIO_DEVICE_ATTR with custom show functions to the
> > IIO framework's read_avail callback. This uses the framework's built-in
> > support for _available attributes, removing the need for manual sysfs
> > formatting.
> > 
> > Precompute the available scale values at probe time since they depend on
> > the reference voltage which does not change after initialization.  
> 
> ...
> 
> > +static const int ads8688_offset_avail[] = {
> > +	-(1 << (ADS8688_REALBITS - 1)),  
> 
> This is fragile code (however it probably won't be exposed IRL)
> if ADS8688_REALBITS == 32 this is UB in accordance with C standard.
> 
> Also the trick with -BIT(x) is harder to read than simple GENMASK().
> If ADS8688_REALBITS == 1, this becomes -1 (all ones), is it correct?
> 
> > +	0,  

Replying here because the follow up lost too much context.
This is a classic offset for bipolar ADC, which is - half the full
range binary value.  So we won't hit the edge cases, but maybe
expressing it different will be clearer.

-(GENMASK(ADS8688_REALBITS - 1, 0) >> 1) does feel a bit overkill
but -GENMASK(ADS8688_REALBITS - 2, 0) feels under explained (much
like the current).

We 'could' use -(U16MAX >> 1) but that feels very specific to
it being 16 bits.

Maybe just leave this as it already is in the table the value
is copied from.