Detailed firmware error reports from Intel wifi drivers are quite noisy so it makes sense to hide them by default, especially on systems running -release.
When Tx agg support was added to iwn and iwm it was important to see detailed firmware error reports. These were hidden behind the IWN_DEBUG/IWM_DEBUG compile-time switch which is inconvenient for people who don't build their own kernels. Hence detailed reports are currently enabled by default in -current. I think we should have a run-time toggle for this so that the drivers aren't noisy by default but people who want to provide detailed reports can do so without having to rebuild the kernel. With this patch the drivers will report "fatal firmware error" on a single line by default and provide the full trace only if debugging has been enabled with 'ifconfig iwm0 debug'. Firmware error traces turned out to be mostly useless without the extra debugging context anyway. ok? diff 8623942bf7ff67cae8f933fb97da088c2c7046a4 0b24590fd0d5395719c4562567e6ab8f7d826463 blob - 00212d2924e3225700ec971e9a3b371cdbe8e034 blob + c52b45ddf739e2e18847f355ca13e79bb12a2a7c --- sys/dev/pci/if_iwm.c +++ sys/dev/pci/if_iwm.c @@ -517,12 +517,10 @@ void iwm_start(struct ifnet *); void iwm_stop(struct ifnet *); void iwm_watchdog(struct ifnet *); int iwm_ioctl(struct ifnet *, u_long, caddr_t); -#if 1 const char *iwm_desc_lookup(uint32_t); void iwm_nic_error(struct iwm_softc *); void iwm_dump_driver_status(struct iwm_softc *); void iwm_nic_umac_error(struct iwm_softc *); -#endif void iwm_rx_mpdu(struct iwm_softc *, struct mbuf *, void *, size_t, struct mbuf_list *); void iwm_flip_address(uint8_t *); @@ -9933,10 +9931,10 @@ iwm_watchdog(struct ifnet *ifp) if (sc->sc_tx_timer > 0) { if (--sc->sc_tx_timer == 0) { printf("%s: device timeout\n", DEVNAME(sc)); -#if 1 - iwm_nic_error(sc); - iwm_dump_driver_status(sc); -#endif + if (ifp->if_flags & IFF_DEBUG) { + iwm_nic_error(sc); + iwm_dump_driver_status(sc); + } if ((sc->sc_flags & IWM_FLAG_SHUTDOWN) == 0) task_add(systq, &sc->init_task); ifp->if_oerrors++; @@ -10001,7 +9999,6 @@ iwm_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) return err; } -#if 1 /* * Note: This structure is read from the device with IO accesses, * and the reading already does the endian conversion. As it is @@ -10268,7 +10265,6 @@ iwm_dump_driver_status(struct iwm_softc *sc) printf(" 802.11 state %s\n", ieee80211_state_name[sc->sc_ic.ic_state]); } -#endif #define SYNC_RESP_STRUCT(_var_, _pkt_) \ do { \ @@ -10701,6 +10697,8 @@ int iwm_intr(void *arg) { struct iwm_softc *sc = arg; + struct ieee80211com *ic = &sc->sc_ic; + struct ifnet *ifp = IC2IFP(ic); int handled = 0; int rv = 0; uint32_t r1, r2; @@ -10763,11 +10761,10 @@ iwm_intr(void *arg) } if (r1 & IWM_CSR_INT_BIT_SW_ERR) { -#if 1 - iwm_nic_error(sc); - iwm_dump_driver_status(sc); -#endif - + if (ifp->if_flags & IFF_DEBUG) { + iwm_nic_error(sc); + iwm_dump_driver_status(sc); + } printf("%s: fatal firmware error\n", DEVNAME(sc)); if ((sc->sc_flags & IWM_FLAG_SHUTDOWN) == 0) task_add(systq, &sc->init_task); @@ -10836,6 +10833,8 @@ int iwm_intr_msix(void *arg) { struct iwm_softc *sc = arg; + struct ieee80211com *ic = &sc->sc_ic; + struct ifnet *ifp = IC2IFP(ic); uint32_t inta_fh, inta_hw; int vector = 0; @@ -10860,11 +10859,10 @@ iwm_intr_msix(void *arg) if ((inta_fh & IWM_MSIX_FH_INT_CAUSES_FH_ERR) || (inta_hw & IWM_MSIX_HW_INT_CAUSES_REG_SW_ERR) || (inta_hw & IWM_MSIX_HW_INT_CAUSES_REG_SW_ERR_V2)) { -#if 1 - iwm_nic_error(sc); - iwm_dump_driver_status(sc); -#endif - + if (ifp->if_flags & IFF_DEBUG) { + iwm_nic_error(sc); + iwm_dump_driver_status(sc); + } printf("%s: fatal firmware error\n", DEVNAME(sc)); if ((sc->sc_flags & IWM_FLAG_SHUTDOWN) == 0) task_add(systq, &sc->init_task); blob - a5a78bfcf86b73174a5d3c9b01c703d60405dae3 blob + a9b3cd06166fb9573bfb2ff780808230cc205b9b --- sys/dev/pci/if_iwn.c +++ sys/dev/pci/if_iwn.c @@ -904,8 +904,6 @@ iwn_mem_write_2(struct iwn_softc *sc, uint32_t addr, u iwn_mem_write(sc, addr & ~3, tmp); } -#ifdef IWN_DEBUG - static __inline void iwn_mem_read_region_4(struct iwn_softc *sc, uint32_t addr, uint32_t *data, int count) @@ -914,8 +912,6 @@ iwn_mem_read_region_4(struct iwn_softc *sc, uint32_t a *data++ = iwn_mem_read(sc, addr); } -#endif - static __inline void iwn_mem_set_region_4(struct iwn_softc *sc, uint32_t addr, uint32_t val, int count) @@ -3075,7 +3071,6 @@ iwn_wakeup_intr(struct iwn_softc *sc) } } -#ifdef IWN_DEBUG /* * Dump the error log of the firmware when a firmware panic occurs. Although * we can't debug the firmware because it is neither open source nor free, it @@ -3135,7 +3130,6 @@ iwn_fatal_intr(struct iwn_softc *sc) printf(" rx ring: cur=%d\n", sc->rxq.cur); printf(" 802.11 state %d\n", sc->sc_ic.ic_state); } -#endif int iwn_intr(void *arg) @@ -3197,9 +3191,8 @@ iwn_intr(void *arg) sc->sc_flags &= ~IWN_FLAG_CALIB_DONE; /* Dump firmware error log and stop. */ -#ifdef IWN_DEBUG - iwn_fatal_intr(sc); -#endif + if (ifp->if_flags & IFF_DEBUG) + iwn_fatal_intr(sc); iwn_stop(ifp); task_add(systq, &sc->init_task); return 1; blob - a214f1eef6f12b97e8eb99d90b3a624522116efe blob + 98e28318b7bf04875e96397afb4dacb2ecdb6b5d --- sys/dev/pci/if_iwx.c +++ sys/dev/pci/if_iwx.c @@ -462,6 +462,7 @@ void iwx_watchdog(struct ifnet *); int iwx_ioctl(struct ifnet *, u_long, caddr_t); const char *iwx_desc_lookup(uint32_t); void iwx_nic_error(struct iwx_softc *); +void iwx_dump_driver_status(struct iwx_softc *); void iwx_nic_umac_error(struct iwx_softc *); int iwx_detect_duplicate(struct iwx_softc *, struct mbuf *, struct iwx_rx_mpdu_desc *, struct ieee80211_rxinfo *); @@ -8145,9 +8146,10 @@ iwx_watchdog(struct ifnet *ifp) if (sc->sc_tx_timer > 0) { if (--sc->sc_tx_timer == 0) { printf("%s: device timeout\n", DEVNAME(sc)); -#ifdef IWX_DEBUG - iwx_nic_error(sc); -#endif + if (ifp->if_flags & IFF_DEBUG) { + iwx_nic_error(sc); + iwx_dump_driver_status(sc); + } if ((sc->sc_flags & IWX_FLAG_SHUTDOWN) == 0) task_add(systq, &sc->init_task); ifp->if_oerrors++; @@ -8212,7 +8214,6 @@ iwx_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) return err; } -#if 1 /* usually #ifdef IWX_DEBUG but always enabled for now */ /* * Note: This structure is read from the device with IO accesses, * and the reading already does the endian conversion. As it is @@ -8462,8 +8463,24 @@ iwx_nic_error(struct iwx_softc *sc) if (sc->sc_uc.uc_umac_error_event_table) iwx_nic_umac_error(sc); } -#endif +void +iwx_dump_driver_status(struct iwx_softc *sc) +{ + int i; + + printf("driver status:\n"); + for (i = 0; i < IWX_MAX_QUEUES; i++) { + struct iwx_tx_ring *ring = &sc->txq[i]; + printf(" tx ring %2d: qid=%-2d cur=%-3d " + "queued=%-3d\n", + i, ring->qid, ring->cur, ring->queued); + } + printf(" rx ring: cur=%d\n", sc->rxq.cur); + printf(" 802.11 state %s\n", + ieee80211_state_name[sc->sc_ic.ic_state]); +} + #define SYNC_RESP_STRUCT(_var_, _pkt_) \ do { \ bus_dmamap_sync(sc->sc_dmat, data->map, sizeof(*(_pkt_)), \ @@ -8874,6 +8891,8 @@ int iwx_intr(void *arg) { struct iwx_softc *sc = arg; + struct ieee80211com *ic = &sc->sc_ic; + struct ifnet *ifp = IC2IFP(ic); int handled = 0; int r1, r2, rv = 0; @@ -8938,24 +8957,10 @@ iwx_intr(void *arg) } if (r1 & IWX_CSR_INT_BIT_SW_ERR) { -#if 1 /* usually #ifdef IWX_DEBUG but always enabled for now */ - int i; - - iwx_nic_error(sc); - - /* Dump driver status (TX and RX rings) while we're here. */ - printf("driver status:\n"); - for (i = 0; i < IWX_MAX_QUEUES; i++) { - struct iwx_tx_ring *ring = &sc->txq[i]; - printf(" tx ring %2d: qid=%-2d cur=%-3d " - "queued=%-3d\n", - i, ring->qid, ring->cur, ring->queued); + if (ifp->if_flags & IFF_DEBUG) { + iwx_nic_error(sc); + iwx_dump_driver_status(sc); } - printf(" rx ring: cur=%d\n", sc->rxq.cur); - printf(" 802.11 state %s\n", - ieee80211_state_name[sc->sc_ic.ic_state]); -#endif - printf("%s: fatal firmware error\n", DEVNAME(sc)); if ((sc->sc_flags & IWX_FLAG_SHUTDOWN) == 0) task_add(systq, &sc->init_task); @@ -9024,6 +9029,8 @@ int iwx_intr_msix(void *arg) { struct iwx_softc *sc = arg; + struct ieee80211com *ic = &sc->sc_ic; + struct ifnet *ifp = IC2IFP(ic); uint32_t inta_fh, inta_hw; int vector = 0; @@ -9048,24 +9055,10 @@ iwx_intr_msix(void *arg) if ((inta_fh & IWX_MSIX_FH_INT_CAUSES_FH_ERR) || (inta_hw & IWX_MSIX_HW_INT_CAUSES_REG_SW_ERR) || (inta_hw & IWX_MSIX_HW_INT_CAUSES_REG_SW_ERR_V2)) { -#if 1 /* usually #ifdef IWX_DEBUG but always enabled for now */ - int i; - - iwx_nic_error(sc); - - /* Dump driver status (TX and RX rings) while we're here. */ - printf("driver status:\n"); - for (i = 0; i < IWX_MAX_QUEUES; i++) { - struct iwx_tx_ring *ring = &sc->txq[i]; - printf(" tx ring %2d: qid=%-2d cur=%-3d " - "queued=%-3d\n", - i, ring->qid, ring->cur, ring->queued); + if (ifp->if_flags & IFF_DEBUG) { + iwx_nic_error(sc); + iwx_dump_driver_status(sc); } - printf(" rx ring: cur=%d\n", sc->rxq.cur); - printf(" 802.11 state %s\n", - ieee80211_state_name[sc->sc_ic.ic_state]); -#endif - printf("%s: fatal firmware error\n", DEVNAME(sc)); if ((sc->sc_flags & IWX_FLAG_SHUTDOWN) == 0) task_add(systq, &sc->init_task);