Re: [PATCH 2/2] iio: Documentation: Add ina2xx shunt_resistor attribute documentation

[Jonathan Cameron]

On Mon, 17 Jul 2017 21:44:19 +0000
"Brüns, Stefan" <Stefan.Bruens@xxxxxxxxxxxxxx> wrote:

> On Montag, 17. Juli 2017 22:53:57 CEST Jonathan Cameron wrote:
> > On Mon, 17 Jul 2017 01:34:10 +0200
> > 
> > Stefan Brüns <stefan.bruens@xxxxxxxxxxxxxx> wrote:  
> > > The ina2xx driver appeared in the Linux kernel version 4.5, but provided
> > > no documentation. Contrary to other uses of resistance in IIO, ina2xx uses
> > > microohms instead of ohms in the sysfs attribute.
> > > 
> > > Signed-off-by: Stefan Brüns <stefan.bruens@xxxxxxxxxxxxxx>  
> > 
> > This has me confused.  The documentation is already
> > there and lists it as being in ohms.  
> 
> Which obviously, from reading the source code, is wrong - it uses microohms.
Hmm. Not good
> 
> Also, the current Description contains IMHO irrelevant stuff:
> "The value of the shunt resistor may be known only at runtime fom an
>  eeprom content read by a client application."
> 
> > Ah, subtle filename difference.  Docs in are
> > Documentation/ABI/testing/sysfs-bus-iio-ina2xx-adc  
> 
> Well spotted ...
> 
> > This attribute effectively exists for other parts
> > as well now.  
> 
> My grep foo failed me here, the only parts I could find are the MAX9611 (which 
> specifies and uses ohms) and the INA2xx.
> 
> adc/qcom-spmi-iadc.c allows to read the RSENSE value from device tree 
> (specified in microohms), but does not expose it via sysfs.
> 
> Can you point me to other devices?
You found it - just the one other part be it with several different shunt
related entries.

Now this is a problem as the ABI has to be consistent.  So one of these
drivers is buggy, and right now I'm thinking it is this one given the docs
are consistent.  The whole 'micro/milli' units was an ill  judged
attempt to try and be consistent with hwmon (we rapidly ran into issues
generalizing but by then were were stuck with a few odd units).

Recent policy has been the basic SI units if at all possible.
> 
> Slightly off-topic: Some attributes in iio are read from device-tree (e.g. 
> shunt resistor values), should the iio documentation point to the dt bindings? 
> Currently, only mount_matrix in sysfs-bus-iio vaguely mentions device-tree.
> 
The ABI docs probably shouldn't as it's an implementation detail.
However, if it's particularly useful for some reason I don't see any
real problem with doing so.

Jonathan

> Kind regards,
> 
> Stefan
> 
> > > ---
> > > 
> > >  Documentation/ABI/testing/sysfs-bus-iio-adc-ina2xx-adc | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ina2xx-adc
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ina2xx-adc
> > > b/Documentation/ABI/testing/sysfs-bus-iio-adc-ina2xx-adc new file mode
> > > 100644
> > > index 000000000000..9525098f6822
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ina2xx-adc
> > > @@ -0,0 +1,7 @@
> > > +What:		/sys/bus/iio/devices/iio:deviceX/in_shunt_resistor
> > > +Date:		December 2015
> > > +KernelVersion:	4.5
> > > +Contact:	linux-iio@xxxxxxxxxxxxxxx
> > > +Description: 	The value of the shunt resistor connected to the V_IN+   
> and
> > > V_IN- +		inputs, used to compute the current flowing through the shunt
> > > and +		the corresponding power. In microohms.  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html