Re: [PATCH v10 4/6] iio: adc: ad4691: add SPI offload support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The software-triggered buffer and SPI offload paths produce memory buffers with differing endianness, but they share the exact same IIO channel `scan_type`, corrupting data on Little-Endian hosts for one of the paths.
--

commit 7a6cc358e39a6b61c1d3018b7172b6303e1f28e5
Author: Radu Sabau <radu.sabau@analog.com>

iio: adc: ad4691: add SPI offload support

This commit adds SPI offload support to enable DMA-based, CPU-independent
data acquisition using the SPI Engine offload framework. It supports both
CNV Burst Mode and Manual Mode, sharing the channel scan_type between the
software-triggered and offload paths.

> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c

[ ... ]

> @@ -847,6 +924,223 @@ static const struct iio_buffer_setup_ops ad4691_cnv_burst_buffer_setup_ops = {

[ ... ]

> +static int ad4691_cnv_burst_offload_buffer_postenable(struct iio_dev *indio_dev)
> +{

[ ... ]

> +	iio_for_each_active_channel(indio_dev, bit) {
> +		st->scan_tx[k] = 0x8000 | AD4691_AVG_IN(bit);
> +
> +		/* TX: address phase, CS stays asserted into data phase */
> +		st->scan_xfers[2 * k].tx_buf = &st->scan_tx[k];
> +		st->scan_xfers[2 * k].len = sizeof(st->scan_tx[k]);
> +		st->scan_xfers[2 * k].bits_per_word = bpw;
> +
> +		/* RX: data phase, CS toggles after to delimit the next register op */
> +		st->scan_xfers[2 * k + 1].len = sizeof(st->scan_tx[k]);
> +		st->scan_xfers[2 * k + 1].bits_per_word = bpw;
> +		st->scan_xfers[2 * k + 1].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> +		st->scan_xfers[2 * k + 1].cs_change = 1;
> +		k++;
> +	}

Will this offload RX stream configuration result in different endianness
compared to the software-triggered path?

In the software-triggered path, the SPI RX uses the default bits_per_word
of 8, meaning the ADC's MSB-first 16-bit samples are stored byte-by-byte in
memory as Big-Endian.

However, here in the offload path, bits_per_word is set to 16. The SPI
engine will reconstruct the 16-bit frames, and the DMA controller will
write them to memory as native 16-bit words, which will be Little-Endian
on LE hosts.

Since both paths share the same IIO channel scan_type definition (which
lacks an explicit endianness and defaults to IIO_CPU), won't one of the
paths produce incorrectly decoded data for userspace on Little-Endian
systems?

It is understood that a subsequent commit ("iio: adc: ad4691: add
oversampling support") adds an endianness of IIO_BE to the channel
definitions to address the software path. Since the offload path still
shares these definitions, this would cause IIO to advertise the offload
data as Big-Endian even though it provides CPU-endian data.

It appears the driver might need separate channel arrays for the offload
path with endianness set to IIO_CPU, or it needs to byte-swap the software
buffer to CPU endianness before pushing it to IIO.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-ad4692-multichannel-sar-adc-driver-v10-0-e1fbb1744e38@analog.com?part=4