linux-kernel - Re: [PATCH RFC v1 1/2] iio: light: rpr0521: Use generic iio_pollfunc_store

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: <20240928161017.29fd249c@jic23-huawei>
Date: Sat, 28 Sep 2024 16:10:17 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Vasileios Amoiridis <vassilisamir@...il.com>
Cc: lars@...afoo.de, andy.shevchenko@...il.com,
 u.kleine-koenig@...gutronix.de, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v1 1/2] iio: light: rpr0521: Use generic
 iio_pollfunc_store_time()

On Sun, 22 Sep 2024 18:20:40 +0200
Vasileios Amoiridis <vassilisamir@...il.com> wrote:

> The custom rpr0521_trigger_consumer_store_time() is registered as trigger
> handler in the devm_iio_triggered_buffer_setup() function. This function
> is called from the calling of the iio_trigger_poll() used in the
> sysfs/hrt triggers and it is not used anywhere else in this driver.
It might be any number of other triggers (hardware triggers from other
drivers for example).

Other than that I think your reasoning is correct but would ideally
like some input from someone more familiar with this driver.

If that isn't forthcoming I'll pick this up in a week or two.

Jonathan

> 
> The irq handler of the driver is the rpr0521_drdy_irq_handler() which
> saves the timestamp and then wakes the irq thread. The irq thread is
> the rpr0521_drdy_irq_thread() function which checks if the irq came
> from the sensor and wakes up the trigger threaded handler through
> iio_trigger_poll_nested() or returns IRQ_NONE in case the irq didn't
> come from this sensor.
> 
> This means that in the current driver, you can't reach the
> rpr0521_trigger_consumer_store_time() when the device's irq is
> triggered. This means that the extra check of iio_trigger_using_own()
> is redundant since it will always be false so the general
> iio_pollfunc_store_time() can be used.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@...il.com>
> ---
>  drivers/iio/light/rpr0521.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
> index 78c08e0bd077..56f5fbbf79ac 100644
> --- a/drivers/iio/light/rpr0521.c
> +++ b/drivers/iio/light/rpr0521.c
> @@ -438,18 +438,6 @@ static irqreturn_t rpr0521_drdy_irq_thread(int irq, void *private)
>  	return IRQ_NONE;
>  }
>  
> -static irqreturn_t rpr0521_trigger_consumer_store_time(int irq, void *p)
> -{
> -	struct iio_poll_func *pf = p;
> -	struct iio_dev *indio_dev = pf->indio_dev;
> -
> -	/* Other trigger polls store time here. */
> -	if (!iio_trigger_using_own(indio_dev))
> -		pf->timestamp = iio_get_time_ns(indio_dev);
> -
> -	return IRQ_WAKE_THREAD;
> -}
> -
>  static irqreturn_t rpr0521_trigger_consumer_handler(int irq, void *p)
>  {
>  	struct iio_poll_func *pf = p;
> @@ -1016,7 +1004,7 @@ static int rpr0521_probe(struct i2c_client *client)
>  		/* Trigger consumer setup */
>  		ret = devm_iio_triggered_buffer_setup(indio_dev->dev.parent,
>  			indio_dev,
> -			rpr0521_trigger_consumer_store_time,
> +			iio_pollfunc_store_time,
>  			rpr0521_trigger_consumer_handler,
>  			&rpr0521_buffer_setup_ops);
>  		if (ret < 0) {