Hello, [adding rmk to Cc as the docs state that he invented lazy disabling] On Thu, Oct 31, 2024 at 01:05:21PM +0100, Nuno Sá wrote: > On Thu, 2024-10-31 at 11:40 +0100, Uwe Kleine-König wrote: > > On Wed, Oct 30, 2024 at 08:44:29PM +0000, Jonathan Cameron wrote: > > > On Wed, 30 Oct 2024 14:04:58 +0100 > > > Nuno Sá <noname.nuno@xxxxxxxxx> wrote: > > > > Regarding this, I do share some of the concerns already raised by Jonathan. I fear > > > > that we're papering around an issue with the IRQ controller rather than being an > > > > issue with the device. When I look at irq_disable() docs [1], it feels that we're > > > > already doing what we're supposed to do. IOW, we disable the lazy approach so we > > > > *should* not get any pending IRQ. > > > > I think this is wrong and you always have to be prepared to see an irq > > triggering that became pending while masked. I did some research, here are my findings: https://www.kernel.org/doc/html/v6.12-rc6/core-api/genericirq.html#delayed-interrupt-disable reads: The interrupt is kept enabled and is masked in the flow handler when an interrupt event happens. This prevents losing edge interrupts on hardware which does not store an edge interrupt event while the interrupt is disabled at the hardware level. This suggests that lazy disabling is needed for some controllers that stop their event detection when disabled. I read that as: *Normally* an irq event gets pending in hardware while the irq is disabled. The lazy disable approach is expected to work fine always, the reason to implement non-lazy disabling is "only" a performance optimisation. See commit e9849777d0e27cdd2902805be51da73e7c79578c. With the DOUT/̅R̅D̅Y pin the ad7124 (and others) is in this "Unfortunately there are devices which do not allow the interrupt to be disabled easily at the device level." class. However that makes me wonder what is the difference between the irq_mask() and irq_disable() callbacks defined in struct irq_chip. I don't know, but there is a difference for sure. So please forgive me for (probably) using the terms disable and mask wrongly. Also I wonder what happens if a device driver calls irq_set_status_flags(myirq, IRQ_DISABLE_UNLAZY); when the respective irq controller is one of those that miss events while masked. Shouldn't that better be caught? Best regards Uwe
Attachment:
signature.asc
Description: PGP signature
