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@

2 files changed, 70 insertions, 40 deletions

diff --git a/sys/dev/pci/if_iwm.c b/sys/dev/pci/if_iwm.c
index a4aef73e7ce..0447b445424 100644
--- 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 f2b4b48a4b7..892fdfb4a68 100644
--- 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);