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

[Jonathan Cameron <jic23@kernel.org>]

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

> On 1/18/25 10:57 AM, Jonathan Cameron wrote:
> > On Fri, 17 Jan 2025 15:45:35 -0600
> > David Lechner <dlechner@baylibre.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.
> >