* [PATCH 0/4 V2] staging:iio: Add support for multiple buffers
@ 2012-05-30 19:36 Jonathan Cameron
2012-05-30 19:36 ` [PATCH 1/4] " Jonathan Cameron
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Jonathan Cameron @ 2012-05-30 19:36 UTC (permalink / raw)
To: linux-iio; +Cc: Jonathan Cameron
This is basically a rebase of the series from a few months back.
I held this one off as it's 'interesting and invasive'.
Basically this adds a level of indirection to the path between
the hardware and the software buffers. This allows us to have
multiple buffers each with their own demuxer. Clearly the
hardware has to be configured to provide all the channels
anyone is interested in, but then the flow looks something like.
Data ---> Demux1 ---> subset of data --> buffer1
|
-------> Demux2 ---> subset of data --> buffer2.
Note the demuxers should be very nearly transparent
if all the data is going on to the relevant buffer.
So you may ask ' Why?' In short - it allows us to
do clean data flow to multiple consumers such as an
IIO buffer, input, hwmon etc for devices running in
an trigger driven mode.
I also have a few thoughts on basic in kernel filtering
dependent on new buffer implementations.
Anyhow. Whilst the patch is relatively large it mostly
boils down to code for tearing down all current buffers,
reconfiguring the device and then building them all back
up again.
The input driver shoes the sort of client we might have.
(It probably needs more work) I'd like to get the
first three patches in the linux-next asap so they
get lots of testing prior to the next merge window.
Hence if anyone has time to take a look that would be
great!
Jonathan
Jonathan Cameron (4):
staging:iio: Add support for multiple buffers
staging:iio:in kernel users: Add a data field for channel specific
info.
staging:iio: add a callback buffer for in kernel push interface
staging:iio: Proof of concept input driver.
drivers/iio/Kconfig | 6 +
drivers/iio/Makefile | 1 +
drivers/iio/buffer_cb.c | 115 ++++++++
drivers/iio/industrialio-buffer.c | 335 +++++++++++++++--------
drivers/iio/industrialio-core.c | 1 +
drivers/iio/inkern.c | 1 +
drivers/staging/iio/Kconfig | 11 +
drivers/staging/iio/Makefile | 1 +
drivers/staging/iio/accel/adis16201_ring.c | 4 +-
drivers/staging/iio/accel/adis16203_ring.c | 6 +-
drivers/staging/iio/accel/adis16204_ring.c | 3 +-
drivers/staging/iio/accel/adis16209_ring.c | 3 +-
drivers/staging/iio/accel/adis16240_ring.c | 4 +-
drivers/staging/iio/accel/lis3l02dq_ring.c | 3 +-
drivers/staging/iio/adc/ad7192.c | 3 +-
drivers/staging/iio/adc/ad7298_ring.c | 5 +-
drivers/staging/iio/adc/ad7476_ring.c | 2 +-
drivers/staging/iio/adc/ad7606_ring.c | 3 +-
drivers/staging/iio/adc/ad7793.c | 3 +-
drivers/staging/iio/adc/ad7887_ring.c | 2 +-
drivers/staging/iio/adc/ad799x_ring.c | 3 +-
drivers/staging/iio/adc/max1363_ring.c | 2 +-
drivers/staging/iio/gyro/adis16260_ring.c | 3 +-
drivers/staging/iio/iio_input.c | 176 ++++++++++++
drivers/staging/iio/iio_input.h | 23 ++
drivers/staging/iio/iio_simple_dummy_buffer.c | 5 +-
drivers/staging/iio/impedance-analyzer/ad5933.c | 3 +-
drivers/staging/iio/imu/adis16400_ring.c | 2 +-
drivers/staging/iio/meter/ade7758_ring.c | 3 +-
include/linux/iio/buffer.h | 24 +-
include/linux/iio/consumer.h | 48 ++++
include/linux/iio/iio.h | 2 +
include/linux/iio/machine.h | 2 +
33 files changed, 651 insertions(+), 157 deletions(-)
create mode 100644 drivers/iio/buffer_cb.c
create mode 100644 drivers/staging/iio/iio_input.c
create mode 100644 drivers/staging/iio/iio_input.h
--
1.7.10.2
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/4] staging:iio: Add support for multiple buffers 2012-05-30 19:36 [PATCH 0/4 V2] staging:iio: Add support for multiple buffers Jonathan Cameron @ 2012-05-30 19:36 ` Jonathan Cameron 2012-06-08 15:13 ` Lars-Peter Clausen 2012-05-30 19:36 ` [PATCH 2/4] staging:iio:in kernel users: Add a data field for channel specific info Jonathan Cameron ` (2 subsequent siblings) 3 siblings, 1 reply; 18+ messages in thread From: Jonathan Cameron @ 2012-05-30 19:36 UTC (permalink / raw) To: linux-iio; +Cc: Jonathan Cameron Route all buffer writes through the demux. Addition or removal of a buffer results in tear down and setup of all the buffers for a given device. Signed-off-by: Jonathan Cameron <jic23@kernel.org> --- drivers/iio/industrialio-buffer.c | 335 +++++++++++++++-------- drivers/iio/industrialio-core.c | 1 + drivers/staging/iio/accel/adis16201_ring.c | 4 +- drivers/staging/iio/accel/adis16203_ring.c | 6 +- drivers/staging/iio/accel/adis16204_ring.c | 3 +- drivers/staging/iio/accel/adis16209_ring.c | 3 +- drivers/staging/iio/accel/adis16240_ring.c | 4 +- drivers/staging/iio/accel/lis3l02dq_ring.c | 3 +- drivers/staging/iio/adc/ad7192.c | 3 +- drivers/staging/iio/adc/ad7298_ring.c | 5 +- drivers/staging/iio/adc/ad7476_ring.c | 2 +- drivers/staging/iio/adc/ad7606_ring.c | 3 +- drivers/staging/iio/adc/ad7793.c | 3 +- drivers/staging/iio/adc/ad7887_ring.c | 2 +- drivers/staging/iio/adc/ad799x_ring.c | 3 +- drivers/staging/iio/adc/max1363_ring.c | 2 +- drivers/staging/iio/gyro/adis16260_ring.c | 3 +- drivers/staging/iio/iio_simple_dummy_buffer.c | 5 +- drivers/staging/iio/impedance-analyzer/ad5933.c | 3 +- drivers/staging/iio/imu/adis16400_ring.c | 2 +- drivers/staging/iio/meter/ade7758_ring.c | 3 +- include/linux/iio/buffer.h | 24 +- include/linux/iio/iio.h | 2 + 23 files changed, 267 insertions(+), 157 deletions(-) diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c index ac185b8..b04b543 100644 --- a/drivers/iio/industrialio-buffer.c +++ b/drivers/iio/industrialio-buffer.c @@ -31,6 +31,16 @@ static const char * const iio_endian_prefix[] = { [IIO_LE] = "le", }; +static bool iio_buffer_is_primary_active(struct iio_dev *indio_dev) +{ + struct list_head *p; + + list_for_each(p, &indio_dev->buffer_list) + if (p == &indio_dev->buffer->buffer_list) + return true; + return false; +} + /** * iio_buffer_read_first_n_outer() - chrdev read for buffer access * @@ -134,7 +144,7 @@ static ssize_t iio_scan_el_store(struct device *dev, if (ret < 0) return ret; mutex_lock(&indio_dev->mlock); - if (iio_buffer_enabled(indio_dev)) { + if (iio_buffer_is_primary_active(indio_dev)) { ret = -EBUSY; goto error_ret; } @@ -180,12 +190,11 @@ static ssize_t iio_scan_el_ts_store(struct device *dev, return ret; mutex_lock(&indio_dev->mlock); - if (iio_buffer_enabled(indio_dev)) { + if (iio_buffer_is_primary_active(indio_dev)) { ret = -EBUSY; goto error_ret; } indio_dev->buffer->scan_timestamp = state; - indio_dev->scan_timestamp = state; error_ret: mutex_unlock(&indio_dev->mlock); @@ -395,96 +404,6 @@ ssize_t iio_buffer_write_length(struct device *dev, } EXPORT_SYMBOL(iio_buffer_write_length); -ssize_t iio_buffer_store_enable(struct device *dev, - struct device_attribute *attr, - const char *buf, - size_t len) -{ - int ret; - bool requested_state, current_state; - int previous_mode; - struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct iio_buffer *buffer = indio_dev->buffer; - - mutex_lock(&indio_dev->mlock); - previous_mode = indio_dev->currentmode; - requested_state = !(buf[0] == '0'); - current_state = iio_buffer_enabled(indio_dev); - if (current_state == requested_state) { - printk(KERN_INFO "iio-buffer, current state requested again\n"); - goto done; - } - if (requested_state) { - if (indio_dev->setup_ops->preenable) { - ret = indio_dev->setup_ops->preenable(indio_dev); - if (ret) { - printk(KERN_ERR - "Buffer not started:" - "buffer preenable failed\n"); - goto error_ret; - } - } - if (buffer->access->request_update) { - ret = buffer->access->request_update(buffer); - if (ret) { - printk(KERN_INFO - "Buffer not started:" - "buffer parameter update failed\n"); - goto error_ret; - } - } - /* Definitely possible for devices to support both of these.*/ - if (indio_dev->modes & INDIO_BUFFER_TRIGGERED) { - if (!indio_dev->trig) { - printk(KERN_INFO - "Buffer not started: no trigger\n"); - ret = -EINVAL; - goto error_ret; - } - indio_dev->currentmode = INDIO_BUFFER_TRIGGERED; - } else if (indio_dev->modes & INDIO_BUFFER_HARDWARE) - indio_dev->currentmode = INDIO_BUFFER_HARDWARE; - else { /* should never be reached */ - ret = -EINVAL; - goto error_ret; - } - - if (indio_dev->setup_ops->postenable) { - ret = indio_dev->setup_ops->postenable(indio_dev); - if (ret) { - printk(KERN_INFO - "Buffer not started:" - "postenable failed\n"); - indio_dev->currentmode = previous_mode; - if (indio_dev->setup_ops->postdisable) - indio_dev->setup_ops-> - postdisable(indio_dev); - goto error_ret; - } - } - } else { - if (indio_dev->setup_ops->predisable) { - ret = indio_dev->setup_ops->predisable(indio_dev); - if (ret) - goto error_ret; - } - indio_dev->currentmode = INDIO_DIRECT_MODE; - if (indio_dev->setup_ops->postdisable) { - ret = indio_dev->setup_ops->postdisable(indio_dev); - if (ret) - goto error_ret; - } - } -done: - mutex_unlock(&indio_dev->mlock); - return len; - -error_ret: - mutex_unlock(&indio_dev->mlock); - return ret; -} -EXPORT_SYMBOL(iio_buffer_store_enable); - ssize_t iio_buffer_show_enable(struct device *dev, struct device_attribute *attr, char *buf) @@ -534,31 +453,186 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev, const long *mask, return bytes; } -int iio_sw_buffer_preenable(struct iio_dev *indio_dev) +int iio_update_buffers(struct iio_dev *indio_dev, + struct iio_buffer *insert_buffer, + struct iio_buffer *remove_buffer) { - struct iio_buffer *buffer = indio_dev->buffer; - dev_dbg(&indio_dev->dev, "%s\n", __func__); + int ret; + struct iio_buffer *buffer; + unsigned long *compound_mask; - /* How much space will the demuxed element take? */ - indio_dev->scan_bytes = - iio_compute_scan_bytes(indio_dev, buffer->scan_mask, - buffer->scan_timestamp); - buffer->access->set_bytes_per_datum(buffer, indio_dev->scan_bytes); + /* Wind down existing buffers - iff there are any */ + if (!list_empty(&indio_dev->buffer_list)) { + if (indio_dev->setup_ops->predisable) { + ret = indio_dev->setup_ops->predisable(indio_dev); + if (ret) + goto error_ret; + } + indio_dev->currentmode = INDIO_DIRECT_MODE; + if (indio_dev->setup_ops->postdisable) { + ret = indio_dev->setup_ops->postdisable(indio_dev); + if (ret) + goto error_ret; + } + } + if (!indio_dev->available_scan_masks) + kfree(indio_dev->active_scan_mask); + + if (insert_buffer) + list_add(&insert_buffer->buffer_list, &indio_dev->buffer_list); + if (remove_buffer) + list_del(&remove_buffer->buffer_list); + + /* If no buffers in list, we are done */ + if (list_empty(&indio_dev->buffer_list)) + return 0; /* What scan mask do we actually have ?*/ + compound_mask = kzalloc(BITS_TO_LONGS(indio_dev->masklength) + *sizeof(long), GFP_KERNEL); + if (compound_mask == NULL) + return -ENOMEM; + list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) { + bitmap_or(compound_mask, compound_mask, buffer->scan_mask, + indio_dev->masklength); + indio_dev->scan_timestamp |= buffer->scan_timestamp; + } if (indio_dev->available_scan_masks) indio_dev->active_scan_mask = iio_scan_mask_match(indio_dev->available_scan_masks, indio_dev->masklength, - buffer->scan_mask); + compound_mask); else - indio_dev->active_scan_mask = buffer->scan_mask; + indio_dev->active_scan_mask = compound_mask; + iio_update_demux(indio_dev); - if (indio_dev->info->update_scan_mode) - return indio_dev->info + /* Wind up again */ + if (indio_dev->setup_ops->preenable) { + ret = indio_dev->setup_ops->preenable(indio_dev); + if (ret) { + printk(KERN_ERR + "Buffer not started:" + "buffer preenable failed\n"); + goto error_free_compound_mask; + } + } + indio_dev->scan_bytes = + iio_compute_scan_bytes(indio_dev, + indio_dev->active_scan_mask, + indio_dev->scan_timestamp); + list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) + if (buffer->access->request_update) { + ret = buffer->access->request_update(buffer); + if (ret) { + printk(KERN_INFO + "Buffer not started:" + "buffer parameter update failed\n"); + goto error_ret; + } + } + if (indio_dev->info->update_scan_mode) { + ret = indio_dev->info ->update_scan_mode(indio_dev, indio_dev->active_scan_mask); + if (ret < 0) + goto error_free_compound_mask; + } + /* Definitely possible for devices to support both of these.*/ + if (indio_dev->modes & INDIO_BUFFER_TRIGGERED) { + if (!indio_dev->trig) { + printk(KERN_INFO "Buffer not started: no trigger\n"); + ret = -EINVAL; + goto error_free_compound_mask; + } + indio_dev->currentmode = INDIO_BUFFER_TRIGGERED; + } else if (indio_dev->modes & INDIO_BUFFER_HARDWARE) { + indio_dev->currentmode = INDIO_BUFFER_HARDWARE; + } else { /* should never be reached */ + ret = -EINVAL; + goto error_free_compound_mask; + } + + if (indio_dev->setup_ops->postenable) { + ret = indio_dev->setup_ops->postenable(indio_dev); + if (ret) { + printk(KERN_INFO + "Buffer not started: postenable failed\n"); + indio_dev->currentmode = INDIO_DIRECT_MODE; + if (indio_dev->setup_ops->postdisable) + indio_dev->setup_ops->postdisable(indio_dev); + goto error_free_compound_mask; + } + } +error_free_compound_mask: + indio_dev->active_scan_mask = NULL; + kfree(compound_mask); +error_ret: + + return ret; +} +EXPORT_SYMBOL_GPL(iio_update_buffers); + +ssize_t iio_buffer_store_enable(struct device *dev, + struct device_attribute *attr, + const char *buf, + size_t len) +{ + int ret; + bool requested_state; + struct iio_dev *indio_dev = dev_get_drvdata(dev); + struct iio_buffer *pbuf = indio_dev->buffer; + struct list_head *p; + bool inlist = false; + + ret = strtobool(buf, &requested_state); + if (ret < 0) + return ret; + + mutex_lock(&indio_dev->mlock); + + /* Find out if it is in the list */ + list_for_each(p, &indio_dev->buffer_list) + if (p == &pbuf->buffer_list) { + inlist = true; + break; + } + /* Already enabled */ + if (inlist && requested_state) + goto done; + /* Already disabled */ + if (!inlist && !requested_state) + goto done; + + if (requested_state) + ret = iio_update_buffers(indio_dev, + indio_dev->buffer, NULL); + else + ret = iio_update_buffers(indio_dev, + NULL, indio_dev->buffer); + + if (ret < 0) + goto done; +done: + mutex_unlock(&indio_dev->mlock); + return (ret < 0) ? ret : len; +} +EXPORT_SYMBOL(iio_buffer_store_enable); + +int iio_sw_buffer_preenable(struct iio_dev *indio_dev) +{ + struct iio_buffer *buffer; + unsigned bytes; + dev_dbg(&indio_dev->dev, "%s\n", __func__); + + list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) + if (buffer->access->set_bytes_per_datum) { + bytes = iio_compute_scan_bytes(indio_dev, + buffer->scan_mask, + buffer->scan_timestamp); + + buffer->access->set_bytes_per_datum(buffer, bytes); + } return 0; } EXPORT_SYMBOL(iio_sw_buffer_preenable); @@ -567,7 +641,11 @@ EXPORT_SYMBOL(iio_sw_buffer_preenable); * iio_scan_mask_set() - set particular bit in the scan mask * @buffer: the buffer whose scan mask we are interested in * @bit: the bit to be set. - **/ + * + * Note that at this point we have no way of knowing what other + * buffers might request, hence this code only verifies that the + * individual buffers request is plausible. + */ int iio_scan_mask_set(struct iio_dev *indio_dev, struct iio_buffer *buffer, int bit) { @@ -597,7 +675,7 @@ int iio_scan_mask_set(struct iio_dev *indio_dev, return -EINVAL; } } - bitmap_copy(buffer->scan_mask, trialmask, indio_dev->masklength); + set_bit(bit, buffer->scan_mask); kfree(trialmask); @@ -646,14 +724,13 @@ static unsigned char *iio_demux(struct iio_buffer *buffer, return buffer->demux_bounce; } -int iio_push_to_buffer(struct iio_buffer *buffer, unsigned char *data, +static int iio_push_to_buffer(struct iio_buffer *buffer, unsigned char *data, s64 timestamp) { unsigned char *dataout = iio_demux(buffer, data); return buffer->access->store_to(buffer, dataout, timestamp); } -EXPORT_SYMBOL_GPL(iio_push_to_buffer); static void iio_buffer_demux_free(struct iio_buffer *buffer) { @@ -664,10 +741,27 @@ static void iio_buffer_demux_free(struct iio_buffer *buffer) } } -int iio_update_demux(struct iio_dev *indio_dev) + +int iio_push_to_buffers(struct iio_dev *indio_dev, unsigned char *data, + s64 timestamp) +{ + int ret; + struct iio_buffer *buf; + + list_for_each_entry(buf, &indio_dev->buffer_list, buffer_list) { + ret = iio_push_to_buffer(buf, data, timestamp); + if (ret < 0) + return ret; + } + + return 0; +} +EXPORT_SYMBOL_GPL(iio_push_to_buffers); + +static int iio_buffer_update_demux(struct iio_dev *indio_dev, + struct iio_buffer *buffer) { const struct iio_chan_spec *ch; - struct iio_buffer *buffer = indio_dev->buffer; int ret, in_ind = -1, out_ind, length; unsigned in_loc = 0, out_loc = 0; struct iio_demux_table *p; @@ -752,4 +846,23 @@ error_clear_mux_table: return ret; } + +int iio_update_demux(struct iio_dev *indio_dev) +{ + struct iio_buffer *buffer; + int ret; + + list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) { + ret = iio_buffer_update_demux(indio_dev, buffer); + if (ret < 0) + goto error_clear_mux_table; + } + return 0; + +error_clear_mux_table: + list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) + iio_buffer_demux_free(buffer); + + return ret; +} EXPORT_SYMBOL_GPL(iio_update_demux); diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index 07b053b..aa2edf4 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -834,6 +834,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv) return NULL; } dev_set_name(&dev->dev, "iio:device%d", dev->id); + INIT_LIST_HEAD(&dev->buffer_list); } return dev; diff --git a/drivers/staging/iio/accel/adis16201_ring.c b/drivers/staging/iio/accel/adis16201_ring.c index 247602a..64ba02a 100644 --- a/drivers/staging/iio/accel/adis16201_ring.c +++ b/drivers/staging/iio/accel/adis16201_ring.c @@ -62,8 +62,6 @@ static irqreturn_t adis16201_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct adis16201_state *st = iio_priv(indio_dev); - struct iio_buffer *ring = indio_dev->buffer; - int i = 0; s16 *data; @@ -83,7 +81,7 @@ static irqreturn_t adis16201_trigger_handler(int irq, void *p) if (indio_dev->scan_timestamp) *((s64 *)(data + ((i + 3)/4)*4)) = pf->timestamp; - ring->access->store_to(ring, (u8 *)data, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)data, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); kfree(data); diff --git a/drivers/staging/iio/accel/adis16203_ring.c b/drivers/staging/iio/accel/adis16203_ring.c index 7bbd2c2..57480f4 100644 --- a/drivers/staging/iio/accel/adis16203_ring.c +++ b/drivers/staging/iio/accel/adis16203_ring.c @@ -61,8 +61,6 @@ static irqreturn_t adis16203_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct adis16203_state *st = iio_priv(indio_dev); - struct iio_buffer *ring = indio_dev->buffer; - int i = 0; s16 *data; @@ -82,9 +80,7 @@ static irqreturn_t adis16203_trigger_handler(int irq, void *p) if (indio_dev->scan_timestamp) *((s64 *)(data + ((i + 3)/4)*4)) = pf->timestamp; - ring->access->store_to(ring, - (u8 *)data, - pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)data, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); kfree(data); diff --git a/drivers/staging/iio/accel/adis16204_ring.c b/drivers/staging/iio/accel/adis16204_ring.c index f73518b..3a5d459 100644 --- a/drivers/staging/iio/accel/adis16204_ring.c +++ b/drivers/staging/iio/accel/adis16204_ring.c @@ -59,7 +59,6 @@ static irqreturn_t adis16204_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct adis16204_state *st = iio_priv(indio_dev); - struct iio_buffer *ring = indio_dev->buffer; int i = 0; s16 *data; @@ -79,7 +78,7 @@ static irqreturn_t adis16204_trigger_handler(int irq, void *p) if (indio_dev->scan_timestamp) *((s64 *)(data + ((i + 3)/4)*4)) = pf->timestamp; - ring->access->store_to(ring, (u8 *)data, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)data, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); kfree(data); diff --git a/drivers/staging/iio/accel/adis16209_ring.c b/drivers/staging/iio/accel/adis16209_ring.c index 0906075..155fc79 100644 --- a/drivers/staging/iio/accel/adis16209_ring.c +++ b/drivers/staging/iio/accel/adis16209_ring.c @@ -59,7 +59,6 @@ static irqreturn_t adis16209_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct adis16209_state *st = iio_priv(indio_dev); - struct iio_buffer *ring = indio_dev->buffer; int i = 0; s16 *data; @@ -79,7 +78,7 @@ static irqreturn_t adis16209_trigger_handler(int irq, void *p) if (indio_dev->scan_timestamp) *((s64 *)(data + ((i + 3)/4)*4)) = pf->timestamp; - ring->access->store_to(ring, (u8 *)data, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)data, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); kfree(data); diff --git a/drivers/staging/iio/accel/adis16240_ring.c b/drivers/staging/iio/accel/adis16240_ring.c index 86a2a47..dec6646 100644 --- a/drivers/staging/iio/accel/adis16240_ring.c +++ b/drivers/staging/iio/accel/adis16240_ring.c @@ -56,8 +56,6 @@ static irqreturn_t adis16240_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct adis16240_state *st = iio_priv(indio_dev); - struct iio_buffer *ring = indio_dev->buffer; - int i = 0; s16 *data; @@ -77,7 +75,7 @@ static irqreturn_t adis16240_trigger_handler(int irq, void *p) if (indio_dev->scan_timestamp) *((s64 *)(data + ((i + 3)/4)*4)) = pf->timestamp; - ring->access->store_to(ring, (u8 *)data, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)data, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); kfree(data); diff --git a/drivers/staging/iio/accel/lis3l02dq_ring.c b/drivers/staging/iio/accel/lis3l02dq_ring.c index 51b00df..7129e22 100644 --- a/drivers/staging/iio/accel/lis3l02dq_ring.c +++ b/drivers/staging/iio/accel/lis3l02dq_ring.c @@ -135,7 +135,6 @@ static irqreturn_t lis3l02dq_trigger_handler(int irq, void *p) { struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; - struct iio_buffer *buffer = indio_dev->buffer; int len = 0; char *data; @@ -154,7 +153,7 @@ static irqreturn_t lis3l02dq_trigger_handler(int irq, void *p) *(s64 *)(((phys_addr_t)data + len + sizeof(s64) - 1) & ~(sizeof(s64) - 1)) = pf->timestamp; - buffer->access->store_to(buffer, (u8 *)data, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)data, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); kfree(data); diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c index 5eaeaf1..b6b32f6 100644 --- a/drivers/staging/iio/adc/ad7192.c +++ b/drivers/staging/iio/adc/ad7192.c @@ -510,7 +510,6 @@ static irqreturn_t ad7192_trigger_handler(int irq, void *p) { struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; - struct iio_buffer *ring = indio_dev->buffer; struct ad7192_state *st = iio_priv(indio_dev); s64 dat64[2]; s32 *dat32 = (s32 *)dat64; @@ -524,7 +523,7 @@ static irqreturn_t ad7192_trigger_handler(int irq, void *p) if (indio_dev->scan_timestamp) dat64[1] = pf->timestamp; - ring->access->store_to(ring, (u8 *)dat64, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)dat64, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); st->irq_dis = false; diff --git a/drivers/staging/iio/adc/ad7298_ring.c b/drivers/staging/iio/adc/ad7298_ring.c index 908a3e5..146afd8 100644 --- a/drivers/staging/iio/adc/ad7298_ring.c +++ b/drivers/staging/iio/adc/ad7298_ring.c @@ -83,7 +83,6 @@ static irqreturn_t ad7298_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct ad7298_state *st = iio_priv(indio_dev); - struct iio_buffer *ring = indio_dev->buffer; s64 time_ns; __u16 buf[16]; int b_sent, i; @@ -99,10 +98,10 @@ static irqreturn_t ad7298_trigger_handler(int irq, void *p) } for (i = 0; i < bitmap_weight(indio_dev->active_scan_mask, - indio_dev->masklength); i++) + indio_dev->masklength); i++) buf[i] = be16_to_cpu(st->rx_buf[i]); - indio_dev->buffer->access->store_to(ring, (u8 *)buf, time_ns); + iio_push_to_buffers(indio_dev, (u8 *)buf, time_ns); iio_trigger_notify_done(indio_dev->trig); return IRQ_HANDLED; diff --git a/drivers/staging/iio/adc/ad7476_ring.c b/drivers/staging/iio/adc/ad7476_ring.c index 383611b..950a11e 100644 --- a/drivers/staging/iio/adc/ad7476_ring.c +++ b/drivers/staging/iio/adc/ad7476_ring.c @@ -44,7 +44,7 @@ static irqreturn_t ad7476_trigger_handler(int irq, void *p) memcpy(rxbuf + indio_dev->scan_bytes - sizeof(s64), &time_ns, sizeof(time_ns)); - indio_dev->buffer->access->store_to(indio_dev->buffer, rxbuf, time_ns); + iio_push_to_buffers(indio_dev, rxbuf, time_ns); done: iio_trigger_notify_done(indio_dev->trig); kfree(rxbuf); diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c index 24ce8fc..497c1fc 100644 --- a/drivers/staging/iio/adc/ad7606_ring.c +++ b/drivers/staging/iio/adc/ad7606_ring.c @@ -46,7 +46,6 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s) struct ad7606_state *st = container_of(work_s, struct ad7606_state, poll_work); struct iio_dev *indio_dev = iio_priv_to_dev(st); - struct iio_buffer *ring = indio_dev->buffer; s64 time_ns; __u8 *buf; int ret; @@ -84,7 +83,7 @@ static void ad7606_poll_bh_to_ring(struct work_struct *work_s) if (indio_dev->scan_timestamp) *((s64 *)(buf + indio_dev->scan_bytes - sizeof(s64))) = time_ns; - ring->access->store_to(indio_dev->buffer, buf, time_ns); + iio_push_to_buffers(indio_dev, buf, time_ns); done: gpio_set_value(st->pdata->gpio_convst, 0); iio_trigger_notify_done(indio_dev->trig); diff --git a/drivers/staging/iio/adc/ad7793.c b/drivers/staging/iio/adc/ad7793.c index b36556f..22875a5 100644 --- a/drivers/staging/iio/adc/ad7793.c +++ b/drivers/staging/iio/adc/ad7793.c @@ -375,7 +375,6 @@ static irqreturn_t ad7793_trigger_handler(int irq, void *p) { struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; - struct iio_buffer *ring = indio_dev->buffer; struct ad7793_state *st = iio_priv(indio_dev); s64 dat64[2]; s32 *dat32 = (s32 *)dat64; @@ -389,7 +388,7 @@ static irqreturn_t ad7793_trigger_handler(int irq, void *p) if (indio_dev->scan_timestamp) dat64[1] = pf->timestamp; - ring->access->store_to(ring, (u8 *)dat64, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)dat64, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); st->irq_dis = false; diff --git a/drivers/staging/iio/adc/ad7887_ring.c b/drivers/staging/iio/adc/ad7887_ring.c index fd91384..9e11052 100644 --- a/drivers/staging/iio/adc/ad7887_ring.c +++ b/drivers/staging/iio/adc/ad7887_ring.c @@ -95,7 +95,7 @@ static irqreturn_t ad7887_trigger_handler(int irq, void *p) memcpy(buf + indio_dev->scan_bytes - sizeof(s64), &time_ns, sizeof(time_ns)); - indio_dev->buffer->access->store_to(indio_dev->buffer, buf, time_ns); + iio_push_to_buffers(indio_dev, buf, time_ns); done: kfree(buf); iio_trigger_notify_done(indio_dev->trig); diff --git a/drivers/staging/iio/adc/ad799x_ring.c b/drivers/staging/iio/adc/ad799x_ring.c index 1c7ff44..5d567fe 100644 --- a/drivers/staging/iio/adc/ad799x_ring.c +++ b/drivers/staging/iio/adc/ad799x_ring.c @@ -56,7 +56,6 @@ static irqreturn_t ad799x_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct ad799x_state *st = iio_priv(indio_dev); - struct iio_buffer *ring = indio_dev->buffer; s64 time_ns; __u8 *rxbuf; int b_sent; @@ -99,7 +98,7 @@ static irqreturn_t ad799x_trigger_handler(int irq, void *p) memcpy(rxbuf + indio_dev->scan_bytes - sizeof(s64), &time_ns, sizeof(time_ns)); - ring->access->store_to(indio_dev->buffer, rxbuf, time_ns); + iio_push_to_buffers(indio_dev, rxbuf, time_ns); done: kfree(rxbuf); if (b_sent < 0) diff --git a/drivers/staging/iio/adc/max1363_ring.c b/drivers/staging/iio/adc/max1363_ring.c index b302013..73273c0 100644 --- a/drivers/staging/iio/adc/max1363_ring.c +++ b/drivers/staging/iio/adc/max1363_ring.c @@ -80,7 +80,7 @@ static irqreturn_t max1363_trigger_handler(int irq, void *p) if (indio_dev->scan_timestamp) memcpy(rxbuf + d_size - sizeof(s64), &time_ns, sizeof(time_ns)); - iio_push_to_buffer(indio_dev->buffer, rxbuf, time_ns); + iio_push_to_buffers(indio_dev, rxbuf, time_ns); done: iio_trigger_notify_done(indio_dev->trig); diff --git a/drivers/staging/iio/gyro/adis16260_ring.c b/drivers/staging/iio/gyro/adis16260_ring.c index 0fe2d9d..4922a6d 100644 --- a/drivers/staging/iio/gyro/adis16260_ring.c +++ b/drivers/staging/iio/gyro/adis16260_ring.c @@ -62,7 +62,6 @@ static irqreturn_t adis16260_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct adis16260_state *st = iio_priv(indio_dev); - struct iio_buffer *ring = indio_dev->buffer; int i = 0; s16 *data; @@ -82,7 +81,7 @@ static irqreturn_t adis16260_trigger_handler(int irq, void *p) if (indio_dev->scan_timestamp) *((s64 *)(data + ((i + 3)/4)*4)) = pf->timestamp; - ring->access->store_to(ring, (u8 *)data, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)data, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); kfree(data); diff --git a/drivers/staging/iio/iio_simple_dummy_buffer.c b/drivers/staging/iio/iio_simple_dummy_buffer.c index fdfc873..5931cbb 100644 --- a/drivers/staging/iio/iio_simple_dummy_buffer.c +++ b/drivers/staging/iio/iio_simple_dummy_buffer.c @@ -46,7 +46,6 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p) { struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; - struct iio_buffer *buffer = indio_dev->buffer; int len = 0; u16 *data; @@ -76,7 +75,7 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p) i < bitmap_weight(indio_dev->active_scan_mask, indio_dev->masklength); i++) { - j = find_next_bit(buffer->scan_mask, + j = find_next_bit(indio_dev->buffer->scan_mask, indio_dev->masklength, j + 1); /* random access read form the 'device' */ data[i] = fakedata[j]; @@ -88,7 +87,7 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p) *(s64 *)(((phys_addr_t)data + len + sizeof(s64) - 1) & ~(sizeof(s64) - 1)) = iio_get_time_ns(); - buffer->access->store_to(buffer, (u8 *)data, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)data, pf->timestamp); kfree(data); diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c index a8e51bc..3599ec1 100644 --- a/drivers/staging/iio/impedance-analyzer/ad5933.c +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c @@ -647,7 +647,6 @@ static void ad5933_work(struct work_struct *work) struct ad5933_state *st = container_of(work, struct ad5933_state, work.work); struct iio_dev *indio_dev = i2c_get_clientdata(st->client); - struct iio_buffer *ring = indio_dev->buffer; signed short buf[2]; unsigned char status; @@ -678,7 +677,7 @@ static void ad5933_work(struct work_struct *work) buf[0] = be16_to_cpu(buf[0]); } /* save datum to the ring */ - ring->access->store_to(ring, (u8 *)buf, iio_get_time_ns()); + iio_push_to_buffers(indio_dev, (u8 *)buf, iio_get_time_ns()); } else { /* no data available - try again later */ schedule_delayed_work(&st->work, st->poll_time_jiffies); diff --git a/drivers/staging/iio/imu/adis16400_ring.c b/drivers/staging/iio/imu/adis16400_ring.c index 809e2c4..4c0dac8 100644 --- a/drivers/staging/iio/imu/adis16400_ring.c +++ b/drivers/staging/iio/imu/adis16400_ring.c @@ -150,7 +150,7 @@ static irqreturn_t adis16400_trigger_handler(int irq, void *p) /* Guaranteed to be aligned with 8 byte boundary */ if (ring->scan_timestamp) *((s64 *)(data + ((i + 3)/4)*4)) = pf->timestamp; - ring->access->store_to(indio_dev->buffer, (u8 *) data, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *) data, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); diff --git a/drivers/staging/iio/meter/ade7758_ring.c b/drivers/staging/iio/meter/ade7758_ring.c index 92159f2..0b0edd2 100644 --- a/drivers/staging/iio/meter/ade7758_ring.c +++ b/drivers/staging/iio/meter/ade7758_ring.c @@ -61,7 +61,6 @@ static irqreturn_t ade7758_trigger_handler(int irq, void *p) { struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; - struct iio_buffer *ring = indio_dev->buffer; struct ade7758_state *st = iio_priv(indio_dev); s64 dat64[2]; u32 *dat32 = (u32 *)dat64; @@ -74,7 +73,7 @@ static irqreturn_t ade7758_trigger_handler(int irq, void *p) if (indio_dev->scan_timestamp) dat64[1] = pf->timestamp; - ring->access->store_to(ring, (u8 *)dat64, pf->timestamp); + iio_push_to_buffers(indio_dev, (u8 *)dat64, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h index fb0fe46..22f7bfd 100644 --- a/include/linux/iio/buffer.h +++ b/include/linux/iio/buffer.h @@ -66,7 +66,8 @@ struct iio_buffer_access_funcs { * @stufftoread: [INTERN] flag to indicate new data. * @demux_list: [INTERN] list of operations required to demux the scan. * @demux_bounce: [INTERN] buffer for doing gather from incoming scan. - **/ + * @buffer_list: [INTERN] entry in the devices list of current buffers. + */ struct iio_buffer { int length; int bytes_per_datum; @@ -81,9 +82,22 @@ struct iio_buffer { const struct attribute_group *attrs; struct list_head demux_list; unsigned char *demux_bounce; + struct list_head buffer_list; }; /** + * iio_update_buffers() - add or remove buffer from active list + * @indio_dev: device to add buffer to + * @insert_buffer: buffer to insert + * @remove_buffer: buffer_to_remove + * + * Note this will tear down the all buffering and build it up again + */ +int iio_update_buffers(struct iio_dev *indio_dev, + struct iio_buffer *insert_buffer, + struct iio_buffer *remove_buffer); + +/** * iio_buffer_init() - Initialize the buffer structure * @buffer: buffer to be initialized **/ @@ -114,12 +128,12 @@ int iio_scan_mask_set(struct iio_dev *indio_dev, struct iio_buffer *buffer, int bit); /** - * iio_push_to_buffer() - push to a registered buffer. - * @buffer: IIO buffer structure for device - * @scan: Full scan. + * iio_push_to_buffers() - push to a registered buffer. + * @indio_dev: iio_dev structure for device. + * @data: Full scan. * @timestamp: */ -int iio_push_to_buffer(struct iio_buffer *buffer, unsigned char *data, +int iio_push_to_buffers(struct iio_dev *indio_dev, unsigned char *data, s64 timestamp); int iio_update_demux(struct iio_dev *indio_dev); diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h index 7378bbd..e486216 100644 --- a/include/linux/iio/iio.h +++ b/include/linux/iio/iio.h @@ -381,6 +381,7 @@ struct iio_buffer_setup_ops { * and owner * @event_interface: [INTERN] event chrdevs associated with interrupt lines * @buffer: [DRIVER] any buffer present + * @buffer_list: [INTERN] list of all buffers currently attached * @scan_bytes: [INTERN] num bytes captured to be fed to buffer demux * @mlock: [INTERN] lock used to prevent simultaneous device state * changes @@ -419,6 +420,7 @@ struct iio_dev { struct iio_event_interface *event_interface; struct iio_buffer *buffer; + struct list_head buffer_list; int scan_bytes; struct mutex mlock; -- 1.7.10.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] staging:iio: Add support for multiple buffers 2012-05-30 19:36 ` [PATCH 1/4] " Jonathan Cameron @ 2012-06-08 15:13 ` Lars-Peter Clausen 2012-06-09 12:17 ` Jonathan Cameron 0 siblings, 1 reply; 18+ messages in thread From: Lars-Peter Clausen @ 2012-06-08 15:13 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio On 05/30/2012 09:36 PM, Jonathan Cameron wrote: > Route all buffer writes through the demux. > Addition or removal of a buffer results in tear down and > setup of all the buffers for a given device. > My test platform is currently broken in staging-next, so just some comments based on code review for now. > Signed-off-by: Jonathan Cameron <jic23@kernel.org> > --- > drivers/iio/industrialio-buffer.c | 335 +++++++++++++++-------- > drivers/iio/industrialio-core.c | 1 + > drivers/staging/iio/accel/adis16201_ring.c | 4 +- > drivers/staging/iio/accel/adis16203_ring.c | 6 +- > drivers/staging/iio/accel/adis16204_ring.c | 3 +- > drivers/staging/iio/accel/adis16209_ring.c | 3 +- > drivers/staging/iio/accel/adis16240_ring.c | 4 +- > drivers/staging/iio/accel/lis3l02dq_ring.c | 3 +- > drivers/staging/iio/adc/ad7192.c | 3 +- > drivers/staging/iio/adc/ad7298_ring.c | 5 +- > drivers/staging/iio/adc/ad7476_ring.c | 2 +- > drivers/staging/iio/adc/ad7606_ring.c | 3 +- > drivers/staging/iio/adc/ad7793.c | 3 +- > drivers/staging/iio/adc/ad7887_ring.c | 2 +- > drivers/staging/iio/adc/ad799x_ring.c | 3 +- > drivers/staging/iio/adc/max1363_ring.c | 2 +- > drivers/staging/iio/gyro/adis16260_ring.c | 3 +- > drivers/staging/iio/iio_simple_dummy_buffer.c | 5 +- > drivers/staging/iio/impedance-analyzer/ad5933.c | 3 +- > drivers/staging/iio/imu/adis16400_ring.c | 2 +- > drivers/staging/iio/meter/ade7758_ring.c | 3 +- > include/linux/iio/buffer.h | 24 +- > include/linux/iio/iio.h | 2 + The at91_adc driver is not handled by this patch > 23 files changed, 267 insertions(+), 157 deletions(-) > > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c > index ac185b8..b04b543 100644 > --- a/drivers/iio/industrialio-buffer.c > +++ b/drivers/iio/industrialio-buffer.c > @@ -31,6 +31,16 @@ static const char * const iio_endian_prefix[] = { > [IIO_LE] = "le", > }; > > +static bool iio_buffer_is_primary_active(struct iio_dev *indio_dev) > +{ > + struct list_head *p; > + > + list_for_each(p, &indio_dev->buffer_list) > + if (p == &indio_dev->buffer->buffer_list) > + return true; More or less the same code is used below. I think it makes sense to have a generic iio_buffer_is_active(indio_dev, buffer) > + return false; > +} > [...] > @@ -534,31 +453,186 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev, const long *mask, > return bytes; > } > > -int iio_sw_buffer_preenable(struct iio_dev *indio_dev) > +int iio_update_buffers(struct iio_dev *indio_dev, > + struct iio_buffer *insert_buffer, > + struct iio_buffer *remove_buffer) > { > - struct iio_buffer *buffer = indio_dev->buffer; > - dev_dbg(&indio_dev->dev, "%s\n", __func__); > + int ret; drivers/iio/industrialio-buffer.c: In function ‘iio_update_buffers’: drivers/iio/industrialio-buffer.c:460: warning: ‘ret’ may be used uninitialized in this function I think there a missing 'return 0', before the error handling. Right now the code always sets active_scan_mask to NULL. > + struct iio_buffer *buffer; > + unsigned long *compound_mask; > > - /* How much space will the demuxed element take? */ > - indio_dev->scan_bytes = > - iio_compute_scan_bytes(indio_dev, buffer->scan_mask, > - buffer->scan_timestamp); > - buffer->access->set_bytes_per_datum(buffer, indio_dev->scan_bytes); > + /* Wind down existing buffers - iff there are any */ > + if (!list_empty(&indio_dev->buffer_list)) { > + if (indio_dev->setup_ops->predisable) { > + ret = indio_dev->setup_ops->predisable(indio_dev); > + if (ret) > + goto error_ret; > + } > + indio_dev->currentmode = INDIO_DIRECT_MODE; > + if (indio_dev->setup_ops->postdisable) { > + ret = indio_dev->setup_ops->postdisable(indio_dev); > + if (ret) > + goto error_ret; > + } > + } > + if (!indio_dev->available_scan_masks) > + kfree(indio_dev->active_scan_mask); If the code errors out before reassigning active_scan_mask we'll leave a dangling pointer to the old scan mask, which will cause access after free or a double free. > + > + if (insert_buffer) > + list_add(&insert_buffer->buffer_list, &indio_dev->buffer_list); > + if (remove_buffer) > + list_del(&remove_buffer->buffer_list); I'd do the remove before the insert, makes this more robust, if the function is ever going to be called with both set to the same buffer. > + > + /* If no buffers in list, we are done */ > + if (list_empty(&indio_dev->buffer_list)) > + return 0; > > /* What scan mask do we actually have ?*/ > + compound_mask = kzalloc(BITS_TO_LONGS(indio_dev->masklength) > + *sizeof(long), GFP_KERNEL); > + if (compound_mask == NULL) > + return -ENOMEM; > + list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) { > + bitmap_or(compound_mask, compound_mask, buffer->scan_mask, > + indio_dev->masklength); > + indio_dev->scan_timestamp |= buffer->scan_timestamp; > + } > if (indio_dev->available_scan_masks) > indio_dev->active_scan_mask = > iio_scan_mask_match(indio_dev->available_scan_masks, > indio_dev->masklength, > - buffer->scan_mask); > + compound_mask); active_scan_mask may be NULL if the device isn't able to handle the compound mask. Btw. if it is not possible to add the new buffer to the list, e.g. because the compound scan mask is not supported, do we to do a rollback to the previous working configuration? I think this might be a better option, rather than leaving things broken. > else > - indio_dev->active_scan_mask = buffer->scan_mask; > + indio_dev->active_scan_mask = compound_mask; > + > iio_update_demux(indio_dev); > > - if (indio_dev->info->update_scan_mode) > - return indio_dev->info > + /* Wind up again */ > + if (indio_dev->setup_ops->preenable) { > + ret = indio_dev->setup_ops->preenable(indio_dev); > + if (ret) { > + printk(KERN_ERR > + "Buffer not started:" > + "buffer preenable failed\n"); > + goto error_free_compound_mask; > + } > + } > + indio_dev->scan_bytes = > + iio_compute_scan_bytes(indio_dev, > + indio_dev->active_scan_mask, > + indio_dev->scan_timestamp); > + list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) > + if (buffer->access->request_update) { > + ret = buffer->access->request_update(buffer); > + if (ret) { > + printk(KERN_INFO > + "Buffer not started:" > + "buffer parameter update failed\n"); > + goto error_ret; > + } > + } > + if (indio_dev->info->update_scan_mode) { > + ret = indio_dev->info > ->update_scan_mode(indio_dev, > indio_dev->active_scan_mask); > + if (ret < 0) > + goto error_free_compound_mask; > + } > + /* Definitely possible for devices to support both of these.*/ > + if (indio_dev->modes & INDIO_BUFFER_TRIGGERED) { > + if (!indio_dev->trig) { > + printk(KERN_INFO "Buffer not started: no trigger\n"); > + ret = -EINVAL; > + goto error_free_compound_mask; > + } > + indio_dev->currentmode = INDIO_BUFFER_TRIGGERED; > + } else if (indio_dev->modes & INDIO_BUFFER_HARDWARE) { > + indio_dev->currentmode = INDIO_BUFFER_HARDWARE; > + } else { /* should never be reached */ > + ret = -EINVAL; > + goto error_free_compound_mask; > + } > + > + if (indio_dev->setup_ops->postenable) { > + ret = indio_dev->setup_ops->postenable(indio_dev); > + if (ret) { > + printk(KERN_INFO > + "Buffer not started: postenable failed\n"); > + indio_dev->currentmode = INDIO_DIRECT_MODE; > + if (indio_dev->setup_ops->postdisable) > + indio_dev->setup_ops->postdisable(indio_dev); > + goto error_free_compound_mask; > + } > + } > +error_free_compound_mask: > + indio_dev->active_scan_mask = NULL; > + kfree(compound_mask); > +error_ret: > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(iio_update_buffers); > + > +ssize_t iio_buffer_store_enable(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t len) > +{ > + int ret; > + bool requested_state; > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > + struct iio_buffer *pbuf = indio_dev->buffer; > + struct list_head *p; > + bool inlist = false; > + > + ret = strtobool(buf, &requested_state); > + if (ret < 0) > + return ret; > + > + mutex_lock(&indio_dev->mlock); > + > + /* Find out if it is in the list */ > + list_for_each(p, &indio_dev->buffer_list) > + if (p == &pbuf->buffer_list) { > + inlist = true; > + break; > + } > + /* Already enabled */ > + if (inlist && requested_state) > + goto done; > + /* Already disabled */ > + if (!inlist && !requested_state) > + goto done; if (inlist == requested_state) goto done; > + > + if (requested_state) > + ret = iio_update_buffers(indio_dev, > + indio_dev->buffer, NULL); > + else > + ret = iio_update_buffers(indio_dev, > + NULL, indio_dev->buffer); > + > + if (ret < 0) > + goto done; > +done: > + mutex_unlock(&indio_dev->mlock); > + return (ret < 0) ? ret : len; > +} > +EXPORT_SYMBOL(iio_buffer_store_enable); > + [...] > @@ -567,7 +641,11 @@ EXPORT_SYMBOL(iio_sw_buffer_preenable); > * iio_scan_mask_set() - set particular bit in the scan mask > * @buffer: the buffer whose scan mask we are interested in > * @bit: the bit to be set. > - **/ > + * > + * Note that at this point we have no way of knowing what other > + * buffers might request, hence this code only verifies that the > + * individual buffers request is plausible. > + */ > int iio_scan_mask_set(struct iio_dev *indio_dev, > struct iio_buffer *buffer, int bit) > { > @@ -597,7 +675,7 @@ int iio_scan_mask_set(struct iio_dev *indio_dev, > return -EINVAL; > } > } > - bitmap_copy(buffer->scan_mask, trialmask, indio_dev->masklength); > + set_bit(bit, buffer->scan_mask); Is this a unrelated fix? > > kfree(trialmask); > [...] > diff --git a/drivers/staging/iio/iio_simple_dummy_buffer.c b/drivers/staging/iio/iio_simple_dummy_buffer.c > index fdfc873..5931cbb 100644 > --- a/drivers/staging/iio/iio_simple_dummy_buffer.c > +++ b/drivers/staging/iio/iio_simple_dummy_buffer.c > @@ -46,7 +46,6 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p) > { > struct iio_poll_func *pf = p; > struct iio_dev *indio_dev = pf->indio_dev; > - struct iio_buffer *buffer = indio_dev->buffer; > int len = 0; > u16 *data; > > @@ -76,7 +75,7 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p) > i < bitmap_weight(indio_dev->active_scan_mask, > indio_dev->masklength); > i++) { > - j = find_next_bit(buffer->scan_mask, > + j = find_next_bit(indio_dev->buffer->scan_mask, should this be indio_dev->active_scan_mask? > indio_dev->masklength, j + 1); > /* random access read form the 'device' */ > data[i] = fakedata[j]; ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] staging:iio: Add support for multiple buffers 2012-06-08 15:13 ` Lars-Peter Clausen @ 2012-06-09 12:17 ` Jonathan Cameron 2012-06-09 11:50 ` Lars-Peter Clausen 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Cameron @ 2012-06-09 12:17 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: linux-iio On 06/08/2012 04:13 PM, Lars-Peter Clausen wrote: > On 05/30/2012 09:36 PM, Jonathan Cameron wrote: >> Route all buffer writes through the demux. >> Addition or removal of a buffer results in tear down and >> setup of all the buffers for a given device. >> >=20 > My test platform is currently broken in staging-next, so just some co= mments > based on code review for now. Annoying when that happens! Thanks for taking a look. This codes now been sitting around long enough that I'm not entirely sure what some of it does myself... >=20 >> Signed-off-by: Jonathan Cameron <jic23@kernel.org> >> --- >> drivers/iio/industrialio-buffer.c | 335 ++++++++++++= +++-------- >> drivers/iio/industrialio-core.c | 1 + >> drivers/staging/iio/accel/adis16201_ring.c | 4 +- >> drivers/staging/iio/accel/adis16203_ring.c | 6 +- >> drivers/staging/iio/accel/adis16204_ring.c | 3 +- >> drivers/staging/iio/accel/adis16209_ring.c | 3 +- >> drivers/staging/iio/accel/adis16240_ring.c | 4 +- >> drivers/staging/iio/accel/lis3l02dq_ring.c | 3 +- >> drivers/staging/iio/adc/ad7192.c | 3 +- >> drivers/staging/iio/adc/ad7298_ring.c | 5 +- >> drivers/staging/iio/adc/ad7476_ring.c | 2 +- >> drivers/staging/iio/adc/ad7606_ring.c | 3 +- >> drivers/staging/iio/adc/ad7793.c | 3 +- >> drivers/staging/iio/adc/ad7887_ring.c | 2 +- >> drivers/staging/iio/adc/ad799x_ring.c | 3 +- >> drivers/staging/iio/adc/max1363_ring.c | 2 +- >> drivers/staging/iio/gyro/adis16260_ring.c | 3 +- >> drivers/staging/iio/iio_simple_dummy_buffer.c | 5 +- >> drivers/staging/iio/impedance-analyzer/ad5933.c | 3 +- >> drivers/staging/iio/imu/adis16400_ring.c | 2 +- >> drivers/staging/iio/meter/ade7758_ring.c | 3 +- >> include/linux/iio/buffer.h | 24 +- >> include/linux/iio/iio.h | 2 + >=20 > The at91_adc driver is not handled by this patch good point. oops. Darn these new drivers ;) >=20 >> 23 files changed, 267 insertions(+), 157 deletions(-) >> >> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industr= ialio-buffer.c >> index ac185b8..b04b543 100644 >> --- a/drivers/iio/industrialio-buffer.c >> +++ b/drivers/iio/industrialio-buffer.c >> @@ -31,6 +31,16 @@ static const char * const iio_endian_prefix[] =3D= { >> [IIO_LE] =3D "le", >> }; >> =20 >> +static bool iio_buffer_is_primary_active(struct iio_dev *indio_dev) >> +{ >> + struct list_head *p; >> + >> + list_for_each(p, &indio_dev->buffer_list) >> + if (p =3D=3D &indio_dev->buffer->buffer_list) >> + return true; >=20 > More or less the same code is used below. I think it makes sense to h= ave a > generic iio_buffer_is_active(indio_dev, buffer) Good point. >=20 >> + return false; >> +} >=20 >> [...] >> @@ -534,31 +453,186 @@ static int iio_compute_scan_bytes(struct iio_= dev *indio_dev, const long *mask, >> return bytes; >> } >> =20 >> -int iio_sw_buffer_preenable(struct iio_dev *indio_dev) >> +int iio_update_buffers(struct iio_dev *indio_dev, >> + struct iio_buffer *insert_buffer, >> + struct iio_buffer *remove_buffer) >> { >> - struct iio_buffer *buffer =3D indio_dev->buffer; >> - dev_dbg(&indio_dev->dev, "%s\n", __func__); >> + int ret; >=20 > drivers/iio/industrialio-buffer.c: In function =91iio_update_buffers=92= : > drivers/iio/industrialio-buffer.c:460: warning: =91ret=92 may be used > uninitialized in this function >=20 > I think there a missing 'return 0', before the error handling. Right = now the > code always sets active_scan_mask to NULL. The setting to NULL definitely isn't right, but we do still need to free the compoundmask. I'll fix that up. >=20 >=20 >> + struct iio_buffer *buffer; >> + unsigned long *compound_mask; >> =20 >> - /* How much space will the demuxed element take? */ >> - indio_dev->scan_bytes =3D >> - iio_compute_scan_bytes(indio_dev, buffer->scan_mask, >> - buffer->scan_timestamp); >> - buffer->access->set_bytes_per_datum(buffer, indio_dev->scan_bytes)= ; >> + /* Wind down existing buffers - iff there are any */ >> + if (!list_empty(&indio_dev->buffer_list)) { >> + if (indio_dev->setup_ops->predisable) { >> + ret =3D indio_dev->setup_ops->predisable(indio_dev); >> + if (ret) >> + goto error_ret; >> + } >> + indio_dev->currentmode =3D INDIO_DIRECT_MODE; >> + if (indio_dev->setup_ops->postdisable) { >> + ret =3D indio_dev->setup_ops->postdisable(indio_dev); >> + if (ret) >> + goto error_ret; >> + } >> + } >> + if (!indio_dev->available_scan_masks) >> + kfree(indio_dev->active_scan_mask); >=20 > If the code errors out before reassigning active_scan_mask we'll leav= e a > dangling pointer to the old scan mask, which will cause access after = free or > a double free. good point this needs to be set to NULL. >=20 >> + >> + if (insert_buffer) >> + list_add(&insert_buffer->buffer_list, &indio_dev->buffer_list); >> + if (remove_buffer) >> + list_del(&remove_buffer->buffer_list); >=20 > I'd do the remove before the insert, makes this more robust, if the f= unction > is ever going to be called with both set to the same buffer. Tempting as it is to take the view we should crash hard if anyone does that you are right of course ;) >=20 >> + >> + /* If no buffers in list, we are done */ >> + if (list_empty(&indio_dev->buffer_list)) >> + return 0; >> =20 >> /* What scan mask do we actually have ?*/ >> + compound_mask =3D kzalloc(BITS_TO_LONGS(indio_dev->masklength) >> + *sizeof(long), GFP_KERNEL); >> + if (compound_mask =3D=3D NULL) >> + return -ENOMEM; >> + list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) = { >> + bitmap_or(compound_mask, compound_mask, buffer->scan_mask, >> + indio_dev->masklength); >> + indio_dev->scan_timestamp |=3D buffer->scan_timestamp; >> + } >> if (indio_dev->available_scan_masks) >> indio_dev->active_scan_mask =3D >> iio_scan_mask_match(indio_dev->available_scan_masks, >> indio_dev->masklength, >> - buffer->scan_mask); >> + compound_mask); >=20 > active_scan_mask may be NULL if the device isn't able to handle the c= ompound > mask. >=20 > Btw. if it is not possible to add the new buffer to the list, e.g. be= cause > the compound scan mask is not supported, do we to do a rollback to th= e > previous working configuration? I think this might be a better option= , > rather than leaving things broken. hmm.. Indeed, that is a good idea... Adds a bit of complexity, but not too bad. >=20 >> else >> - indio_dev->active_scan_mask =3D buffer->scan_mask; >> + indio_dev->active_scan_mask =3D compound_mask; >> + >> iio_update_demux(indio_dev); >> =20 >> - if (indio_dev->info->update_scan_mode) >> - return indio_dev->info >> + /* Wind up again */ >> + if (indio_dev->setup_ops->preenable) { >> + ret =3D indio_dev->setup_ops->preenable(indio_dev); >> + if (ret) { >> + printk(KERN_ERR >> + "Buffer not started:" >> + "buffer preenable failed\n"); >> + goto error_free_compound_mask; >> + } >> + } >> + indio_dev->scan_bytes =3D >> + iio_compute_scan_bytes(indio_dev, >> + indio_dev->active_scan_mask, >> + indio_dev->scan_timestamp); >> + list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) >> + if (buffer->access->request_update) { >> + ret =3D buffer->access->request_update(buffer); >> + if (ret) { >> + printk(KERN_INFO >> + "Buffer not started:" >> + "buffer parameter update failed\n"); >> + goto error_ret; >> + } >> + } >> + if (indio_dev->info->update_scan_mode) { >> + ret =3D indio_dev->info >> ->update_scan_mode(indio_dev, >> indio_dev->active_scan_mask); >> + if (ret < 0) >> + goto error_free_compound_mask; >> + } >> + /* Definitely possible for devices to support both of these.*/ >> + if (indio_dev->modes & INDIO_BUFFER_TRIGGERED) { >> + if (!indio_dev->trig) { >> + printk(KERN_INFO "Buffer not started: no trigger\n"); >> + ret =3D -EINVAL; >> + goto error_free_compound_mask; >> + } >> + indio_dev->currentmode =3D INDIO_BUFFER_TRIGGERED; >> + } else if (indio_dev->modes & INDIO_BUFFER_HARDWARE) { >> + indio_dev->currentmode =3D INDIO_BUFFER_HARDWARE; >> + } else { /* should never be reached */ >> + ret =3D -EINVAL; >> + goto error_free_compound_mask; >> + } >> + >> + if (indio_dev->setup_ops->postenable) { >> + ret =3D indio_dev->setup_ops->postenable(indio_dev); >> + if (ret) { >> + printk(KERN_INFO >> + "Buffer not started: postenable failed\n"); >> + indio_dev->currentmode =3D INDIO_DIRECT_MODE; >> + if (indio_dev->setup_ops->postdisable) >> + indio_dev->setup_ops->postdisable(indio_dev); >> + goto error_free_compound_mask; >> + } >> + } >> +error_free_compound_mask: >> + indio_dev->active_scan_mask =3D NULL; >> + kfree(compound_mask); >> +error_ret: >> + >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(iio_update_buffers); >> + >> +ssize_t iio_buffer_store_enable(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, >> + size_t len) >> +{ >> + int ret; >> + bool requested_state; >> + struct iio_dev *indio_dev =3D dev_get_drvdata(dev); >> + struct iio_buffer *pbuf =3D indio_dev->buffer; >> + struct list_head *p; >> + bool inlist =3D false; >> + >> + ret =3D strtobool(buf, &requested_state); >> + if (ret < 0) >> + return ret; >> + >> + mutex_lock(&indio_dev->mlock); >> + >> + /* Find out if it is in the list */ >> + list_for_each(p, &indio_dev->buffer_list) >> + if (p =3D=3D &pbuf->buffer_list) { >> + inlist =3D true; >> + break; >> + } >> + /* Already enabled */ >> + if (inlist && requested_state) >> + goto done; >> + /* Already disabled */ >> + if (!inlist && !requested_state) >> + goto done; >=20 > if (inlist =3D=3D requested_state) > goto done; >=20 >> + >> + if (requested_state) >> + ret =3D iio_update_buffers(indio_dev, >> + indio_dev->buffer, NULL); >> + else >> + ret =3D iio_update_buffers(indio_dev, >> + NULL, indio_dev->buffer); >> + >> + if (ret < 0) >> + goto done; >> +done: >> + mutex_unlock(&indio_dev->mlock); >> + return (ret < 0) ? ret : len; >> +} >> +EXPORT_SYMBOL(iio_buffer_store_enable); >> + > [...] >> @@ -567,7 +641,11 @@ EXPORT_SYMBOL(iio_sw_buffer_preenable); >> * iio_scan_mask_set() - set particular bit in the scan mask >> * @buffer: the buffer whose scan mask we are interested in >> * @bit: the bit to be set. >> - **/ >> + * >> + * Note that at this point we have no way of knowing what other >> + * buffers might request, hence this code only verifies that the >> + * individual buffers request is plausible. >> + */ >> int iio_scan_mask_set(struct iio_dev *indio_dev, >> struct iio_buffer *buffer, int bit) >> { >> @@ -597,7 +675,7 @@ int iio_scan_mask_set(struct iio_dev *indio_dev, >> return -EINVAL; >> } >> } >> - bitmap_copy(buffer->scan_mask, trialmask, indio_dev->masklength); >> + set_bit(bit, buffer->scan_mask); >=20 > Is this a unrelated fix? Slight simplification. Doesn't belong in this patch (and probably not worth having at all really... >=20 >> =20 >> kfree(trialmask); >> =20 > [...] >> diff --git a/drivers/staging/iio/iio_simple_dummy_buffer.c b/drivers= /staging/iio/iio_simple_dummy_buffer.c >> index fdfc873..5931cbb 100644 >> --- a/drivers/staging/iio/iio_simple_dummy_buffer.c >> +++ b/drivers/staging/iio/iio_simple_dummy_buffer.c >> @@ -46,7 +46,6 @@ static irqreturn_t iio_simple_dummy_trigger_h(int = irq, void *p) >> { >> struct iio_poll_func *pf =3D p; >> struct iio_dev *indio_dev =3D pf->indio_dev; >> - struct iio_buffer *buffer =3D indio_dev->buffer; >> int len =3D 0; >> u16 *data; >> =20 >> @@ -76,7 +75,7 @@ static irqreturn_t iio_simple_dummy_trigger_h(int = irq, void *p) >> i < bitmap_weight(indio_dev->active_scan_mask, >> indio_dev->masklength); >> i++) { >> - j =3D find_next_bit(buffer->scan_mask, >> + j =3D find_next_bit(indio_dev->buffer->scan_mask, >=20 > should this be indio_dev->active_scan_mask? This bit always give me a headache. You are right of course. It's in the trigger handler so should not care at all about the specific buffer mask. >=20 >> indio_dev->masklength, j + 1); >> /* random access read form the 'device' */ >> data[i] =3D fakedata[j]; >=20 > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" = in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] staging:iio: Add support for multiple buffers 2012-06-09 12:17 ` Jonathan Cameron @ 2012-06-09 11:50 ` Lars-Peter Clausen 2012-06-09 17:56 ` Jonathan Cameron 0 siblings, 1 reply; 18+ messages in thread From: Lars-Peter Clausen @ 2012-06-09 11:50 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio On 06/09/2012 02:17 PM, Jonathan Cameron wrote: >>> [...] >>> @@ -534,31 +453,186 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev, const long *mask, >>> return bytes; >>> } >>> >>> -int iio_sw_buffer_preenable(struct iio_dev *indio_dev) >>> +int iio_update_buffers(struct iio_dev *indio_dev, >>> + struct iio_buffer *insert_buffer, >>> + struct iio_buffer *remove_buffer) >>> { >>> - struct iio_buffer *buffer = indio_dev->buffer; >>> - dev_dbg(&indio_dev->dev, "%s\n", __func__); >>> + int ret; >> >> drivers/iio/industrialio-buffer.c: In function ‘iio_update_buffers’: >> drivers/iio/industrialio-buffer.c:460: warning: ‘ret’ may be used >> uninitialized in this function >> >> I think there a missing 'return 0', before the error handling. Right now the >> code always sets active_scan_mask to NULL. > The setting to NULL definitely isn't right, but we do still need to > free the compoundmask. I'll fix that up. Only if compoundmask != active_scan_mask ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] staging:iio: Add support for multiple buffers 2012-06-09 11:50 ` Lars-Peter Clausen @ 2012-06-09 17:56 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2012-06-09 17:56 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: linux-iio On 06/09/2012 12:50 PM, Lars-Peter Clausen wrote: > On 06/09/2012 02:17 PM, Jonathan Cameron wrote: >>>> [...] >>>> @@ -534,31 +453,186 @@ static int iio_compute_scan_bytes(struct ii= o_dev *indio_dev, const long *mask, >>>> return bytes; >>>> } >>>> =20 >>>> -int iio_sw_buffer_preenable(struct iio_dev *indio_dev) >>>> +int iio_update_buffers(struct iio_dev *indio_dev, >>>> + struct iio_buffer *insert_buffer, >>>> + struct iio_buffer *remove_buffer) >>>> { >>>> - struct iio_buffer *buffer =3D indio_dev->buffer; >>>> - dev_dbg(&indio_dev->dev, "%s\n", __func__); >>>> + int ret; >>> >>> drivers/iio/industrialio-buffer.c: In function =91iio_update_buffer= s=92: >>> drivers/iio/industrialio-buffer.c:460: warning: =91ret=92 may be us= ed >>> uninitialized in this function >>> >>> I think there a missing 'return 0', before the error handling. Righ= t now the >>> code always sets active_scan_mask to NULL. >> The setting to NULL definitely isn't right, but we do still need to >> free the compoundmask. I'll fix that up. >=20 > Only if compoundmask !=3D active_scan_mask indeed. > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" = in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/4] staging:iio:in kernel users: Add a data field for channel specific info. 2012-05-30 19:36 [PATCH 0/4 V2] staging:iio: Add support for multiple buffers Jonathan Cameron 2012-05-30 19:36 ` [PATCH 1/4] " Jonathan Cameron @ 2012-05-30 19:36 ` Jonathan Cameron 2012-05-30 19:36 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron 2012-05-30 19:36 ` [PATCH 4/4] staging:iio: Proof of concept input driver Jonathan Cameron 3 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2012-05-30 19:36 UTC (permalink / raw) To: linux-iio; +Cc: Jonathan Cameron Used to allow information about a given channel mapping to be passed through from board files to the consumer drivers. Signed-off-by: Jonathan Cameron <jic23@kernel.org> --- drivers/iio/inkern.c | 1 + include/linux/iio/consumer.h | 2 ++ include/linux/iio/machine.h | 2 ++ 3 files changed, 5 insertions(+) diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c index 6c03cf5..c1ea198 100644 --- a/drivers/iio/inkern.c +++ b/drivers/iio/inkern.c @@ -188,6 +188,7 @@ struct iio_channel *iio_channel_get_all(const char *name) if (name && strcmp(name, c->map->consumer_dev_name) != 0) continue; chans[mapind].indio_dev = c->indio_dev; + chans[mapind].data = c->map->consumer_data; chans[mapind].channel = iio_chan_spec_from_name(chans[mapind].indio_dev, c->map->adc_channel_label); diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h index e2657e6..26040c2 100644 --- a/include/linux/iio/consumer.h +++ b/include/linux/iio/consumer.h @@ -18,10 +18,12 @@ struct iio_chan_spec; * struct iio_channel - everything needed for a consumer to use a channel * @indio_dev: Device on which the channel exists. * @channel: Full description of the channel. + * @data: Data about the channel used by consumer. */ struct iio_channel { struct iio_dev *indio_dev; const struct iio_chan_spec *channel; + void *data; }; /** diff --git a/include/linux/iio/machine.h b/include/linux/iio/machine.h index 0b1f19b..7409100 100644 --- a/include/linux/iio/machine.h +++ b/include/linux/iio/machine.h @@ -16,9 +16,11 @@ * @consumer_dev_name: Name to uniquely identify the consumer device. * @consumer_channel: Unique name used to idenitify the channel on the * consumer side. + * @consumer_data: Data about the channel for use by the consumer driver. */ struct iio_map { const char *adc_channel_label; const char *consumer_dev_name; const char *consumer_channel; + void *consumer_data; }; -- 1.7.10.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface 2012-05-30 19:36 [PATCH 0/4 V2] staging:iio: Add support for multiple buffers Jonathan Cameron 2012-05-30 19:36 ` [PATCH 1/4] " Jonathan Cameron 2012-05-30 19:36 ` [PATCH 2/4] staging:iio:in kernel users: Add a data field for channel specific info Jonathan Cameron @ 2012-05-30 19:36 ` Jonathan Cameron 2012-05-30 19:36 ` [PATCH 4/4] staging:iio: Proof of concept input driver Jonathan Cameron 3 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2012-05-30 19:36 UTC (permalink / raw) To: linux-iio; +Cc: Jonathan Cameron This callback buffer is meant to be opaque to users, but basically adds a very simple pass through buffer to which data may be pushed when it is inserted into the buffer list. Signed-off-by: Jonathan Cameron <jic23@kernel.org> --- drivers/iio/Kconfig | 6 +++ drivers/iio/Makefile | 1 + drivers/iio/buffer_cb.c | 115 ++++++++++++++++++++++++++++++++++++++++++ include/linux/iio/consumer.h | 46 +++++++++++++++++ 4 files changed, 168 insertions(+) diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig index 62fda86..c9e9483 100644 --- a/drivers/iio/Kconfig +++ b/drivers/iio/Kconfig @@ -21,6 +21,12 @@ config IIO_BUFFER if IIO_BUFFER +config IIO_BUFFER_CB + boolean "IIO callback buffer used for push in kernel interfaces" + help + Should be selected by any drivers that do inkernel push + usage. That is, those where the data is pushed to the consumer. + config IIO_KFIFO_BUF select IIO_TRIGGER tristate "Industrial I/O buffering based on kfifo" diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile index 30a9144..2df439c 100644 --- a/drivers/iio/Makefile +++ b/drivers/iio/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_IIO) += industrialio.o industrialio-y := industrialio-core.o industrialio-event.o inkern.o industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o +industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o diff --git a/drivers/iio/buffer_cb.c b/drivers/iio/buffer_cb.c new file mode 100644 index 0000000..d686bdf --- /dev/null +++ b/drivers/iio/buffer_cb.c @@ -0,0 +1,115 @@ +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/err.h> +#include <linux/export.h> +#include <linux/iio/buffer.h> +#include <linux/iio/consumer.h> + +struct iio_cb_buffer { + struct iio_buffer buffer; + int (*cb)(u8 *data, void *private); + void *private; + struct iio_channel *channels; +}; + +static int iio_buffer_cb_store_to(struct iio_buffer *buffer, + u8 *data, + s64 timestamp) +{ + struct iio_cb_buffer *cb_buff = container_of(buffer, + struct iio_cb_buffer, + buffer); + + return cb_buff->cb(data, cb_buff->private); +} + +static struct iio_buffer_access_funcs iio_cb_access = { + .store_to = &iio_buffer_cb_store_to, +}; + +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, + int (*cb)(u8 *data, + void *private), + void *private) +{ + int ret; + struct iio_cb_buffer *cb_buff; + struct iio_dev *indio_dev; + struct iio_channel *chan; + + cb_buff = kzalloc(sizeof *cb_buff, GFP_KERNEL); + if (cb_buff == NULL) { + ret = -ENOMEM; + goto error_ret; + } + + cb_buff->private = private; + cb_buff->cb = cb; + cb_buff->buffer.access = &iio_cb_access; + INIT_LIST_HEAD(&cb_buff->buffer.demux_list); + + cb_buff->channels = iio_channel_get_all(name); + if (IS_ERR(cb_buff->channels)) { + ret = PTR_ERR(cb_buff->channels); + goto error_free_cb_buff; + } + + indio_dev = cb_buff->channels[0].indio_dev; + cb_buff->buffer.scan_mask + = kzalloc(sizeof(long)*BITS_TO_LONGS(indio_dev->masklength), + GFP_KERNEL); + if (cb_buff->buffer.scan_mask == NULL) { + ret = -ENOMEM; + goto error_release_channels; + } + chan = &cb_buff->channels[0]; + while (chan->indio_dev) { + if (chan->indio_dev != indio_dev) { + ret = -EINVAL; + goto error_release_channels; + } + set_bit(chan->channel->scan_index, + cb_buff->buffer.scan_mask); + chan++; + } + + return cb_buff; + +error_release_channels: + iio_channel_release_all(cb_buff->channels); +error_free_cb_buff: + kfree(cb_buff); +error_ret: + return ERR_PTR(ret); +} +EXPORT_SYMBOL_GPL(iio_channel_get_all_cb); + +void iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff) +{ + iio_update_buffers(cb_buff->channels[0].indio_dev, + &cb_buff->buffer, + NULL); +} +EXPORT_SYMBOL_GPL(iio_channel_start_all_cb); + +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff) +{ + iio_update_buffers(cb_buff->channels[0].indio_dev, + NULL, + &cb_buff->buffer); +} +EXPORT_SYMBOL_GPL(iio_channel_stop_all_cb); + +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buff) +{ + iio_channel_release_all(cb_buff->channels); + kfree(cb_buff); +} +EXPORT_SYMBOL_GPL(iio_channel_release_all_cb); + +struct iio_channel +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer) +{ + return cb_buffer->channels; +} +EXPORT_SYMBOL_GPL(iio_channel_cb_get_channels); diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h index 26040c2..efd377e 100644 --- a/include/linux/iio/consumer.h +++ b/include/linux/iio/consumer.h @@ -61,6 +61,52 @@ struct iio_channel *iio_channel_get_all(const char *name); */ void iio_channel_release_all(struct iio_channel *chan); +struct iio_cb_buffer; +/** + * iio_channel_get_all_cb() - register callback for triggered capture + * @name: Name of client device. + * @cb: Callback function. + * @private: Private data passed to callback. + * + * NB right now we have no ability to mux data from multiple devices. + * So if the channels requested come from different devices this will + * fail. + */ +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, + int (*cb)(u8 *data, + void *private), + void *private); +/** + * iio_channel_release_all_cb() - release and unregister the callback. + * @cb_buffer: The callback buffer that was allocated. + */ +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buffer); + +/** + * iio_channel_start_all_cb() - start the flow of data through callback. + * @cb_buff: The callback buffer we are starting. + */ +void iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff); + +/** + * iio_channel_stop_all_cb() - stop the flow of data through the callback. + * @cb_buff: The callback buffer we are stopping. + */ +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff); + +/** + * iio_channel_cb_get_channels() - get access to the underlying channels. + * @cb_buff: The callback buffers from whom we want the channel + * information. + * + * This function allows one to obtain information about the channels. + * Whilst this may allow direct reading if all buffers are disabled, the + * primary aim is to allow drivers that are consuming a channel to query + * things like scaling of the channel. + */ +struct iio_channel +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer); + /** * iio_read_channel_raw() - read from a given channel * @channel: The channel being queried. -- 1.7.10.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/4] staging:iio: Proof of concept input driver. 2012-05-30 19:36 [PATCH 0/4 V2] staging:iio: Add support for multiple buffers Jonathan Cameron ` (2 preceding siblings ...) 2012-05-30 19:36 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron @ 2012-05-30 19:36 ` Jonathan Cameron 2012-06-08 15:27 ` Lars-Peter Clausen 3 siblings, 1 reply; 18+ messages in thread From: Jonathan Cameron @ 2012-05-30 19:36 UTC (permalink / raw) To: linux-iio; +Cc: Jonathan Cameron, Jonathan Cameron From: Jonathan Cameron <jic23@cam.ac.uk> This is no where near ready to merge. Lots of stuff missing. Signed-off-by: Jonathan Cameron <jic23@kernel.org> --- drivers/staging/iio/Kconfig | 11 +++ drivers/staging/iio/Makefile | 1 + drivers/staging/iio/iio_input.c | 176 +++++++++++++++++++++++++++++++++++++++ drivers/staging/iio/iio_input.h | 23 +++++ 4 files changed, 211 insertions(+) diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig index 04cd6ec..022463e 100644 --- a/drivers/staging/iio/Kconfig +++ b/drivers/staging/iio/Kconfig @@ -4,6 +4,17 @@ menu "IIO staging drivers" depends on IIO +config IIO_ST_INPUT + tristate "Input driver that uses channels specified via iio maps" + depends on INPUT + depends on IIO_BUFFER + select IIO_BUFFER_CB + help + Client driver for IIO via the push interfaces. Used to provide + and input interface for IIO devices that can feed a buffer on + a trigger interrupt. Note that all channels must come from a + single IIO supported device. + config IIO_ST_HWMON tristate "Hwmon driver that uses channels specified via iio maps" depends on HWMON diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile index fa6937d..9e71498 100644 --- a/drivers/staging/iio/Makefile +++ b/drivers/staging/iio/Makefile @@ -12,6 +12,7 @@ iio_dummy-$(CONFIG_IIO_SIMPLE_DUMMY_BUFFER) += iio_simple_dummy_buffer.o obj-$(CONFIG_IIO_DUMMY_EVGEN) += iio_dummy_evgen.o obj-$(CONFIG_IIO_ST_HWMON) += iio_hwmon.o +obj-$(CONFIG_IIO_ST_INPUT) += iio_input.o obj-y += accel/ obj-y += adc/ diff --git a/drivers/staging/iio/iio_input.c b/drivers/staging/iio/iio_input.c new file mode 100644 index 0000000..42f4cfb --- /dev/null +++ b/drivers/staging/iio/iio_input.c @@ -0,0 +1,176 @@ +/* + * The industrial I/O input client driver + * + * Copyright (c) 2011 Jonathan Cameron + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + */ + +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/module.h> +#include <linux/err.h> +#include <linux/platform_device.h> +#include <linux/input.h> +#include <linux/iio/buffer.h> +#include <linux/iio/consumer.h> +#include "iio_input.h" + +struct iio_input_state { + struct iio_cb_buffer *buff; + struct input_dev *idev; +}; + +static int iio_channel_value(u8 *data, + const struct iio_chan_spec *chan, + int *val) +{ + int value; + + if (chan->scan_type.sign == 's') { + switch (chan->scan_type.storagebits) { + case 8: + value = *(s8 *)(data); + break; + case 16: + value = *(s16 *)(data); + break; + case 32: + value = *(s32 *)(data); + break; + default: + return -EINVAL; + } + value >>= chan->scan_type.shift; + value &= (1 << chan->scan_type.realbits) - 1; + value = (value << (sizeof(value)*8 - chan->scan_type.realbits)) + >> (sizeof(value)*8 - chan->scan_type.realbits); + } else { + switch (chan->scan_type.storagebits) { + case 8: + value = *(u8 *)(data); + break; + case 16: + value = *(u16 *)(data); + break; + case 32: + value = *(u32 *)(data); + break; + default: + return -EINVAL; + } + value >>= chan->scan_type.shift; + value &= (1 << chan->scan_type.realbits) - 1; + } + *val = value; + + return 0; +} + +static int iio_input_store_to(u8 *data, void *private) +{ + struct iio_input_state *st = private; + struct iio_channel *channel; + struct iio_input_channel_data *input_data; + int offset = 0; + int value, ret; + + channel = iio_channel_cb_get_channels(st->buff); + while (channel->indio_dev) { + input_data = channel->data; + offset = ALIGN(offset, + channel->channel->scan_type.storagebits/8); + offset += channel->channel->scan_type.storagebits/8; + ret = iio_channel_value(&data[offset], + channel->channel, + &value); + if (ret < 0) + return ret; + input_report_abs(st->idev, input_data->code, value); + } + input_sync(st->idev); + + return 0; +} + +static int __devinit iio_input_probe(struct platform_device *pdev) +{ + struct iio_input_state *st; + int ret; + struct iio_channel *channel; + struct iio_input_channel_data *input_data; + + st = kzalloc(sizeof(*st), GFP_KERNEL); + if (st == NULL) + return -ENOMEM; + platform_set_drvdata(pdev, st); + st->buff = iio_channel_get_all_cb(dev_name(&pdev->dev), + &iio_input_store_to, + st); + if (IS_ERR(st->buff)) { + ret = PTR_ERR(st->buff); + goto error_free_state; + } + + st->idev = input_allocate_device(); + if (!st->idev) { + ret = -ENOMEM; + goto error_channels_release_all; + } + + __set_bit(EV_ABS, st->idev->evbit); + channel = iio_channel_cb_get_channels(st->buff); + while (channel->indio_dev) { + input_data = channel->data; + input_set_abs_params(st->idev, input_data->code, + input_data->min, + input_data->max, + input_data->fuzz, + input_data->flat); + } + + ret = input_register_device(st->idev); + if (ret < 0) + goto error_free_idev; + + /* NORMALLY IN THE OPEN */ + iio_channel_start_all_cb(st->buff); + + return 0; +error_free_idev: + input_free_device(st->idev); +error_channels_release_all: + iio_channel_release_all_cb(st->buff); +error_free_state: + kfree(st); + return ret; +} + +static int __devexit iio_input_remove(struct platform_device *pdev) +{ + struct iio_input_state *st = platform_get_drvdata(pdev); + /* NORMALLY IN THE CLOSE */ + iio_channel_stop_all_cb(st->buff); + input_unregister_device(st->idev); + iio_channel_release_all_cb(st->buff); + + kfree(st); + return 0; +} + +static struct platform_driver iio_input_driver = { + .driver = { + .name = "iio_snoop", + .owner = THIS_MODULE, + }, + .probe = iio_input_probe, + .remove = __devexit_p(iio_input_remove), +}; + +module_platform_driver(iio_input_driver); + +MODULE_AUTHOR("Jonathan Cameron <jic23@cam.ac.uk>"); +MODULE_DESCRIPTION("IIO input buffer driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/staging/iio/iio_input.h b/drivers/staging/iio/iio_input.h new file mode 100644 index 0000000..cfd1d34 --- /dev/null +++ b/drivers/staging/iio/iio_input.h @@ -0,0 +1,23 @@ +/* + * The industrial I/O input client driver + * + * Copyright (c) 2011 Jonathan Cameron + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + */ + +/** + * iio_input_channel_data - description of the channel for input subsystem + * @code: Absolute axis. + * @min: Minimum value. + * @max: Maximum value. + * @fuzz: Used to filter noise from the event stream. + * @flat: Values within this value will be discarded by joydev + * and reported as 0 instead. + */ +struct iio_input_channel_data { + unsigned int code; + int min, max, fuzz, flat; +}; -- 1.7.10.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] staging:iio: Proof of concept input driver. 2012-05-30 19:36 ` [PATCH 4/4] staging:iio: Proof of concept input driver Jonathan Cameron @ 2012-06-08 15:27 ` Lars-Peter Clausen 2012-06-09 17:56 ` Jonathan Cameron 0 siblings, 1 reply; 18+ messages in thread From: Lars-Peter Clausen @ 2012-06-08 15:27 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron On 05/30/2012 09:36 PM, Jonathan Cameron wrote: > From: Jonathan Cameron <jic23@cam.ac.uk> > > This is no where near ready to merge. Lots of stuff missing. > > Signed-off-by: Jonathan Cameron <jic23@kernel.org> > --- > drivers/staging/iio/Kconfig | 11 +++ > drivers/staging/iio/Makefile | 1 + > drivers/staging/iio/iio_input.c | 176 +++++++++++++++++++++++++++++++++++++++ > drivers/staging/iio/iio_input.h | 23 +++++ > 4 files changed, 211 insertions(+) > > diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig > index 04cd6ec..022463e 100644 > --- a/drivers/staging/iio/Kconfig > +++ b/drivers/staging/iio/Kconfig > @@ -4,6 +4,17 @@ [...] > +}; > + > +static int iio_channel_value(u8 *data, > + const struct iio_chan_spec *chan, > + int *val) > +{ > + int value; > + > + if (chan->scan_type.sign == 's') { > + switch (chan->scan_type.storagebits) { > + case 8: > + value = *(s8 *)(data); > + break; > + case 16: > + value = *(s16 *)(data); > + break; > + case 32: > + value = *(s32 *)(data); > + break; > + default: > + return -EINVAL; > + } > + value >>= chan->scan_type.shift; > + value &= (1 << chan->scan_type.realbits) - 1; > + value = (value << (sizeof(value)*8 - chan->scan_type.realbits)) > + >> (sizeof(value)*8 - chan->scan_type.realbits); This looks scarey. There is a sign_extend32 function. It probably makes sense to add one which works on ints. Btw. I think until the sign extension the type should still be unsigned. This will also get rid of the duplicated code. > + } else { > + switch (chan->scan_type.storagebits) { > + case 8: > + value = *(u8 *)(data); > + break; > + case 16: > + value = *(u16 *)(data); > + break; > + case 32: > + value = *(u32 *)(data); > + break; > + default: > + return -EINVAL; > + } > + value >>= chan->scan_type.shift; > + value &= (1 << chan->scan_type.realbits) - 1; > + } > + *val = value; This function probably needs to be extended to deal with the endianness field of the scan type at some point. > + > + return 0; > +} > + > +static int iio_input_store_to(u8 *data, void *private) > +{ > + struct iio_input_state *st = private; > + struct iio_channel *channel; > + struct iio_input_channel_data *input_data; > + int offset = 0; > + int value, ret; > + > + channel = iio_channel_cb_get_channels(st->buff); > + while (channel->indio_dev) { > + input_data = channel->data; > + offset = ALIGN(offset, > + channel->channel->scan_type.storagebits/8); > + offset += channel->channel->scan_type.storagebits/8; Maybe I'm missing something, but shouldn't offset be increased after reading the data? > + ret = iio_channel_value(&data[offset], > + channel->channel, > + &value); > + if (ret < 0) > + return ret; > + input_report_abs(st->idev, input_data->code, value); > + } > + input_sync(st->idev); > + > + return 0; > +} > + [...] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] staging:iio: Proof of concept input driver. 2012-06-08 15:27 ` Lars-Peter Clausen @ 2012-06-09 17:56 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2012-06-09 17:56 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: linux-iio, Jonathan Cameron On 06/08/2012 04:27 PM, Lars-Peter Clausen wrote: > On 05/30/2012 09:36 PM, Jonathan Cameron wrote: >> From: Jonathan Cameron <jic23@cam.ac.uk> >> >> This is no where near ready to merge. Lots of stuff missing. >> >> Signed-off-by: Jonathan Cameron <jic23@kernel.org> >> --- >> drivers/staging/iio/Kconfig | 11 +++ >> drivers/staging/iio/Makefile | 1 + >> drivers/staging/iio/iio_input.c | 176 +++++++++++++++++++++++++++++++++++++++ >> drivers/staging/iio/iio_input.h | 23 +++++ >> 4 files changed, 211 insertions(+) >> >> diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig >> index 04cd6ec..022463e 100644 >> --- a/drivers/staging/iio/Kconfig >> +++ b/drivers/staging/iio/Kconfig >> @@ -4,6 +4,17 @@ > [...] >> +}; >> + >> +static int iio_channel_value(u8 *data, >> + const struct iio_chan_spec *chan, >> + int *val) >> +{ >> + int value; >> + >> + if (chan->scan_type.sign == 's') { >> + switch (chan->scan_type.storagebits) { >> + case 8: >> + value = *(s8 *)(data); >> + break; >> + case 16: >> + value = *(s16 *)(data); >> + break; >> + case 32: >> + value = *(s32 *)(data); >> + break; >> + default: >> + return -EINVAL; >> + } >> + value >>= chan->scan_type.shift; >> + value &= (1 << chan->scan_type.realbits) - 1; >> + value = (value << (sizeof(value)*8 - chan->scan_type.realbits)) >> + >> (sizeof(value)*8 - chan->scan_type.realbits); > > This looks scarey. There is a sign_extend32 function. It probably makes > sense to add one which works on ints. Btw. I think until the sign extension > the type should still be unsigned. This will also get rid of the duplicated > code. Not sure there is any real reason not to use sign_extend32 as is. Can just convert much later at the input_report_abs call. > >> + } else { >> + switch (chan->scan_type.storagebits) { >> + case 8: >> + value = *(u8 *)(data); >> + break; >> + case 16: >> + value = *(u16 *)(data); >> + break; >> + case 32: >> + value = *(u32 *)(data); >> + break; >> + default: >> + return -EINVAL; >> + } >> + value >>= chan->scan_type.shift; >> + value &= (1 << chan->scan_type.realbits) - 1; >> + } >> + *val = value; > > This function probably needs to be extended to deal with the endianness > field of the scan type at some point. Yup. Lots of extensions needed to this driver.... >> + >> + return 0; >> +} >> + >> +static int iio_input_store_to(u8 *data, void *private) >> +{ >> + struct iio_input_state *st = private; >> + struct iio_channel *channel; >> + struct iio_input_channel_data *input_data; >> + int offset = 0; >> + int value, ret; >> + >> + channel = iio_channel_cb_get_channels(st->buff); >> + while (channel->indio_dev) { >> + input_data = channel->data; >> + offset = ALIGN(offset, >> + channel->channel->scan_type.storagebits/8); >> + offset += channel->channel->scan_type.storagebits/8; > > Maybe I'm missing something, but shouldn't offset be increased after reading > the data? Indeed. I'm clearly going mad ;) > >> + ret = iio_channel_value(&data[offset], >> + channel->channel, >> + &value); >> + if (ret < 0) >> + return ret; >> + input_report_abs(st->idev, input_data->code, value); >> + } >> + input_sync(st->idev); >> + >> + return 0; >> +} >> + > [...] > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/4 V3] staging:iio: Add support for multiple buffers
@ 2012-06-30 19:06 Jonathan Cameron
2012-06-30 19:06 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron
0 siblings, 1 reply; 18+ messages in thread
From: Jonathan Cameron @ 2012-06-30 19:06 UTC (permalink / raw)
To: linux-iio; +Cc: Jonathan Cameron
Changes since V2.
Improved handling of error conditions in the buffer insertion and
removal code. (suggested by Lars-Peter). I want to take another
look at these, but didn't want to keep people from testing this
in the meantime.
Also a number of minor fixes that came up during testing.
Thanks,
Jonathan
Jonathan Cameron (4):
staging:iio: Add support for multiple buffers
staging:iio:in kernel users: Add a data field for channel specific
info.
staging:iio: add a callback buffer for in kernel push interface
staging:iio: Proof of concept input driver.
drivers/iio/Kconfig | 6 +
drivers/iio/Makefile | 1 +
drivers/iio/adc/at91_adc.c | 4 +-
drivers/iio/buffer_cb.c | 115 +++++++
drivers/iio/industrialio-buffer.c | 378 ++++++++++++++++--------
drivers/iio/industrialio-core.c | 1 +
drivers/iio/inkern.c | 1 +
drivers/staging/iio/Kconfig | 11 +
drivers/staging/iio/Makefile | 1 +
drivers/staging/iio/accel/adis16201_ring.c | 4 +-
drivers/staging/iio/accel/adis16203_ring.c | 6 +-
drivers/staging/iio/accel/adis16204_ring.c | 3 +-
drivers/staging/iio/accel/adis16209_ring.c | 3 +-
drivers/staging/iio/accel/adis16240_ring.c | 4 +-
drivers/staging/iio/accel/lis3l02dq_ring.c | 3 +-
drivers/staging/iio/adc/ad7192.c | 3 +-
drivers/staging/iio/adc/ad7298_ring.c | 5 +-
drivers/staging/iio/adc/ad7476_ring.c | 2 +-
drivers/staging/iio/adc/ad7606_ring.c | 3 +-
drivers/staging/iio/adc/ad7793.c | 3 +-
drivers/staging/iio/adc/ad7887_ring.c | 2 +-
drivers/staging/iio/adc/ad799x_ring.c | 3 +-
drivers/staging/iio/adc/max1363_ring.c | 2 +-
drivers/staging/iio/gyro/adis16260_ring.c | 3 +-
drivers/staging/iio/iio_input.c | 221 ++++++++++++++
drivers/staging/iio/iio_input.h | 23 ++
drivers/staging/iio/iio_simple_dummy_buffer.c | 5 +-
drivers/staging/iio/impedance-analyzer/ad5933.c | 3 +-
drivers/staging/iio/imu/adis16400_ring.c | 2 +-
drivers/staging/iio/meter/ade7758_ring.c | 3 +-
include/linux/iio/buffer.h | 24 +-
include/linux/iio/consumer.h | 48 +++
include/linux/iio/iio.h | 2 +
include/linux/iio/machine.h | 2 +
34 files changed, 734 insertions(+), 166 deletions(-)
create mode 100644 drivers/iio/buffer_cb.c
create mode 100644 drivers/staging/iio/iio_input.c
create mode 100644 drivers/staging/iio/iio_input.h
--
1.7.11.1
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface 2012-06-30 19:06 [PATCH 0/4 V3] staging:iio: Add support for multiple buffers Jonathan Cameron @ 2012-06-30 19:06 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2012-06-30 19:06 UTC (permalink / raw) To: linux-iio; +Cc: Jonathan Cameron This callback buffer is meant to be opaque to users, but basically adds a very simple pass through buffer to which data may be pushed when it is inserted into the buffer list. Signed-off-by: Jonathan Cameron <jic23@kernel.org> --- drivers/iio/Kconfig | 6 +++ drivers/iio/Makefile | 1 + drivers/iio/buffer_cb.c | 115 +++++++++++++++++++++++++++++++++++++++++++ include/linux/iio/consumer.h | 46 +++++++++++++++++ 4 files changed, 168 insertions(+) diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig index d4984c8..9e92501 100644 --- a/drivers/iio/Kconfig +++ b/drivers/iio/Kconfig @@ -20,6 +20,12 @@ config IIO_BUFFER if IIO_BUFFER +config IIO_BUFFER_CB + boolean "IIO callback buffer used for push in kernel interfaces" + help + Should be selected by any drivers that do inkernel push + usage. That is, those where the data is pushed to the consumer. + config IIO_KFIFO_BUF select IIO_TRIGGER tristate "Industrial I/O buffering based on kfifo" diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile index 34309ab..14d5e4c 100644 --- a/drivers/iio/Makefile +++ b/drivers/iio/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_IIO) += industrialio.o industrialio-y := industrialio-core.o industrialio-event.o inkern.o industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o +industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o diff --git a/drivers/iio/buffer_cb.c b/drivers/iio/buffer_cb.c new file mode 100644 index 0000000..4a807f0 --- /dev/null +++ b/drivers/iio/buffer_cb.c @@ -0,0 +1,115 @@ +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/err.h> +#include <linux/export.h> +#include <linux/iio/buffer.h> +#include <linux/iio/consumer.h> + +struct iio_cb_buffer { + struct iio_buffer buffer; + int (*cb)(u8 *data, void *private); + void *private; + struct iio_channel *channels; +}; + +static int iio_buffer_cb_store_to(struct iio_buffer *buffer, + u8 *data, + s64 timestamp) +{ + struct iio_cb_buffer *cb_buff = container_of(buffer, + struct iio_cb_buffer, + buffer); + + return cb_buff->cb(data, cb_buff->private); +} + +static struct iio_buffer_access_funcs iio_cb_access = { + .store_to = &iio_buffer_cb_store_to, +}; + +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, + int (*cb)(u8 *data, + void *private), + void *private) +{ + int ret; + struct iio_cb_buffer *cb_buff; + struct iio_dev *indio_dev; + struct iio_channel *chan; + + cb_buff = kzalloc(sizeof *cb_buff, GFP_KERNEL); + if (cb_buff == NULL) { + ret = -ENOMEM; + goto error_ret; + } + + cb_buff->private = private; + cb_buff->cb = cb; + cb_buff->buffer.access = &iio_cb_access; + INIT_LIST_HEAD(&cb_buff->buffer.demux_list); + + cb_buff->channels = iio_channel_get_all(name); + if (IS_ERR(cb_buff->channels)) { + ret = PTR_ERR(cb_buff->channels); + goto error_free_cb_buff; + } + + indio_dev = cb_buff->channels[0].indio_dev; + cb_buff->buffer.scan_mask + = kzalloc(sizeof(long)*BITS_TO_LONGS(indio_dev->masklength), + GFP_KERNEL); + if (cb_buff->buffer.scan_mask == NULL) { + ret = -ENOMEM; + goto error_release_channels; + } + chan = &cb_buff->channels[0]; + while (chan->indio_dev) { + if (chan->indio_dev != indio_dev) { + ret = -EINVAL; + goto error_release_channels; + } + set_bit(chan->channel->scan_index, + cb_buff->buffer.scan_mask); + chan++; + } + + return cb_buff; + +error_release_channels: + iio_channel_release_all(cb_buff->channels); +error_free_cb_buff: + kfree(cb_buff); +error_ret: + return ERR_PTR(ret); +} +EXPORT_SYMBOL_GPL(iio_channel_get_all_cb); + +int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff) +{ + return iio_update_buffers(cb_buff->channels[0].indio_dev, + &cb_buff->buffer, + NULL); +} +EXPORT_SYMBOL_GPL(iio_channel_start_all_cb); + +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff) +{ + iio_update_buffers(cb_buff->channels[0].indio_dev, + NULL, + &cb_buff->buffer); +} +EXPORT_SYMBOL_GPL(iio_channel_stop_all_cb); + +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buff) +{ + iio_channel_release_all(cb_buff->channels); + kfree(cb_buff); +} +EXPORT_SYMBOL_GPL(iio_channel_release_all_cb); + +struct iio_channel +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer) +{ + return cb_buffer->channels; +} +EXPORT_SYMBOL_GPL(iio_channel_cb_get_channels); diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h index 26040c2..9e22b73 100644 --- a/include/linux/iio/consumer.h +++ b/include/linux/iio/consumer.h @@ -61,6 +61,52 @@ struct iio_channel *iio_channel_get_all(const char *name); */ void iio_channel_release_all(struct iio_channel *chan); +struct iio_cb_buffer; +/** + * iio_channel_get_all_cb() - register callback for triggered capture + * @name: Name of client device. + * @cb: Callback function. + * @private: Private data passed to callback. + * + * NB right now we have no ability to mux data from multiple devices. + * So if the channels requested come from different devices this will + * fail. + */ +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, + int (*cb)(u8 *data, + void *private), + void *private); +/** + * iio_channel_release_all_cb() - release and unregister the callback. + * @cb_buffer: The callback buffer that was allocated. + */ +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buffer); + +/** + * iio_channel_start_all_cb() - start the flow of data through callback. + * @cb_buff: The callback buffer we are starting. + */ +int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff); + +/** + * iio_channel_stop_all_cb() - stop the flow of data through the callback. + * @cb_buff: The callback buffer we are stopping. + */ +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff); + +/** + * iio_channel_cb_get_channels() - get access to the underlying channels. + * @cb_buff: The callback buffers from whom we want the channel + * information. + * + * This function allows one to obtain information about the channels. + * Whilst this may allow direct reading if all buffers are disabled, the + * primary aim is to allow drivers that are consuming a channel to query + * things like scaling of the channel. + */ +struct iio_channel +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer); + /** * iio_read_channel_raw() - read from a given channel * @channel: The channel being queried. -- 1.7.11.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 0/4 V5] staging:iio: Add support for multiple buffers (testing required!)
@ 2012-10-13 9:24 Jonathan Cameron
2012-10-13 9:24 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron
0 siblings, 1 reply; 18+ messages in thread
From: Jonathan Cameron @ 2012-10-13 9:24 UTC (permalink / raw)
To: linux-iio; +Cc: Michael.Hennerich, lars, srinivas.pandruvada, Jonathan Cameron
In quick summary this is needed to do interrupt driven in kernel
clients as it adds the option to push to multiple 'buffer' interfaces.
Note that some of these may not actually do any buffering but rather
push directly into another subsytem (the example is input).
I would like to merge at least the first 3 patches early in this cycle.
What I need is some testing across a reasonable range of devices. Review
is welcome of course as well!
The complexity in this set is entirely in the changes to industrialio-buffer.c
concerning how we go about adding and removing client buffers. To do this
we need to work out which scan mask is appropriate given the requests of
all the given buffers (and whether we can satisfy it). I've been testing
with the max1363 driver which probably has the most complex set of available
scan masks. Thus when a new buffer is added or one is removed the whole
buffer set are torn down and built up again. This may be undesirable for
some applications but should occur only rarely.
The example driver is for bridging to the input subsystem and still needs
some work, but that can occur later in this merge cycle whereas the changes
to the core really want to go in asap.
Changes since v3
Rebased and added other drivers that have occured whilst this patch series
has been languishing in my local tree.
Changes since V2.
Improved handling of error conditions in the buffer insertion and
removal code. (suggested by Lars-Peter). I want to take another
look at these, but didn't want to keep people from testing this
in the meantime.
Also a number of minor fixes that came up during testing.
Thanks,
Jonathan
Jonathan Cameron (4):
staging:iio: Add support for multiple buffers
staging:iio:in kernel users: Add a data field for channel specific
info.
staging:iio: add a callback buffer for in kernel push interface
staging:iio: Proof of concept input driver.
drivers/iio/Kconfig | 6 +
drivers/iio/Makefile | 1 +
drivers/iio/accel/hid-sensor-accel-3d.c | 15 +-
drivers/iio/adc/ad7266.c | 3 +-
drivers/iio/adc/ad7476.c | 2 +-
drivers/iio/adc/ad_sigma_delta.c | 2 +-
drivers/iio/adc/at91_adc.c | 3 +-
drivers/iio/buffer_cb.c | 113 +++++++
drivers/iio/gyro/hid-sensor-gyro-3d.c | 15 +-
drivers/iio/industrialio-buffer.c | 377 ++++++++++++++++--------
drivers/iio/industrialio-core.c | 1 +
drivers/iio/inkern.c | 1 +
drivers/iio/light/adjd_s311.c | 3 +-
drivers/iio/light/hid-sensor-als.c | 15 +-
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 15 +-
drivers/staging/iio/Kconfig | 11 +
drivers/staging/iio/Makefile | 1 +
drivers/staging/iio/accel/adis16201_ring.c | 2 +-
drivers/staging/iio/accel/adis16203_ring.c | 2 +-
drivers/staging/iio/accel/adis16204_ring.c | 2 +-
drivers/staging/iio/accel/adis16209_ring.c | 2 +-
drivers/staging/iio/accel/adis16240_ring.c | 2 +-
drivers/staging/iio/accel/lis3l02dq_ring.c | 2 +-
drivers/staging/iio/adc/ad7298_ring.c | 2 +-
drivers/staging/iio/adc/ad7606_ring.c | 2 +-
drivers/staging/iio/adc/ad7887_ring.c | 2 +-
drivers/staging/iio/adc/ad799x_ring.c | 2 +-
drivers/staging/iio/adc/max1363_ring.c | 2 +-
drivers/staging/iio/gyro/adis16260_ring.c | 2 +-
drivers/staging/iio/iio_input.c | 225 ++++++++++++++
drivers/staging/iio/iio_input.h | 23 ++
drivers/staging/iio/iio_simple_dummy_buffer.c | 5 +-
drivers/staging/iio/impedance-analyzer/ad5933.c | 4 +-
drivers/staging/iio/imu/adis16400_ring.c | 5 +-
drivers/staging/iio/meter/ade7758_ring.c | 2 +-
include/linux/iio/buffer.h | 24 +-
include/linux/iio/consumer.h | 48 +++
include/linux/iio/iio.h | 2 +
include/linux/iio/machine.h | 2 +
39 files changed, 740 insertions(+), 208 deletions(-)
create mode 100644 drivers/iio/buffer_cb.c
create mode 100644 drivers/staging/iio/iio_input.c
create mode 100644 drivers/staging/iio/iio_input.h
--
1.7.12.2
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface 2012-10-13 9:24 [PATCH 0/4 V5] staging:iio: Add support for multiple buffers (testing required!) Jonathan Cameron @ 2012-10-13 9:24 ` Jonathan Cameron 2012-10-17 8:50 ` Lars-Peter Clausen 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Cameron @ 2012-10-13 9:24 UTC (permalink / raw) To: linux-iio; +Cc: Michael.Hennerich, lars, srinivas.pandruvada, Jonathan Cameron This callback buffer is meant to be opaque to users, but basically adds a very simple pass through buffer to which data may be pushed when it is inserted into the buffer list. Signed-off-by: Jonathan Cameron <jic23@kernel.org> --- drivers/iio/Kconfig | 6 +++ drivers/iio/Makefile | 1 + drivers/iio/buffer_cb.c | 113 +++++++++++++++++++++++++++++++++++++++++++ include/linux/iio/consumer.h | 46 ++++++++++++++++++ 4 files changed, 166 insertions(+) diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig index 6e3f143..0aad08a 100644 --- a/drivers/iio/Kconfig +++ b/drivers/iio/Kconfig @@ -20,6 +20,12 @@ config IIO_BUFFER if IIO_BUFFER +config IIO_BUFFER_CB + boolean "IIO callback buffer used for push in kernel interfaces" + help + Should be selected by any drivers that do inkernel push + usage. That is, those where the data is pushed to the consumer. + config IIO_KFIFO_BUF select IIO_TRIGGER tristate "Industrial I/O buffering based on kfifo" diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile index f7fa3c0..a27ec1c 100644 --- a/drivers/iio/Makefile +++ b/drivers/iio/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_IIO) += industrialio.o industrialio-y := industrialio-core.o industrialio-event.o inkern.o industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o +industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o diff --git a/drivers/iio/buffer_cb.c b/drivers/iio/buffer_cb.c new file mode 100644 index 0000000..d1e4881 --- /dev/null +++ b/drivers/iio/buffer_cb.c @@ -0,0 +1,113 @@ +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/err.h> +#include <linux/export.h> +#include <linux/iio/buffer.h> +#include <linux/iio/consumer.h> + +struct iio_cb_buffer { + struct iio_buffer buffer; + int (*cb)(u8 *data, void *private); + void *private; + struct iio_channel *channels; +}; + +static int iio_buffer_cb_store_to(struct iio_buffer *buffer, u8 *data) +{ + struct iio_cb_buffer *cb_buff = container_of(buffer, + struct iio_cb_buffer, + buffer); + + return cb_buff->cb(data, cb_buff->private); +} + +static struct iio_buffer_access_funcs iio_cb_access = { + .store_to = &iio_buffer_cb_store_to, +}; + +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, + int (*cb)(u8 *data, + void *private), + void *private) +{ + int ret; + struct iio_cb_buffer *cb_buff; + struct iio_dev *indio_dev; + struct iio_channel *chan; + + cb_buff = kzalloc(sizeof *cb_buff, GFP_KERNEL); + if (cb_buff == NULL) { + ret = -ENOMEM; + goto error_ret; + } + + cb_buff->private = private; + cb_buff->cb = cb; + cb_buff->buffer.access = &iio_cb_access; + INIT_LIST_HEAD(&cb_buff->buffer.demux_list); + + cb_buff->channels = iio_channel_get_all(name); + if (IS_ERR(cb_buff->channels)) { + ret = PTR_ERR(cb_buff->channels); + goto error_free_cb_buff; + } + + indio_dev = cb_buff->channels[0].indio_dev; + cb_buff->buffer.scan_mask + = kzalloc(sizeof(long)*BITS_TO_LONGS(indio_dev->masklength), + GFP_KERNEL); + if (cb_buff->buffer.scan_mask == NULL) { + ret = -ENOMEM; + goto error_release_channels; + } + chan = &cb_buff->channels[0]; + while (chan->indio_dev) { + if (chan->indio_dev != indio_dev) { + ret = -EINVAL; + goto error_release_channels; + } + set_bit(chan->channel->scan_index, + cb_buff->buffer.scan_mask); + chan++; + } + + return cb_buff; + +error_release_channels: + iio_channel_release_all(cb_buff->channels); +error_free_cb_buff: + kfree(cb_buff); +error_ret: + return ERR_PTR(ret); +} +EXPORT_SYMBOL_GPL(iio_channel_get_all_cb); + +int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff) +{ + return iio_update_buffers(cb_buff->channels[0].indio_dev, + &cb_buff->buffer, + NULL); +} +EXPORT_SYMBOL_GPL(iio_channel_start_all_cb); + +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff) +{ + iio_update_buffers(cb_buff->channels[0].indio_dev, + NULL, + &cb_buff->buffer); +} +EXPORT_SYMBOL_GPL(iio_channel_stop_all_cb); + +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buff) +{ + iio_channel_release_all(cb_buff->channels); + kfree(cb_buff); +} +EXPORT_SYMBOL_GPL(iio_channel_release_all_cb); + +struct iio_channel +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer) +{ + return cb_buffer->channels; +} +EXPORT_SYMBOL_GPL(iio_channel_cb_get_channels); diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h index 57efee6..caf103b 100644 --- a/include/linux/iio/consumer.h +++ b/include/linux/iio/consumer.h @@ -61,6 +61,52 @@ struct iio_channel *iio_channel_get_all(const char *name); */ void iio_channel_release_all(struct iio_channel *chan); +struct iio_cb_buffer; +/** + * iio_channel_get_all_cb() - register callback for triggered capture + * @name: Name of client device. + * @cb: Callback function. + * @private: Private data passed to callback. + * + * NB right now we have no ability to mux data from multiple devices. + * So if the channels requested come from different devices this will + * fail. + */ +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, + int (*cb)(u8 *data, + void *private), + void *private); +/** + * iio_channel_release_all_cb() - release and unregister the callback. + * @cb_buffer: The callback buffer that was allocated. + */ +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buffer); + +/** + * iio_channel_start_all_cb() - start the flow of data through callback. + * @cb_buff: The callback buffer we are starting. + */ +int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff); + +/** + * iio_channel_stop_all_cb() - stop the flow of data through the callback. + * @cb_buff: The callback buffer we are stopping. + */ +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff); + +/** + * iio_channel_cb_get_channels() - get access to the underlying channels. + * @cb_buff: The callback buffers from whom we want the channel + * information. + * + * This function allows one to obtain information about the channels. + * Whilst this may allow direct reading if all buffers are disabled, the + * primary aim is to allow drivers that are consuming a channel to query + * things like scaling of the channel. + */ +struct iio_channel +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer); + /** * iio_read_channel_raw() - read from a given channel * @chan: The channel being queried. -- 1.7.12.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface 2012-10-13 9:24 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron @ 2012-10-17 8:50 ` Lars-Peter Clausen 2012-10-17 8:52 ` Jonathan Cameron 0 siblings, 1 reply; 18+ messages in thread From: Lars-Peter Clausen @ 2012-10-17 8:50 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio, Michael.Hennerich, srinivas.pandruvada On 10/13/2012 11:24 AM, Jonathan Cameron wrote: > This callback buffer is meant to be opaque to users, but basically > adds a very simple pass through buffer to which data may be > pushed when it is inserted into the buffer list. > > Signed-off-by: Jonathan Cameron <jic23@kernel.org> > --- > drivers/iio/Kconfig | 6 +++ > drivers/iio/Makefile | 1 + > drivers/iio/buffer_cb.c | 113 +++++++++++++++++++++++++++++++++++++++++++ > include/linux/iio/consumer.h | 46 ++++++++++++++++++ > 4 files changed, 166 insertions(+) > > diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig > index 6e3f143..0aad08a 100644 > --- a/drivers/iio/Kconfig > +++ b/drivers/iio/Kconfig > @@ -20,6 +20,12 @@ config IIO_BUFFER > > if IIO_BUFFER > > +config IIO_BUFFER_CB > + boolean "IIO callback buffer used for push in kernel interfaces" Any specific reason why this has to be bool? > + help > + Should be selected by any drivers that do inkernel push > + usage. That is, those where the data is pushed to the consumer. > + > config IIO_KFIFO_BUF > select IIO_TRIGGER > tristate "Industrial I/O buffering based on kfifo" > diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile > index f7fa3c0..a27ec1c 100644 > --- a/drivers/iio/Makefile > +++ b/drivers/iio/Makefile > @@ -6,6 +6,7 @@ obj-$(CONFIG_IIO) += industrialio.o > industrialio-y := industrialio-core.o industrialio-event.o inkern.o > industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o > industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o > +industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o > > obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o > obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o > diff --git a/drivers/iio/buffer_cb.c b/drivers/iio/buffer_cb.c > new file mode 100644 > index 0000000..d1e4881 > --- /dev/null > +++ b/drivers/iio/buffer_cb.c > @@ -0,0 +1,113 @@ > +#include <linux/kernel.h> > +#include <linux/slab.h> > +#include <linux/err.h> > +#include <linux/export.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/consumer.h> > + > +struct iio_cb_buffer { > + struct iio_buffer buffer; > + int (*cb)(u8 *data, void *private); > + void *private; > + struct iio_channel *channels; > +}; > + > +static int iio_buffer_cb_store_to(struct iio_buffer *buffer, u8 *data) > +{ > + struct iio_cb_buffer *cb_buff = container_of(buffer, > + struct iio_cb_buffer, > + buffer); > + > + return cb_buff->cb(data, cb_buff->private); > +} > + > +static struct iio_buffer_access_funcs iio_cb_access = { > + .store_to = &iio_buffer_cb_store_to, > +}; > + > +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, > + int (*cb)(u8 *data, > + void *private), > + void *private) > +{ > + int ret; > + struct iio_cb_buffer *cb_buff; > + struct iio_dev *indio_dev; > + struct iio_channel *chan; > + > + cb_buff = kzalloc(sizeof *cb_buff, GFP_KERNEL); sizeof(*cb_buff) > + if (cb_buff == NULL) { > + ret = -ENOMEM; > + goto error_ret; > + } > + > + cb_buff->private = private; > + cb_buff->cb = cb; > + cb_buff->buffer.access = &iio_cb_access; > + INIT_LIST_HEAD(&cb_buff->buffer.demux_list); It probably makes sense to have some kind of iio_buffer_init(...) for this > + > + cb_buff->channels = iio_channel_get_all(name); > + if (IS_ERR(cb_buff->channels)) { > + ret = PTR_ERR(cb_buff->channels); > + goto error_free_cb_buff; > + } > + > + indio_dev = cb_buff->channels[0].indio_dev; > + cb_buff->buffer.scan_mask > + = kzalloc(sizeof(long)*BITS_TO_LONGS(indio_dev->masklength), > + GFP_KERNEL); kcalloc > + if (cb_buff->buffer.scan_mask == NULL) { > + ret = -ENOMEM; > + goto error_release_channels; > + } > + chan = &cb_buff->channels[0]; > + while (chan->indio_dev) { > + if (chan->indio_dev != indio_dev) { > + ret = -EINVAL; > + goto error_release_channels; > + } > + set_bit(chan->channel->scan_index, > + cb_buff->buffer.scan_mask); > + chan++; > + } > + > + return cb_buff; > + > +error_release_channels: > + iio_channel_release_all(cb_buff->channels); > +error_free_cb_buff: > + kfree(cb_buff); > +error_ret: > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL_GPL(iio_channel_get_all_cb); > + > +int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff) > +{ > + return iio_update_buffers(cb_buff->channels[0].indio_dev, > + &cb_buff->buffer, > + NULL); > +} > +EXPORT_SYMBOL_GPL(iio_channel_start_all_cb); > + > +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff) > +{ > + iio_update_buffers(cb_buff->channels[0].indio_dev, > + NULL, > + &cb_buff->buffer); > +} > +EXPORT_SYMBOL_GPL(iio_channel_stop_all_cb); > + > +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buff) > +{ > + iio_channel_release_all(cb_buff->channels); > + kfree(cb_buff); > +} > +EXPORT_SYMBOL_GPL(iio_channel_release_all_cb); > + > +struct iio_channel > +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer) > +{ > + return cb_buffer->channels; > +} > +EXPORT_SYMBOL_GPL(iio_channel_cb_get_channels); > diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h > index 57efee6..caf103b 100644 > --- a/include/linux/iio/consumer.h > +++ b/include/linux/iio/consumer.h > @@ -61,6 +61,52 @@ struct iio_channel *iio_channel_get_all(const char *name); > */ > void iio_channel_release_all(struct iio_channel *chan); > > +struct iio_cb_buffer; > +/** > + * iio_channel_get_all_cb() - register callback for triggered capture > + * @name: Name of client device. > + * @cb: Callback function. > + * @private: Private data passed to callback. > + * > + * NB right now we have no ability to mux data from multiple devices. > + * So if the channels requested come from different devices this will > + * fail. > + */ > +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, > + int (*cb)(u8 *data, > + void *private), > + void *private); > +/** > + * iio_channel_release_all_cb() - release and unregister the callback. > + * @cb_buffer: The callback buffer that was allocated. > + */ > +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buffer); > + > +/** > + * iio_channel_start_all_cb() - start the flow of data through callback. > + * @cb_buff: The callback buffer we are starting. > + */ > +int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff); > + > +/** > + * iio_channel_stop_all_cb() - stop the flow of data through the callback. > + * @cb_buff: The callback buffer we are stopping. > + */ > +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff); > + > +/** > + * iio_channel_cb_get_channels() - get access to the underlying channels. > + * @cb_buff: The callback buffers from whom we want the channel > + * information. > + * > + * This function allows one to obtain information about the channels. > + * Whilst this may allow direct reading if all buffers are disabled, the > + * primary aim is to allow drivers that are consuming a channel to query > + * things like scaling of the channel. > + */ > +struct iio_channel > +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer); > + > /** > * iio_read_channel_raw() - read from a given channel > * @chan: The channel being queried. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface 2012-10-17 8:50 ` Lars-Peter Clausen @ 2012-10-17 8:52 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2012-10-17 8:52 UTC (permalink / raw) To: Lars-Peter Clausen Cc: Jonathan Cameron, linux-iio, Michael.Hennerich, srinivas.pandruvada On 17/10/12 09:50, Lars-Peter Clausen wrote: > On 10/13/2012 11:24 AM, Jonathan Cameron wrote: >> This callback buffer is meant to be opaque to users, but basically >> adds a very simple pass through buffer to which data may be >> pushed when it is inserted into the buffer list. >> >> Signed-off-by: Jonathan Cameron <jic23@kernel.org> >> --- >> drivers/iio/Kconfig | 6 +++ >> drivers/iio/Makefile | 1 + >> drivers/iio/buffer_cb.c | 113 +++++++++++++++++++++++++++++++++++++++++++ >> include/linux/iio/consumer.h | 46 ++++++++++++++++++ >> 4 files changed, 166 insertions(+) >> >> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig >> index 6e3f143..0aad08a 100644 >> --- a/drivers/iio/Kconfig >> +++ b/drivers/iio/Kconfig >> @@ -20,6 +20,12 @@ config IIO_BUFFER >> >> if IIO_BUFFER >> >> +config IIO_BUFFER_CB >> + boolean "IIO callback buffer used for push in kernel interfaces" > > Any specific reason why this has to be bool? Doesn't have it's own module as it's so small. Gets built in to the core. Agreed to all the rest. Thanks for looking at these. > >> + help >> + Should be selected by any drivers that do inkernel push >> + usage. That is, those where the data is pushed to the consumer. >> + >> config IIO_KFIFO_BUF >> select IIO_TRIGGER >> tristate "Industrial I/O buffering based on kfifo" >> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile >> index f7fa3c0..a27ec1c 100644 >> --- a/drivers/iio/Makefile >> +++ b/drivers/iio/Makefile >> @@ -6,6 +6,7 @@ obj-$(CONFIG_IIO) += industrialio.o >> industrialio-y := industrialio-core.o industrialio-event.o inkern.o >> industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o >> industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o >> +industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o >> >> obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o >> obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o >> diff --git a/drivers/iio/buffer_cb.c b/drivers/iio/buffer_cb.c >> new file mode 100644 >> index 0000000..d1e4881 >> --- /dev/null >> +++ b/drivers/iio/buffer_cb.c >> @@ -0,0 +1,113 @@ >> +#include <linux/kernel.h> >> +#include <linux/slab.h> >> +#include <linux/err.h> >> +#include <linux/export.h> >> +#include <linux/iio/buffer.h> >> +#include <linux/iio/consumer.h> >> + >> +struct iio_cb_buffer { >> + struct iio_buffer buffer; >> + int (*cb)(u8 *data, void *private); >> + void *private; >> + struct iio_channel *channels; >> +}; >> + >> +static int iio_buffer_cb_store_to(struct iio_buffer *buffer, u8 *data) >> +{ >> + struct iio_cb_buffer *cb_buff = container_of(buffer, >> + struct iio_cb_buffer, >> + buffer); >> + >> + return cb_buff->cb(data, cb_buff->private); >> +} >> + >> +static struct iio_buffer_access_funcs iio_cb_access = { >> + .store_to = &iio_buffer_cb_store_to, >> +}; >> + >> +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, >> + int (*cb)(u8 *data, >> + void *private), >> + void *private) >> +{ >> + int ret; >> + struct iio_cb_buffer *cb_buff; >> + struct iio_dev *indio_dev; >> + struct iio_channel *chan; >> + >> + cb_buff = kzalloc(sizeof *cb_buff, GFP_KERNEL); > > sizeof(*cb_buff) > >> + if (cb_buff == NULL) { >> + ret = -ENOMEM; >> + goto error_ret; >> + } >> + >> + cb_buff->private = private; >> + cb_buff->cb = cb; >> + cb_buff->buffer.access = &iio_cb_access; >> + INIT_LIST_HEAD(&cb_buff->buffer.demux_list); > > It probably makes sense to have some kind of iio_buffer_init(...) for this > >> + >> + cb_buff->channels = iio_channel_get_all(name); >> + if (IS_ERR(cb_buff->channels)) { >> + ret = PTR_ERR(cb_buff->channels); >> + goto error_free_cb_buff; >> + } >> + >> + indio_dev = cb_buff->channels[0].indio_dev; >> + cb_buff->buffer.scan_mask >> + = kzalloc(sizeof(long)*BITS_TO_LONGS(indio_dev->masklength), >> + GFP_KERNEL); > > kcalloc > >> + if (cb_buff->buffer.scan_mask == NULL) { >> + ret = -ENOMEM; >> + goto error_release_channels; >> + } >> + chan = &cb_buff->channels[0]; >> + while (chan->indio_dev) { >> + if (chan->indio_dev != indio_dev) { >> + ret = -EINVAL; >> + goto error_release_channels; >> + } >> + set_bit(chan->channel->scan_index, >> + cb_buff->buffer.scan_mask); >> + chan++; >> + } >> + >> + return cb_buff; >> + >> +error_release_channels: >> + iio_channel_release_all(cb_buff->channels); >> +error_free_cb_buff: >> + kfree(cb_buff); >> +error_ret: >> + return ERR_PTR(ret); >> +} >> +EXPORT_SYMBOL_GPL(iio_channel_get_all_cb); >> + >> +int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff) >> +{ >> + return iio_update_buffers(cb_buff->channels[0].indio_dev, >> + &cb_buff->buffer, >> + NULL); >> +} >> +EXPORT_SYMBOL_GPL(iio_channel_start_all_cb); >> + >> +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff) >> +{ >> + iio_update_buffers(cb_buff->channels[0].indio_dev, >> + NULL, >> + &cb_buff->buffer); >> +} >> +EXPORT_SYMBOL_GPL(iio_channel_stop_all_cb); >> + >> +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buff) >> +{ >> + iio_channel_release_all(cb_buff->channels); >> + kfree(cb_buff); >> +} >> +EXPORT_SYMBOL_GPL(iio_channel_release_all_cb); >> + >> +struct iio_channel >> +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer) >> +{ >> + return cb_buffer->channels; >> +} >> +EXPORT_SYMBOL_GPL(iio_channel_cb_get_channels); >> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h >> index 57efee6..caf103b 100644 >> --- a/include/linux/iio/consumer.h >> +++ b/include/linux/iio/consumer.h >> @@ -61,6 +61,52 @@ struct iio_channel *iio_channel_get_all(const char *name); >> */ >> void iio_channel_release_all(struct iio_channel *chan); >> >> +struct iio_cb_buffer; >> +/** >> + * iio_channel_get_all_cb() - register callback for triggered capture >> + * @name: Name of client device. >> + * @cb: Callback function. >> + * @private: Private data passed to callback. >> + * >> + * NB right now we have no ability to mux data from multiple devices. >> + * So if the channels requested come from different devices this will >> + * fail. >> + */ >> +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, >> + int (*cb)(u8 *data, >> + void *private), >> + void *private); >> +/** >> + * iio_channel_release_all_cb() - release and unregister the callback. >> + * @cb_buffer: The callback buffer that was allocated. >> + */ >> +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buffer); >> + >> +/** >> + * iio_channel_start_all_cb() - start the flow of data through callback. >> + * @cb_buff: The callback buffer we are starting. >> + */ >> +int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff); >> + >> +/** >> + * iio_channel_stop_all_cb() - stop the flow of data through the callback. >> + * @cb_buff: The callback buffer we are stopping. >> + */ >> +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff); >> + >> +/** >> + * iio_channel_cb_get_channels() - get access to the underlying channels. >> + * @cb_buff: The callback buffers from whom we want the channel >> + * information. >> + * >> + * This function allows one to obtain information about the channels. >> + * Whilst this may allow direct reading if all buffers are disabled, the >> + * primary aim is to allow drivers that are consuming a channel to query >> + * things like scaling of the channel. >> + */ >> +struct iio_channel >> +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer); >> + >> /** >> * iio_read_channel_raw() - read from a given channel >> * @chan: The channel being queried. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/4 V6] staging:iio: Add support for multiple buffers (testing required!)
@ 2012-10-31 10:30 Jonathan Cameron
2012-10-31 10:30 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron
0 siblings, 1 reply; 18+ messages in thread
From: Jonathan Cameron @ 2012-10-31 10:30 UTC (permalink / raw)
To: linux-iio; +Cc: Jonathan Cameron
Same as V5 but with the changes from Lars-Peter's review to close some
memory leaks and catch up with some changes in the subsystem that had
passed this by. It should also now be possible to disable the
timestamp! (oops).
Thanks to Lars for his work reviewing my patches and also for his
excellent work reviewing in general! Without him the whole subsystem
would be moving a lot slower.
Again, more testing would be good (thanks for those who already
have).
In quick summary this is needed to do interrupt driven in kernel
clients as it adds the option to push to multiple 'buffer' interfaces.
Note that some of these may not actually do any buffering but rather
push directly into another subsytem (the example is input).
I would like to merge at least the first 3 patches early in this cycle.
What I need is some testing across a reasonable range of devices. Review
is welcome of course as well!
The complexity in this set is entirely in the changes to industrialio-buffer.c
concerning how we go about adding and removing client buffers. To do this
we need to work out which scan mask is appropriate given the requests of
all the given buffers (and whether we can satisfy it). I've been testing
with the max1363 driver which probably has the most complex set of available
scan masks. Thus when a new buffer is added or one is removed the whole
buffer set are torn down and built up again. This may be undesirable for
some applications but should occur only rarely.
The example driver is for bridging to the input subsystem and still needs
some work, but that can occur later in this merge cycle whereas the changes
to the core really want to go in asap.
Changes since v3
Rebased and added other drivers that have occured whilst this patch series
has been languishing in my local tree.
Changes since V2.
Improved handling of error conditions in the buffer insertion and
removal code. (suggested by Lars-Peter). I want to take another
look at these, but didn't want to keep people from testing this
in the meantime.
Also a number of minor fixes that came up during testing.
Thanks,
Jonathan
Jonathan Cameron (4):
staging:iio: Add support for multiple buffers
staging:iio:in kernel users: Add a data field for channel specific
info.
staging:iio: add a callback buffer for in kernel push interface
staging:iio: Proof of concept input driver.
drivers/iio/Kconfig | 6 +
drivers/iio/Makefile | 1 +
drivers/iio/accel/hid-sensor-accel-3d.c | 15 +-
drivers/iio/adc/ad7266.c | 3 +-
drivers/iio/adc/ad7476.c | 2 +-
drivers/iio/adc/ad_sigma_delta.c | 2 +-
drivers/iio/adc/at91_adc.c | 3 +-
drivers/iio/buffer_cb.c | 113 +++++++
drivers/iio/gyro/hid-sensor-gyro-3d.c | 15 +-
drivers/iio/industrialio-buffer.c | 380 ++++++++++++++++--------
drivers/iio/industrialio-core.c | 1 +
drivers/iio/inkern.c | 1 +
drivers/iio/light/adjd_s311.c | 3 +-
drivers/iio/light/hid-sensor-als.c | 15 +-
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 15 +-
drivers/staging/iio/Kconfig | 11 +
drivers/staging/iio/Makefile | 1 +
drivers/staging/iio/accel/adis16201_ring.c | 2 +-
drivers/staging/iio/accel/adis16203_ring.c | 2 +-
drivers/staging/iio/accel/adis16204_ring.c | 2 +-
drivers/staging/iio/accel/adis16209_ring.c | 2 +-
drivers/staging/iio/accel/adis16240_ring.c | 2 +-
drivers/staging/iio/accel/lis3l02dq_ring.c | 2 +-
drivers/staging/iio/adc/ad7298_ring.c | 2 +-
drivers/staging/iio/adc/ad7606_ring.c | 2 +-
drivers/staging/iio/adc/ad7887_ring.c | 2 +-
drivers/staging/iio/adc/ad799x_ring.c | 2 +-
drivers/staging/iio/adc/max1363_ring.c | 2 +-
drivers/staging/iio/gyro/adis16260_ring.c | 2 +-
drivers/staging/iio/iio_input.c | 225 ++++++++++++++
drivers/staging/iio/iio_input.h | 23 ++
drivers/staging/iio/iio_simple_dummy_buffer.c | 5 +-
drivers/staging/iio/impedance-analyzer/ad5933.c | 4 +-
drivers/staging/iio/imu/adis16400_ring.c | 5 +-
drivers/staging/iio/meter/ade7758_ring.c | 2 +-
include/linux/iio/buffer.h | 24 +-
include/linux/iio/consumer.h | 48 +++
include/linux/iio/iio.h | 2 +
include/linux/iio/machine.h | 2 +
39 files changed, 743 insertions(+), 208 deletions(-)
create mode 100644 drivers/iio/buffer_cb.c
create mode 100644 drivers/staging/iio/iio_input.c
create mode 100644 drivers/staging/iio/iio_input.h
--
1.7.12.4
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface 2012-10-31 10:30 [PATCH 0/4 V6] staging:iio: Add support for multiple buffers (testing required!) Jonathan Cameron @ 2012-10-31 10:30 ` Jonathan Cameron 2012-10-31 14:10 ` Peter Meerwald 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Cameron @ 2012-10-31 10:30 UTC (permalink / raw) To: linux-iio; +Cc: Jonathan Cameron This callback buffer is meant to be opaque to users, but basically adds a very simple pass through buffer to which data may be pushed when it is inserted into the buffer list. Signed-off-by: Jonathan Cameron <jic23@kernel.org> --- drivers/iio/Kconfig | 6 +++ drivers/iio/Makefile | 1 + drivers/iio/buffer_cb.c | 113 +++++++++++++++++++++++++++++++++++++++++++ include/linux/iio/consumer.h | 46 ++++++++++++++++++ 4 files changed, 166 insertions(+) diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig index fc937ac..f57f725 100644 --- a/drivers/iio/Kconfig +++ b/drivers/iio/Kconfig @@ -20,6 +20,12 @@ config IIO_BUFFER if IIO_BUFFER +config IIO_BUFFER_CB + boolean "IIO callback buffer used for push in kernel interfaces" + help + Should be selected by any drivers that do inkernel push + usage. That is, those where the data is pushed to the consumer. + config IIO_KFIFO_BUF select IIO_TRIGGER tristate "Industrial I/O buffering based on kfifo" diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile index 761f2b6..31d76a0 100644 --- a/drivers/iio/Makefile +++ b/drivers/iio/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_IIO) += industrialio.o industrialio-y := industrialio-core.o industrialio-event.o inkern.o industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o +industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o diff --git a/drivers/iio/buffer_cb.c b/drivers/iio/buffer_cb.c new file mode 100644 index 0000000..4d40e24 --- /dev/null +++ b/drivers/iio/buffer_cb.c @@ -0,0 +1,113 @@ +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/err.h> +#include <linux/export.h> +#include <linux/iio/buffer.h> +#include <linux/iio/consumer.h> + +struct iio_cb_buffer { + struct iio_buffer buffer; + int (*cb)(u8 *data, void *private); + void *private; + struct iio_channel *channels; +}; + +static int iio_buffer_cb_store_to(struct iio_buffer *buffer, u8 *data) +{ + struct iio_cb_buffer *cb_buff = container_of(buffer, + struct iio_cb_buffer, + buffer); + + return cb_buff->cb(data, cb_buff->private); +} + +static struct iio_buffer_access_funcs iio_cb_access = { + .store_to = &iio_buffer_cb_store_to, +}; + +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, + int (*cb)(u8 *data, + void *private), + void *private) +{ + int ret; + struct iio_cb_buffer *cb_buff; + struct iio_dev *indio_dev; + struct iio_channel *chan; + + cb_buff = kzalloc(sizeof(*cb_buff), GFP_KERNEL); + if (cb_buff == NULL) { + ret = -ENOMEM; + goto error_ret; + } + + cb_buff->private = private; + cb_buff->cb = cb; + cb_buff->buffer.access = &iio_cb_access; + INIT_LIST_HEAD(&cb_buff->buffer.demux_list); + + cb_buff->channels = iio_channel_get_all(name); + if (IS_ERR(cb_buff->channels)) { + ret = PTR_ERR(cb_buff->channels); + goto error_free_cb_buff; + } + + indio_dev = cb_buff->channels[0].indio_dev; + cb_buff->buffer.scan_mask + = kcalloc(BITS_TO_LONGS(indio_dev->masklength), sizeof(long), + GFP_KERNEL); + if (cb_buff->buffer.scan_mask == NULL) { + ret = -ENOMEM; + goto error_release_channels; + } + chan = &cb_buff->channels[0]; + while (chan->indio_dev) { + if (chan->indio_dev != indio_dev) { + ret = -EINVAL; + goto error_release_channels; + } + set_bit(chan->channel->scan_index, + cb_buff->buffer.scan_mask); + chan++; + } + + return cb_buff; + +error_release_channels: + iio_channel_release_all(cb_buff->channels); +error_free_cb_buff: + kfree(cb_buff); +error_ret: + return ERR_PTR(ret); +} +EXPORT_SYMBOL_GPL(iio_channel_get_all_cb); + +int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff) +{ + return iio_update_buffers(cb_buff->channels[0].indio_dev, + &cb_buff->buffer, + NULL); +} +EXPORT_SYMBOL_GPL(iio_channel_start_all_cb); + +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff) +{ + iio_update_buffers(cb_buff->channels[0].indio_dev, + NULL, + &cb_buff->buffer); +} +EXPORT_SYMBOL_GPL(iio_channel_stop_all_cb); + +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buff) +{ + iio_channel_release_all(cb_buff->channels); + kfree(cb_buff); +} +EXPORT_SYMBOL_GPL(iio_channel_release_all_cb); + +struct iio_channel +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer) +{ + return cb_buffer->channels; +} +EXPORT_SYMBOL_GPL(iio_channel_cb_get_channels); diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h index 57efee6..caf103b 100644 --- a/include/linux/iio/consumer.h +++ b/include/linux/iio/consumer.h @@ -61,6 +61,52 @@ struct iio_channel *iio_channel_get_all(const char *name); */ void iio_channel_release_all(struct iio_channel *chan); +struct iio_cb_buffer; +/** + * iio_channel_get_all_cb() - register callback for triggered capture + * @name: Name of client device. + * @cb: Callback function. + * @private: Private data passed to callback. + * + * NB right now we have no ability to mux data from multiple devices. + * So if the channels requested come from different devices this will + * fail. + */ +struct iio_cb_buffer *iio_channel_get_all_cb(const char *name, + int (*cb)(u8 *data, + void *private), + void *private); +/** + * iio_channel_release_all_cb() - release and unregister the callback. + * @cb_buffer: The callback buffer that was allocated. + */ +void iio_channel_release_all_cb(struct iio_cb_buffer *cb_buffer); + +/** + * iio_channel_start_all_cb() - start the flow of data through callback. + * @cb_buff: The callback buffer we are starting. + */ +int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff); + +/** + * iio_channel_stop_all_cb() - stop the flow of data through the callback. + * @cb_buff: The callback buffer we are stopping. + */ +void iio_channel_stop_all_cb(struct iio_cb_buffer *cb_buff); + +/** + * iio_channel_cb_get_channels() - get access to the underlying channels. + * @cb_buff: The callback buffers from whom we want the channel + * information. + * + * This function allows one to obtain information about the channels. + * Whilst this may allow direct reading if all buffers are disabled, the + * primary aim is to allow drivers that are consuming a channel to query + * things like scaling of the channel. + */ +struct iio_channel +*iio_channel_cb_get_channels(const struct iio_cb_buffer *cb_buffer); + /** * iio_read_channel_raw() - read from a given channel * @chan: The channel being queried. -- 1.7.12.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface 2012-10-31 10:30 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron @ 2012-10-31 14:10 ` Peter Meerwald 2012-11-02 10:53 ` Jonathan Cameron 0 siblings, 1 reply; 18+ messages in thread From: Peter Meerwald @ 2012-10-31 14:10 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio Hi, I'm sure you missed my spellchecking :) > +config IIO_BUFFER_CB > + boolean "IIO callback buffer used for push in kernel interfaces" > + help > + Should be selected by any drivers that do inkernel push > + usage. That is, those where the data is pushed to the consumer. > + is it 'in kernel' or 'inkernel'? I'd prefer in-kernel :) indentation seems inconsistent > +/** > + * iio_channel_cb_get_channels() - get access to the underlying channels. > + * @cb_buff: The callback buffers from whom we want the channel > + * information. buffers from which? -- Peter Meerwald +43-664-2444418 (mobile) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface 2012-10-31 14:10 ` Peter Meerwald @ 2012-11-02 10:53 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2012-11-02 10:53 UTC (permalink / raw) To: Peter Meerwald; +Cc: linux-iio On 10/31/2012 02:10 PM, Peter Meerwald wrote: > Hi, > > I'm sure you missed my spellchecking :) I am always happy to accept fixes like this that mean I don't look as much of an idiot when patches hit mainline ;) > >> +config IIO_BUFFER_CB >> + boolean "IIO callback buffer used for push in kernel interfaces" >> + help >> + Should be selected by any drivers that do inkernel push >> + usage. That is, those where the data is pushed to the consumer. >> + > > is it 'in kernel' or 'inkernel'? I'd prefer in-kernel :) > indentation seems inconsistent in-kernel is indeed nicer. I'll do that here and maybe clean up the other uses later. > >> +/** >> + * iio_channel_cb_get_channels() - get access to the underlying channels. >> + * @cb_buff: The callback buffers from whom we want the channel >> + * information. > > buffers from which? actually should be 'buffer from whom'. Good catch though - I could have read that 100 times and never noticed. I'm lazy so won't repost the patch with these changes but will wait a little longer then merge it if no one shouts loudly that I shouldn't. (they'll be in the wip branch if anyone actually cares enough to look) ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-11-02 10:53 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-30 19:36 [PATCH 0/4 V2] staging:iio: Add support for multiple buffers Jonathan Cameron 2012-05-30 19:36 ` [PATCH 1/4] " Jonathan Cameron 2012-06-08 15:13 ` Lars-Peter Clausen 2012-06-09 12:17 ` Jonathan Cameron 2012-06-09 11:50 ` Lars-Peter Clausen 2012-06-09 17:56 ` Jonathan Cameron 2012-05-30 19:36 ` [PATCH 2/4] staging:iio:in kernel users: Add a data field for channel specific info Jonathan Cameron 2012-05-30 19:36 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron 2012-05-30 19:36 ` [PATCH 4/4] staging:iio: Proof of concept input driver Jonathan Cameron 2012-06-08 15:27 ` Lars-Peter Clausen 2012-06-09 17:56 ` Jonathan Cameron -- strict thread matches above, loose matches on Subject: below -- 2012-06-30 19:06 [PATCH 0/4 V3] staging:iio: Add support for multiple buffers Jonathan Cameron 2012-06-30 19:06 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron 2012-10-13 9:24 [PATCH 0/4 V5] staging:iio: Add support for multiple buffers (testing required!) Jonathan Cameron 2012-10-13 9:24 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron 2012-10-17 8:50 ` Lars-Peter Clausen 2012-10-17 8:52 ` Jonathan Cameron 2012-10-31 10:30 [PATCH 0/4 V6] staging:iio: Add support for multiple buffers (testing required!) Jonathan Cameron 2012-10-31 10:30 ` [PATCH 3/4] staging:iio: add a callback buffer for in kernel push interface Jonathan Cameron 2012-10-31 14:10 ` Peter Meerwald 2012-11-02 10:53 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).