(Late response) On Wed, Jun 30, 2021 at 5:47 PM Pkshih <pkshih@xxxxxxxxxxx> wrote: > > -----Original Message----- > > From: Pkshih > > I read IRQ handler of rtw88 that looks much simpler, and I picture the > > new flow: > > > > int_handler int_threadfn napi_poll > > ----------- ------------ --------- > > | > > | > > | 1) dis. intr > > o | > > | 2) read interrupt status locally > > | 3) do TX reclaim > > | 4) check if RX? > > | 5) re-enable intr > > | (RX is optional) > > | 6) schedule_napi > > | (if RX) > > : >>>-------------------+ 7) (tasklet start immediately) > > : | > > : | 8) set 'doing RX' flag > > : | 9) do RX things > > : | 10) clear 'doing RX' flag > > : | 11) re-enable intr of RX > > : | > > : <<<-------------------o > > : > > o > > > > Step 2) read and clear interrupt status with spin_lock_irqsave, and use local > > variables to save the status. Then, the status will be correct, even more > > interrupts are triggered. > > > > Step 8)/10) add a 'doing RX' flag we don't enable RX interrupt in this period, so > > step 5) will not make a mistake to enable RX interrupt improperly. BTW, I think you might be pointing out a sort of bug with rtw88 -- a non-RX interrupt might cause RX interrupts to get re-enabled in the midst of what's *supposed* to be a NAPI poll. It's not a fatal functional problem or anything, but it does mean we might get excess RX interrupts, which can defeat the purpose of polling. I suppose the impact of such a bug depends on how frequently we're receiving other interrupts (say, H2C?). > > I attach the patch based on v5, and these changes will be included in v6. > > Further suggestions are welcome. > > > > Sorry, I missed the changes of pci.h, so send reference patch again. I haven't proven to myself that every line in there is good, but I think the general layout is better. I'm more likely to get a closer look+test when a v6 comes around. Thanks, Brian
