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