Re: [PATCH v10 1/4] iio: pressure: dps310: read buffered samples from the hardware FIFO

[Andy Shevchenko]

On Wed, Sep 30, 2026 at 01:01:55PM +0300, Rupesh Majhi wrote:
> DPS310 has a 32-entry FIFO shared by both measurements. Drain it from a
> work item and push what it held, so buffered capture needs no trigger.
> Nothing in tree wires the interrupt pin, so the work rearms itself at
> half the FIFO fill time.
> 
> Each entry carries one measurement, so a pressure entry is compensated
> with the temperature ahead of it. Pressure read before a session's first
> temperature is held until one arrives rather than dropped, so the first
> push can wait a temperature period.
> 
> FIFO entries are not timestamped, so postenable refuses the timestamp
> channel unless a trigger is attached.
> 
> Tested on a DPS310 on a BeagleBone Black.

...

> +/* The drain rearms itself, so stop it even if the buffer never disabled */
> +static void dps310_cancel_fifo_work(void *action_data)
> +{
> +	struct dps310_data *data = action_data;
> +
> +	cancel_delayed_work_sync(&data->fifo_work);
> +}

...

>  	mutex_init(&data->lock);

This has to be devm_mutex_init().

...

> +	INIT_DELAYED_WORK(&data->fifo_work, dps310_fifo_work);

Can't we use devm-helpers.h for this?

...

> +	/*
> +	 * Both modes advertised: the core picks TRIGGERED with a trigger
> +	 * attached and falls back to SOFTWARE, which the FIFO path uses.
> +	 */
> +	iio->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED |
> +		     INDIO_BUFFER_SOFTWARE;

I would rearrange this as

	iio->modes = INDIO_BUFFER_TRIGGERED | INDIO_BUFFER_SOFTWARE |
		     INDIO_DIRECT_MODE;

...

>  	/*
> -	 * The device measures continuously in background mode, so a capture is
> -	 * just a read of the latest results. The trigger is not aligned with
> -	 * the measurements, so the timestamp is taken in the handler.
> +	 * The device measures continuously in background mode, so a triggered
> +	 * capture is just a read of the latest results. The setup ops run the
> +	 * FIFO drain when no trigger is attached. The trigger is not aligned
> +	 * with the measurements, so the timestamp is taken in the handler.
>  	 */
>  	rc = devm_iio_triggered_buffer_setup(dev, iio, NULL,
> -					     dps310_trigger_handler, NULL);
> +					     dps310_trigger_handler,
> +					     &dps310_buffer_setup_ops);
> +	if (rc)
> +		return rc;

> +	rc = devm_add_action_or_reset(dev, dps310_cancel_fifo_work, data);
>  	if (rc)
>  		return rc;

Are you sure it's in the correct location?

-- 
With Best Regards,
Andy Shevchenko