linux-kernel - Re: [PATCH v1 04/12] iio: accel: adxl345: reorganize irq handler

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: <20250201164338.3359f306@jic23-huawei>
Date: Sat, 1 Feb 2025 16:43:38 +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 v1 04/12] iio: accel: adxl345: reorganize irq handler

On Tue, 28 Jan 2025 12:00:52 +0000
Lothar Rubusch <l.rubusch@...il.com> wrote:

> Reorganize the IRQ handler. Move the overrun handling to the bottom.
> Overrun leads to reset the interrupt register. This also happens at
> evaluation of a particular interrupt event. So, actually it makes more
> sense to evaluate the event if possible, and only fall back to pure
> overrun handling as a last resort. Further simplify fetching the
> interrupt status function. Both is in preparation to build interrupt

Both are preparatory steps to build

> handling up for the handling of different detected events, implemented
> in follow up patches.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@...il.com>

> ---
>  drivers/iio/accel/adxl345_core.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 8fbf0a43746f..7ee50a0b23ea 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -491,16 +491,9 @@ static const struct iio_buffer_setup_ops adxl345_buffer_ops = {
>  	.predisable = adxl345_buffer_predisable,
>  };
>  
> -static int adxl345_get_status(struct adxl345_state *st)
> +static int adxl345_get_status(struct adxl345_state *st, unsigned int *int_stat)
>  {
> -	int ret;
> -	unsigned int regval;
> -
> -	ret = regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, &regval);
> -	if (ret < 0)
> -		return ret;
> -
> -	return FIELD_GET(ADXL345_REG_INT_SOURCE_MSK, regval);

Maybe worth commenting in the patch description that this is the whole register
anyway.  Makes it obvious why this isn't a functional change.

Also delete that MSK value as it is rather pointless!
> +	return regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, int_stat);
>  }
>  
>  static int adxl345_fifo_push(struct iio_dev *indio_dev,
> @@ -536,14 +529,10 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
>  	int int_stat;
>  	int samples;
>  
> -	int_stat = adxl345_get_status(st);
> -	if (int_stat <= 0)
> +	if (adxl345_get_status(st, &int_stat))
>  		return IRQ_NONE;
>  
> -	if (int_stat & ADXL345_INT_OVERRUN)
> -		goto err;
> -
> -	if (int_stat & ADXL345_INT_WATERMARK) {
> +	if (FIELD_GET(ADXL345_INT_WATERMARK, int_stat)) {
>  		samples = adxl345_get_samples(st);
>  		if (samples < 0)
>  			goto err;
> @@ -551,6 +540,10 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
>  		if (adxl345_fifo_push(indio_dev, samples) < 0)
>  			goto err;
>  	}
> +
> +	if (FIELD_GET(ADXL345_INT_OVERRUN, int_stat))
> +		goto err;
> +
>  	return IRQ_HANDLED;
>  
>  err: