Re: [PATCH 1/2] iio: light: vcnl4000: Make irq handling more generic

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 9 Jan 2023 12:32:10 +0100
Marten Lindahl <martenli@axis.com> wrote:
> > > +			if (ret < 0) {
> > > +				dev_err(&client->dev,
> > > +					"unable to setup iio triggered buffer\n");
> > > +				return ret;
> > > +			}
> > >  		}
> > >  
> > > -		ret = devm_request_threaded_irq(&client->dev, client->irq,
> > > -						NULL, vcnl4010_irq_thread,
> > > -						IRQF_TRIGGER_FALLING |
> > > -						IRQF_ONESHOT,
> > > -						"vcnl4010_irq",
> > > -						indio_dev);
> > > -		if (ret < 0) {
> > > -			dev_err(&client->dev, "irq request failed\n");
> > > -			return ret;
> > > +		if (data->chip_spec->irq_thread) {
> > > +			ret = devm_request_threaded_irq(&client->dev,
> > > +							client->irq, NULL,
> > > +							data->chip_spec->irq_thread,
> > > +							IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> > > +							"vcnl4000_irq",
> > > +							indio_dev);
> > > +			if (ret < 0) {
> > > +				dev_err(&client->dev, "irq request failed\n");
> > > +				return ret;
> > > +			}
> > >  		}
> > >  
> > >  		ret = vcnl4010_probe_trigger(indio_dev);  
> > Does it make sense to add the trigger even if we have no irq_thread?
> >   
> The irq_thread is dependent on the iio_event_interface, but I can not see that
> the trigger is dependent on the irq_thread. I am not sure of this.

The trigger sets up the infrastructure (under the hood it's a software
only demux of interrupts) to route to the registered consumers of the trigger.
That happens via iio_trigger_poll[_chained]() - the call in question is in the
irq handler, so whilst you can register the trigger without the irq_thread, it
won't do anything useful (hence I would not register it).

Jonathan

> 
> Kind regards
> Mårten
> >