Re: [PATCH v2 21/22] iio: accel: adxl345: sync FIFO reading with sensor

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 17 Nov 2024 18:26:50 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Pause the measurement while reading fifo values. Initially an interrupt
> is triggered if watermark of the FIFO is reached, or in case of
> OVERRUN. The sensor stays mute until FIFO is cleared and interrupts are
> read. Situations now can arise when the watermark is configured to a
> lower value. While reading the values, new values arrive such that a
> permanent OVERRUN state of the FIFO is reached, i.e. either the FIFO
> never gets emptied entirely because of permanently new arriving
> measurements. No more interrupts will be issued and the setup results
> in OVERRUN. To avoid such situation, stop measuring while solving an
> OVERRUN condition and generally reading FIFO entries.
Whilst solving overrun it makes sense.  Whilst dealing with a watermark
interrupt not so much.  The whole point of those is to allow capture
of data without missing samples. We should continue to acquire data
during the readback.

> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/iio/accel/adxl345_core.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 07c336211f..c79f0f5e3b 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -730,6 +730,11 @@ static irqreturn_t adxl345_trigger_handler(int irq, void *p)
>  
>  	if (int_stat & (ADXL345_INT_DATA_READY | ADXL345_INT_WATERMARK)) {
>  		pr_debug("%s(): WATERMARK or DATA_READY event detected\n", __func__);
> +
> +		/* Pause measuring, at low watermarks this would easily brick the
> +		 * sensor in permanent OVERRUN state
> +		 */
> +		adxl345_set_measure_en(st, false);
>  		if (adxl345_get_fifo_entries(st, &fifo_entries) < 0)
>  			goto err;
>  
> @@ -737,12 +742,15 @@ static irqreturn_t adxl345_trigger_handler(int irq, void *p)
>  			goto err;
>  
>  		iio_trigger_notify_done(indio_dev->trig);
> +		adxl345_set_measure_en(st, true);
>  	}
>  
>  	goto done;
>  err:
>  	iio_trigger_notify_done(indio_dev->trig);
> +	adxl345_set_measure_en(st, false);
>  	adxl345_empty_fifo(st);
> +	adxl345_set_measure_en(st, true);
>  	return IRQ_NONE;
>  
>  done: