linux-kernel - Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x

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: <47175ff9-8b05-4b6a-8689-a3d0722d7571@gmail.com>
Date: Tue, 9 Apr 2024 11:10:57 +0300
From: "Ceclan, Dumitru" <mitrutzceclan@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: David Lechner <dlechner@...libre.com>, dumitru.ceclan@...log.com,
 Lars-Peter Clausen <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x

On 06/04/2024 17:26, Jonathan Cameron wrote:
> On Thu, 4 Apr 2024 16:08:56 +0300
> "Ceclan, Dumitru" <mitrutzceclan@...il.com> wrote:
> 
>> On 03/04/2024 18:22, David Lechner wrote:
>>> On Wed, Apr 3, 2024 at 2:50 AM Ceclan, Dumitru <mitrutzceclan@...il.com> wrote:  
>>>> On 02/04/2024 00:16, David Lechner wrote:  
>>>>> On Mon, Apr 1, 2024 at 2:37 PM David Lechner <dlechner@...libre.com> wrote:  
>>>>>> On Mon, Apr 1, 2024 at 10:10 AM Dumitru Ceclan via B4 Relay
>>>>>> <devnull+dumitru.ceclan.analog.com@...nel.org> wrote:  
>>>>>>> From: Dumitru Ceclan <dumitru.ceclan@...log.com>
>>>>>>>  
>>>> ...
>>>>  
>>>>>>>      properties:
>>>>>>>        reg:
>>>>>>> +        description:
>>>>>>> +          Reg values 16-19 are only permitted for ad4111/ad4112 current channels.
>>>>>>>          minimum: 0
>>>>>>> -        maximum: 15
>>>>>>> +        maximum: 19  
>>>>>> This looks wrong. Isn't reg describing the number of logical channels
>>>>>> (# of channel config registers)?
>>>>>>
>>>>>> After reviewing the driver, I see that > 16 is used as a way of
>>>>>> flagging current inputs, but still seems like the wrong way to do it.
>>>>>> See suggestion below.
>>>>>>  
>>>>>>>        diff-channels:
>>>>>>> +        description:
>>>>>>> +          For using current channels specify only the positive channel.
>>>>>>> +            (IIN2+, IIN2−) -> diff-channels = <2 0>  
>>>>>> I find this a bit confusing since 2 is already VIN2 and 0 is already
>>>>>> VIN0. I think it would make more sense to assign unique channel
>>>>>> numbers individually to the negative and positive current inputs.
>>>>>> Also, I think it makes sense to use the same numbers that the
>>>>>> registers in the datasheet use (8 - 11 for negative and 12 to 15 for
>>>>>> positive).
>>>>>>
>>>>>> So: (IIN2+, IIN2−) -> diff-channels = <13 10>  
>>>>> Thinking about this a bit more...
>>>>>
>>>>> Since the current inputs have dedicated pins and aren't mix-and-match
>>>>> with multiple valid wiring configurations like the voltage inputs, do
>>>>> we even need to describe them in the devicetree?
>>>>>
>>>>> In the driver, the current channels would just be hard-coded like the
>>>>> temperature channel since there isn't any application-specific
>>>>> variation.  
>>>>  Sure, but we still need to offer the user a way to configure which
>>>> current inputs he wants and if they should use bipolar or unipolar coding.  
>>> From the datasheet, it looks like only positive current input is
>>> allowed so I'm not sure bipolar applies here. But, yes, if there is
>>> some other variation in wiring or electrical signal that needs to be
>>> describe here, then it makes sense to allow a channel configuration
>>> node for it.  
>>
>> AD4111 datasheet pg.29:
>> When the ADC is configured for bipolar operation, the output
>> code is offset binary with a negative full-scale voltage resulting
>> in a code of 000 … 000, a zero differential input voltage resulting in
>> a code of 100 … 000, and a positive full-scale input voltage
>> resulting in a code of 111 … 111. The output code for any
>> analog input voltage can be represented as
>> Code = 2^(N – 1) × ((V_IN × 0.1/V REF) + 1)
>> The output code for any input current is represented as
>> Code = 2^(N − 1) × ((I_IN × 50 Ω/V REF) + 1)
>>
>> I would say bipolar applies here, not a great idea because of the limitation on
>>  the negative side (Input Current Range min:−0.5 max:+24 mA) so still, the option
>>  is available.
> Just to check I am correct in thinking you 'might' use bipolar if you want
> to be able to measure small negative currents, but the range is much larger
> in the positive direction?
> 
> J

Yes, exactly