The threaded IRQ handler queues an irq_work only so that
iio_trigger_poll() can run from a hardirq-like context.  The driver's
own buffer consumer does not need that context: the only thing a
standard IIO pollfunc top half does is store a timestamp in
pf->timestamp, and deferring through the per-CPU irq_work queue moves
that timestamp further away from the actual event.

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.

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().

Suggested-by: Jonathan Cameron <jic23@kernel.org>
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/iio/light/gp2ap020a00f.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 330d597..00164fd 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -37,7 +37,6 @@
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
-#include <linux/irq_work.h>
 #include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
@@ -245,7 +244,6 @@ struct gp2ap020a00f_data {
 	struct iio_trigger *trig;
 	struct regmap *regmap;
 	unsigned int thresh_val[4];
-	struct irq_work work;
 	wait_queue_head_t data_ready_queue;
 };
 
@@ -802,14 +800,6 @@ static void gp2ap020a00f_output_to_lux(struct gp2ap020a00f_data *data,
 		*output_val *= 16;
 }
 
-static void gp2ap020a00f_iio_trigger_work(struct irq_work *work)
-{
-	struct gp2ap020a00f_data *data =
-		container_of(work, struct gp2ap020a00f_data, work);
-
-	iio_trigger_poll(data->trig);
-}
-
 static irqreturn_t gp2ap020a00f_prox_sensing_handler(int irq, void *data)
 {
 	struct iio_dev *indio_dev = data;
@@ -932,8 +922,7 @@ static irqreturn_t gp2ap020a00f_thresh_event_handler(int irq, void *data)
 	if (test_bit(GP2AP020A00F_FLAG_ALS_CLEAR_TRIGGER, &priv->flags) ||
 	    test_bit(GP2AP020A00F_FLAG_ALS_IR_TRIGGER, &priv->flags) ||
 	    test_bit(GP2AP020A00F_FLAG_PROX_TRIGGER, &priv->flags))
-		/* This fires off the trigger. */
-		irq_work_queue(&priv->work);
+		iio_trigger_poll_nested(priv->trig);
 
 done:
 	return IRQ_HANDLED;
@@ -944,9 +933,12 @@ static irqreturn_t gp2ap020a00f_trigger_handler(int irq, void *data)
 	struct iio_poll_func *pf = data;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct gp2ap020a00f_data *priv = iio_priv(indio_dev);
+	s64 timestamp;
 	size_t d_size = 0;
 	int i, out_val, ret;
 
+	timestamp = iio_get_time_ns(indio_dev);
+
 	iio_for_each_active_channel(indio_dev, i) {
 		ret = regmap_bulk_read(priv->regmap, GP2AP020A00F_DATA_REG(i),
 				       &priv->buffer[d_size], 2);
@@ -964,7 +956,7 @@ static irqreturn_t gp2ap020a00f_trigger_handler(int irq, void *data)
 		}
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, priv->buffer, pf->timestamp);
+	iio_push_to_buffers_with_timestamp(indio_dev, priv->buffer, timestamp);
 done:
 	iio_trigger_notify_done(indio_dev->trig);
 
@@ -1455,8 +1447,9 @@ static int gp2ap020a00f_probe(struct i2c_client *client)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	/* Allocate buffer */
-	err = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time,
-		&gp2ap020a00f_trigger_handler, &gp2ap020a00f_buffer_setup_ops);
+	err = iio_triggered_buffer_setup(indio_dev, NULL,
+					 &gp2ap020a00f_trigger_handler,
+					 &gp2ap020a00f_buffer_setup_ops);
 	if (err < 0)
 		goto error_regulator_disable;
 
@@ -1480,8 +1473,6 @@ static int gp2ap020a00f_probe(struct i2c_client *client)
 		goto error_uninit_buffer;
 	}
 
-	init_irq_work(&data->work, gp2ap020a00f_iio_trigger_work);
-
 	err = iio_trigger_register(data->trig);
 	if (err < 0) {
 		dev_err(dev, "Failed to register iio trigger.\n");
@@ -1498,7 +1489,6 @@ error_trigger_unregister:
 	iio_trigger_unregister(data->trig);
 error_free_irq:
 	free_irq(client->irq, indio_dev);
-	irq_work_sync(&data->work);
 error_uninit_buffer:
 	iio_triggered_buffer_cleanup(indio_dev);
 error_regulator_disable:
@@ -1521,7 +1511,6 @@ static void gp2ap020a00f_remove(struct i2c_client *client)
 	iio_device_unregister(indio_dev);
 	iio_trigger_unregister(data->trig);
 	free_irq(client->irq, indio_dev);
-	irq_work_sync(&data->work);
 	iio_triggered_buffer_cleanup(indio_dev);
 	regulator_disable(data->vled_reg);
 }

On Mon, 17 Aug 2026 02:40:23 +0000
Fan Wu <fanwu01@zju.edu.cn> 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@kernel.org>
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>

> ---

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

On Sat, Aug 22, 2026 at 12:56:39AM +0100, Jonathan Cameron wrote:
> On Mon, 17 Aug 2026 02:40:23 +0000
> Fan Wu <fanwu01@zju.edu.cn> 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!

You mean two? This is an (old) standard. See also '  For' below
or '  Something'. I haven't commented on this, because it's consistent
over the text.

> > own buffer consumer does not need that context:

-- 
With Best Regards,
Andy Shevchenko