linux-kernel - Re: [PATCH v2 1/9] 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: <20240519175438.5959fab2@jic23-huawei>
Date: Sun, 19 May 2024 17:54:38 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: "Ceclan, Dumitru" <mitrutzceclan@...il.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 v2 1/9] dt-bindings: adc: ad7173: add support for ad411x


> >  
> > >> +
> > >> +          There are special values that can be selected besides the voltage
> > >> +          analog inputs:
> > >> +            21: REF+
> > >> +            22: REF−
> > >> +          Supported only by AD7172-2, AD7172-4, AD7175-2, AD7175-8, AD7177-2:
> > >> +            19: ((AVDD1 − AVSS)/5)+
> > >> +            20: ((AVDD1 − AVSS)/5)−
> > >> +          Supported only by AD4111, AD4112:
> > >> +            12: IIN3+
> > >> +            11: IIN3−
> > >> +            13: IIN2+
> > >> +            10: IIN2−
> > >> +            14: IIN1+
> > >> +             9: IIN1−
> > >> +            15: IIN0+
> > >> +             8: IIN0−  
> > >
> > > I just made a late reply on v1 where Jonathan suggested that the
> > > current inputs are differential with a similar comment to this:
> > >
> > > It doesn't seem to me like current inputs are differential if they are
> > > only measuring one current. They take 2 pins because you need a way
> > > for current to come in and go back out, but the datasheet calls them
> > > single-ended inputs.
> > >  
> > It seemed to me that the conclusion that we arrived to was to expose the
> > precise pins that are used in the conversion and document the selection.
> >
> > Yes, it is a single-ended channel. So revert to the way it was in V1 using
> > single-channel?  
> 
> I'd like to hear Jonathan's opinion on this one.

Yes.  I think we rather went off on a false tangent on this.  Any current input
using a shunt is of this form and so far we've treated them as single ended :(
e.g. pac1934 does this for it's sense inputs where there is an external
'sense resitor'.  Similar for the stand alone afe driver for a current sense shunt
which is used with a differential voltage input, but presents a single ended
current measurement.

Sorry for misguiding things :( 

At the end of this, I don't suppose anyone fancies writing up some notes
on how to describe different types of channel?

> 
> >  
> > >> +
> > >>          items:
> > >>            minimum: 0
> > >>            maximum: 31
> > >> @@ -154,6 +195,23 @@ patternProperties:
> > >>            - avdd
> > >>          default: refout-avss
> > >>
> > >> +      adi,current-channel:
> > >> +        description: |
> > >> +          Signal that the selected inputs are current channels.
> > >> +          Only available on AD4111 and AD4112.
> > >> +        type: boolean

I'm lost. Why do we need this one?  Is the channel selection not sufficient
to tell us this?

> > >> +
> > >> +      adi,channel-type:
> > >> +        description:
> > >> +          Used to differentiate between different channel types as the device
> > >> +           register configurations are the same for all usage types.
> > >> +        $ref: /schemas/types.yaml#/definitions/string
> > >> +        enum:
> > >> +          - single-ended
> > >> +          - pseudo-differential
> > >> +          - differential
> > >> +        default: differential
> > >> +  
> > >
> > > As suggested above, we should soon have diff-channels and
> > > single-channel to differentiate between (fully) differential and
> > > single-ended. Do we actually need to differentiate between
> > > single-ended and pseudo-differential though?
> > >  
> > Not really, so just a bool differential flag? (this seems weird as we have diff-channels)  
> 
> Or we need to change the proposed single-channel property to allow two
> inputs. I guess we'll see what Johnathan has to say.

I think single ended fits better for the current channels with just one
parameter.