2017-07-16 23:34:28

by Stefan Brüns

[permalink] [raw]
Subject: [PATCH 2/2] iio: Documentation: Add ina2xx shunt_resistor attribute documentation

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 <[email protected]>
---
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: [email protected]
+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.
--
2.13.2


2017-07-17 20:54:03

by Jonathan Cameron

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

On Mon, 17 Jul 2017 01:34:10 +0200
Stefan Brüns <[email protected]> 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 <[email protected]>
This has me confused. The documentation is already
there and lists it as being in ohms.

Ah, subtle filename difference. Docs in are
Documentation/ABI/testing/sysfs-bus-iio-ina2xx-adc

This attribute effectively exists for other parts
as well now.

Jonathan
> ---
> 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: [email protected]
> +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.

2017-07-17 21:44:23

by Stefan Brüns

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

On Montag, 17. Juli 2017 22:53:57 CEST Jonathan Cameron wrote:
> On Mon, 17 Jul 2017 01:34:10 +0200
>
> Stefan Br?ns <[email protected]> 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 <[email protected]>
>
> 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.

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?

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.

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: [email protected]
> > +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.

2017-07-18 10:23:45

by Jonathan Cameron

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

On Mon, 17 Jul 2017 21:44:19 +0000
"Br?ns, Stefan" <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >
> > 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: [email protected]
> > > +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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html