2011-04-10 16:32:24

by Felix Fietkau

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

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

Signed-off-by: Felix Fietkau <[email protected]>
---
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



2011-04-12 07:04:32

by 海藻敬之

[permalink] [raw]
Subject: 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<[email protected]>
> ---
> 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)



2011-04-10 16:32:25

by Felix Fietkau

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

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

Signed-off-by: Felix Fietkau <[email protected]>
---
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


2011-04-11 06:29:35

by Kalle Valo

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

Felix Fietkau <[email protected]> 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

2011-04-13 18:34:39

by Kalle Valo

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

"John W. Linville" <[email protected]> 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

2011-04-10 16:32:24

by Felix Fietkau

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

It is no longer necessary for preparing mac80211 tx status

Signed-off-by: Felix Fietkau <[email protected]>
---
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


2011-04-10 16:32:24

by Felix Fietkau

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

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 <[email protected]>
---
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


2011-04-10 16:32:24

by Felix Fietkau

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

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 <[email protected]>
---
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


2011-04-10 16:32:24

by Felix Fietkau

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

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

Signed-off-by: Felix Fietkau <[email protected]>
---
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


2011-04-13 04:23:41

by Sedat Dilek

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

On Wed, Apr 13, 2011 at 12:07 AM, Felix Fietkau <[email protected]> wrote:
> Use local variables to reduce the number of load/store operations on uncached
> memory.
>
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
>  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

2011-04-10 16:32:24

by Felix Fietkau

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

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 <[email protected]>
---
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


2011-04-13 13:00:32

by John W. Linville

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

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

> 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
[email protected] might be all we have. Be ready.

2011-04-12 21:55:52

by Felix Fietkau

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

On 2011-04-11 8:29 AM, Kalle Valo wrote:
> Felix Fietkau<[email protected]> 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

2011-04-12 21:43:26

by Felix Fietkau

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

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

2011-04-14 07:27:52

by Sedat Dilek

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

On Wed, Apr 13, 2011 at 8:34 PM, Kalle Valo <[email protected]> wrote:
> "John W. Linville" <[email protected]> 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

2011-04-12 22:07:23

by Felix Fietkau

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

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

Signed-off-by: Felix Fietkau <[email protected]>
---
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