[PATCH] ath5k: fix missing output in monitor mode after ifconfig up

[PATCH] ath5k: fix missing output in monitor mode after ifconfig up

[Joerg Albert]

Let ath5k_chan_set() call ath5k_reset() if ath5k_config() was told
that the channel changed. This fixes the bug that we don't
get any packets in monitor mode after:

ifconfig wlan0 down
iwconfig wlan0 mode monitor channel 1
ifconfig wlan0 up

but they arrive after

iwconfig wlan0 channel 2

Signed-off-by: Joerg Albert <jal2@gmx.de>
---
  drivers/net/wireless/ath/ath5k/base.c |   17 +++++++++--------
  1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 3a1c156..09ca89c 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -289,7 +289,8 @@ static unsigned int ath5k_copy_channels(struct ath5k_hw *ah,
                                 unsigned int max);
  static int     ath5k_setup_bands(struct ieee80211_hw *hw);
  static int     ath5k_chan_set(struct ath5k_softc *sc,
-                               struct ieee80211_channel *chan);
+                              struct ieee80211_channel *chan,
+                              int chan_changed);
  static void    ath5k_setcurmode(struct ath5k_softc *sc,
                                 unsigned int mode);
  static void    ath5k_mode_setup(struct ath5k_softc *sc);
@@ -1079,14 +1080,13 @@ ath5k_setup_bands(struct ieee80211_hw *hw)
   * Called with sc->lock.
   */
  static int
-ath5k_chan_set(struct ath5k_softc *sc, struct ieee80211_channel *chan)
+ath5k_chan_set(struct ath5k_softc *sc, struct ieee80211_channel *chan,
+              int chan_changed)
  {
-       ATH5K_DBG(sc, ATH5K_DEBUG_RESET, "(%u MHz) -> (%u MHz)\n",
-               sc->curchan->center_freq, chan->center_freq);
-
-       if (chan->center_freq != sc->curchan->center_freq ||
-               chan->hw_value != sc->curchan->hw_value) {
+       ATH5K_DBG(sc, ATH5K_DEBUG_RESET, "(%u MHz) -> (%u MHz) chan_changed %x\n",
+                 sc->curchan->center_freq, chan->center_freq, chan_changed);

+       if (chan_changed) {
                 /*
                  * To switch channels clear any pending DMA operations;
                  * wait long enough for the RX fifo to drain, reset the
@@ -2811,7 +2811,8 @@ ath5k_config(struct ieee80211_hw *hw, u32 changed)

         mutex_lock(&sc->lock);

-       ret = ath5k_chan_set(sc, conf->channel);
+       ret = ath5k_chan_set(sc, conf->channel,
+                            changed & IEEE80211_CONF_CHANGE_CHANNEL);
         if (ret < 0)
                 goto unlock;

-- 
1.6.0.4

Re: [PATCH] ath5k: fix missing output in monitor mode after ifconfig up

[Bob Copeland]

On Sun, Aug 02, 2009 at 12:33:26AM +0200, Joerg Albert wrote:
> Let ath5k_chan_set() call ath5k_reset() if ath5k_config() was told
> that the channel changed. This fixes the bug that we don't
> get any packets in monitor mode after:

Cool, thanks for the patch.  Just a couple of comments:

> +                              int chan_changed);

Should use bool for that.

> -       ret = ath5k_chan_set(sc, conf->channel);
> +       ret = ath5k_chan_set(sc, conf->channel,
> +                            changed & IEEE80211_CONF_CHANGE_CHANNEL);

Since this is the only place ath5k_chan_set is called, how about changing
ath5k_chan_set to unconditionally do a reset, and put the test outside?

i.e.:

        if (changed & IEEE80211_CONF_CHANGE_CHANNEL)
                ath5k_chan_set(sc, conf->channel);

Thanks!

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] ath5k: fix missing output in monitor mode after ifconfig up

[Joerg Albert]

On 08/02/2009 05:20 AM, Bob Copeland wrote:
> On Sun, Aug 02, 2009 at 12:33:26AM +0200, Joerg Albert wrote:
>> Let ath5k_chan_set() call ath5k_reset() if ath5k_config() was told
>> that the channel changed. This fixes the bug that we don't
>> get any packets in monitor mode after:
> 
> Cool, thanks for the patch.  Just a couple of comments:
> 
>> +                              int chan_changed);
> 
> Should use bool for that.
> 
>> -       ret = ath5k_chan_set(sc, conf->channel);
>> +       ret = ath5k_chan_set(sc, conf->channel,
>> +                            changed & IEEE80211_CONF_CHANGE_CHANNEL);
> 
> Since this is the only place ath5k_chan_set is called, how about changing
> ath5k_chan_set to unconditionally do a reset, and put the test outside?
> 
> i.e.:
> 
>         if (changed & IEEE80211_CONF_CHANGE_CHANNEL)
>                 ath5k_chan_set(sc, conf->channel);

Good idea. I'll redo the patch.

BTW, it assumes that IEEE80211_CONF_CHANGE_CHANNEL is set even if only chan->hw_value has changed,
e.g. by switching from A to TurboA on the same channel. AFAIK TurboX isn't currently supported by ath5k.

Regards,
Jörg.

Re: [PATCH v2] ath5k: fix missing output in monitor mode after ifconfig up

[Joerg Albert]

Let ath5k_chan_set() always call ath5k_reset().
This fixes the bug that we don't
get any packets in monitor mode after:

ifconfig wlan0 down
iwconfig wlan0 mode monitor channel 1
ifconfig wlan0 up

but they arrive after

iwconfig wlan0 channel 2

Signed-off-by: Joerg Albert <jal2@gmx.de>
---
  drivers/net/wireless/ath/ath5k/base.c |   35 ++++++++++++++------------------
  1 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 3a1c156..5d50285 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1071,10 +1071,9 @@ ath5k_setup_bands(struct ieee80211_hw *hw)
  }

  /*
- * 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 after a la
- * ath5k_init.
+ * Set/change channels. We always reset the chip.
+ * To accomplish this we must first cleanup any pending DMA,
+ * then restart stuff after a la  ath5k_init.
   *
   * Called with sc->lock.
   */
@@ -1084,19 +1083,13 @@ ath5k_chan_set(struct ath5k_softc *sc, struct ieee80211_channel *chan)
         ATH5K_DBG(sc, ATH5K_DEBUG_RESET, "(%u MHz) -> (%u MHz)\n",
                 sc->curchan->center_freq, chan->center_freq);

-       if (chan->center_freq != sc->curchan->center_freq ||
-               chan->hw_value != sc->curchan->hw_value) {
-
-               /*
-                * 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.
-                */
-               return ath5k_reset(sc, chan);
-       }
-
-       return 0;
+       /*
+        * 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.
+        */
+       return ath5k_reset(sc, chan);
  }

  static void
@@ -2811,9 +2804,11 @@ ath5k_config(struct ieee80211_hw *hw, u32 changed)

         mutex_lock(&sc->lock);

-       ret = ath5k_chan_set(sc, conf->channel);
-       if (ret < 0)
-               goto unlock;
+       if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
+               ret = ath5k_chan_set(sc, conf->channel);
+               if (ret < 0)
+                       goto unlock;
+       }

         if ((changed & IEEE80211_CONF_CHANGE_POWER) &&
         (sc->power_level != conf->power_level)) {
-- 
1.6.0.4

Re: [PATCH v2] ath5k: fix missing output in monitor mode after ifconfig up

[Bob Copeland]

On Sun, Aug 2, 2009 at 3:50 AM, Joerg Albert<jal2@gmx.de> wrote:
> Let ath5k_chan_set() always call ath5k_reset().
> This fixes the bug that we don't
> get any packets in monitor mode after:

Thanks!  Looks good.

Acked-by: Bob Copeland <me@bobcopeland.com>

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH v2] ath5k: fix missing output in monitor mode after ifconfig up

[John W. Linville]

On Sun, Aug 02, 2009 at 09:50:58AM +0200, Joerg Albert wrote:
> Let ath5k_chan_set() always call ath5k_reset().
> This fixes the bug that we don't
> get any packets in monitor mode after:
>
> ifconfig wlan0 down
> iwconfig wlan0 mode monitor channel 1
> ifconfig wlan0 up
>
> but they arrive after
>
> iwconfig wlan0 channel 2
>
> Signed-off-by: Joerg Albert <jal2@gmx.de>

patching file drivers/net/wireless/ath/ath5k/base.c
Hunk #1 FAILED at 1071.
Hunk #2 FAILED at 1083.
Hunk #3 FAILED at 2804.
3 out of 3 hunks FAILED -- saving rejects to file drivers/net/wireless/ath/ath5k/base.c.rej

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

Re: [PATCH v2] ath5k: fix missing output in monitor mode after ifconfig up

[Joerg Albert]

On 08/04/2009 05:41 PM, John W. Linville wrote:
> On Sun, Aug 02, 2009 at 09:50:58AM +0200, Joerg Albert wrote:
>> Let ath5k_chan_set() always call ath5k_reset().
>> This fixes the bug that we don't
>> get any packets in monitor mode after:
>>
>> ifconfig wlan0 down
>> iwconfig wlan0 mode monitor channel 1
>> ifconfig wlan0 up
>>
>> but they arrive after
>>
>> iwconfig wlan0 channel 2
>>
>> Signed-off-by: Joerg Albert <jal2@gmx.de>
> 
> patching file drivers/net/wireless/ath/ath5k/base.c
> Hunk #1 FAILED at 1071.
> Hunk #2 FAILED at 1083.
> Hunk #3 FAILED at 2804.
> 3 out of 3 hunks FAILED -- saving rejects to file drivers/net/wireless/ath/ath5k/base.c.rej

Sorry, seems like Thunderbird has converted the whitespaces.
I'll repost the patch.

[PATCH v3] ath5k: fix missing output in monitor mode after ifconfig up

[Joerg Albert]

Let ath5k_chan_set() always call ath5k_reset().
This fixes the bug that we don't
get any packets in monitor mode after:

ifconfig wlan0 down
iwconfig wlan0 mode monitor channel 1
ifconfig wlan0 up

but they arrive after

iwconfig wlan0 channel 2

Signed-off-by: Joerg Albert <jal2@gmx.de>
---

Same version as [PATCH v2] but with correct whitespaces (hopefully).

 drivers/net/wireless/ath/ath5k/base.c |   35 ++++++++++++++------------------
 1 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 3a1c156..5d50285 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1071,10 +1071,9 @@ ath5k_setup_bands(struct ieee80211_hw *hw)
 }
 
 /*
- * 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 after a la
- * ath5k_init.
+ * Set/change channels. We always reset the chip.
+ * To accomplish this we must first cleanup any pending DMA,
+ * then restart stuff after a la  ath5k_init.
  *
  * Called with sc->lock.
  */
@@ -1084,19 +1083,13 @@ ath5k_chan_set(struct ath5k_softc *sc, struct ieee80211_channel *chan)
 	ATH5K_DBG(sc, ATH5K_DEBUG_RESET, "(%u MHz) -> (%u MHz)\n",
 		sc->curchan->center_freq, chan->center_freq);
 
-	if (chan->center_freq != sc->curchan->center_freq ||
-		chan->hw_value != sc->curchan->hw_value) {
-
-		/*
-		 * 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.
-		 */
-		return ath5k_reset(sc, chan);
-	}
-
-	return 0;
+	/*
+	 * 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.
+	 */
+	return ath5k_reset(sc, chan);
 }
 
 static void
@@ -2811,9 +2804,11 @@ ath5k_config(struct ieee80211_hw *hw, u32 changed)
 
 	mutex_lock(&sc->lock);
 
-	ret = ath5k_chan_set(sc, conf->channel);
-	if (ret < 0)
-		goto unlock;
+	if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
+		ret = ath5k_chan_set(sc, conf->channel);
+		if (ret < 0)
+			goto unlock;
+	}
 
 	if ((changed & IEEE80211_CONF_CHANGE_POWER) &&
 	(sc->power_level != conf->power_level)) {
-- 
1.6.0.4