2011-08-29 18:40:56

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v7 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-29 18:40:56

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v7 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 | 299 ++++++++++++++-------------------
1 files changed, 126 insertions(+), 173 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 76fcd4f..92158d9 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -212,83 +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_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,
@@ -296,21 +270,93 @@ 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);
+
+ 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 0;
+}
+
+
+/*
+ * 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;
}

@@ -823,28 +869,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);
@@ -857,11 +888,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
@@ -872,13 +902,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);
@@ -900,48 +924,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->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;
@@ -954,12 +941,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;
@@ -969,9 +950,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)
@@ -992,11 +971,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;
@@ -1016,9 +992,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
@@ -1089,28 +1063,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 |
@@ -1133,12 +1085,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) {
@@ -1231,10 +1185,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);
+ ath_cancel_work(sc);

if (sc->sc_flags & SC_OP_INVALID) {
ath_dbg(common, ATH_DBG_ANY, "Device not present\n");
@@ -2349,9 +2300,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 06:09:29

by Felix Fietkau

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

On 2011-08-30 7:57 AM, Mohammed Shafi wrote:
> On Tue, Aug 30, 2011 at 12:10 AM, Felix Fietkau<[email protected]> 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]>
>> ---
>> drivers/net/wireless/ath/ath9k/ath9k.h | 3 +-
>> drivers/net/wireless/ath/ath9k/beacon.c | 4 +-
>> drivers/net/wireless/ath/ath9k/init.c | 1 +
>> drivers/net/wireless/ath/ath9k/main.c | 152 ++++++++++++++++---------------
>> drivers/net/wireless/ath/ath9k/xmit.c | 6 +-
>> 5 files changed, 86 insertions(+), 80 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>> index 49b93c2..f60706b 100644
>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>> @@ -582,7 +582,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
>> rcu_read_unlock();
>>
>> if (needreset)
>> - ath_reset(sc, false);
>> + ieee80211_queue_work(sc->hw,&sc->hw_reset_work);
>
> Hi Felix,
>
> retry_tx is set to 'false' in ath_reset here, but ath_reset_work
> always has it true. had i missed some thing?
Since the reset work is deferred now, it can be allowed to retry tx now
instead of dropping all packets in the queue.

- Felix

2011-08-30 06:51:27

by Mohammed Shafi

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

On Tue, Aug 30, 2011 at 12:14 PM, Felix Fietkau <[email protected]> wrote:
> On 2011-08-30 8:20 AM, Mohammed Shafi wrote:
>>
>> On Tue, Aug 30, 2011 at 11:39 AM, Felix Fietkau<[email protected]> ?wrote:
>>>
>>> ?On 2011-08-30 7:57 AM, Mohammed Shafi wrote:
>>>>
>>>> ?On Tue, Aug 30, 2011 at 12:10 AM, Felix Fietkau<[email protected]>
>>>> ?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]>
>>>>> ? ---
>>>>> ? ?drivers/net/wireless/ath/ath9k/ath9k.h ?| ? ?3 +-
>>>>> ? ?drivers/net/wireless/ath/ath9k/beacon.c | ? ?4 +-
>>>>> ? ?drivers/net/wireless/ath/ath9k/init.c ? | ? ?1 +
>>>>> ? ?drivers/net/wireless/ath/ath9k/main.c ? | ?152
>>>>> ?++++++++++++++++---------------
>>>>> ? ?drivers/net/wireless/ath/ath9k/xmit.c ? | ? ?6 +-
>>>>> ? ?5 files changed, 86 insertions(+), 80 deletions(-)
>>>>>
>>>>> ? diff --git a/drivers/net/wireless/ath/ath9k/xmit.c
>>>>> ?b/drivers/net/wireless/ath/ath9k/xmit.c
>>>>> ? index 49b93c2..f60706b 100644
>>>>> ? --- a/drivers/net/wireless/ath/ath9k/xmit.c
>>>>> ? +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>>>>> ? @@ -582,7 +582,7 @@ static void ath_tx_complete_aggr(struct ath_softc
>>>>> ?*sc, struct ath_txq *txq,
>>>>> ? ? ? ? ?rcu_read_unlock();
>>>>>
>>>>> ? ? ? ? ?if (needreset)
>>>>> ? - ? ? ? ? ? ? ? ath_reset(sc, false);
>>>>> ? + ? ? ? ? ? ? ? ieee80211_queue_work(sc->hw,&sc->hw_reset_work);
>>>>
>>>> ?Hi Felix,
>>>>
>>>> ?retry_tx is set to 'false' in ath_reset here, but ath_reset_work
>>>> ?always has it true. had i missed some thing?
>>>
>>> ?Since the reset work is deferred now, it can be allowed to retry tx now
>>> ?instead of dropping all packets in the queue.
>>
>> oh ok, in this path we need reset when the chip becomes deaf/mute(as
>> mentioned ?in the comment),
>> so i am not sure retrying tx will help rather than doing immediate
>> reset of the chip.
>> thanks.
>
> Well, tx will be retried only after the full reset, so it should be fine.
> I'll make sure the next version will bail out of the tx completion tasklet
> when a reset is necessary, so that the reset can run right away.

oh ok, i did not know it, i thought the other way around. thanks.

>
> - Felix
>



--
shafi

2011-08-30 07:21:28

by Rajkumar Manoharan

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

On Mon, Aug 29, 2011 at 08:40:44PM +0200, Felix Fietkau wrote:
> +
> + 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 0;
> +}
Return proper error code.

--
Rajkumar

2011-08-30 05:57:37

by Mohammed Shafi

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

On Tue, Aug 30, 2011 at 12:10 AM, Felix Fietkau <[email protected]> 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]>
> ---
> ?drivers/net/wireless/ath/ath9k/ath9k.h ?| ? ?3 +-
> ?drivers/net/wireless/ath/ath9k/beacon.c | ? ?4 +-
> ?drivers/net/wireless/ath/ath9k/init.c ? | ? ?1 +
> ?drivers/net/wireless/ath/ath9k/main.c ? | ?152 ++++++++++++++++---------------
> ?drivers/net/wireless/ath/ath9k/xmit.c ? | ? ?6 +-
> ?5 files changed, 86 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 5d9a9aa..b2992d4 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);
> @@ -604,6 +605,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 +652,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..76fcd4f 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -595,74 +595,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 +607,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,7 +898,7 @@ 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);
> @@ -1035,6 +965,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 */
> ?/**********************/
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 49b93c2..f60706b 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -582,7 +582,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
> ? ? ? ?rcu_read_unlock();
>
> ? ? ? ?if (needreset)
> - ? ? ? ? ? ? ? ath_reset(sc, false);
> + ? ? ? ? ? ? ? ieee80211_queue_work(sc->hw, &sc->hw_reset_work);

Hi Felix,

retry_tx is set to 'false' in ath_reset here, but ath_reset_work
always has it true. had i missed some thing?

> ?}
>
> ?static bool ath_lookup_legacy(struct ath_buf *bf)
> @@ -2234,9 +2234,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,
> --
> 1.7.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



--
shafi

2011-08-29 18:40:57

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v7 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 | 3 +-
drivers/net/wireless/ath/ath9k/beacon.c | 4 +-
drivers/net/wireless/ath/ath9k/init.c | 1 +
drivers/net/wireless/ath/ath9k/main.c | 152 ++++++++++++++++---------------
drivers/net/wireless/ath/ath9k/xmit.c | 6 +-
5 files changed, 86 insertions(+), 80 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 5d9a9aa..b2992d4 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);
@@ -604,6 +605,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 +652,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..76fcd4f 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -595,74 +595,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 +607,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,7 +898,7 @@ 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);
@@ -1035,6 +965,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 */
/**********************/
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 49b93c2..f60706b 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -582,7 +582,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
rcu_read_unlock();

if (needreset)
- ath_reset(sc, false);
+ ieee80211_queue_work(sc->hw, &sc->hw_reset_work);
}

static bool ath_lookup_legacy(struct ath_buf *bf)
@@ -2234,9 +2234,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,
--
1.7.3.2


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

On Mon, Aug 29, 2011 at 08:40:43PM +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.

Can you please explain where you'll run into race during
ath_reset()?. It's always called with pcu_lock held.

> @@ -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);

We'll expect a reset happening as soon possible here. Handling this
in a work item may not be appropriate.

> void ath9k_tasklet(unsigned long data)
> {
> @@ -675,9 +607,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);

Above comment applies here as well.

Vasanth

2011-08-30 05:40:19

by Felix Fietkau

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

On 2011-08-30 7:26 AM, Vasanthakumar Thiagarajan wrote:
> On Mon, Aug 29, 2011 at 08:40:43PM +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.
>
> Can you please explain where you'll run into race during
> ath_reset()?. It's always called with pcu_lock held.
I didn't actually observe any race, but I'm not sure if all work items
can properly deal with being interrupted by a full reset, so I'm trying
to make it more safe here.

>> @@ -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);
>
> We'll expect a reset happening as soon possible here. Handling this
> in a work item may not be appropriate.
We've already waited for multiples of 100ms. 1-2ms extra won't hurt now.

>> void ath9k_tasklet(unsigned long data)
>> {
>> @@ -675,9 +607,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);
>
> Above comment applies here as well.
I don't think the additional delay will be significant enough to matter
here. What matters to me more is to be able to reliably kill all the
currently running work, which can only be done if ath_reset gets called
from process context.

- Felix

2011-08-30 06:44:05

by Felix Fietkau

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

On 2011-08-30 8:20 AM, Mohammed Shafi wrote:
> On Tue, Aug 30, 2011 at 11:39 AM, Felix Fietkau<[email protected]> wrote:
>> On 2011-08-30 7:57 AM, Mohammed Shafi wrote:
>>>
>>> On Tue, Aug 30, 2011 at 12:10 AM, Felix Fietkau<[email protected]> 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]>
>>>> ---
>>>> drivers/net/wireless/ath/ath9k/ath9k.h | 3 +-
>>>> drivers/net/wireless/ath/ath9k/beacon.c | 4 +-
>>>> drivers/net/wireless/ath/ath9k/init.c | 1 +
>>>> drivers/net/wireless/ath/ath9k/main.c | 152
>>>> ++++++++++++++++---------------
>>>> drivers/net/wireless/ath/ath9k/xmit.c | 6 +-
>>>> 5 files changed, 86 insertions(+), 80 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c
>>>> b/drivers/net/wireless/ath/ath9k/xmit.c
>>>> index 49b93c2..f60706b 100644
>>>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>>>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>>>> @@ -582,7 +582,7 @@ static void ath_tx_complete_aggr(struct ath_softc
>>>> *sc, struct ath_txq *txq,
>>>> rcu_read_unlock();
>>>>
>>>> if (needreset)
>>>> - ath_reset(sc, false);
>>>> + ieee80211_queue_work(sc->hw,&sc->hw_reset_work);
>>>
>>> Hi Felix,
>>>
>>> retry_tx is set to 'false' in ath_reset here, but ath_reset_work
>>> always has it true. had i missed some thing?
>>
>> Since the reset work is deferred now, it can be allowed to retry tx now
>> instead of dropping all packets in the queue.
>
> oh ok, in this path we need reset when the chip becomes deaf/mute(as
> mentioned in the comment),
> so i am not sure retrying tx will help rather than doing immediate
> reset of the chip.
> thanks.
Well, tx will be retried only after the full reset, so it should be
fine. I'll make sure the next version will bail out of the tx completion
tasklet when a reset is necessary, so that the reset can run right away.

- Felix

2011-08-29 18:40:58

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v7 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 b2992d4..0fb4a26 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -649,6 +649,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);
@@ -669,6 +670,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 92158d9..e7ec486 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;
@@ -2381,6 +2397,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,
@@ -2407,4 +2476,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 06:20:54

by Mohammed Shafi

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

On Tue, Aug 30, 2011 at 11:39 AM, Felix Fietkau <[email protected]> wrote:
> On 2011-08-30 7:57 AM, Mohammed Shafi wrote:
>>
>> On Tue, Aug 30, 2011 at 12:10 AM, Felix Fietkau<[email protected]> ?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]>
>>> ?---
>>> ? drivers/net/wireless/ath/ath9k/ath9k.h ?| ? ?3 +-
>>> ? drivers/net/wireless/ath/ath9k/beacon.c | ? ?4 +-
>>> ? drivers/net/wireless/ath/ath9k/init.c ? | ? ?1 +
>>> ? drivers/net/wireless/ath/ath9k/main.c ? | ?152
>>> ++++++++++++++++---------------
>>> ? drivers/net/wireless/ath/ath9k/xmit.c ? | ? ?6 +-
>>> ? 5 files changed, 86 insertions(+), 80 deletions(-)
>>>
>>> ?diff --git a/drivers/net/wireless/ath/ath9k/xmit.c
>>> b/drivers/net/wireless/ath/ath9k/xmit.c
>>> ?index 49b93c2..f60706b 100644
>>> ?--- a/drivers/net/wireless/ath/ath9k/xmit.c
>>> ?+++ b/drivers/net/wireless/ath/ath9k/xmit.c
>>> ?@@ -582,7 +582,7 @@ static void ath_tx_complete_aggr(struct ath_softc
>>> *sc, struct ath_txq *txq,
>>> ? ? ? ? rcu_read_unlock();
>>>
>>> ? ? ? ? if (needreset)
>>> ?- ? ? ? ? ? ? ? ath_reset(sc, false);
>>> ?+ ? ? ? ? ? ? ? ieee80211_queue_work(sc->hw,&sc->hw_reset_work);
>>
>> Hi Felix,
>>
>> retry_tx is set to 'false' in ath_reset here, but ath_reset_work
>> always has it true. had i missed some thing?
>
> Since the reset work is deferred now, it can be allowed to retry tx now
> instead of dropping all packets in the queue.

oh ok, in this path we need reset when the chip becomes deaf/mute(as
mentioned in the comment),
so i am not sure retrying tx will help rather than doing immediate
reset of the chip.
thanks.

>
> - Felix
>



--
shafi

2011-08-30 06:25:51

by Rajkumar Manoharan

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

On Mon, Aug 29, 2011 at 08:40:43PM +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]>
> + 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);

work never be cancelled at ath9k_stop.
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 085ec20..76fcd4f 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -595,74 +595,6 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta)
> ath_tx_node_cleanup(sc, an);
> }
>

> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 49b93c2..f60706b 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -582,7 +582,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
> rcu_read_unlock();
>
> if (needreset)
> - ath_reset(sc, false);
> + ieee80211_queue_work(sc->hw, &sc->hw_reset_work);
> }
Once you decide to do reset, do not continue with tx process.
>
> static bool ath_lookup_legacy(struct ath_buf *bf)
> @@ -2234,9 +2234,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);
> }
This is already in work thread. why don't you use ath_reset here like
ath_hw_pll_rx_hang_check.

--
Rajkumar

2011-08-30 06:42:24

by Felix Fietkau

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

On 2011-08-30 8:26 AM, Rajkumar Manoharan wrote:
> On Mon, Aug 29, 2011 at 08:40:43PM +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]>
>> + 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);
>
> work never be cancelled at ath9k_stop.
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index 085ec20..76fcd4f 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -595,74 +595,6 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta)
>> ath_tx_node_cleanup(sc, an);
>> }
>>
>
>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>> index 49b93c2..f60706b 100644
>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>> @@ -582,7 +582,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
>> rcu_read_unlock();
>>
>> if (needreset)
>> - ath_reset(sc, false);
>> + ieee80211_queue_work(sc->hw,&sc->hw_reset_work);
>> }
> Once you decide to do reset, do not continue with tx process.
I'll look into it.

>> static bool ath_lookup_legacy(struct ath_buf *bf)
>> @@ -2234,9 +2234,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);
>> }
> This is already in work thread. why don't you use ath_reset here like
> ath_hw_pll_rx_hang_check.
I also changed ath_hw_pll_rx_hang_check to queue the reset work, because
ath_reset cancels the work items before continuing with the hardware reset.

- Felix