On Sat, Sep 28, 2024 at 04:15:27PM +0100, Jonathan Cameron wrote:
> On Sun, 22 Sep 2024 18:20:41 +0200
> Vasileios Amoiridis <vassilisamir@xxxxxxxxx> wrote:
>
> > The rpr0521_trigger_consumer_handler() is registered as the trigger
> > threaded handler in the devm_iio_triggered_buffer_setup() function.
> > This function is being called in 2 ways:
> >
> > a) when there is a registered trigger being trigger like sysfs or hrt.
> > The call of the trigger handler (which is the iio_pollfunc_store_time())
> > follows which saves the timestamp and then, wakes up the trigger
> > threaded handler.
> >
> > b) The irq handler is using the iio_trigger_poll_nested() which wakes
> > up the trigger threaded handler.
> >
> > In both cases, the pf->timestamp has already been assigned a value
> > so there is no need to check if it is 0, neither to 0 it after
> > the push to the buffer.
>
> Not quite right (I think). The caller of iio_trigger_poll_nested() might not
> be this driver. There are other potential triggers that are nested
> because of need to check some status register, but can still be used
> for other devices. In theory you could drive light capture of
> the as3935 for when you want to know how bright it was just after
> a lightening strike :)
>
> We don't have a general solution for timestamps when that
> happens, so this driver is papering over that with this code.
>
>
> Jonathan
>
Hi Jonathan,
Thank you very much for the reply! I see your point, I also think I
am wrong after all. I was just interested to see why this driver makes
it so different from the other ones when it is coming to trigger/irq
handling. Thank you very much for the explanation!
Cheers,
Vasilis
>
>
> >
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@xxxxxxxxx>
> > ---
> > drivers/iio/light/rpr0521.c | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
> > index 56f5fbbf79ac..ae6a22b91b8d 100644
> > --- a/drivers/iio/light/rpr0521.c
> > +++ b/drivers/iio/light/rpr0521.c
> > @@ -446,13 +446,8 @@ static irqreturn_t rpr0521_trigger_consumer_handler(int irq, void *p)
> > int err;
> >
> > /* Use irq timestamp when reasonable. */
> > - if (iio_trigger_using_own(indio_dev) && data->irq_timestamp) {
> > + if (iio_trigger_using_own(indio_dev))
> > pf->timestamp = data->irq_timestamp;
> > - data->irq_timestamp = 0;
> > - }
> > - /* Other chained trigger polls get timestamp only here. */
> > - if (!pf->timestamp)
> > - pf->timestamp = iio_get_time_ns(indio_dev);
> >
> > err = regmap_bulk_read(data->regmap, RPR0521_REG_PXS_DATA,
> > data->scan.channels,
> > @@ -463,7 +458,6 @@ static irqreturn_t rpr0521_trigger_consumer_handler(int irq, void *p)
> > else
> > dev_err(&data->client->dev,
> > "Trigger consumer can't read from sensor.\n");
> > - pf->timestamp = 0;
> >
> > iio_trigger_notify_done(indio_dev->trig);
> >
>
