linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Add "extended_name" to label
@ 2021-06-18 12:30 Paul Cercueil
  2021-06-18 12:30 ` [PATCH v3 1/2] iio: core: Forbid use of both labels and extended names Paul Cercueil
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Paul Cercueil @ 2021-06-18 12:30 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen, Alexandru Ardelean
  Cc: linux-iio, linux-kernel, Paul Cercueil

Hi Jonathan,

This is the v3 of my patchset that sets the label attribute to the
"extended_name".

v3 only refactors the code of patch [2/2] to make it look a bit better.

Cheers,
-Paul

Paul Cercueil (2):
  iio: core: Forbid use of both labels and extended names
  iio: core: Support reading extended name as label

 drivers/iio/industrialio-core.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] iio: core: Forbid use of both labels and extended names
  2021-06-18 12:30 [PATCH v3 0/2] Add "extended_name" to label Paul Cercueil
@ 2021-06-18 12:30 ` Paul Cercueil
  2021-06-18 12:30 ` [PATCH v3 2/2] iio: core: Support reading extended name as label Paul Cercueil
  2021-07-04 17:34 ` [PATCH v3 0/2] Add "extended_name" to label Jonathan Cameron
  2 siblings, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2021-06-18 12:30 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen, Alexandru Ardelean
  Cc: linux-iio, linux-kernel, Paul Cercueil

Extended names are a problem for user-space as they make the filenames
in sysfs sometimes not parsable. They are now deprecated in favor of
labels.

This change makes sure that a device driver won't provide both labels
and extended names for its channels. It has never been the case and we
don't want it to happen.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
---
 drivers/iio/industrialio-core.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 59efb36db2c7..81f40dab778a 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1836,6 +1836,24 @@ static int iio_check_unique_scan_index(struct iio_dev *indio_dev)
 	return 0;
 }
 
+static int iio_check_extended_name(const struct iio_dev *indio_dev)
+{
+	unsigned int i;
+
+	if (!indio_dev->info->read_label)
+		return 0;
+
+	for (i = 0; i < indio_dev->num_channels; i++) {
+		if (indio_dev->channels[i].extend_name) {
+			dev_err(&indio_dev->dev,
+				"Cannot use labels and extend_name at the same time\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static const struct iio_buffer_setup_ops noop_ring_setup_ops;
 
 int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
@@ -1860,6 +1878,10 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 	if (ret < 0)
 		return ret;
 
+	ret = iio_check_extended_name(indio_dev);
+	if (ret < 0)
+		return ret;
+
 	iio_device_register_debugfs(indio_dev);
 
 	ret = iio_buffers_alloc_sysfs_and_mask(indio_dev);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] iio: core: Support reading extended name as label
  2021-06-18 12:30 [PATCH v3 0/2] Add "extended_name" to label Paul Cercueil
  2021-06-18 12:30 ` [PATCH v3 1/2] iio: core: Forbid use of both labels and extended names Paul Cercueil
@ 2021-06-18 12:30 ` Paul Cercueil
  2021-07-04 17:34 ` [PATCH v3 0/2] Add "extended_name" to label Jonathan Cameron
  2 siblings, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2021-06-18 12:30 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen, Alexandru Ardelean
  Cc: linux-iio, linux-kernel, Paul Cercueil

The point of this new change is to make the IIO tree actually parsable.

Before, given this attribute as a filename:
in_voltage0_aux_sample_rate

Userspace had no way to know if the attribute name was
"aux_sample_rate" with no extended name, or "sample_rate" with "aux" as
the extended name, or just "rate" with "aux_sample" as the extended
name.

This was somewhat possible to deduce when there was more than one
attribute present for a given channel, e.g:
in_voltage0_aux_sample_rate
in_voltage0_aux_frequency

There, it was possible to deduce that "aux" was the extended name. But
even with more than one attribute, this wasn't very robust, as two
attributes starting with the same prefix (e.g. "sample_rate" and
"sample_size") would result in the first part of the prefix being
interpreted as being part of the extended name.

To address the issue, knowing that channels will never have both a label
and an extended name, set the channel's label to the extended name.
In this case, the label's attribute will also have the extended name in
its filename, but we can live with that - userspace can open
in_voltage0_<prefix>_label and verify that it returns <prefix> to obtain
the extended name.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v3: Refactor code to make it look better

 drivers/iio/industrialio-core.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 81f40dab778a..a19938b0c3a6 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -717,10 +717,13 @@ static ssize_t iio_read_channel_label(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 
-	if (!indio_dev->info->read_label)
-		return -EINVAL;
+	if (indio_dev->info->read_label)
+		return indio_dev->info->read_label(indio_dev, this_attr->c, buf);
+
+	if (this_attr->c->extend_name)
+		return sprintf(buf, "%s\n", this_attr->c->extend_name);
 
-	return indio_dev->info->read_label(indio_dev, this_attr->c, buf);
+	return -EINVAL;
 }
 
 static ssize_t iio_read_channel_info(struct device *dev,
@@ -1160,7 +1163,7 @@ static int iio_device_add_channel_label(struct iio_dev *indio_dev,
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
 	int ret;
 
-	if (!indio_dev->info->read_label)
+	if (!indio_dev->info->read_label && !chan->extend_name)
 		return 0;
 
 	ret = __iio_add_chan_devattr("label",
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/2] Add "extended_name" to label
  2021-06-18 12:30 [PATCH v3 0/2] Add "extended_name" to label Paul Cercueil
  2021-06-18 12:30 ` [PATCH v3 1/2] iio: core: Forbid use of both labels and extended names Paul Cercueil
  2021-06-18 12:30 ` [PATCH v3 2/2] iio: core: Support reading extended name as label Paul Cercueil
@ 2021-07-04 17:34 ` Jonathan Cameron
  2021-07-04 21:18   ` Paul Cercueil
  2 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2021-07-04 17:34 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Lars-Peter Clausen, Alexandru Ardelean, linux-iio, linux-kernel

On Fri, 18 Jun 2021 13:30:03 +0100
Paul Cercueil <paul@crapouillou.net> wrote:

> Hi Jonathan,
> 
> This is the v3 of my patchset that sets the label attribute to the
> "extended_name".
> 
> v3 only refactors the code of patch [2/2] to make it look a bit better.

Hi Paul,

Just thought I'd let you know v3 looks great to me.  Timing meant I didn't
manage to get it into last cycle, and am now waiting for rc1 to give a nice
place to rebase my trees on.  Hence I'll pick this up in a week or two.

Thanks,

Jonathan

> 
> Cheers,
> -Paul
> 
> Paul Cercueil (2):
>   iio: core: Forbid use of both labels and extended names
>   iio: core: Support reading extended name as label
> 
>  drivers/iio/industrialio-core.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/2] Add "extended_name" to label
  2021-07-04 17:34 ` [PATCH v3 0/2] Add "extended_name" to label Jonathan Cameron
@ 2021-07-04 21:18   ` Paul Cercueil
  2021-07-18 14:42     ` Jonathan Cameron
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Cercueil @ 2021-07-04 21:18 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Alexandru Ardelean, linux-iio, linux-kernel

Hi Jonathan,

Le dim., juil. 4 2021 at 18:34:25 +0100, Jonathan Cameron 
<jic23@kernel.org> a écrit :
> On Fri, 18 Jun 2021 13:30:03 +0100
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  Hi Jonathan,
>> 
>>  This is the v3 of my patchset that sets the label attribute to the
>>  "extended_name".
>> 
>>  v3 only refactors the code of patch [2/2] to make it look a bit 
>> better.
> 
> Hi Paul,
> 
> Just thought I'd let you know v3 looks great to me.  Timing meant I 
> didn't
> manage to get it into last cycle, and am now waiting for rc1 to give 
> a nice
> place to rebase my trees on.  Hence I'll pick this up in a week or 
> two.

That's perfectly fine, I wasn't expecting it to land before rc1.

Cheers,
-Paul



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/2] Add "extended_name" to label
  2021-07-04 21:18   ` Paul Cercueil
@ 2021-07-18 14:42     ` Jonathan Cameron
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2021-07-18 14:42 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Lars-Peter Clausen, Alexandru Ardelean, linux-iio, linux-kernel

On Sun, 04 Jul 2021 22:18:10 +0100
Paul Cercueil <paul@crapouillou.net> wrote:

> Hi Jonathan,
> 
> Le dim., juil. 4 2021 at 18:34:25 +0100, Jonathan Cameron 
> <jic23@kernel.org> a écrit :
> > On Fri, 18 Jun 2021 13:30:03 +0100
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >   
> >>  Hi Jonathan,
> >> 
> >>  This is the v3 of my patchset that sets the label attribute to the
> >>  "extended_name".
> >> 
> >>  v3 only refactors the code of patch [2/2] to make it look a bit 
> >> better.  
> > 
> > Hi Paul,
> > 
> > Just thought I'd let you know v3 looks great to me.  Timing meant I 
> > didn't
> > manage to get it into last cycle, and am now waiting for rc1 to give 
> > a nice
> > place to rebase my trees on.  Hence I'll pick this up in a week or 
> > two.  
> 
> That's perfectly fine, I wasn't expecting it to land before rc1.

Applied to the togreg branch of iio.git and pushed out as testing for 0-day
to poke at it before I push it out in a fashion that makes tidying
any issues up messy.

Thanks,

Jonathan

> 
> Cheers,
> -Paul
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-07-18 14:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-18 12:30 [PATCH v3 0/2] Add "extended_name" to label Paul Cercueil
2021-06-18 12:30 ` [PATCH v3 1/2] iio: core: Forbid use of both labels and extended names Paul Cercueil
2021-06-18 12:30 ` [PATCH v3 2/2] iio: core: Support reading extended name as label Paul Cercueil
2021-07-04 17:34 ` [PATCH v3 0/2] Add "extended_name" to label Jonathan Cameron
2021-07-04 21:18   ` Paul Cercueil
2021-07-18 14:42     ` 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).