linux-kernel - Re: [PATCH 2/3] dt-bindings: iio: wrapper: add io-channel-unit-converter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqJ3RZoMX+Q5boK-w3uHc=qgPCye7esXxJuwQcMM0HPLNw@mail.gmail.com>
Date:   Thu, 29 Mar 2018 08:55:34 -0500
From:   Rob Herring <robh@...nel.org>
To:     Peter Rosin <peda@...ntia.se>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        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>,
        Randy Dunlap <rdunlap@...radead.org>,
        linux-iio@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: iio: wrapper: add io-channel-unit-converter

On Tue, Mar 27, 2018 at 3:01 AM, Peter Rosin <peda@...ntia.se> wrote:
> On 2018-03-27 00:23, Rob Herring wrote:
>> On Mon, Mar 19, 2018 at 06:02:45PM +0100, Peter Rosin wrote:
>>> Allow linear scaling and modification of the type of an io-channel.
>>>
>>> When an ADC channel measures the midpoint of a voltage divider, the
>>> interesting voltage is often the voltage over the full resistance
>>> of the divider. Likewise, measuring the voltage over a resistor is
>>> often a way to get to the current through it.
>>>
>>> This binding allows description of such hardware which is external
>>> to the ADC.
>>>
>>> Signed-off-by: Peter Rosin <peda@...ntia.se>
>>> ---
>>>  .../iio/wrapper/io-channel-unit-converter.txt      | 84 ++++++++++++++++++++++
>>>  MAINTAINERS                                        |  6 ++
>>>  2 files changed, 90 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>> new file mode 100644
>>> index 000000000000..23af661abe32
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/wrapper/io-channel-unit-converter.txt
>>> @@ -0,0 +1,84 @@
>>> +I/O channel unit converter bindings
>>> +
>>> +Allow linear scaling and modification of the type of an io-channel.
>>> +
>>> +When an ADC channel measures the midpoint of a voltage divider, the
>>> +interesting voltage is often the voltage over the full resistance
>>> +of the divider. Likewise, measuring the voltage over a resistor is
>>> +often a way to get to the current through it.
>>> +
>>> +Required properties:
>>> +- compatible : "io-channel-unit-converter"
>>
>> Would this apply to something besides ADCs?
>
> Not that I can think of. At the moment.
>
>>> +- io-channels : Channel node of the parent channel.
>>> +- io-channel-names : Should be "parent".
>>> +
>>> +Optional properties:
>>> +- numerator : The parent channel scale is multiplied by this value (default 1).
>>> +- denominator : The parent channel scale is divided by this value (default 1).
>>> +- type : The type of the wrapped channel is modified to this type. The default
>>> +     is to use the same type as the parent channel. Recognized types are:
>>> +            "voltage"
>>> +            "current"
>>
>> This seems overly complicated for just describing a couple of resistors
>> on an ADC. OTOH, keeping the ADC node and what's attached to the ADC
>> separate
>>
>> Perhaps the type should be part of the compatible. For example, if you
>> have a current measurement resistor/circuit, then the compatible should
>> be based on that.
>
> Is a compatible like "current-sense-circuit" too long(ish)?

No, that's fine.

> Is a vendor prefix absolutely required?

No, it can be common.

> Sure, I can use "axentia,"
> because we "implemented" the circuit this time, but it seems like
> the list can grow very long if we should add every vendor that
> may use something like this? Something more generic would be good
> for something as simple as this IMHO.
>
> I propose the compatible "voltage-divider" for the other example,
> but I have the same issue with the vendor prefix. Even more so
> in this case.

"voltage-divider" doesn't really say what the purpose is.
>
> Then the above type can be inferred from the compatible. I suppose
> I should write the above two bindings as two separate files?

One is fine if the only difference is the compatible.

>>> +
>>> +Example 1:
>>> +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 = "io-channel-unit-converter";
>>> +    io-channles = <&maxadc 1>;
>>> +    io-channel-names = "parent";
>>
>> parent doesn't seem to describe something about the h/w.
>
> Extrapolating from your first question, you are thinking "adc", right?

I don't really have any suggestion. With only 1 channel, there's not
really much point to having a name at all.

Rob