Linux Documentation
 help / color / mirror / Atom feed
From: "Nuno Sá" <noname.nuno@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: "Janani Sunil" <jananisunil.dev@gmail.com>,
	"David Lechner" <dlechner@baylibre.com>,
	"Janani Sunil" <janani.sunil@analog.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Michael Hennerich" <Michael.Hennerich@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Olivier Moysan" <olivier.moysan@foss.st.com>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"Linus Walleij" <linusw@kernel.org>,
	"Bartosz Golaszewski" <brgl@kernel.org>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Shuah Khan" <skhan@linuxfoundation.org>,
	linux@analog.com, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
Subject: Re: [PATCH 1/6] dt-bindings: iio: adc: Add AD7768
Date: Thu, 23 Jul 2026 09:15:08 +0100	[thread overview]
Message-ID: <amHLZK46lxqSZIC8@nsa> (raw)
In-Reply-To: <20260722025853.6ca3961d@jic23-huawei>

On Wed, Jul 22, 2026 at 02:58:53AM +0100, Jonathan Cameron wrote:
> On Tue, 21 Jul 2026 10:03:26 +0200
> Janani Sunil <jananisunil.dev@gmail.com> 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 <janani.sunil@analog.com>
> > >>>> ---
> > >>>>   
> > >>>> +
> > >>>> +  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 :).

> 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?  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...

Or given this also looks like a system level decision (I don't think this
is something one wants to change at runtime) maybe DT (acting as an override)
would not be off the table.

- Nuno Sá

> 
> Jonathan
> 
> 
> 
> > 
> 

  reply	other threads:[~2026-07-23  8:14 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09  8:50 [PATCH 0/6] iio: adc: Add AD7768/AD7768-4 ADC driver support Janani Sunil
2026-07-09  8:50 ` [PATCH 1/6] dt-bindings: iio: adc: Add AD7768 Janani Sunil
2026-07-09 15:43   ` David Lechner
2026-07-10  0:33     ` Jonathan Cameron
2026-07-11 14:40       ` David Lechner
2026-07-12  1:39         ` Jonathan Cameron
2026-07-12 16:07           ` David Lechner
2026-07-20 14:00     ` Janani Sunil
2026-07-21  1:39       ` David Lechner
2026-07-21  8:03         ` Janani Sunil
2026-07-22  1:58           ` Jonathan Cameron
2026-07-23  8:15             ` Nuno Sá [this message]
2026-07-10  1:39   ` Jonathan Cameron
2026-07-09  8:50 ` [PATCH 2/6] iio: backend: Add support for CRC Janani Sunil
2026-07-10  0:36   ` Jonathan Cameron
2026-07-09  8:50 ` [PATCH 3/6] iio: adc: adi-axi-adc: " Janani Sunil
2026-07-09 15:54   ` David Lechner
2026-07-10  0:39   ` Jonathan Cameron
2026-07-10  0:46   ` Jonathan Cameron
2026-07-14 14:18     ` Nuno Sá
2026-07-14 14:33       ` Nuno Sá
2026-07-09  8:50 ` [PATCH 4/6] iio: adc: Add AD7768 IIO Driver support Janani Sunil
2026-07-10  2:10   ` Jonathan Cameron
2026-07-10  7:41   ` Uwe Kleine-König
2026-07-09  8:50 ` [PATCH 5/6] gpio: ad7768: Add AD7768 GPIO auxiliary driver Janani Sunil
2026-07-09 11:05   ` Andy Shevchenko
2026-07-14 11:03     ` Janani Sunil
2026-07-14 11:31       ` Andy Shevchenko
2026-07-10  2:14   ` Jonathan Cameron
2026-07-10 20:06   ` Linus Walleij
2026-07-09  8:50 ` [PATCH 6/6] Documentation: iio: Add AD7768 Documentation Janani Sunil
2026-07-10  2:16   ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amHLZK46lxqSZIC8@nsa \
    --to=noname.nuno@gmail.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=andy@kernel.org \
    --cc=brgl@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=janani.sunil@analog.com \
    --cc=jananisunil.dev@gmail.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@analog.com \
    --cc=nuno.sa@analog.com \
    --cc=olivier.moysan@foss.st.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robh@kernel.org \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox