Re: [PATCH v6 5/6] iio: pressure: dps310: add hardware FIFO support

[Jonathan Cameron <jic23@kernel.org>]

On Sun,  6 Sep 2026 12:45:04 +0300
Rupesh Majhi <zoone.rupert@gmail.com> wrote:

> On Mon, 31 Aug 2026 02:14:41 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > Dropping data rather defeats the point of the fifo.  Can we avoid it
> > by holding them locally until we will definitely have seen a useable
> > temperature?  
> 
> The temperature is dropped too. dps310_fifo_drain() sets fifo_temp_valid
> only in its second pass but returns at "if (!scans)" first, and FIFO reads
> are destructive, so a batch that pushes nothing loses the temperature it
> read. With pressure enabled, only a batch holding one ahead of a pressure
> entry gets through.
> 
> DPS310 on a BeagleBone Black, pressure only, 32 Hz and 1 Hz, 3 s per trial:
> 
>   watermark 1  (31 ms drain):   4 of 6 enables produced nothing
>   watermark 32 (484 ms drain):  0 of 6
> 
> Other runs gave 3 of 8 and 3 of 6. Silent, nothing in dmesg.

Maybe we just set the minimum allowed watermark to something that
in practice always has a temperature (or almost does)? It's kind
of true that a watermark below that doesn't work because we end
up dropping data.

> 
> Seeding instead of holding, one temperature read in postenable, fixes it:
> 0 of 16. But TMP_RDY is cleared on read, so after a sysfs read the seed
> waits for the next conversion. Buffer enable took:
> 
>   temp 1 Hz:   661, 781, 802 ms
>   temp 32 Hz:  148, 151, 151 ms   (period is 31 ms, so this is the floor)
> 
> Timeout is 1000000/rate, and on timeout dps310_ready() reinits, putting
> PRS_CFG and TMP_CFG back to defaults. Not something I want in a buffer
> enable.
> 
> Hold across batches as you suggested, since a 31 ms batch usually has no
> temperature in it at all, or a non-blocking dps310_refresh_temp_locked()
> seed plus a validity flag?

Validity flag in the data we push to userspace?  That never works because
we don't have a universal encoding for one.

> 
> > For initial patch just reject starting the buffer if timestamps are
> > set to be captured.  
> 
> No sanctioned way that I can find. buffer->scan_timestamp is private and
> kept out of the scan mask: active_scan_mask reads 2 with in_timestamp_en
> set, and validate_scan_mask() sees the same. indio_dev->scan_timestamp is
> reachable and iio_enable_buffers() sets it before postenable:
> 
> 	if (ACCESS_PRIVATE(iio, scan_timestamp))
> 		return -EINVAL;
> 
> but it is __private, used only there and in
> iio_push_to_buffers_with_timestamp(). That, a helper, or drop the soft
> timestamp channel for now? Triggered-path timestamps are real, so I would
> rather keep those.

Given we have a user I'm fine with having a read only accessor function.
We can't drop anything already supported upstream as it would be an ABI
regression and we only do those when fixing bugs.

So just add a little helper that has the ACCESS_PRIVATE magic hidden away
in it. That stuff is really there to stop writes and ensure we have
an interface that lets us mess around with the internal storage without
changing lots of driver code.

Jonathan
> 
> Rupesh