Hi,
I tried to figure out the reason for the lost Tx receive interrupt
by comparing iwm with iwlwifi and I think our handling of the
periodic RX interrupt is a bit off.
In linux on receive of any of the possible RX interrupts the periodic
interrupt is disabled and then reenabled. As far as I understand this
is to make sure that no matter what kind of RX interrupt was received,
we make sure there will be another one in 8 ms when the newly activated
periodic interrupt will trigger.
iwm on the other hand disables the periodic interrupt only when a
periodic interrupt was received AND at the same time SW_RX or FH_RX is
set, and then reenables it only on a SW_RX or FH_RX interrupt that was
not periodic.
A discrepany between iwm and iwlwifi is that in iwm IWM_CSR_INT_BIT_RX_PERIODIC
alone can lead to a write of IWM_CSR_FH_INT_RX_MASK to IWM_CSR_FH_INT_STATUS
(which might be the receive confirmation), which in iwlwifi only happens if
SW_RX or FH_RX are set.
The attached diff makes iwm behave more like iwlwifi in the cases described
above.
ok?
diff --git a/sys/dev/pci/if_iwm.c b/sys/dev/pci/if_iwm.c
index 2a276915cfe..1a17590bacf 100644
--- a/sys/dev/pci/if_iwm.c
+++ b/sys/dev/pci/if_iwm.c
@@ -8615,7 +8615,6 @@ iwm_intr(void *arg)
struct iwm_softc *sc = arg;
int handled = 0;
int rv = 0;
- int isperiodic = 0;
uint32_t r1, r2;
IWM_WRITE(sc, IWM_CSR_INT_MASK, 0);
@@ -8722,27 +8721,25 @@ iwm_intr(void *arg)
wakeup(&sc->sc_fw);
}
- if (r1 & IWM_CSR_INT_BIT_RX_PERIODIC) {
- handled |= IWM_CSR_INT_BIT_RX_PERIODIC;
- IWM_WRITE(sc, IWM_CSR_INT, IWM_CSR_INT_BIT_RX_PERIODIC);
- if ((r1 & (IWM_CSR_INT_BIT_FH_RX | IWM_CSR_INT_BIT_SW_RX)) == 0)
- IWM_WRITE_1(sc,
- IWM_CSR_INT_PERIODIC_REG, IWM_CSR_INT_PERIODIC_DIS);
- isperiodic = 1;
- }
+ if (r1 & (IWM_CSR_INT_BIT_FH_RX | IWM_CSR_INT_BIT_SW_RX |
+ IWM_CSR_INT_BIT_RX_PERIODIC)) {
+ if (r1 & (IWM_CSR_INT_BIT_FH_RX | IWM_CSR_INT_BIT_SW_RX)) {
+ handled |= (IWM_CSR_INT_BIT_FH_RX |
IWM_CSR_INT_BIT_SW_RX);
+ IWM_WRITE(sc, IWM_CSR_FH_INT_STATUS,
IWM_CSR_FH_INT_RX_MASK);
+ }
+ if (r1 & IWM_CSR_INT_BIT_RX_PERIODIC) {
+ handled |= IWM_CSR_INT_BIT_RX_PERIODIC;
+ IWM_WRITE(sc, IWM_CSR_INT, IWM_CSR_INT_BIT_RX_PERIODIC);
+ }
- if ((r1 & (IWM_CSR_INT_BIT_FH_RX | IWM_CSR_INT_BIT_SW_RX)) ||
- isperiodic) {
- handled |= (IWM_CSR_INT_BIT_FH_RX | IWM_CSR_INT_BIT_SW_RX);
- IWM_WRITE(sc, IWM_CSR_FH_INT_STATUS, IWM_CSR_FH_INT_RX_MASK);
-
- iwm_notif_intr(sc);
+ IWM_WRITE_1(sc, IWM_CSR_INT_PERIODIC_REG,
IWM_CSR_INT_PERIODIC_DIS);
/* enable periodic interrupt, see above */
- if (r1 & (IWM_CSR_INT_BIT_FH_RX | IWM_CSR_INT_BIT_SW_RX) &&
- !isperiodic)
+ if (r1 & (IWM_CSR_INT_BIT_FH_RX | IWM_CSR_INT_BIT_SW_RX))
IWM_WRITE_1(sc, IWM_CSR_INT_PERIODIC_REG,
IWM_CSR_INT_PERIODIC_ENA);
+
+ iwm_notif_intr(sc);
}
rv = 1;
