Linux IIO development
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: ASSI <Stromeko@nexgo.de>
Cc: linux-iio@vger.kernel.org
Subject: Re: [bmp280 v1 3/6] iio: pressure: bmp280: implement sampling_frequency for BMx280
Date: Sun, 17 Aug 2025 16:10:23 +0100	[thread overview]
Message-ID: <20250817161023.6b887db2@jic23-huawei> (raw)
In-Reply-To: <87ldnonwpv.fsf@Gerda.invalid>

On Tue, 12 Aug 2025 21:53:00 +0200
ASSI <Stromeko@nexgo.de> wrote:

> Jonathan Cameron writes:
> > For cases where we 'trigger' an event we normally use sampling frequency
> > to be 1/(total time to capture a set of readings)  Idea being it's
> > the frequency at which you get results if you immediately trigger
> > another one.  Doing that here doesn't correspond to 0Hz.  
> 
> From the device point of view the setting _is_ 0Hz.  That you are going
> to read it back-to-back is not something it can know.

We can argue this for ever and there is lots of precedence
of treating on demand sensor sampling frequency as 1/period.
(though seems we haven't documented that which isn't great - see reference
later).

> 
> There is one more problem with your idea: since that parameter is the
> only way to switch between modes, implementing it you way would mean
> that MODE_NORMAL+t_stby=0 would map to the same frequency as MODE_FORCED
> and then we're back to square one that there is no parameter to tell us
> which mode we want.

That's a fair point that I'd missed. I was kind of assuming there was
a power up time that made the (pseudo)frequency for MODE_FORCED a
good bit less than the the MODE_NORMAL case which thinking  more on it
would have put it in the middle of the MODE_NORMAL cases so not
helpful either.

> 
> > Main problem is no standard userspace code is going to understand
> > a value of 0Hz as that effectively means no readings ever.  
> 
> I am "userspace" and I well enough that I'm going to get one sample per
> read.

You are writer of one specific bit of userspace code. 
If you look at a generic application like the stuff ADI builds on top
of IIO then that is going to be a very odd value to see.

> 
> >>   
> >> > Even if it is a pain to work out, we should estimate what it means to
> >> > use force mode wrt to how fast a single sample can be obtain.    
> >> 
> >> MODE_FORCED just doesn't work well ffor devices that do mukltiple
> >> measurements at once that then need to be read one by one.  At least not
> >> by using the sysfs read request as the trigger for the measurement,
> >> which is what the earlier patch did that went into v6.13  
> >
> > I understand it works poorly under at least some circumstances. It's
> > just the issue of 0Hz not being a way people would expect to control this
> > or how any other driver does it.  
> 
> Beside any other driver (which I've not looked at), you're willing to
> break an existing one?  Because no existing userspace that use this
> driver will be working past that change.

Existing code doesn't use sampling frequency and works, though slowly
after that power saving related change.  It was the wrong default
choice but we missed it at the time - probably because both the author
and reviewers were thinking slow reads where the only thing likely
to be required by users of this device (see later). We should have
stuck to the periodic sampling unless we detected (runtime pm
autosuspend probably) that we were not doing frequent reads.

> 
> > Another approach might be to stash the timestamp of the last 'forced'
> > event and if it is 'sufficiently' recent just go with the readings of
> > other channels grabbed at that time. If it's not we pay the cost of
> > another read cycle.  That's close to doing a runtime pm autosuspend
> > on many other sensors where we take the view that if no one asked
> > for a while, we will pay the cost of a new power up and hence put the
> > sensor to sleep.  
> 
> I've actually looked at that and found it lacking.  That just makes
> MODE_FORCED a permanent thing that totally ignores that the sensor is
> supposed to be operated in free-running MODE_NORMAL in many
> applications.

An additional feature would be to detect repeated reads
and deciding to to autoswitch out of mode forced (probably on second read
of a channel within a short period).

> MODE_FORCED really is for things like wheather stations
> where it gets woken up onece every few minutes.  As long as there is no
> separate attribute or module parameter to switch between operation
> modes, this would be removing exactly the functionality that I want to
> use.

Ok.  I misunderstood this.  It's not that you want to grab
all the channels quickly (but only do that infrequently) it's that you
want to do measurement of rapidly changing pressure?  That's fair enough
as a use case, I just wasn't expecting to see it for this part (vs the
ones that have pipe connectors or similar)

> 
> > Hooking a single forced capture up to a hrtimer trigger would also
> > be somewhat similar to this but the interface to userspace is
> > rather more different so perhaps not the right way to think about it
> > here.  
> 
> I haven't been able to get this to work, I've never got a reading out of
> the buffer.  I'll try again once the distribution I#m using has enabled
> sysfs triggers, which are easier to test with hopefully.
Ok. Lets park that one for now.  This isn't a case where we have to get
readings at the same time for multiple channels to be useful, they just
need for your usecase not to be too far apart. So buffered support isn't
urgent.

Given the situation I'll accept the slight oddity of a 0Hz sampling_frequency.
It may confuse some software but as long as the default is sensible
sampling frequency isn't (I believe) that commonly tweaked for sysfs
reads anyway.  Tends to be much more relevant with high speed buffered
capture.

Hmm. Seems I let an exception for this in once before. Sorry had forgotten
that until I was wondering what docs update we need.

https://elixir.bootlin.com/linux/v6.16/source/Documentation/ABI/testing/sysfs-bus-iio#L112

So can you add a note to that doc covering this case (can be a follow up
patch if everything else is fine for v2)?

Sometimes these corner cases take a bit of arguing out. Thanks for
your patience!

Thanks,

Jonathan




> 
> 
> Achim.


  reply	other threads:[~2025-08-17 15:10 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-17 23:30 [PATCH v9 0/4] pressure: bmp280: Minor cleanup and interrupt support Vasileios Amoiridis
2024-10-17 23:30 ` [PATCH v9 1/4] iio: pressure: bmp280: Use sleep and forced mode for oneshot captures Vasileios Amoiridis
2025-06-28 18:45   ` ASSI
2025-06-28 20:57     ` David Lechner
2025-06-29  7:43       ` ASSI
2025-07-11 19:17       ` ASSI
2025-07-12 14:49         ` ASSI
2025-07-21 19:15           ` ASSI
2025-07-26 18:34           ` ASSI
2025-07-27 19:08             ` ASSI
2024-10-17 23:30 ` [PATCH v9 2/4] dt-bindings: iio: pressure: bmp085: Add interrupts for BMP3xx and BMP5xx devices Vasileios Amoiridis
2024-10-17 23:30 ` [PATCH v9 3/4] iio: pressure: bmp280: Add data ready trigger support Vasileios Amoiridis
2024-10-17 23:30 ` [PATCH v9 4/4] iio: pressure: bmp280: Move bmp085 interrupt to new configuration Vasileios Amoiridis
2024-10-19 13:55 ` [PATCH v9 0/4] pressure: bmp280: Minor cleanup and interrupt support Jonathan Cameron
2025-08-03 14:07 ` [bmp280 v1 0/6] Fixes and enhancements for the bmp280 driver Achim Gratz
2025-08-03 14:07   ` [bmp280 v1 1/6] iio: pressure: bmp280: correct meas_time_us calculation Achim Gratz
2025-08-06 15:46     ` Jonathan Cameron
2025-08-06 17:53       ` ASSI
2025-08-10 18:04         ` Jonathan Cameron
2025-08-03 14:07   ` [bmp280 v1 2/6] iio: pressure: bmp280: reduce overhead on read with MODE_FORCED Achim Gratz
2025-08-03 20:12     ` Andy Shevchenko
2025-08-06 15:58     ` Jonathan Cameron
2025-08-06 18:00       ` ASSI
2025-08-03 14:07   ` [bmp280 v1 3/6] iio: pressure: bmp280: implement sampling_frequency for BMx280 Achim Gratz
2025-08-03 20:26     ` Andy Shevchenko
2025-08-04 17:29       ` ASSI
2025-08-10 18:11         ` Jonathan Cameron
2025-08-10 19:12           ` ASSI
2025-08-11 19:48             ` Jonathan Cameron
2025-08-12 19:53               ` ASSI
2025-08-17 15:10                 ` Jonathan Cameron [this message]
2025-08-17 16:36                   ` ASSI
2025-08-03 14:08   ` [bmp280 v1 4/6] iio: pressure: bmp280: enable filter settings " Achim Gratz
2025-08-03 20:28     ` Andy Shevchenko
2025-08-04 17:14       ` ASSI
2025-08-10 18:13     ` Jonathan Cameron
2025-08-10 19:01       ` ASSI
2025-08-11 20:14         ` Jonathan Cameron
2025-08-12 19:34           ` ASSI
2025-08-17 14:51             ` Jonathan Cameron
2025-08-03 14:08   ` [bmp280 v1 5/6] iio: pressure: bmp280: remove code duplication Achim Gratz
2025-08-03 20:30     ` Andy Shevchenko
2025-08-10 18:19     ` Jonathan Cameron
2025-08-03 14:08   ` [bmp280 v1 6/6] iio: pressure: bmp280: implement sampling_frequency calculation for BMx280 Achim Gratz
2025-08-03 20:37     ` Andy Shevchenko
2025-08-04 17:20       ` ASSI
2025-08-03 19:20   ` [bmp280 v1 0/6] Fixes and enhancements for the bmp280 driver Andy Shevchenko
2025-08-10 18:58 ` [RFC PATCH v2 0/9] " Achim Gratz
2025-08-10 18:58   ` [RFC PATCH v2 1/9] iio: pressure: bmp280: correct meas_time_us calculation Achim Gratz
2025-08-17 15:16     ` Jonathan Cameron
2025-08-10 18:58   ` [RFC PATCH v2 2/9] iio: pressure: bmp280: implement adaptive wait for BMx280 devices Achim Gratz
2025-08-10 19:49     ` Andy Shevchenko
2025-08-16 18:42       ` ASSI
2025-08-10 18:58   ` [RFC PATCH v2 3/9] iio: pressure: bmp280: implement adaptive wait for BMP380 devices Achim Gratz
2025-08-10 18:58   ` [RFC PATCH v2 4/9] iio: pressure: bmp280: refactoring Achim Gratz
2025-08-17 15:23     ` Jonathan Cameron
2025-08-10 18:58   ` [RFC PATCH v2 5/9] iio: pressure: bmp280: remove code duplication Achim Gratz
2025-08-10 18:58   ` [RFC PATCH v2 6/9] iio: pressure: bmp280: enable filter settings for BMx280 Achim Gratz
2025-08-17 16:37     ` Jonathan Cameron
2025-08-10 18:58   ` [RFC PATCH v2 7/9] iio: pressure: bmp280: implement sampling_frequency " Achim Gratz
2025-08-17 16:53     ` Jonathan Cameron
2025-08-17 17:36       ` ASSI
2025-08-10 18:58   ` [RFC PATCH v2 8/9] iio: pressure: bmp280: implement sampling_frequency calculation " Achim Gratz
2025-08-17 17:04     ` Jonathan Cameron
2025-08-17 17:40       ` ASSI
2025-08-18 17:52         ` Jonathan Cameron
2025-08-10 18:58   ` [RFC PATCH v2 9/9] iio: pressure: bmp280: test longer autosuspend (WIP) Achim Gratz
2025-08-17 17:05     ` Jonathan Cameron
2025-08-17 17:44       ` ASSI
2025-08-11 12:14   ` [RFC PATCH v2 0/9] Fixes and enhancements for the bmp280 driver Andy Shevchenko
2025-08-11 20:17   ` 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=20250817161023.6b887db2@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Stromeko@nexgo.de \
    --cc=linux-iio@vger.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