Re: [PATCH] iio: light: gp2ap020a00f: use iio_trigger_poll_nested()

[Jonathan Cameron]

On Mon, 17 Aug 2026 02:40:23 +0000
Fan Wu <fanwu01@xxxxxxxxxx> wrote:


> The threaded IRQ handler queues an irq_work only so that
> iio_trigger_poll() can run from a hardirq-like context.  The driver's

A lot of spaces before that The!

> own buffer consumer does not need that context: the only thing a
> standard IIO pollfunc top half does is store a timestamp in

Call out what function is.  It isn't obviously a 'standard' even
though there is only one that anyone actually uses.

> pf->timestamp, and deferring through the per-CPU irq_work queue moves
> that timestamp further away from the actual event.

This should mention in passing the possibility that this trigger is
being used to trigger capture on another device that is relying on
that pollfunc timestamp.  Then add something about that being considered
unlikely because there are few uses for synchronising data capture
between a light sensor and any other sensors.  Something like

"Whilst in theory there can be other consumers of this trigger relying on
 the pollfunc top half running, given this is a light sensor those are
 considered unlikely to exist in practice."



> 
> The device IRQ handler is already threaded, so call
> iio_trigger_poll_nested() directly from
> gp2ap020a00f_thresh_event_handler() and drop the irq_work, its
> callback, its init and the irq_work_sync() drains added by the
> preceding use-after-free fix.

We don't need all this detail in the commit description. Something like
"
Switch from irq_work to direct call of iio_trigger_poll_nested().
Remove now unnecessary irq_work related infrastructure and ensure a
local time stamp is acquired.
"
Covers the key stuff from this paragraph and the next.

> 
> As the trigger is now dispatched via handle_nested_irq(), a consumer's
> top half no longer runs and its threaded handler executes
> synchronously in this device's IRQ thread.  For consumers still using
> iio_pollfunc_store_time() this means pf->timestamp is no longer
> populated; a consumer that needs a timestamp should take it locally,
> as this driver now does in gp2ap020a00f_trigger_handler().
This hints at the possibility of other consumers - I'd call that out
above.
> 
> Suggested-by: Jonathan Cameron <jic23@xxxxxxxxxx>
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>

> ---

Down here add a note on the dependency so I don't forget about it!

Patch looks good to me. For v2, don't reply to the earlier patches.
That can rapidly get very confusing, so for IIO (and most / possibly
all of the rest of the kernel) separate threads preferred. Ideally
include a link instead to earlier versions as part of the change log.

Thanks for tidying this up!

Jonathan