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 > > 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" > > Thanks > Daniel >
