diff mbox

[2/3] dt-bindings: iio: wrapper: add io-channel-unit-converter

Message ID 20180319170246.26830-3-peda@axentia.se (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Rosin March 19, 2018, 5:02 p.m. UTC
Allow linear scaling and modification of the type of an io-channel.

When an ADC channel measures the midpoint of a voltage divider, the
interesting voltage is often the voltage over the full resistance
of the divider. Likewise, measuring the voltage over a resistor is
often a way to get to the current through it.

This binding allows description of such hardware which is external
to the ADC.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../iio/wrapper/io-channel-unit-converter.txt      | 84 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 ++
 2 files changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt

Comments

Jonathan Cameron March 24, 2018, 1:53 p.m. UTC | #1
On Mon, 19 Mar 2018 18:02:45 +0100
Peter Rosin <peda@axentia.se> wrote:

> Allow linear scaling and modification of the type of an io-channel.
> 
> When an ADC channel measures the midpoint of a voltage divider, the
> interesting voltage is often the voltage over the full resistance
> of the divider. Likewise, measuring the voltage over a resistor is
> often a way to get to the current through it.
> 
> This binding allows description of such hardware which is external
> to the ADC.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  .../iio/wrapper/io-channel-unit-converter.txt      | 84 ++++++++++++++++++++++
Hmm. I'm not convinced by the naming really though I can see where you care
coming from as it can effectively use a voltage ADC to measure a current.

Lets see if the devicetree people or anyone else has a suggestion on this.

Could go with AFE as that is how a chip doing this would normally be described.
It's just that here we are doing it in old fashioned resistors...

There are a few unusual elements in here binding wise so definitely looking
for input on the bindings!

Thanks,

Jonathan

>  MAINTAINERS                                        |  6 ++
>  2 files changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> new file mode 100644
> index 000000000000..23af661abe32
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> @@ -0,0 +1,84 @@
> +I/O channel unit converter bindings
> +
> +Allow linear scaling and modification of the type of an io-channel.
I can certainly conceive that we will have simple non linear cases in future
though they get awfully hard to describe so we can tackle that when it
happens.

> +
> +When an ADC channel measures the midpoint of a voltage divider, the
> +interesting voltage is often the voltage over the full resistance
> +of the divider. Likewise, measuring the voltage over a resistor is
> +often a way to get to the current through it.
> +
> +Required properties:
> +- compatible : "io-channel-unit-converter"
> +- io-channels : Channel node of the parent channel.
> +- io-channel-names : Should be "parent".
> +
> +Optional properties:
> +- numerator : The parent channel scale is multiplied by this value (default 1).
> +- denominator : The parent channel scale is divided by this value (default 1).
> +- type : The type of the wrapped channel is modified to this type. The default
> +	 is to use the same type as the parent channel. Recognized types are:
> +		"voltage"
> +		"current"
> +
> +Example 1:
> +The system voltage is circa 12V, but divided down with a 22/200
> +voltage divider to adjust it to the ADC range.
> +
> +SYSV        ADC       GND
> +  +          +         +
> +  |  .-----. | .----.  |
> +  '--| 200 |-+-| 22 |--'
> +     '-----'   '----'
> +
> +sysv {
> +	compatible = "io-channel-unit-converter";
> +	io-channles = <&maxadc 1>;
> +	io-channel-names = "parent";
> +
> +	/* multiply the ADC voltage by 222/22 to get the system voltage */
> +	numerator = <222>; /* 200 + 22 */
> +	denominator = <22>;
> +}
> +
> +&spi {
> +	maxadc: adc@0 {
> +		compatible = "maxim,max1027";
> +	      	reg = <0>;
> +		#io-channel-cells = <1>;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <15 IRQ_TYPE_EDGE_RISING>;
> +		spi-max-frequency = <1000000>;
> +	};
> +};
> +
> +Example 2:
> +The system current is measured by measuring the voltage over a
> +3.3 ohm resistor.
> +
> +sysi {
> +	compatible = "io-channel-unit-converter";
> +	io-channles = <&tiadc 0>;
> +	io-channel-names = "parent";
> +
> +	/* divide the ADC voltage by 33/10 (i.e. 3.3) to get current */
> +	numerator = <10>;
> +	denominator = <33>;
> +	type = "current";
> +}
> +
> +&i2c {
> +	tiadc: adc@48 {
> +		compatible = "ti,ads1015";
> +	      	reg = <0x48>;
> +		#io-channel-cells = <1>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		channel@0 { /* IN0,IN1 differential */
> +			reg = <0>;
> +			ti,gain = <1>;
> +			ti,datarate = <4>;
> +		};
> +	};
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 96e5503bfb60..5dd555c7b1b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6884,6 +6884,12 @@ F:	drivers/staging/iio/
>  F:	include/linux/iio/
>  F:	tools/iio/
>  
> +IIO UNIT CONVERTER
> +M:	Peter Rosin <peda@axentia.se>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> +
>  IKANOS/ADI EAGLE ADSL USB DRIVER
>  M:	Matthieu Castet <castet.matthieu@free.fr>
>  M:	Stanislaw Gruszka <stf_xl@wp.pl>

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jonathan Cameron March 24, 2018, 2:06 p.m. UTC | #2
On Sat, 24 Mar 2018 13:53:19 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 19 Mar 2018 18:02:45 +0100
> Peter Rosin <peda@axentia.se> wrote:
> 
> > Allow linear scaling and modification of the type of an io-channel.
> > 
> > When an ADC channel measures the midpoint of a voltage divider, the
> > interesting voltage is often the voltage over the full resistance
> > of the divider. Likewise, measuring the voltage over a resistor is
> > often a way to get to the current through it.
> > 
> > This binding allows description of such hardware which is external
> > to the ADC.
> > 
> > Signed-off-by: Peter Rosin <peda@axentia.se>
> > ---
> >  .../iio/wrapper/io-channel-unit-converter.txt      | 84 ++++++++++++++++++++++  
> Hmm. I'm not convinced by the naming really though I can see where you care
> coming from as it can effectively use a voltage ADC to measure a current.
> 
> Lets see if the devicetree people or anyone else has a suggestion on this.
> 
> Could go with AFE as that is how a chip doing this would normally be described.
> It's just that here we are doing it in old fashioned resistors...
> 
> There are a few unusual elements in here binding wise so definitely looking
> for input on the bindings!
> 
> Thanks,
> 
> Jonathan
> 
> >  MAINTAINERS                                        |  6 ++
> >  2 files changed, 90 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> > new file mode 100644
> > index 000000000000..23af661abe32
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> > @@ -0,0 +1,84 @@
> > +I/O channel unit converter bindings
> > +
> > +Allow linear scaling and modification of the type of an io-channel.  
> I can certainly conceive that we will have simple non linear cases in future
> though they get awfully hard to describe so we can tackle that when it
> happens.
> 
> > +
> > +When an ADC channel measures the midpoint of a voltage divider, the
> > +interesting voltage is often the voltage over the full resistance
> > +of the divider. Likewise, measuring the voltage over a resistor is
> > +often a way to get to the current through it.
> > +
> > +Required properties:
> > +- compatible : "io-channel-unit-converter"
> > +- io-channels : Channel node of the parent channel.
> > +- io-channel-names : Should be "parent".
> > +
> > +Optional properties:
> > +- numerator : The parent channel scale is multiplied by this value (default 1).
> > +- denominator : The parent channel scale is divided by this value (default 1).
> > +- type : The type of the wrapped channel is modified to this type. The default
> > +	 is to use the same type as the parent channel. Recognized types are:
> > +		"voltage"
> > +		"current"
> > +
> > +Example 1:
> > +The system voltage is circa 12V, but divided down with a 22/200
> > +voltage divider to adjust it to the ADC range.
> > +
> > +SYSV        ADC       GND
> > +  +          +         +
> > +  |  .-----. | .----.  |
> > +  '--| 200 |-+-| 22 |--'
> > +     '-----'   '----'
> > +
> > +sysv {
> > +	compatible = "io-channel-unit-converter";
> > +	io-channles = <&maxadc 1>;
> > +	io-channel-names = "parent";
> > +
> > +	/* multiply the ADC voltage by 222/22 to get the system voltage */
> > +	numerator = <222>; /* 200 + 22 */
> > +	denominator = <22>;
> > +}
> > +
> > +&spi {
> > +	maxadc: adc@0 {
> > +		compatible = "maxim,max1027";
> > +	      	reg = <0>;
> > +		#io-channel-cells = <1>;
> > +		interrupt-parent = <&gpio5>;
> > +		interrupts = <15 IRQ_TYPE_EDGE_RISING>;
> > +		spi-max-frequency = <1000000>;
> > +	};
> > +};
> > +
> > +Example 2:
> > +The system current is measured by measuring the voltage over a
> > +3.3 ohm resistor.
> > +
> > +sysi {
> > +	compatible = "io-channel-unit-converter";
> > +	io-channles = <&tiadc 0>;
io-channels and same above.

> > +	io-channel-names = "parent";
> > +
> > +	/* divide the ADC voltage by 33/10 (i.e. 3.3) to get current */
> > +	numerator = <10>;
> > +	denominator = <33>;
> > +	type = "current";
> > +}
> > +
> > +&i2c {
> > +	tiadc: adc@48 {
> > +		compatible = "ti,ads1015";
> > +	      	reg = <0x48>;
> > +		#io-channel-cells = <1>;
> > +
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		channel@0 { /* IN0,IN1 differential */
> > +			reg = <0>;
> > +			ti,gain = <1>;
> > +			ti,datarate = <4>;
> > +		};
> > +	};
> > +};
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 96e5503bfb60..5dd555c7b1b0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6884,6 +6884,12 @@ F:	drivers/staging/iio/
> >  F:	include/linux/iio/
> >  F:	tools/iio/
> >  
> > +IIO UNIT CONVERTER
> > +M:	Peter Rosin <peda@axentia.se>
> > +L:	linux-iio@vger.kernel.org
> > +S:	Maintained
> > +F:	Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> > +
> >  IKANOS/ADI EAGLE ADSL USB DRIVER
> >  M:	Matthieu Castet <castet.matthieu@free.fr>
> >  M:	Stanislaw Gruszka <stf_xl@wp.pl>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) March 26, 2018, 10:23 p.m. UTC | #3
On Mon, Mar 19, 2018 at 06:02:45PM +0100, Peter Rosin wrote:
> Allow linear scaling and modification of the type of an io-channel.
> 
> When an ADC channel measures the midpoint of a voltage divider, the
> interesting voltage is often the voltage over the full resistance
> of the divider. Likewise, measuring the voltage over a resistor is
> often a way to get to the current through it.
> 
> This binding allows description of such hardware which is external
> to the ADC.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  .../iio/wrapper/io-channel-unit-converter.txt      | 84 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 ++
>  2 files changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> new file mode 100644
> index 000000000000..23af661abe32
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> @@ -0,0 +1,84 @@
> +I/O channel unit converter bindings
> +
> +Allow linear scaling and modification of the type of an io-channel.
> +
> +When an ADC channel measures the midpoint of a voltage divider, the
> +interesting voltage is often the voltage over the full resistance
> +of the divider. Likewise, measuring the voltage over a resistor is
> +often a way to get to the current through it.
> +
> +Required properties:
> +- compatible : "io-channel-unit-converter"

Would this apply to something besides ADCs?

> +- io-channels : Channel node of the parent channel.
> +- io-channel-names : Should be "parent".
> +
> +Optional properties:
> +- numerator : The parent channel scale is multiplied by this value (default 1).
> +- denominator : The parent channel scale is divided by this value (default 1).
> +- type : The type of the wrapped channel is modified to this type. The default
> +	 is to use the same type as the parent channel. Recognized types are:
> +		"voltage"
> +		"current"

This seems overly complicated for just describing a couple of resistors 
on an ADC. OTOH, keeping the ADC node and what's attached to the ADC 
separate 

Perhaps the type should be part of the compatible. For example, if you 
have a current measurement resistor/circuit, then the compatible should 
be based on that.

> +
> +Example 1:
> +The system voltage is circa 12V, but divided down with a 22/200
> +voltage divider to adjust it to the ADC range.
> +
> +SYSV        ADC       GND
> +  +          +         +
> +  |  .-----. | .----.  |
> +  '--| 200 |-+-| 22 |--'
> +     '-----'   '----'
> +
> +sysv {
> +	compatible = "io-channel-unit-converter";
> +	io-channles = <&maxadc 1>;
> +	io-channel-names = "parent";

parent doesn't seem to describe something about the h/w.

> +
> +	/* multiply the ADC voltage by 222/22 to get the system voltage */
> +	numerator = <222>; /* 200 + 22 */
> +	denominator = <22>;
> +}
> +
> +&spi {
> +	maxadc: adc@0 {
> +		compatible = "maxim,max1027";
> +	      	reg = <0>;
> +		#io-channel-cells = <1>;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <15 IRQ_TYPE_EDGE_RISING>;
> +		spi-max-frequency = <1000000>;
> +	};
> +};
> +
> +Example 2:
> +The system current is measured by measuring the voltage over a
> +3.3 ohm resistor.
> +
> +sysi {
> +	compatible = "io-channel-unit-converter";
> +	io-channles = <&tiadc 0>;
> +	io-channel-names = "parent";
> +
> +	/* divide the ADC voltage by 33/10 (i.e. 3.3) to get current */
> +	numerator = <10>;
> +	denominator = <33>;
> +	type = "current";
> +}
> +
> +&i2c {
> +	tiadc: adc@48 {
> +		compatible = "ti,ads1015";
> +	      	reg = <0x48>;
> +		#io-channel-cells = <1>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		channel@0 { /* IN0,IN1 differential */
> +			reg = <0>;
> +			ti,gain = <1>;
> +			ti,datarate = <4>;
> +		};
> +	};
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 96e5503bfb60..5dd555c7b1b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6884,6 +6884,12 @@ F:	drivers/staging/iio/
>  F:	include/linux/iio/
>  F:	tools/iio/
>  
> +IIO UNIT CONVERTER
> +M:	Peter Rosin <peda@axentia.se>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
> +
>  IKANOS/ADI EAGLE ADSL USB DRIVER
>  M:	Matthieu Castet <castet.matthieu@free.fr>
>  M:	Stanislaw Gruszka <stf_xl@wp.pl>
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Rosin March 27, 2018, 8:01 a.m. UTC | #4
On 2018-03-27 00:23, Rob Herring wrote:
> On Mon, Mar 19, 2018 at 06:02:45PM +0100, Peter Rosin wrote:
>> Allow linear scaling and modification of the type of an io-channel.
>>
>> When an ADC channel measures the midpoint of a voltage divider, the
>> interesting voltage is often the voltage over the full resistance
>> of the divider. Likewise, measuring the voltage over a resistor is
>> often a way to get to the current through it.
>>
>> This binding allows description of such hardware which is external
>> to the ADC.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>  .../iio/wrapper/io-channel-unit-converter.txt      | 84 ++++++++++++++++++++++
>>  MAINTAINERS                                        |  6 ++
>>  2 files changed, 90 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>> new file mode 100644
>> index 000000000000..23af661abe32
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>> @@ -0,0 +1,84 @@
>> +I/O channel unit converter bindings
>> +
>> +Allow linear scaling and modification of the type of an io-channel.
>> +
>> +When an ADC channel measures the midpoint of a voltage divider, the
>> +interesting voltage is often the voltage over the full resistance
>> +of the divider. Likewise, measuring the voltage over a resistor is
>> +often a way to get to the current through it.
>> +
>> +Required properties:
>> +- compatible : "io-channel-unit-converter"
> 
> Would this apply to something besides ADCs?

Not that I can think of. At the moment.

>> +- io-channels : Channel node of the parent channel.
>> +- io-channel-names : Should be "parent".
>> +
>> +Optional properties:
>> +- numerator : The parent channel scale is multiplied by this value (default 1).
>> +- denominator : The parent channel scale is divided by this value (default 1).
>> +- type : The type of the wrapped channel is modified to this type. The default
>> +	 is to use the same type as the parent channel. Recognized types are:
>> +		"voltage"
>> +		"current"
> 
> This seems overly complicated for just describing a couple of resistors 
> on an ADC. OTOH, keeping the ADC node and what's attached to the ADC 
> separate 
> 
> Perhaps the type should be part of the compatible. For example, if you 
> have a current measurement resistor/circuit, then the compatible should 
> be based on that.

Is a compatible like "current-sense-circuit" too long(ish)? Is a
vendor prefix absolutely required? Sure, I can use "axentia,"
because we "implemented" the circuit this time, but it seems like
the list can grow very long if we should add every vendor that
may use something like this? Something more generic would be good
for something as simple as this IMHO.

I propose the compatible "voltage-divider" for the other example,
but I have the same issue with the vendor prefix. Even more so
in this case.

Then the above type can be inferred from the compatible. I suppose
I should write the above two bindings as two separate files?

>> +
>> +Example 1:
>> +The system voltage is circa 12V, but divided down with a 22/200
>> +voltage divider to adjust it to the ADC range.
>> +
>> +SYSV        ADC       GND
>> +  +          +         +
>> +  |  .-----. | .----.  |
>> +  '--| 200 |-+-| 22 |--'
>> +     '-----'   '----'
>> +
>> +sysv {
>> +	compatible = "io-channel-unit-converter";
>> +	io-channles = <&maxadc 1>;
>> +	io-channel-names = "parent";
> 
> parent doesn't seem to describe something about the h/w.

Extrapolating from your first question, you are thinking "adc", right?

Cheers,
Peter

>> +
>> +	/* multiply the ADC voltage by 222/22 to get the system voltage */
>> +	numerator = <222>; /* 200 + 22 */
>> +	denominator = <22>;
>> +}
>> +
>> +&spi {
>> +	maxadc: adc@0 {
>> +		compatible = "maxim,max1027";
>> +	      	reg = <0>;
>> +		#io-channel-cells = <1>;
>> +		interrupt-parent = <&gpio5>;
>> +		interrupts = <15 IRQ_TYPE_EDGE_RISING>;
>> +		spi-max-frequency = <1000000>;
>> +	};
>> +};
>> +
>> +Example 2:
>> +The system current is measured by measuring the voltage over a
>> +3.3 ohm resistor.
>> +
>> +sysi {
>> +	compatible = "io-channel-unit-converter";
>> +	io-channles = <&tiadc 0>;
>> +	io-channel-names = "parent";
>> +
>> +	/* divide the ADC voltage by 33/10 (i.e. 3.3) to get current */
>> +	numerator = <10>;
>> +	denominator = <33>;
>> +	type = "current";
>> +}
>> +
>> +&i2c {
>> +	tiadc: adc@48 {
>> +		compatible = "ti,ads1015";
>> +	      	reg = <0x48>;
>> +		#io-channel-cells = <1>;
>> +
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		channel@0 { /* IN0,IN1 differential */
>> +			reg = <0>;
>> +			ti,gain = <1>;
>> +			ti,datarate = <4>;
>> +		};
>> +	};
>> +};
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 96e5503bfb60..5dd555c7b1b0 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -6884,6 +6884,12 @@ F:	drivers/staging/iio/
>>  F:	include/linux/iio/
>>  F:	tools/iio/
>>  
>> +IIO UNIT CONVERTER
>> +M:	Peter Rosin <peda@axentia.se>
>> +L:	linux-iio@vger.kernel.org
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>> +
>>  IKANOS/ADI EAGLE ADSL USB DRIVER
>>  M:	Matthieu Castet <castet.matthieu@free.fr>
>>  M:	Stanislaw Gruszka <stf_xl@wp.pl>
>> -- 
>> 2.11.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Phil Reid March 28, 2018, 2:29 a.m. UTC | #5
On 27/03/2018 16:01, Peter Rosin wrote:
> On 2018-03-27 00:23, Rob Herring wrote:
>> On Mon, Mar 19, 2018 at 06:02:45PM +0100, Peter Rosin wrote:
>>> Allow linear scaling and modification of the type of an io-channel.
>>>
>>> When an ADC channel measures the midpoint of a voltage divider, the
>>> interesting voltage is often the voltage over the full resistance
>>> of the divider. Likewise, measuring the voltage over a resistor is
>>> often a way to get to the current through it.
>>>
>>> This binding allows description of such hardware which is external
>>> to the ADC.
>>>

*snip*

>>> +++ b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>> @@ -0,0 +1,84 @@
>>> +I/O channel unit converter bindings
>>> +
>>> +Allow linear scaling and modification of the type of an io-channel.
>>> +
>>> +When an ADC channel measures the midpoint of a voltage divider, the
>>> +interesting voltage is often the voltage over the full resistance
>>> +of the divider. Likewise, measuring the voltage over a resistor is
>>> +often a way to get to the current through it.
>>> +
>>> +Required properties:
>>> +- compatible : "io-channel-unit-converter"
>> Would this apply to something besides ADCs?
> Not that I can think of. At the moment.
> 

I like the concept. I can think of use case on my end to set a RADC (digital pot)
to set a threshold voltage. Being able to define the hardware scaling in the dt would be nice.
Which would allow all the hardware definition to be in the dt which would nice.
So this would be voltage -> resistance.

Setting a DAC voltage to set output current is also a distinct possibility.
Rob Herring (Arm) March 29, 2018, 1:55 p.m. UTC | #6
On Tue, Mar 27, 2018 at 3:01 AM, Peter Rosin <peda@axentia.se> wrote:
> On 2018-03-27 00:23, Rob Herring wrote:
>> On Mon, Mar 19, 2018 at 06:02:45PM +0100, Peter Rosin wrote:
>>> Allow linear scaling and modification of the type of an io-channel.
>>>
>>> When an ADC channel measures the midpoint of a voltage divider, the
>>> interesting voltage is often the voltage over the full resistance
>>> of the divider. Likewise, measuring the voltage over a resistor is
>>> often a way to get to the current through it.
>>>
>>> This binding allows description of such hardware which is external
>>> to the ADC.
>>>
>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>> ---
>>>  .../iio/wrapper/io-channel-unit-converter.txt      | 84 ++++++++++++++++++++++
>>>  MAINTAINERS                                        |  6 ++
>>>  2 files changed, 90 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>> new file mode 100644
>>> index 000000000000..23af661abe32
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>> @@ -0,0 +1,84 @@
>>> +I/O channel unit converter bindings
>>> +
>>> +Allow linear scaling and modification of the type of an io-channel.
>>> +
>>> +When an ADC channel measures the midpoint of a voltage divider, the
>>> +interesting voltage is often the voltage over the full resistance
>>> +of the divider. Likewise, measuring the voltage over a resistor is
>>> +often a way to get to the current through it.
>>> +
>>> +Required properties:
>>> +- compatible : "io-channel-unit-converter"
>>
>> Would this apply to something besides ADCs?
>
> Not that I can think of. At the moment.
>
>>> +- io-channels : Channel node of the parent channel.
>>> +- io-channel-names : Should be "parent".
>>> +
>>> +Optional properties:
>>> +- numerator : The parent channel scale is multiplied by this value (default 1).
>>> +- denominator : The parent channel scale is divided by this value (default 1).
>>> +- type : The type of the wrapped channel is modified to this type. The default
>>> +     is to use the same type as the parent channel. Recognized types are:
>>> +            "voltage"
>>> +            "current"
>>
>> This seems overly complicated for just describing a couple of resistors
>> on an ADC. OTOH, keeping the ADC node and what's attached to the ADC
>> separate
>>
>> Perhaps the type should be part of the compatible. For example, if you
>> have a current measurement resistor/circuit, then the compatible should
>> be based on that.
>
> Is a compatible like "current-sense-circuit" too long(ish)?

No, that's fine.

> Is a vendor prefix absolutely required?

No, it can be common.

> Sure, I can use "axentia,"
> because we "implemented" the circuit this time, but it seems like
> the list can grow very long if we should add every vendor that
> may use something like this? Something more generic would be good
> for something as simple as this IMHO.
>
> I propose the compatible "voltage-divider" for the other example,
> but I have the same issue with the vendor prefix. Even more so
> in this case.

"voltage-divider" doesn't really say what the purpose is.
>
> Then the above type can be inferred from the compatible. I suppose
> I should write the above two bindings as two separate files?

One is fine if the only difference is the compatible.

>>> +
>>> +Example 1:
>>> +The system voltage is circa 12V, but divided down with a 22/200
>>> +voltage divider to adjust it to the ADC range.
>>> +
>>> +SYSV        ADC       GND
>>> +  +          +         +
>>> +  |  .-----. | .----.  |
>>> +  '--| 200 |-+-| 22 |--'
>>> +     '-----'   '----'
>>> +
>>> +sysv {
>>> +    compatible = "io-channel-unit-converter";
>>> +    io-channles = <&maxadc 1>;
>>> +    io-channel-names = "parent";
>>
>> parent doesn't seem to describe something about the h/w.
>
> Extrapolating from your first question, you are thinking "adc", right?

I don't really have any suggestion. With only 1 channel, there's not
really much point to having a name at all.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Rosin March 30, 2018, 10:38 p.m. UTC | #7
On 2018-03-29 15:55, Rob Herring wrote:
> On Tue, Mar 27, 2018 at 3:01 AM, Peter Rosin <peda@axentia.se> wrote:
>> On 2018-03-27 00:23, Rob Herring wrote:
>>> On Mon, Mar 19, 2018 at 06:02:45PM +0100, Peter Rosin wrote:
>>>> Allow linear scaling and modification of the type of an io-channel.
>>>>
>>>> When an ADC channel measures the midpoint of a voltage divider, the
>>>> interesting voltage is often the voltage over the full resistance
>>>> of the divider. Likewise, measuring the voltage over a resistor is
>>>> often a way to get to the current through it.
>>>>
>>>> This binding allows description of such hardware which is external
>>>> to the ADC.
>>>>
>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>> ---
>>>>  .../iio/wrapper/io-channel-unit-converter.txt      | 84 ++++++++++++++++++++++
>>>>  MAINTAINERS                                        |  6 ++
>>>>  2 files changed, 90 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>>> new file mode 100644
>>>> index 000000000000..23af661abe32
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>>> @@ -0,0 +1,84 @@
>>>> +I/O channel unit converter bindings
>>>> +
>>>> +Allow linear scaling and modification of the type of an io-channel.
>>>> +
>>>> +When an ADC channel measures the midpoint of a voltage divider, the
>>>> +interesting voltage is often the voltage over the full resistance
>>>> +of the divider. Likewise, measuring the voltage over a resistor is
>>>> +often a way to get to the current through it.
>>>> +
>>>> +Required properties:
>>>> +- compatible : "io-channel-unit-converter"
>>>
>>> Would this apply to something besides ADCs?
>>
>> Not that I can think of. At the moment.
>>
>>>> +- io-channels : Channel node of the parent channel.
>>>> +- io-channel-names : Should be "parent".
>>>> +
>>>> +Optional properties:
>>>> +- numerator : The parent channel scale is multiplied by this value (default 1).
>>>> +- denominator : The parent channel scale is divided by this value (default 1).
>>>> +- type : The type of the wrapped channel is modified to this type. The default
>>>> +     is to use the same type as the parent channel. Recognized types are:
>>>> +            "voltage"
>>>> +            "current"
>>>
>>> This seems overly complicated for just describing a couple of resistors
>>> on an ADC. OTOH, keeping the ADC node and what's attached to the ADC
>>> separate
>>>
>>> Perhaps the type should be part of the compatible. For example, if you
>>> have a current measurement resistor/circuit, then the compatible should
>>> be based on that.
>>
>> Is a compatible like "current-sense-circuit" too long(ish)?
> 
> No, that's fine.
> 
>> Is a vendor prefix absolutely required?
> 
> No, it can be common.
> 
>> Sure, I can use "axentia,"
>> because we "implemented" the circuit this time, but it seems like
>> the list can grow very long if we should add every vendor that
>> may use something like this? Something more generic would be good
>> for something as simple as this IMHO.
>>
>> I propose the compatible "voltage-divider" for the other example,
>> but I have the same issue with the vendor prefix. Even more so
>> in this case.
> 
> "voltage-divider" doesn't really say what the purpose is.

No, but it says what it *is*, which is pretty much what the compatible
is all about? If you have, I don't know, let's pick some regulator, you
don't expect the compatible to give any details of the purpose. Not
other than regulating of course. But a voltage divider pretty much divides
a voltage, much like a regulator regulates, and that's it. The node name
should be what hints at the purpose, no?

>>
>> Then the above type can be inferred from the compatible. I suppose
>> I should write the above two bindings as two separate files?
> 
> One is fine if the only difference is the compatible.
> 
>>>> +
>>>> +Example 1:
>>>> +The system voltage is circa 12V, but divided down with a 22/200
>>>> +voltage divider to adjust it to the ADC range.
>>>> +
>>>> +SYSV        ADC       GND
>>>> +  +          +         +
>>>> +  |  .-----. | .----.  |
>>>> +  '--| 200 |-+-| 22 |--'
>>>> +     '-----'   '----'
>>>> +
>>>> +sysv {
>>>> +    compatible = "io-channel-unit-converter";
>>>> +    io-channles = <&maxadc 1>;
>>>> +    io-channel-names = "parent";
>>>
>>> parent doesn't seem to describe something about the h/w.
>>
>> Extrapolating from your first question, you are thinking "adc", right?
> 
> I don't really have any suggestion. With only 1 channel, there's not
> really much point to having a name at all.

Right, I'm ditching the -names line.

Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
new file mode 100644
index 000000000000..23af661abe32
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
@@ -0,0 +1,84 @@ 
+I/O channel unit converter bindings
+
+Allow linear scaling and modification of the type of an io-channel.
+
+When an ADC channel measures the midpoint of a voltage divider, the
+interesting voltage is often the voltage over the full resistance
+of the divider. Likewise, measuring the voltage over a resistor is
+often a way to get to the current through it.
+
+Required properties:
+- compatible : "io-channel-unit-converter"
+- io-channels : Channel node of the parent channel.
+- io-channel-names : Should be "parent".
+
+Optional properties:
+- numerator : The parent channel scale is multiplied by this value (default 1).
+- denominator : The parent channel scale is divided by this value (default 1).
+- type : The type of the wrapped channel is modified to this type. The default
+	 is to use the same type as the parent channel. Recognized types are:
+		"voltage"
+		"current"
+
+Example 1:
+The system voltage is circa 12V, but divided down with a 22/200
+voltage divider to adjust it to the ADC range.
+
+SYSV        ADC       GND
+  +          +         +
+  |  .-----. | .----.  |
+  '--| 200 |-+-| 22 |--'
+     '-----'   '----'
+
+sysv {
+	compatible = "io-channel-unit-converter";
+	io-channles = <&maxadc 1>;
+	io-channel-names = "parent";
+
+	/* multiply the ADC voltage by 222/22 to get the system voltage */
+	numerator = <222>; /* 200 + 22 */
+	denominator = <22>;
+}
+
+&spi {
+	maxadc: adc@0 {
+		compatible = "maxim,max1027";
+	      	reg = <0>;
+		#io-channel-cells = <1>;
+		interrupt-parent = <&gpio5>;
+		interrupts = <15 IRQ_TYPE_EDGE_RISING>;
+		spi-max-frequency = <1000000>;
+	};
+};
+
+Example 2:
+The system current is measured by measuring the voltage over a
+3.3 ohm resistor.
+
+sysi {
+	compatible = "io-channel-unit-converter";
+	io-channles = <&tiadc 0>;
+	io-channel-names = "parent";
+
+	/* divide the ADC voltage by 33/10 (i.e. 3.3) to get current */
+	numerator = <10>;
+	denominator = <33>;
+	type = "current";
+}
+
+&i2c {
+	tiadc: adc@48 {
+		compatible = "ti,ads1015";
+	      	reg = <0x48>;
+		#io-channel-cells = <1>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		channel@0 { /* IN0,IN1 differential */
+			reg = <0>;
+			ti,gain = <1>;
+			ti,datarate = <4>;
+		};
+	};
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 96e5503bfb60..5dd555c7b1b0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6884,6 +6884,12 @@  F:	drivers/staging/iio/
 F:	include/linux/iio/
 F:	tools/iio/
 
+IIO UNIT CONVERTER
+M:	Peter Rosin <peda@axentia.se>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
+
 IKANOS/ADI EAGLE ADSL USB DRIVER
 M:	Matthieu Castet <castet.matthieu@free.fr>
 M:	Stanislaw Gruszka <stf_xl@wp.pl>