| Message ID | 20210501172515.513486-5-jic23@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IIO: Alignment fixes part 4 - bounce buffers for the hard cases. | expand |
> From: Jonathan Cameron <jic23@kernel.org> > Sent: Saturday, May 1, 2021 7:25 PM > To: linux-iio@vger.kernel.org > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>; Sa, Nuno > <Nuno.Sa@analog.com> > Subject: [RFC PATCH 4/4] iio: imu: adis16400: Fix buffer alignment > requirements. > > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > iio_push_to_buffers_with_timestamp() requires that the buffer > is 8 byte alignment to ensure an inserted timestamp is naturally > aligned. > > This requirement was not met here when burst mode is in use beause > of a leading u16. Use the new iio_push_to_buffers_with_ts_na() > function > that has more relaxed requirements. > > It is somewhat complex to access that actual data length, but a > safe bound can be found by using scan_bytes - sizeof(timestamp) so > that > is used in this path. > > More efficient approaches exist, but this ensure correctness at the > cost of using a bounce buffer. > > Fixes: 5075e0720d93 ("iio: imu: adis: generalize burst mode support") > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Cc: Nuno Sa <nuno.sa@analog.com> > --- > drivers/iio/imu/adis16400.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c > index 768aa493a1a6..c6d03a37373b 100644 > --- a/drivers/iio/imu/adis16400.c > +++ b/drivers/iio/imu/adis16400.c > @@ -663,13 +663,23 @@ static irqreturn_t > adis16400_trigger_handler(int irq, void *p) > spi_setup(st->adis.spi); > } > > - if (st->variant->flags & ADIS16400_BURST_DIAG_STAT) > + if (st->variant->flags & ADIS16400_BURST_DIAG_STAT) { > buffer = adis->buffer + sizeof(u16); > - else > - buffer = adis->buffer; > + /* > + * The size here is always larger than, or equal to the > true > + * size of the channel data. This may result in a larger > copy > + * than necessary, but as the target buffer will be > + * buffer->scan_bytes this will be safe. > + */ > + iio_push_to_buffers_with_ts_na(indio_dev, buffer, > + indio_dev->scan_bytes - > sizeof(pf->timestamp), > + pf->timestamp); > + } else { > + iio_push_to_buffers_with_timestamp(indio_dev, > + adis->buffer, > + pf->timestamp); > + } > Hi Jonathan, This looks good to me although I think there's some stuff to care in ' iio_push_to_buffers_with_ts_na()'. However, for this patch alone: Reviewed-by: Nuno Sá <nuno.sa@analog.com> I will also check if I find any HW to test this... - Nuno Sá
diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c index 768aa493a1a6..c6d03a37373b 100644 --- a/drivers/iio/imu/adis16400.c +++ b/drivers/iio/imu/adis16400.c @@ -663,13 +663,23 @@ static irqreturn_t adis16400_trigger_handler(int irq, void *p) spi_setup(st->adis.spi); } - if (st->variant->flags & ADIS16400_BURST_DIAG_STAT) + if (st->variant->flags & ADIS16400_BURST_DIAG_STAT) { buffer = adis->buffer + sizeof(u16); - else - buffer = adis->buffer; + /* + * The size here is always larger than, or equal to the true + * size of the channel data. This may result in a larger copy + * than necessary, but as the target buffer will be + * buffer->scan_bytes this will be safe. + */ + iio_push_to_buffers_with_ts_na(indio_dev, buffer, + indio_dev->scan_bytes - sizeof(pf->timestamp), + pf->timestamp); + } else { + iio_push_to_buffers_with_timestamp(indio_dev, + adis->buffer, + pf->timestamp); + } - iio_push_to_buffers_with_timestamp(indio_dev, buffer, - pf->timestamp); iio_trigger_notify_done(indio_dev->trig);