Re: [PATCH v3 1/2] dt-bindings: iio: afe: add current-sense-shunt and voltage-divider

[Jonathan Cameron]

On Mon, 16 Apr 2018 16:00:44 +0200
Peter Rosin  wrote:

> On 2018-04-10 17:28, Peter Rosin wrote:
> > +Example:
> > +The system voltage is circa 12V, but divided down with a 22/200
> > +voltage divider to adjust it to the ADC range.
> > +
> > +SYSVADC   GND
> > +  +  + +
> > +  |  .-. | ..  |
> > +  '--| 200 |-+-| 22 |--'
> > + '-'   ''
> > +
> > +sysv {
> > +   compatible = "voltage-divider";
> > +   io-channels = < 1>;
> > +
> > +   /* Multiply the ADC voltage by 222/22 to get the system voltage. */
> > +   numerator = <222>; /* 200 + 22 */
> > +   denominator = <22>;
> > +};  
> 
> While I already got a reviewed-by from Rob, and maybe I shouldn't be
> stirring the pot, but I had an umpteenth look and I now think this
> one looks a bit odd. It shows a bit that it originates from when the
> compatible was the very generic "io-channel-unit-converter" in v1
> of the series. What I mean is that a voltage divider presumable always
> gets you a lower voltage. Therefore, one would assume that the
> denominator should be larger than the numerator. The fact that this
> translates into the inverted fraction when calculating backwards
> through the voltage divider should probably not affect the binding.
> 
> So, in the above example, I think it would make more sense to have
> 
>   numerator = <22>;
>   denominator = <222>; /* 200 + 22 */
> 
> (and then, naturally, adjust the driver to invert the fraction)
> 
> Comments?
Agreed - it is odd as we currently have it.  I wouldn't have
a particular problem with renaming this compatible as voltage
scaler (voltage multiplier has a specific meaning we should avoid
stepping on!)  Anyhow, either way is fine with me, but I agree
the current version is odd.

Jonathan

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

Re: [PATCH v3 1/2] dt-bindings: iio: afe: add current-sense-shunt and voltage-divider

[Jonathan Cameron]

On Mon, 16 Apr 2018 16:00:44 +0200
Peter Rosin  wrote:

> On 2018-04-10 17:28, Peter Rosin wrote:
> > +Example:
> > +The system voltage is circa 12V, but divided down with a 22/200
> > +voltage divider to adjust it to the ADC range.
> > +
> > +SYSVADC   GND
> > +  +  + +
> > +  |  .-. | ..  |
> > +  '--| 200 |-+-| 22 |--'
> > + '-'   ''
> > +
> > +sysv {
> > +   compatible = "voltage-divider";
> > +   io-channels = < 1>;
> > +
> > +   /* Multiply the ADC voltage by 222/22 to get the system voltage. */
> > +   numerator = <222>; /* 200 + 22 */
> > +   denominator = <22>;
> > +};  
> 
> While I already got a reviewed-by from Rob, and maybe I shouldn't be
> stirring the pot, but I had an umpteenth look and I now think this
> one looks a bit odd. It shows a bit that it originates from when the
> compatible was the very generic "io-channel-unit-converter" in v1
> of the series. What I mean is that a voltage divider presumable always
> gets you a lower voltage. Therefore, one would assume that the
> denominator should be larger than the numerator. The fact that this
> translates into the inverted fraction when calculating backwards
> through the voltage divider should probably not affect the binding.
> 
> So, in the above example, I think it would make more sense to have
> 
>   numerator = <22>;
>   denominator = <222>; /* 200 + 22 */
> 
> (and then, naturally, adjust the driver to invert the fraction)
> 
> Comments?
Agreed - it is odd as we currently have it.  I wouldn't have
a particular problem with renaming this compatible as voltage
scaler (voltage multiplier has a specific meaning we should avoid
stepping on!)  Anyhow, either way is fine with me, but I agree
the current version is odd.

Jonathan

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

Re: [PATCH v3 1/2] dt-bindings: iio: afe: add current-sense-shunt and voltage-divider

[Peter Rosin]

On 2018-04-10 17:28, Peter Rosin wrote:
> +Example:
> +The system voltage is circa 12V, but divided down with a 22/200
> +voltage divider to adjust it to the ADC range.
> +
> +SYSVADC   GND
> +  +  + +
> +  |  .-. | ..  |
> +  '--| 200 |-+-| 22 |--'
> + '-'   ''
> +
> +sysv {
> + compatible = "voltage-divider";
> + io-channels = < 1>;
> +
> + /* Multiply the ADC voltage by 222/22 to get the system voltage. */
> + numerator = <222>; /* 200 + 22 */
> + denominator = <22>;
> +};

While I already got a reviewed-by from Rob, and maybe I shouldn't be
stirring the pot, but I had an umpteenth look and I now think this
one looks a bit odd. It shows a bit that it originates from when the
compatible was the very generic "io-channel-unit-converter" in v1
of the series. What I mean is that a voltage divider presumable always
gets you a lower voltage. Therefore, one would assume that the
denominator should be larger than the numerator. The fact that this
translates into the inverted fraction when calculating backwards
through the voltage divider should probably not affect the binding.

So, in the above example, I think it would make more sense to have

numerator = <22>;
denominator = <222>; /* 200 + 22 */

(and then, naturally, adjust the driver to invert the fraction)

Comments?

Cheers,
Peter

Re: [PATCH v3 1/2] dt-bindings: iio: afe: add current-sense-shunt and voltage-divider

[Peter Rosin]

On 2018-04-10 17:28, Peter Rosin wrote:
> +Example:
> +The system voltage is circa 12V, but divided down with a 22/200
> +voltage divider to adjust it to the ADC range.
> +
> +SYSVADC   GND
> +  +  + +
> +  |  .-. | ..  |
> +  '--| 200 |-+-| 22 |--'
> + '-'   ''
> +
> +sysv {
> + compatible = "voltage-divider";
> + io-channels = < 1>;
> +
> + /* Multiply the ADC voltage by 222/22 to get the system voltage. */
> + numerator = <222>; /* 200 + 22 */
> + denominator = <22>;
> +};

While I already got a reviewed-by from Rob, and maybe I shouldn't be
stirring the pot, but I had an umpteenth look and I now think this
one looks a bit odd. It shows a bit that it originates from when the
compatible was the very generic "io-channel-unit-converter" in v1
of the series. What I mean is that a voltage divider presumable always
gets you a lower voltage. Therefore, one would assume that the
denominator should be larger than the numerator. The fact that this
translates into the inverted fraction when calculating backwards
through the voltage divider should probably not affect the binding.

So, in the above example, I think it would make more sense to have

numerator = <22>;
denominator = <222>; /* 200 + 22 */

(and then, naturally, adjust the driver to invert the fraction)

Comments?

Cheers,
Peter

Re: [PATCH v3 1/2] dt-bindings: iio: afe: add current-sense-shunt and voltage-divider

[Rob Herring]

On Tue, Apr 10, 2018 at 05:28:01PM +0200, Peter Rosin wrote:
> An ADC is often used to measure other quantities indirectly. These
> bindings describe two cases, a current through a shunt resistor, and
> a "big" voltage measured with the help of a voltage divider.
> 
> Signed-off-by: Peter Rosin 
> ---
>  .../bindings/iio/afe/current-sense-shunt.txt   | 41 
>  .../bindings/iio/afe/voltage-divider.txt   | 45 
> ++
>  MAINTAINERS|  7 
>  3 files changed, 93 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/iio/afe/current-sense-shunt.txt
>  create mode 100644 
> Documentation/devicetree/bindings/iio/afe/voltage-divider.txt

Reviewed-by: Rob Herring 

Re: [PATCH v3 1/2] dt-bindings: iio: afe: add current-sense-shunt and voltage-divider

[Rob Herring]

On Tue, Apr 10, 2018 at 05:28:01PM +0200, Peter Rosin wrote:
> An ADC is often used to measure other quantities indirectly. These
> bindings describe two cases, a current through a shunt resistor, and
> a "big" voltage measured with the help of a voltage divider.
> 
> Signed-off-by: Peter Rosin 
> ---
>  .../bindings/iio/afe/current-sense-shunt.txt   | 41 
>  .../bindings/iio/afe/voltage-divider.txt   | 45 
> ++
>  MAINTAINERS|  7 
>  3 files changed, 93 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/iio/afe/current-sense-shunt.txt
>  create mode 100644 
> Documentation/devicetree/bindings/iio/afe/voltage-divider.txt

Reviewed-by: Rob Herring