2011-08-30 08:30:59

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v9 1/4] ath9k: eliminate common->{rx,tx}_chainmask

we already have ah->{rx,tx}chainmask for the same purpose

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath.h | 3 ---
drivers/net/wireless/ath/ath9k/ar9003_paprd.c | 4 ++--
drivers/net/wireless/ath/ath9k/beacon.c | 2 +-
drivers/net/wireless/ath/ath9k/debug.c | 20 ++++++++++----------
drivers/net/wireless/ath/ath9k/htc_drv_init.c | 7 ++-----
drivers/net/wireless/ath/ath9k/htc_drv_main.c | 3 +--
drivers/net/wireless/ath/ath9k/hw.c | 5 ++---
drivers/net/wireless/ath/ath9k/init.c | 9 ++-------
drivers/net/wireless/ath/ath9k/main.c | 7 +++----
drivers/net/wireless/ath/ath9k/xmit.c | 8 ++++----
10 files changed, 27 insertions(+), 41 deletions(-)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index 17c4b56..f6b77c2 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -140,9 +140,6 @@ struct ath_common {
u8 curbssid[ETH_ALEN];
u8 bssidmask[ETH_ALEN];

- u8 tx_chainmask;
- u8 rx_chainmask;
-
u32 rx_bufsize;

u32 keymax;
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_paprd.c b/drivers/net/wireless/ath/ath9k/ar9003_paprd.c
index f80d1d6..bb2214f 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_paprd.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_paprd.c
@@ -113,7 +113,7 @@ static int ar9003_get_training_power_5g(struct ath_hw *ah)
if (delta > scale)
return -1;

- switch (get_streams(common->tx_chainmask)) {
+ switch (get_streams(ah->txchainmask)) {
case 1:
delta = 6;
break;
@@ -126,7 +126,7 @@ static int ar9003_get_training_power_5g(struct ath_hw *ah)
default:
delta = 0;
ath_dbg(common, ATH_DBG_CALIBRATE,
- "Invalid tx-chainmask: %u\n", common->tx_chainmask);
+ "Invalid tx-chainmask: %u\n", ah->txchainmask);
}

power += delta;
diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index 086c9c8..0c757c9 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -107,7 +107,7 @@ static void ath_beacon_setup(struct ath_softc *sc, struct ath_vif *avp,
series[0].Tries = 1;
series[0].Rate = rate;
series[0].ChSel = ath_txchainmask_reduction(sc,
- common->tx_chainmask, series[0].Rate);
+ ah->txchainmask, series[0].Rate);
series[0].RateFlags = (ctsrate) ? ATH9K_RATESERIES_RTS_CTS : 0;
ath9k_hw_set11n_ratescenario(ah, ds, ds, 0, ctsrate, ctsduration,
series, 4, 0);
diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index e1f1a96..4062e077 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -95,11 +95,11 @@ static ssize_t read_file_tx_chainmask(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
struct ath_softc *sc = file->private_data;
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ath_hw *ah = sc->sc_ah;
char buf[32];
unsigned int len;

- len = sprintf(buf, "0x%08x\n", common->tx_chainmask);
+ len = sprintf(buf, "0x%08x\n", ah->txchainmask);
return simple_read_from_buffer(user_buf, count, ppos, buf, len);
}

@@ -107,7 +107,7 @@ static ssize_t write_file_tx_chainmask(struct file *file, const char __user *use
size_t count, loff_t *ppos)
{
struct ath_softc *sc = file->private_data;
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ath_hw *ah = sc->sc_ah;
unsigned long mask;
char buf[32];
ssize_t len;
@@ -120,8 +120,8 @@ static ssize_t write_file_tx_chainmask(struct file *file, const char __user *use
if (strict_strtoul(buf, 0, &mask))
return -EINVAL;

- common->tx_chainmask = mask;
- sc->sc_ah->caps.tx_chainmask = mask;
+ ah->txchainmask = mask;
+ ah->caps.tx_chainmask = mask;
return count;
}

@@ -138,11 +138,11 @@ static ssize_t read_file_rx_chainmask(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
struct ath_softc *sc = file->private_data;
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ath_hw *ah = sc->sc_ah;
char buf[32];
unsigned int len;

- len = sprintf(buf, "0x%08x\n", common->rx_chainmask);
+ len = sprintf(buf, "0x%08x\n", ah->rxchainmask);
return simple_read_from_buffer(user_buf, count, ppos, buf, len);
}

@@ -150,7 +150,7 @@ static ssize_t write_file_rx_chainmask(struct file *file, const char __user *use
size_t count, loff_t *ppos)
{
struct ath_softc *sc = file->private_data;
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ath_hw *ah = sc->sc_ah;
unsigned long mask;
char buf[32];
ssize_t len;
@@ -163,8 +163,8 @@ static ssize_t write_file_rx_chainmask(struct file *file, const char __user *use
if (strict_strtoul(buf, 0, &mask))
return -EINVAL;

- common->rx_chainmask = mask;
- sc->sc_ah->caps.rx_chainmask = mask;
+ ah->rxchainmask = mask;
+ ah->caps.rx_chainmask = mask;
return count;
}

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
index 9cf42f6..966661c 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
@@ -509,8 +509,8 @@ static void setup_ht_cap(struct ath9k_htc_priv *priv,
memset(&ht_info->mcs, 0, sizeof(ht_info->mcs));

/* ath9k_htc supports only 1 or 2 stream devices */
- tx_streams = ath9k_cmn_count_streams(common->tx_chainmask, 2);
- rx_streams = ath9k_cmn_count_streams(common->rx_chainmask, 2);
+ tx_streams = ath9k_cmn_count_streams(priv->ah->txchainmask, 2);
+ rx_streams = ath9k_cmn_count_streams(priv->ah->rxchainmask, 2);

ath_dbg(common, ATH_DBG_CONFIG,
"TX streams %d, RX streams: %d\n",
@@ -601,9 +601,6 @@ static void ath9k_init_misc(struct ath9k_htc_priv *priv)
{
struct ath_common *common = ath9k_hw_common(priv->ah);

- common->tx_chainmask = priv->ah->caps.tx_chainmask;
- common->rx_chainmask = priv->ah->caps.rx_chainmask;
-
memcpy(common->bssidmask, ath_bcast_mac, ETH_ALEN);

priv->ah->opmode = NL80211_IFTYPE_STATION;
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
index 0248024..379f6ba 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
@@ -826,8 +826,7 @@ void ath9k_htc_ani_work(struct work_struct *work)
if (longcal || shortcal)
common->ani.caldone =
ath9k_hw_calibrate(ah, ah->curchan,
- common->rx_chainmask,
- longcal);
+ ah->rxchainmask, longcal);

ath9k_htc_ps_restore(priv);
}
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 3ccadeb..d63f9c8 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -1470,9 +1470,6 @@ int ath9k_hw_reset(struct ath_hw *ah, struct ath9k_channel *chan,
u64 tsf = 0;
int i, r;

- ah->txchainmask = common->tx_chainmask;
- ah->rxchainmask = common->rx_chainmask;
-
if (!ath9k_hw_setpower(ah, ATH9K_PM_AWAKE))
return -EIO;

@@ -2086,6 +2083,8 @@ int ath9k_hw_fill_cap_info(struct ath_hw *ah)

pCap->tx_chainmask = fixup_chainmask(chip_chainmask, pCap->tx_chainmask);
pCap->rx_chainmask = fixup_chainmask(chip_chainmask, pCap->rx_chainmask);
+ ah->txchainmask = pCap->tx_chainmask;
+ ah->rxchainmask = pCap->rx_chainmask;

ah->misc_mode |= AR_PCU_MIC_NEW_LOC_ENA;

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index d7761d1..31ef501 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -270,8 +270,8 @@ static void setup_ht_cap(struct ath_softc *sc,

/* set up supported mcs set */
memset(&ht_info->mcs, 0, sizeof(ht_info->mcs));
- tx_streams = ath9k_cmn_count_streams(common->tx_chainmask, max_streams);
- rx_streams = ath9k_cmn_count_streams(common->rx_chainmask, max_streams);
+ tx_streams = ath9k_cmn_count_streams(ah->txchainmask, max_streams);
+ rx_streams = ath9k_cmn_count_streams(ah->rxchainmask, max_streams);

ath_dbg(common, ATH_DBG_CONFIG,
"TX streams %d, RX streams: %d\n",
@@ -506,9 +506,6 @@ static void ath9k_init_misc(struct ath_softc *sc)
sc->sc_flags |= SC_OP_RXAGGR;
}

- common->tx_chainmask = sc->sc_ah->caps.tx_chainmask;
- common->rx_chainmask = sc->sc_ah->caps.rx_chainmask;
-
ath9k_hw_set_diversity(sc->sc_ah, true);
sc->rx.defant = ath9k_hw_getdefantenna(sc->sc_ah);

@@ -645,10 +642,8 @@ static void ath9k_init_band_txpower(struct ath_softc *sc, int band)
static void ath9k_init_txpower_limits(struct ath_softc *sc)
{
struct ath_hw *ah = sc->sc_ah;
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
struct ath9k_channel *curchan = ah->curchan;

- ah->txchainmask = common->tx_chainmask;
if (ah->caps.hw_caps & ATH9K_HW_CAP_2GHZ)
ath9k_init_band_txpower(sc, IEEE80211_BAND_2GHZ);
if (ah->caps.hw_caps & ATH9K_HW_CAP_5GHZ)
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 7ac1c21..085ec20 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -318,7 +318,6 @@ static void ath_paprd_activate(struct ath_softc *sc)
{
struct ath_hw *ah = sc->sc_ah;
struct ath9k_hw_cal_data *caldata = ah->caldata;
- struct ath_common *common = ath9k_hw_common(ah);
int chain;

if (!caldata || !caldata->paprd_done)
@@ -327,7 +326,7 @@ static void ath_paprd_activate(struct ath_softc *sc)
ath9k_ps_wakeup(sc);
ar9003_paprd_enable(ah, false);
for (chain = 0; chain < AR9300_MAX_CHAINS; chain++) {
- if (!(common->tx_chainmask & BIT(chain)))
+ if (!(ah->txchainmask & BIT(chain)))
continue;

ar9003_paprd_populate_single_table(ah, caldata, chain);
@@ -414,7 +413,7 @@ void ath_paprd_calibrate(struct work_struct *work)
memcpy(hdr->addr3, hw->wiphy->perm_addr, ETH_ALEN);

for (chain = 0; chain < AR9300_MAX_CHAINS; chain++) {
- if (!(common->tx_chainmask & BIT(chain)))
+ if (!(ah->txchainmask & BIT(chain)))
continue;

chain_ok = 0;
@@ -535,7 +534,7 @@ void ath_ani_calibrate(unsigned long data)
if (longcal || shortcal) {
common->ani.caldone =
ath9k_hw_calibrate(ah, ah->curchan,
- common->rx_chainmask, longcal);
+ ah->rxchainmask, longcal);
}

ath9k_ps_restore(sc);
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 68066c5..49b93c2 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1633,7 +1633,7 @@ u8 ath_txchainmask_reduction(struct ath_softc *sc, u8 chainmask, u32 rate)

static void ath_buf_set_rate(struct ath_softc *sc, struct ath_buf *bf, int len)
{
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ath_hw *ah = sc->sc_ah;
struct ath9k_11n_rate_series series[4];
struct sk_buff *skb;
struct ieee80211_tx_info *tx_info;
@@ -1693,7 +1693,7 @@ static void ath_buf_set_rate(struct ath_softc *sc, struct ath_buf *bf, int len)
/* MCS rates */
series[i].Rate = rix | 0x80;
series[i].ChSel = ath_txchainmask_reduction(sc,
- common->tx_chainmask, series[i].Rate);
+ ah->txchainmask, series[i].Rate);
series[i].PktDuration = ath_pkt_duration(sc, rix, len,
is_40, is_sgi, is_sp);
if (rix < 8 && (tx_info->flags & IEEE80211_TX_CTL_STBC))
@@ -1718,10 +1718,10 @@ static void ath_buf_set_rate(struct ath_softc *sc, struct ath_buf *bf, int len)
}

if (bf->bf_state.bfs_paprd)
- series[i].ChSel = common->tx_chainmask;
+ series[i].ChSel = ah->txchainmask;
else
series[i].ChSel = ath_txchainmask_reduction(sc,
- common->tx_chainmask, series[i].Rate);
+ ah->txchainmask, series[i].Rate);

series[i].PktDuration = ath9k_hw_computetxtime(sc->sc_ah,
phy, rate->bitrate * 100, len, rix, is_sp);
--
1.7.3.2



2011-08-30 15:52:12

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v9 2/4] ath9k: always call ath_reset from workqueue context

On 2011-08-30 4:20 PM, Rajkumar Manoharan wrote:
> On Tue, Aug 30, 2011 at 10:30:45AM +0200, Felix Fietkau wrote:
>> This makes it much easier to add further rework to avoid race conditions
>> between reset and other work items.
>> Move other functions to make ath_reset static.
>>
>> Signed-off-by: Felix Fietkau<[email protected]>
>> ---
>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>> index 49b93c2..2b2a975 100644
>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>> @@ -581,8 +581,10 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
>>
>> rcu_read_unlock();
>>
>> - if (needreset)
>> - ath_reset(sc, false);
>> + if (needreset) {
>> + sc->sc_flags |= SC_OP_HW_RESET;
>> + ieee80211_queue_work(sc->hw,&sc->hw_reset_work);
>> + }
>> }
>>
> Still ath_txq_schedule can be scheduled. Return a error code from
> ath_tx_complete_aggr to abort tx process intead of SC_OP_HW_RESET.
> That looks much more generic and simple.
I added this to make sure that there won't be any subsequent attempts at
processing the tx queue until the reset (e.g. if there was a tx status
IRQ in the mean time).

- Felix

2011-08-30 08:31:00

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v9 3/4] ath9k: merge reset related functions

reduces unnecessary code duplication. Also takes the sc_pcu_lock within
ath_reset instead of callsites, which makes it possible to always cancel
all queued work items before the reset, possibly fixing a few race
conditions (work items vs reset) along with it.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/main.c | 304 ++++++++++++++-------------------
1 files changed, 128 insertions(+), 176 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 4f74ba5..944c50a 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -212,84 +212,57 @@ static int ath_update_survey_stats(struct ath_softc *sc)
return ret;
}

-/*
- * Set/change channels. If the channel is really being changed, it's done
- * by reseting the chip. To accomplish this we must first cleanup any pending
- * DMA, then restart stuff.
-*/
-static int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
- struct ath9k_channel *hchan)
+static void __ath_cancel_work(struct ath_softc *sc)
{
- struct ath_hw *ah = sc->sc_ah;
- struct ath_common *common = ath9k_hw_common(ah);
- struct ieee80211_conf *conf = &common->hw->conf;
- bool fastcc = true, stopped;
- struct ieee80211_channel *channel = hw->conf.channel;
- struct ath9k_hw_cal_data *caldata = NULL;
- int r;
-
- if (sc->sc_flags & SC_OP_INVALID)
- return -EIO;
-
- sc->hw_busy_count = 0;
-
- del_timer_sync(&common->ani.timer);
cancel_work_sync(&sc->paprd_work);
cancel_work_sync(&sc->hw_check_work);
- cancel_work_sync(&sc->hw_reset_work);
cancel_delayed_work_sync(&sc->tx_complete_work);
cancel_delayed_work_sync(&sc->hw_pll_work);
+}

- ath9k_ps_wakeup(sc);
+static void ath_cancel_work(struct ath_softc *sc)
+{
+ __ath_cancel_work(sc);
+ cancel_work_sync(&sc->hw_reset_work);
+}

- spin_lock_bh(&sc->sc_pcu_lock);
+static bool ath_prepare_reset(struct ath_softc *sc, bool retry_tx, bool flush)
+{
+ struct ath_hw *ah = sc->sc_ah;
+ struct ath_common *common = ath9k_hw_common(ah);
+ bool ret;

- /*
- * This is only performed if the channel settings have
- * actually changed.
- *
- * To switch channels clear any pending DMA operations;
- * wait long enough for the RX fifo to drain, reset the
- * hardware at the new frequency, and then re-enable
- * the relevant bits of the h/w.
- */
- ath9k_hw_disable_interrupts(ah);
- stopped = ath_drain_all_txq(sc, false);
+ ieee80211_stop_queues(sc->hw);

- if (!ath_stoprecv(sc))
- stopped = false;
+ sc->hw_busy_count = 0;
+ del_timer_sync(&common->ani.timer);

- if (!ath9k_hw_check_alive(ah))
- stopped = false;
+ ath9k_hw_disable_interrupts(ah);

- /* XXX: do not flush receive queue here. We don't want
- * to flush data frames already in queue because of
- * changing channel. */
+ ret = ath_drain_all_txq(sc, retry_tx);

- if (!stopped || !(sc->sc_flags & SC_OP_OFFCHANNEL))
- fastcc = false;
+ if (!ath_stoprecv(sc))
+ ret = false;

- if (!(sc->sc_flags & SC_OP_OFFCHANNEL))
- caldata = &sc->caldata;
+ if (!flush) {
+ if (ah->caps.hw_caps & ATH9K_HW_CAP_EDMA)
+ ath_rx_tasklet(sc, 0, true);
+ ath_rx_tasklet(sc, 0, false);
+ } else {
+ ath_flushrecv(sc);
+ }

- ath_dbg(common, ATH_DBG_CONFIG,
- "(%u MHz) -> (%u MHz), conf_is_ht40: %d fastcc: %d\n",
- sc->sc_ah->curchan->channel,
- channel->center_freq, conf_is_ht40(conf),
- fastcc);
+ return ret;
+}

- r = ath9k_hw_reset(ah, hchan, caldata, fastcc);
- if (r) {
- ath_err(common,
- "Unable to reset channel (%u MHz), reset status %d\n",
- channel->center_freq, r);
- goto ps_restore;
- }
+static bool ath_complete_reset(struct ath_softc *sc, bool start)
+{
+ struct ath_hw *ah = sc->sc_ah;
+ struct ath_common *common = ath9k_hw_common(ah);

if (ath_startrecv(sc) != 0) {
ath_err(common, "Unable to restart recv logic\n");
- r = -EIO;
- goto ps_restore;
+ return false;
}

ath9k_cmn_update_txpow(ah, sc->curtxpow,
@@ -297,21 +270,95 @@ static int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
ath9k_hw_set_interrupts(ah, ah->imask);
ath9k_hw_enable_interrupts(ah);

- if (!(sc->sc_flags & (SC_OP_OFFCHANNEL))) {
+ if (!(sc->sc_flags & (SC_OP_OFFCHANNEL)) && start) {
if (sc->sc_flags & SC_OP_BEACONS)
ath_set_beacon(sc);
+
ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
ieee80211_queue_delayed_work(sc->hw, &sc->hw_pll_work, HZ/2);
if (!common->disable_ani)
ath_start_ani(common);
}

- ps_restore:
- ieee80211_wake_queues(hw);
+ ieee80211_wake_queues(sc->hw);

+ return true;
+}
+
+static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
+ bool retry_tx)
+{
+ struct ath_hw *ah = sc->sc_ah;
+ struct ath_common *common = ath9k_hw_common(ah);
+ struct ath9k_hw_cal_data *caldata = NULL;
+ bool fastcc = true;
+ bool flush = false;
+ int r;
+
+ __ath_cancel_work(sc);
+
+ spin_lock_bh(&sc->sc_pcu_lock);
+
+ sc->sc_flags &= ~SC_OP_HW_RESET;
+
+ if (!(sc->sc_flags & SC_OP_OFFCHANNEL)) {
+ fastcc = false;
+ caldata = &sc->caldata;
+ }
+
+ if (!hchan) {
+ fastcc = false;
+ flush = true;
+ hchan = ah->curchan;
+ }
+
+ if (fastcc && !ath9k_hw_check_alive(ah))
+ fastcc = false;
+
+ if (!ath_prepare_reset(sc, retry_tx, flush))
+ fastcc = false;
+
+ ath_dbg(common, ATH_DBG_CONFIG,
+ "Reset to %u MHz, HT40: %d fastcc: %d\n",
+ hchan->channel, !!(hchan->channelFlags & (CHANNEL_HT40MINUS |
+ CHANNEL_HT40PLUS)),
+ fastcc);
+
+ r = ath9k_hw_reset(ah, hchan, caldata, fastcc);
+ if (r) {
+ ath_err(common,
+ "Unable to reset channel, reset status %d\n", r);
+ goto out;
+ }
+
+ if (!ath_complete_reset(sc, true))
+ r = -EIO;
+
+out:
spin_unlock_bh(&sc->sc_pcu_lock);
+ return r;
+}
+
+
+/*
+ * Set/change channels. If the channel is really being changed, it's done
+ * by reseting the chip. To accomplish this we must first cleanup any pending
+ * DMA, then restart stuff.
+*/
+static int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
+ struct ath9k_channel *hchan)
+{
+ int r;
+
+ if (sc->sc_flags & SC_OP_INVALID)
+ return -EIO;
+
+ ath9k_ps_wakeup(sc);
+
+ r = ath_reset_internal(sc, hchan, false);

ath9k_ps_restore(sc);
+
return r;
}

@@ -824,28 +871,13 @@ static void ath_radio_enable(struct ath_softc *sc, struct ieee80211_hw *hw)
channel->center_freq, r);
}

- ath9k_cmn_update_txpow(ah, sc->curtxpow,
- sc->config.txpowlimit, &sc->curtxpow);
- if (ath_startrecv(sc) != 0) {
- ath_err(common, "Unable to restart recv logic\n");
- goto out;
- }
- if (sc->sc_flags & SC_OP_BEACONS)
- ath_set_beacon(sc); /* restart beacons */
-
- /* Re-Enable interrupts */
- ath9k_hw_set_interrupts(ah, ah->imask);
- ath9k_hw_enable_interrupts(ah);
+ ath_complete_reset(sc, true);

/* Enable LED */
ath9k_hw_cfg_output(ah, ah->led_pin,
AR_GPIO_OUTPUT_MUX_AS_OUTPUT);
ath9k_hw_set_gpio(ah, ah->led_pin, 0);

- ieee80211_wake_queues(hw);
- ieee80211_queue_delayed_work(hw, &sc->hw_pll_work, HZ/2);
-
-out:
spin_unlock_bh(&sc->sc_pcu_lock);

ath9k_ps_restore(sc);
@@ -858,11 +890,10 @@ void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw)
int r;

ath9k_ps_wakeup(sc);
- cancel_delayed_work_sync(&sc->hw_pll_work);

- spin_lock_bh(&sc->sc_pcu_lock);
+ ath_cancel_work(sc);

- ieee80211_stop_queues(hw);
+ spin_lock_bh(&sc->sc_pcu_lock);

/*
* Keep the LED on when the radio is disabled
@@ -873,13 +904,7 @@ void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw)
ath9k_hw_cfg_gpio_input(ah, ah->led_pin);
}

- /* Disable interrupts */
- ath9k_hw_disable_interrupts(ah);
-
- ath_drain_all_txq(sc, false); /* clear pending tx frames */
-
- ath_stoprecv(sc); /* turn off frame recv */
- ath_flushrecv(sc); /* flush recv queue */
+ ath_prepare_reset(sc, false, true);

if (!ah->curchan)
ah->curchan = ath9k_cmn_get_curchannel(hw, ah);
@@ -901,49 +926,11 @@ void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw)

static int ath_reset(struct ath_softc *sc, bool retry_tx)
{
- struct ath_hw *ah = sc->sc_ah;
- struct ath_common *common = ath9k_hw_common(ah);
- struct ieee80211_hw *hw = sc->hw;
int r;

- sc->sc_flags &= ~SC_OP_HW_RESET;
- sc->hw_busy_count = 0;
-
- /* Stop ANI */
-
- del_timer_sync(&common->ani.timer);
-
ath9k_ps_wakeup(sc);

- ieee80211_stop_queues(hw);
-
- ath9k_hw_disable_interrupts(ah);
- ath_drain_all_txq(sc, retry_tx);
-
- ath_stoprecv(sc);
- ath_flushrecv(sc);
-
- r = ath9k_hw_reset(ah, sc->sc_ah->curchan, ah->caldata, false);
- if (r)
- ath_err(common,
- "Unable to reset hardware; reset status %d\n", r);
-
- if (ath_startrecv(sc) != 0)
- ath_err(common, "Unable to start recv logic\n");
-
- /*
- * We may be doing a reset in response to a request
- * that changes the channel so update any state that
- * might change as a result.
- */
- ath9k_cmn_update_txpow(ah, sc->curtxpow,
- sc->config.txpowlimit, &sc->curtxpow);
-
- if ((sc->sc_flags & SC_OP_BEACONS) || !(sc->sc_flags & (SC_OP_OFFCHANNEL)))
- ath_set_beacon(sc); /* restart beacons */
-
- ath9k_hw_set_interrupts(ah, ah->imask);
- ath9k_hw_enable_interrupts(ah);
+ r = ath_reset_internal(sc, NULL, retry_tx);

if (retry_tx) {
int i;
@@ -956,12 +943,6 @@ static int ath_reset(struct ath_softc *sc, bool retry_tx)
}
}

- ieee80211_wake_queues(hw);
-
- /* Start ANI */
- if (!common->disable_ani)
- ath_start_ani(common);
-
ath9k_ps_restore(sc);

return r;
@@ -971,9 +952,7 @@ void ath_reset_work(struct work_struct *work)
{
struct ath_softc *sc = container_of(work, struct ath_softc, hw_check_work);

- spin_lock_bh(&sc->sc_pcu_lock);
ath_reset(sc, true);
- spin_unlock_bh(&sc->sc_pcu_lock);
}

void ath_hw_check(struct work_struct *work)
@@ -994,11 +973,8 @@ void ath_hw_check(struct work_struct *work)
ath_dbg(common, ATH_DBG_RESET, "Possible baseband hang, "
"busy=%d (try %d)\n", busy, sc->hw_busy_count + 1);
if (busy >= 99) {
- if (++sc->hw_busy_count >= 3) {
- spin_lock_bh(&sc->sc_pcu_lock);
- ath_reset(sc, true);
- spin_unlock_bh(&sc->sc_pcu_lock);
- }
+ if (++sc->hw_busy_count >= 3)
+ ieee80211_queue_work(sc->hw, &sc->hw_reset_work);

} else if (busy >= 0)
sc->hw_busy_count = 0;
@@ -1018,9 +994,7 @@ static void ath_hw_pll_rx_hang_check(struct ath_softc *sc, u32 pll_sqsum)
/* Rx is hung for more than 500ms. Reset it */
ath_dbg(common, ATH_DBG_RESET,
"Possible RX hang, resetting");
- spin_lock_bh(&sc->sc_pcu_lock);
- ath_reset(sc, true);
- spin_unlock_bh(&sc->sc_pcu_lock);
+ ieee80211_queue_work(sc->hw, &sc->hw_reset_work);
count = 0;
}
} else
@@ -1091,28 +1065,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
goto mutex_unlock;
}

- /*
- * This is needed only to setup initial state
- * but it's best done after a reset.
- */
- ath9k_cmn_update_txpow(ah, sc->curtxpow,
- sc->config.txpowlimit, &sc->curtxpow);
-
- /*
- * Setup the hardware after reset:
- * The receive engine is set going.
- * Frame transmit is handled entirely
- * in the frame output path; there's nothing to do
- * here except setup the interrupt mask.
- */
- if (ath_startrecv(sc) != 0) {
- ath_err(common, "Unable to start recv logic\n");
- r = -EIO;
- spin_unlock_bh(&sc->sc_pcu_lock);
- goto mutex_unlock;
- }
- spin_unlock_bh(&sc->sc_pcu_lock);
-
/* Setup our intr mask. */
ah->imask = ATH9K_INT_TX | ATH9K_INT_RXEOL |
ATH9K_INT_RXORN | ATH9K_INT_FATAL |
@@ -1135,12 +1087,14 @@ static int ath9k_start(struct ieee80211_hw *hw)

/* Disable BMISS interrupt when we're not associated */
ah->imask &= ~(ATH9K_INT_SWBA | ATH9K_INT_BMISS);
- ath9k_hw_set_interrupts(ah, ah->imask);
- ath9k_hw_enable_interrupts(ah);

- ieee80211_wake_queues(hw);
+ if (!ath_complete_reset(sc, false)) {
+ r = -EIO;
+ spin_unlock_bh(&sc->sc_pcu_lock);
+ goto mutex_unlock;
+ }

- ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
+ spin_unlock_bh(&sc->sc_pcu_lock);

if ((ah->btcoex_hw.scheme != ATH_BTCOEX_CFG_NONE) &&
!ah->btcoex_hw.enabled) {
@@ -1233,11 +1187,7 @@ static void ath9k_stop(struct ieee80211_hw *hw)

mutex_lock(&sc->mutex);

- cancel_delayed_work_sync(&sc->tx_complete_work);
- cancel_delayed_work_sync(&sc->hw_pll_work);
- cancel_work_sync(&sc->paprd_work);
- cancel_work_sync(&sc->hw_check_work);
- cancel_work_sync(&sc->hw_reset_work);
+ ath_cancel_work(sc);

if (sc->sc_flags & SC_OP_INVALID) {
ath_dbg(common, ATH_DBG_ANY, "Device not present\n");
@@ -2352,9 +2302,11 @@ static void ath9k_flush(struct ieee80211_hw *hw, bool drop)
ath9k_ps_wakeup(sc);
spin_lock_bh(&sc->sc_pcu_lock);
drain_txq = ath_drain_all_txq(sc, false);
+ spin_unlock_bh(&sc->sc_pcu_lock);
+
if (!drain_txq)
ath_reset(sc, false);
- spin_unlock_bh(&sc->sc_pcu_lock);
+
ath9k_ps_restore(sc);
ieee80211_wake_queues(hw);

--
1.7.3.2


2011-08-30 16:58:39

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v9 2/4] ath9k: always call ath_reset from workqueue context

On 2011-08-30 6:54 PM, Rajkumar Manoharan wrote:
> On Tue, Aug 30, 2011 at 05:52:02PM +0200, Felix Fietkau wrote:
>> On 2011-08-30 4:20 PM, Rajkumar Manoharan wrote:
>> >On Tue, Aug 30, 2011 at 10:30:45AM +0200, Felix Fietkau wrote:
>> >> This makes it much easier to add further rework to avoid race conditions
>> >> between reset and other work items.
>> >> Move other functions to make ath_reset static.
>> >>
>> >> Signed-off-by: Felix Fietkau<[email protected]>
>> >> ---
>> >> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>> >> index 49b93c2..2b2a975 100644
>> >> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>> >> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>> >> @@ -581,8 +581,10 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
>> >>
>> >> rcu_read_unlock();
>> >>
>> >> - if (needreset)
>> >> - ath_reset(sc, false);
>> >> + if (needreset) {
>> >> + sc->sc_flags |= SC_OP_HW_RESET;
>> >> + ieee80211_queue_work(sc->hw,&sc->hw_reset_work);
>> >> + }
>> >> }
>> >>
>> >Still ath_txq_schedule can be scheduled. Return a error code from
>> >ath_tx_complete_aggr to abort tx process intead of SC_OP_HW_RESET.
>> >That looks much more generic and simple.
>> I added this to make sure that there won't be any subsequent
>> attempts at processing the tx queue until the reset (e.g. if there
>> was a tx status IRQ in the mean time).
>>
> If so, it would be safer not to re-enable interrupts at the end of ath9k_tasklet
> if SC_OP_HW_RESET is set.
I think tx is the primary issue here, so I only added the check to the
tx processing functions.

- Felix


2011-08-30 08:30:59

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v9 2/4] ath9k: always call ath_reset from workqueue context

This makes it much easier to add further rework to avoid race conditions
between reset and other work items.
Move other functions to make ath_reset static.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 4 +-
drivers/net/wireless/ath/ath9k/beacon.c | 4 +-
drivers/net/wireless/ath/ath9k/init.c | 1 +
drivers/net/wireless/ath/ath9k/main.c | 155 ++++++++++++++++--------------
drivers/net/wireless/ath/ath9k/xmit.c | 16 ++-
5 files changed, 99 insertions(+), 81 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 5d9a9aa..bfc90e4 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -425,6 +425,7 @@ void ath9k_set_beaconing_status(struct ath_softc *sc, bool status);

#define ATH_PAPRD_TIMEOUT 100 /* msecs */

+void ath_reset_work(struct work_struct *work);
void ath_hw_check(struct work_struct *work);
void ath_hw_pll_work(struct work_struct *work);
void ath_paprd_calibrate(struct work_struct *work);
@@ -555,6 +556,7 @@ struct ath_ant_comb {
#define SC_OP_RXFLUSH BIT(7)
#define SC_OP_LED_ASSOCIATED BIT(8)
#define SC_OP_LED_ON BIT(9)
+#define SC_OP_HW_RESET BIT(10)
#define SC_OP_TSF_RESET BIT(11)
#define SC_OP_BT_PRIORITY_DETECTED BIT(12)
#define SC_OP_BT_SCAN BIT(13)
@@ -604,6 +606,7 @@ struct ath_softc {
struct mutex mutex;
struct work_struct paprd_work;
struct work_struct hw_check_work;
+ struct work_struct hw_reset_work;
struct completion paprd_complete;

unsigned int hw_busy_count;
@@ -650,7 +653,6 @@ struct ath_softc {
};

void ath9k_tasklet(unsigned long data);
-int ath_reset(struct ath_softc *sc, bool retry_tx);
int ath_cabq_update(struct ath_softc *);

static inline void ath_read_cachesize(struct ath_common *common, int *csz)
diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index 0c757c9..22e8e25 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -386,9 +386,7 @@ void ath_beacon_tasklet(unsigned long data)
ath_dbg(common, ATH_DBG_BSTUCK,
"beacon is officially stuck\n");
sc->sc_flags |= SC_OP_TSF_RESET;
- spin_lock(&sc->sc_pcu_lock);
- ath_reset(sc, true);
- spin_unlock(&sc->sc_pcu_lock);
+ ieee80211_queue_work(sc->hw, &sc->hw_reset_work);
}

return;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 31ef501..6ee3be9 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -776,6 +776,7 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc,
goto error_world;
}

+ INIT_WORK(&sc->hw_reset_work, ath_reset_work);
INIT_WORK(&sc->hw_check_work, ath_hw_check);
INIT_WORK(&sc->paprd_work, ath_paprd_calibrate);
INIT_DELAYED_WORK(&sc->hw_pll_work, ath_hw_pll_work);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 085ec20..4f74ba5 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -236,6 +236,7 @@ static int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
del_timer_sync(&common->ani.timer);
cancel_work_sync(&sc->paprd_work);
cancel_work_sync(&sc->hw_check_work);
+ cancel_work_sync(&sc->hw_reset_work);
cancel_delayed_work_sync(&sc->tx_complete_work);
cancel_delayed_work_sync(&sc->hw_pll_work);

@@ -595,74 +596,6 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta)
ath_tx_node_cleanup(sc, an);
}

-void ath_hw_check(struct work_struct *work)
-{
- struct ath_softc *sc = container_of(work, struct ath_softc, hw_check_work);
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
- unsigned long flags;
- int busy;
-
- ath9k_ps_wakeup(sc);
- if (ath9k_hw_check_alive(sc->sc_ah))
- goto out;
-
- spin_lock_irqsave(&common->cc_lock, flags);
- busy = ath_update_survey_stats(sc);
- spin_unlock_irqrestore(&common->cc_lock, flags);
-
- ath_dbg(common, ATH_DBG_RESET, "Possible baseband hang, "
- "busy=%d (try %d)\n", busy, sc->hw_busy_count + 1);
- if (busy >= 99) {
- if (++sc->hw_busy_count >= 3) {
- spin_lock_bh(&sc->sc_pcu_lock);
- ath_reset(sc, true);
- spin_unlock_bh(&sc->sc_pcu_lock);
- }
- } else if (busy >= 0)
- sc->hw_busy_count = 0;
-
-out:
- ath9k_ps_restore(sc);
-}
-
-static void ath_hw_pll_rx_hang_check(struct ath_softc *sc, u32 pll_sqsum)
-{
- static int count;
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
-
- if (pll_sqsum >= 0x40000) {
- count++;
- if (count == 3) {
- /* Rx is hung for more than 500ms. Reset it */
- ath_dbg(common, ATH_DBG_RESET,
- "Possible RX hang, resetting");
- spin_lock_bh(&sc->sc_pcu_lock);
- ath_reset(sc, true);
- spin_unlock_bh(&sc->sc_pcu_lock);
- count = 0;
- }
- } else
- count = 0;
-}
-
-void ath_hw_pll_work(struct work_struct *work)
-{
- struct ath_softc *sc = container_of(work, struct ath_softc,
- hw_pll_work.work);
- u32 pll_sqsum;
-
- if (AR_SREV_9485(sc->sc_ah)) {
-
- ath9k_ps_wakeup(sc);
- pll_sqsum = ar9003_get_pll_sqsum_dvc(sc->sc_ah);
- ath9k_ps_restore(sc);
-
- ath_hw_pll_rx_hang_check(sc, pll_sqsum);
-
- ieee80211_queue_delayed_work(sc->hw, &sc->hw_pll_work, HZ/5);
- }
-}
-

void ath9k_tasklet(unsigned long data)
{
@@ -675,9 +608,7 @@ void ath9k_tasklet(unsigned long data)

if ((status & ATH9K_INT_FATAL) ||
(status & ATH9K_INT_BB_WATCHDOG)) {
- spin_lock(&sc->sc_pcu_lock);
- ath_reset(sc, true);
- spin_unlock(&sc->sc_pcu_lock);
+ ieee80211_queue_work(sc->hw, &sc->hw_reset_work);
return;
}

@@ -968,13 +899,14 @@ void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw)
ath9k_ps_restore(sc);
}

-int ath_reset(struct ath_softc *sc, bool retry_tx)
+static int ath_reset(struct ath_softc *sc, bool retry_tx)
{
struct ath_hw *ah = sc->sc_ah;
struct ath_common *common = ath9k_hw_common(ah);
struct ieee80211_hw *hw = sc->hw;
int r;

+ sc->sc_flags &= ~SC_OP_HW_RESET;
sc->hw_busy_count = 0;

/* Stop ANI */
@@ -1035,6 +967,84 @@ int ath_reset(struct ath_softc *sc, bool retry_tx)
return r;
}

+void ath_reset_work(struct work_struct *work)
+{
+ struct ath_softc *sc = container_of(work, struct ath_softc, hw_check_work);
+
+ spin_lock_bh(&sc->sc_pcu_lock);
+ ath_reset(sc, true);
+ spin_unlock_bh(&sc->sc_pcu_lock);
+}
+
+void ath_hw_check(struct work_struct *work)
+{
+ struct ath_softc *sc = container_of(work, struct ath_softc, hw_check_work);
+ struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ unsigned long flags;
+ int busy;
+
+ ath9k_ps_wakeup(sc);
+ if (ath9k_hw_check_alive(sc->sc_ah))
+ goto out;
+
+ spin_lock_irqsave(&common->cc_lock, flags);
+ busy = ath_update_survey_stats(sc);
+ spin_unlock_irqrestore(&common->cc_lock, flags);
+
+ ath_dbg(common, ATH_DBG_RESET, "Possible baseband hang, "
+ "busy=%d (try %d)\n", busy, sc->hw_busy_count + 1);
+ if (busy >= 99) {
+ if (++sc->hw_busy_count >= 3) {
+ spin_lock_bh(&sc->sc_pcu_lock);
+ ath_reset(sc, true);
+ spin_unlock_bh(&sc->sc_pcu_lock);
+ }
+
+ } else if (busy >= 0)
+ sc->hw_busy_count = 0;
+
+out:
+ ath9k_ps_restore(sc);
+}
+
+static void ath_hw_pll_rx_hang_check(struct ath_softc *sc, u32 pll_sqsum)
+{
+ static int count;
+ struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+
+ if (pll_sqsum >= 0x40000) {
+ count++;
+ if (count == 3) {
+ /* Rx is hung for more than 500ms. Reset it */
+ ath_dbg(common, ATH_DBG_RESET,
+ "Possible RX hang, resetting");
+ spin_lock_bh(&sc->sc_pcu_lock);
+ ath_reset(sc, true);
+ spin_unlock_bh(&sc->sc_pcu_lock);
+ count = 0;
+ }
+ } else
+ count = 0;
+}
+
+void ath_hw_pll_work(struct work_struct *work)
+{
+ struct ath_softc *sc = container_of(work, struct ath_softc,
+ hw_pll_work.work);
+ u32 pll_sqsum;
+
+ if (AR_SREV_9485(sc->sc_ah)) {
+
+ ath9k_ps_wakeup(sc);
+ pll_sqsum = ar9003_get_pll_sqsum_dvc(sc->sc_ah);
+ ath9k_ps_restore(sc);
+
+ ath_hw_pll_rx_hang_check(sc, pll_sqsum);
+
+ ieee80211_queue_delayed_work(sc->hw, &sc->hw_pll_work, HZ/5);
+ }
+}
+
/**********************/
/* mac80211 callbacks */
/**********************/
@@ -1227,6 +1237,7 @@ static void ath9k_stop(struct ieee80211_hw *hw)
cancel_delayed_work_sync(&sc->hw_pll_work);
cancel_work_sync(&sc->paprd_work);
cancel_work_sync(&sc->hw_check_work);
+ cancel_work_sync(&sc->hw_reset_work);

if (sc->sc_flags & SC_OP_INVALID) {
ath_dbg(common, ATH_DBG_ANY, "Device not present\n");
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 49b93c2..2b2a975 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -581,8 +581,10 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,

rcu_read_unlock();

- if (needreset)
- ath_reset(sc, false);
+ if (needreset) {
+ sc->sc_flags |= SC_OP_HW_RESET;
+ ieee80211_queue_work(sc->hw, &sc->hw_reset_work);
+ }
}

static bool ath_lookup_legacy(struct ath_buf *bf)
@@ -2147,6 +2149,9 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)

spin_lock_bh(&txq->axq_lock);
for (;;) {
+ if (sc->sc_flags & SC_OP_HW_RESET)
+ break;
+
if (list_empty(&txq->axq_q)) {
txq->axq_link = NULL;
if (sc->sc_flags & SC_OP_TXAGGR)
@@ -2234,9 +2239,7 @@ static void ath_tx_complete_poll_work(struct work_struct *work)
if (needreset) {
ath_dbg(ath9k_hw_common(sc->sc_ah), ATH_DBG_RESET,
"tx hung, resetting the chip\n");
- spin_lock_bh(&sc->sc_pcu_lock);
- ath_reset(sc, true);
- spin_unlock_bh(&sc->sc_pcu_lock);
+ ieee80211_queue_work(sc->hw, &sc->hw_reset_work);
}

ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work,
@@ -2269,6 +2272,9 @@ void ath_tx_edma_tasklet(struct ath_softc *sc)
int status;

for (;;) {
+ if (sc->sc_flags & SC_OP_HW_RESET)
+ break;
+
status = ath9k_hw_txprocdesc(ah, NULL, (void *)&ts);
if (status == -EINPROGRESS)
break;
--
1.7.3.2


2011-08-30 14:19:43

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH v9 2/4] ath9k: always call ath_reset from workqueue context

On Tue, Aug 30, 2011 at 10:30:45AM +0200, Felix Fietkau wrote:
> This makes it much easier to add further rework to avoid race conditions
> between reset and other work items.
> Move other functions to make ath_reset static.
>
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 49b93c2..2b2a975 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -581,8 +581,10 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
>
> rcu_read_unlock();
>
> - if (needreset)
> - ath_reset(sc, false);
> + if (needreset) {
> + sc->sc_flags |= SC_OP_HW_RESET;
> + ieee80211_queue_work(sc->hw, &sc->hw_reset_work);
> + }
> }
>
Still ath_txq_schedule can be scheduled. Return a error code from
ath_tx_complete_aggr to abort tx process intead of SC_OP_HW_RESET.
That looks much more generic and simple.

--
Rajkumar

2011-08-30 08:31:10

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v9 4/4] ath9k: implement .get_antenna and .set_antenna

On MIMO chips this can be used to enable/disable hardware chains, ensuring
that the MCS information is updated accordingly.
On non-MIMO chips with rx diversity (e.g. 9285), this configures the rx
input antenna.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 2 +
drivers/net/wireless/ath/ath9k/init.c | 32 +++++++++++---
drivers/net/wireless/ath/ath9k/main.c | 71 ++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath9k/recv.c | 2 +-
4 files changed, 99 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index bfc90e4..5330803 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -650,6 +650,7 @@ struct ath_softc {
struct ath_descdma txsdma;

struct ath_ant_comb ant_comb;
+ u8 ant_tx, ant_rx;
};

void ath9k_tasklet(unsigned long data);
@@ -670,6 +671,7 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc,
const struct ath_bus_ops *bus_ops);
void ath9k_deinit_device(struct ath_softc *sc);
void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw);
+void ath9k_reload_chainmask_settings(struct ath_softc *sc);

void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw);
bool ath9k_uses_beacons(int type);
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 6ee3be9..0eeffd1 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -652,9 +652,22 @@ static void ath9k_init_txpower_limits(struct ath_softc *sc)
ah->curchan = curchan;
}

+void ath9k_reload_chainmask_settings(struct ath_softc *sc)
+{
+ if (!(sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT))
+ return;
+
+ if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_2GHZ)
+ setup_ht_cap(sc, &sc->sbands[IEEE80211_BAND_2GHZ].ht_cap);
+ if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_5GHZ)
+ setup_ht_cap(sc, &sc->sbands[IEEE80211_BAND_5GHZ].ht_cap);
+}
+
+
void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
{
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ath_hw *ah = sc->sc_ah;
+ struct ath_common *common = ath9k_hw_common(ah);

hw->flags = IEEE80211_HW_RX_INCLUDES_FCS |
IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
@@ -692,6 +705,16 @@ void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
hw->sta_data_size = sizeof(struct ath_node);
hw->vif_data_size = sizeof(struct ath_vif);

+ hw->wiphy->available_antennas_rx = BIT(ah->caps.max_rxchains) - 1;
+ hw->wiphy->available_antennas_tx = BIT(ah->caps.max_txchains) - 1;
+
+ /* single chain devices with rx diversity */
+ if (ah->caps.hw_caps & ATH9K_HW_CAP_ANT_DIV_COMB)
+ hw->wiphy->available_antennas_rx = BIT(0) | BIT(1);
+
+ sc->ant_rx = hw->wiphy->available_antennas_rx;
+ sc->ant_tx = hw->wiphy->available_antennas_tx;
+
#ifdef CONFIG_ATH9K_RATE_CONTROL
hw->rate_control_algorithm = "ath9k_rate_control";
#endif
@@ -703,12 +726,7 @@ void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
hw->wiphy->bands[IEEE80211_BAND_5GHZ] =
&sc->sbands[IEEE80211_BAND_5GHZ];

- if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) {
- if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_2GHZ)
- setup_ht_cap(sc, &sc->sbands[IEEE80211_BAND_2GHZ].ht_cap);
- if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_5GHZ)
- setup_ht_cap(sc, &sc->sbands[IEEE80211_BAND_5GHZ].ht_cap);
- }
+ ath9k_reload_chainmask_settings(sc);

SET_IEEE80211_PERM_ADDR(hw, common->macaddr);
}
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 944c50a..8a01720 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -280,6 +280,22 @@ static bool ath_complete_reset(struct ath_softc *sc, bool start)
ath_start_ani(common);
}

+ if (ath9k_hw_ops(ah)->antdiv_comb_conf_get && sc->ant_rx != 3) {
+ struct ath_hw_antcomb_conf div_ant_conf;
+ u8 lna_conf;
+
+ ath9k_hw_antdiv_comb_conf_get(ah, &div_ant_conf);
+
+ if (sc->ant_rx == 1)
+ lna_conf = ATH_ANT_DIV_COMB_LNA1;
+ else
+ lna_conf = ATH_ANT_DIV_COMB_LNA2;
+ div_ant_conf.main_lna_conf = lna_conf;
+ div_ant_conf.alt_lna_conf = lna_conf;
+
+ ath9k_hw_antdiv_comb_conf_set(ah, &div_ant_conf);
+ }
+
ieee80211_wake_queues(sc->hw);

return true;
@@ -2383,6 +2399,59 @@ static int ath9k_get_stats(struct ieee80211_hw *hw,
return 0;
}

+static u32 fill_chainmask(u32 cap, u32 new)
+{
+ u32 filled = 0;
+ int i;
+
+ for (i = 0; cap && new; i++, cap >>= 1) {
+ if (!(cap & BIT(0)))
+ continue;
+
+ if (new & BIT(0))
+ filled |= BIT(i);
+
+ new >>= 1;
+ }
+
+ return filled;
+}
+
+static int ath9k_set_antenna(struct ieee80211_hw *hw, u32 tx_ant, u32 rx_ant)
+{
+ struct ath_softc *sc = hw->priv;
+ struct ath_hw *ah = sc->sc_ah;
+
+ if (!rx_ant || !tx_ant)
+ return -EINVAL;
+
+ sc->ant_rx = rx_ant;
+ sc->ant_tx = tx_ant;
+
+ if (ah->caps.rx_chainmask == 1)
+ return 0;
+
+ /* AR9100 runs into calibration issues if not all rx chains are enabled */
+ if (AR_SREV_9100(ah))
+ ah->rxchainmask = 0x7;
+ else
+ ah->rxchainmask = fill_chainmask(ah->caps.rx_chainmask, rx_ant);
+
+ ah->txchainmask = fill_chainmask(ah->caps.tx_chainmask, tx_ant);
+ ath9k_reload_chainmask_settings(sc);
+
+ return 0;
+}
+
+static int ath9k_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant)
+{
+ struct ath_softc *sc = hw->priv;
+
+ *tx_ant = sc->ant_tx;
+ *rx_ant = sc->ant_rx;
+ return 0;
+}
+
struct ieee80211_ops ath9k_ops = {
.tx = ath9k_tx,
.start = ath9k_start,
@@ -2409,4 +2478,6 @@ struct ieee80211_ops ath9k_ops = {
.tx_frames_pending = ath9k_tx_frames_pending,
.tx_last_beacon = ath9k_tx_last_beacon,
.get_stats = ath9k_get_stats,
+ .set_antenna = ath9k_set_antenna,
+ .get_antenna = ath9k_get_antenna,
};
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index ad5f9bd..7e1265c 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1956,7 +1956,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
ath_rx_ps(sc, skb);
spin_unlock_irqrestore(&sc->sc_pm_lock, flags);

- if (ah->caps.hw_caps & ATH9K_HW_CAP_ANT_DIV_COMB)
+ if ((ah->caps.hw_caps & ATH9K_HW_CAP_ANT_DIV_COMB) && sc->ant_rx == 3)
ath_ant_comb_scan(sc, &rs);

ieee80211_rx(hw, skb);
--
1.7.3.2


2011-08-30 16:53:36

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH v9 2/4] ath9k: always call ath_reset from workqueue context

On Tue, Aug 30, 2011 at 05:52:02PM +0200, Felix Fietkau wrote:
> On 2011-08-30 4:20 PM, Rajkumar Manoharan wrote:
> >On Tue, Aug 30, 2011 at 10:30:45AM +0200, Felix Fietkau wrote:
> >> This makes it much easier to add further rework to avoid race conditions
> >> between reset and other work items.
> >> Move other functions to make ath_reset static.
> >>
> >> Signed-off-by: Felix Fietkau<[email protected]>
> >> ---
> >> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> >> index 49b93c2..2b2a975 100644
> >> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> >> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> >> @@ -581,8 +581,10 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
> >>
> >> rcu_read_unlock();
> >>
> >> - if (needreset)
> >> - ath_reset(sc, false);
> >> + if (needreset) {
> >> + sc->sc_flags |= SC_OP_HW_RESET;
> >> + ieee80211_queue_work(sc->hw,&sc->hw_reset_work);
> >> + }
> >> }
> >>
> >Still ath_txq_schedule can be scheduled. Return a error code from
> >ath_tx_complete_aggr to abort tx process intead of SC_OP_HW_RESET.
> >That looks much more generic and simple.
> I added this to make sure that there won't be any subsequent
> attempts at processing the tx queue until the reset (e.g. if there
> was a tx status IRQ in the mean time).
>
If so, it would be safer not to re-enable interrupts at the end of ath9k_tasklet
if SC_OP_HW_RESET is set.

--
Rajkumar