Re: [PATCH v3 01/27] iio: accel: kxsd9: Fix alignment of local buffer.

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 22 Jul 2020 16:50:37 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> iio_push_to_buffers_with_timestamp assumes 8 byte alignment which
> is not guaranteed by an array of smaller elements.
> 
> Note that whilst in this particular case the alignment forcing
> of the ts element is not strictly necessary it acts as good
> documentation.  Doing this where not necessary should cut
> down on the number of cut and paste introduced errors elsewhere.
> 
> Reported-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan



> ---
>  drivers/iio/accel/kxsd9.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 66b2e4cf24cf..0e18b92e2099 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -209,14 +209,20 @@ static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
>  	const struct iio_poll_func *pf = p;
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
> +	/*
> +	 * Ensure correct positioning and alignment of timestamp.
> +	 * No need to zero initialize as all elements written.
> +	 */
> +	struct {
> +		__be16 chan[4];
> +		s64 ts __aligned(8);
> +	} hw_values;
>  	int ret;
> -	/* 4 * 16bit values AND timestamp */
> -	__be16 hw_values[8];
>  
>  	ret = regmap_bulk_read(st->map,
>  			       KXSD9_REG_X,
> -			       &hw_values,
> -			       8);
> +			       hw_values.chan,
> +			       sizeof(hw_values.chan));
>  	if (ret) {
>  		dev_err(st->dev,
>  			"error reading data\n");
> @@ -224,7 +230,7 @@ static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
>  	}
>  
>  	iio_push_to_buffers_with_timestamp(indio_dev,
> -					   hw_values,
> +					   &hw_values,
>  					   iio_get_time_ns(indio_dev));
>  	iio_trigger_notify_done(indio_dev->trig);
>