All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jonathan.cameron@huawei.com>
To: David Lechner <dlechner@baylibre.com>
Cc: "Marilene Andrade Garcia" <marilene.agarcia@gmail.com>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org,
	"Kim Seer Paller" <kimseer.paller@analog.com>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Michael Hennerich" <Michael.Hennerich@analog.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Marcelo Schmitt" <marcelo.schmitt1@gmail.com>,
	"Marcelo Schmitt" <Marcelo.Schmitt@analog.com>,
	"Ceclan Dumitru" <dumitru.ceclan@analog.com>,
	"Jonathan Santos" <Jonathan.Santos@analog.com>,
	"Dragos Bogdan" <dragos.bogdan@analog.com>
Subject: Re: [PATCH v10 0/2] Add MAX14001/MAX14002 support
Date: Wed, 3 Sep 2025 17:11:05 +0100	[thread overview]
Message-ID: <20250903171105.00003dcd@huawei.com> (raw)
In-Reply-To: <dfcdaf9a-1980-4059-9268-2e9ae96831e8@baylibre.com>


> 
> > because of the unique way this device handles communication, such as 
> > inverting bits before sending a message, updating the write enable register 
> > before writing any other register, and updating it again afterward. However, 
> > as I am still new to the IIO kernel code, I may be missing something. If you 
> > could provide further explanation or an example, I would be grateful.
> > 
> > Regarding locking, Kim’s original code implemented it, and it remains in 
> > the driver.
> > 
> > I still have a question about using _mean_raw (IIO_CHAN_INFO_AVERAGE_RAW) 
> > to read the register containing the latest filtered average ADC readings. 
> > Should I create a v11 version with a patch to include in_voltageY_mean_raw 
> > in the file /linux/Documentation/ABI/testing/sysfs-bus-iio?   
> 
> There is already "/sys/bus/iio/devices/iio:deviceX/in_Y_mean_raw" which
> I think is intended to cover that.
> 
> > The idea is to use in_voltageY_mean_raw to return the filtered average and 
> > also to set how many ADC readings (0, 2, 4, or 8) are included in the mean 

0 is an odd value, I assume 1 given average of 0 readings is effectively undefined.

> > calculation. Any feedback on using IIO_CHAN_INFO_AVERAGE_RAW this way would 
> > be appreciated.

Sorry I missed this question in earlier versions.  I'm terrible at reading
cover letters!

Definitely don't use the in_voltage_mean_raw value for the control of the averaging
width.  That is too obscure and normal convention is read and write of
sysfs attributes should see effectively the same value (or not all
either read or write)

This is a little unusual as normally when we see this sort of thing it
easily maps to oversampling but in this case it's a moving window average
rather than a downsampling style.

So a few options come to mind.
1. Treat it as a filter on a channel. Describe with 3dB point and type as
   box filter.
   We probably want to describe it as an additional channel to do this or
   we could have assumption that to read unfiltered, you set the
   filter 3dB to inf (see other discussion ongoing about that).

2. Add another attribute to add richer info to the in_voltage_mean_raw
   Something like in_voltage_mean_num.
   Bit of a special case but we have had the mean_raw interface a long time
   so we can't get rid of that and it seems illogical to force use of filter
   ABI to control it.

> > 
Other stuff from David followed but I have nothing to add to that.

Jonathan


      reply	other threads:[~2025-09-03 16:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-02 13:14 [PATCH v10 0/2] Add MAX14001/MAX14002 support Marilene Andrade Garcia
2025-09-02 13:15 ` [PATCH v10 1/2] dt-bindings: iio: adc: add max14001 Marilene Andrade Garcia
2025-09-02 14:29   ` David Lechner
2025-09-02 15:30     ` David Lechner
2025-09-02 19:42     ` Conor Dooley
2025-09-02 13:16 ` [PATCH v10 2/2] iio: adc: max14001: New driver Marilene Andrade Garcia
2025-09-02 13:45   ` Andy Shevchenko
2025-09-02 14:12     ` David Lechner
2025-09-02 14:23       ` Andy Shevchenko
2025-09-02 15:44   ` David Lechner
2025-09-03 22:28   ` kernel test robot
2025-09-02 16:10 ` [PATCH v10 0/2] Add MAX14001/MAX14002 support David Lechner
2025-09-03 16:11   ` Jonathan Cameron [this message]

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=20250903171105.00003dcd@huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=Jonathan.Santos@analog.com \
    --cc=Marcelo.Schmitt@analog.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=dragos.bogdan@analog.com \
    --cc=dumitru.ceclan@analog.com \
    --cc=jic23@kernel.org \
    --cc=kimseer.paller@analog.com \
    --cc=krzk+dt@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcelo.schmitt1@gmail.com \
    --cc=marilene.agarcia@gmail.com \
    --cc=nuno.sa@analog.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.