Skip to contentSkip to conversation

ath5k: Reorder calibration calls during reset and update hw_set_power

PATCH In review Nick Kossifidis 3 participants 3 messages Jul 20, 2008 – Jul 28, 2008 Networking drivers (wireless)

lore
Share & tools
lore
#0 lore raw

PATCH 6/12 ath5k: Reorder calibration calls during reset and update hw_set_power

* Update ath5k_hw_reset and add some more documentation about PHY calibration
* Fix ath5k_hw_set_power to use AR5K_SLEEP_CTL_SLE_ALLOW for Network sleep
* Preserve sleep duration field while setting AR5K_SLEEP_CTL and reduce delays & checks for register's status (got this from decompiling & dumps, it works for me but it needs testing)

Changes-licensed-under: ISC

---

drivers/net/wireless/ath5k/hw.c +61 -23
index cab1a8a..4fb048c 100644
@@ -1092,34 +1092,57 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum ieee80211_if_types op_mode,
 	data = 0;
 
 	/*
-	 * Enable calibration and wait until completion
+	 * Start automatic gain calibration
+	 *
+	 * During AGC calibration RX path is re-routed to
+	 * a signal detector so we don't receive anything.
+	 *
+	 * This method is used to calibrate some static offsets
+	 * used together with on-the fly I/Q calibration (the
+	 * one performed via ath5k_hw_phy_calibrate), that doesn't
+	 * interrupt rx path.
+	 *
+	 * If we are in a noisy environment AGC calibration may time
+	 * out.
 	 */
 	AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_AGCCTL,
 				AR5K_PHY_AGCCTL_CAL);
 
+	/* At the same time start I/Q calibration for QAM constellation 
+	 * -no need for CCK- */
+	ah->ah_calibration = false;
+	if (!(mode == AR5K_MODE_11B)) {
+		ah->ah_calibration = true;
+		AR5K_REG_WRITE_BITS(ah, AR5K_PHY_IQ,
+				AR5K_PHY_IQ_CAL_NUM_LOG_MAX, 15);
+		AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_IQ,
+				AR5K_PHY_IQ_RUN);
+	}
+
+	/* Wait for gain calibration to finish (we check for I/Q calibration
+	 * during ath5k_phy_calibrate) */
 	if (ath5k_hw_register_timeout(ah, AR5K_PHY_AGCCTL,
 			AR5K_PHY_AGCCTL_CAL, 0, false)) {
-		ATH5K_ERR(ah->ah_sc, "calibration timeout (%uMHz)\n",
+		ATH5K_ERR(ah->ah_sc, "gain calibration timeout (%uMHz)\n",
 			channel->center_freq);
 		return -EAGAIN;
 	}
 
+	/*
+	 * Start noise floor calibration
+	 *
+	 * If we run NF calibration before AGC, it always times out.
+	 * Binary HAL starts NF and AGC calibration at the same time
+	 * and only waits for AGC to finish. I believe that's wrong because
+	 * during NF calibration, rx path is also routed to a detector, so if
+	 * it doesn't finish we won't have RX.
+	 *
+	 * XXX: Find an interval that's OK for all cards...
+	 */
 	ret = ath5k_hw_noise_floor_calibration(ah, channel->center_freq);
 	if (ret)
 		return ret;
 
-	ah->ah_calibration = false;
-
-	/* A and G modes can use QAM modulation which requires enabling
-	 * I and Q calibration. Don't bother in B mode. */
-	if (!(mode == AR5K_MODE_11B)) {
-		ah->ah_calibration = true;
-		AR5K_REG_WRITE_BITS(ah, AR5K_PHY_IQ,
-				AR5K_PHY_IQ_CAL_NUM_LOG_MAX, 15);
-		AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_IQ,
-				AR5K_PHY_IQ_RUN);
-	}
-
 	/*
 	 * Reset queues and start beacon timers at the end of the reset routine
 	 */
@@ -1247,7 +1270,7 @@ int ath5k_hw_set_power(struct ath5k_hw *ah, enum ath5k_power_mode mode,
 		bool set_chip, u16 sleep_duration)
 {
 	unsigned int i;
-	u32 staid;
+	u32 staid, data;
 
 	ATH5K_TRACE(ah->ah_sc);
 	staid = ath5k_hw_reg_read(ah, AR5K_STA_ID1);
@@ -1259,7 +1282,8 @@ int ath5k_hw_set_power(struct ath5k_hw *ah, enum ath5k_power_mode mode,
 	case AR5K_PM_NETWORK_SLEEP:
 		if (set_chip)
 			ath5k_hw_reg_write(ah,
-				AR5K_SLEEP_CTL_SLE | sleep_duration,
+				AR5K_SLEEP_CTL_SLE_ALLOW |
+				sleep_duration,
 				AR5K_SLEEP_CTL);
 
 		staid |= AR5K_STA_ID1_PWR_SV;
@@ -1274,13 +1298,24 @@ int ath5k_hw_set_power(struct ath5k_hw *ah, enum ath5k_power_mode mode,
 		break;
 
 	case AR5K_PM_AWAKE:
+
+		staid &= ~AR5K_STA_ID1_PWR_SV;
+
 		if (!set_chip)
 			goto commit;
 
-		ath5k_hw_reg_write(ah, AR5K_SLEEP_CTL_SLE_WAKE,
-				AR5K_SLEEP_CTL);
+		/* Preserve sleep duration */
+		data = ath5k_hw_reg_read(ah, AR5K_SLEEP_CTL);
+		if( data & 0xffc00000 ){
+			data = 0;
+		} else {
+			data = data & 0xfffcffff;
+		}
 
-		for (i = 5000; i > 0; i--) {
+		ath5k_hw_reg_write(ah, data, AR5K_SLEEP_CTL);
+		udelay(15);
+
+		for (i = 50; i > 0; i--) {
 			/* Check if the chip did wake up */
 			if ((ath5k_hw_reg_read(ah, AR5K_PCICFG) &
 					AR5K_PCICFG_SPWR_DN) == 0)
@@ -1288,15 +1323,13 @@ int ath5k_hw_set_power(struct ath5k_hw *ah, enum ath5k_power_mode mode,
 
 			/* Wait a bit and retry */
 			udelay(200);
-			ath5k_hw_reg_write(ah, AR5K_SLEEP_CTL_SLE_WAKE,
-				AR5K_SLEEP_CTL);
+			ath5k_hw_reg_write(ah, data, AR5K_SLEEP_CTL);
 		}
 
 		/* Fail if the chip didn't wake up */
 		if (i <= 0)
 			return -EIO;
 
-		staid &= ~AR5K_STA_ID1_PWR_SV;
 		break;
 
 	default:
@@ -1325,6 +1358,7 @@ void ath5k_hw_start_rx(struct ath5k_hw *ah)
 {
 	ATH5K_TRACE(ah->ah_sc);
 	ath5k_hw_reg_write(ah, AR5K_CR_RXE, AR5K_CR);
+	ath5k_hw_reg_read(ah, AR5K_CR);
 }
 
 /*
@@ -1411,6 +1445,7 @@ int ath5k_hw_tx_start(struct ath5k_hw *ah, unsigned int queue)
 		}
 		/* Start queue */
 		ath5k_hw_reg_write(ah, tx_queue, AR5K_CR);
+		ath5k_hw_reg_read(ah, AR5K_CR);
 	} else {
 		/* Return if queue is disabled */
 		if (AR5K_REG_READ_Q(ah, AR5K_QCU_TXD, queue))
@@ -1461,6 +1496,7 @@ int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah, unsigned int queue)
 
 		/* Stop queue */
 		ath5k_hw_reg_write(ah, tx_queue, AR5K_CR);
+		ath5k_hw_reg_read(ah, AR5K_CR);
 	} else {
 		/*
 		 * Schedule TX disable and wait until queue is empty
@@ -1705,6 +1741,7 @@ enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw *ah, enum ath5k_int new_mask)
 	 * (they will be re-enabled afterwards).
 	 */
 	ath5k_hw_reg_write(ah, AR5K_IER_DISABLE, AR5K_IER);
+	ath5k_hw_reg_read(ah, AR5K_IER);
 
 	old_mask = ah->ah_imr;
 
@@ -1737,6 +1774,7 @@ enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw *ah, enum ath5k_int new_mask)
 
 	/* ..re-enable interrupts */
 	ath5k_hw_reg_write(ah, AR5K_IER_ENABLE, AR5K_IER);
+	ath5k_hw_reg_read(ah, AR5K_IER);
 
 	return old_mask;
 }
@@ -3507,7 +3545,7 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)
 			if (tq->tqi_flags & AR5K_TXQ_FLAG_RDYTIME_EXP_POLICY_ENABLE)
 				AR5K_REG_ENABLE_BITS(ah,
 					AR5K_QUEUE_MISC(queue),
-					AR5K_QCU_MISC_TXE);
+					AR5K_QCU_MISC_RDY_VEOL_POLICY);
 		}
 
 		if (tq->tqi_flags & AR5K_TXQ_FLAG_BACKOFF_DISABLE)
#1 lore raw
Quoted 12 lines · Nick Kossifidis

* Update ath5k_hw_reset and add some more documentation about PHY calibration
* Fix ath5k_hw_set_power to use AR5K_SLEEP_CTL_SLE_ALLOW for Network sleep
* Preserve sleep duration field while setting AR5K_SLEEP_CTL and reduce delays & checks for register's status (got this from decompiling & dumps, it works for me but it needs testing)

Changes-licensed-under: ISC

---

drivers/net/wireless/ath5k/hw.c +0 -0
index cab1a8a..4fb048c 100644

[...]

drivers/net/wireless/ath5k/hw.c int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah, unsigned int queue) · line 1499

Quoted 5 lines
drivers/net/wireless/ath5k/hw.c +1 -0
@@ -1461,6 +1496,7 @@ int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah, unsigned int queue)
 
 		/* Stop queue */
 		ath5k_hw_reg_write(ah, tx_queue, AR5K_CR);
+		ath5k_hw_reg_read(ah, AR5K_CR);

This one was in my patch already.

drivers/net/wireless/ath5k/hw.c enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw *ah, enum ath5k_int new_mask)

Quoted 4 lines
drivers/net/wireless/ath5k/hw.c +0 -0
 	} else {
 		/*
 		 * Schedule TX disable and wait until queue is empty
@@ -1705,6 +1741,7 @@ enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw *ah, enum ath5k_int new_mask)

Adding from the code:
/*
* Disable card interrupts to prevent any race conditions

drivers/net/wireless/ath5k/hw.c enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw *ah, enum ath5k_int new_mask) · line 1744

Quoted 4 lines
drivers/net/wireless/ath5k/hw.c +1 -0
 	 * (they will be re-enabled afterwards).
 	 */
 	ath5k_hw_reg_write(ah, AR5K_IER_DISABLE, AR5K_IER);
+	ath5k_hw_reg_read(ah, AR5K_IER);

Is this first flush needed? Writes cannot be reordered, they are just asynchronous and flushed below. What race is this against? The interrupt might be processed (or pending) at this moment on other processor anyway.

drivers/net/wireless/ath5k/hw.c enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw *ah, enum ath5k_int new_mask) · line 1777

Quoted 7 lines
drivers/net/wireless/ath5k/hw.c +1 -0
 	old_mask = ah->ah_imr;
 
@@ -1737,6 +1774,7 @@ enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw *ah, enum ath5k_int new_mask)
 
 	/* ..re-enable interrupts */
 	ath5k_hw_reg_write(ah, AR5K_IER_ENABLE, AR5K_IER);
+	ath5k_hw_reg_read(ah, AR5K_IER);

This was fixed too.

#2 lore raw

2008/7/22 Jiri Slaby <[email protected]>:

Quoted 22 lines · Nick Kossifidis
Quoted 18 lines

* Update ath5k_hw_reset and add some more documentation about PHY
calibration
* Fix ath5k_hw_set_power to use AR5K_SLEEP_CTL_SLE_ALLOW for Network
sleep
* Preserve sleep duration field while setting AR5K_SLEEP_CTL and reduce
delays & checks for register's status (got this from decompiling & dumps, it works for me but it needs testing)

Changes-licensed-under: ISC

---

drivers/net/wireless/ath5k/hw.c +0 -0
index cab1a8a..4fb048c 100644

[...]

Quoted 1 line
Quoted 8 lines

drivers/net/wireless/ath5k/hw.c int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah, · line 1499

Quoted 5 lines
drivers/net/wireless/ath5k/hw.c +1 -0
@@ -1461,6 +1496,7 @@ int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah,
 unsigned int queue)
                 /* Stop queue */
               ath5k_hw_reg_write(ah, tx_queue, AR5K_CR);
+               ath5k_hw_reg_read(ah, AR5K_CR);

This one was in my patch already.

Sorry, i just got that from decompiling/tracing registers and put it there, i didn't noticed that you had already fixed it (your patches were mostly for base.c, i didn't notice your changes in hw.c)...

Quoted 10 lines

drivers/net/wireless/ath5k/hw.c enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw · line 1741

Quoted 5 lines
drivers/net/wireless/ath5k/hw.c +0 -0
       } else {
               /*
                * Schedule TX disable and wait until queue is empty
@@ -1705,6 +1741,7 @@ enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw
 *ah, enum ath5k_int new_mask)

Adding from the code:
/*
* Disable card interrupts to prevent any race conditions

Quoted 1 line
Quoted 9 lines
Quoted 4 lines
        * (they will be re-enabled afterwards).
        */
       ath5k_hw_reg_write(ah, AR5K_IER_DISABLE, AR5K_IER);

+ ath5k_hw_reg_read(ah, AR5K_IER);

Is this first flush needed? Writes cannot be reordered, they are just asynchronous and flushed below. What race is this against? The interrupt might be processed (or pending) at this moment on other processor anyway.

I think i also got that from decompiling, i'll check it out but i guess patch can go in until then, the whole series also adds RF2425 support and many people want that (mostly eeepc users).

Signature · 3 lines
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick