[PATCH 1/4] ath9k: always issue a full hw reset after waking up from full-sleep mode

[PATCH 1/4] ath9k: always issue a full hw reset after waking up from full-sleep mode

[Felix Fietkau]

After waking up from full sleep, registers are accessible, but rx/tx
typically fails. A fast channel change will not recover from this, so
ensure that a full-sleep -> wake transition is always followed by a full
reset.

The reason why this hasn't created any serious problems yet is that it's
hidden by the (wrong) behavior of enabling/disabling the radio when the
wiphy idle state changes.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/main.c |   12 ++++++++++--
 drivers/net/wireless/ath/ath9k/xmit.c |    2 +-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index e43c41c..734e854 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -118,7 +118,7 @@ void ath9k_ps_restore(struct ath_softc *sc)
 	if (--sc->ps_usecount != 0)
 		goto unlock;
 
-	if (sc->ps_idle)
+	if (sc->ps_idle && (sc->ps_flags & PS_WAIT_FOR_TX_ACK))
 		mode = ATH9K_PM_FULL_SLEEP;
 	else if (sc->ps_enabled &&
 		 !(sc->ps_flags & (PS_WAIT_FOR_BEACON |
@@ -332,7 +332,8 @@ static int ath_reset_internal(struct ath_softc *sc, struct ath9k_channel *hchan,
 		hchan = ah->curchan;
 	}
 
-	if (fastcc && !ath9k_hw_check_alive(ah))
+	if (fastcc && (ah->chip_fullsleep ||
+	    !ath9k_hw_check_alive(ah)))
 		fastcc = false;
 
 	if (!ath_prepare_reset(sc, retry_tx, flush))
@@ -1176,6 +1177,13 @@ static void ath9k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
 		}
 	}
 
+	/*
+	 * Cannot tx while the hardware is in full sleep, it first needs a full
+	 * chip reset to recover from that
+	 */
+	if (unlikely(sc->sc_ah->power_mode == ATH9K_PM_FULL_SLEEP))
+		goto exit;
+
 	if (unlikely(sc->sc_ah->power_mode != ATH9K_PM_AWAKE)) {
 		/*
 		 * We are using PS-Poll and mac80211 can request TX while in
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 55d077e..363bf33 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1955,7 +1955,7 @@ static void ath_tx_complete(struct ath_softc *sc, struct sk_buff *skb,
 		skb_pull(skb, padsize);
 	}
 
-	if (sc->ps_flags & PS_WAIT_FOR_TX_ACK) {
+	if ((sc->ps_flags & PS_WAIT_FOR_TX_ACK) && !txq->axq_depth) {
 		sc->ps_flags &= ~PS_WAIT_FOR_TX_ACK;
 		ath_dbg(common, ATH_DBG_PS,
 			"Going back to sleep after having received TX status (0x%lx)\n",
-- 
1.7.3.2

[PATCH 2/4] ath9k: rework power state handling

[Felix Fietkau]

Turning off the radio when mac80211 tells the driver that it's idle is not
a good idea, as idle interfaces might still occasionally scan or send packets.
The only time the radio can be safely turned off is when drv_stop has been
called. In the mean time, use sc->ps_idle only to indicate network sleep vs
full sleep.
Move the LED GPIO changes out of the PCI suspend/resume path, the start/stop
functions already take care of that.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/main.c |  156 ++++++++++-----------------------
 drivers/net/wireless/ath/ath9k/pci.c  |   19 +----
 2 files changed, 47 insertions(+), 128 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 734e854..81e1368 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -883,82 +883,6 @@ chip_reset:
 #undef SCHED_INTR
 }
 
-static void ath_radio_enable(struct ath_softc *sc, struct ieee80211_hw *hw)
-{
-	struct ath_hw *ah = sc->sc_ah;
-	struct ath_common *common = ath9k_hw_common(ah);
-	struct ieee80211_channel *channel = hw->conf.channel;
-	int r;
-
-	ath9k_ps_wakeup(sc);
-	spin_lock_bh(&sc->sc_pcu_lock);
-	atomic_set(&ah->intr_ref_cnt, -1);
-
-	ath9k_hw_configpcipowersave(ah, false);
-
-	if (!ah->curchan)
-		ah->curchan = ath9k_cmn_get_curchannel(sc->hw, ah);
-
-	r = ath9k_hw_reset(ah, ah->curchan, ah->caldata, false);
-	if (r) {
-		ath_err(common,
-			"Unable to reset channel (%u MHz), reset status %d\n",
-			channel->center_freq, r);
-	}
-
-	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);
-
-	spin_unlock_bh(&sc->sc_pcu_lock);
-
-	ath9k_ps_restore(sc);
-}
-
-void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw)
-{
-	struct ath_hw *ah = sc->sc_ah;
-	struct ieee80211_channel *channel = hw->conf.channel;
-	int r;
-
-	ath9k_ps_wakeup(sc);
-
-	ath_cancel_work(sc);
-
-	spin_lock_bh(&sc->sc_pcu_lock);
-
-	/*
-	 * Keep the LED on when the radio is disabled
-	 * during idle unassociated state.
-	 */
-	if (!sc->ps_idle) {
-		ath9k_hw_set_gpio(ah, ah->led_pin, 1);
-		ath9k_hw_cfg_gpio_input(ah, ah->led_pin);
-	}
-
-	ath_prepare_reset(sc, false, true);
-
-	if (!ah->curchan)
-		ah->curchan = ath9k_cmn_get_curchannel(hw, ah);
-
-	r = ath9k_hw_reset(ah, ah->curchan, ah->caldata, false);
-	if (r) {
-		ath_err(ath9k_hw_common(sc->sc_ah),
-			"Unable to reset channel (%u MHz), reset status %d\n",
-			channel->center_freq, r);
-	}
-
-	ath9k_hw_phy_disable(ah);
-
-	ath9k_hw_configpcipowersave(ah, true);
-
-	spin_unlock_bh(&sc->sc_pcu_lock);
-	ath9k_ps_restore(sc);
-}
-
 static int ath_reset(struct ath_softc *sc, bool retry_tx)
 {
 	int r;
@@ -1094,6 +1018,9 @@ static int ath9k_start(struct ieee80211_hw *hw)
 	 * and then setup of the interrupt mask.
 	 */
 	spin_lock_bh(&sc->sc_pcu_lock);
+
+	atomic_set(&ah->intr_ref_cnt, -1);
+
 	r = ath9k_hw_reset(ah, init_channel, ah->caldata, false);
 	if (r) {
 		ath_err(common,
@@ -1132,6 +1059,18 @@ static int ath9k_start(struct ieee80211_hw *hw)
 		goto mutex_unlock;
 	}
 
+	if (ah->led_pin >= 0) {
+		ath9k_hw_cfg_output(ah, ah->led_pin,
+				    AR_GPIO_OUTPUT_MUX_AS_OUTPUT);
+		ath9k_hw_set_gpio(ah, ah->led_pin, 0);
+	}
+
+	/*
+	 * Reset key cache to sane defaults (all entries cleared) instead of
+	 * semi-random values after suspend/resume.
+	 */
+	ath9k_cmn_init_crypto(sc->sc_ah);
+
 	spin_unlock_bh(&sc->sc_pcu_lock);
 
 	if ((ah->btcoex_hw.scheme != ATH_BTCOEX_CFG_NONE) &&
@@ -1230,6 +1169,7 @@ static void ath9k_stop(struct ieee80211_hw *hw)
 	struct ath_softc *sc = hw->priv;
 	struct ath_hw *ah = sc->sc_ah;
 	struct ath_common *common = ath9k_hw_common(ah);
+	bool prev_idle;
 
 	mutex_lock(&sc->mutex);
 
@@ -1260,35 +1200,45 @@ static void ath9k_stop(struct ieee80211_hw *hw)
 	 * before setting the invalid flag. */
 	ath9k_hw_disable_interrupts(ah);
 
-	if (!(sc->sc_flags & SC_OP_INVALID)) {
-		ath_drain_all_txq(sc, false);
-		ath_stoprecv(sc);
-		ath9k_hw_phy_disable(ah);
-	} else
-		sc->rx.rxlink = NULL;
+	spin_unlock_bh(&sc->sc_pcu_lock);
+
+	/* we can now sync irq and kill any running tasklets, since we already
+	 * disabled interrupts and not holding a spin lock */
+	synchronize_irq(sc->irq);
+	tasklet_kill(&sc->intr_tq);
+	tasklet_kill(&sc->bcon_tasklet);
+
+	prev_idle = sc->ps_idle;
+	sc->ps_idle = true;
+
+	spin_lock_bh(&sc->sc_pcu_lock);
+
+	if (ah->led_pin >= 0) {
+		ath9k_hw_set_gpio(ah, ah->led_pin, 1);
+		ath9k_hw_cfg_gpio_input(ah, ah->led_pin);
+	}
+
+	ath_prepare_reset(sc, false, true);
 
 	if (sc->rx.frag) {
 		dev_kfree_skb_any(sc->rx.frag);
 		sc->rx.frag = NULL;
 	}
 
-	/* disable HAL and put h/w to sleep */
-	ath9k_hw_disable(ah);
+	if (!ah->curchan)
+		ah->curchan = ath9k_cmn_get_curchannel(hw, ah);
 
-	spin_unlock_bh(&sc->sc_pcu_lock);
+	ath9k_hw_reset(ah, ah->curchan, ah->caldata, false);
+	ath9k_hw_phy_disable(ah);
 
-	/* we can now sync irq and kill any running tasklets, since we already
-	 * disabled interrupts and not holding a spin lock */
-	synchronize_irq(sc->irq);
-	tasklet_kill(&sc->intr_tq);
-	tasklet_kill(&sc->bcon_tasklet);
+	ath9k_hw_configpcipowersave(ah, true);
 
-	ath9k_ps_restore(sc);
+	spin_unlock_bh(&sc->sc_pcu_lock);
 
-	sc->ps_idle = true;
-	ath_radio_disable(sc, hw);
+	ath9k_ps_restore(sc);
 
 	sc->sc_flags |= SC_OP_INVALID;
+	sc->ps_idle = prev_idle;
 
 	mutex_unlock(&sc->mutex);
 
@@ -1628,8 +1578,8 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 	struct ath_hw *ah = sc->sc_ah;
 	struct ath_common *common = ath9k_hw_common(ah);
 	struct ieee80211_conf *conf = &hw->conf;
-	bool disable_radio = false;
 
+	ath9k_ps_wakeup(sc);
 	mutex_lock(&sc->mutex);
 
 	/*
@@ -1638,16 +1588,8 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 	 * of the changes. Likewise we must only disable the radio towards
 	 * the end.
 	 */
-	if (changed & IEEE80211_CONF_CHANGE_IDLE) {
+	if (changed & IEEE80211_CONF_CHANGE_IDLE)
 		sc->ps_idle = !!(conf->flags & IEEE80211_CONF_IDLE);
-		if (!sc->ps_idle) {
-			ath_radio_enable(sc, hw);
-			ath_dbg(common, ATH_DBG_CONFIG,
-				"not-idle: enabling radio\n");
-		} else {
-			disable_radio = true;
-		}
-	}
 
 	/*
 	 * We just prepare to enable PS. We have to wait until our AP has
@@ -1753,18 +1695,12 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 		ath_dbg(common, ATH_DBG_CONFIG,
 			"Set power: %d\n", conf->power_level);
 		sc->config.txpowlimit = 2 * conf->power_level;
-		ath9k_ps_wakeup(sc);
 		ath9k_cmn_update_txpow(ah, sc->curtxpow,
 				       sc->config.txpowlimit, &sc->curtxpow);
-		ath9k_ps_restore(sc);
-	}
-
-	if (disable_radio) {
-		ath_dbg(common, ATH_DBG_CONFIG, "idle: disabling radio\n");
-		ath_radio_disable(sc, hw);
 	}
 
 	mutex_unlock(&sc->mutex);
+	ath9k_ps_restore(sc);
 
 	return 0;
 }
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index 2dcdf63..f42ab6c 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -307,12 +307,11 @@ static int ath_pci_suspend(struct device *device)
 	struct ieee80211_hw *hw = pci_get_drvdata(pdev);
 	struct ath_softc *sc = hw->priv;
 
-	ath9k_hw_set_gpio(sc->sc_ah, sc->sc_ah->led_pin, 1);
-
 	/* The device has to be moved to FULLSLEEP forcibly.
 	 * Otherwise the chip never moved to full sleep,
 	 * when no interface is up.
 	 */
+	ath9k_hw_disable(sc->sc_ah);
 	ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_FULL_SLEEP);
 
 	return 0;
@@ -334,22 +333,6 @@ static int ath_pci_resume(struct device *device)
 	if ((val & 0x0000ff00) != 0)
 		pci_write_config_dword(pdev, 0x40, val & 0xffff00ff);
 
-	ath9k_ps_wakeup(sc);
-	/* Enable LED */
-	ath9k_hw_cfg_output(sc->sc_ah, sc->sc_ah->led_pin,
-			    AR_GPIO_OUTPUT_MUX_AS_OUTPUT);
-	ath9k_hw_set_gpio(sc->sc_ah, sc->sc_ah->led_pin, 0);
-
-	  /*
-	   * Reset key cache to sane defaults (all entries cleared) instead of
-	   * semi-random values after suspend/resume.
-	   */
-	ath9k_cmn_init_crypto(sc->sc_ah);
-	ath9k_ps_restore(sc);
-
-	sc->ps_idle = true;
-	ath_radio_disable(sc, hw);
-
 	return 0;
 }
 
-- 
1.7.3.2

[PATCH 3/4] ath9k: only drop packets in drv_flush when asked to

[Felix Fietkau]

Recently more places in mac80211 call drv_flush to ensure proper order
for state changes wrt. powersave, channel changes, etc. On some systems
such calls lead to spurious logspam about failing to stop tx dma, as well
as hardware resets that go along with that.
Instead of dropping packets in a place where it's completely unnecessary,
only do it when drop == true.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/main.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 81e1368..a7f7926 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -2268,9 +2268,6 @@ static void ath9k_flush(struct ieee80211_hw *hw, bool drop)
 		return;
 	}
 
-	if (drop)
-		timeout = 1;
-
 	for (j = 0; j < timeout; j++) {
 		bool npend = false;
 
@@ -2288,21 +2285,22 @@ static void ath9k_flush(struct ieee80211_hw *hw, bool drop)
 		}
 
 		if (!npend)
-		    goto out;
+		    break;
 	}
 
-	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 (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);
+		if (!drain_txq)
+			ath_reset(sc, false);
 
-	ath9k_ps_restore(sc);
-	ieee80211_wake_queues(hw);
+		ath9k_ps_restore(sc);
+		ieee80211_wake_queues(hw);
+	}
 
-out:
 	ieee80211_queue_delayed_work(hw, &sc->tx_complete_work, 0);
 	mutex_unlock(&sc->mutex);
 }
-- 
1.7.3.2

[PATCH 4/4] ath9k: cancel all workqueue activity when going idle

[Felix Fietkau]

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/main.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index a7f7926..edee011 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1588,8 +1588,11 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 	 * of the changes. Likewise we must only disable the radio towards
 	 * the end.
 	 */
-	if (changed & IEEE80211_CONF_CHANGE_IDLE)
+	if (changed & IEEE80211_CONF_CHANGE_IDLE) {
 		sc->ps_idle = !!(conf->flags & IEEE80211_CONF_IDLE);
+		if (sc->ps_idle)
+			ath_cancel_work(sc);
+	}
 
 	/*
 	 * We just prepare to enable PS. We have to wait until our AP has
-- 
1.7.3.2

Re: [PATCH 2/4] ath9k: rework power state handling

[Luis R. Rodriguez]

On Wed, Nov 16, 2011 at 4:08 AM, Felix Fietkau <nbd@openwrt.org> wrote:
> Turning off the radio when mac80211 tells the driver that it's idle is not
> a good idea, as idle interfaces might still occasionally scan or send packets.
> The only time the radio can be safely turned off is when drv_stop has been
> called. In the mean time, use sc->ps_idle only to indicate network sleep vs
> full sleep.
> Move the LED GPIO changes out of the PCI suspend/resume path, the start/stop
> functions already take care of that.
>
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>

This set of patches are going to need some good amount of testing for
power impact. The benefits and gains need to be considered. I'll yield
to Senthil and Paul for final approval on this.

  Luis

Re: [PATCH 2/4] ath9k: rework power state handling

[Felix Fietkau]

On 16.11.2011, at 18:42, "Luis R. Rodriguez" <mcgrof@qca.qualcomm.com> wrote:

> On Wed, Nov 16, 2011 at 4:08 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>> Turning off the radio when mac80211 tells the driver that it's idle is not
>> a good idea, as idle interfaces might still occasionally scan or send packets.
>> The only time the radio can be safely turned off is when drv_stop has been
>> called. In the mean time, use sc->ps_idle only to indicate network sleep vs
>> full sleep.
>> Move the LED GPIO changes out of the PCI suspend/resume path, the start/stop
>> functions already take care of that.
>> 
>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> 
> This set of patches are going to need some good amount of testing for
> power impact. The benefits and gains need to be considered. I'll yield
> to Senthil and Paul for final approval on this.
One of the gains is that this series fixes some "PCI error" crashes on OpenWrt during hostapd restarts.

- Felix

Re: [PATCH 2/4] ath9k: rework power state handling

[Luis R. Rodriguez]

On Wed, Nov 16, 2011 at 10:16 AM, Felix Fietkau <nbd@nbd.name> wrote:
> On 16.11.2011, at 18:42, "Luis R. Rodriguez" <mcgrof@qca.qualcomm.com> wrote:
>
>> On Wed, Nov 16, 2011 at 4:08 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>> Turning off the radio when mac80211 tells the driver that it's idle is not
>>> a good idea, as idle interfaces might still occasionally scan or send packets.
>>> The only time the radio can be safely turned off is when drv_stop has been
>>> called. In the mean time, use sc->ps_idle only to indicate network sleep vs
>>> full sleep.
>>> Move the LED GPIO changes out of the PCI suspend/resume path, the start/stop
>>> functions already take care of that.
>>>
>>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>>
>> This set of patches are going to need some good amount of testing for
>> power impact. The benefits and gains need to be considered. I'll yield
>> to Senthil and Paul for final approval on this.
>
> One of the gains is that this series fixes some "PCI error" crashes
> on OpenWrt during hostapd restarts.

Thanks for the clarification, I was expecting this to be the case, the
point I am trying to make though is that this "fix" itself may
introduce a regression for non-AP devices, specifically in terms of
power consumption and I'd like the stakeholders who do care about that
to review these changes carefully as well. The difficulty will be to
find a reasonable middle ground.

  Luis

Re: [PATCH 2/4] ath9k: rework power state handling

[Paul Stewart]

In fact, could this be made contingent on whether there's an AP-mode
interface, since the problem you're encountering seems to only be in
AP mode?  What troubles me, Felix, is your statement that " this
series fixes some "PCI error" crashes".  Does this mean that you
haven't root-caused the problem, and that this may only partially mask
it?

--
Paul

On Wed, Nov 16, 2011 at 10:23 AM, Luis R. Rodriguez
<mcgrof@qca.qualcomm.com> wrote:
> On Wed, Nov 16, 2011 at 10:16 AM, Felix Fietkau <nbd@nbd.name> wrote:
>> On 16.11.2011, at 18:42, "Luis R. Rodriguez" <mcgrof@qca.qualcomm.com> wrote:
>>
>>> On Wed, Nov 16, 2011 at 4:08 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>>>> Turning off the radio when mac80211 tells the driver that it's idle is not
>>>> a good idea, as idle interfaces might still occasionally scan or send packets.
>>>> The only time the radio can be safely turned off is when drv_stop has been
>>>> called. In the mean time, use sc->ps_idle only to indicate network sleep vs
>>>> full sleep.
>>>> Move the LED GPIO changes out of the PCI suspend/resume path, the start/stop
>>>> functions already take care of that.
>>>>
>>>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>>>
>>> This set of patches are going to need some good amount of testing for
>>> power impact. The benefits and gains need to be considered. I'll yield
>>> to Senthil and Paul for final approval on this.
>>
>> One of the gains is that this series fixes some "PCI error" crashes
>> on OpenWrt during hostapd restarts.
>
> Thanks for the clarification, I was expecting this to be the case, the
> point I am trying to make though is that this "fix" itself may
> introduce a regression for non-AP devices, specifically in terms of
> power consumption and I'd like the stakeholders who do care about that
> to review these changes carefully as well. The difficulty will be to
> find a reasonable middle ground.
>
>  Luis
>

Re: [PATCH 2/4] ath9k: rework power state handling

[Felix Fietkau]

On 2011-11-16 7:40 PM, Paul Stewart wrote:
> In fact, could this be made contingent on whether there's an AP-mode
> interface, since the problem you're encountering seems to only be in
> AP mode?  What troubles me, Felix, is your statement that " this
> series fixes some "PCI error" crashes".  Does this mean that you
> haven't root-caused the problem, and that this may only partially mask
> it?
The existing code was wrong, especially for client mode. I only noticed
that because I was investigating some AP mode crashes triggered by it.
My main focus when fixing this stuff was actually client mode since
that's the one most affected by this codepath.

- Felix

Re: [PATCH 2/4] ath9k: rework power state handling

[John W. Linville]

On Thu, Nov 17, 2011 at 12:52:38AM +0100, Felix Fietkau wrote:
> On 2011-11-16 7:40 PM, Paul Stewart wrote:
> > In fact, could this be made contingent on whether there's an AP-mode
> > interface, since the problem you're encountering seems to only be in
> > AP mode?  What troubles me, Felix, is your statement that " this
> > series fixes some "PCI error" crashes".  Does this mean that you
> > haven't root-caused the problem, and that this may only partially mask
> > it?
> The existing code was wrong, especially for client mode. I only noticed
> that because I was investigating some AP mode crashes triggered by it.
> My main focus when fixing this stuff was actually client mode since
> that's the one most affected by this codepath.

Are we satisfied w/ this series?  Or does it still need work?

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

Re: [PATCH 2/4] ath9k: rework power state handling

[Paul Stewart]

I haven't had time to do set up power draw testing.  You can put it
in, but we may revisit this when those are done.  If you have power
consumption figures of your own, that'd help.

--
Paul

On Tue, Dec 6, 2011 at 12:04 PM, John W. Linville
<linville@tuxdriver.com> wrote:
> On Thu, Nov 17, 2011 at 12:52:38AM +0100, Felix Fietkau wrote:
>> On 2011-11-16 7:40 PM, Paul Stewart wrote:
>> > In fact, could this be made contingent on whether there's an AP-mode
>> > interface, since the problem you're encountering seems to only be in
>> > AP mode?  What troubles me, Felix, is your statement that " this
>> > series fixes some "PCI error" crashes".  Does this mean that you
>> > haven't root-caused the problem, and that this may only partially mask
>> > it?
>> The existing code was wrong, especially for client mode. I only noticed
>> that because I was investigating some AP mode crashes triggered by it.
>> My main focus when fixing this stuff was actually client mode since
>> that's the one most affected by this codepath.
>
> Are we satisfied w/ this series?  Or does it still need work?
>
> John
> --
> John W. Linville                Someday the world will need a hero, and you
> linville@tuxdriver.com                  might be all we have.  Be ready.