On 11/28/2011 04:15 PM, Lars-Peter Clausen wrote: > On 11/28/2011 10:45 AM, Lars-Peter Clausen wrote: >> On 11/27/2011 02:33 PM, Jonathan Cameron wrote: >>> From: Jonathan Cameron <jic23@xxxxxxxxx> >>> >>> Obviously drivers should only use this for pushing to buffers. >>> They need buffer->scan_mask for pulling from them post demux. >>> >>> Signed-off-by: Jonathan Cameron <jic23@xxxxxxxxx> >>> --- >>> drivers/staging/iio/accel/adis16201_ring.c | 10 +++++----- >>> drivers/staging/iio/accel/adis16203_ring.c | 10 +++++----- >>> drivers/staging/iio/accel/adis16204_ring.c | 10 +++++----- >>> drivers/staging/iio/accel/adis16209_ring.c | 5 +++-- >>> drivers/staging/iio/accel/adis16240_ring.c | 5 +++-- >>> drivers/staging/iio/accel/lis3l02dq_ring.c | 23 +++++++++++++---------- >>> drivers/staging/iio/adc/ad7192.c | 10 ++++++---- >>> drivers/staging/iio/adc/ad7298_ring.c | 12 +++++++----- >>> drivers/staging/iio/adc/ad7476_ring.c | 3 ++- >>> drivers/staging/iio/adc/ad7793.c | 11 ++++++----- >>> drivers/staging/iio/adc/ad7887_ring.c | 8 +++++--- >>> drivers/staging/iio/adc/ad799x_ring.c | 13 ++++++++----- >>> drivers/staging/iio/buffer.h | 2 -- >>> drivers/staging/iio/gyro/adis16260_ring.c | 5 +++-- >>> drivers/staging/iio/iio_simple_dummy_buffer.c | 7 +++++-- >>> drivers/staging/iio/impedance-analyzer/ad5933.c | 14 ++++++++------ >>> drivers/staging/iio/imu/adis16400_ring.c | 19 +++++++++++-------- >>> drivers/staging/iio/industrialio-buffer.c | 2 -- >>> drivers/staging/iio/meter/ade7758_ring.c | 7 ++++--- >>> 19 files changed, 99 insertions(+), 77 deletions(-) >>> >>> diff --git a/drivers/staging/iio/accel/adis16201_ring.c b/drivers/staging/iio/accel/adis16201_ring.c >>> index 936e8cb..68d4b38 100644 >>> --- a/drivers/staging/iio/accel/adis16201_ring.c >>> +++ b/drivers/staging/iio/accel/adis16201_ring.c >>> @@ -74,11 +74,11 @@ static irqreturn_t adis16201_trigger_handler(int irq, void *p) >>> return -ENOMEM; >>> } >>> >>> - if (ring->scan_count) >>> - if (adis16201_read_ring_data(indio_dev, st->rx) >= 0) >>> - for (; i < ring->scan_count; i++) >>> - data[i] = be16_to_cpup( >>> - (__be16 *)&(st->rx[i*2])); >>> + if (!bitmap_empty(indio_dev->active_scan_mask, indio_dev->masklength) >>> + && adis16201_read_ring_data(indio_dev, st->rx) >= 0) >>> + for (; i < bitmap_weight(indio_dev->active_scan_mask, >>> + indio_dev->masklength); i++) >>> + data[i] = be16_to_cpup((__be16 *)&(st->rx[i*2])); >>> >> >> Does it really make sense to recompute bitmap_weight for each transfer? >> Can't we update scan_count once, when we update the scan_mask? I partly got rid of scan mask to make the more 'interesting' route with the active scan mask stuff easier to adapt (information is now in only one place.). I'd not be against caching this somewhere if it is convenient for the driver (such as in the update_scan_mode callback). I'm not however convinced it should be in the core code. Mostly the transfers that rely on this should probably be setup in the preenable functions etc rather than buffers being allocated in this fast path. This particular approach is based on some of my early drivers. I much prefer Michaels drivers where the whole transfer sequence is setup and only changed when the channels being captured change. The only other common use case is a fudge to get the timestamp in the right place. It kind of feels there ought to be a better way of handing software timestamps. Perhaps your timestamp source stuff that mentioned in the ZIO thread. This particular example is a bad one as it shouldn't be there at all given we no longer do endian conversion but rather describe the buffer. Don't suppose you fancy writing the patch that gets rid of these endian conversions? If not I'll get to it, but may not be that soon. > > Also: for chips where we can only read all values at once, should we set > available_scan_masks to ~0 and just let the demuxer handle everything else? No. For consistency (and ability to use some of the standard functions) the available scan masks should list the actual combination that is available. Assuming I've understood you correctly and you don't just mean for the channels that exist? > > - Lars > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
