Re: [PATCH RFC v1 1/2] iio: light: rpr0521: Use generic iio_pollfunc_store_time()

[Jonathan Cameron]

On Sat, 28 Sep 2024 16:10:17 +0100
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:

> On Sun, 22 Sep 2024 18:20:40 +0200
> Vasileios Amoiridis <vassilisamir@xxxxxxxxx> 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.
Two weeks gone. No one surfaced and I think this is fine.

Applied.

Jonathan

> 
> 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@xxxxxxxxx>
> > ---
> >  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) {  
> 
>