Re: [PATCH v4 4/4] iio: adc: ltc2378: Enable triggered buffer data capture

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Single reads via sysfs (`in_voltage0_raw`) return scrambled/garbage data when the SPI offload engine is configured, because they incorrectly use the offload channel's endianness and shift properties.
--

commit 5a2926870e368170637e86d31e6346617e37a6b1
Author: Marcelo Schmitt <marcelo.schmitt@analog.com>

iio: adc: ltc2378: Enable triggered buffer data capture
    
Enable users to run triggered data captures with LTC2378 and similar ADCs.

> diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> index 5b28630003b18..fcccd27745490 100644
> --- a/drivers/iio/adc/ltc2378.c
> +++ b/drivers/iio/adc/ltc2378.c

[ ... ]

> @@ -640,8 +662,16 @@ static int ltc2378_probe(struct spi_device *spi)
>  	/* Fall back to low speed usage when no SPI offload is available. */
>  	if (ret == -ENODEV) {
>  		indio_dev->info = &ltc2378_iio_info;
> -		indio_dev->channels = &st->info->chan;
> -		indio_dev->num_channels = 1;
> +		indio_dev->channels = st->info->chan;
> +		indio_dev->num_channels = ARRAY_SIZE(st->info->chan);
> +
> +		ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> +						      iio_pollfunc_store_time,
> +						      ltc2378_trigger_handler,
> +						      NULL);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "failed to setup triggered buffer\n");
>  	} else if (ret) {
>  		return dev_err_probe(dev, ret, "failed to get offload\n");
>  	} else {

[Severity: High]
This is a pre-existing issue, but do single reads via sysfs
(in_voltage0_raw) return scrambled data when the SPI offload engine is
configured?

When offload is configured, indio_dev->channels is assigned to the
offload channel just after this else block in ltc2378_probe():

		indio_dev->channels = &st->info->offload_chan;

This channel specification is later passed to ltc2378_channel_single_read().
However, the single read function relies on the channel's endianness and
shift properties to decode the raw SPI data:

static int ltc2378_channel_single_read(const struct iio_chan_spec *chan, ...)
{
	...
	if (chan->scan_type.endianness == IIO_BE) {
		if (chan->scan_type.realbits > 16)
			sample = be32_to_cpu(st->scan.data.sample_buf32_be);
		else
			sample = be16_to_cpu(st->scan.data.sample_buf16_be);
	} else {
		if (chan->scan_type.realbits > 16)
			sample = st->scan.data.sample_buf32;
		else
			sample = st->scan.data.sample_buf16;
	}

	sample >>= chan->scan_type.shift;
	...
}

Because the offload_chan specification defines data as CPU-endian with
a 0 shift, the else branch is taken. This treats the natively big-endian,
MSB-aligned SPI data as unshifted CPU-endian data, scrambling the values on
little-endian architectures.

Could this be updated to properly handle manual sysfs reads when offload
is enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782397418.git.marcelo.schmitt@analog.com?part=4