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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241124190949.373d2ac6@jic23-huawei>
Date: Sun, 24 Nov 2024 19:09:49 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Lothar Rubusch <l.rubusch@...il.com>
Cc: lars@...afoo.de, Michael.Hennerich@...log.com,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 eraretuya@...il.com
Subject: Re: [PATCH v2 21/22] iio: accel: adxl345: sync FIFO reading with
 sensor

On Sun, 17 Nov 2024 18:26:50 +0000
Lothar Rubusch <l.rubusch@...il.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@...il.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: