Re: [PATCH v3 1/4] iio: light: ltr501: Switch to read_avail() op from manual attribute registration

[Jonathan Cameron]

On Sun,  8 Nov 2026 11:42:50 +0200
Svyatoslav Ryhel <clamor95@xxxxxxxxx> wrote:

> Switch to the read_avail() operation from manual attribute group
> registration. This eliminates the need for per-chip attribute_group
> structures and related ltr501_chip_info iio_info entries.
> 
> Adjust LTR501_INTENSITY_CHANNEL to account for the new
> info_mask_*_available layout, allowing the removal of _shared since all
> intensity channels share the same layout of available entries.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
Sashiko caught the bug I noticed as well..
https://sashiko.dev/#/patchset/20261108094253.83899-1-clamor95%40gmail.com
> ---
>  drivers/iio/light/ltr501.c | 192 +++++++++++++++----------------------
>  1 file changed, 76 insertions(+), 116 deletions(-)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 302be2fc314c5..1ddd0dfd55ea2 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -133,6 +133,22 @@ static const struct ltr501_gain ltr559_ps_gain_tbl[] = {
>  	{0, 15624},
>  };
>  
> +static const int int_time_avail[] = {
> +	0, 50000,
> +	0, 100000,
> +	0, 200000,
> +	0, 400000
> +};
> +
> +static const int samp_freq_avail[] = {
> +	20, 0,
> +	10, 0,
> +	5,  0,
> +	2,  0,
> +	1,  0,
> +	0,  500000
> +};

> -static ssize_t ltr501_show_proximity_scale_avail(struct device *dev,
> -						 struct device_attribute *attr,
> -						 char *buf)
> +static int ltr501_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     const int **vals, int *type, int *length, long mask)
>  {
> -	struct ltr501_data *data = iio_priv(dev_to_iio_dev(dev));
> +	struct ltr501_data *data = iio_priv(indio_dev);
>  	const struct ltr501_chip_info *info = data->chip_info;
> -	ssize_t len = 0;
> -	int i;
> -
> -	for (i = 0; i < info->ps_gain_tbl_size; i++) {
> -		if (info->ps_gain[i].scale == LTR501_RESERVED_GAIN)
> -			continue;
> -		len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06d ",
> -				 info->ps_gain[i].scale,
> -				 info->ps_gain[i].uscale);
> -	}
>  
> -	buf[len - 1] = '\n';
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		if (chan->type == IIO_INTENSITY) {
> +			int valid_als_gains[ARRAY_SIZE(ltr559_als_gain_tbl) * 2];
> +			int j = 0;
> +
> +			for (int i = 0; i < info->als_gain_tbl_size; i++) {
> +				if (info->als_gain[i].scale == LTR501_RESERVED_GAIN)
> +					continue;
> +
> +				valid_als_gains[j++] = info->als_gain[i].scale;
> +				valid_als_gains[j++] = info->als_gain[i].uscale;
> +			}
> +
> +			*length = j;
> +			*vals = (const int *)valid_als_gains;

As sashiko points out you just passed out a dangling pointer to the stack.
So this never works.  Even without that, there are some rather badly handled
issues with consumers of read_avail and lifetimes.  We had some fixes about
a year ago but they never made it upstream and it's not currently high
priority for me at least to revisit.  Upshot is that we are much better
returning a pointer to pseudo const data.  Can initialise before registering
the device but after that we should only use pointers that remain stable
and not change the data so as to avoid potential tearing.

Given the reserved entries are in the middle, you'll need to stash a copy
of the array with them scrubbed out. Do that in probe and put it somewhere
in the iio_priv() structure

> +		} else {
> +			*length = info->ps_gain_tbl_size * 2;
> +			*vals = (const int *)info->ps_gain;
> +		}
> +		*type = IIO_VAL_INT_PLUS_MICRO;
> +		return IIO_AVAIL_LIST;
>  
> -	return len;
> -}
> +	case IIO_CHAN_INFO_INT_TIME:
> +		*vals = int_time_avail;
> +		*type = IIO_VAL_INT_PLUS_MICRO;
> +		*length = ARRAY_SIZE(int_time_avail);
> +		return IIO_AVAIL_LIST;