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

[Marten Lindahl <martenli@axis.com>]

On Mon, Jan 09, 2023 at 04:30:16PM +0100, Jonathan Cameron wrote:
> 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

Thanks for clarifying this. I will bind it to the irq_thread then.

Kind regards
Mårten
> 
> > 
> > Kind regards
> > Mårten
> > >   
>