> On Behalf Of Brian Norris > On Thu, Aug 1, 2019 at 2:21 AM Tony Chuang <yhchuang@xxxxxxxxxxx> > wrote: > > > Subject: Re: [PATCH] rtw88: pci: enable MSI interrupt > > > On Tue, Jul 30, 2019 at 07:50:14PM +0800, yhchuang@xxxxxxxxxxx > wrote: > > > > --- a/drivers/net/wireless/realtek/rtw88/pci.c > > > > +++ b/drivers/net/wireless/realtek/rtw88/pci.c > > > > @@ -874,6 +878,7 @@ static irqreturn_t rtw_pci_interrupt_handler(int > irq, > > > void *dev) > > > > if (!rtwpci->irq_enabled) > > > > goto out; > > > > > > > > + rtw_pci_disable_interrupt(rtwdev, rtwpci); > > > > > > Why exactly do you have to mask interrupts during the ISR? Is there a > > > race in rtw_pci_irq_recognized() or something? > > > > > > I think there is a race between SW and HW, if we do not stop the > > IRQ first, write 1 clear will make the interrupt to be lost. > > This doesn't need to slow down this patch (I think v2 is fine), but I > still don't quite understand. Before this addition, the sequence is: > (a) read out your IRQ status > (b) ack the un-masked IRQs you see > (c) operate on those IRQs > > Even if a new IRQ comes in the middle of (b), shouldn't it be > sufficient to move on to (c), where you're still prepared to handle > that IRQ? > > Or if the IRQ comes after (b), you won't ACK it, and you should > immediately get a new IRQ after you return? I think it's because that MSI interrupts are edge-triggered. If the interrupt comes when IRQ is being processed, the interrupt won't be received. If the interrupt is not received, the interrupt won't be Write-1-Cleared, and won't be fired again. So driver should disable the interrupt until the ISRs are done. > > I guess that's assuming that these registers are Write 1 to Clear. But > if so, that means rtw_pci_irq_recognized() is effectively atomic, no? > > Also, somewhat unrelated: but why do you unmask HIMR1, when you're not > actually handling any of its IRQ bits? We could use HIMR1, just not handling any of them now :) > > Brian > Yan-Hsuan
