at 00:51, Brian Norris <briannorris@xxxxxxxxxxxx> wrote:
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 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?
According to user feedback [1], this patch makes rtw88 work.
I’ll make it merged into Ubuntu’s kernel for now, hopefully this will be in
upstream version soon.
[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1838133/comments/48
Kai-Heng
Brian
rtw_pci_irq_recognized(rtwdev, rtwpci, irq_status);
if (irq_status[0] & IMR_MGNTDOK)