Re: [PATCH] iio: adc: ad7766: Update to use iio_push_to_buffers_with_ts()

[David Lechner]

On 5/10/26 5:14 PM, Ethan Tidmore wrote:
> The old ABI function iio_push_to_buffers_with_timestamp() is no longer
> preferred due to it being inherently unsafe.
> 
> Update to the current standard iio_push_to_buffers_with_ts().
> 
> Signed-off-by: Ethan Tidmore <ethantidmore06@xxxxxxxxx>
> ---
> I asked a while back regarding possible things that would suffice for a
> GSoC proposal, that has came and went, but I remember Jonathan
> mentioning wanting this to be done. If this is correct, would a series
> be preferred next time?

When I look at these, I always look at how the buffer is being defined
and used elsewhere to see if there are any bugs or room for improvement.

In this case, we could consider changing:

	unsigned char data[ALIGN(3, sizeof(s64)) + sizeof(s64)]	__aligned(IIO_DMA_MINALIGN);

to something a bit easier to understand.

The most direct replacement would be:

	IIO_DECLARE_DMA_BUFFER_WITH_TS(u8, data, 4);

In addition to improving readability, there are two more improvements
in this change that need to be called out. I used u8 because that is
shorter than unsigned char, but it is the same type.

And the 3 in the original implementation is actually wrong. The SPI
transfer is doing:

	/* First byte always 0 */
	ad7766->xfer.rx_buf = &ad7766->data[1];
	ad7766->xfer.len = 3;

So it skipping the first element of the array and writing to the next
3 elements, so we actually need to declare 4 elements in the array.

Other than those 2 changes, the macro expands to the original code.

In cases like this where there is only one channel (or cases where there
are multiple channels, but we always read all channels), we prefer a struct
instead to make it easier to see the layout.

This case is a bit odd since there isn't a 24-bit data type but it works
to use a struct anyway. So we could also write the buffer declaration as:

	struct {
		/* 24-bit big-endian value stored in 32-bits */
		u8 data[4]:
		aligned_s64 ts;
	} __aligned(IIO_DMA_MINALIGN);


> 
>  drivers/iio/adc/ad7766.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7766.c b/drivers/iio/adc/ad7766.c
> index 9e4a66477d2d..4f256ce522c2 100644
> --- a/drivers/iio/adc/ad7766.c
> +++ b/drivers/iio/adc/ad7766.c
> @@ -74,8 +74,8 @@ static irqreturn_t ad7766_trigger_handler(int irq, void *p)
>  	if (ret < 0)
>  		goto done;
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, ad7766->data,
> -		pf->timestamp);
> +	iio_push_to_buffers_with_ts(indio_dev, ad7766->data, sizeof(ad7766->data),
> +				    pf->timestamp);
>  done:
>  	iio_trigger_notify_done(indio_dev->trig);
>