On Sun, 2020-04-05 at 10:56 +0100, Jonathan Cameron wrote:
> [External]
>
> On Mon, 30 Mar 2020 17:57:03 +0300
> Alexandru Ardelean <ardeleanalex@xxxxxxxxx> wrote:
>
> > Since we want to add support for attaching multiple buffers, and we want to
> > add a new list to 'struct iio_dev', it's a good idea to rename the current
> > 'indio->buffer_list' to 'indio_dev->active_buffers'.
> >
> > Fortunately, this is a private field, which is used in
> > 'drivers/iio/industrial-buffer.c', so this is simple to rename.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> This highlights that we are going to need to come up with a clearly concept
> than
> 'buffers' for both the stream of data coming in or out + the interfaces
> within the kernel (consumer buffer etc).
>
> I'm not sure what naming will work though.. For input buffers we could
> use "buffers" for the incoming streams and "consumers" for where that is
> demuxed
> to (including the kfifos etc).
>
> For output I 'think' we are unlikely to have to deal with interleaving
> inputs from multiple devices all wanting to drive their own channels?
> (I hope so as that sounds horrible to deal with give frequency missmatch
> etc) So in output case it'll kind of be one block.
Hmm, I'm still vague about this.
And trying to workout how things already work, to be able to find a route to the
multiple-buffers stuff.
This patch was an initial thought [I had] about how to go directly for multiple
buffers, and one idea was to have 'indio_dev->attached_buffers' which would
replace 'indio_dev->buffer'.
Then a new field called 'iio_buffer->direction' would be added to determine
direction.
In any case, it won't be at the start of the series, since I can't really
motivate this better than "it helps me grep easier for 'indio_dev->buffer' &
'indio_dev->buffer_list'".
>
> Jonathan
>
> > ---
> > drivers/iio/industrialio-buffer.c | 28 ++++++++++++++--------------
> > drivers/iio/industrialio-core.c | 2 +-
> > include/linux/iio/iio.h | 4 ++--
> > 3 files changed, 17 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-
> > buffer.c
> > index e6fa1a4e135d..a585c304cad4 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -591,7 +591,7 @@ static void iio_buffer_activate(struct iio_dev
> > *indio_dev,
> > struct iio_buffer *buffer)
> > {
> > iio_buffer_get(buffer);
> > - list_add(&buffer->buffer_list, &indio_dev->buffer_list);
> > + list_add(&buffer->buffer_list, &indio_dev->active_buffers);
> > }
> >
> > static void iio_buffer_deactivate(struct iio_buffer *buffer)
> > @@ -606,7 +606,7 @@ static void iio_buffer_deactivate_all(struct iio_dev
> > *indio_dev)
> > struct iio_buffer *buffer, *_buffer;
> >
> > list_for_each_entry_safe(buffer, _buffer,
> > - &indio_dev->buffer_list, buffer_list)
> > + &indio_dev->active_buffers, buffer_list)
> > iio_buffer_deactivate(buffer);
> > }
> >
> > @@ -701,12 +701,12 @@ static int iio_verify_update(struct iio_dev
> > *indio_dev,
> > * to verify.
> > */
> > if (remove_buffer && !insert_buffer &&
> > - list_is_singular(&indio_dev->buffer_list))
> > + list_is_singular(&indio_dev->active_buffers))
> > return 0;
> >
> > modes = indio_dev->modes;
> >
> > - list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) {
> > + list_for_each_entry(buffer, &indio_dev->active_buffers, buffer_list) {
> > if (buffer == remove_buffer)
> > continue;
> > modes &= buffer->access->modes;
> > @@ -727,7 +727,7 @@ static int iio_verify_update(struct iio_dev *indio_dev,
> > * Keep things simple for now and only allow a single buffer to
> > * be connected in hardware mode.
> > */
> > - if (insert_buffer && !list_empty(&indio_dev->buffer_list))
> > + if (insert_buffer && !list_empty(&indio_dev->active_buffers))
> > return -EINVAL;
> > config->mode = INDIO_BUFFER_HARDWARE;
> > strict_scanmask = true;
> > @@ -747,7 +747,7 @@ static int iio_verify_update(struct iio_dev *indio_dev,
> >
> > scan_timestamp = false;
> >
> > - list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) {
> > + list_for_each_entry(buffer, &indio_dev->active_buffers, buffer_list) {
> > if (buffer == remove_buffer)
> > continue;
> > bitmap_or(compound_mask, compound_mask, buffer->scan_mask,
> > @@ -896,7 +896,7 @@ static 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) {
> > + list_for_each_entry(buffer, &indio_dev->active_buffers, buffer_list) {
> > ret = iio_buffer_update_demux(indio_dev, buffer);
> > if (ret < 0)
> > goto error_clear_mux_table;
> > @@ -904,7 +904,7 @@ static int iio_update_demux(struct iio_dev *indio_dev)
> > return 0;
> >
> > error_clear_mux_table:
> > - list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list)
> > + list_for_each_entry(buffer, &indio_dev->active_buffers, buffer_list)
> > iio_buffer_demux_free(buffer);
> >
> > return ret;
> > @@ -948,7 +948,7 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
> > indio_dev->info->hwfifo_set_watermark(indio_dev,
> > config->watermark);
> >
> > - list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) {
> > + list_for_each_entry(buffer, &indio_dev->active_buffers, buffer_list) {
> > ret = iio_buffer_enable(buffer, indio_dev);
> > if (ret)
> > goto err_disable_buffers;
> > @@ -968,7 +968,7 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
> > return 0;
> >
> > err_disable_buffers:
> > - list_for_each_entry_continue_reverse(buffer, &indio_dev->buffer_list,
> > + list_for_each_entry_continue_reverse(buffer, &indio_dev->active_buffers,
> > buffer_list)
> > iio_buffer_disable(buffer, indio_dev);
> > err_run_postdisable:
> > @@ -988,7 +988,7 @@ static int iio_disable_buffers(struct iio_dev
> > *indio_dev)
> > int ret2;
> >
> > /* Wind down existing buffers - iff there are any */
> > - if (list_empty(&indio_dev->buffer_list))
> > + if (list_empty(&indio_dev->active_buffers))
> > return 0;
> >
> > /*
> > @@ -1004,7 +1004,7 @@ static int iio_disable_buffers(struct iio_dev
> > *indio_dev)
> > ret = ret2;
> > }
> >
> > - list_for_each_entry(buffer, &indio_dev->buffer_list, buffer_list) {
> > + list_for_each_entry(buffer, &indio_dev->active_buffers, buffer_list) {
> > ret2 = iio_buffer_disable(buffer, indio_dev);
> > if (ret2 && !ret)
> > ret = ret2;
> > @@ -1052,7 +1052,7 @@ static int __iio_update_buffers(struct iio_dev
> > *indio_dev,
> > iio_buffer_activate(indio_dev, insert_buffer);
> >
> > /* If no buffers in list, we are done */
> > - if (list_empty(&indio_dev->buffer_list))
> > + if (list_empty(&indio_dev->active_buffers))
> > return 0;
> >
> > ret = iio_enable_buffers(indio_dev, &new_config);
> > @@ -1413,7 +1413,7 @@ int iio_push_to_buffers(struct iio_dev *indio_dev,
> > const void *data)
> > int ret;
> > struct iio_buffer *buf;
> >
> > - list_for_each_entry(buf, &indio_dev->buffer_list, buffer_list) {
> > + list_for_each_entry(buf, &indio_dev->active_buffers, buffer_list) {
> > ret = iio_push_to_buffer(buf, data);
> > if (ret < 0)
> > return ret;
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-
> > core.c
> > index 157d95a24faa..a13957644c1d 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -1523,7 +1523,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);
> > + INIT_LIST_HEAD(&dev->active_buffers);
> > }
> >
> > return dev;
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index e975020abaa6..a123f8acb192 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -490,7 +490,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
> > + * @active_buffers: [INTERN] list of all buffers currently
> > enabled/active
> > * @scan_bytes: [INTERN] num bytes captured to be fed to buffer
> > demux
> > * @mlock: [INTERN] lock used to prevent simultaneous device state
> > * changes
> > @@ -534,7 +534,7 @@ struct iio_dev {
> > struct iio_event_interface *event_interface;
> >
> > struct iio_buffer *buffer;
> > - struct list_head buffer_list;
> > + struct list_head active_buffers;
> > int scan_bytes;
> > struct mutex mlock;
> >
