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

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 23 Aug 2026 19:21:01 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sat, 22 Aug 2026 02:46:54 +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
> > own buffer consumer does not need that context: the only thing the
> > pollfunc top half, iio_pollfunc_store_time(), 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. 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.
> > 
> > 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.
> > 
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > Assisted-by: Codex:gpt-5.6
> > Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>  
> Thanks.  This looks good to me, but will need to wait for the fix
> to being upstream and for me to merge an rc into the main togreg
> branch of iio.git.  So at least a few weeks, maybe longer.
> 
Applied.
> Thanks,
> 
> Jonathan
> 
> > ---
> > Applies on top of "iio: light: gp2ap020a00f: drain irq_work after
> > free_irq" in the fixes-togreg branch of iio.git; this drops the
> > irq_work_sync() drains that patch added.
> > 
> > v2:
> >   - commit message rework only, per maintainer review; code unchanged.
> > v1: https://lore.kernel.org/linux-iio/20260817024023.291319-1-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);
> >  }
> > 
> >   
> 
>