linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] iio: core: Add optional symbolic label to device attributes
@ 2019-08-27  3:35 Phil Reid
  2019-08-27  3:35 ` [PATCH 1/2] dt-binding: iio: Add optional label property Phil Reid
  2019-08-27  3:35 ` [PATCH 2/2] iio: core: Add optional symbolic label to device attributes Phil Reid
  0 siblings, 2 replies; 12+ messages in thread
From: Phil Reid @ 2019-08-27  3:35 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, robh+dt, mark.rutland, preid,
	linux-iio, devicetree, michal.simek

If a label is defined in the device tree for this device add that
to the device specific attributes. This is useful for userspace to
be able to identify an individual device when multiple identical
chips are present in the system.

Similar to leds, display labels etc.

Phil Reid (2):
  dt-binding: iio: Add optional label property
  iio: core: Add optional symbolic label to device attributes

 Documentation/devicetree/bindings/iio/iio-bindings.txt |  5 +++++
 drivers/iio/industrialio-core.c                        | 17 +++++++++++++++++
 include/linux/iio/iio.h                                |  1 +
 3 files changed, 23 insertions(+)

-- 
1.8.3.1


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

* [PATCH 1/2] dt-binding: iio: Add optional label property
  2019-08-27  3:35 [PATCH 0/2] iio: core: Add optional symbolic label to device attributes Phil Reid
@ 2019-08-27  3:35 ` Phil Reid
  2019-08-27  6:10   ` Michal Simek
  2019-08-28  6:09   ` Michal Simek
  2019-08-27  3:35 ` [PATCH 2/2] iio: core: Add optional symbolic label to device attributes Phil Reid
  1 sibling, 2 replies; 12+ messages in thread
From: Phil Reid @ 2019-08-27  3:35 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, robh+dt, mark.rutland, preid,
	linux-iio, devicetree, michal.simek

This optional property defines a symbolic name for the device.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 Documentation/devicetree/bindings/iio/iio-bindings.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
index 68d6f8ce063b..ffeae5aad8b5 100644
--- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -18,12 +18,17 @@ Required properties:
 		   with a single IIO output and 1 for nodes with multiple
 		   IIO outputs.
 
+Optional properties:
+label:		   A symbolic name for the device.
+
+
 Example for a simple configuration with no trigger:
 
 	adc: voltage-sensor@35 {
 		compatible = "maxim,max1139";
 		reg = <0x35>;
 		#io-channel-cells = <1>;
+		label = "adc_voltage_sensor";
 	};
 
 Example for a configuration with trigger:
-- 
1.8.3.1


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

* [PATCH 2/2] iio: core: Add optional symbolic label to device attributes
  2019-08-27  3:35 [PATCH 0/2] iio: core: Add optional symbolic label to device attributes Phil Reid
  2019-08-27  3:35 ` [PATCH 1/2] dt-binding: iio: Add optional label property Phil Reid
@ 2019-08-27  3:35 ` Phil Reid
  2019-09-08 12:17   ` Jonathan Cameron
  2019-09-09  7:45   ` Michal Simek
  1 sibling, 2 replies; 12+ messages in thread
From: Phil Reid @ 2019-08-27  3:35 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, robh+dt, mark.rutland, preid,
	linux-iio, devicetree, michal.simek

If a label is defined in the device tree for this device add that
to the device specific attributes. This is useful for userspace to
be able to identify an individual device when multiple identical
chips are present in the system.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/iio/industrialio-core.c | 17 +++++++++++++++++
 include/linux/iio/iio.h         |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 8303639963d7..2d7fb7629095 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1300,6 +1300,16 @@ static ssize_t iio_show_dev_name(struct device *dev,
 
 static DEVICE_ATTR(name, S_IRUGO, iio_show_dev_name, NULL);
 
+static ssize_t iio_show_dev_label(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	return snprintf(buf, PAGE_SIZE, "%s\n", indio_dev->label);
+}
+
+static DEVICE_ATTR(label, S_IRUGO, iio_show_dev_label, NULL);
+
 static ssize_t iio_show_timestamp_clock(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
@@ -1416,6 +1426,8 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
 
 	if (indio_dev->name)
 		attrcount++;
+	if (indio_dev->label)
+		attrcount++;
 	if (clk)
 		attrcount++;
 
@@ -1438,6 +1450,8 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
 		indio_dev->chan_attr_group.attrs[attrn++] = &p->dev_attr.attr;
 	if (indio_dev->name)
 		indio_dev->chan_attr_group.attrs[attrn++] = &dev_attr_name.attr;
+	if (indio_dev->label)
+		indio_dev->chan_attr_group.attrs[attrn++] = &dev_attr_label.attr;
 	if (clk)
 		indio_dev->chan_attr_group.attrs[attrn++] = clk;
 
@@ -1709,6 +1723,9 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
 		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
 
+	indio_dev->label = of_get_property(indio_dev->dev.of_node, "label",
+					   NULL);
+
 	ret = iio_check_unique_scan_index(indio_dev);
 	if (ret < 0)
 		return ret;
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index a74cb177dc6f..3f89db50d3f6 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -556,6 +556,7 @@ struct iio_dev {
 	struct list_head		channel_attr_list;
 	struct attribute_group		chan_attr_group;
 	const char			*name;
+	const char			*label;
 	const struct iio_info		*info;
 	clockid_t			clock_id;
 	struct mutex			info_exist_lock;
-- 
1.8.3.1


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

* Re: [PATCH 1/2] dt-binding: iio: Add optional label property
  2019-08-27  3:35 ` [PATCH 1/2] dt-binding: iio: Add optional label property Phil Reid
@ 2019-08-27  6:10   ` Michal Simek
  2019-08-28  6:09   ` Michal Simek
  1 sibling, 0 replies; 12+ messages in thread
From: Michal Simek @ 2019-08-27  6:10 UTC (permalink / raw)
  To: Phil Reid, jic23, knaack.h, lars, pmeerw, robh+dt, mark.rutland,
	linux-iio, devicetree, michal.simek

On 27. 08. 19 5:35, Phil Reid wrote:
> This optional property defines a symbolic name for the device.
> 
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
>  Documentation/devicetree/bindings/iio/iio-bindings.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> index 68d6f8ce063b..ffeae5aad8b5 100644
> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> @@ -18,12 +18,17 @@ Required properties:
>  		   with a single IIO output and 1 for nodes with multiple
>  		   IIO outputs.
>  
> +Optional properties:
> +label:		   A symbolic name for the device.
> +
> +
>  Example for a simple configuration with no trigger:
>  
>  	adc: voltage-sensor@35 {
>  		compatible = "maxim,max1139";
>  		reg = <0x35>;
>  		#io-channel-cells = <1>;
> +		label = "adc_voltage_sensor";
>  	};
>  
>  Example for a configuration with trigger:
> 

Suggested-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

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

* Re: [PATCH 1/2] dt-binding: iio: Add optional label property
  2019-08-27  3:35 ` [PATCH 1/2] dt-binding: iio: Add optional label property Phil Reid
  2019-08-27  6:10   ` Michal Simek
@ 2019-08-28  6:09   ` Michal Simek
  2019-08-29 23:02     ` Rob Herring
  1 sibling, 1 reply; 12+ messages in thread
From: Michal Simek @ 2019-08-28  6:09 UTC (permalink / raw)
  To: Phil Reid, jic23, knaack.h, lars, pmeerw, robh+dt, mark.rutland,
	linux-iio, devicetree, michal.simek

On 27. 08. 19 5:35, Phil Reid wrote:
> This optional property defines a symbolic name for the device.
> 
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
>  Documentation/devicetree/bindings/iio/iio-bindings.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> index 68d6f8ce063b..ffeae5aad8b5 100644
> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> @@ -18,12 +18,17 @@ Required properties:
>  		   with a single IIO output and 1 for nodes with multiple
>  		   IIO outputs.
>  
> +Optional properties:
> +label:		   A symbolic name for the device.
> +
> +
>  Example for a simple configuration with no trigger:
>  
>  	adc: voltage-sensor@35 {
>  		compatible = "maxim,max1139";
>  		reg = <0x35>;
>  		#io-channel-cells = <1>;
> +		label = "adc_voltage_sensor";
>  	};
>  
>  Example for a configuration with trigger:
> 

Just for the record. This patch has been created based on initial
discussion about label property. And Rob had not problem with using
label in connection to ina226. https://lkml.org/lkml/2019/8/27/1213

Thanks,
Michal

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

* Re: [PATCH 1/2] dt-binding: iio: Add optional label property
  2019-08-28  6:09   ` Michal Simek
@ 2019-08-29 23:02     ` Rob Herring
  2019-08-30  1:01       ` Phil Reid
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2019-08-29 23:02 UTC (permalink / raw)
  To: Michal Simek
  Cc: Phil Reid, jic23, knaack.h, lars, pmeerw, mark.rutland,
	linux-iio, devicetree

On Wed, Aug 28, 2019 at 08:09:19AM +0200, Michal Simek wrote:
> On 27. 08. 19 5:35, Phil Reid wrote:
> > This optional property defines a symbolic name for the device.
> > 
> > Signed-off-by: Phil Reid <preid@electromag.com.au>
> > ---
> >  Documentation/devicetree/bindings/iio/iio-bindings.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > index 68d6f8ce063b..ffeae5aad8b5 100644
> > --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> > @@ -18,12 +18,17 @@ Required properties:
> >  		   with a single IIO output and 1 for nodes with multiple
> >  		   IIO outputs.
> >  
> > +Optional properties:
> > +label:		   A symbolic name for the device.
> > +
> > +
> >  Example for a simple configuration with no trigger:
> >  
> >  	adc: voltage-sensor@35 {
> >  		compatible = "maxim,max1139";
> >  		reg = <0x35>;
> >  		#io-channel-cells = <1>;
> > +		label = "adc_voltage_sensor";
> >  	};
> >  
> >  Example for a configuration with trigger:
> > 
> 
> Just for the record. This patch has been created based on initial
> discussion about label property. And Rob had not problem with using
> label in connection to ina226. https://lkml.org/lkml/2019/8/27/1213

I didn't, but based on the name here I'm less convinced. 'label' is 
supposed to be for needing to distinguish between more than 1 of 
something. A name like 'adc_voltage_sensor' doesn't really.

Rob

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

* Re: [PATCH 1/2] dt-binding: iio: Add optional label property
  2019-08-29 23:02     ` Rob Herring
@ 2019-08-30  1:01       ` Phil Reid
  2019-08-30 12:34         ` Rob Herring
  0 siblings, 1 reply; 12+ messages in thread
From: Phil Reid @ 2019-08-30  1:01 UTC (permalink / raw)
  To: Rob Herring, Michal Simek
  Cc: jic23, knaack.h, lars, pmeerw, mark.rutland, linux-iio, devicetree

On 30/08/2019 07:02, Rob Herring wrote:
> On Wed, Aug 28, 2019 at 08:09:19AM +0200, Michal Simek wrote:
>> On 27. 08. 19 5:35, Phil Reid wrote:
>>> This optional property defines a symbolic name for the device.
>>>
>>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>>> ---
>>>   Documentation/devicetree/bindings/iio/iio-bindings.txt | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>> index 68d6f8ce063b..ffeae5aad8b5 100644
>>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>> @@ -18,12 +18,17 @@ Required properties:
>>>   		   with a single IIO output and 1 for nodes with multiple
>>>   		   IIO outputs.
>>>   
>>> +Optional properties:
>>> +label:		   A symbolic name for the device.
>>> +
>>> +
>>>   Example for a simple configuration with no trigger:
>>>   
>>>   	adc: voltage-sensor@35 {
>>>   		compatible = "maxim,max1139";
>>>   		reg = <0x35>;
>>>   		#io-channel-cells = <1>;
>>> +		label = "adc_voltage_sensor";
>>>   	};
>>>   
>>>   Example for a configuration with trigger:
>>>
>>
>> Just for the record. This patch has been created based on initial
>> discussion about label property. And Rob had not problem with using
>> label in connection to ina226. https://lkml.org/lkml/2019/8/27/1213
> 
> I didn't, but based on the name here I'm less convinced. 'label' is
> supposed to be for needing to distinguish between more than 1 of
> something. A name like 'adc_voltage_sensor' doesn't really.
> 
> Rob
> 
> 

That's the problem we're try to solve. Having multiple devices and try to
determine which device is which.
eg: Mutliple adc's.
For example I have the same dac chip on multiple boards that do different
things, it's difficult to id them.

so label examples could be:
label = "current_control_group1";
label = "voltage_control_group1";

Are you totally against this or is it a problem with me not being clear
with the problem and the wording of the commit message or the example?




-- 
Regards
Phil Reid


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

* Re: [PATCH 1/2] dt-binding: iio: Add optional label property
  2019-08-30  1:01       ` Phil Reid
@ 2019-08-30 12:34         ` Rob Herring
  2019-08-31 10:19           ` Phil Reid
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2019-08-30 12:34 UTC (permalink / raw)
  To: Phil Reid
  Cc: Michal Simek, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Mark Rutland,
	open list:IIO SUBSYSTEM AND DRIVERS, devicetree

On Thu, Aug 29, 2019 at 8:01 PM Phil Reid <preid@electromag.com.au> wrote:
>
> On 30/08/2019 07:02, Rob Herring wrote:
> > On Wed, Aug 28, 2019 at 08:09:19AM +0200, Michal Simek wrote:
> >> On 27. 08. 19 5:35, Phil Reid wrote:
> >>> This optional property defines a symbolic name for the device.
> >>>
> >>> Signed-off-by: Phil Reid <preid@electromag.com.au>
> >>> ---
> >>>   Documentation/devicetree/bindings/iio/iio-bindings.txt | 5 +++++
> >>>   1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>> index 68d6f8ce063b..ffeae5aad8b5 100644
> >>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>> @@ -18,12 +18,17 @@ Required properties:
> >>>                with a single IIO output and 1 for nodes with multiple
> >>>                IIO outputs.
> >>>
> >>> +Optional properties:
> >>> +label:                A symbolic name for the device.
> >>> +
> >>> +
> >>>   Example for a simple configuration with no trigger:
> >>>
> >>>     adc: voltage-sensor@35 {
> >>>             compatible = "maxim,max1139";
> >>>             reg = <0x35>;
> >>>             #io-channel-cells = <1>;
> >>> +           label = "adc_voltage_sensor";
> >>>     };
> >>>
> >>>   Example for a configuration with trigger:
> >>>
> >>
> >> Just for the record. This patch has been created based on initial
> >> discussion about label property. And Rob had not problem with using
> >> label in connection to ina226. https://lkml.org/lkml/2019/8/27/1213
> >
> > I didn't, but based on the name here I'm less convinced. 'label' is
> > supposed to be for needing to distinguish between more than 1 of
> > something. A name like 'adc_voltage_sensor' doesn't really.
> >
> > Rob
> >
> >
>
> That's the problem we're try to solve. Having multiple devices and try to
> determine which device is which.
> eg: Mutliple adc's.
> For example I have the same dac chip on multiple boards that do different
> things, it's difficult to id them.
>
> so label examples could be:
> label = "current_control_group1";
> label = "voltage_control_group1";
>
> Are you totally against this or is it a problem with me not being clear
> with the problem and the wording of the commit message or the example?

It's just the example is less than ideal. But it's just an example, so:

Reviewed-by: Rob Herring <robh@kernel.org>

Feel free to update the example if you respin.

Rob

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

* Re: [PATCH 1/2] dt-binding: iio: Add optional label property
  2019-08-30 12:34         ` Rob Herring
@ 2019-08-31 10:19           ` Phil Reid
  2019-09-08 12:14             ` Jonathan Cameron
  0 siblings, 1 reply; 12+ messages in thread
From: Phil Reid @ 2019-08-31 10:19 UTC (permalink / raw)
  To: Rob Herring
  Cc: Michal Simek, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald, Mark Rutland,
	open list:IIO SUBSYSTEM AND DRIVERS, devicetree

On 30/08/2019 20:34, Rob Herring wrote:
> On Thu, Aug 29, 2019 at 8:01 PM Phil Reid <preid@electromag.com.au> wrote:
>>
>> On 30/08/2019 07:02, Rob Herring wrote:
>>> On Wed, Aug 28, 2019 at 08:09:19AM +0200, Michal Simek wrote:
>>>> On 27. 08. 19 5:35, Phil Reid wrote:
>>>>> This optional property defines a symbolic name for the device.
>>>>>
>>>>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>>>>> ---
>>>>>    Documentation/devicetree/bindings/iio/iio-bindings.txt | 5 +++++
>>>>>    1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>>> index 68d6f8ce063b..ffeae5aad8b5 100644
>>>>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>>>> @@ -18,12 +18,17 @@ Required properties:
>>>>>                 with a single IIO output and 1 for nodes with multiple
>>>>>                 IIO outputs.
>>>>>
>>>>> +Optional properties:
>>>>> +label:                A symbolic name for the device.
>>>>> +
>>>>> +
>>>>>    Example for a simple configuration with no trigger:
>>>>>
>>>>>      adc: voltage-sensor@35 {
>>>>>              compatible = "maxim,max1139";
>>>>>              reg = <0x35>;
>>>>>              #io-channel-cells = <1>;
>>>>> +           label = "adc_voltage_sensor";
>>>>>      };
>>>>>
>>>>>    Example for a configuration with trigger:
>>>>>
>>>>
>>>> Just for the record. This patch has been created based on initial
>>>> discussion about label property. And Rob had not problem with using
>>>> label in connection to ina226. https://lkml.org/lkml/2019/8/27/1213
>>>
>>> I didn't, but based on the name here I'm less convinced. 'label' is
>>> supposed to be for needing to distinguish between more than 1 of
>>> something. A name like 'adc_voltage_sensor' doesn't really.
>>>
>>> Rob
>>>
>>>
>>
>> That's the problem we're try to solve. Having multiple devices and try to
>> determine which device is which.
>> eg: Mutliple adc's.
>> For example I have the same dac chip on multiple boards that do different
>> things, it's difficult to id them.
>>
>> so label examples could be:
>> label = "current_control_group1";
>> label = "voltage_control_group1";
>>
>> Are you totally against this or is it a problem with me not being clear
>> with the problem and the wording of the commit message or the example?
> 
> It's just the example is less than ideal. But it's just an example, so:
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> Feel free to update the example if you respin.
> 
Thanks Rob,

I'll update the example if the series gets a respin.



-- 
Regards
Phil Reid

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

* Re: [PATCH 1/2] dt-binding: iio: Add optional label property
  2019-08-31 10:19           ` Phil Reid
@ 2019-09-08 12:14             ` Jonathan Cameron
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2019-09-08 12:14 UTC (permalink / raw)
  To: Phil Reid
  Cc: Rob Herring, Michal Simek, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald, Mark Rutland,
	open list:IIO SUBSYSTEM AND DRIVERS, devicetree

On Sat, 31 Aug 2019 18:19:53 +0800
Phil Reid <preid@electromag.com.au> wrote:

> On 30/08/2019 20:34, Rob Herring wrote:
> > On Thu, Aug 29, 2019 at 8:01 PM Phil Reid <preid@electromag.com.au> wrote:  
> >>
> >> On 30/08/2019 07:02, Rob Herring wrote:  
> >>> On Wed, Aug 28, 2019 at 08:09:19AM +0200, Michal Simek wrote:  
> >>>> On 27. 08. 19 5:35, Phil Reid wrote:  
> >>>>> This optional property defines a symbolic name for the device.
> >>>>>
> >>>>> Signed-off-by: Phil Reid <preid@electromag.com.au>
> >>>>> ---
> >>>>>    Documentation/devicetree/bindings/iio/iio-bindings.txt | 5 +++++
> >>>>>    1 file changed, 5 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>>>> index 68d6f8ce063b..ffeae5aad8b5 100644
> >>>>> --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>>>> +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
> >>>>> @@ -18,12 +18,17 @@ Required properties:
> >>>>>                 with a single IIO output and 1 for nodes with multiple
> >>>>>                 IIO outputs.
> >>>>>
> >>>>> +Optional properties:
> >>>>> +label:                A symbolic name for the device.
> >>>>> +
> >>>>> +
> >>>>>    Example for a simple configuration with no trigger:
> >>>>>
> >>>>>      adc: voltage-sensor@35 {
> >>>>>              compatible = "maxim,max1139";
> >>>>>              reg = <0x35>;
> >>>>>              #io-channel-cells = <1>;
> >>>>> +           label = "adc_voltage_sensor";
> >>>>>      };
> >>>>>
> >>>>>    Example for a configuration with trigger:
> >>>>>  
> >>>>
> >>>> Just for the record. This patch has been created based on initial
> >>>> discussion about label property. And Rob had not problem with using
> >>>> label in connection to ina226. https://lkml.org/lkml/2019/8/27/1213  
> >>>
> >>> I didn't, but based on the name here I'm less convinced. 'label' is
> >>> supposed to be for needing to distinguish between more than 1 of
> >>> something. A name like 'adc_voltage_sensor' doesn't really.
> >>>
> >>> Rob
> >>>
> >>>  
> >>
> >> That's the problem we're try to solve. Having multiple devices and try to
> >> determine which device is which.
> >> eg: Mutliple adc's.
> >> For example I have the same dac chip on multiple boards that do different
> >> things, it's difficult to id them.
> >>
> >> so label examples could be:
> >> label = "current_control_group1";
> >> label = "voltage_control_group1";
> >>
> >> Are you totally against this or is it a problem with me not being clear
> >> with the problem and the wording of the commit message or the example?  
> > 
> > It's just the example is less than ideal. But it's just an example, so:
> > 
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > 
> > Feel free to update the example if you respin.
> >   
> Thanks Rob,
> 
> I'll update the example if the series gets a respin.

Please do respin some more 'example' suited names :)

Thanks,

Jonathan

> 
> 
> 


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

* Re: [PATCH 2/2] iio: core: Add optional symbolic label to device attributes
  2019-08-27  3:35 ` [PATCH 2/2] iio: core: Add optional symbolic label to device attributes Phil Reid
@ 2019-09-08 12:17   ` Jonathan Cameron
  2019-09-09  7:45   ` Michal Simek
  1 sibling, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2019-09-08 12:17 UTC (permalink / raw)
  To: Phil Reid
  Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, linux-iio,
	devicetree, michal.simek

On Tue, 27 Aug 2019 11:35:24 +0800
Phil Reid <preid@electromag.com.au> wrote:

> If a label is defined in the device tree for this device add that
> to the device specific attributes. This is useful for userspace to
> be able to identify an individual device when multiple identical
> chips are present in the system.
> 
> Signed-off-by: Phil Reid <preid@electromag.com.au>

Looks good to me.  I like this as it finally gets us around some
perhaps slightly dubious decisions I made about the name attribute
a long time ago and the bunch of uses that don't actually conform
even to what I had in mind back then.

I'll pick it up for the 5.5 cycle once you've respun to give
more example friendly names in patch 1 and Rob is happy with it.

Thanks,

Jonathan

> ---
>  drivers/iio/industrialio-core.c | 17 +++++++++++++++++
>  include/linux/iio/iio.h         |  1 +
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 8303639963d7..2d7fb7629095 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1300,6 +1300,16 @@ static ssize_t iio_show_dev_name(struct device *dev,
>  
>  static DEVICE_ATTR(name, S_IRUGO, iio_show_dev_name, NULL);
>  
> +static ssize_t iio_show_dev_label(struct device *dev,
> +				 struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	return snprintf(buf, PAGE_SIZE, "%s\n", indio_dev->label);
> +}
> +
> +static DEVICE_ATTR(label, S_IRUGO, iio_show_dev_label, NULL);
> +
>  static ssize_t iio_show_timestamp_clock(struct device *dev,
>  					struct device_attribute *attr,
>  					char *buf)
> @@ -1416,6 +1426,8 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
>  
>  	if (indio_dev->name)
>  		attrcount++;
> +	if (indio_dev->label)
> +		attrcount++;
>  	if (clk)
>  		attrcount++;
>  
> @@ -1438,6 +1450,8 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
>  		indio_dev->chan_attr_group.attrs[attrn++] = &p->dev_attr.attr;
>  	if (indio_dev->name)
>  		indio_dev->chan_attr_group.attrs[attrn++] = &dev_attr_name.attr;
> +	if (indio_dev->label)
> +		indio_dev->chan_attr_group.attrs[attrn++] = &dev_attr_label.attr;
>  	if (clk)
>  		indio_dev->chan_attr_group.attrs[attrn++] = clk;
>  
> @@ -1709,6 +1723,9 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
>  	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
>  		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
>  
> +	indio_dev->label = of_get_property(indio_dev->dev.of_node, "label",
> +					   NULL);
> +
>  	ret = iio_check_unique_scan_index(indio_dev);
>  	if (ret < 0)
>  		return ret;
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index a74cb177dc6f..3f89db50d3f6 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -556,6 +556,7 @@ struct iio_dev {
>  	struct list_head		channel_attr_list;
>  	struct attribute_group		chan_attr_group;
>  	const char			*name;
> +	const char			*label;
>  	const struct iio_info		*info;
>  	clockid_t			clock_id;
>  	struct mutex			info_exist_lock;


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

* Re: [PATCH 2/2] iio: core: Add optional symbolic label to device attributes
  2019-08-27  3:35 ` [PATCH 2/2] iio: core: Add optional symbolic label to device attributes Phil Reid
  2019-09-08 12:17   ` Jonathan Cameron
@ 2019-09-09  7:45   ` Michal Simek
  1 sibling, 0 replies; 12+ messages in thread
From: Michal Simek @ 2019-09-09  7:45 UTC (permalink / raw)
  To: Phil Reid, jic23, knaack.h, lars, pmeerw, robh+dt, mark.rutland,
	linux-iio, devicetree, michal.simek

On 27. 08. 19 5:35, Phil Reid wrote:
> If a label is defined in the device tree for this device add that
> to the device specific attributes. This is useful for userspace to
> be able to identify an individual device when multiple identical
> chips are present in the system.
> 
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
>  drivers/iio/industrialio-core.c | 17 +++++++++++++++++
>  include/linux/iio/iio.h         |  1 +
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 8303639963d7..2d7fb7629095 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1300,6 +1300,16 @@ static ssize_t iio_show_dev_name(struct device *dev,
>  
>  static DEVICE_ATTR(name, S_IRUGO, iio_show_dev_name, NULL);
>  
> +static ssize_t iio_show_dev_label(struct device *dev,
> +				 struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	return snprintf(buf, PAGE_SIZE, "%s\n", indio_dev->label);
> +}
> +
> +static DEVICE_ATTR(label, S_IRUGO, iio_show_dev_label, NULL);
> +
>  static ssize_t iio_show_timestamp_clock(struct device *dev,
>  					struct device_attribute *attr,
>  					char *buf)
> @@ -1416,6 +1426,8 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
>  
>  	if (indio_dev->name)
>  		attrcount++;
> +	if (indio_dev->label)
> +		attrcount++;
>  	if (clk)
>  		attrcount++;
>  
> @@ -1438,6 +1450,8 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
>  		indio_dev->chan_attr_group.attrs[attrn++] = &p->dev_attr.attr;
>  	if (indio_dev->name)
>  		indio_dev->chan_attr_group.attrs[attrn++] = &dev_attr_name.attr;
> +	if (indio_dev->label)
> +		indio_dev->chan_attr_group.attrs[attrn++] = &dev_attr_label.attr;
>  	if (clk)
>  		indio_dev->chan_attr_group.attrs[attrn++] = clk;
>  
> @@ -1709,6 +1723,9 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
>  	if (!indio_dev->dev.of_node && indio_dev->dev.parent)
>  		indio_dev->dev.of_node = indio_dev->dev.parent->of_node;
>  
> +	indio_dev->label = of_get_property(indio_dev->dev.of_node, "label",
> +					   NULL);
> +
>  	ret = iio_check_unique_scan_index(indio_dev);
>  	if (ret < 0)
>  		return ret;
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index a74cb177dc6f..3f89db50d3f6 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -556,6 +556,7 @@ struct iio_dev {
>  	struct list_head		channel_attr_list;
>  	struct attribute_group		chan_attr_group;
>  	const char			*name;
> +	const char			*label;
>  	const struct iio_info		*info;
>  	clockid_t			clock_id;
>  	struct mutex			info_exist_lock;
> 

Tested-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal


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

end of thread, other threads:[~2019-09-09  7:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27  3:35 [PATCH 0/2] iio: core: Add optional symbolic label to device attributes Phil Reid
2019-08-27  3:35 ` [PATCH 1/2] dt-binding: iio: Add optional label property Phil Reid
2019-08-27  6:10   ` Michal Simek
2019-08-28  6:09   ` Michal Simek
2019-08-29 23:02     ` Rob Herring
2019-08-30  1:01       ` Phil Reid
2019-08-30 12:34         ` Rob Herring
2019-08-31 10:19           ` Phil Reid
2019-09-08 12:14             ` Jonathan Cameron
2019-08-27  3:35 ` [PATCH 2/2] iio: core: Add optional symbolic label to device attributes Phil Reid
2019-09-08 12:17   ` Jonathan Cameron
2019-09-09  7:45   ` Michal Simek

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).