Date:   Mon, 9 Apr 2018 13:12:16 +0200
From:   Peter Rosin <peda@...ntia.se>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     linux-kernel@...r.kernel.org, Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        "David S. Miller" <davem@...emloft.net>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        Michael Hennerich <michael.hennerich@...log.com>,
        Phil Reid <preid@...ctromag.com.au>, linux-iio@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: afe: add current-sense-cuicuit
 and voltage-divider

On 2018-04-08 18:50, Jonathan Cameron wrote:
> On Tue,  3 Apr 2018 17:36:34 +0200
> Peter Rosin <peda@...ntia.se> wrote:
> 
> circuit in the patch title is spelled wrong.
> 
>> An ADC is often used to measure other quantities indirectly. These
>> bindings describe two cases, a current through a sense resistor, and
>> a "big" voltage measured with the help of a voltage divider.
>>
>> Signed-off-by: Peter Rosin <peda@...ntia.se>
> 
> Will definitely be wanting wide opinions on this one - Rob in particularly
> for the binding side.
> 
> One comment inline. What we have here is nice and generic, but is
> it what would be 'expected' for current sense circuit?  Should we
> also be more specific in the naming.  There are lots of options for
> current sense circuits and this is just the simplest (current loop
> for example).
> 
> One option would be to use current-sense-shunt perhaps?

Heh, I'm obviously happy to not use anything "circuit", because I fail
to type it correctly most of the time. I don't know how many instances
I fixed before sending and there were still a few lingering. Anyway,
I have fixed those locally and I have also locally removed the error
message on registration failure in patch 2/2 so that will be in v3.

But I'm holding off that v3, since I expect more input on the naming.

> https://en.wikipedia.org/wiki/Current_sensing_techniques
> Gives a few that I didn't think of beyond current loops etc.

I'll give it a read.

Cheers,
Peter

>> ---
>>  .../bindings/iio/afe/current-sense-circuit.txt     | 45 ++++++++++++++++++++++
>>  .../bindings/iio/afe/voltage-divider.txt           | 45 ++++++++++++++++++++++
>>  MAINTAINERS                                        |  7 ++++
>>  3 files changed, 97 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt
>>  create mode 100644 Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt b/Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt
>> new file mode 100644
>> index 000000000000..0bc7d89387c0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt
>> @@ -0,0 +1,45 @@
>> +Current Sense Curcuit
>> +=====================
>> +
>> +When an io-channel measures the voltage over a current sense resistor,
>> +the interesting mesaurement is often the current through the resistor,
>> +not the voltage over it. This binding describes such a current sense
>> +curcuit.
>> +
>> +Required properties:
>> +- compatible : "current-sense-circuit"
>> +- io-channels : Channel node of a voltage io-channel.
>> +
>> +Optional properties:
>> +- numerator : The io-channel scale is multiplied by this value (default 1).
>> +- denominator : The io-channel scale is divided by this value (default 1).
>> +
>> +Example:
>> +The system current is measured by measuring the voltage over a
>> +3.3 ohms sense resistor.
> 
> Hmm. It sort of feels like the binding doesn't really reflect the
> hardware as directly as it might.   Should we be explicitly having the
> resistance in this case?  (with some more mapping logic needed in the
> driver to figure out the scaling this causes).
> 
>> +
>> +sysi {
>> +	compatible = "current-sense-circuit";
>> +	io-channels = <&tiadc 0>;
>> +
>> +	/* Divide the ADC voltage by 33/10 (i.e. 3.3) to get the current. */
>> +	numerator = <10>;
>> +	denominator = <33>;
>> +};
>> +
>> +&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/Documentation/devicetree/bindings/iio/afe/voltage-divider.txt b/Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
>> new file mode 100644
>> index 000000000000..fd4a215d9e6d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
>> @@ -0,0 +1,45 @@
>> +Voltage divider
>> +===============
>> +
>> +When an io-channel measures the midpoint of a voltage divider, the
>> +interesting voltage is often the voltage over the full resistance
>> +of the divider. This binding describes the voltage divider in such
>> +a curcuit.
>> +
>> +Required properties:
>> +- compatible : "voltage-divider"
>> +- io-channels : Channel node of a voltage io-channel.
>> +
>> +Optional properties:
>> +- numerator : The io-channel scale is multiplied by this value (default 1).
>> +- denominator : The io-channel scale is divided by this value (default 1).
>> +
>> +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>;
>> +};
>> +
>> +&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>;
>> +	};
>> +};
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 36a28e979e9a..9dbe5019c6bd 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -6889,6 +6889,13 @@ F:	drivers/staging/iio/
>>  F:	include/linux/iio/
>>  F:	tools/iio/
>>  
>> +IIO UNIT CONVERTER
>> +M:	Peter Rosin <peda@...ntia.se>
>> +L:	linux-iio@...r.kernel.org
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/iio/afe/current-sense-circuit.txt
>> +F:	Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
>> +
>>  IKANOS/ADI EAGLE ADSL USB DRIVER
>>  M:	Matthieu Castet <castet.matthieu@...e.fr>
>>  M:	Stanislaw Gruszka <stf_xl@...pl>
> 

Powered by blists - more mailing lists