iwm/iwx: propagate errors out of iw{m,x}_set_bits_mask_prph routines

This might help with troubleshooting "iwx0: acquiring device failed"
errors.

OK stsp@

diff --git a/sys/dev/pci/if_iwm.c b/sys/dev/pci/if_iwm.c
index a4aef73..0447b44 100644 (file)
--- a/sys/dev/pci/if_iwm.c
+++ b/sys/dev/pci/if_iwm.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_iwm.c,v 1.362 2021/08/26 07:11:09 kevlo Exp $      */
+/*     $OpenBSD: if_iwm.c,v 1.363 2021/08/29 20:31:18 gnezdo Exp $     */
 
 /*
  * Copyright (c) 2014, 2016 genua gmbh <info@genua.de>
@@ -266,10 +266,10 @@ int       iwm_poll_bit(struct iwm_softc *, int, uint32_t, uint32_t, int);
 int    iwm_nic_lock(struct iwm_softc *);
 void   iwm_nic_assert_locked(struct iwm_softc *);
 void   iwm_nic_unlock(struct iwm_softc *);
-void   iwm_set_bits_mask_prph(struct iwm_softc *, uint32_t, uint32_t,
+int    iwm_set_bits_mask_prph(struct iwm_softc *, uint32_t, uint32_t,
            uint32_t);
-void   iwm_set_bits_prph(struct iwm_softc *, uint32_t, uint32_t);
-void   iwm_clear_bits_prph(struct iwm_softc *, uint32_t, uint32_t);
+int    iwm_set_bits_prph(struct iwm_softc *, uint32_t, uint32_t);
+int    iwm_clear_bits_prph(struct iwm_softc *, uint32_t, uint32_t);
 int    iwm_dma_contig_alloc(bus_dma_tag_t, struct iwm_dma_info *, bus_size_t,
            bus_size_t);
 void   iwm_dma_contig_free(struct iwm_dma_info *);
@@ -1167,31 +1167,32 @@ iwm_nic_unlock(struct iwm_softc *sc)
                printf("%s: NIC already unlocked\n", DEVNAME(sc));
 }
 
-void
+int
 iwm_set_bits_mask_prph(struct iwm_softc *sc, uint32_t reg, uint32_t bits,
     uint32_t mask)
 {
        uint32_t val;
 
-       /* XXX: no error path? */
        if (iwm_nic_lock(sc)) {
                val = iwm_read_prph(sc, reg) & mask;
                val |= bits;
                iwm_write_prph(sc, reg, val);
                iwm_nic_unlock(sc);
+               return 0;
        }
+       return EBUSY;
 }
 
-void
+int
 iwm_set_bits_prph(struct iwm_softc *sc, uint32_t reg, uint32_t bits)
 {
-       iwm_set_bits_mask_prph(sc, reg, bits, ~0);
+       return iwm_set_bits_mask_prph(sc, reg, bits, ~0);
 }
 
-void
+int
 iwm_clear_bits_prph(struct iwm_softc *sc, uint32_t reg, uint32_t bits)
 {
-       iwm_set_bits_mask_prph(sc, reg, 0, ~bits);
+       return iwm_set_bits_mask_prph(sc, reg, 0, ~bits);
 }
 
 int
@@ -1825,7 +1826,10 @@ iwm_apm_init(struct iwm_softc *sc)
                        iwm_read_prph(sc, IWM_OSC_CLK);
                        iwm_nic_unlock(sc);
                }
-               iwm_set_bits_prph(sc, IWM_OSC_CLK, IWM_OSC_CLK_FORCE_CONTROL);
+               err = iwm_set_bits_prph(sc, IWM_OSC_CLK,
+                   IWM_OSC_CLK_FORCE_CONTROL);
+               if (err)
+                       goto out;
                if (iwm_nic_lock(sc)) {
                        iwm_read_prph(sc, IWM_OSC_CLK);
                        iwm_read_prph(sc, IWM_OSC_CLK);
@@ -1849,8 +1853,10 @@ iwm_apm_init(struct iwm_softc *sc)
                DELAY(20);
 
                /* Disable L1-Active */
-               iwm_set_bits_prph(sc, IWM_APMG_PCIDEV_STT_REG,
+               err = iwm_set_bits_prph(sc, IWM_APMG_PCIDEV_STT_REG,
                    IWM_APMG_PCIDEV_STT_VAL_L1_ACT_DIS);
+               if (err)
+                       goto out;
 
                /* Clear the interrupt in APMG if the NIC is in RFKILL */
                if (iwm_nic_lock(sc)) {
@@ -2292,7 +2298,7 @@ iwm_nic_rx_legacy_init(struct iwm_softc *sc)
 int
 iwm_nic_tx_init(struct iwm_softc *sc)
 {
-       int qid;
+       int qid, err;
 
        if (!iwm_nic_lock(sc))
                return EBUSY;
@@ -2311,13 +2317,13 @@ iwm_nic_tx_init(struct iwm_softc *sc)
                    txq->desc_dma.paddr >> 8);
        }
 
-       iwm_set_bits_prph(sc, IWM_SCD_GP_CTRL,
+       err = iwm_set_bits_prph(sc, IWM_SCD_GP_CTRL,
            IWM_SCD_GP_CTRL_AUTO_ACTIVE_MODE |
            IWM_SCD_GP_CTRL_ENABLE_31_QUEUES);
 
        iwm_nic_unlock(sc);
 
-       return 0;
+       return err;
 }
 
 int
@@ -2369,6 +2375,7 @@ const uint8_t iwm_ac_to_tx_fifo[] = {
 int
 iwm_enable_ac_txq(struct iwm_softc *sc, int qid, int fifo)
 {
+       int err;
        iwm_nic_assert_locked(sc);
 
        IWM_WRITE(sc, IWM_HBUS_TARG_WRPTR, qid << 8 | 0);
@@ -2377,7 +2384,10 @@ iwm_enable_ac_txq(struct iwm_softc *sc, int qid, int fifo)
            (0 << IWM_SCD_QUEUE_STTS_REG_POS_ACTIVE)
            | (1 << IWM_SCD_QUEUE_STTS_REG_POS_SCD_ACT_EN));
 
-       iwm_clear_bits_prph(sc, IWM_SCD_AGGR_SEL, (1 << qid));
+       err = iwm_clear_bits_prph(sc, IWM_SCD_AGGR_SEL, (1 << qid));
+       if (err) {
+               return err;
+       }
 
        iwm_write_prph(sc, IWM_SCD_QUEUE_RDPTR(qid), 0);
 
@@ -2511,9 +2521,10 @@ iwm_post_alive(struct iwm_softc *sc)
        iwm_nic_unlock(sc);
 
        /* Enable L1-Active */
-       if (sc->sc_device_family < IWM_DEVICE_FAMILY_8000)
-               iwm_clear_bits_prph(sc, IWM_APMG_PCIDEV_STT_REG,
+       if (sc->sc_device_family < IWM_DEVICE_FAMILY_8000) {
+               err = iwm_clear_bits_prph(sc, IWM_APMG_PCIDEV_STT_REG,
                    IWM_APMG_PCIDEV_STT_VAL_L1_ACT_DIS);
+       }
 
        return err;
 }
@@ -3936,9 +3947,12 @@ iwm_firmware_load_chunk(struct iwm_softc *sc, uint32_t dst_addr,
            dma->map, 0, byte_cnt, BUS_DMASYNC_PREWRITE);
 
        if (dst_addr >= IWM_FW_MEM_EXTENDED_START &&
-           dst_addr <= IWM_FW_MEM_EXTENDED_END)
-               iwm_set_bits_prph(sc, IWM_LMPM_CHICK,
+           dst_addr <= IWM_FW_MEM_EXTENDED_END) {
+               err = iwm_set_bits_prph(sc, IWM_LMPM_CHICK,
                    IWM_LMPM_CHICK_EXTENDED_ADDR_SPACE);
+               if (err)
+                       return err;
+       }
 
        sc->sc_fw_chunk_done = 0;
 
@@ -3979,8 +3993,10 @@ iwm_firmware_load_chunk(struct iwm_softc *sc, uint32_t dst_addr,
 
        if (dst_addr >= IWM_FW_MEM_EXTENDED_START &&
            dst_addr <= IWM_FW_MEM_EXTENDED_END) {
-               iwm_clear_bits_prph(sc, IWM_LMPM_CHICK,
+               int err2 = iwm_clear_bits_prph(sc, IWM_LMPM_CHICK,
                    IWM_LMPM_CHICK_EXTENDED_ADDR_SPACE);
+               if (!err)
+                       err = err2;
        }
 
        return err;

diff --git a/sys/dev/pci/if_iwx.c b/sys/dev/pci/if_iwx.c
index f2b4b48..892fdfb 100644 (file)
--- a/sys/dev/pci/if_iwx.c
+++ b/sys/dev/pci/if_iwx.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_iwx.c,v 1.97 2021/08/26 07:11:09 kevlo Exp $       */
+/*     $OpenBSD: if_iwx.c,v 1.98 2021/08/29 20:31:18 gnezdo Exp $      */
 
 /*
  * Copyright (c) 2014, 2016 genua gmbh <info@genua.de>
@@ -261,10 +261,10 @@ int       iwx_poll_bit(struct iwx_softc *, int, uint32_t, uint32_t, int);
 int    iwx_nic_lock(struct iwx_softc *);
 void   iwx_nic_assert_locked(struct iwx_softc *);
 void   iwx_nic_unlock(struct iwx_softc *);
-void   iwx_set_bits_mask_prph(struct iwx_softc *, uint32_t, uint32_t,
+int    iwx_set_bits_mask_prph(struct iwx_softc *, uint32_t, uint32_t,
            uint32_t);
-void   iwx_set_bits_prph(struct iwx_softc *, uint32_t, uint32_t);
-void   iwx_clear_bits_prph(struct iwx_softc *, uint32_t, uint32_t);
+int    iwx_set_bits_prph(struct iwx_softc *, uint32_t, uint32_t);
+int    iwx_clear_bits_prph(struct iwx_softc *, uint32_t, uint32_t);
 int    iwx_dma_contig_alloc(bus_dma_tag_t, struct iwx_dma_info *, bus_size_t,
            bus_size_t);
 void   iwx_dma_contig_free(struct iwx_dma_info *);
@@ -284,7 +284,7 @@ void        iwx_disable_interrupts(struct iwx_softc *);
 void   iwx_ict_reset(struct iwx_softc *);
 int    iwx_set_hw_ready(struct iwx_softc *);
 int    iwx_prepare_card_hw(struct iwx_softc *);
-void   iwx_force_power_gating(struct iwx_softc *);
+int    iwx_force_power_gating(struct iwx_softc *);
 void   iwx_apm_config(struct iwx_softc *);
 int    iwx_apm_init(struct iwx_softc *);
 void   iwx_apm_stop(struct iwx_softc *);
@@ -814,10 +814,14 @@ iwx_apply_debug_destination(struct iwx_softc *sc)
                        iwx_write_prph(sc, addr, val);
                        break;
                case PRPH_SETBIT:
-                       iwx_set_bits_prph(sc, addr, (1 << val));
+                       err = iwx_set_bits_prph(sc, addr, (1 << val));
+                       if (err)
+                               return err;
                        break;
                case PRPH_CLEARBIT:
-                       iwx_clear_bits_prph(sc, addr, (1 << val));
+                       err = iwx_clear_bits_prph(sc, addr, (1 << val));
+                       if (err)
+                               return err;
                        break;
                case PRPH_BLOCKBIT:
                        if (iwx_read_prph(sc, addr) & (1 << val))
@@ -1547,31 +1551,32 @@ iwx_nic_unlock(struct iwx_softc *sc)
                printf("%s: NIC already unlocked\n", DEVNAME(sc));
 }
 
-void
+int
 iwx_set_bits_mask_prph(struct iwx_softc *sc, uint32_t reg, uint32_t bits,
     uint32_t mask)
 {
        uint32_t val;
 
-       /* XXX: no error path? */
        if (iwx_nic_lock(sc)) {
                val = iwx_read_prph(sc, reg) & mask;
                val |= bits;
                iwx_write_prph(sc, reg, val);
                iwx_nic_unlock(sc);
+               return 0;
        }
+       return EBUSY;
 }
 
-void
+int
 iwx_set_bits_prph(struct iwx_softc *sc, uint32_t reg, uint32_t bits)
 {
-       iwx_set_bits_mask_prph(sc, reg, bits, ~0);
+       return iwx_set_bits_mask_prph(sc, reg, bits, ~0);
 }
 
-void
+int
 iwx_clear_bits_prph(struct iwx_softc *sc, uint32_t reg, uint32_t bits)
 {
-       iwx_set_bits_mask_prph(sc, reg, 0, ~bits);
+       return iwx_set_bits_mask_prph(sc, reg, 0, ~bits);
 }
 
 int
@@ -2070,18 +2075,25 @@ iwx_prepare_card_hw(struct iwx_softc *sc)
        return ETIMEDOUT;
 }
 
-void
+int
 iwx_force_power_gating(struct iwx_softc *sc)
 {
-       iwx_set_bits_prph(sc, IWX_HPM_HIPM_GEN_CFG,
+       int err;
+
+       err = iwx_set_bits_prph(sc, IWX_HPM_HIPM_GEN_CFG,
            IWX_HPM_HIPM_GEN_CFG_CR_FORCE_ACTIVE);
+       if (err)
+               return err;
        DELAY(20);
-       iwx_set_bits_prph(sc, IWX_HPM_HIPM_GEN_CFG,
+       err = iwx_set_bits_prph(sc, IWX_HPM_HIPM_GEN_CFG,
            IWX_HPM_HIPM_GEN_CFG_CR_PG_EN |
            IWX_HPM_HIPM_GEN_CFG_CR_SLP_EN);
+       if (err)
+               return err;
        DELAY(20);
-       iwx_clear_bits_prph(sc, IWX_HPM_HIPM_GEN_CFG,
+       err = iwx_clear_bits_prph(sc, IWX_HPM_HIPM_GEN_CFG,
            IWX_HPM_HIPM_GEN_CFG_CR_FORCE_ACTIVE);
+       return err;
 }
 
 void
@@ -2342,7 +2354,9 @@ iwx_start_hw(struct iwx_softc *sc)
                        return ETIMEDOUT;
                }
 
-               iwx_force_power_gating(sc);
+               err = iwx_force_power_gating(sc);
+               if (err)
+                       return err;
 
                /* Reset the entire device */
                IWX_SETBITS(sc, IWX_CSR_RESET, IWX_CSR_RESET_REG_FLAG_SW_RESET);
@@ -3361,7 +3375,7 @@ iwx_load_firmware(struct iwx_softc *sc)
                err = tsleep_nsec(&sc->sc_uc, 0, "iwxuc", MSEC_TO_NSEC(100));
        }
        if (err || !sc->sc_uc.uc_ok)
-               printf("%s: could not load firmware\n", DEVNAME(sc));
+               printf("%s: could not load firmware, %d\n", DEVNAME(sc), err);
 
        iwx_ctxt_info_free_fw_img(sc);