Hi Daniel and Ján, > From: Ján Veselý > Sent: Friday, August 23, 2019 3:22 PM > On Fri, Aug 23, 2019 at 2:37 AM Daniel Drake <drake@xxxxxxxxxxxx> wrote: > > > > > + rtw_pci_disable_interrupt(rtwdev, rtwpci); > > > > I checked the discussion on the v1 patch thread but I still don't follow > > this. > > > > You're worried about the case where we're inside the interrupt handler and: > > 1. We read the interrupt status to note what needs to be done > > 2. <another interrupt arrives here, requiring other work to be done> > > 3. We clear the interrupt status bits > > 4. We proceed to handle the interrupt but missing any work requested by > > the interrupt in step 2. > > > > Is that right? > > > > I'm not an expert here, but I don't think this is something that drivers > > have to worry about. Surely the interrupt controller can be expected to > > have a mechanism to "queue up" any interrupt that arrives while an > > interrupt is being handled? Otherwise handling of all types of > > edge-triggered interrupts (not just MSI) would be overly painful across the > > board. > > That's my understanding as well. > entering the interrupt vector clears the IFLAG, so any interrupt will > wait until the IFLAG is restored, or delivered to a different CPU. > wouldn't it be safer to enable interrupts only _after_ registering the > handler in the "rtw_pci_request_irq" function? > > regards, > Jan Yes that's not something drivers need to care about. But I think it is Because there's a race condition between SW/HW when clearing the ISR. If interrupt comes after reading ISR and before write-1-clear, the interrupt controller would have interrupt status raised, and never issue interrupt signal to host when other new interrupts status are raised. To avoid this, driver requires to protect the ISR write-1-clear process by disabling the IMR. > > > > > > See e.g. https://patchwork.kernel.org/patch/3333681/ as a reference for > > what correct interrupt controller behaviour should look like. > > > > > + ret = pci_enable_msi(pdev); > > > > pci_enable_msi() is "deprecated, don't use" Do you mean I should remove this? But I cannot find another proper way to enable the MSI. Maybe pci_alloc_irq_vectors() could work but I am not sure if It is recommended. Yan-Hsuan
