devicetree: xilinx-xadc: Add optional xlnx, extend-name property
Subsystems (3)
PATCH 1/2 devicetree: xilinx-xadc: Add optional xlnx, extend-name property
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom extended name for defined channels.
--
Natinst-ReviewBoard-ID: 97119
--- Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt | 3 +++ 1 file changed, 3 insertions(+)
Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt +3 -0
@@ -68,6 +68,8 @@ Optional properties: Note each channel number should only be used at most once. Optional properties: + * xlnx,extend-name: Custom extended name for the + channel. * xlnx,bipolar: If set the channel is used in bipolar mode.
@@ -107,6 +109,7 @@ Examples: #size-cells = <0>; channel at 0 { reg = <0>; + xlnx,extend-name = "vin_v"; xlnx,bipolar; }; };
Signature · 1 line
1.9.1
PATCH 2/2 iio: adc: xilinx-xadc: Add xlnx, extend-name as an optional argument for aux channels
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom extended name for defined channels.
--
Natinst-ReviewBoard-ID: 97119
--- drivers/iio/adc/xilinx-xadc-core.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/iio/adc/xilinx-xadc-core.c +3 -0
@@ -1118,6 +1118,9 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np,if (ret || reg >16 )continue ; +of_property_read_string (child,"xlnx,extend-name" , + &chan->extend_name); +if (of_property_read_bool (child,"xlnx,bipolar" )) chan->scan_type.sign ='s' ;
Signature · 1 line
1.9.1
PATCH 2/2 iio: adc: xilinx-xadc: Add xlnx,extend-name as an optional argument for aux channels
Quoted 2 lines · Xander Huff
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom extended name for defined channels.
Hi,
'extend-name' is kind of a IIO specific term. I think a better name in the devicetree context would be 'label'. That's used everywhere else when giving a short description string to a node.
- Lars
PATCH v2 1/2 devicetree: xilinx-xadc: Add optional label property
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom label for defined channels.
--- Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt | 2 ++ 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt +2 -0
@@ -68,6 +68,7 @@ Optional properties: Note each channel number should only be used at most once. Optional properties: + * label: Custom label for the channel. * xlnx,bipolar: If set the channel is used in bipolar mode.
@@ -107,6 +108,7 @@ Examples: #size-cells = <0>; channel at 0 { reg = <0>; + label = "vin_v"; xlnx,bipolar; }; };
Signature · 1 line
1.9.1
PATCH v2 2/2 iio: adc: xilinx-xadc: Add label as an optional argument for aux channels
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom label for defined channels.
--- drivers/iio/adc/xilinx-xadc-core.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/iio/adc/xilinx-xadc-core.c +3 -0
@@ -1119,6 +1119,9 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np,if (ret || reg >16 )continue ; +of_property_read_string (child,"label" , + &chan->extend_name); +if (of_property_read_bool (child,"xlnx,bipolar" )) chan->scan_type.sign ='s' ;
Signature · 1 line
1.9.1
Jonathan Cameron
maintainerPATCH v2 2/2 iio: adc: xilinx-xadc: Add label as an optional argument for aux channels
Quoted 16 lines · Xander Huff
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom label for defined channels.
Signed-off-by Xander Huff <[email protected]>Reviewed-by Joe Hershberger <[email protected]>Reviewed-by Josh Cartwright <[email protected]>--- drivers/iio/adc/xilinx-xadc-core.c | 3 +++ 1 file changed, 3 insertions(+)drivers/iio/adc/xilinx-xadc-core.c +0
-0
drivers/iio/adc/xilinx-xadc-core.c static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, · line 1127
Quoted 9 lines
drivers/iio/adc/xilinx-xadc-core.c +3
-0@@ -1119,6 +1119,9 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np,if (ret || reg >16 )continue ; +of_property_read_string (child,"label" , + &chan->extend_name); +if (of_property_read_bool (child,"xlnx,bipolar" )) chan->scan_type.sign ='s' ;
I think what Lars was suggesting was a change to the IIO side of this rather than just changing the device tree binding. Perhaps an additional per channel attribute called _label which would allow for board specific labelling?
PATCH v2 2/2 iio: adc: xilinx-xadc: Add label as an optional argument for aux channels
Quoted 19 lines · Jonathan Cameron
Quoted 18 lines · Xander Huff
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom label for defined channels.
Signed-off-by Xander Huff <[email protected]>Reviewed-by Joe Hershberger <[email protected]>Reviewed-by Josh Cartwright <[email protected]>--- drivers/iio/adc/xilinx-xadc-core.c | 3 +++ 1 file changed, 3 insertions(+)drivers/iio/adc/xilinx-xadc-core.c +0
-0@@ -1119,6 +1119,9 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np,if (ret || reg >16 )continue ;
Quoted 9 lines
drivers/iio/adc/xilinx-xadc-core.c static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, · line 1127
Quoted 7 lines
drivers/iio/adc/xilinx-xadc-core.c +3
-0+of_property_read_string (child,"label" , + &chan->extend_name); +if (of_property_read_bool (child,"xlnx,bipolar" )) chan->scan_type.sign ='s' ;I think what Lars was suggesting was a change to the IIO side of this rather than just changing the device tree binding.
I'm not so sure. His observation was just that the typical name of these properties is 'label', which I think makes sense and is commonly used.
Naming things is hard.
Perhaps, though, an improvement may be made if we lifted this out of the xadc driver and moved it into the IIO core (I can't imagine that this driver is the only one representing each channel with a separate node in DT).
Quoted 2 lines
Perhaps an additional per channel attribute called
_labelwhich would allow for board specific labelling?
This would seem a bit odd in DT-land; if a user wanted a different label name for their board, they would just rename it in their device tree for that board. Fortunately, with the structured dtsi include files, this is pretty painless to do.
Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150608/962be065/attachment.sig>
Jonathan Cameron
maintainerPATCH v2 2/2 iio: adc: xilinx-xadc: Add label as an optional argument for aux channels
Quoted 41 lines · Josh Cartwright
Quoted 19 lines · Jonathan Cameron
Quoted 18 lines · Xander Huff
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom label for defined channels.
Signed-off-by Xander Huff <[email protected]>Reviewed-by Joe Hershberger <[email protected]>Reviewed-by Josh Cartwright <[email protected]>--- drivers/iio/adc/xilinx-xadc-core.c | 3 +++ 1 file changed, 3 insertions(+)drivers/iio/adc/xilinx-xadc-core.c +0
-0@@ -1119,6 +1119,9 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np,if (ret || reg >16 )continue ;Quoted 9 lines
drivers/iio/adc/xilinx-xadc-core.c static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, · line 1127
Quoted 7 lines
drivers/iio/adc/xilinx-xadc-core.c +3
-0+of_property_read_string (child,"label" , + &chan->extend_name); +if (of_property_read_bool (child,"xlnx,bipolar" )) chan->scan_type.sign ='s' ;I think what Lars was suggesting was a change to the IIO side of this rather than just changing the device tree binding.
I'm not so sure. His observation was just that the typical name of these properties is 'label', which I think makes sense and is commonly used.
Naming things is hard.
Perhaps, though, an improvement may be made if we lifted this out of the xadc driver and moved it into the IIO core (I can't imagine that this driver is the only one representing each channel with a separate node in DT).
Quoted 7 lines
Quoted 2 lines
Perhaps an additional per channel attribute called
_labelwhich would allow for board specific labelling?This would seem a bit odd in DT-land; if a user wanted a different label name for their board, they would just rename it in their device tree for that board. Fortunately, with the structured dtsi include files, this is pretty painless to do.
I don't really mind how it is done. From a userspace point of view in IIO though I think these belong in their own attribute rather than in the naming of the attributes. The extended name stuff just makes for really hideous userspace code!
Hence you could use the device tree node naming to provide it, or some other means, but I'd like to have it provided to userspace as say
in_voltage0_label which when read provides the name as a straightforward string.
This would obviously want to be provided by the IIO core.
As for moving the per channel DT node stuff into the IIO core, at this point there are a few drivers doing it, but far from all, so the best bet would be a utility function to help the drivers handle the manipulation rather than direct support in the core.
Jonathan
Quoted 3 lines
Josh
Jonathan Cameron
maintainerPATCH 2/2 iio: adc: xilinx-xadc: Add xlnx,extend-name as an optional argument for aux channels
In reply to Lars-Peter ClausenQuoted 8 lines · Lars-Peter Clausen
Quoted 2 lines · Xander Huff
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom extended name for defined channels.
Hi,
'extend-name' is kind of a IIO specific term. I think a better name in the devicetree context would be 'label'. That's used everywhere else when giving a short description string to a node.
Hi Lars,
What do you think of the general idea? I'm unconvinced we want to do this in channel naming. Perhaps we want an additional attribute giving access to this sort of 'semantic' information?
PATCH 2/2 iio: adc: xilinx-xadc: Add xlnx,extend-name as an optional argument for aux channels
Quoted 5 lines · Jonathan Cameron
Quoted 4 lines · Lars-Peter Clausen
Quoted 2 lines · Xander Huff
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom extended name for defined channels.
Quoted 8 lines
Quoted 4 lines
Hi,
'extend-name' is kind of a IIO specific term. I think a better name in the devicetree context would be 'label'. That's used everywhere else when giving a short description string to a node.
Hi Lars,
What do you think of the general idea? I'm unconvinced we want to do this in channel naming. Perhaps we want an additional attribute giving access to this sort of 'semantic' information?
I'm not convinced about it either. My gut feeling is that this is not the right approach, but I don't really have any better ideas at the moment.
Xander can you give a short description of how this information will be used by an application? Would having a label property for the channel also work for you (e.g. in_voltage0_label)?
- Lars
PATCH 2/2 iio: adc: xilinx-xadc: Add xlnx,extend-name as an optional argument for aux channels
Quoted 19 lines · Lars-Peter Clausen
Quoted 11 lines · Jonathan Cameron
Quoted 10 lines · Lars-Peter Clausen
Quoted 2 lines · Xander Huff
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom extended name for defined channels.
Hi,
'extend-name' is kind of a IIO specific term. I think a better name in the devicetree context would be 'label'. That's used everywhere else when giving a short description string to a node.
Quoted 6 lines
Hi Lars,
What do you think of the general idea? I'm unconvinced we want to do this in channel naming.
Perhaps we want an additional attribute giving access to this sort of 'semantic' information?
Quoted 9 lines
I'm not convinced about it either. My gut feeling is that this is not the right approach, but I don't really have any better ideas at the moment.
Xander can you give a short description of how this information will be used by an application? Would having a label property for the channel also work for you (e.g.
in_voltage0_label)?- Lars
We use 10 of the optional aux channels available in the XADC and have them wired up to specific power supplies. For example, channel 9 is connected to the current of a 6V power supply and channel 12 is connected to the voltage of that same supply, channel 4 is connected to the current of a 3.3V power supply and channel 5 is connected to its voltage.
Without adding extended names to distinguish these aux channels, the handles available inside /sys/bus/iio/devices/[device] are generic: in_voltage10_raw, in_voltage10_scale, for example. If I added the label 'user5v_v', then these handles would be in_voltage10_user5v_v_raw and in_voltage10_user5v_v_scale. This makes it a lot easier to get started and let the handles describe themselves, rather than having to look up which channels are connected to which things.
As far as using 'label' instead of 'extend-name', that's fine with me. I've already sent out a v2 using 'label' instead.
Signature · 3 lines
Xander Huff Staff Software Engineer National Instruments
Jonathan Cameron
maintainerPATCH 1/2 devicetree: xilinx-xadc: Add optional xlnx, extend-name property
In reply to Xander HuffQuoted 8 lines · Xander Huff
To better facilitate user-mode access to optional aux channels, allow device trees to specify a custom extended name for defined channels.
Signed-off-by Xander Huff <[email protected]>Reviewed-by Joe Hershberger <[email protected]>Reviewed-by Josh Cartwright <[email protected]>--
Natinst-ReviewBoard-ID: 97119
Interesting idea. I'd like to let this sit for a fair while to see what comments it gets. If it makes sense for this driver, it probably makes sense for a lot of others. As such we may well want to have a general binding for this, rather than one for just this part.
Thanks,
Jonathan
Quoted 26 lines
--- Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt | 3 +++ 1 file changed, 3 insertions(+)Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt +3
-0@@ -68,6 +68,8 @@ Optional properties: Note each channel number should only be used at most once. Optional properties: + * xlnx,extend-name: Custom extended name for the + channel. * xlnx,bipolar: If set the channel is used in bipolar mode.@@ -107,6 +109,7 @@ Examples: #size-cells = <0>; channel at 0 { reg = <0>; + xlnx,extend-name = "vin_v"; xlnx,bipolar; }; };