[PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout

[PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout

[Arik Nemtsov]

With PSM handled in FW, the checks in wl1271_elp_work() are always true.
Thus during active traffic we constantly enter and exit ELP (many times
per second). As each ELP exit takes ~10ms, this can have an adverse
effect on throughput and interactivity.
Set the ELP timeout to the dyn-ps timeout. This period is longer and
avoids the above problem. It also makes sense to stay out of ELP while
we are awake on the network, to minimize delays in Tx/Rx. The same thing
was done by the mac80211 dynamic-ps mechanism before the FW DPS changes.

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 drivers/net/wireless/wl12xx/ps.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/ps.c b/drivers/net/wireless/wl12xx/ps.c
index 23d6750..78f598b 100644
--- a/drivers/net/wireless/wl12xx/ps.c
+++ b/drivers/net/wireless/wl12xx/ps.c
@@ -69,8 +69,6 @@ out:
 	mutex_unlock(&wl->mutex);
 }
 
-#define ELP_ENTRY_DELAY  5
-
 /* Routines to toggle sleep mode while in ELP */
 void wl1271_ps_elp_sleep(struct wl1271 *wl)
 {
@@ -90,7 +88,7 @@ void wl1271_ps_elp_sleep(struct wl1271 *wl)
 	}
 
 	ieee80211_queue_delayed_work(wl->hw, &wl->elp_work,
-				     msecs_to_jiffies(ELP_ENTRY_DELAY));
+		msecs_to_jiffies(wl->conf.conn.dynamic_ps_timeout));
 }
 
 int wl1271_ps_elp_wakeup(struct wl1271 *wl)
-- 
1.7.5.4

[PATCH 2/7] wl12xx: change bits in the link_map under spin lock

[Arik Nemtsov]

These bits are used in op_tx to determine if a packet should be dropped.
As such we should use the spin lock to sync the state.

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 drivers/net/wireless/wl12xx/cmd.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
index b776d9d..ef994da 100644
--- a/drivers/net/wireless/wl12xx/cmd.c
+++ b/drivers/net/wireless/wl12xx/cmd.c
@@ -459,23 +459,32 @@ out:
 
 int wl12xx_allocate_link(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 *hlid)
 {
+	unsigned long flags;
 	u8 link = find_first_zero_bit(wl->links_map, WL12XX_MAX_LINKS);
 	if (link >= WL12XX_MAX_LINKS)
 		return -EBUSY;
 
+	/* these bits are used by op_tx */
+	spin_lock_irqsave(&wl->wl_lock, flags);
 	__set_bit(link, wl->links_map);
 	__set_bit(link, wlvif->links_map);
+	spin_unlock_irqrestore(&wl->wl_lock, flags);
 	*hlid = link;
 	return 0;
 }
 
 void wl12xx_free_link(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 *hlid)
 {
+	unsigned long flags;
+
 	if (*hlid == WL12XX_INVALID_LINK_ID)
 		return;
 
+	/* these bits are used by op_tx */
+	spin_lock_irqsave(&wl->wl_lock, flags);
 	__clear_bit(*hlid, wl->links_map);
 	__clear_bit(*hlid, wlvif->links_map);
+	spin_unlock_irqrestore(&wl->wl_lock, flags);
 	*hlid = WL12XX_INVALID_LINK_ID;
 }
 
-- 
1.7.5.4

[PATCH 3/7] wl12xx: reset link Tx queues when freeing it

[Arik Nemtsov]

Before, the link was first freed (invalidating it in the map), and later
on vif removal, all valid wlvif-related links were reset. Since these
links were already invalid, we failed to reset them.
The bug was made worse by op_stop, which set the tx_queue_count to 0
arbitrarily. This resulted in a negative tx_queue_count in some scenarios.

Fix this by resetting the Tx-queues of a link when freeing it. Add a
WARN_ON and reset all link Tx-queues in op_stop, to avoid a negative
tx_queue_count.

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 drivers/net/wireless/wl12xx/cmd.c  |    7 +++++++
 drivers/net/wireless/wl12xx/main.c |    1 -
 drivers/net/wireless/wl12xx/tx.c   |   13 ++++++++++---
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
index ef994da..ae1f3d7 100644
--- a/drivers/net/wireless/wl12xx/cmd.c
+++ b/drivers/net/wireless/wl12xx/cmd.c
@@ -485,6 +485,13 @@ void wl12xx_free_link(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 *hlid)
 	__clear_bit(*hlid, wl->links_map);
 	__clear_bit(*hlid, wlvif->links_map);
 	spin_unlock_irqrestore(&wl->wl_lock, flags);
+
+	/*
+	 * At this point op_tx() will not add more packets to the queues. We
+	 * can purge them.
+	 */
+	wl1271_tx_reset_link_queues(wl, *hlid);
+
 	*hlid = WL12XX_INVALID_LINK_ID;
 }
 
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index adf9bbc..712e385 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -4228,7 +4228,6 @@ void wl1271_free_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 hlid)
 	clear_bit(hlid, wlvif->ap.sta_hlid_map);
 	memset(wl->links[hlid].addr, 0, ETH_ALEN);
 	wl->links[hlid].ba_bitmap = 0;
-	wl1271_tx_reset_link_queues(wl, hlid);
 	__clear_bit(hlid, &wl->ap_ps_map);
 	__clear_bit(hlid, (unsigned long *)&wl->ap_fw_ps_map);
 	wl12xx_free_link(wl, wlvif, &hlid);
diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 6446e4d..311b5a2 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -527,6 +527,7 @@ static struct sk_buff *wl12xx_lnk_skb_dequeue(struct wl1271 *wl,
 	if (skb) {
 		int q = wl1271_tx_get_queue(skb_get_queue_mapping(skb));
 		spin_lock_irqsave(&wl->wl_lock, flags);
+		WARN_ON(wl->tx_queue_count[q] <= 0);
 		wl->tx_queue_count[q]--;
 		spin_unlock_irqrestore(&wl->wl_lock, flags);
 	}
@@ -602,6 +603,7 @@ static struct sk_buff *wl1271_skb_dequeue(struct wl1271 *wl)
 		skb = wl->dummy_packet;
 		q = wl1271_tx_get_queue(skb_get_queue_mapping(skb));
 		spin_lock_irqsave(&wl->wl_lock, flags);
+		WARN_ON(wl->tx_queue_count[q] <= 0);
 		wl->tx_queue_count[q]--;
 		spin_unlock_irqrestore(&wl->wl_lock, flags);
 	}
@@ -959,7 +961,6 @@ void wl12xx_tx_reset_wlvif(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 		else
 			wlvif->sta.ba_rx_bitmap = 0;
 
-		wl1271_tx_reset_link_queues(wl, i);
 		wl->links[i].allocated_pkts = 0;
 		wl->links[i].prev_freed_pkts = 0;
 	}
@@ -973,8 +974,14 @@ void wl12xx_tx_reset(struct wl1271 *wl, bool reset_tx_queues)
 	struct sk_buff *skb;
 	struct ieee80211_tx_info *info;
 
-	for (i = 0; i < NUM_TX_QUEUES; i++)
-		wl->tx_queue_count[i] = 0;
+	/* only reset the queues if something bad happened */
+	if (WARN_ON(wl1271_tx_total_queue_count(wl) != 0)) {
+		for (i = 0; i < WL12XX_MAX_LINKS; i++)
+			wl1271_tx_reset_link_queues(wl, i);
+
+		for (i = 0; i < NUM_TX_QUEUES; i++)
+			wl->tx_queue_count[i] = 0;
+	}
 
 	wl->stopped_queues_map = 0;
 
-- 
1.7.5.4

[PATCH 4/7] wl12xx: avoid starving the system hlid

[Arik Nemtsov]

Re-factor the Tx scheduler so that the system_hlid is taken into account
before restarting an iteration over the wlvifs. Previously this
hlid had a lower priority and would starve if some wlvif had many
packets.
In addition avoid iterating over wlvifs past last_wlvif when performing
the a second pass. If we had packets in those wlvifs they would have
been found earlier.

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 drivers/net/wireless/wl12xx/tx.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 311b5a2..76480a0 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -572,6 +572,7 @@ static struct sk_buff *wl1271_skb_dequeue(struct wl1271 *wl)
 	struct wl12xx_vif *wlvif = wl->last_wlvif;
 	struct sk_buff *skb = NULL;
 
+	/* continue from last wlvif (round robin) */
 	if (wlvif) {
 		wl12xx_for_each_wlvif_continue(wl, wlvif) {
 			skb = wl12xx_vif_skb_dequeue(wl, wlvif);
@@ -582,7 +583,11 @@ static struct sk_buff *wl1271_skb_dequeue(struct wl1271 *wl)
 		}
 	}
 
-	/* do another pass */
+	/* dequeue from the system HLID before the restarting wlvif list */
+	if (!skb)
+		skb = wl12xx_lnk_skb_dequeue(wl, &wl->links[wl->system_hlid]);
+
+	/* do a new pass over the wlvif list */
 	if (!skb) {
 		wl12xx_for_each_wlvif(wl, wlvif) {
 			skb = wl12xx_vif_skb_dequeue(wl, wlvif);
@@ -590,12 +595,16 @@ static struct sk_buff *wl1271_skb_dequeue(struct wl1271 *wl)
 				wl->last_wlvif = wlvif;
 				break;
 			}
+
+			/*
+			 * No need to continue after last_wlvif. The previous
+			 * pass should have found it.
+			 */
+			if (wlvif == wl->last_wlvif)
+				break;
 		}
 	}
 
-	if (!skb)
-		skb = wl12xx_lnk_skb_dequeue(wl, &wl->links[wl->system_hlid]);
-
 	if (!skb &&
 	    test_and_clear_bit(WL1271_FLAG_DUMMY_PACKET_PENDING, &wl->flags)) {
 		int q;
-- 
1.7.5.4

[PATCH 5/7] wl12xx: flush all Tx queues on tx_flush timeout

[Arik Nemtsov]

Ensure our queues are empty at the end of a tx_flush(), in case we
timeout on passively waiting for them. This makes sure no left-overs are
transmitted when we are on the wrong channel.

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 drivers/net/wireless/wl12xx/tx.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 76480a0..3540d4a 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -1040,6 +1040,7 @@ void wl12xx_tx_reset(struct wl1271 *wl, bool reset_tx_queues)
 void wl1271_tx_flush(struct wl1271 *wl)
 {
 	unsigned long timeout;
+	int i;
 	timeout = jiffies + usecs_to_jiffies(WL1271_TX_FLUSH_TIMEOUT);
 
 	while (!time_after(jiffies, timeout)) {
@@ -1057,6 +1058,12 @@ void wl1271_tx_flush(struct wl1271 *wl)
 	}
 
 	wl1271_warning("Unable to flush all TX buffers, timed out.");
+
+	/* forcibly flush all Tx buffers on our queues */
+	mutex_lock(&wl->mutex);
+	for (i = 0; i < WL12XX_MAX_LINKS; i++)
+		wl1271_tx_reset_link_queues(wl, i);
+	mutex_unlock(&wl->mutex);
 }
 
 u32 wl1271_tx_min_rate_get(struct wl1271 *wl, u32 rate_set)
-- 
1.7.5.4

[PATCH 6/7] wl12xx: flush Tx during suspend and 802.11h chan switch

[Arik Nemtsov]

Flush our Tx queues before suspending or changing the channel due to a
channel_switch element in the AP beacon.

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 drivers/net/wireless/wl12xx/main.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 712e385..ad1a4ad 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1737,6 +1737,8 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
 	wl1271_debug(DEBUG_MAC80211, "mac80211 suspend wow=%d", !!wow);
 	WARN_ON(!wow || !wow->any);
 
+	wl1271_tx_flush(wl);
+
 	wl->wow_enabled = true;
 	wl12xx_for_each_wlvif(wl, wlvif) {
 		ret = wl1271_configure_suspend(wl, wlvif);
@@ -4496,6 +4498,8 @@ static void wl12xx_op_channel_switch(struct ieee80211_hw *hw,
 
 	wl1271_debug(DEBUG_MAC80211, "mac80211 channel switch");
 
+	wl1271_tx_flush(wl);
+
 	mutex_lock(&wl->mutex);
 
 	if (unlikely(wl->state == WL1271_STATE_OFF)) {
-- 
1.7.5.4

[PATCH 7/7] wl12xx: increase dynamic PS timeout to 200ms

[Arik Nemtsov]

This ensures the user won't encounter lag associated with getting in and
out of PSM when the card is in use.

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 drivers/net/wireless/wl12xx/main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index ad1a4ad..418b2b8 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -246,7 +246,7 @@ static struct conf_drv_settings default_conf = {
 		.psm_entry_retries           = 8,
 		.psm_exit_retries            = 16,
 		.psm_entry_nullfunc_retries  = 3,
-		.dynamic_ps_timeout          = 100,
+		.dynamic_ps_timeout          = 200,
 		.forced_ps                   = false,
 		.keep_alive_interval         = 55000,
 		.max_listen_interval         = 20,
-- 
1.7.5.4

Re: [PATCH 3/7] wl12xx: reset link Tx queues when freeing it

[Luciano Coelho]

On Tue, 2012-02-28 at 00:41 +0200, Arik Nemtsov wrote: 
> Before, the link was first freed (invalidating it in the map), and later
> on vif removal, all valid wlvif-related links were reset. Since these
> links were already invalid, we failed to reset them.
> The bug was made worse by op_stop, which set the tx_queue_count to 0
> arbitrarily. This resulted in a negative tx_queue_count in some scenarios.
> 
> Fix this by resetting the Tx-queues of a link when freeing it. Add a
> WARN_ON and reset all link Tx-queues in op_stop, to avoid a negative
> tx_queue_count.
> 
> Signed-off-by: Arik Nemtsov <arik@wizery.com>
> ---

[...]

> diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
> index 6446e4d..311b5a2 100644
> --- a/drivers/net/wireless/wl12xx/tx.c
> +++ b/drivers/net/wireless/wl12xx/tx.c
> @@ -527,6 +527,7 @@ static struct sk_buff *wl12xx_lnk_skb_dequeue(struct wl1271 *wl,
>  	if (skb) {
>  		int q = wl1271_tx_get_queue(skb_get_queue_mapping(skb));
>  		spin_lock_irqsave(&wl->wl_lock, flags);
> +		WARN_ON(wl->tx_queue_count[q] <= 0);
>  		wl->tx_queue_count[q]--;
>  		spin_unlock_irqrestore(&wl->wl_lock, flags);
>  	}
> @@ -602,6 +603,7 @@ static struct sk_buff *wl1271_skb_dequeue(struct wl1271 *wl)
>  		skb = wl->dummy_packet;
>  		q = wl1271_tx_get_queue(skb_get_queue_mapping(skb));
>  		spin_lock_irqsave(&wl->wl_lock, flags);
> +		WARN_ON(wl->tx_queue_count[q] <= 0);
>  		wl->tx_queue_count[q]--;
>  		spin_unlock_irqrestore(&wl->wl_lock, flags);
>  	}

[...]

> @@ -973,8 +974,14 @@ void wl12xx_tx_reset(struct wl1271 *wl, bool reset_tx_queues)
>  	struct sk_buff *skb;
>  	struct ieee80211_tx_info *info;
>  
> -	for (i = 0; i < NUM_TX_QUEUES; i++)
> -		wl->tx_queue_count[i] = 0;
> +	/* only reset the queues if something bad happened */
> +	if (WARN_ON(wl1271_tx_total_queue_count(wl) != 0)) {

Let's make all these WARN_ONs as WARN_ON_ONCE as it is recommended
nowadays.  This will make Kalle Valo happier. :)

I can do a quick sed before I apply this, if you agree.

-- 
Cheers,
Luca.

Re: [PATCH 3/7] wl12xx: reset link Tx queues when freeing it

[Arik Nemtsov]

On Tue, Feb 28, 2012 at 11:51, Luciano Coelho <coelho@ti.com> wrote:
> On Tue, 2012-02-28 at 00:41 +0200, Arik Nemtsov wrote:
>> Before, the link was first freed (invalidating it in the map), and later
>> on vif removal, all valid wlvif-related links were reset. Since these
>> links were already invalid, we failed to reset them.
>> The bug was made worse by op_stop, which set the tx_queue_count to 0
>> arbitrarily. This resulted in a negative tx_queue_count in some scenarios.
>>
>> Fix this by resetting the Tx-queues of a link when freeing it. Add a
>> WARN_ON and reset all link Tx-queues in op_stop, to avoid a negative
>> tx_queue_count.
>>
>> Signed-off-by: Arik Nemtsov <arik@wizery.com>
>> ---
>
> [...]
>
>> diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
>> index 6446e4d..311b5a2 100644
>> --- a/drivers/net/wireless/wl12xx/tx.c
>> +++ b/drivers/net/wireless/wl12xx/tx.c
>> @@ -527,6 +527,7 @@ static struct sk_buff *wl12xx_lnk_skb_dequeue(struct wl1271 *wl,
>>       if (skb) {
>>               int q = wl1271_tx_get_queue(skb_get_queue_mapping(skb));
>>               spin_lock_irqsave(&wl->wl_lock, flags);
>> +             WARN_ON(wl->tx_queue_count[q] <= 0);
>>               wl->tx_queue_count[q]--;
>>               spin_unlock_irqrestore(&wl->wl_lock, flags);
>>       }
>> @@ -602,6 +603,7 @@ static struct sk_buff *wl1271_skb_dequeue(struct wl1271 *wl)
>>               skb = wl->dummy_packet;
>>               q = wl1271_tx_get_queue(skb_get_queue_mapping(skb));
>>               spin_lock_irqsave(&wl->wl_lock, flags);
>> +             WARN_ON(wl->tx_queue_count[q] <= 0);
>>               wl->tx_queue_count[q]--;
>>               spin_unlock_irqrestore(&wl->wl_lock, flags);
>>       }
>
> [...]
>
>> @@ -973,8 +974,14 @@ void wl12xx_tx_reset(struct wl1271 *wl, bool reset_tx_queues)
>>       struct sk_buff *skb;
>>       struct ieee80211_tx_info *info;
>>
>> -     for (i = 0; i < NUM_TX_QUEUES; i++)
>> -             wl->tx_queue_count[i] = 0;
>> +     /* only reset the queues if something bad happened */
>> +     if (WARN_ON(wl1271_tx_total_queue_count(wl) != 0)) {
>
> Let's make all these WARN_ONs as WARN_ON_ONCE as it is recommended
> nowadays.  This will make Kalle Valo happier. :)
>
> I can do a quick sed before I apply this, if you agree.

Sure thing.

Arik

Re: [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout

[Luciano Coelho]

On Tue, 2012-02-28 at 00:41 +0200, Arik Nemtsov wrote: 
> With PSM handled in FW, the checks in wl1271_elp_work() are always true.
> Thus during active traffic we constantly enter and exit ELP (many times
> per second). As each ELP exit takes ~10ms, this can have an adverse
> effect on throughput and interactivity.
> Set the ELP timeout to the dyn-ps timeout. This period is longer and
> avoids the above problem. It also makes sense to stay out of ELP while
> we are awake on the network, to minimize delays in Tx/Rx. The same thing
> was done by the mac80211 dynamic-ps mechanism before the FW DPS changes.
> 
> Signed-off-by: Arik Nemtsov <arik@wizery.com>
> ---

Applied this whole series with the change from WARN_ON to WARN_ON_ONCE
on patch 3/7 and pushed.

Thanks, Arik!

-- 
Cheers,
Luca.

Re: [PATCH 3/7] wl12xx: reset link Tx queues when freeing it

[Kalle Valo]

Luciano Coelho <coelho@ti.com> writes:

>> @@ -973,8 +974,14 @@ void wl12xx_tx_reset(struct wl1271 *wl, bool reset_tx_queues)
>>  	struct sk_buff *skb;
>>  	struct ieee80211_tx_info *info;
>>  
>> -	for (i = 0; i < NUM_TX_QUEUES; i++)
>> -		wl->tx_queue_count[i] = 0;
>> +	/* only reset the queues if something bad happened */
>> +	if (WARN_ON(wl1271_tx_total_queue_count(wl) != 0)) {
>
> Let's make all these WARN_ONs as WARN_ON_ONCE as it is recommended
> nowadays.  This will make Kalle Valo happier. :)

Hehe. Actually the dislike against WARN_ON() was started by John, I
again started the war to killing BUG_ON() :)

But nevertheless, this changes makes also me happy. I have seen cases
where the whole system reboots because of WARN_ON() spam, so
WARN_ON_ONCE() is much better in in cases where's a possibility that it
will get printed a lot. Of course printk_ratelimit() is another option,
but for some reason it isn't that popular.

-- 
Kalle Valo

Re: [PATCH 3/7] wl12xx: reset link Tx queues when freeing it

[Luciano Coelho]

On Wed, 2012-02-29 at 10:31 +0200, Kalle Valo wrote: 
> Luciano Coelho <coelho@ti.com> writes:
> 
> >> @@ -973,8 +974,14 @@ void wl12xx_tx_reset(struct wl1271 *wl, bool reset_tx_queues)
> >>  	struct sk_buff *skb;
> >>  	struct ieee80211_tx_info *info;
> >>  
> >> -	for (i = 0; i < NUM_TX_QUEUES; i++)
> >> -		wl->tx_queue_count[i] = 0;
> >> +	/* only reset the queues if something bad happened */
> >> +	if (WARN_ON(wl1271_tx_total_queue_count(wl) != 0)) {
> >
> > Let's make all these WARN_ONs as WARN_ON_ONCE as it is recommended
> > nowadays.  This will make Kalle Valo happier. :)
> 
> Hehe. Actually the dislike against WARN_ON() was started by John, I
> again started the war to killing BUG_ON() :)

Right, John's email was where I got the "recommended" from. ;)


> But nevertheless, this changes makes also me happy. I have seen cases
> where the whole system reboots because of WARN_ON() spam, so
> WARN_ON_ONCE() is much better in in cases where's a possibility that it
> will get printed a lot. Of course printk_ratelimit() is another option,
> but for some reason it isn't that popular.

Yeah, I totally agree.  Especially when we have a loop, WARN_ON_ONCE
should be sufficient, in my opinion.

-- 
Cheers,
Luca.