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);

Reply via email to