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

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 16 Apr 2018 16:00:44 +0200
Peter Rosin <peda@axentia.se> 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.
> > +
> > +SYSV        ADC       GND
> > +  +          +         +
> > +  |  .-----. | .----.  |
> > +  '--| 200 |-+-| 22 |--'
> > +     '-----'   '----'
> > +
> > +sysv {
> > +	compatible = "voltage-divider";
> > +	io-channels = <&maxadc 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html