[PATCH 1/7] ath5k: optimize tx descriptor setup

[PATCH 1/7] ath5k: optimize tx descriptor setup

[Felix Fietkau]

Use local variables to reduce the number of load/store operations on uncached
memory.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath5k/desc.c |   38 +++++++++++++++++---------------
 1 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
index 16b44ff..0a8a9ef 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -184,6 +184,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 {
 	struct ath5k_hw_4w_tx_ctl *tx_ctl;
 	unsigned int frame_len;
+	u32 txctl0 = 0, txctl1 = 0, txctl2 = 0, txctl3 = 0;
 
 	tx_ctl = &desc->ud.ds_tx5212.tx_ctl;
 
@@ -209,7 +210,8 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 		tx_power = AR5K_TUNE_MAX_TXPOWER;
 
 	/* Clear descriptor */
-	memset(&desc->ud.ds_tx5212, 0, sizeof(struct ath5k_hw_5212_tx_desc));
+	memset(&desc->ud.ds_tx5212.tx_stat, 0,
+	       sizeof(desc->ud.ds_tx5212.tx_stat));
 
 	/* Setup control descriptor */
 
@@ -221,7 +223,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	if (frame_len & ~AR5K_4W_TX_DESC_CTL0_FRAME_LEN)
 		return -EINVAL;
 
-	tx_ctl->tx_control_0 = frame_len & AR5K_4W_TX_DESC_CTL0_FRAME_LEN;
+	txctl0 = frame_len & AR5K_4W_TX_DESC_CTL0_FRAME_LEN;
 
 	/* Verify and set buffer length */
 
@@ -232,21 +234,17 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	if (pkt_len & ~AR5K_4W_TX_DESC_CTL1_BUF_LEN)
 		return -EINVAL;
 
-	tx_ctl->tx_control_1 = pkt_len & AR5K_4W_TX_DESC_CTL1_BUF_LEN;
+	txctl1 = pkt_len & AR5K_4W_TX_DESC_CTL1_BUF_LEN;
 
-	tx_ctl->tx_control_0 |=
-		AR5K_REG_SM(tx_power, AR5K_4W_TX_DESC_CTL0_XMIT_POWER) |
-		AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
-	tx_ctl->tx_control_1 |= AR5K_REG_SM(type,
-					AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
-	tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0,
-					AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
-	tx_ctl->tx_control_3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;
+	txctl0 |= AR5K_REG_SM(tx_power, AR5K_4W_TX_DESC_CTL0_XMIT_POWER) |
+		  AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
+	txctl1 |= AR5K_REG_SM(type, AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
+	txctl2 = AR5K_REG_SM(tx_tries0, AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
+	txctl3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;
 
 #define _TX_FLAGS(_c, _flag)					\
 	if (flags & AR5K_TXDESC_##_flag) {			\
-		tx_ctl->tx_control_##_c |=			\
-			AR5K_4W_TX_DESC_CTL##_c##_##_flag;	\
+		txctl##_c |= AR5K_4W_TX_DESC_CTL##_c##_##_flag;	\
 	}
 
 	_TX_FLAGS(0, CLRDMASK);
@@ -262,8 +260,8 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	 * WEP crap
 	 */
 	if (key_index != AR5K_TXKEYIX_INVALID) {
-		tx_ctl->tx_control_0 |= AR5K_4W_TX_DESC_CTL0_ENCRYPT_KEY_VALID;
-		tx_ctl->tx_control_1 |= AR5K_REG_SM(key_index,
+		txctl0 |= AR5K_4W_TX_DESC_CTL0_ENCRYPT_KEY_VALID;
+		txctl1 |= AR5K_REG_SM(key_index,
 				AR5K_4W_TX_DESC_CTL1_ENCRYPT_KEY_IDX);
 	}
 
@@ -274,12 +272,16 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 		if ((flags & AR5K_TXDESC_RTSENA) &&
 				(flags & AR5K_TXDESC_CTSENA))
 			return -EINVAL;
-		tx_ctl->tx_control_2 |= rtscts_duration &
-				AR5K_4W_TX_DESC_CTL2_RTS_DURATION;
-		tx_ctl->tx_control_3 |= AR5K_REG_SM(rtscts_rate,
+		txctl2 |= rtscts_duration & AR5K_4W_TX_DESC_CTL2_RTS_DURATION;
+		txctl3 |= AR5K_REG_SM(rtscts_rate,
 				AR5K_4W_TX_DESC_CTL3_RTS_CTS_RATE);
 	}
 
+	tx_ctl->tx_control_0 = txctl0;
+	tx_ctl->tx_control_1 = txctl1;
+	tx_ctl->tx_control_2 = txctl2;
+	tx_ctl->tx_control_3 = txctl3;
+
 	return 0;
 }
 
-- 
1.7.3.2

[PATCH 2/7] ath5k: remove ts_rate from ath5k_tx_status

[Felix Fietkau]

It is no longer necessary for preparing mac80211 tx status

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath5k/ath5k.h |    1 -
 drivers/net/wireless/ath/ath5k/desc.c  |   13 -------------
 2 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index 4bb381c..aa588a0 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -452,7 +452,6 @@ struct ath5k_tx_status {
 	u16	ts_seqnum;
 	u16	ts_tstamp;
 	u8	ts_status;
-	u8	ts_rate[4];
 	u8	ts_retry[4];
 	u8	ts_final_idx;
 	s8	ts_rssi;
diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
index 0a8a9ef..990a3b4 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -375,8 +375,6 @@ static int ath5k_hw_proc_2word_tx_status(struct ath5k_hw *ah,
 		AR5K_DESC_TX_STATUS1_ACK_SIG_STRENGTH);
 	ts->ts_antenna = 1;
 	ts->ts_status = 0;
-	ts->ts_rate[0] = AR5K_REG_MS(tx_ctl->tx_control_0,
-		AR5K_2W_TX_DESC_CTL0_XMIT_RATE);
 	ts->ts_retry[0] = ts->ts_longretry;
 	ts->ts_final_idx = 0;
 
@@ -439,32 +437,21 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
 	ts->ts_retry[ts->ts_final_idx] = ts->ts_longretry;
 	switch (ts->ts_final_idx) {
 	case 3:
-		ts->ts_rate[3] = AR5K_REG_MS(tx_ctl->tx_control_3,
-			AR5K_4W_TX_DESC_CTL3_XMIT_RATE3);
-
 		ts->ts_retry[2] = AR5K_REG_MS(tx_ctl->tx_control_2,
 			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES2);
 		ts->ts_longretry += ts->ts_retry[2];
 		/* fall through */
 	case 2:
-		ts->ts_rate[2] = AR5K_REG_MS(tx_ctl->tx_control_3,
-			AR5K_4W_TX_DESC_CTL3_XMIT_RATE2);
-
 		ts->ts_retry[1] = AR5K_REG_MS(tx_ctl->tx_control_2,
 			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES1);
 		ts->ts_longretry += ts->ts_retry[1];
 		/* fall through */
 	case 1:
-		ts->ts_rate[1] = AR5K_REG_MS(tx_ctl->tx_control_3,
-			AR5K_4W_TX_DESC_CTL3_XMIT_RATE1);
-
 		ts->ts_retry[0] = AR5K_REG_MS(tx_ctl->tx_control_2,
 			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES1);
 		ts->ts_longretry += ts->ts_retry[0];
 		/* fall through */
 	case 0:
-		ts->ts_rate[0] = tx_ctl->tx_control_3 &
-			AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;
 		break;
 	}
 
-- 
1.7.3.2

[PATCH 3/7] ath5k: optimize tx status processing

[Felix Fietkau]

Use ACCESS_ONCE to reduce the number of variable reloads on uncached memory

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath5k/desc.c |   37 ++++++++++++++++++--------------
 1 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
index 990a3b4..3758b96 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -401,32 +401,38 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
 {
 	struct ath5k_hw_4w_tx_ctl *tx_ctl;
 	struct ath5k_hw_tx_status *tx_status;
+	u32 txstat0, txstat1, txctl2;
 
 	tx_ctl = &desc->ud.ds_tx5212.tx_ctl;
 	tx_status = &desc->ud.ds_tx5212.tx_stat;
 
+	txstat1 = ACCESS_ONCE(tx_status->tx_status_1);
+
 	/* No frame has been send or error */
-	if (unlikely(!(tx_status->tx_status_1 & AR5K_DESC_TX_STATUS1_DONE)))
+	if (unlikely(!(txstat1 & AR5K_DESC_TX_STATUS1_DONE)))
 		return -EINPROGRESS;
 
+	txstat0 = ACCESS_ONCE(tx_status->tx_status_0);
+	txctl2 = ACCESS_ONCE(tx_ctl->tx_control_2);
+
 	/*
 	 * Get descriptor status
 	 */
-	ts->ts_tstamp = AR5K_REG_MS(tx_status->tx_status_0,
+	ts->ts_tstamp = AR5K_REG_MS(txstat0,
 		AR5K_DESC_TX_STATUS0_SEND_TIMESTAMP);
-	ts->ts_shortretry = AR5K_REG_MS(tx_status->tx_status_0,
+	ts->ts_shortretry = AR5K_REG_MS(txstat0,
 		AR5K_DESC_TX_STATUS0_SHORT_RETRY_COUNT);
-	ts->ts_longretry = AR5K_REG_MS(tx_status->tx_status_0,
+	ts->ts_longretry = AR5K_REG_MS(txstat0,
 		AR5K_DESC_TX_STATUS0_LONG_RETRY_COUNT);
-	ts->ts_seqnum = AR5K_REG_MS(tx_status->tx_status_1,
+	ts->ts_seqnum = AR5K_REG_MS(txstat1,
 		AR5K_DESC_TX_STATUS1_SEQ_NUM);
-	ts->ts_rssi = AR5K_REG_MS(tx_status->tx_status_1,
+	ts->ts_rssi = AR5K_REG_MS(txstat1,
 		AR5K_DESC_TX_STATUS1_ACK_SIG_STRENGTH);
-	ts->ts_antenna = (tx_status->tx_status_1 &
+	ts->ts_antenna = (txstat1 &
 		AR5K_DESC_TX_STATUS1_XMIT_ANTENNA_5212) ? 2 : 1;
 	ts->ts_status = 0;
 
-	ts->ts_final_idx = AR5K_REG_MS(tx_status->tx_status_1,
+	ts->ts_final_idx = AR5K_REG_MS(txstat1,
 			AR5K_DESC_TX_STATUS1_FINAL_TS_IX_5212);
 
 	/* The longretry counter has the number of un-acked retries
@@ -437,17 +443,17 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
 	ts->ts_retry[ts->ts_final_idx] = ts->ts_longretry;
 	switch (ts->ts_final_idx) {
 	case 3:
-		ts->ts_retry[2] = AR5K_REG_MS(tx_ctl->tx_control_2,
+		ts->ts_retry[2] = AR5K_REG_MS(txctl2,
 			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES2);
 		ts->ts_longretry += ts->ts_retry[2];
 		/* fall through */
 	case 2:
-		ts->ts_retry[1] = AR5K_REG_MS(tx_ctl->tx_control_2,
+		ts->ts_retry[1] = AR5K_REG_MS(txctl2,
 			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES1);
 		ts->ts_longretry += ts->ts_retry[1];
 		/* fall through */
 	case 1:
-		ts->ts_retry[0] = AR5K_REG_MS(tx_ctl->tx_control_2,
+		ts->ts_retry[0] = AR5K_REG_MS(txctl2,
 			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES1);
 		ts->ts_longretry += ts->ts_retry[0];
 		/* fall through */
@@ -456,15 +462,14 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
 	}
 
 	/* TX error */
-	if (!(tx_status->tx_status_0 & AR5K_DESC_TX_STATUS0_FRAME_XMIT_OK)) {
-		if (tx_status->tx_status_0 &
-				AR5K_DESC_TX_STATUS0_EXCESSIVE_RETRIES)
+	if (!(txstat0 & AR5K_DESC_TX_STATUS0_FRAME_XMIT_OK)) {
+		if (txstat0 & AR5K_DESC_TX_STATUS0_EXCESSIVE_RETRIES)
 			ts->ts_status |= AR5K_TXERR_XRETRY;
 
-		if (tx_status->tx_status_0 & AR5K_DESC_TX_STATUS0_FIFO_UNDERRUN)
+		if (txstat0 & AR5K_DESC_TX_STATUS0_FIFO_UNDERRUN)
 			ts->ts_status |= AR5K_TXERR_FIFO;
 
-		if (tx_status->tx_status_0 & AR5K_DESC_TX_STATUS0_FILTERED)
+		if (txstat0 & AR5K_DESC_TX_STATUS0_FILTERED)
 			ts->ts_status |= AR5K_TXERR_FILT;
 	}
 
-- 
1.7.3.2

[PATCH 4/7] ath5k: optimize rx status processing

[Felix Fietkau]

Use ACCESS_ONCE to reduce the number of redundant loads on uncached memory

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath5k/desc.c |   41 ++++++++++++++------------------
 1 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
index 3758b96..e366d30 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -603,37 +603,37 @@ static int ath5k_hw_proc_5212_rx_status(struct ath5k_hw *ah,
 					struct ath5k_rx_status *rs)
 {
 	struct ath5k_hw_rx_status *rx_status;
+	u32 rxstat0, rxstat1;
 
 	rx_status = &desc->ud.ds_rx.rx_stat;
+	rxstat1 = ACCESS_ONCE(rx_status->rx_status_1);
 
 	/* No frame received / not ready */
-	if (unlikely(!(rx_status->rx_status_1 &
-				AR5K_5212_RX_DESC_STATUS1_DONE)))
+	if (unlikely(!(rxstat1 & AR5K_5212_RX_DESC_STATUS1_DONE)))
 		return -EINPROGRESS;
 
 	memset(rs, 0, sizeof(struct ath5k_rx_status));
+	rxstat0 = ACCESS_ONCE(rx_status->rx_status_0);
 
 	/*
 	 * Frame receive status
 	 */
-	rs->rs_datalen = rx_status->rx_status_0 &
-		AR5K_5212_RX_DESC_STATUS0_DATA_LEN;
-	rs->rs_rssi = AR5K_REG_MS(rx_status->rx_status_0,
+	rs->rs_datalen = rxstat0 & AR5K_5212_RX_DESC_STATUS0_DATA_LEN;
+	rs->rs_rssi = AR5K_REG_MS(rxstat0,
 		AR5K_5212_RX_DESC_STATUS0_RECEIVE_SIGNAL);
-	rs->rs_rate = AR5K_REG_MS(rx_status->rx_status_0,
+	rs->rs_rate = AR5K_REG_MS(rxstat0,
 		AR5K_5212_RX_DESC_STATUS0_RECEIVE_RATE);
-	rs->rs_antenna = AR5K_REG_MS(rx_status->rx_status_0,
+	rs->rs_antenna = AR5K_REG_MS(rxstat0,
 		AR5K_5212_RX_DESC_STATUS0_RECEIVE_ANTENNA);
-	rs->rs_more = !!(rx_status->rx_status_0 &
-		AR5K_5212_RX_DESC_STATUS0_MORE);
-	rs->rs_tstamp = AR5K_REG_MS(rx_status->rx_status_1,
+	rs->rs_more = !!(rxstat0 & AR5K_5212_RX_DESC_STATUS0_MORE);
+	rs->rs_tstamp = AR5K_REG_MS(rxstat1,
 		AR5K_5212_RX_DESC_STATUS1_RECEIVE_TIMESTAMP);
 
 	/*
 	 * Key table status
 	 */
-	if (rx_status->rx_status_1 & AR5K_5212_RX_DESC_STATUS1_KEY_INDEX_VALID)
-		rs->rs_keyix = AR5K_REG_MS(rx_status->rx_status_1,
+	if (rxstat1 & AR5K_5212_RX_DESC_STATUS1_KEY_INDEX_VALID)
+		rs->rs_keyix = AR5K_REG_MS(rxstat1,
 					   AR5K_5212_RX_DESC_STATUS1_KEY_INDEX);
 	else
 		rs->rs_keyix = AR5K_RXKEYIX_INVALID;
@@ -641,27 +641,22 @@ static int ath5k_hw_proc_5212_rx_status(struct ath5k_hw *ah,
 	/*
 	 * Receive/descriptor errors
 	 */
-	if (!(rx_status->rx_status_1 &
-	    AR5K_5212_RX_DESC_STATUS1_FRAME_RECEIVE_OK)) {
-		if (rx_status->rx_status_1 &
-				AR5K_5212_RX_DESC_STATUS1_CRC_ERROR)
+	if (!(rxstat1 & AR5K_5212_RX_DESC_STATUS1_FRAME_RECEIVE_OK)) {
+		if (rxstat1 & AR5K_5212_RX_DESC_STATUS1_CRC_ERROR)
 			rs->rs_status |= AR5K_RXERR_CRC;
 
-		if (rx_status->rx_status_1 &
-				AR5K_5212_RX_DESC_STATUS1_PHY_ERROR) {
+		if (rxstat1 & AR5K_5212_RX_DESC_STATUS1_PHY_ERROR) {
 			rs->rs_status |= AR5K_RXERR_PHY;
-			rs->rs_phyerr = AR5K_REG_MS(rx_status->rx_status_1,
+			rs->rs_phyerr = AR5K_REG_MS(rxstat1,
 				AR5K_5212_RX_DESC_STATUS1_PHY_ERROR_CODE);
 			if (!ah->ah_capabilities.cap_has_phyerr_counters)
 				ath5k_ani_phy_error_report(ah, rs->rs_phyerr);
 		}
 
-		if (rx_status->rx_status_1 &
-				AR5K_5212_RX_DESC_STATUS1_DECRYPT_CRC_ERROR)
+		if (rxstat1 & AR5K_5212_RX_DESC_STATUS1_DECRYPT_CRC_ERROR)
 			rs->rs_status |= AR5K_RXERR_DECRYPT;
 
-		if (rx_status->rx_status_1 &
-				AR5K_5212_RX_DESC_STATUS1_MIC_ERROR)
+		if (rxstat1 & AR5K_5212_RX_DESC_STATUS1_MIC_ERROR)
 			rs->rs_status |= AR5K_RXERR_MIC;
 	}
 	return 0;
-- 
1.7.3.2

[PATCH 5/7] ath5k: remove ts_retry from ath5k_tx_status

[Felix Fietkau]

Reusing the configured retry counts from the skb cb is more efficient than
reloading the data from uncached memory.
Replace ts_longretry (unused) with ts_final_retry which contains the retry
count for the final rate only

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath5k/ath5k.h |    3 +-
 drivers/net/wireless/ath/ath5k/base.c  |   11 ++++++++-
 drivers/net/wireless/ath/ath5k/desc.c  |   34 ++-----------------------------
 3 files changed, 13 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index aa588a0..fcaf4ed 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -452,11 +452,10 @@ struct ath5k_tx_status {
 	u16	ts_seqnum;
 	u16	ts_tstamp;
 	u8	ts_status;
-	u8	ts_retry[4];
 	u8	ts_final_idx;
+	u8	ts_final_retry;
 	s8	ts_rssi;
 	u8	ts_shortretry;
-	u8	ts_longretry;
 	u8	ts_virtcol;
 	u8	ts_antenna;
 };
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 753662f..1a561b8 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1573,20 +1573,27 @@ ath5k_tx_frame_completed(struct ath5k_softc *sc, struct sk_buff *skb,
 			 struct ath5k_txq *txq, struct ath5k_tx_status *ts)
 {
 	struct ieee80211_tx_info *info;
+	u8 tries[3];
 	int i;
 
 	sc->stats.tx_all_count++;
 	sc->stats.tx_bytes_count += skb->len;
 	info = IEEE80211_SKB_CB(skb);
 
+	tries[0] = info->status.rates[0].count;
+	tries[1] = info->status.rates[1].count;
+	tries[2] = info->status.rates[2].count;
+
 	ieee80211_tx_info_clear_status(info);
-	for (i = 0; i <= ts->ts_final_idx; i++) {
+
+	for (i = 0; i < ts->ts_final_idx; i++) {
 		struct ieee80211_tx_rate *r =
 			&info->status.rates[i];
 
-		r->count = ts->ts_retry[i];
+		r->count = tries[i];
 	}
 
+	info->status.rates[ts->ts_final_idx].count = ts->ts_final_retry;
 	info->status.rates[ts->ts_final_idx + 1].idx = -1;
 
 	if (unlikely(ts->ts_status)) {
diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
index e366d30..0391813 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -366,7 +366,7 @@ static int ath5k_hw_proc_2word_tx_status(struct ath5k_hw *ah,
 		AR5K_DESC_TX_STATUS0_SEND_TIMESTAMP);
 	ts->ts_shortretry = AR5K_REG_MS(tx_status->tx_status_0,
 		AR5K_DESC_TX_STATUS0_SHORT_RETRY_COUNT);
-	ts->ts_longretry = AR5K_REG_MS(tx_status->tx_status_0,
+	ts->ts_final_retry = AR5K_REG_MS(tx_status->tx_status_0,
 		AR5K_DESC_TX_STATUS0_LONG_RETRY_COUNT);
 	/*TODO: ts->ts_virtcol + test*/
 	ts->ts_seqnum = AR5K_REG_MS(tx_status->tx_status_1,
@@ -375,7 +375,6 @@ static int ath5k_hw_proc_2word_tx_status(struct ath5k_hw *ah,
 		AR5K_DESC_TX_STATUS1_ACK_SIG_STRENGTH);
 	ts->ts_antenna = 1;
 	ts->ts_status = 0;
-	ts->ts_retry[0] = ts->ts_longretry;
 	ts->ts_final_idx = 0;
 
 	if (!(tx_status->tx_status_0 & AR5K_DESC_TX_STATUS0_FRAME_XMIT_OK)) {
@@ -401,7 +400,7 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
 {
 	struct ath5k_hw_4w_tx_ctl *tx_ctl;
 	struct ath5k_hw_tx_status *tx_status;
-	u32 txstat0, txstat1, txctl2;
+	u32 txstat0, txstat1;
 
 	tx_ctl = &desc->ud.ds_tx5212.tx_ctl;
 	tx_status = &desc->ud.ds_tx5212.tx_stat;
@@ -413,7 +412,6 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
 		return -EINPROGRESS;
 
 	txstat0 = ACCESS_ONCE(tx_status->tx_status_0);
-	txctl2 = ACCESS_ONCE(tx_ctl->tx_control_2);
 
 	/*
 	 * Get descriptor status
@@ -422,7 +420,7 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
 		AR5K_DESC_TX_STATUS0_SEND_TIMESTAMP);
 	ts->ts_shortretry = AR5K_REG_MS(txstat0,
 		AR5K_DESC_TX_STATUS0_SHORT_RETRY_COUNT);
-	ts->ts_longretry = AR5K_REG_MS(txstat0,
+	ts->ts_final_retry = AR5K_REG_MS(txstat0,
 		AR5K_DESC_TX_STATUS0_LONG_RETRY_COUNT);
 	ts->ts_seqnum = AR5K_REG_MS(txstat1,
 		AR5K_DESC_TX_STATUS1_SEQ_NUM);
@@ -435,32 +433,6 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
 	ts->ts_final_idx = AR5K_REG_MS(txstat1,
 			AR5K_DESC_TX_STATUS1_FINAL_TS_IX_5212);
 
-	/* The longretry counter has the number of un-acked retries
-	 * for the final rate. To get the total number of retries
-	 * we have to add the retry counters for the other rates
-	 * as well
-	 */
-	ts->ts_retry[ts->ts_final_idx] = ts->ts_longretry;
-	switch (ts->ts_final_idx) {
-	case 3:
-		ts->ts_retry[2] = AR5K_REG_MS(txctl2,
-			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES2);
-		ts->ts_longretry += ts->ts_retry[2];
-		/* fall through */
-	case 2:
-		ts->ts_retry[1] = AR5K_REG_MS(txctl2,
-			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES1);
-		ts->ts_longretry += ts->ts_retry[1];
-		/* fall through */
-	case 1:
-		ts->ts_retry[0] = AR5K_REG_MS(txctl2,
-			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES1);
-		ts->ts_longretry += ts->ts_retry[0];
-		/* fall through */
-	case 0:
-		break;
-	}
-
 	/* TX error */
 	if (!(txstat0 & AR5K_DESC_TX_STATUS0_FRAME_XMIT_OK)) {
 		if (txstat0 & AR5K_DESC_TX_STATUS0_EXCESSIVE_RETRIES)
-- 
1.7.3.2

[PATCH 6/7] ath5k: clean up debugfs code

[Felix Fietkau]

The pointers to the debugfs entries do not need to be saved, because they
will be recursively removed when the wiphy is unregistered.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath5k/base.c  |    1 -
 drivers/net/wireless/ath/ath5k/debug.c |   65 +++++++++----------------------
 drivers/net/wireless/ath/ath5k/debug.h |   17 --------
 3 files changed, 19 insertions(+), 64 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 1a561b8..a799d04 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -2901,7 +2901,6 @@ ath5k_deinit_softc(struct ath5k_softc *sc)
 	 * XXX: ??? detach ath5k_hw ???
 	 * Other than that, it's straightforward...
 	 */
-	ath5k_debug_finish_device(sc);
 	ieee80211_unregister_hw(hw);
 	ath5k_desc_free(sc);
 	ath5k_txq_release(sc);
diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c
index 0230f30..0bf7313 100644
--- a/drivers/net/wireless/ath/ath5k/debug.c
+++ b/drivers/net/wireless/ath/ath5k/debug.c
@@ -888,65 +888,38 @@ static const struct file_operations fops_queue = {
 void
 ath5k_debug_init_device(struct ath5k_softc *sc)
 {
-	sc->debug.level = ath5k_debug;
+	struct dentry *phydir;
 
-	sc->debug.debugfs_phydir = debugfs_create_dir("ath5k",
-				sc->hw->wiphy->debugfsdir);
+	sc->debug.level = ath5k_debug;
 
-	sc->debug.debugfs_debug = debugfs_create_file("debug",
-				S_IWUSR | S_IRUSR,
-				sc->debug.debugfs_phydir, sc, &fops_debug);
+	phydir = debugfs_create_dir("ath5k", sc->hw->wiphy->debugfsdir);
+	if (!phydir)
+	    return;
 
-	sc->debug.debugfs_registers = debugfs_create_file("registers", S_IRUSR,
-				sc->debug.debugfs_phydir, sc, &fops_registers);
+	debugfs_create_file("debug", S_IWUSR | S_IRUSR, phydir, sc,
+			    &fops_debug);
 
-	sc->debug.debugfs_beacon = debugfs_create_file("beacon",
-				S_IWUSR | S_IRUSR,
-				sc->debug.debugfs_phydir, sc, &fops_beacon);
+	debugfs_create_file("registers", S_IRUSR, phydir, sc, &fops_registers);
 
-	sc->debug.debugfs_reset = debugfs_create_file("reset", S_IWUSR,
-				sc->debug.debugfs_phydir, sc, &fops_reset);
+	debugfs_create_file("beacon", S_IWUSR | S_IRUSR, phydir, sc,
+			    &fops_beacon);
 
-	sc->debug.debugfs_antenna = debugfs_create_file("antenna",
-				S_IWUSR | S_IRUSR,
-				sc->debug.debugfs_phydir, sc, &fops_antenna);
+	debugfs_create_file("reset", S_IWUSR, phydir, sc, &fops_reset);
 
-	sc->debug.debugfs_misc = debugfs_create_file("misc",
-				S_IRUSR,
-				sc->debug.debugfs_phydir, sc, &fops_misc);
+	debugfs_create_file("antenna", S_IWUSR | S_IRUSR, phydir, sc,
+			    &fops_antenna);
 
-	sc->debug.debugfs_frameerrors = debugfs_create_file("frameerrors",
-				S_IWUSR | S_IRUSR,
-				sc->debug.debugfs_phydir, sc,
-				&fops_frameerrors);
+	debugfs_create_file("misc", S_IRUSR, phydir, sc, &fops_misc);
 
-	sc->debug.debugfs_ani = debugfs_create_file("ani",
-				S_IWUSR | S_IRUSR,
-				sc->debug.debugfs_phydir, sc,
-				&fops_ani);
+	debugfs_create_file("frameerrors", S_IWUSR | S_IRUSR, phydir, sc,
+			    &fops_frameerrors);
 
-	sc->debug.debugfs_queue = debugfs_create_file("queue",
-				S_IWUSR | S_IRUSR,
-				sc->debug.debugfs_phydir, sc,
-				&fops_queue);
-}
+	debugfs_create_file("ani", S_IWUSR | S_IRUSR, phydir, sc, &fops_ani);
 
-void
-ath5k_debug_finish_device(struct ath5k_softc *sc)
-{
-	debugfs_remove(sc->debug.debugfs_debug);
-	debugfs_remove(sc->debug.debugfs_registers);
-	debugfs_remove(sc->debug.debugfs_beacon);
-	debugfs_remove(sc->debug.debugfs_reset);
-	debugfs_remove(sc->debug.debugfs_antenna);
-	debugfs_remove(sc->debug.debugfs_misc);
-	debugfs_remove(sc->debug.debugfs_frameerrors);
-	debugfs_remove(sc->debug.debugfs_ani);
-	debugfs_remove(sc->debug.debugfs_queue);
-	debugfs_remove(sc->debug.debugfs_phydir);
+	debugfs_create_file("queue", S_IWUSR | S_IRUSR, phydir, sc,
+			    &fops_queue);
 }
 
-
 /* functions used in other places */
 
 void
diff --git a/drivers/net/wireless/ath/ath5k/debug.h b/drivers/net/wireless/ath/ath5k/debug.h
index b0355ae..193dd2d 100644
--- a/drivers/net/wireless/ath/ath5k/debug.h
+++ b/drivers/net/wireless/ath/ath5k/debug.h
@@ -68,17 +68,6 @@ struct ath5k_buf;
 
 struct ath5k_dbg_info {
 	unsigned int		level;		/* debug level */
-	/* debugfs entries */
-	struct dentry		*debugfs_phydir;
-	struct dentry		*debugfs_debug;
-	struct dentry		*debugfs_registers;
-	struct dentry		*debugfs_beacon;
-	struct dentry		*debugfs_reset;
-	struct dentry		*debugfs_antenna;
-	struct dentry		*debugfs_misc;
-	struct dentry		*debugfs_frameerrors;
-	struct dentry		*debugfs_ani;
-	struct dentry		*debugfs_queue;
 };
 
 /**
@@ -141,9 +130,6 @@ void
 ath5k_debug_init_device(struct ath5k_softc *sc);
 
 void
-ath5k_debug_finish_device(struct ath5k_softc *sc);
-
-void
 ath5k_debug_printrxbuffs(struct ath5k_softc *sc, struct ath5k_hw *ah);
 
 void
@@ -167,9 +153,6 @@ static inline void
 ath5k_debug_init_device(struct ath5k_softc *sc) {}
 
 static inline void
-ath5k_debug_finish_device(struct ath5k_softc *sc) {}
-
-static inline void
 ath5k_debug_printrxbuffs(struct ath5k_softc *sc, struct ath5k_hw *ah) {}
 
 static inline void
-- 
1.7.3.2

[PATCH 7/7] ath5k: reduce interrupt load caused by rx/tx interrupts

[Felix Fietkau]

While the rx/tx tasklet is pending, new unnecessary interrupts may arrive.
Decrease the load by temporarily disabling the interrupts until the tasklet
has completed.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath5k/ath5k.h |   13 +++++++++
 drivers/net/wireless/ath/ath5k/base.c  |   45 +++++++++++++++++++++++++++++--
 drivers/net/wireless/ath/ath5k/base.h  |    4 +++
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index fcaf4ed..e303db7 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -872,6 +872,19 @@ enum ath5k_int {
 	AR5K_INT_QTRIG	=	0x40000000, /* Non common */
 	AR5K_INT_GLOBAL =	0x80000000,
 
+	AR5K_INT_TX_ALL = AR5K_INT_TXOK
+		| AR5K_INT_TXDESC
+		| AR5K_INT_TXERR
+		| AR5K_INT_TXEOL
+		| AR5K_INT_TXURN,
+
+	AR5K_INT_RX_ALL = AR5K_INT_RXOK
+		| AR5K_INT_RXDESC
+		| AR5K_INT_RXERR
+		| AR5K_INT_RXNOFRM
+		| AR5K_INT_RXEOL
+		| AR5K_INT_RXORN,
+
 	AR5K_INT_COMMON  = AR5K_INT_RXOK
 		| AR5K_INT_RXDESC
 		| AR5K_INT_RXERR
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index a799d04..c7da004 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1444,6 +1444,21 @@ ath5k_receive_frame_ok(struct ath5k_softc *sc, struct ath5k_rx_status *rs)
 }
 
 static void
+ath5k_set_current_imask(struct ath5k_softc *sc)
+{
+	enum ath5k_int imask = sc->imask;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sc->irqlock, flags);
+	if (sc->rx_pending)
+		imask &= ~AR5K_INT_RX_ALL;
+	if (sc->tx_pending)
+		imask &= ~AR5K_INT_TX_ALL;
+	ath5k_hw_set_imr(sc->ah, imask);
+	spin_unlock_irqrestore(&sc->irqlock, flags);
+}
+
+static void
 ath5k_tasklet_rx(unsigned long data)
 {
 	struct ath5k_rx_status rs = {};
@@ -1506,6 +1521,8 @@ next:
 	} while (ath5k_rxbuf_setup(sc, bf) == 0);
 unlock:
 	spin_unlock(&sc->rxbuflock);
+	sc->rx_pending = false;
+	ath5k_set_current_imask(sc);
 }
 
 
@@ -1693,6 +1710,9 @@ ath5k_tasklet_tx(unsigned long data)
 	for (i=0; i < AR5K_NUM_TX_QUEUES; i++)
 		if (sc->txqs[i].setup && (sc->ah->ah_txq_isr & BIT(i)))
 			ath5k_tx_processq(sc, &sc->txqs[i]);
+
+	sc->tx_pending = false;
+	ath5k_set_current_imask(sc);
 }
 
 
@@ -2122,6 +2142,20 @@ ath5k_intr_calibration_poll(struct ath5k_hw *ah)
 	 * AR5K_REG_ENABLE_BITS(ah, AR5K_CR, AR5K_CR_SWI); */
 }
 
+static void
+ath5k_schedule_rx(struct ath5k_softc *sc)
+{
+	sc->rx_pending = true;
+	tasklet_schedule(&sc->rxtq);
+}
+
+static void
+ath5k_schedule_tx(struct ath5k_softc *sc)
+{
+	sc->tx_pending = true;
+	tasklet_schedule(&sc->txtq);
+}
+
 irqreturn_t
 ath5k_intr(int irq, void *dev_id)
 {
@@ -2164,7 +2198,7 @@ ath5k_intr(int irq, void *dev_id)
 				ieee80211_queue_work(sc->hw, &sc->reset_work);
 			}
 			else
-				tasklet_schedule(&sc->rxtq);
+				ath5k_schedule_rx(sc);
 		} else {
 			if (status & AR5K_INT_SWBA) {
 				tasklet_hi_schedule(&sc->beacontq);
@@ -2182,10 +2216,10 @@ ath5k_intr(int irq, void *dev_id)
 				ath5k_hw_update_tx_triglevel(ah, true);
 			}
 			if (status & (AR5K_INT_RXOK | AR5K_INT_RXERR))
-				tasklet_schedule(&sc->rxtq);
+				ath5k_schedule_rx(sc);
 			if (status & (AR5K_INT_TXOK | AR5K_INT_TXDESC
 					| AR5K_INT_TXERR | AR5K_INT_TXEOL))
-				tasklet_schedule(&sc->txtq);
+				ath5k_schedule_tx(sc);
 			if (status & AR5K_INT_BMISS) {
 				/* TODO */
 			}
@@ -2204,6 +2238,9 @@ ath5k_intr(int irq, void *dev_id)
 
 	} while (ath5k_hw_is_intr_pending(ah) && --counter > 0);
 
+	if (sc->rx_pending || sc->tx_pending)
+		ath5k_set_current_imask(sc);
+
 	if (unlikely(!counter))
 		ATH5K_WARN(sc, "too many interrupts, giving up for now\n");
 
@@ -2575,6 +2612,8 @@ done:
 
 static void stop_tasklets(struct ath5k_softc *sc)
 {
+	sc->rx_pending = false;
+	sc->tx_pending = false;
 	tasklet_kill(&sc->rxtq);
 	tasklet_kill(&sc->txtq);
 	tasklet_kill(&sc->calib);
diff --git a/drivers/net/wireless/ath/ath5k/base.h b/drivers/net/wireless/ath/ath5k/base.h
index 978f1f4..4c4e360 100644
--- a/drivers/net/wireless/ath/ath5k/base.h
+++ b/drivers/net/wireless/ath/ath5k/base.h
@@ -207,6 +207,10 @@ struct ath5k_softc {
 
 	enum ath5k_int		imask;		/* interrupt mask copy */
 
+	spinlock_t		irqlock;
+	bool			rx_pending;	/* rx tasklet pending */
+	bool			tx_pending;	/* tx tasklet pending */
+
 	u8			lladdr[ETH_ALEN];
 	u8			bssidmask[ETH_ALEN];
 
-- 
1.7.3.2

Re: [ath5k-devel] [PATCH 5/7] ath5k: remove ts_retry from ath5k_tx_status

[海藻敬之]

Hi,

Isn't there any chance in which info->status.rates[x].count has
  different retry-count value from the one which the firmware actually did.

  For example, driver requests invalid value of retry count and firmware rejects it (take other value).

  no such chance  ?

Thanks
Takayuki Kaiso
> Reusing the configured retry counts from the skb cb is more efficient than
> reloading the data from uncached memory.
> Replace ts_longretry (unused) with ts_final_retry which contains the retry
> count for the final rate only
>
> Signed-off-by: Felix Fietkau<nbd@openwrt.org>
> ---
>   drivers/net/wireless/ath/ath5k/ath5k.h |    3 +-
>   drivers/net/wireless/ath/ath5k/base.c  |   11 ++++++++-
>   drivers/net/wireless/ath/ath5k/desc.c  |   34 ++-----------------------------
>   3 files changed, 13 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
> index aa588a0..fcaf4ed 100644
> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
> @@ -452,11 +452,10 @@ struct ath5k_tx_status {
>   	u16	ts_seqnum;
>   	u16	ts_tstamp;
>   	u8	ts_status;
> -	u8	ts_retry[4];
>   	u8	ts_final_idx;
> +	u8	ts_final_retry;
>   	s8	ts_rssi;
>   	u8	ts_shortretry;
> -	u8	ts_longretry;
>   	u8	ts_virtcol;
>   	u8	ts_antenna;
>   };
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> index 753662f..1a561b8 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -1573,20 +1573,27 @@ ath5k_tx_frame_completed(struct ath5k_softc *sc, struct sk_buff *skb,
>   			 struct ath5k_txq *txq, struct ath5k_tx_status *ts)
>   {
>   	struct ieee80211_tx_info *info;
> +	u8 tries[3];
>   	int i;
>
>   	sc->stats.tx_all_count++;
>   	sc->stats.tx_bytes_count += skb->len;
>   	info = IEEE80211_SKB_CB(skb);
>
> +	tries[0] = info->status.rates[0].count;
> +	tries[1] = info->status.rates[1].count;
> +	tries[2] = info->status.rates[2].count;
> +
>   	ieee80211_tx_info_clear_status(info);
> -	for (i = 0; i<= ts->ts_final_idx; i++) {
> +
> +	for (i = 0; i<  ts->ts_final_idx; i++) {
>   		struct ieee80211_tx_rate *r =
>   			&info->status.rates[i];
>
> -		r->count = ts->ts_retry[i];
> +		r->count = tries[i];
>   	}
>
> +	info->status.rates[ts->ts_final_idx].count = ts->ts_final_retry;
>   	info->status.rates[ts->ts_final_idx + 1].idx = -1;
>
>   	if (unlikely(ts->ts_status)) {
> diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
> index e366d30..0391813 100644
> --- a/drivers/net/wireless/ath/ath5k/desc.c
> +++ b/drivers/net/wireless/ath/ath5k/desc.c
> @@ -366,7 +366,7 @@ static int ath5k_hw_proc_2word_tx_status(struct ath5k_hw *ah,
>   		AR5K_DESC_TX_STATUS0_SEND_TIMESTAMP);
>   	ts->ts_shortretry = AR5K_REG_MS(tx_status->tx_status_0,
>   		AR5K_DESC_TX_STATUS0_SHORT_RETRY_COUNT);
> -	ts->ts_longretry = AR5K_REG_MS(tx_status->tx_status_0,
> +	ts->ts_final_retry = AR5K_REG_MS(tx_status->tx_status_0,
>   		AR5K_DESC_TX_STATUS0_LONG_RETRY_COUNT);
>   	/*TODO: ts->ts_virtcol + test*/
>   	ts->ts_seqnum = AR5K_REG_MS(tx_status->tx_status_1,
> @@ -375,7 +375,6 @@ static int ath5k_hw_proc_2word_tx_status(struct ath5k_hw *ah,
>   		AR5K_DESC_TX_STATUS1_ACK_SIG_STRENGTH);
>   	ts->ts_antenna = 1;
>   	ts->ts_status = 0;
> -	ts->ts_retry[0] = ts->ts_longretry;
>   	ts->ts_final_idx = 0;
>
>   	if (!(tx_status->tx_status_0&  AR5K_DESC_TX_STATUS0_FRAME_XMIT_OK)) {
> @@ -401,7 +400,7 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
>   {
>   	struct ath5k_hw_4w_tx_ctl *tx_ctl;
>   	struct ath5k_hw_tx_status *tx_status;
> -	u32 txstat0, txstat1, txctl2;
> +	u32 txstat0, txstat1;
>
>   	tx_ctl =&desc->ud.ds_tx5212.tx_ctl;
>   	tx_status =&desc->ud.ds_tx5212.tx_stat;
> @@ -413,7 +412,6 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
>   		return -EINPROGRESS;
>
>   	txstat0 = ACCESS_ONCE(tx_status->tx_status_0);
> -	txctl2 = ACCESS_ONCE(tx_ctl->tx_control_2);
>
>   	/*
>   	 * Get descriptor status
> @@ -422,7 +420,7 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
>   		AR5K_DESC_TX_STATUS0_SEND_TIMESTAMP);
>   	ts->ts_shortretry = AR5K_REG_MS(txstat0,
>   		AR5K_DESC_TX_STATUS0_SHORT_RETRY_COUNT);
> -	ts->ts_longretry = AR5K_REG_MS(txstat0,
> +	ts->ts_final_retry = AR5K_REG_MS(txstat0,
>   		AR5K_DESC_TX_STATUS0_LONG_RETRY_COUNT);
>   	ts->ts_seqnum = AR5K_REG_MS(txstat1,
>   		AR5K_DESC_TX_STATUS1_SEQ_NUM);
> @@ -435,32 +433,6 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
>   	ts->ts_final_idx = AR5K_REG_MS(txstat1,
>   			AR5K_DESC_TX_STATUS1_FINAL_TS_IX_5212);
>
> -	/* The longretry counter has the number of un-acked retries
> -	 * for the final rate. To get the total number of retries
> -	 * we have to add the retry counters for the other rates
> -	 * as well
> -	 */
> -	ts->ts_retry[ts->ts_final_idx] = ts->ts_longretry;
> -	switch (ts->ts_final_idx) {
> -	case 3:
> -		ts->ts_retry[2] = AR5K_REG_MS(txctl2,
> -			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES2);
> -		ts->ts_longretry += ts->ts_retry[2];
> -		/* fall through */
> -	case 2:
> -		ts->ts_retry[1] = AR5K_REG_MS(txctl2,
> -			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES1);
> -		ts->ts_longretry += ts->ts_retry[1];
> -		/* fall through */
> -	case 1:
> -		ts->ts_retry[0] = AR5K_REG_MS(txctl2,
> -			AR5K_4W_TX_DESC_CTL2_XMIT_TRIES1);
> -		ts->ts_longretry += ts->ts_retry[0];
> -		/* fall through */
> -	case 0:
> -		break;
> -	}
> -
>   	/* TX error */
>   	if (!(txstat0&  AR5K_DESC_TX_STATUS0_FRAME_XMIT_OK)) {
>   		if (txstat0&  AR5K_DESC_TX_STATUS0_EXCESSIVE_RETRIES)

Re: [ath5k-devel] [PATCH 5/7] ath5k: remove ts_retry from ath5k_tx_status

[Felix Fietkau]

On 2011-04-12 9:04 AM, 海藻敬之 wrote:
> Hi,
>
> Isn't there any chance in which info->status.rates[x].count has
>    different retry-count value from the one which the firmware actually did.
>
>    For example, driver requests invalid value of retry count and firmware rejects it (take other value).
>
>    no such chance  ?
The driver will not request an invalid value, and even if it would, 
reading back the information from the control field in the descriptor 
would simply return the same value that was put in there in the first 
place. Only the retry count for the final rate in the status field 
contains useful data.

- Felix

Re: [PATCH 1/7] ath5k: optimize tx descriptor setup

[Kalle Valo]

Felix Fietkau <nbd@openwrt.org> writes:

> Use local variables to reduce the number of load/store operations on uncached
> memory.

A comment in the code would be nice as well. Otherwise later on
someone decides to "optimise" and remove the local variables :)

>  	/* Clear descriptor */
> -	memset(&desc->ud.ds_tx5212, 0, sizeof(struct ath5k_hw_5212_tx_desc));
> +	memset(&desc->ud.ds_tx5212.tx_stat, 0,
> +	       sizeof(desc->ud.ds_tx5212.tx_stat));

Is this an unrelated change? Or maybe I just missed something.

-- 
Kalle Valo

Re: [PATCH 1/7] ath5k: optimize tx descriptor setup

[Felix Fietkau]

On 2011-04-11 8:29 AM, Kalle Valo wrote:
> Felix Fietkau<nbd@openwrt.org>  writes:
>
>>  Use local variables to reduce the number of load/store operations on uncached
>>  memory.
>
> A comment in the code would be nice as well. Otherwise later on
> someone decides to "optimise" and remove the local variables :)
Makes sense.

>>   	/* Clear descriptor */
>>  -	memset(&desc->ud.ds_tx5212, 0, sizeof(struct ath5k_hw_5212_tx_desc));
>>  +	memset(&desc->ud.ds_tx5212.tx_stat, 0,
>>  +	       sizeof(desc->ud.ds_tx5212.tx_stat));
>
> Is this an unrelated change? Or maybe I just missed something.
This is related. After my change, all the control fields of the 
descriptor are properly written to, so the memset only needs to cover 
the status area.

- Felix

[PATCH v2 1/7] ath5k: optimize tx descriptor setup

[Felix Fietkau]

Use local variables to reduce the number of load/store operations on uncached
memory.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath5k/desc.c |   45 +++++++++++++++++++--------------
 1 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
index 16b44ff..6996d55 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -185,6 +185,12 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	struct ath5k_hw_4w_tx_ctl *tx_ctl;
 	unsigned int frame_len;
 
+	/*
+	 * Use local variables for these to reduce load/store access on
+	 * uncached memory
+	 */
+	u32 txctl0 = 0, txctl1 = 0, txctl2 = 0, txctl3 = 0;
+
 	tx_ctl = &desc->ud.ds_tx5212.tx_ctl;
 
 	/*
@@ -208,8 +214,9 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	if (tx_power > AR5K_TUNE_MAX_TXPOWER)
 		tx_power = AR5K_TUNE_MAX_TXPOWER;
 
-	/* Clear descriptor */
-	memset(&desc->ud.ds_tx5212, 0, sizeof(struct ath5k_hw_5212_tx_desc));
+	/* Clear descriptor status area */
+	memset(&desc->ud.ds_tx5212.tx_stat, 0,
+	       sizeof(desc->ud.ds_tx5212.tx_stat));
 
 	/* Setup control descriptor */
 
@@ -221,7 +228,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	if (frame_len & ~AR5K_4W_TX_DESC_CTL0_FRAME_LEN)
 		return -EINVAL;
 
-	tx_ctl->tx_control_0 = frame_len & AR5K_4W_TX_DESC_CTL0_FRAME_LEN;
+	txctl0 = frame_len & AR5K_4W_TX_DESC_CTL0_FRAME_LEN;
 
 	/* Verify and set buffer length */
 
@@ -232,21 +239,17 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	if (pkt_len & ~AR5K_4W_TX_DESC_CTL1_BUF_LEN)
 		return -EINVAL;
 
-	tx_ctl->tx_control_1 = pkt_len & AR5K_4W_TX_DESC_CTL1_BUF_LEN;
+	txctl1 = pkt_len & AR5K_4W_TX_DESC_CTL1_BUF_LEN;
 
-	tx_ctl->tx_control_0 |=
-		AR5K_REG_SM(tx_power, AR5K_4W_TX_DESC_CTL0_XMIT_POWER) |
-		AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
-	tx_ctl->tx_control_1 |= AR5K_REG_SM(type,
-					AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
-	tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0,
-					AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
-	tx_ctl->tx_control_3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;
+	txctl0 |= AR5K_REG_SM(tx_power, AR5K_4W_TX_DESC_CTL0_XMIT_POWER) |
+		  AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
+	txctl1 |= AR5K_REG_SM(type, AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
+	txctl2 = AR5K_REG_SM(tx_tries0, AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
+	txctl3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;
 
 #define _TX_FLAGS(_c, _flag)					\
 	if (flags & AR5K_TXDESC_##_flag) {			\
-		tx_ctl->tx_control_##_c |=			\
-			AR5K_4W_TX_DESC_CTL##_c##_##_flag;	\
+		txctl##_c |= AR5K_4W_TX_DESC_CTL##_c##_##_flag;	\
 	}
 
 	_TX_FLAGS(0, CLRDMASK);
@@ -262,8 +265,8 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	 * WEP crap
 	 */
 	if (key_index != AR5K_TXKEYIX_INVALID) {
-		tx_ctl->tx_control_0 |= AR5K_4W_TX_DESC_CTL0_ENCRYPT_KEY_VALID;
-		tx_ctl->tx_control_1 |= AR5K_REG_SM(key_index,
+		txctl0 |= AR5K_4W_TX_DESC_CTL0_ENCRYPT_KEY_VALID;
+		txctl1 |= AR5K_REG_SM(key_index,
 				AR5K_4W_TX_DESC_CTL1_ENCRYPT_KEY_IDX);
 	}
 
@@ -274,12 +277,16 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 		if ((flags & AR5K_TXDESC_RTSENA) &&
 				(flags & AR5K_TXDESC_CTSENA))
 			return -EINVAL;
-		tx_ctl->tx_control_2 |= rtscts_duration &
-				AR5K_4W_TX_DESC_CTL2_RTS_DURATION;
-		tx_ctl->tx_control_3 |= AR5K_REG_SM(rtscts_rate,
+		txctl2 |= rtscts_duration & AR5K_4W_TX_DESC_CTL2_RTS_DURATION;
+		txctl3 |= AR5K_REG_SM(rtscts_rate,
 				AR5K_4W_TX_DESC_CTL3_RTS_CTS_RATE);
 	}
 
+	tx_ctl->tx_control_0 = txctl0;
+	tx_ctl->tx_control_1 = txctl1;
+	tx_ctl->tx_control_2 = txctl2;
+	tx_ctl->tx_control_3 = txctl3;
+
 	return 0;
 }
 
-- 
1.7.3.2

Re: [PATCH v2 1/7] ath5k: optimize tx descriptor setup

[Sedat Dilek]

On Wed, Apr 13, 2011 at 12:07 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> Use local variables to reduce the number of load/store operations on uncached
> memory.
>
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> ---
>  drivers/net/wireless/ath/ath5k/desc.c |   45 +++++++++++++++++++--------------
>  1 files changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
> index 16b44ff..6996d55 100644
> --- a/drivers/net/wireless/ath/ath5k/desc.c
> +++ b/drivers/net/wireless/ath/ath5k/desc.c
> @@ -185,6 +185,12 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
>        struct ath5k_hw_4w_tx_ctl *tx_ctl;
>        unsigned int frame_len;
>
> +       /*
> +        * Use local variables for these to reduce load/store access on
> +        * uncached memory
> +        */
> +       u32 txctl0 = 0, txctl1 = 0, txctl2 = 0, txctl3 = 0;
> +
>        tx_ctl = &desc->ud.ds_tx5212.tx_ctl;
>
>        /*
> @@ -208,8 +214,9 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
>        if (tx_power > AR5K_TUNE_MAX_TXPOWER)
>                tx_power = AR5K_TUNE_MAX_TXPOWER;
>
> -       /* Clear descriptor */
> -       memset(&desc->ud.ds_tx5212, 0, sizeof(struct ath5k_hw_5212_tx_desc));
> +       /* Clear descriptor status area */
> +       memset(&desc->ud.ds_tx5212.tx_stat, 0,
> +              sizeof(desc->ud.ds_tx5212.tx_stat));
>
>        /* Setup control descriptor */
>
> @@ -221,7 +228,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
>        if (frame_len & ~AR5K_4W_TX_DESC_CTL0_FRAME_LEN)
>                return -EINVAL;
>
> -       tx_ctl->tx_control_0 = frame_len & AR5K_4W_TX_DESC_CTL0_FRAME_LEN;
> +       txctl0 = frame_len & AR5K_4W_TX_DESC_CTL0_FRAME_LEN;
>
>        /* Verify and set buffer length */
>
> @@ -232,21 +239,17 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
>        if (pkt_len & ~AR5K_4W_TX_DESC_CTL1_BUF_LEN)
>                return -EINVAL;
>
> -       tx_ctl->tx_control_1 = pkt_len & AR5K_4W_TX_DESC_CTL1_BUF_LEN;
> +       txctl1 = pkt_len & AR5K_4W_TX_DESC_CTL1_BUF_LEN;
>
> -       tx_ctl->tx_control_0 |=
> -               AR5K_REG_SM(tx_power, AR5K_4W_TX_DESC_CTL0_XMIT_POWER) |
> -               AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
> -       tx_ctl->tx_control_1 |= AR5K_REG_SM(type,
> -                                       AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
> -       tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0,
> -                                       AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
> -       tx_ctl->tx_control_3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;
> +       txctl0 |= AR5K_REG_SM(tx_power, AR5K_4W_TX_DESC_CTL0_XMIT_POWER) |
> +                 AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
> +       txctl1 |= AR5K_REG_SM(type, AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
> +       txctl2 = AR5K_REG_SM(tx_tries0, AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
> +       txctl3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;
>
>  #define _TX_FLAGS(_c, _flag)                                   \
>        if (flags & AR5K_TXDESC_##_flag) {                      \
> -               tx_ctl->tx_control_##_c |=                      \
> -                       AR5K_4W_TX_DESC_CTL##_c##_##_flag;      \
> +               txctl##_c |= AR5K_4W_TX_DESC_CTL##_c##_##_flag; \
>        }
>
>        _TX_FLAGS(0, CLRDMASK);
> @@ -262,8 +265,8 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
>         * WEP crap
>         */
>        if (key_index != AR5K_TXKEYIX_INVALID) {
> -               tx_ctl->tx_control_0 |= AR5K_4W_TX_DESC_CTL0_ENCRYPT_KEY_VALID;
> -               tx_ctl->tx_control_1 |= AR5K_REG_SM(key_index,
> +               txctl0 |= AR5K_4W_TX_DESC_CTL0_ENCRYPT_KEY_VALID;
> +               txctl1 |= AR5K_REG_SM(key_index,
>                                AR5K_4W_TX_DESC_CTL1_ENCRYPT_KEY_IDX);
>        }
>
> @@ -274,12 +277,16 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
>                if ((flags & AR5K_TXDESC_RTSENA) &&
>                                (flags & AR5K_TXDESC_CTSENA))
>                        return -EINVAL;
> -               tx_ctl->tx_control_2 |= rtscts_duration &
> -                               AR5K_4W_TX_DESC_CTL2_RTS_DURATION;
> -               tx_ctl->tx_control_3 |= AR5K_REG_SM(rtscts_rate,
> +               txctl2 |= rtscts_duration & AR5K_4W_TX_DESC_CTL2_RTS_DURATION;
> +               txctl3 |= AR5K_REG_SM(rtscts_rate,
>                                AR5K_4W_TX_DESC_CTL3_RTS_CTS_RATE);
>        }
>
> +       tx_ctl->tx_control_0 = txctl0;
> +       tx_ctl->tx_control_1 = txctl1;
> +       tx_ctl->tx_control_2 = txctl2;
> +       tx_ctl->tx_control_3 = txctl3;
> +
>        return 0;
>  }
>
> --
> 1.7.3.2
>

Looks like this v2 did not found its way to wireless-next [1].

- Sedat -

[1] http://git.kernel.org/?p=linux/kernel/git/linville/wireless-next-2.6.git;a=commit;h=c5e0a88aa2e0f42cdb4c79c977c52f6bc38ec160

Re: [PATCH v2 1/7] ath5k: optimize tx descriptor setup

[John W. Linville]

On Wed, Apr 13, 2011 at 06:23:40AM +0200, Sedat Dilek wrote:
> On Wed, Apr 13, 2011 at 12:07 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> > Use local variables to reduce the number of load/store operations on uncached
> > memory.
> >
> > Signed-off-by: Felix Fietkau <nbd@openwrt.org>

> Looks like this v2 did not found its way to wireless-next [1].

That's what happens, when it is posted after I've already merged the v1. :-)

I fixed things up and will commit the difference with the following message:

    ath5k: improve comments for optimized tx descriptor setup
    
    Comment the use of local variables to reduce the number of load/store
    operations on uncached memory, in hopes of not losing this optimization
    accidentally in the future.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH v2 1/7] ath5k: optimize tx descriptor setup

[Kalle Valo]

"John W. Linville" <linville@tuxdriver.com> writes:

>> Looks like this v2 did not found its way to wireless-next [1].
>
> That's what happens, when it is posted after I've already merged the v1. :-)
>
> I fixed things up and will commit the difference with the following message:
>
>     ath5k: improve comments for optimized tx descriptor setup
>     
>     Comment the use of local variables to reduce the number of load/store
>     operations on uncached memory, in hopes of not losing this optimization
>     accidentally in the future.

Thanks John. It was my fault as I was so slow to comment. But I blame
the sun for this, it's so amazing to see the sun again after six
months! ;)

-- 
Kalle Valo

Re: [PATCH v2 1/7] ath5k: optimize tx descriptor setup

[Sedat Dilek]

On Wed, Apr 13, 2011 at 8:34 PM, Kalle Valo <kvalo@adurom.com> wrote:
> "John W. Linville" <linville@tuxdriver.com> writes:
>
>>> Looks like this v2 did not found its way to wireless-next [1].
>>
>> That's what happens, when it is posted after I've already merged the v1. :-)
>>
>> I fixed things up and will commit the difference with the following message:
>>
>>     ath5k: improve comments for optimized tx descriptor setup
>>
>>     Comment the use of local variables to reduce the number of load/store
>>     operations on uncached memory, in hopes of not losing this optimization
>>     accidentally in the future.
>
> Thanks John. It was my fault as I was so slow to comment. But I blame
> the sun for this, it's so amazing to see the sun again after six
> months! ;)
>
> --
> Kalle Valo
>

Nice, the diff went into wireless-next-2.6 GIT [1] (even comments are welcome!).

I would enjoy in the future, if you add by yourself a Reported-by as I
saw from Linus Torvalds adding it to commits where maintainers simply
forget it.
Without reviewing/testing/reporting the code won't get better and more stable.
I had tested all 10 or 11 recent ath5k patches, but lazy or simply
trust the work by Felix.

Not sure if I should add a Tested-by to all single patches of patchset
or it's OK to reply to the 1st in series?
Often I see a summary describing the patchset, something like "0/7: My
super patchset".
Replying only to 0/7 would make life easier... but I am *still*
learning from git use-case-2-use-case.
Ted (maintainer of ext4) told me even he is distinguising between
Reported-and-tested-by and Reported-by/Tested-by.
As a summary, don't forget to give credits to people.
(P.S. I am currently here on a lame-ass IBM T40p with Pentium-M... I
will contribute till it explodes someday).

- Sedat -

P.S. I am currently here on a lame-ass IBM T40p with Pentium-M... I
will contribute till it explodes someday.

[1] http://git.us.kernel.org/?p=linux/kernel/git/linville/wireless-next-2.6.git;a=commit;h=8962d87129ec0a820d17ac44cbf3f51010ad8db8