Re: [PATCH v2 2/2] iio: pressure: dps310: add a data-ready trigger

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] iio: pressure: dps310: blocking temp wait in
  dps310_fill_channels() on IRQ
--

commit fcac9adbf4cd29f69b8e01eb7d00e6810af90fad
Author: Rupesh Majhi <zoone.rupert@gmail.com>

iio: pressure: dps310: add a data-ready trigger

This commit adds an IIO data-ready trigger driven by the DPS310 interrupt
to capture pressure or temperature results with interrupt timestamps.

> diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> index 5cee23986f307..4bff9e93ea92c 100644
> --- a/drivers/iio/pressure/dps310.c
> +++ b/drivers/iio/pressure/dps310.c

[ ... ]

> @@ -1375,6 +1393,7 @@ static irqreturn_t dps310_trigger_handler(int irq, void *p)
>  		s32 channels[2];
>  		aligned_s64 timestamp;
>  	} scan = { };
> +	s64 timestamp;
>  	int rc;
>  
>  	mutex_lock(&data->lock);
> @@ -1383,7 +1402,12 @@ static irqreturn_t dps310_trigger_handler(int irq, void *p)
>  	if (rc)
>  		goto err;
>  
> -	iio_push_to_buffers_with_ts(iio, &scan, sizeof(scan), iio_get_time_ns(iio));
> +	if (iio_trigger_using_own(iio))
> +		timestamp = data->irq_timestamp;
> +	else
> +		timestamp = iio_get_time_ns(iio);
> +
> +	iio_push_to_buffers_with_ts(iio, &scan, sizeof(scan), timestamp);
>  
>  err:
>  	iio_trigger_notify_done(iio->trig);
> @@ -1391,6 +1415,105 @@ static irqreturn_t dps310_trigger_handler(int irq, void *p)
>  	return IRQ_HANDLED;
>  }
>  
> +static irqreturn_t dps310_irq_handler(int irq, void *private)
> +{
> +	struct dps310_data *data = private;
> +
> +	data->irq_timestamp = iio_get_time_ns(data->iio);
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t dps310_irq_thread(int irq, void *private)
> +{
> +	struct dps310_data *data = private;
> +	unsigned int status;
> +
> +	if (regmap_read(data->regmap, DPS310_INT_STS, &status))
> +		return IRQ_NONE;
> +
> +	if (!(status & (DPS310_INT_STS_TMP | DPS310_INT_STS_PRS)))
> +		return IRQ_NONE;
> +
> +	iio_trigger_poll_nested(data->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int dps310_trigger_set_state(struct iio_trigger *trig, bool state)
> +{
> +	struct dps310_data *data = iio_trigger_get_drvdata(trig);
> +	struct regmap *map = data->regmap;
> +	unsigned int status, bits = 0;
> +	int rc;
> +
> +	guard(mutex)(&data->lock);
> +
> +	if (state) {
> +		/* Status latched while disabled would fire a stale interrupt */
> +		rc = regmap_read(map, DPS310_INT_STS, &status);
> +		if (rc)
> +			return rc;
> +
> +		/* Without pressure, scans follow the temperature results */
> +		if (test_bit(DPS310_SCAN_PRESSURE, data->iio->active_scan_mask))
> +			bits = DPS310_INT_PRS;
> +		else
> +			bits = DPS310_INT_TMP;
> +	}

[Severity: High]
Does triggering on DPS310_INT_PRS block in dps310_fill_channels() waiting
for DPS310_TMP_RDY whenever the pressure rate exceeds the temperature rate?

drivers/iio/pressure/dps310.c:dps310_fill_channels() {
	/* Compensation needs a temperature, so it is sampled either way */
	rc = dps310_read_temp_raw_locked(data);
	if (rc)
		return rc;
    ...
	if (test_bit(DPS310_SCAN_PRESSURE, scan_mask)) {
		rc = dps310_read_pres_raw_locked(data);
}

Each scan reads DPS310_TMP_BASE and clears DPS310_TMP_RDY. When the next
DPS310_INT_PRS interrupt fires before a new temperature conversion has
completed (for example at any pressure rate above the 1 Hz default
temperature rate), dps310_read_temp_raw_locked() sleeps until the next
temperature conversion finishes.

While the IRQ thread sleeps with DPS310_INT_STS already cleared and the
oneshot IRQ masked, new pressure conversions overwrite DPS310_PRS_B0..B2
and re-latch DPS310_INT_STS_PRS. The scan then pairs the overwritten
pressure sample with the earlier data->irq_timestamp, throttles capture to
the temperature rate, and leaves DPS310_INT_STS_PRS latched on return,
which causes a spurious level interrupt or stalls edge-triggered
interrupts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114083828.18059-1-zoone.rupert@gmail.com?part=2