Author: adrian
Date: Sun Oct 2 14:08:56 2011
New Revision: 225925
URL: http://svn.freebsd.org/changeset/base/225925
Log:
Various interrupt handling and RX interrupt mitigation fixes.
* The AR_ISR_RAC interrupt processing method has a subtle bug in all
the MAC revisions (including pre-11n NICs) until AR9300v2.
If you're unlucky, the clear phase clears an update to one of the
secondary registers, which includes TX status.
This shows up as a "watchdog timeout" if you're doing very low levels
of TX traffic. If you're doing a lot of non-11n TX traffic, you'll
end up receiving a TX interrupt from some later traffic anyway.
But when TX'ing 11n aggregation session traffic (which -HEAD isn't yet
doing), you may find that you're only able to TX one frame (due to
BAW restrictions) and this may end up hitting this race condition.
The only solution is to not use RAC and instead use AR_ISR and the
AR_ISR_Sx registers. The bit in AR_ISR which represents the secondary
registers are not cleared; only the AR_ISR_Sx bits are. This way
any updates which occur between the read and subsequent write will
stay asserted and (correctly) trigger a subsequent interrupt.
I've tested this on the AR5416, AR9160, AR9280. I will soon test
the AR9285 and AR9287.
* The AR_ISR TX and RX bits (and all others!) are set regardless of
whether the contents of the AR_IMR register. So if RX mitigation is
enabled, RXOK is going to be set in AR_ISR and it would normally set
HAL_INT_RX.
Fix the code to not set HAL_INT_RX when RXOK is set and RX mitigation
is compiled in. That way the RX path isn't prematurely called.
I would see:
* An interrupt would come in (eg a beacon, or TX completion) where
RXOK was set but RXINTM/RXMINT wasn't;
* ath_rx_proc() be called - completing RX frames;
* RXINTM/RXMINT would then fire;
* ath_rx_proc() would then be called again but find no frames in the
queue.
This fixes the RX mitigation behaviour to not overly call ath_rx_proc().
* Start to flesh out more correct timer interrupt handling - it isn't
kite/merlin specific. It's actually based on whether autosleep support
is enabled or not.
This is sourced from my 11n TX branch and has been tested for a few weeks.
Finally, the interrupt handling change should likely be implemented
for AR5210, AR5211 and AR5212.
Modified:
head/sys/dev/ath/ath_hal/ar5416/ar5416_interrupts.c
head/sys/dev/ath/ath_hal/ar5416/ar5416reg.h
Modified: head/sys/dev/ath/ath_hal/ar5416/ar5416_interrupts.c
==============================================================================
--- head/sys/dev/ath/ath_hal/ar5416/ar5416_interrupts.c Sun Oct 2 13:51:26
2011 (r225924)
+++ head/sys/dev/ath/ath_hal/ar5416/ar5416_interrupts.c Sun Oct 2 14:08:56
2011 (r225925)
@@ -68,6 +68,7 @@ HAL_BOOL
ar5416GetPendingInterrupts(struct ath_hal *ah, HAL_INT *masked)
{
uint32_t isr, isr0, isr1, sync_cause = 0;
+ HAL_CAPABILITIES *pCap = &AH_PRIVATE(ah)->ah_caps;
/*
* Verify there's a mac interrupt and the RTC is on.
@@ -110,42 +111,95 @@ ar5416GetPendingInterrupts(struct ath_ha
mask2 |= HAL_INT_CST;
if (isr2 & AR_ISR_S2_TSFOOR)
mask2 |= HAL_INT_TSFOOR;
+
+ /*
+ * Don't mask out AR_BCNMISC; instead mask
+ * out what causes it.
+ */
+ OS_REG_WRITE(ah, AR_ISR_S2, isr2);
+ isr &= ~AR_ISR_BCNMISC;
}
- isr = OS_REG_READ(ah, AR_ISR_RAC);
if (isr == 0xffffffff) {
*masked = 0;
return AH_FALSE;
}
*masked = isr & HAL_INT_COMMON;
+
+ if (isr & (AR_ISR_RXMINTR | AR_ISR_RXINTM))
+ *masked |= HAL_INT_RX;
+ if (isr & (AR_ISR_TXMINTR | AR_ISR_TXINTM))
+ *masked |= HAL_INT_TX;
+
+ /*
+ * When doing RX interrupt mitigation, the RXOK bit is set
+ * in AR_ISR even if the relevant bit in AR_IMR is clear.
+ * Since this interrupt may be due to another source, don't
+ * just automatically set HAL_INT_RX if it's set, otherwise
+ * we could prematurely service the RX queue.
+ *
+ * In some cases, the driver can even handle all the RX
+ * frames just before the mitigation interrupt fires.
+ * The subsequent RX processing trip will then end up
+ * processing 0 frames.
+ */
+#ifdef AH_AR5416_INTERRUPT_MITIGATION
+ if (isr & AR_ISR_RXERR)
+ *masked |= HAL_INT_RX;
+#else
if (isr & (AR_ISR_RXOK | AR_ISR_RXERR))
*masked |= HAL_INT_RX;
- if (isr & (AR_ISR_TXOK | AR_ISR_TXDESC | AR_ISR_TXERR |
AR_ISR_TXEOL)) {
+#endif
+
+ if (isr & (AR_ISR_TXOK | AR_ISR_TXDESC | AR_ISR_TXERR |
+ AR_ISR_TXEOL)) {
*masked |= HAL_INT_TX;
- isr0 = OS_REG_READ(ah, AR_ISR_S0_S);
+
+ isr0 = OS_REG_READ(ah, AR_ISR_S0);
+ OS_REG_WRITE(ah, AR_ISR_S0, isr0);
+ isr1 = OS_REG_READ(ah, AR_ISR_S1);
+ OS_REG_WRITE(ah, AR_ISR_S1, isr1);
+
+ /*
+ * Don't clear the primary ISR TX bits, clear
+ * what causes them (S0/S1.)
+ */
+ isr &= ~(AR_ISR_TXOK | AR_ISR_TXDESC |
+ AR_ISR_TXERR | AR_ISR_TXEOL);
+
ahp->ah_intrTxqs |= MS(isr0, AR_ISR_S0_QCU_TXOK);
ahp->ah_intrTxqs |= MS(isr0, AR_ISR_S0_QCU_TXDESC);
- isr1 = OS_REG_READ(ah, AR_ISR_S1_S);
ahp->ah_intrTxqs |= MS(isr1, AR_ISR_S1_QCU_TXERR);
ahp->ah_intrTxqs |= MS(isr1, AR_ISR_S1_QCU_TXEOL);
}
- if (AR_SREV_MERLIN(ah) || AR_SREV_KITE(ah)) {
+ if ((isr & AR_ISR_GENTMR) || (! pCap->halAutoSleepSupport)) {
uint32_t isr5;
- isr5 = OS_REG_READ(ah, AR_ISR_S5_S);
- if (isr5 & AR_ISR_S5_TIM_TIMER)
- *masked |= HAL_INT_TIM_TIMER;
+ isr5 = OS_REG_READ(ah, AR_ISR_S5);
+ OS_REG_WRITE(ah, AR_ISR_S5, isr5);
+ isr &= ~AR_ISR_GENTMR;
+
+ if (! pCap->halAutoSleepSupport)
+ if (isr5 & AR_ISR_S5_TIM_TIMER)
+ *masked |= HAL_INT_TIM_TIMER;
}
-
- /* Interrupt Mitigation on AR5416 */
-#ifdef AH_AR5416_INTERRUPT_MITIGATION
- if (isr & (AR_ISR_RXMINTR | AR_ISR_RXINTM))
- *masked |= HAL_INT_RX;
-#endif
*masked |= mask2;
}
+ /*
+ * Since we're not using AR_ISR_RAC, clear the status bits
+ * for handled interrupts here. For bits whose interrupt
+ * source is a secondary register, those bits should've been
+ * masked out - instead of those bits being written back,
+ * their source (ie, the secondary status registers) should
+ * be cleared. That way there are no race conditions with
+ * new triggers coming in whilst they've been read/cleared.
+ */
+ OS_REG_WRITE(ah, AR_ISR, isr);
+ /* Flush previous write */
+ OS_REG_READ(ah, AR_ISR);
+
if (AR_SREV_HOWL(ah))
return AH_TRUE;
Modified: head/sys/dev/ath/ath_hal/ar5416/ar5416reg.h
==============================================================================
--- head/sys/dev/ath/ath_hal/ar5416/ar5416reg.h Sun Oct 2 13:51:26 2011
(r225924)
+++ head/sys/dev/ath/ath_hal/ar5416/ar5416reg.h Sun Oct 2 14:08:56 2011
(r225925)
@@ -250,6 +250,7 @@
/* Interrupts */
#define AR_ISR_TXMINTR 0x00080000 /* Maximum interrupt tx
rate */
#define AR_ISR_RXMINTR 0x01000000 /* Maximum interrupt rx
rate */
+#define AR_ISR_GENTMR 0x10000000 /* OR of generic timer
bits in S5 */
#define AR_ISR_TXINTM 0x40000000 /* Tx int after
mitigation */
#define AR_ISR_RXINTM 0x80000000 /* Rx int after
mitigation */
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"
