[PATCH] iio: trigger: Add validation to reject devices requiring top half

[PATCH] iio: trigger: Add validation to reject devices requiring top half

[Gyeyoung Baek]

Some device drivers implement top-half handler,
which is not compatible with threaded handler trigger.
This patch adds a validation function to reject such devices,
allowing only iio_pollfunc_store_time().

Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
 drivers/iio/trigger/iio-trig-loop.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/trigger/iio-trig-loop.c b/drivers/iio/trigger/iio-trig-loop.c
index 7aaed0611899..a37615567a6c 100644
--- a/drivers/iio/trigger/iio-trig-loop.c
+++ b/drivers/iio/trigger/iio-trig-loop.c
@@ -7,18 +7,12 @@
  *
  * Note this is still rather experimental and may eat babies.
  *
- * Todo
- * * Protect against connection of devices that 'need' the top half
- *   handler.
- * * Work out how to run top half handlers in this context if it is
- *   safe to do so (timestamp grabbing for example)
- *
  * Tested against a max1363. Used about 33% cpu for the thread and 20%
  * for generic_buffer piping to /dev/null. Watermark set at 64 on a 128
  * element kfifo buffer.
  */
 
-#include <linux/kernel.h>
+#include <linux/errno.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -27,8 +21,10 @@
 #include <linux/freezer.h>
 
 #include <linux/iio/iio.h>
-#include <linux/iio/trigger.h>
 #include <linux/iio/sw_trigger.h>
+#include <linux/iio/trigger.h>
+
+#include "linux/iio/trigger_consumer.h"
 
 struct iio_loop_info {
 	struct iio_sw_trigger swt;
@@ -71,8 +67,25 @@ static int iio_loop_trigger_set_state(struct iio_trigger *trig, bool state)
 	return 0;
 }
 
+/*
+ * Protect against connection of devices that 'need' the top half
+ * handler.
+ */
+static int iio_loop_trigger_validate_device(struct iio_trigger *trig,
+						struct iio_dev *indio_dev)
+{
+	struct iio_poll_func *pf = indio_dev->pollfunc;
+
+	/* Only iio timestamp grabbing is allowed. */
+	if (pf->h && pf->h != iio_pollfunc_store_time)
+		return -EINVAL;
+
+	return 0;
+}
+
 static const struct iio_trigger_ops iio_loop_trigger_ops = {
 	.set_trigger_state = iio_loop_trigger_set_state,
+	.validate_device = iio_loop_trigger_validate_device,
 };
 
 static struct iio_sw_trigger *iio_trig_loop_probe(const char *name)
-- 
2.43.0

Re: [PATCH] iio: trigger: Add validation to reject devices requiring top half

[Gyeyoung Baek]

On Sun, May 4, 2025 at 4:00 AM Gyeyoung Baek <gye976@gmail.com> wrote:
>

> +{
> +       struct iio_poll_func *pf = indio_dev->pollfunc;

Sorry, it should be `indio_dev->pollfunc_event` instead of
`indio_dev->pollfunc`. Please ignore this patch.

> +
> +       /* Only iio timestamp grabbing is allowed. */
> +       if (pf->h && pf->h != iio_pollfunc_store_time)
> +               return -EINVAL;
> +
> +       return 0;
> +}

Re: [PATCH] iio: trigger: Add validation to reject devices requiring top half

[Jonathan Cameron]

On Sun,  4 May 2025 04:00:43 +0900
Gyeyoung Baek <gye976@gmail.com> wrote:

> Some device drivers implement top-half handler,
> which is not compatible with threaded handler trigger.
> This patch adds a validation function to reject such devices,
> allowing only iio_pollfunc_store_time().

This needs more reasoning.  What makes it not work?
+ what do we mean by not compatible?
I'd expect at least a reference to it using iio_trigger_poll_nested()
directly.

It's unfortunately hard to tell whether a top half handler is
actually needed or not.  As a follow up question, what cases do we have
of top half / interrupt context handlers other than iio_pollfunc_store_time()?

Maybe we don't need this code to be this complex any more at all
(i.e. could it become a flag to say whether the timestamp is useful or not)
rather than registering the callback.

Jonathan


> 
> Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
> ---
>  drivers/iio/trigger/iio-trig-loop.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/trigger/iio-trig-loop.c b/drivers/iio/trigger/iio-trig-loop.c
> index 7aaed0611899..a37615567a6c 100644
> --- a/drivers/iio/trigger/iio-trig-loop.c
> +++ b/drivers/iio/trigger/iio-trig-loop.c
> @@ -7,18 +7,12 @@
>   *
>   * Note this is still rather experimental and may eat babies.
>   *
> - * Todo
> - * * Protect against connection of devices that 'need' the top half
> - *   handler.
> - * * Work out how to run top half handlers in this context if it is
> - *   safe to do so (timestamp grabbing for example)
> - *
>   * Tested against a max1363. Used about 33% cpu for the thread and 20%
>   * for generic_buffer piping to /dev/null. Watermark set at 64 on a 128
>   * element kfifo buffer.
>   */
>  
> -#include <linux/kernel.h>
> +#include <linux/errno.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> @@ -27,8 +21,10 @@
>  #include <linux/freezer.h>
>  
>  #include <linux/iio/iio.h>
> -#include <linux/iio/trigger.h>
>  #include <linux/iio/sw_trigger.h>
> +#include <linux/iio/trigger.h>
> +

Improving header ordering / which headers are here is good to do, but
it needs to be in a separate patch.

> +#include "linux/iio/trigger_consumer.h"
>  
>  struct iio_loop_info {
>  	struct iio_sw_trigger swt;
> @@ -71,8 +67,25 @@ static int iio_loop_trigger_set_state(struct iio_trigger *trig, bool state)
>  	return 0;
>  }
>  
> +/*
> + * Protect against connection of devices that 'need' the top half
> + * handler.
> + */
> +static int iio_loop_trigger_validate_device(struct iio_trigger *trig,
> +						struct iio_dev *indio_dev)
> +{
> +	struct iio_poll_func *pf = indio_dev->pollfunc;
> +
> +	/* Only iio timestamp grabbing is allowed. */
> +	if (pf->h && pf->h != iio_pollfunc_store_time)

Why is iio_pollfunc_store_time useable here?  It's not going to store the
time if we don't call it...  We could special case it probably but we'd
need to ensure the call is actually made.


> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>  static const struct iio_trigger_ops iio_loop_trigger_ops = {
>  	.set_trigger_state = iio_loop_trigger_set_state,
> +	.validate_device = iio_loop_trigger_validate_device,
>  };
>  
>  static struct iio_sw_trigger *iio_trig_loop_probe(const char *name)

Re: [PATCH] iio: trigger: Add validation to reject devices requiring top half

[Gyeyoung Baek]

Hello Jonathan, thank you for the review.
I would appreciate it if you could review my additional comments.

On Sun, May 4, 2025 at 11:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sun,  4 May 2025 04:00:43 +0900
> Gyeyoung Baek <gye976@gmail.com> wrote:
>
> > Some device drivers implement top-half handler,
> > which is not compatible with threaded handler trigger.
> > This patch adds a validation function to reject such devices,
> > allowing only iio_pollfunc_store_time().
>
> This needs more reasoning.  What makes it not work?
> + what do we mean by not compatible?
> I'd expect at least a reference to it using iio_trigger_poll_nested()
> directly.

Of course, even if the IIO device registers a top-half,
`iio_trigger_poll_nested()` ignores the top-half and only calls the
bottom-half, so it works properly.
What I misunderstood here is that I thought there were other IIO
devices implementing a top-half other than
`iio_pollfunc_store_time()`. So I assumed that the TODO was to block
those.
I had confused it with the IIO trigger's top-half handler, apologies
for the confusion.

---

> It's unfortunately hard to tell whether a top half handler is
> actually needed or not.  As a follow up question, what cases do we have
> of top half / interrupt context handlers other than iio_pollfunc_store_time()?

No, it seems that `iio_pollfunc_store_time()` is the only top-half
handler for IIO devices.

---

> Maybe we don't need this code to be this complex any more at all
> (i.e. could it become a flag to say whether the timestamp is useful or not)
> rather than registering the callback.

my new understanding of TODO is as follows:
    - Since `iio_loop_thread()` can only call
`iio_trigger_poll_nested()` and not `iio_trigger_poll()`,
      if the connected IIO device expects a top half such as
`iio_pollfunc_store_time()`,
      then `iio_loop_thread()` needs to directly call
`iio_pollfunc_store_time().`

Would my understanding be correct?

---

> >
> > +/*
> > + * Protect against connection of devices that 'need' the top half
> > + * handler.
> > + */
> > +static int iio_loop_trigger_validate_device(struct iio_trigger *trig,
> > +                                             struct iio_dev *indio_dev)
> > +{
> > +     struct iio_poll_func *pf = indio_dev->pollfunc;
> > +
> > +     /* Only iio timestamp grabbing is allowed. */
> > +     if (pf->h && pf->h != iio_pollfunc_store_time)
>
> Why is iio_pollfunc_store_time useable here?  It's not going to store the
> time if we don't call it...  We could special case it probably but we'd
> need to ensure the call is actually made.

Yes, If my new understanding is correct, `iio_loop_thread()` needs to
call `iio_pollfunc_store_time()` directly,
depending on whether the IIO device's top-half is NULL or
`iio_pollfunc_store_time()`.

--
Regards,
Gyeyoung

Re: [PATCH] iio: trigger: Add validation to reject devices requiring top half

[Jonathan Cameron]

On Wed, 7 May 2025 00:55:27 +0900
Gyeyoung Baek <gye976@gmail.com> wrote:

> Hello Jonathan, thank you for the review.
> I would appreciate it if you could review my additional comments.
> 
> On Sun, May 4, 2025 at 11:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Sun,  4 May 2025 04:00:43 +0900
> > Gyeyoung Baek <gye976@gmail.com> wrote:
> >  
> > > Some device drivers implement top-half handler,
> > > which is not compatible with threaded handler trigger.
> > > This patch adds a validation function to reject such devices,
> > > allowing only iio_pollfunc_store_time().  
> >
> > This needs more reasoning.  What makes it not work?
> > + what do we mean by not compatible?
> > I'd expect at least a reference to it using iio_trigger_poll_nested()
> > directly.  
> 
> Of course, even if the IIO device registers a top-half,
> `iio_trigger_poll_nested()` ignores the top-half and only calls the
> bottom-half, so it works properly.
> What I misunderstood here is that I thought there were other IIO
> devices implementing a top-half other than
> `iio_pollfunc_store_time()`. So I assumed that the TODO was to block
> those.
> I had confused it with the IIO trigger's top-half handler, apologies
> for the confusion.

There were...  I'm rather surprised that there are none any more!

> 
> ---
> 
> > It's unfortunately hard to tell whether a top half handler is
> > actually needed or not.  As a follow up question, what cases do we have
> > of top half / interrupt context handlers other than iio_pollfunc_store_time()?  
> 
> No, it seems that `iio_pollfunc_store_time()` is the only top-half
> handler for IIO devices.
Excellent.

> 
> ---
> 
> > Maybe we don't need this code to be this complex any more at all
> > (i.e. could it become a flag to say whether the timestamp is useful or not)
> > rather than registering the callback.  
> 
> my new understanding of TODO is as follows:
>     - Since `iio_loop_thread()` can only call
> `iio_trigger_poll_nested()` and not `iio_trigger_poll()`,
>       if the connected IIO device expects a top half such as
> `iio_pollfunc_store_time()`,
>       then `iio_loop_thread()` needs to directly call
> `iio_pollfunc_store_time().`

I'd take a different approach (slightly) though it's more effort.

Step 1. Tidy up current situation.

Patch to convert all existing calls to devm_iio_triggered_buffer_setup()
and iio_triggered_buffer_setup() to not take a top half function but replace
that variable with a bool early_timestamp or something along those lines.
Replace the h in struct iio_poll_func with a similarly named bool.
Bunch of plumbing to make that all get filled in correctly.

Then in iio_trigger_attach_pollfunc() check that bool and if appropriate
pass iio_pollfunc_store_time() it to request_threaded_irq()

Step 2. Make what you want work cleanly now we only have that one handler.

In iio_trigger_poll_nested() we can't know if that flag is set and I'm not
really keen on trying to get to this from elsewhere. We have previously considered
solving this case via whether the timestamp is set or not in the threaded
handler. I've never like that much as in theory timestamp 0 is valid (was
a while ago). The rpr0521 light sensor has handling for this.

I wonder if the following would work.

In iio_trigger_attach_poll_func() we have access to the trigger and
the pollfunc.  So if the pollfunc flag for wanting an early timestamp is set and
we know the trigger is going to use iio_poll_trigger_nested() then we could
wrap the registered handler in a local one that calls the iio_pollfunc_store_time()

The additional magic needed here is that today we don't know that about the trigger.
So we'd need to add a bool to the struct iio_trig to indicate it and set that
for all drivers that use iio_trigger_poll_nested()   bool nested; will do.

It's not perfect as there are driver that do iio_trigger_poll() and iio_trigger_poll_nested()
depending on path. To handle those we'd need a flag to say don't overwrite my timestamp.
at91-sama5d2-adc.c is the first one I found.

There are ways to make even that work but lets skip that for now as they'd
slightly complicate things. That driver won't call the timestamp capture in
some paths but it doesn't today so we are no worse off.

Jonathan


> 
> Would my understanding be correct?
> 
> ---
> 
> > >
> > > +/*
> > > + * Protect against connection of devices that 'need' the top half
> > > + * handler.
> > > + */
> > > +static int iio_loop_trigger_validate_device(struct iio_trigger *trig,
> > > +                                             struct iio_dev *indio_dev)
> > > +{
> > > +     struct iio_poll_func *pf = indio_dev->pollfunc;
> > > +
> > > +     /* Only iio timestamp grabbing is allowed. */
> > > +     if (pf->h && pf->h != iio_pollfunc_store_time)  
> >
> > Why is iio_pollfunc_store_time useable here?  It's not going to store the
> > time if we don't call it...  We could special case it probably but we'd
> > need to ensure the call is actually made.  
> 
> Yes, If my new understanding is correct, `iio_loop_thread()` needs to
> call `iio_pollfunc_store_time()` directly,
> depending on whether the IIO device's top-half is NULL or
> `iio_pollfunc_store_time()`.
Yes. But it doesn't have direct access to the required pollfunc.


> 
> --
> Regards,
> Gyeyoung
> 

Re: [PATCH] iio: trigger: Add validation to reject devices requiring top half

[Gyeyoung Baek]

Hello Jonathan, the example drivers you pointed out have been good references.
I appreciate your detailed comments.

--
Regards,
Gyeyoung

Re: [PATCH] iio: trigger: Add validation to reject devices requiring top half

[Gyeyoung Baek]

Hello Jonathan,
I’ve referred to your previous comments and implemented the ideas.
Thank you for your earlier feedback.
I now have a few follow-up questions and would appreciate your
thoughts on the below points.

On Thu, May 8, 2025 at 4:40 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed, 7 May 2025 00:55:27 +0900
> Gyeyoung Baek <gye976@gmail.com> wrote:

> I'd take a different approach (slightly) though it's more effort.
>
> Step 1. Tidy up current situation.
>
> Patch to convert all existing calls to devm_iio_triggered_buffer_setup()
> and iio_triggered_buffer_setup() to not take a top half function but replace
> that variable with a bool early_timestamp or something along those lines.
> Replace the h in struct iio_poll_func with a similarly named bool.
> Bunch of plumbing to make that all get filled in correctly.
>
> Then in iio_trigger_attach_pollfunc() check that bool and if appropriate
> pass iio_pollfunc_store_time() it to request_threaded_irq()

Now we have both the existing `devm_iio_triggered_buffer_setup()`,
and a new version with the additional arguments of that.
Should these two coexist for compatibility, or should the before one
be replaced by the new one?

> Step 2. Make what you want work cleanly now we only have that one handler.
>
> In iio_trigger_poll_nested() we can't know if that flag is set and I'm not
> really keen on trying to get to this from elsewhere. We have previously considered
> solving this case via whether the timestamp is set or not in the threaded
> handler. I've never like that much as in theory timestamp 0 is valid (was
> a while ago). The rpr0521 light sensor has handling for this.

What I'm trying to do is a mechanism where device drivers can
automatically get timestamps without manually handling them — simply
by setting a argument to indicate whether to capture the timestamp in
the tophalf or bottomhalf.

But there are cases like the rpr0521 where the driver sets the
timestamp manually within its own trigger.
Would it make sense to extend this to automatically set the timestamp
in cases where the driver uses its own trigger as well?
To that, I believe we would need a unified interface that can cover
all trigger types (e.g., interrupt, software trigger) that invoke
poll() or poll_nested().
Would it be the right direction?
Or would it be more appropriate to consider only the top/bottom of a
consumer device?

> I wonder if the following would work.
>
> In iio_trigger_attach_poll_func() we have access to the trigger and
> the pollfunc.  So if the pollfunc flag for wanting an early timestamp is set and
> we know the trigger is going to use iio_poll_trigger_nested() then we could
> wrap the registered handler in a local one that calls the iio_pollfunc_store_time()
>
> The additional magic needed here is that today we don't know that about the trigger.
> So we'd need to add a bool to the struct iio_trig to indicate it and set that
> for all drivers that use iio_trigger_poll_nested()   bool nested; will do.
>
> It's not perfect as there are driver that do iio_trigger_poll() and iio_trigger_poll_nested()
> depending on path. To handle those we'd need a flag to say don't overwrite my timestamp.
> at91-sama5d2-adc.c is the first one I found.

> There are ways to make even that work but lets skip that for now as they'd
> slightly complicate things. That driver won't call the timestamp capture in
> some paths but it doesn't today so we are no worse off.

--
Regards,
Gyeyoung

Re: [PATCH] iio: trigger: Add validation to reject devices requiring top half

[Jonathan Cameron]

On Mon, 12 May 2025 00:47:39 +0900
Gyeyoung Baek <gye976@gmail.com> wrote:

> Hello Jonathan,
> I’ve referred to your previous comments and implemented the ideas.
> Thank you for your earlier feedback.
> I now have a few follow-up questions and would appreciate your
> thoughts on the below points.
> 
> On Thu, May 8, 2025 at 4:40 AM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Wed, 7 May 2025 00:55:27 +0900
> > Gyeyoung Baek <gye976@gmail.com> wrote:  
> 
> > I'd take a different approach (slightly) though it's more effort.
> >
> > Step 1. Tidy up current situation.
> >
> > Patch to convert all existing calls to devm_iio_triggered_buffer_setup()
> > and iio_triggered_buffer_setup() to not take a top half function but replace
> > that variable with a bool early_timestamp or something along those lines.
> > Replace the h in struct iio_poll_func with a similarly named bool.
> > Bunch of plumbing to make that all get filled in correctly.
> >
> > Then in iio_trigger_attach_pollfunc() check that bool and if appropriate
> > pass iio_pollfunc_store_time() it to request_threaded_irq()  
> 
> Now we have both the existing `devm_iio_triggered_buffer_setup()`,
> and a new version with the additional arguments of that.
> Should these two coexist for compatibility, or should the before one
> be replaced by the new one?

So the aim will be to replace the existing function. 
How to get there is indeed an excellent question.

I'd want to do it in one go, but as it affects a lot of drivers a 
single patch is probably not appropriate.

So we'd introduce something like
devm_iio_triggered_buffer_setup2() with new parameters

Move everything over to that then a single patch to remove
the old function and rename it all back to the original.

Alternatively we could find a reasonable alternative name to avoid
that 'rename all at the end' patch.

devm_iio_triggered_buf_setup() perhaps?  

> 
> > Step 2. Make what you want work cleanly now we only have that one handler.
> >
> > In iio_trigger_poll_nested() we can't know if that flag is set and I'm not
> > really keen on trying to get to this from elsewhere. We have previously considered
> > solving this case via whether the timestamp is set or not in the threaded
> > handler. I've never like that much as in theory timestamp 0 is valid (was
> > a while ago). The rpr0521 light sensor has handling for this.  
> 
> What I'm trying to do is a mechanism where device drivers can
> automatically get timestamps without manually handling them — simply
> by setting a argument to indicate whether to capture the timestamp in
> the tophalf or bottomhalf.
> 
> But there are cases like the rpr0521 where the driver sets the
> timestamp manually within its own trigger.
> Would it make sense to extend this to automatically set the timestamp
> in cases where the driver uses its own trigger as well?
> To that, I believe we would need a unified interface that can cover
> all trigger types (e.g., interrupt, software trigger) that invoke
> poll() or poll_nested().
> Would it be the right direction?
> Or would it be more appropriate to consider only the top/bottom of a
> consumer device?

I'd like to avoid grabbing the timestamp for particular drivers
that never need it (as they want to grab it in the thread for
reasons of how they work - typically the capture only starts
when then write to the device).  Other than that I'm not against
having it grabbed in drivers that 'sometimes' need it whether
or not it turns out they do.

The dance is that we can't see the right information in poll / poll_nested
so the best we can do is see if someone already filled in the
timestamp in the handler we are currently running.  For that we need
a flag alongside the pollfunc timestamp.  Care will be needed to ensure
there are no races though as we might clear that flag just after another
top half interrupt has been taken on a different cpu core.

I'm not sure yet exactly how this will work.  Needs some experimenting
and thought.

J
> 
> > I wonder if the following would work.
> >
> > In iio_trigger_attach_poll_func() we have access to the trigger and
> > the pollfunc.  So if the pollfunc flag for wanting an early timestamp is set and
> > we know the trigger is going to use iio_poll_trigger_nested() then we could
> > wrap the registered handler in a local one that calls the iio_pollfunc_store_time()
> >
> > The additional magic needed here is that today we don't know that about the trigger.
> > So we'd need to add a bool to the struct iio_trig to indicate it and set that
> > for all drivers that use iio_trigger_poll_nested()   bool nested; will do.
> >
> > It's not perfect as there are driver that do iio_trigger_poll() and iio_trigger_poll_nested()
> > depending on path. To handle those we'd need a flag to say don't overwrite my timestamp.
> > at91-sama5d2-adc.c is the first one I found.  
> 
> > There are ways to make even that work but lets skip that for now as they'd
> > slightly complicate things. That driver won't call the timestamp capture in
> > some paths but it doesn't today so we are no worse off.  
> 
> --
> Regards,
> Gyeyoung

Re: [PATCH] iio: trigger: Add validation to reject devices requiring top half

[Gyeyoung Baek]

On Thu, May 15, 2025 at 4:49 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 12 May 2025 00:47:39 +0900
> Gyeyoung Baek <gye976@gmail.com> wrote:
>
> > Hello Jonathan,
> > I’ve referred to your previous comments and implemented the ideas.
> > Thank you for your earlier feedback.
> > I now have a few follow-up questions and would appreciate your
> > thoughts on the below points.
> >
> > On Thu, May 8, 2025 at 4:40 AM Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > On Wed, 7 May 2025 00:55:27 +0900
> > > Gyeyoung Baek <gye976@gmail.com> wrote:
> >
> > > I'd take a different approach (slightly) though it's more effort.
> > >
> > > Step 1. Tidy up current situation.
> > >
> > > Patch to convert all existing calls to devm_iio_triggered_buffer_setup()
> > > and iio_triggered_buffer_setup() to not take a top half function but replace
> > > that variable with a bool early_timestamp or something along those lines.
> > > Replace the h in struct iio_poll_func with a similarly named bool.
> > > Bunch of plumbing to make that all get filled in correctly.
> > >
> > > Then in iio_trigger_attach_pollfunc() check that bool and if appropriate
> > > pass iio_pollfunc_store_time() it to request_threaded_irq()
> >
> > Now we have both the existing `devm_iio_triggered_buffer_setup()`,
> > and a new version with the additional arguments of that.
> > Should these two coexist for compatibility, or should the before one
> > be replaced by the new one?
>
> So the aim will be to replace the existing function.
> How to get there is indeed an excellent question.
>
> I'd want to do it in one go, but as it affects a lot of drivers a
> single patch is probably not appropriate.
>
> So we'd introduce something like
> devm_iio_triggered_buffer_setup2() with new parameters
>
> Move everything over to that then a single patch to remove
> the old function and rename it all back to the original.
>
> Alternatively we could find a reasonable alternative name to avoid
> that 'rename all at the end' patch.
>
> devm_iio_triggered_buf_setup() perhaps?
>
> >
> > > Step 2. Make what you want work cleanly now we only have that one handler.
> > >
> > > In iio_trigger_poll_nested() we can't know if that flag is set and I'm not
> > > really keen on trying to get to this from elsewhere. We have previously considered
> > > solving this case via whether the timestamp is set or not in the threaded
> > > handler. I've never like that much as in theory timestamp 0 is valid (was
> > > a while ago). The rpr0521 light sensor has handling for this.
> >
> > What I'm trying to do is a mechanism where device drivers can
> > automatically get timestamps without manually handling them — simply
> > by setting a argument to indicate whether to capture the timestamp in
> > the tophalf or bottomhalf.
> >
> > But there are cases like the rpr0521 where the driver sets the
> > timestamp manually within its own trigger.
> > Would it make sense to extend this to automatically set the timestamp
> > in cases where the driver uses its own trigger as well?
> > To that, I believe we would need a unified interface that can cover
> > all trigger types (e.g., interrupt, software trigger) that invoke
> > poll() or poll_nested().
> > Would it be the right direction?
> > Or would it be more appropriate to consider only the top/bottom of a
> > consumer device?
>
> I'd like to avoid grabbing the timestamp for particular drivers
> that never need it (as they want to grab it in the thread for
> reasons of how they work - typically the capture only starts
> when then write to the device).  Other than that I'm not against
> having it grabbed in drivers that 'sometimes' need it whether
> or not it turns out they do.
>
> The dance is that we can't see the right information in poll / poll_nested
> so the best we can do is see if someone already filled in the
> timestamp in the handler we are currently running.  For that we need
> a flag alongside the pollfunc timestamp.  Care will be needed to ensure
> there are no races though as we might clear that flag just after another
> top half interrupt has been taken on a different cpu core.
>
> I'm not sure yet exactly how this will work.  Needs some experimenting
> and thought.

I think it would be possible to avoid checking whether the timestamp
was overwritten, by explicitly specifying which of poll() or
poll_nested() is being called in the `iio_trigger` structure.
I have submitted an RFC patch on this, and I would greatly appreciate
it if you could review it whenever you have time. Thanks!

Best regards,
Gyeyoung