On 4 December 2017 at 15:24, David Miller <da...@davemloft.net> wrote: > From: Heiner Kallweit <hkallwe...@gmail.com> > Date: Thu, 30 Nov 2017 23:55:15 +0100 > >> After commits c974bdbc3e "net: phy: Use threaded IRQ, to allow IRQ from >> sleeping devices" and 664fcf123a30 "net: phy: Threaded interrupts allow >> some simplification" all relevant code pieces run in process context >> anyway and I don't think we need the disabling of interrupts any longer. >> >> Interestingly enough, latter commit already removed the comment >> explaining why interrupts need to be temporarily disabled. >> >> On my system phy interrupt mode works fine with this patch. >> However I may miss something, especially in the context of shared phy >> interrupts, therefore I'd appreciate if more people could test this. >> >> Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com> >> Acked-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > > Ok, applied. > > But if this causes regressions I'm reverting.
Thanks. But please note that the code in question does seem to use the interrupt API incorrectly, and tbh, I was expecting some more discussion first. For reference, here's the commit log for the mostly equivalent patch [0] I sent out almost at the same time: """ The PHY interrupt handling code registers its interrupt as a oneshot threaded interrupt, which guarantees that the interrupt will be masked throughout both the primary and secondary handling stages. However, the handling code still disables the interrupt by calling disable_irq(), and re-enables it by calling enable_irq() after having acked the interrupt in the PHY hardware. This causes problems with hierarchical irqchip implementations built on top of the GIC, because the core threaded interrupt code will only EOI the interrupt if it is still masked after the secondary handler completes. If this is not the case, the EOI is not emitted, and the interrupt remains active, blocking further interrupts from the same source. Disabling and enabling the interrupt will result in the secondary handler completing with the interrupt unmasked, resulting in the above behavior. So remove the disable_irq/enable_irq, and rely on the fact that the interrupt remains masked already. """ [0] https://marc.info/?l=linux-netdev&m=151016412011661&w=2
