* [PATCH 2/7] wl12xx: change bits in the link_map under spin lock
2012-02-27 22:41 [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout Arik Nemtsov
@ 2012-02-27 22:41 ` Arik Nemtsov
2012-02-27 22:41 ` [PATCH 3/7] wl12xx: reset link Tx queues when freeing it Arik Nemtsov
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Arik Nemtsov @ 2012-02-27 22:41 UTC (permalink / raw)
To: linux-wireless; +Cc: Luciano Coelho, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/7] wl12xx: reset link Tx queues when freeing it
2012-02-27 22:41 [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout Arik Nemtsov
2012-02-27 22:41 ` [PATCH 2/7] wl12xx: change bits in the link_map under spin lock Arik Nemtsov
@ 2012-02-27 22:41 ` Arik Nemtsov
2012-02-28 9:51 ` Luciano Coelho
2012-02-27 22:41 ` [PATCH 4/7] wl12xx: avoid starving the system hlid Arik Nemtsov
` (4 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Arik Nemtsov @ 2012-02-27 22:41 UTC (permalink / raw)
To: linux-wireless; +Cc: Luciano Coelho, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/7] wl12xx: reset link Tx queues when freeing it
2012-02-27 22:41 ` [PATCH 3/7] wl12xx: reset link Tx queues when freeing it Arik Nemtsov
@ 2012-02-28 9:51 ` Luciano Coelho
2012-02-28 10:09 ` Arik Nemtsov
2012-02-29 8:31 ` Kalle Valo
0 siblings, 2 replies; 12+ messages in thread
From: Luciano Coelho @ 2012-02-28 9:51 UTC (permalink / raw)
To: Arik Nemtsov; +Cc: linux-wireless
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.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/7] wl12xx: reset link Tx queues when freeing it
2012-02-28 9:51 ` Luciano Coelho
@ 2012-02-28 10:09 ` Arik Nemtsov
2012-02-29 8:31 ` Kalle Valo
1 sibling, 0 replies; 12+ messages in thread
From: Arik Nemtsov @ 2012-02-28 10:09 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/7] wl12xx: reset link Tx queues when freeing it
2012-02-28 9:51 ` Luciano Coelho
2012-02-28 10:09 ` Arik Nemtsov
@ 2012-02-29 8:31 ` Kalle Valo
2012-02-29 8:39 ` Luciano Coelho
1 sibling, 1 reply; 12+ messages in thread
From: Kalle Valo @ 2012-02-29 8:31 UTC (permalink / raw)
To: Luciano Coelho; +Cc: Arik Nemtsov, linux-wireless
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/7] wl12xx: reset link Tx queues when freeing it
2012-02-29 8:31 ` Kalle Valo
@ 2012-02-29 8:39 ` Luciano Coelho
0 siblings, 0 replies; 12+ messages in thread
From: Luciano Coelho @ 2012-02-29 8:39 UTC (permalink / raw)
To: Kalle Valo; +Cc: Arik Nemtsov, linux-wireless
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/7] wl12xx: avoid starving the system hlid
2012-02-27 22:41 [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout Arik Nemtsov
2012-02-27 22:41 ` [PATCH 2/7] wl12xx: change bits in the link_map under spin lock Arik Nemtsov
2012-02-27 22:41 ` [PATCH 3/7] wl12xx: reset link Tx queues when freeing it Arik Nemtsov
@ 2012-02-27 22:41 ` Arik Nemtsov
2012-02-27 22:41 ` [PATCH 5/7] wl12xx: flush all Tx queues on tx_flush timeout Arik Nemtsov
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Arik Nemtsov @ 2012-02-27 22:41 UTC (permalink / raw)
To: linux-wireless; +Cc: Luciano Coelho, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/7] wl12xx: flush all Tx queues on tx_flush timeout
2012-02-27 22:41 [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout Arik Nemtsov
` (2 preceding siblings ...)
2012-02-27 22:41 ` [PATCH 4/7] wl12xx: avoid starving the system hlid Arik Nemtsov
@ 2012-02-27 22:41 ` Arik Nemtsov
2012-02-27 22:41 ` [PATCH 6/7] wl12xx: flush Tx during suspend and 802.11h chan switch Arik Nemtsov
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Arik Nemtsov @ 2012-02-27 22:41 UTC (permalink / raw)
To: linux-wireless; +Cc: Luciano Coelho, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/7] wl12xx: flush Tx during suspend and 802.11h chan switch
2012-02-27 22:41 [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout Arik Nemtsov
` (3 preceding siblings ...)
2012-02-27 22:41 ` [PATCH 5/7] wl12xx: flush all Tx queues on tx_flush timeout Arik Nemtsov
@ 2012-02-27 22:41 ` Arik Nemtsov
2012-02-27 22:41 ` [PATCH 7/7] wl12xx: increase dynamic PS timeout to 200ms Arik Nemtsov
2012-02-28 12:05 ` [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout Luciano Coelho
6 siblings, 0 replies; 12+ messages in thread
From: Arik Nemtsov @ 2012-02-27 22:41 UTC (permalink / raw)
To: linux-wireless; +Cc: Luciano Coelho, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/7] wl12xx: increase dynamic PS timeout to 200ms
2012-02-27 22:41 [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout Arik Nemtsov
` (4 preceding siblings ...)
2012-02-27 22:41 ` [PATCH 6/7] wl12xx: flush Tx during suspend and 802.11h chan switch Arik Nemtsov
@ 2012-02-27 22:41 ` Arik Nemtsov
2012-02-28 12:05 ` [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout Luciano Coelho
6 siblings, 0 replies; 12+ messages in thread
From: Arik Nemtsov @ 2012-02-27 22:41 UTC (permalink / raw)
To: linux-wireless; +Cc: Luciano Coelho, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout
2012-02-27 22:41 [PATCH 1/7] wl12xx: set the ELP entry delay to the FW dyn-ps timeout Arik Nemtsov
` (5 preceding siblings ...)
2012-02-27 22:41 ` [PATCH 7/7] wl12xx: increase dynamic PS timeout to 200ms Arik Nemtsov
@ 2012-02-28 12:05 ` Luciano Coelho
6 siblings, 0 replies; 12+ messages in thread
From: Luciano Coelho @ 2012-02-28 12:05 UTC (permalink / raw)
To: Arik Nemtsov; +Cc: linux-wireless
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.
^ permalink raw reply [flat|nested] 12+ messages in thread