linux-kernel - Re: [PATCH 6/7] dt-bindings: iio: adc: add adi,ad7606c-{16,18} compatible strings

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: <20240821212606.6981eae1@jic23-huawei>
Date: Wed, 21 Aug 2024 21:26:06 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Alexandru Ardelean <aardelean@...libre.com>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 krzk+dt@...nel.org, robh@...nel.org, lars@...afoo.de,
 michael.hennerich@...log.com, gstols@...libre.com
Subject: Re: [PATCH 6/7] dt-bindings: iio: adc: add adi,ad7606c-{16,18}
 compatible strings


> > > +    type: object
> > > +    $ref: adc.yaml
> > > +    unevaluatedProperties: false
> > > +
> > > +    properties:
> > > +      reg:
> > > +        description: The channel number.
> > > +        minimum: 0
> > > +        maximum: 7
> > > +
> > > +      diff-channel:
> > > +        description: Channel is bipolar differential.  
> >
> > There is diff-channels property, why do we need one more?  
> 
> Yeah, I wanted to use that.
> Maybe I will try another spin at that.
> The thing with "diff-channels" is that it requires 2 ints.
> So,  diff-channels = <0 0>.
> To use it here, a rule would need to be put in place where  "reg ==
> diff-channels[0] && reg == diff-channels[1]".
> That also works from my side.
> Part of the reason for this patchset, was to also get some feedback
> (if this is the correct direction).
> 
So I 'think' this is a datasheet matching thing.
In many cases, even for strictly differential devices, the pin
naming allows for a clear A - B channel description. Here
in the non differential modes, the negative pins are effectively
not used (from a really quick look at the datasheet)

So we 'could' introduce magic channels (give them high numbers) for
the negative ends. I think we may want to do that for the
userspace ABI (0-0 on the few times it has come up has been a
calibration / self check mode not what you have here - it
wires the actual inputs together).  Alternative is just present
them as a simple voltage and don't worry about the differential aspect
as it's not hugely different to bipolar (where the zero level is
effectively the negative input of a differential ADC.

For the binding I'm fine with the binding using A, A as you suggest
with an update to adc.yaml to cover this corner.

We never (I think) have bindings for the self check case where the input
is wired to both sides. It's just a mode that is applied to
any inputs that are wired. 

> >  
> > > +        type: boolean
> > > +
> > > +      bipolar:
> > > +        description: |
> > > +          Channel is bipolar single-ended. If 'diff-channel' is set, then
> > > +          the value of this property will be ignored.  
> >
> > Then provide here allOf:if:then which makes it false if diff-channel(s)
> > is present. And then drop entire property, because you duplicate what's
> > in adc.yaml.  
> 
> Ack.