[v3,01/27] iio: accel: kxsd9: Fix alignment of local buffer.
diff mbox series

Message ID 20200722155103.979802-2-jic23@kernel.org
State New
Headers show
Series
  • IIO: Fused set 1 and 2 of timestamp alignment fixes
Related show

Commit Message

Jonathan Cameron July 22, 2020, 3:50 p.m. UTC
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>
---
 drivers/iio/accel/kxsd9.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Jonathan Cameron Aug. 9, 2020, 5:07 p.m. UTC | #1
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);
>

Patch
diff mbox series

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);