From: Alexandru Ardelean <ardeleanalex@gmail.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: "Alexandru Ardelean" <alexandru.ardelean@analog.com>,
LKML <linux-kernel@vger.kernel.org>,
linux-iio <linux-iio@vger.kernel.org>,
"Lars-Peter Clausen" <lars@metafoo.de>,
"Hennerich, Michael" <Michael.Hennerich@analog.com>,
"Jonathan Cameron" <jic23@kernel.org>,
"Nuno Sá" <nuno.sa@analog.com>,
"Bogdan, Dragos" <dragos.bogdan@analog.com>,
"Rafael J. Wysocki" <rafael@kernel.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Subject: Re: [PATCH v3 07/11] iio: add reference to iio buffer on iio_dev_attr
Date: Fri, 5 Feb 2021 10:26:32 +0200 [thread overview]
Message-ID: <CA+U=Dsq34ixRT71qD8ZTR7MQ7NDgDZV2EJ+3gT=QQ8wBz4GiGA@mail.gmail.com> (raw)
In-Reply-To: <20210204180926.00005e4c@Huawei.com>
On Thu, Feb 4, 2021 at 8:16 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Mon, 1 Feb 2021 16:51:01 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > This change adds a reference to a 'struct iio_buffer' object on the
> > iio_dev_attr object. This way, we can use the created iio_dev_attr objects
> > on per-buffer basis (since they're allocated anyway).
> >
> > A minor downside of this change is that the number of parameters on
> > __iio_add_chan_devattr() grows by 1. This looks like it could do with a bit
> > of a re-think.
>
> Could use a bit of macro magic or static inline to keep the old
> version set of parameter and have __iio_add_chan_devattr_with_buf
> or similar. I'm not sure I'd bother given we don't have that many callers.
If there isn't a strong opinion to add the
__iio_add_chan_devattr_with_buf() version + macro/inline magic, I'd
probably not do it.
In terms of patch-noise, it's not considerably better, and not
necessarily worse.
When I was thinking about the re-think of __iio_add_chan_devattr, I
was thinking of a way to maybe re-architect this.
Maybe, add some {device_}attribute that act as template [at least for
the R/W functions].
This definitely needs some thinking to make it clean and nice.
>
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > drivers/iio/iio_core.h | 2 ++
> > drivers/iio/industrialio-buffer.c | 4 ++++
> > drivers/iio/industrialio-core.c | 6 ++++++
> > drivers/iio/industrialio-event.c | 1 +
> > include/linux/iio/sysfs.h | 3 +++
> > 5 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> > index 7d5b179c1fe7..731f5170d5b9 100644
> > --- a/drivers/iio/iio_core.h
> > +++ b/drivers/iio/iio_core.h
> > @@ -12,6 +12,7 @@
> > #include <linux/kernel.h>
> > #include <linux/device.h>
> >
> > +struct iio_buffer;
> > struct iio_chan_spec;
> > struct iio_dev;
> >
> > @@ -43,6 +44,7 @@ int __iio_add_chan_devattr(const char *postfix,
> > u64 mask,
> > enum iio_shared_by shared_by,
> > struct device *dev,
> > + struct iio_buffer *buffer,
> > struct list_head *attr_list);
> > void iio_free_chan_devattr_list(struct list_head *attr_list);
> >
> > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> > index f82decf92b7c..a525e88b302f 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
>
> > @@ -447,6 +447,7 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
> > 0,
> > IIO_SEPARATE,
> > &indio_dev->dev,
> > + buffer,
> > &buffer->scan_el_dev_attr_list);
> > if (ret)
> > return ret;
> > @@ -458,6 +459,7 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
> > 0,
> > 0,
> > &indio_dev->dev,
> > + buffer,
> > &buffer->scan_el_dev_attr_list);
> > if (ret)
> > return ret;
> > @@ -470,6 +472,7 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
> > chan->scan_index,
> > 0,
> > &indio_dev->dev,
> > + buffer,
> > &buffer->scan_el_dev_attr_list);
> > else
> > ret = __iio_add_chan_devattr("en",
> > @@ -479,6 +482,7 @@ static int iio_buffer_add_channel_sysfs(struct iio_dev *indio_dev,
> > chan->scan_index,
> > 0,
> > &indio_dev->dev,
> > + buffer,
> > &buffer->scan_el_dev_attr_list);
> > if (ret)
> > return ret;
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index ccd7aaff6d13..c68130885d83 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -1114,6 +1114,7 @@ int __iio_add_chan_devattr(const char *postfix,
> > u64 mask,
> > enum iio_shared_by shared_by,
> > struct device *dev,
> > + struct iio_buffer *buffer,
> > struct list_head *attr_list)
> > {
> > int ret;
> > @@ -1129,6 +1130,7 @@ int __iio_add_chan_devattr(const char *postfix,
> > goto error_iio_dev_attr_free;
> > iio_attr->c = chan;
> > iio_attr->address = mask;
> > + iio_attr->buffer = buffer;
> > list_for_each_entry(t, attr_list, l)
> > if (strcmp(t->dev_attr.attr.name,
> > iio_attr->dev_attr.attr.name) == 0) {
> > @@ -1165,6 +1167,7 @@ static int iio_device_add_channel_label(struct iio_dev *indio_dev,
> > 0,
> > IIO_SEPARATE,
> > &indio_dev->dev,
> > + NULL,
> > &iio_dev_opaque->channel_attr_list);
> > if (ret < 0)
> > return ret;
> > @@ -1190,6 +1193,7 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
> > i,
> > shared_by,
> > &indio_dev->dev,
> > + NULL,
> > &iio_dev_opaque->channel_attr_list);
> > if ((ret == -EBUSY) && (shared_by != IIO_SEPARATE))
> > continue;
> > @@ -1226,6 +1230,7 @@ static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
> > i,
> > shared_by,
> > &indio_dev->dev,
> > + NULL,
> > &iio_dev_opaque->channel_attr_list);
> > kfree(avail_postfix);
> > if ((ret == -EBUSY) && (shared_by != IIO_SEPARATE))
> > @@ -1322,6 +1327,7 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> > i,
> > ext_info->shared,
> > &indio_dev->dev,
> > + NULL,
> > &iio_dev_opaque->channel_attr_list);
> > i++;
> > if (ret == -EBUSY && ext_info->shared)
> > diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
> > index ea8947cc21e4..a30e289fc362 100644
> > --- a/drivers/iio/industrialio-event.c
> > +++ b/drivers/iio/industrialio-event.c
> > @@ -385,6 +385,7 @@ static int iio_device_add_event(struct iio_dev *indio_dev,
> >
> > ret = __iio_add_chan_devattr(postfix, chan, show, store,
> > (i << 16) | spec_index, shared_by, &indio_dev->dev,
> > + NULL,
> > &iio_dev_opaque->event_interface->dev_attr_list);
> > kfree(postfix);
> >
> > diff --git a/include/linux/iio/sysfs.h b/include/linux/iio/sysfs.h
> > index b532c875bc24..e51fba66de4b 100644
> > --- a/include/linux/iio/sysfs.h
> > +++ b/include/linux/iio/sysfs.h
> > @@ -9,6 +9,7 @@
> > #ifndef _INDUSTRIAL_IO_SYSFS_H_
> > #define _INDUSTRIAL_IO_SYSFS_H_
> >
> > +struct iio_buffer;
> > struct iio_chan_spec;
> >
> > /**
> > @@ -17,12 +18,14 @@ struct iio_chan_spec;
> > * @address: associated register address
> > * @l: list head for maintaining list of dynamically created attrs
> > * @c: specification for the underlying channel
> > + * @buffer: the IIO buffer to which this attribute belongs to (if any)
> > */
> > struct iio_dev_attr {
> > struct device_attribute dev_attr;
> > u64 address;
> > struct list_head l;
> > struct iio_chan_spec const *c;
> > + struct iio_buffer *buffer;
> > };
> >
> > #define to_iio_dev_attr(_dev_attr) \
>
next prev parent reply other threads:[~2021-02-05 8:27 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-01 14:50 [PATCH v3 00/11] iio: core,buffer: add support for multiple IIO buffers per IIO device Alexandru Ardelean
2021-02-01 14:50 ` [PATCH v3 01/11] docs: ioctl-number.rst: reserve IIO subsystem ioctl() space Alexandru Ardelean
2021-02-04 17:06 ` Jonathan Cameron
2021-02-05 7:08 ` Alexandru Ardelean
2021-02-01 14:50 ` [PATCH v3 02/11] iio: core: register chardev only if needed Alexandru Ardelean
2021-02-04 17:17 ` Jonathan Cameron
2021-02-01 14:50 ` [PATCH v3 03/11] iio: core-trigger: make iio_device_register_trigger_consumer() an int return Alexandru Ardelean
2021-02-01 14:50 ` [PATCH v3 04/11] iio: core: rework iio device group creation Alexandru Ardelean
2021-02-04 17:32 ` Jonathan Cameron
2021-02-05 7:32 ` Alexandru Ardelean
2021-02-01 14:50 ` [PATCH v3 05/11] iio: buffer: group attr count and attr alloc Alexandru Ardelean
2021-02-04 17:49 ` Jonathan Cameron
2021-02-05 8:12 ` Alexandru Ardelean
2021-02-05 12:33 ` Jonathan Cameron
2021-02-01 14:51 ` [PATCH v3 06/11] iio: core: merge buffer/ & scan_elements/ attributes Alexandru Ardelean
2021-02-01 20:02 ` kernel test robot
2021-02-01 20:02 ` kernel test robot
2021-02-02 6:07 ` Dan Carpenter
2021-02-02 6:07 ` Dan Carpenter
2021-02-02 6:07 ` Dan Carpenter
2021-02-03 10:02 ` Andy Shevchenko
2021-02-04 13:41 ` Alexandru Ardelean
2021-02-05 11:07 ` Andy Shevchenko
2021-02-01 14:51 ` [PATCH v3 07/11] iio: add reference to iio buffer on iio_dev_attr Alexandru Ardelean
2021-02-04 18:09 ` Jonathan Cameron
2021-02-05 8:26 ` Alexandru Ardelean [this message]
2021-02-01 14:51 ` [PATCH v3 08/11] iio: buffer: wrap all buffer attributes into iio_dev_attr Alexandru Ardelean
2021-02-04 18:23 ` Jonathan Cameron
2021-02-05 9:17 ` Alexandru Ardelean
2021-02-05 12:39 ` Jonathan Cameron
2021-02-05 12:57 ` Alexandru Ardelean
2021-02-06 14:46 ` Jonathan Cameron
2021-02-01 14:51 ` [PATCH v3 09/11] iio: core: wrap iio device & buffer into struct for character devices Alexandru Ardelean
2021-02-01 14:51 ` [PATCH v3 10/11] iio: buffer: introduce support for attaching more IIO buffers Alexandru Ardelean
2021-02-04 18:34 ` Jonathan Cameron
2021-02-05 9:32 ` Alexandru Ardelean
2021-02-01 14:51 ` [PATCH v3 11/11] iio: buffer: add ioctl() to support opening extra buffers for IIO device Alexandru Ardelean
2021-02-04 19:00 ` Jonathan Cameron
2021-02-05 9:51 ` Alexandru Ardelean
2021-02-05 12:44 ` Jonathan Cameron
2021-02-05 12:48 ` Alexandru Ardelean
2021-02-06 14:48 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CA+U=Dsq34ixRT71qD8ZTR7MQ7NDgDZV2EJ+3gT=QQ8wBz4GiGA@mail.gmail.com' \
--to=ardeleanalex@gmail.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=Michael.Hennerich@analog.com \
--cc=alexandru.ardelean@analog.com \
--cc=dragos.bogdan@analog.com \
--cc=gregkh@linuxfoundation.org \
--cc=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nuno.sa@analog.com \
--cc=rafael@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.