Re: [PATCH 1/6] dt-bindings: iio: adc: Add AD7768

On Thu, Jul 23, 2026 at 11:48:55PM +0100, Jonathan Cameron wrote:
> On Thu, 23 Jul 2026 09:15:08 +0100
> Nuno Sá <[email protected]> wrote:
> 
> > On Wed, Jul 22, 2026 at 02:58:53AM +0100, Jonathan Cameron wrote:
> > > On Tue, 21 Jul 2026 10:03:26 +0200
> > > Janani Sunil <[email protected]> wrote:
> > >   
> > > > On 7/21/26 03:39, David Lechner wrote:  
> > > > > On 7/20/26 9:00 AM, Janani Sunil wrote:    
> > > > >> On 7/9/26 17:43, David Lechner wrote:    
> > > > >>> On 7/9/26 3:50 AM, Janani Sunil wrote:    
> > > > >>>> Devicetree Bindings for AD7768-4 (4 channel) and AD7768 (8
> > > > >>>> channel) simultaneous sampling ADC
> > > > >>>>
> > > > >>>> Signed-off-by: Janani Sunil <[email protected]>
> > > > >>>> ---
> > > > >>>>   
> > > > >>>> +
> > > > >>>> +  adi,power-mode:
> > > > >>>> +    $ref: /schemas/types.yaml#/definitions/string
> > > > >>>> +    enum:
> > > > >>>> +      - low
> > > > >>>> +      - median
> > > > >>>> +      - fast
> > > > >>>> +    description:
> > > > >>>> +      Power mode selection.    
> > > > >>> Unless there are pins that control this, it seems like it should
> > > > >>> be left up to the driver to decide how to set this.
> > > > >>>
> > > > >>> In this case, it looks like the power mode also influences sample
> > > > >>> rate which is normally something controlled at runtime.    
> > > > > Looking at this again, there is also an MCLK divider that influences
> > > > > sample rate, so sampling_frequency to power mode is not
> > > > > straight-forward anyway.
> > > > >    
> > > > >> Hi David,
> > > > >>
> > > > >> The reason we'd like to retain power mode control is that certain
> > > > >> ODRs are supported across all three power modes (low/median/fast),
> > > > >> and the RMS noise and power consumption differ significantly
> > > > >> between them at the same ODR.
> > > > >>
> > > > >> The higher the power mode, the better the noise performance, but
> > > > >> power consumption nearly doubles for every ~3 dB improvement in
> > > > >> dynamic range. Silently selecting one power mode in the driver
> > > > >> would remove a meaningful hardware tradeoff from the user.
> > > > >>
> > > > >> We'd like to propose the following instead:
> > > > >> - Remove adi,power-mode from the DT as suggested.
> > > > >> - Expose power mode as a per-device sysfs attribute.
> > > > >> - in_voltage<N>_sampling_frequency_available dynamically reflects
> > > > >> only the ODRs valid for the currently selected power mode.
> > > > >>
> > > > >> This keeps the DT clean while still giving the user explicit
> > > > >> control over the noise versus power trade off. Would this approach
> > > > >> be acceptable?
> > > > >>
> > > > >> Thanks,
> > > > >> Jan
> > > > >>
> > > > >>    
> > > > > Jonathan usually pushes back against userspace power controls. We
> > > > > do have this for accelerometers, but not ADCs currently.
> > > > >
> > > > > If we can't think of anything better, maybe we could use this. It
> > > > > only has low_noise and low_power options though, so the driver
> > > > > would still need to chose the best power mode of the 3 based on the
> > > > > other requested parameters. E.g. always make all sampling_frequency
> > > > > available  and just pick the highest power or lowest power mode
> > > > > that can provide that rate based on the power_mode attribute.
> > > > >
> > > > > I wanted to suggest maybe adding some kind of noise attribute
> > > > > instead, but I'm not sure how we could do that in a way using SI
> > > > > units since the value would depend on so many things (at least
> > > > > V_REF voltage, filter type, temperature and even the physical
> > > > > input).    
> > > > 
> > > > We considered the low_power/balanced/low_noise approach, but the
> > > > customers typically use the datasheet alongside the driver and the
> > > > datasheet explicitly uses the terms "low power", "median" and "fast"
> > > > for the three modes. Abstracting them with different names in the
> > > > sysfs attribute would create a confusion- the users would have to
> > > > mentally translate between the two naming conventions. The noise
> > > > attribute would not actually configure anything on a register level-
> > > > it would purely be informational. Furthermore, noise performance is
> > > > not solely determined by the power mode. There are other parameters
> > > > (eg. filter mode) that has a significant impact. A power_mode
> > > > attribute directly configures the hardware register and has a
> > > > deterministic effect on the device.
> > > > 
> > > > Jonathan, could we keep the power_mode as an attribute in this case?  
> > > 
> > > My really strong resistance to 'mode' type controls is they are
> > > meaningless to general purpose software.  It has no idea how to
> > > set them correctly. The purpose of IIO is to provide general abstractions
> > > and power modes are never that.
> > >   
> > 
> > Unfortunately lot's of apps engineers for these parts just like to
> > control it all and don't care about generic interfaces (given typically
> > these the devices are the only ones they care). So we also have some pain
> > trying to explain all of the advantages :).
> 
> Good thing they have to play by our rules if they want to have it
> upstream and not ship the driver to very possible customer ;)
> 
> > 
> > > Hence we put a lot of effort into mapping them to controls or measures
> > > that have precise meaning.
> > > 
> > > I can't remember how I got talked into allowing the accelerometer
> > > ones either :(  It was way back in 2013 and we have only one user.
> > > I should shift that ABI doc to a driver specific one so as not tempt
> > > more users.
> > > 
> > > To give a firmer opinion I guess I'll go read the datasheet at somepoint
> > > but it's not a small one so that may take a little while.  I do see
> > > that there is a recommendation for each power setting to only cover
> > > one range of f_mod.  Maybe that gives us a route to a control that
> > > it at least numerically meaningful (For those who know what effect
> > > the modulator frequency has in a sigma delta modulator! Which
> > > doesn't include me ;()
> > > 
> > > Any chance we can kick this into the future and get a useful base
> > > driver ready for upstream?  
> 
> However we go with this I'd like to keep it separate from initial driver
> merge.  Then we can have a focused discussion around a follow up patch.
> 
> >It is a lot easier to have these discussions
> > > when they are the only topic related to a thread.  One path to move
> > > on would be to decide to default (only option for now) to the
> > > best noise option.  Later we can add controls that result in that
> > > initial default changing,  
> > 
> > Yeah, those were my internal 2 cents. Not sure if this is exactly what
> > you're proposing but what also came to my mind was to have a RMS lookup
> > table (same as in DS) and for the overlapping frequencies, just go with
> > the one with better RMS figure. My assumptions is that most users will
> > prefer that. If not else better comes, we can have a driver argument to
> > override the prefered mode (same way we do for some ADIS IMU devices).
> > And yes, I know module arguments are not very encouraged...
> 
> Yeah, for other cases a bit like this we've always assumed they want
> minimum noise for a given set of parameters but that isn't quite always
> the case when power consumption matters on the platform. 
> 
> The IMU low rate allow thing (I guess that is what you mean) is perhaps
> a little similar as you suggest.  It is an intentional override of
> datasheet recommendations.  I'm not against doing that here, but definitely
> as a follow up patch where the reasons are laid out clearly.

Agreed!

- Nuno Sá