Re: [PATCH v3 4/5] iio: light: ROHM BU27008 color sensor

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 5/7/23 17:22, Jonathan Cameron wrote:

>>> If there is nothing to do in the actual interrupt as it's a data ready
>>> only signal, then you should just call iio_trigger_poll() in the top half and
>>> use devm_request_irq() only as there is no thread in this interrupt (though
>>> there is one for the interrupt below the software interrupt chip).
>>
>> I haven't tested this yet so please ignore me if I am writing nonsense -
>> but... The BU27008 will keep the IRQ line asserted until a register is
>> read. We can't read the register form HW-IRQ so we need to keep the IRQ
>> disabled until the threaded trigger handler is ran. With the setup we
>> have here, the IRQF_ONESHOT, took care of this. I assume that changing
>> to call the iio_poll_trigger() from top-half means I need to explicitly
>> disable the IRQ and re-enable it at the end of the trigger thread after
>> reading the register which debounces the IRQ line?
> 
> Hmm. I'm trying to remember how this works (wrote this a very long time ago).
> I'm fairly sure it's not an issue because we use IRQF_ONESHOT down one level
> so exercise the same prevention of the threads triggering multiple times etc > 
https://elixir.bootlin.com/linux/latest/source/drivers/iio/buffer/industrialio-triggered-buffer.c#L57
> 
> It doesn't matter if the device interrupt fires again as it will still be masked
> at our software irqchip level and will then get queued up and the thread will
> run again.

After reading this I am not at all sure I am using the trigger 
correctly. I see the iio_trigger_attach_poll_func() registering threaded 
handler with the IRQF_ONESHOT which is stored in the 
iio_alloc_pollfunc() as you pointed above.

The iio_trigger_attach_poll_func() is called from sysfs callback when 
trigger is enabled. So, if this is supposed to be the device IRQ, then I 
am not at all sure the driver should be requesting the IRQ at the 
probe(). If it is not the device's IRQ, then I guess the IRQF_ONESHOT 
passed in here won't help. I need to try seeing some examples how other 
drivers are using the triggers. Getting back to this tomorrow...

Yours,
	-- Matti.

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~