Re: [PATCH 04/16] iio: light: vcnl4035: Use a structure to make buffer arrangement explicit.

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 02/08/2025 19:44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Using a structure to arrange the data to be passed to
> iio_push_to_buffers_with_timestamp() makes the placement and padding
> explicit, removing the need for comments to explain what is going on.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>   drivers/iio/light/vcnl4035.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
> index 79ec41b60530..dca229e74725 100644
> --- a/drivers/iio/light/vcnl4035.c
> +++ b/drivers/iio/light/vcnl4035.c
> @@ -103,7 +103,10 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
>   	struct iio_dev *indio_dev = pf->indio_dev;
>   	struct vcnl4035_data *data = iio_priv(indio_dev);
>   	/* Ensure naturally aligned timestamp */
> -	u8 buffer[ALIGN(sizeof(u16), sizeof(s64)) + sizeof(s64)]  __aligned(8) = { };
> +	struct {
> +		u16 chan;
> +		aligned_s64 ts;
> +	} scan;
>   	int val;
>   	int ret;
>   
> @@ -113,8 +116,8 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
>   			"Trigger consumer can't read from sensor.\n");
>   		goto fail_read;
>   	}
> -	*((u16 *)buffer) = val;
> -	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> +	scan.chan = val;
> +	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
>   					iio_get_time_ns(indio_dev));
>   
>   fail_read:

I suppose I was too hasty commenting the previous patch :) Besides, I 
misread the buffer definition. I just thought there were room for the 
u16 and couple of s64s. Hence I suggested a packed struct...

Now, re-reading this and seeing your patch - this looks correct :)

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

Yours,
	-- Matti