d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a corner-case of interrupt handling which resulted in potentially

Browse Source 
spurious (and fatal) interrupt errors.

One user reported seeing this:

Apr 22 18:04:24 ceres kernel: ar5416GetPendingInterrupts: fatal error,
  ISR_RAC 0x0 SYNC_CAUSE 0x2000

SYNC_CAUSE of 0x2000 is AR_INTR_SYNC_LOCAL_TIMEOUT which is a bus timeout;
this shouldn't cause HAL_INT_FATAL to be set.

After checking out ath9k, ath9k_ar9002_hw_get_isr() clears (*masked)
before continuing, regardless of whether any bits in the ISR registers
are set. So if AR_INTR_SYNC_CAUSE is set to something that isn't
treated as fatal, and AR_ISR isn't read or is read and is 0, then
(*masked) wouldn't be cleared. Thus any of the existing bits set
that were passed in would be preserved in the output.

The caller in if_ath - ath_intr() - wasn't setting the masked value
to 0 before calling ath_hal_getisr(), so anything that was present
in that uninitialised variable would be preserved in the case above
of AR_ISR=0, AR_INTR_SYNC_CAUSE != 0; and if the HAL_INT_FATAL bit
was set, a fatal condition would be interpreted and the chip was
reset.

This patch does the following:

* ath_intr() - set masked to 0 before calling ath_hal_getisr();
* ar5416GetPendingInterrupts() - clear (*masked) before processing
  continues; so if the interrupt source is AR_INTR_SYNC_CAUSE
  and it isn't fatal, the hardware isn't reset via returning
  HAL_INT_FATAL.

This doesn't fix any underlying errors which trigger
AR_INTR_SYNC_LOCAL_TIMEOUT - which is a bus timeout of some
sort - so that likely should be further investigated.
This commit is contained in:
Adrian Chadd 2011-04-23 06:37:09 +00:00
parent e094d2c0c9
commit 6f5fe81e02
2 changed files with 11 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
sys/dev/ath/ath_hal/ar5416/ar5416_interrupts.c
View File

@ -55,6 +55,8 @@ ar5416IsInterruptPending(struct ath_hal *ah)
* values. The value returned is mapped to abstract the hw-specific bit
* locations in the Interrupt Status Register.
*
* (*masked) is cleared on initial call.
*
* Returns: A hardware-abstracted bitmap of all non-masked-out
* interrupts pending, as well as an unmasked value
*/
@ -73,10 +75,10 @@ ar5416GetPendingInterrupts(struct ath_hal *ah, HAL_INT *masked)
isr = 0;
sync_cause = OS_REG_READ(ah, AR_INTR_SYNC_CAUSE);
sync_cause &= AR_INTR_SYNC_DEFAULT;
if (isr == 0 && sync_cause == 0) {
*masked = 0;
*masked = 0;
if (isr == 0 && sync_cause == 0)
return AH_FALSE;
}
if (isr != 0) {
struct ath_hal_5212 *ahp = AH5212(ah);

7
sys/dev/ath/if_ath.c
View File

@ -1265,7 +1265,7 @@ ath_intr(void *arg)
struct ath_softc *sc = arg;
struct ifnet *ifp = sc->sc_ifp;
struct ath_hal *ah = sc->sc_ah;
HAL_INT status;
HAL_INT status = 0;
if (sc->sc_invalid) {
/*
@ -1296,6 +1296,11 @@ ath_intr(void *arg)
ath_hal_getisr(ah, &status); /* NB: clears ISR too */
DPRINTF(sc, ATH_DEBUG_INTR, "%s: status 0x%x\n", __func__, status);
status &= sc->sc_imask; /* discard unasked for bits */
/* Short-circuit un-handled interrupts */
if (status == 0x0)
return;
if (status & HAL_INT_FATAL) {
sc->sc_stats.ast_hardware++;
ath_hal_intrset(ah, 0); /* disable intr's until reset */