linux-kernel - Re: [PATCH v3 3/4] iio: light: ltr390: Interrupts and threshold event support

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20240928164516.548c923f@jic23-huawei>
Date: Sat, 28 Sep 2024 16:45:16 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Abhash Jha <abhashkumarjha123@...il.com>
Cc: linux-iio@...r.kernel.org, anshulusr@...il.com, lars@...afoo.de,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/4] iio: light: ltr390: Interrupts and threshold
 event support

On Sat, 14 Sep 2024 23:42:45 +0530
Abhash Jha <abhashkumarjha123@...il.com> wrote:

> Added support for threshold events for both the ALS and UVI channels.
> The events are reported when the threshold interrupt is triggered. Both
> rising and falling threshold types are supported.
> 
> Signed-off-by: Abhash Jha <abhashkumarjha123@...il.com>
I made a few more tweaks whilst picking this up.
Please take a look at the result.

Jonathan

> ---
>  drivers/iio/light/ltr390.c | 212 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 212 insertions(+)
> 
> diff --git a/drivers/iio/light/ltr390.c b/drivers/iio/light/ltr390.c
> index e152009b5..57bf48595 100644
> --- a/drivers/iio/light/ltr390.c
> +++ b/drivers/iio/light/ltr390.c
> @@ -25,8 +25,11 @@
>  #include <linux/regmap.h>
>  #include <linux/bitfield.h>
>  #include <linux/device.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
I moved these as well + cheeky move of the bitfield.h in here
as that was beginning to annoy me.

>  
>  #include <linux/iio/iio.h>
> +#include <linux/iio/events.h>

> +
>  static int ltr390_probe(struct i2c_client *client)
>  {
>  	struct ltr390_data *data;
> @@ -428,6 +627,19 @@ static int ltr390_probe(struct i2c_client *client)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to enable the sensor\n");
>  
> +	if (client->irq) {
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
we have dev, so I used it.
> +						NULL, ltr390_interrupt_handler,
> +						IRQF_TRIGGER_FALLING |
Dropped this specifying of direction. Direction should be set by firmware not the driver.
We have it done in some older drivers and can't fix that now, but for new
code we should never force it here.

> +						IRQF_ONESHOT,
> +						"ltr390_thresh_event",
> +						indio_dev);
> +		if (ret) {
> +			dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
return dev_err_probe(dev, ret, ...

in keeping with other local code and as it is generally nicer.
> +			return ret;
> +		}
> +	}
> +
>  	return devm_iio_device_register(dev, indio_dev);
>  }
>