| Message ID | 20190507082304.11692-1-sean@geanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] iio: adc: ti-ads8688: save values correct in buffer | expand |
On Tue, 7 May 2019 10:23:03 +0200 Sean Nyekjaer <sean@geanix.com> wrote: > Fill the read values in the buffer so we comply > with the given index values. Why? They are not supposed to always remain in the same place. If some channels are turned off then they will shift 'down'. That has to happen in general to allow us to do more efficient packing when they happen to fit into a smaller power of 2 size. So if channels 0-3 (8 bits each) are enabled and timestamp, it should be 0 1 2 3 X X X X Timestamp, 0 1 2 3 X X X X Timestamp.. If no timestamp it should fall back to the packing 0 1 2 3, 0 1 2 3 If only channels 0, 1 and 3 we should get 0 1 3 X, 0 1 3 X (padding to 32 bits) For only 0 and 3 0 3, 0 3, 0 3 For only 0 0, 0, 0, 0, 0, 0 Jonathan > > Fixes: 2a86487786b5c ("iio: adc: ti-ads8688: add trigger and buffer support") > Signed-off-by: Sean Nyekjaer <sean@geanix.com> > --- > drivers/iio/adc/ti-ads8688.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/iio/adc/ti-ads8688.c b/drivers/iio/adc/ti-ads8688.c > index f9461070a74a..d9c354dbd7e4 100644 > --- a/drivers/iio/adc/ti-ads8688.c > +++ b/drivers/iio/adc/ti-ads8688.c > @@ -387,13 +387,12 @@ static irqreturn_t ads8688_trigger_handler(int irq, void *p) > struct iio_poll_func *pf = p; > struct iio_dev *indio_dev = pf->indio_dev; > u16 buffer[ADS8688_MAX_CHANNELS + sizeof(s64)/sizeof(u16)]; > - int i, j = 0; > + int bit; > > - for (i = 0; i < indio_dev->masklength; i++) { > - if (!test_bit(i, indio_dev->active_scan_mask)) > - continue; > - buffer[j] = ads8688_read(indio_dev, i); > - j++; > + memset(buffer, 0, sizeof(buffer)); > + > + for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) { > + buffer[bit] = ads8688_read(indio_dev, bit); > } > > iio_push_to_buffers_with_timestamp(indio_dev, buffer,
On 11/05/2019 13.44, Jonathan Cameron wrote: > On Tue, 7 May 2019 10:23:03 +0200 > Sean Nyekjaer <sean@geanix.com> wrote: > >> Fill the read values in the buffer so we comply >> with the given index values. > Why? > > They are not supposed to always remain in the same place. If some channels > are turned off then they will shift 'down'. That has to happen in general > to allow us to do more efficient packing when they happen to fit into a > smaller power of 2 size. > So if channels 0-3 (8 bits each) are enabled and timestamp, it should be > > 0 1 2 3 X X X X Timestamp, 0 1 2 3 X X X X Timestamp.. > > If no timestamp it should fall back to the packing > 0 1 2 3, 0 1 2 3 > > If only channels 0, 1 and 3 we should get > > 0 1 3 X, 0 1 3 X (padding to 32 bits) > > For only 0 and 3 > 0 3, 0 3, 0 3 > > For only 0 > 0, 0, 0, 0, 0, 0 > > Jonathan > Thanks for clarifying :-) /Sean
diff --git a/drivers/iio/adc/ti-ads8688.c b/drivers/iio/adc/ti-ads8688.c index f9461070a74a..d9c354dbd7e4 100644 --- a/drivers/iio/adc/ti-ads8688.c +++ b/drivers/iio/adc/ti-ads8688.c @@ -387,13 +387,12 @@ static irqreturn_t ads8688_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; u16 buffer[ADS8688_MAX_CHANNELS + sizeof(s64)/sizeof(u16)]; - int i, j = 0; + int bit; - for (i = 0; i < indio_dev->masklength; i++) { - if (!test_bit(i, indio_dev->active_scan_mask)) - continue; - buffer[j] = ads8688_read(indio_dev, i); - j++; + memset(buffer, 0, sizeof(buffer)); + + for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) { + buffer[bit] = ads8688_read(indio_dev, bit); } iio_push_to_buffers_with_timestamp(indio_dev, buffer,
Fill the read values in the buffer so we comply with the given index values. Fixes: 2a86487786b5c ("iio: adc: ti-ads8688: add trigger and buffer support") Signed-off-by: Sean Nyekjaer <sean@geanix.com> --- drivers/iio/adc/ti-ads8688.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)