linux-kernel - Re: [PATCH v2 07/22] iio: accel: adxl345: initialize IRQ number

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: <20241124181428.59b85176@jic23-huawei>
Date: Sun, 24 Nov 2024 18:14:28 +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 07/22] iio: accel: adxl345: initialize IRQ number

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

> Add the possibility to claim an interrupt and init the state structure
> with interrupt number and interrupt line to use. The adxl345 can use
> two different interrupt lines, mainly to signal FIFO watermark events,
> single or double tap, activity, etc. Hence, having the interrupt line
> available is crucial to implement such features.

If there are two interrupt lines, you need to be more clever.
Imagine only one of them is wired. How do you know which one it is?

The query needs to be done by name.  When there are multiple interrupts
the ones found in spi and i2c structures could be anything, so don't use
those.

See fwnode_irq_get_by_name()


> 
> Signed-off-by: Lothar Rubusch <l.rubusch@...il.com>
> ---
>  drivers/iio/accel/adxl345.h      | 1 +
>  drivers/iio/accel/adxl345_core.c | 6 ++++++
>  drivers/iio/accel/adxl345_i2c.c  | 2 +-
>  drivers/iio/accel/adxl345_spi.c  | 8 ++++++--
>  4 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
> index 3d5c8719db..cf4132715c 100644
> --- a/drivers/iio/accel/adxl345.h
> +++ b/drivers/iio/accel/adxl345.h
> @@ -62,6 +62,7 @@ struct adxl345_chip_info {
>  };
>  
>  int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> +		       int irq,
>  		       int (*setup)(struct device*, struct regmap*));
>  
>  #endif /* _ADXL345_H_ */
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 81688a9eaf..902bd3568b 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -11,6 +11,7 @@
>  #include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/units.h>
> +#include <linux/interrupt.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> @@ -18,6 +19,7 @@
>  #include "adxl345.h"
>  
>  struct adxl34x_state {
> +	int irq;
>  	const struct adxl345_chip_info *info;
>  	struct regmap *regmap;
>  };
> @@ -196,12 +198,14 @@ static const struct iio_info adxl345_info = {
>   *                        also covers the adxl375 and adxl346 accelerometer
>   * @dev:	Driver model representation of the device
>   * @regmap:	Regmap instance for the device
> + * @irq:	Interrupt handling for async usage

This is an integer, not a handling.   See if you can come up with clearer comment.

>   * @setup:	Setup routine to be executed right before the standard device
>   *		setup
>   *
>   * Return: 0 on success, negative errno on error
>   */
>  int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> +		       int irq,
>  		       int (*setup)(struct device*, struct regmap*))
>  {
>  	struct adxl34x_state *st;
> @@ -224,6 +228,8 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
>  
>  	st = iio_priv(indio_dev);
>  	st->regmap = regmap;
> +
> +	st->irq = irq;
>  	st->info = device_get_match_data(dev);
>  	if (!st->info)
>  		return -ENODEV;
> diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c
> index 4065b8f7c8..604b706c29 100644
> --- a/drivers/iio/accel/adxl345_i2c.c
> +++ b/drivers/iio/accel/adxl345_i2c.c
> @@ -27,7 +27,7 @@ static int adxl345_i2c_probe(struct i2c_client *client)
>  	if (IS_ERR(regmap))
>  		return dev_err_probe(&client->dev, PTR_ERR(regmap), "Error initializing regmap\n");
>  
> -	return adxl345_core_probe(&client->dev, regmap, NULL);
> +	return adxl345_core_probe(&client->dev, regmap, client->irq, NULL);
>  }
>  
>  static const struct adxl345_chip_info adxl345_i2c_info = {
> diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
> index 61fd9a6f5f..39e7d71e1d 100644
> --- a/drivers/iio/accel/adxl345_spi.c
> +++ b/drivers/iio/accel/adxl345_spi.c
> @@ -39,9 +39,13 @@ static int adxl345_spi_probe(struct spi_device *spi)
>  		return dev_err_probe(&spi->dev, PTR_ERR(regmap), "Error initializing regmap\n");
>  
>  	if (spi->mode & SPI_3WIRE)
> -		return adxl345_core_probe(&spi->dev, regmap, adxl345_spi_setup);
> +		return adxl345_core_probe(&spi->dev, regmap,
> +					  spi->irq,
> +					  adxl345_spi_setup);
Very early wrap. I think spi->irq fits on the line above.

>  	else
> -		return adxl345_core_probe(&spi->dev, regmap, NULL);
> +		return adxl345_core_probe(&spi->dev, regmap,
> +					  spi->irq,
> +					  NULL);
>  }
>  
>  static const struct adxl345_chip_info adxl345_spi_info = {