linux-kernel - Re: [PATCH v10 8/8] iio: adc: ad4851: add ad485x driver

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: <20250118174135.1f6e24a3@jic23-huawei>
Date: Sat, 18 Jan 2025 17:41:35 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Antoniu Miclaus <antoniu.miclaus@...log.com>, robh@...nel.org,
 conor+dt@...nel.org, linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v10 8/8] iio: adc: ad4851: add ad485x driver

On Sat, 18 Jan 2025 11:09:12 -0600
David Lechner <dlechner@...libre.com> wrote:

> On 1/18/25 10:57 AM, Jonathan Cameron wrote:
> > On Fri, 17 Jan 2025 15:45:35 -0600
> > David Lechner <dlechner@...libre.com> wrote:
> >   
> >> On 1/17/25 7:07 AM, Antoniu Miclaus wrote:  
> 
> ...
> 
> >>> +		if (fwnode_property_present(child, "diff-channels")) {
> >>> +			*channels = ad4851_chan_diff;
> >>> +			channels->scan_index = index++;
> >>> +			channels->channel = reg;
> >>> +			channels->channel2 = reg;    
> >>
> >> Typically we don't set channel == channel2 for differential channels.  
> > So i guess this is tripping up on these being dedicated pairs labelled
> > +IN1,-IN1 on the datasheet.  The binding documents those as matching
> > the diff-channels - hence both channels and reg are the same.
> > So maybe best bet is to enforce that in the driver by checking it is
> > true.  
> 
> Are you saying that in_voltage0-voltage0_raw in userspace is OK?
That is indeed the question.  It's odd.. 

Also causes us problems because we do have a few devices where we
actually do read a single pin wired to either side of a differential ADC
as part of calibration where in_voltage0-voltage0_raw has
a different meaning to here.

So, gut feeling - too odd and I think every time we've met this
before we have gone with inventing some more channels so that the
inputs have different numbers.  I can't immediately point at an
example though.

Do we need to handle that at the binding level? I'm not sure
if it is clearer to insist we do in both places or let the binding
stick to datasheet numbering and just deal with inventing channel
numbers in the driver.

Jonathan

> 
> > 
> > It is a slightly weird description but only alternative would be to
> > invent some more channel numbers for the negative sides which is
> > less than ideal.  We could go that way though.
> > 
> > Some comments alongside a sanity check is probably the best way to
> > handle this and no surprise us in the future.
> >