public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Antoniu Miclaus <antoniu.miclaus@analog.com>
Cc: <robh@kernel.org>, <conor+dt@kernel.org>,
	<devicetree@vger.kernel.org>, <linux-iio@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 5/5] Documentation: ABI: iio: adc: add ade9000 ABI
Date: Sun, 27 Jul 2025 14:38:01 +0100	[thread overview]
Message-ID: <20250727143801.69cb377d@jic23-huawei> (raw)
In-Reply-To: <20250721112455.23948-5-antoniu.miclaus@analog.com>

On Mon, 21 Jul 2025 14:24:45 +0300
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add sysfs ABI documentation for the ADE9000 ADC driver,
> documenting the device-specific attributes and interfaces.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
>  new in v2.
>  .../ABI/testing/sysfs-bus-iio-adc-ade9000     | 64 +++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ade9000
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-ade9000 b/Documentation/ABI/testing/sysfs-bus-iio-adc-ade9000
> new file mode 100644
> index 000000000000..fa92fd67483f
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-ade9000
> @@ -0,0 +1,64 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/wf_cap_en
> +KernelVersion:	6.13
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Enable fixed data rate for waveform buffer instead of resampled data.
> +		When enabled (1), the waveform buffer uses a fixed data rate.
> +		When disabled (0), the waveform buffer uses resampled data.

I had to go read the datasheet section on this to find out what this means.
It is changing the sampling frequency to the wave form frequency / 128.
We need to figure out how to map this to something related to sampling frequency.
Given the fixes sample rates are 8k or larger, maybe we just use anything below 8K to mean
use this mode?  Bit hacky but mostly that's the right thing to do as line frequencies
tend to be lower than that anyway.

> +
> +		This attribute is shared by all channels and represents a device-wide
> +		setting that affects the entire waveform buffer configuration.
> +		Changes immediately update the hardware configuration.
> +
> +		Reading: Returns current setting (0 or 1)
> +		Writing: Accepts 0, 1
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/wf_mode
> +KernelVersion:	6.13
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Waveform buffer filling and trigger mode configuration.
> +
> +		Valid values:
> +		0 - Stop when waveform buffer is full
> +		1 - Continuous fill, stop only on enabled trigger events
> +		2 - Continuous filling, center capture around enabled trigger events
> +		3 - Streaming mode
> +
> +		This attribute is shared by all channels and represents a device-wide
> +		setting that affects the entire waveform buffer configuration.
> +		Changes immediately update the hardware configuration.
> +
> +		Reading: Returns current mode (0-3)
> +		Writing: Accepts values 0, 1, 2, or 3
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/wf_in_en
> +KernelVersion:	6.13
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Enable IN waveform samples readout from waveform buffer.
> +		When enabled (1), IN waveform samples are included in buffer readout.

What does buffer readout mean here? Is this the IIO buffer?  In which case why isn't it
just a channel?

> +		When disabled (0), IN waveform samples are excluded from buffer readout.

Hmm. This waveform buffer stuff needs some more thought.  We should really be mapping this
to a buffer with control over the triggering.   Smells a bit like the old impact sensors
but we never actually got those out of staging ;(


I'd be tempted to drop this support from the initial driver so that we can revisit
it and consider it carefully after the main part of the driver is upstream.

Gut feeling is this needs to be using a separate buffer from main channels with
separate trigger controls etc.  The multibuffer stuff is not yet much used so
there may be some core features missing.

> +
> +		This attribute is shared by all channels and represents a device-wide
> +		setting that affects the entire waveform buffer configuration.
> +		Changes immediately update the hardware configuration.
> +
> +		Reading: Returns current setting (0 or 1)
> +		Writing: Accepts 0, 1
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/egy_time
If we keep this, the name definitely needs some work. Probably needs to be standard
ABI as well.

> +KernelVersion:	6.13
That was a while back!

> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Energy accumulation time setting for energy registers.
> +		This value configures the time period over which energy
> +		measurements are accumulated in the ADE9000 device.

So this is interesting.  Feels a bit like it corresponds to a low pass filter
on a power measurement? Or kind of scaling on the power measurement but in terms
of timing.  I'd like inputs from others on how to handle this but I don't think
a custom ABI is the way to go

> +
> +		This attribute is shared by all channels and represents a device-wide
> +		setting that affects energy accumulation across all phases.
> +		Changes immediately update the hardware configuration.
> +
> +		Reading: Returns current energy accumulation time value
> +		Writing: Accepts any valid 32-bit unsigned integer value
> +


  reply	other threads:[~2025-07-27 13:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-21 11:24 [PATCH v2 1/5] iio: add power and energy measurement modifiers Antoniu Miclaus
2025-07-21 11:24 ` [PATCH v2 2/5] dt-bindings: iio: adc: add ade9000 Antoniu Miclaus
2025-07-21 14:23   ` Rob Herring (Arm)
2025-07-26 20:46   ` David Lechner
2025-07-27 13:24   ` Jonathan Cameron
2025-07-21 11:24 ` [PATCH v2 3/5] iio: adc: add ade9000 support Antoniu Miclaus
2025-07-27 14:10   ` Jonathan Cameron
2025-07-21 11:24 ` [PATCH v2 4/5] Documentation: ABI: iio: add sinc4+iir and dsp types Antoniu Miclaus
2025-07-27 13:27   ` Jonathan Cameron
2025-07-21 11:24 ` [PATCH v2 5/5] Documentation: ABI: iio: adc: add ade9000 ABI Antoniu Miclaus
2025-07-27 13:38   ` Jonathan Cameron [this message]
2025-07-27 13:18 ` [PATCH v2 1/5] iio: add power and energy measurement modifiers 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=20250727143801.69cb377d@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=antoniu.miclaus@analog.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.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