[PATCH 0/4] wl1271: TX optimizations & fixes

[PATCH 0/4] wl1271: TX optimizations & fixes

[Ido Yariv]

The following patches fix some issues in the TX path, as well as optimize it.

The first patch fixes an issue in wl1271_tx_work. In case the aggregation
buffer is completely filled, the content of the buffer is transferred and no
more packets are sent. Fixed by flushing the buffer and continue aggregating
packets.

The second patch solves a TX starvation issue in wl1271_irq_work. Since TX is
handled by wl1271_tx_work, packets are transmitted after all interrupts are
handled in wl1271_irq_work. Since these include TX completion interrupts the
FW status might be read multiple times needlessly, which could hurt performance.

The third patch is more of a cosmetic change. Instead of traversing the array
of TX descriptors in order to find a free entry, use a bitmap for that
purpose.

The last patch fixes an issue with the TX queue low watermark. The number of
items in the TX queue is checked against the low watermark in
wl1271_tx_complete. However, the fact that a TX completion interrupt was fired
does not necessarily mean that there are any less skbs in the TX queue. Fixed
by moving the handling logic to the TX work, after skbs are actually dequeued.

These patches were tested on a Zoom2 platform (SDIO only). While throughput in
RX scenarios was hardly affected, throughput in TX scenarios was significantly
improved.

Ido Yariv (4):
  wl1271: TX aggregation optimization
  wl1271: Fix TX starvation
  wl1271: Allocate TX descriptors more efficiently
  wl1271: Fix TX queue low watermark handling

 drivers/net/wireless/wl12xx/wl1271.h      |    1 +
 drivers/net/wireless/wl12xx/wl1271_main.c |   20 ++++--
 drivers/net/wireless/wl12xx/wl1271_tx.c   |  108 +++++++++++++++++++----------
 drivers/net/wireless/wl12xx/wl1271_tx.h   |    1 +
 4 files changed, 87 insertions(+), 43 deletions(-)

[PATCH 1/4] wl1271: TX aggregation optimization

[Ido Yariv]

In case the aggregation buffer is too small to hold all available packets,
the buffer is transferred to the FW and no more packets are aggregated.
Although there may be enough available TX blocks, no additional packets will
be handled by the current TX work.

Fix this by flushing the aggregation buffer when it's full, and continue
transferring packets as long as there are enough available TX blocks.

Signed-off-by: Ido Yariv <ido@wizery.com>
---
 drivers/net/wireless/wl12xx/wl1271_tx.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_tx.c b/drivers/net/wireless/wl12xx/wl1271_tx.c
index e3dc13c..63bc52c 100644
--- a/drivers/net/wireless/wl12xx/wl1271_tx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_tx.c
@@ -52,7 +52,7 @@ static int wl1271_tx_allocate(struct wl1271 *wl, struct sk_buff *skb, u32 extra,
 	int id, ret = -EBUSY;
 
 	if (buf_offset + total_len > WL1271_AGGR_BUFFER_SIZE)
-		return -EBUSY;
+		return -EAGAIN;
 
 	/* allocate free identifier for the packet */
 	id = wl1271_tx_id(wl, skb);
@@ -211,6 +211,7 @@ void wl1271_tx_work(struct work_struct *work)
 	bool woken_up = false;
 	u32 sta_rates = 0;
 	u32 buf_offset;
+	bool sent_packets = false;
 	int ret;
 
 	/* check if the rates supported by the AP have changed */
@@ -245,10 +246,20 @@ void wl1271_tx_work(struct work_struct *work)
 		}
 
 		ret = wl1271_prepare_tx_frame(wl, skb, buf_offset);
-		if (ret == -EBUSY) {
+		if (ret == -EAGAIN) {
 			/*
-			 * Either the firmware buffer is full, or the
-			 * aggregation buffer is.
+			 * Aggregation buffer is full.
+			 * Flush buffer and try again.
+			 */
+			skb_queue_head(&wl->tx_queue, skb);
+			wl1271_write(wl, WL1271_SLV_MEM_DATA, wl->aggr_buf,
+				buf_offset, true);
+			sent_packets = true;
+			buf_offset = 0;
+			continue;
+		} else if (ret == -EBUSY) {
+			/*
+			 * Firmware buffer is full.
 			 * Queue back last skb, and stop aggregating.
 			 */
 			skb_queue_head(&wl->tx_queue, skb);
@@ -265,6 +276,9 @@ out_ack:
 	if (buf_offset) {
 		wl1271_write(wl, WL1271_SLV_MEM_DATA, wl->aggr_buf,
 				buf_offset, true);
+		sent_packets = true;
+	}
+	if (sent_packets) {
 		/* interrupt the firmware with the new packets */
 		wl1271_write32(wl, WL1271_HOST_WR_ACCESS, wl->tx_packets_count);
 	}
-- 
1.7.0.4

[PATCH 2/4] wl1271: Fix TX starvation

[Ido Yariv]

While wl1271_irq_work handles RX directly (by calling wl1271_rx), a different
work is queued for transmitting packets. The IRQ work might handle more than
one interrupt during a single call, including multiple TX completion
interrupts. This might starve TX, since no packets are transmitted until all
interrupts are handled.

Fix this by calling the TX work function directly, instead of deferring
it.

Signed-off-by: Ido Yariv <ido@wizery.com>
---
 drivers/net/wireless/wl12xx/wl1271_main.c |   19 +++++++++++++------
 drivers/net/wireless/wl12xx/wl1271_tx.c   |   12 ++++++++----
 drivers/net/wireless/wl12xx/wl1271_tx.h   |    1 +
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index 48a4b99..5643834 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -458,7 +458,6 @@ static void wl1271_fw_status(struct wl1271 *wl,
 			     struct wl1271_fw_status *status)
 {
 	struct timespec ts;
-	u32 total = 0;
 	int i;
 
 	wl1271_raw_read(wl, FW_STATUS_ADDR, status, sizeof(*status), false);
@@ -478,13 +477,8 @@ static void wl1271_fw_status(struct wl1271 *wl,
 		wl->tx_blocks_freed[i] =
 			le32_to_cpu(status->tx_released_blks[i]);
 		wl->tx_blocks_available += cnt;
-		total += cnt;
 	}
 
-	/* if more blocks are available now, schedule some tx work */
-	if (total && !skb_queue_empty(&wl->tx_queue))
-		ieee80211_queue_work(wl->hw, &wl->tx_work);
-
 	/* update the host-chipset time offset */
 	getnstimeofday(&ts);
 	wl->time_offset = (timespec_to_ns(&ts) >> 10) -
@@ -499,6 +493,7 @@ static void wl1271_irq_work(struct work_struct *work)
 	u32 intr;
 	int loopcount = WL1271_IRQ_MAX_LOOPS;
 	unsigned long flags;
+	u32 prev_tx_blocks;
 	struct wl1271 *wl =
 		container_of(work, struct wl1271, irq_work);
 
@@ -519,6 +514,7 @@ static void wl1271_irq_work(struct work_struct *work)
 		spin_unlock_irqrestore(&wl->wl_lock, flags);
 		loopcount--;
 
+		prev_tx_blocks = wl->tx_blocks_available;
 		wl1271_fw_status(wl, wl->fw_status);
 		intr = le32_to_cpu(wl->fw_status->intr);
 		if (!intr) {
@@ -537,6 +533,17 @@ static void wl1271_irq_work(struct work_struct *work)
 			    (wl->tx_results_count & 0xff))
 				wl1271_tx_complete(wl);
 
+			/* Check if any tx blocks were freed */
+			if ((wl->tx_blocks_available > prev_tx_blocks) &&
+					!skb_queue_empty(&wl->tx_queue)) {
+				/*
+				 * In order to avoid starvation of the TX path,
+				 * call the work function directly.
+				 */
+				cancel_work_sync(&wl->tx_work);
+				wl1271_tx_work_locked(wl);
+			}
+
 			wl1271_rx(wl, wl->fw_status);
 		}
 
diff --git a/drivers/net/wireless/wl12xx/wl1271_tx.c b/drivers/net/wireless/wl12xx/wl1271_tx.c
index 63bc52c..90a8909 100644
--- a/drivers/net/wireless/wl12xx/wl1271_tx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_tx.c
@@ -204,9 +204,8 @@ u32 wl1271_tx_enabled_rates_get(struct wl1271 *wl, u32 rate_set)
 	return enabled_rates;
 }
 
-void wl1271_tx_work(struct work_struct *work)
+void wl1271_tx_work_locked(struct wl1271 *wl)
 {
-	struct wl1271 *wl = container_of(work, struct wl1271, tx_work);
 	struct sk_buff *skb;
 	bool woken_up = false;
 	u32 sta_rates = 0;
@@ -223,8 +222,6 @@ void wl1271_tx_work(struct work_struct *work)
 		spin_unlock_irqrestore(&wl->wl_lock, flags);
 	}
 
-	mutex_lock(&wl->mutex);
-
 	if (unlikely(wl->state == WL1271_STATE_OFF))
 		goto out;
 
@@ -286,7 +283,14 @@ out_ack:
 out:
 	if (woken_up)
 		wl1271_ps_elp_sleep(wl);
+}
 
+void wl1271_tx_work(struct work_struct *work)
+{
+	struct wl1271 *wl = container_of(work, struct wl1271, tx_work);
+
+	mutex_lock(&wl->mutex);
+	wl1271_tx_work_locked(wl);
 	mutex_unlock(&wl->mutex);
 }
 
diff --git a/drivers/net/wireless/wl12xx/wl1271_tx.h b/drivers/net/wireless/wl12xx/wl1271_tx.h
index d12a129..f1c9065 100644
--- a/drivers/net/wireless/wl12xx/wl1271_tx.h
+++ b/drivers/net/wireless/wl12xx/wl1271_tx.h
@@ -140,6 +140,7 @@ static inline int wl1271_tx_get_queue(int queue)
 }
 
 void wl1271_tx_work(struct work_struct *work);
+void wl1271_tx_work_locked(struct wl1271 *wl);
 void wl1271_tx_complete(struct wl1271 *wl);
 void wl1271_tx_reset(struct wl1271 *wl);
 void wl1271_tx_flush(struct wl1271 *wl);
-- 
1.7.0.4

Re: [PATCH 2/4] wl1271: Fix TX starvation

[Juuso Oikarinen]

On Mon, 2010-10-11 at 10:48 +0200, ext Ido Yariv wrote:
> While wl1271_irq_work handles RX directly (by calling wl1271_rx), a different
> work is queued for transmitting packets. The IRQ work might handle more than
> one interrupt during a single call, including multiple TX completion
> interrupts. This might starve TX, since no packets are transmitted until all
> interrupts are handled.
> 
> Fix this by calling the TX work function directly, instead of deferring
> it.
> 
> Signed-off-by: Ido Yariv <ido@wizery.com>
> ---
>  drivers/net/wireless/wl12xx/wl1271_main.c |   19 +++++++++++++------
>  drivers/net/wireless/wl12xx/wl1271_tx.c   |   12 ++++++++----
>  drivers/net/wireless/wl12xx/wl1271_tx.h   |    1 +
>  3 files changed, 22 insertions(+), 10 deletions(-)
> 

*snip*


> @@ -537,6 +533,17 @@ static void wl1271_irq_work(struct work_struct *work)
>  			    (wl->tx_results_count & 0xff))
>  				wl1271_tx_complete(wl);
>  
> +			/* Check if any tx blocks were freed */
> +			if ((wl->tx_blocks_available > prev_tx_blocks) &&
> +					!skb_queue_empty(&wl->tx_queue)) {
> +				/*
> +				 * In order to avoid starvation of the TX path,
> +				 * call the work function directly.
> +				 */
> +				cancel_work_sync(&wl->tx_work);

Hmm, isn't this causing a theoretical potential for a dead-lock? The
tx_work could be waiting in mutex-lock already when cancel_work_sync is
called, in which case cancel_work_sync would lock forever.

IIRC the irq_work and tx_work currently run in the same queue, so this
may work with the current driver. Smells like a hazard anyway, and
changing the workqueues for each work could easily lead to dead locks
here. So at minimum I'd like to see it documented in the comment why
this cannot cause a deadlock.

> +				wl1271_tx_work_locked(wl);
> +			}
> +
>  			wl1271_rx(wl, wl->fw_status);
>  		}
>  
> diff --git a/drivers/net/wireless/wl12xx/wl1271_tx.c b/drivers/net/wireless/wl12xx/wl1271_tx.c
> index 63bc52c..90a8909 100644

Re: [PATCH 2/4] wl1271: Fix TX starvation

[Johannes Berg]

On Mon, 2010-10-11 at 12:56 +0300, Juuso Oikarinen wrote:

> > @@ -537,6 +533,17 @@ static void wl1271_irq_work(struct work_struct *work)
> >  			    (wl->tx_results_count & 0xff))
> >  				wl1271_tx_complete(wl);
> >  
> > +			/* Check if any tx blocks were freed */
> > +			if ((wl->tx_blocks_available > prev_tx_blocks) &&
> > +					!skb_queue_empty(&wl->tx_queue)) {
> > +				/*
> > +				 * In order to avoid starvation of the TX path,
> > +				 * call the work function directly.
> > +				 */
> > +				cancel_work_sync(&wl->tx_work);
> 
> Hmm, isn't this causing a theoretical potential for a dead-lock? The
> tx_work could be waiting in mutex-lock already when cancel_work_sync is
> called, in which case cancel_work_sync would lock forever.
> 
> IIRC the irq_work and tx_work currently run in the same queue, so this
> may work with the current driver. Smells like a hazard anyway, and
> changing the workqueues for each work could easily lead to dead locks
> here. So at minimum I'd like to see it documented in the comment why
> this cannot cause a deadlock.

It should still cause a lockdep warning which you may want to avoid ...
and unless I'm looking at the wrong driver, this is in an irqs disabled
section which is wrong as well.

johannes

Re: [PATCH 2/4] wl1271: Fix TX starvation

[Ido Yariv]

Hi Juuso,

You're absolutely right. I had an implicit assumption that both irq_work
and tx_work cannot run concurrently, since they're on the same work
queue.
The reason cancel_work_sync was called in the first place was to
minimize the number of times tx_work is being called without any work to do.
While the impact of simply not cancelling tx_work is quite minor, v2
will include an alternative implementation which tries to achieve the above
goal without calling cancel_work_sync().

Thanks,
Ido.

On Mon, Oct 11, 2010 at 12:56:58PM +0300, Juuso Oikarinen wrote:
> On Mon, 2010-10-11 at 10:48 +0200, ext Ido Yariv wrote:
> > While wl1271_irq_work handles RX directly (by calling wl1271_rx), a different
> > work is queued for transmitting packets. The IRQ work might handle more than
> > one interrupt during a single call, including multiple TX completion
> > interrupts. This might starve TX, since no packets are transmitted until all
> > interrupts are handled.
> > 
> > Fix this by calling the TX work function directly, instead of deferring
> > it.
> > 
> > Signed-off-by: Ido Yariv <ido@wizery.com>
> > ---
> >  drivers/net/wireless/wl12xx/wl1271_main.c |   19 +++++++++++++------
> >  drivers/net/wireless/wl12xx/wl1271_tx.c   |   12 ++++++++----
> >  drivers/net/wireless/wl12xx/wl1271_tx.h   |    1 +
> >  3 files changed, 22 insertions(+), 10 deletions(-)
> > 
> 
> *snip*
> 
> 
> > @@ -537,6 +533,17 @@ static void wl1271_irq_work(struct work_struct *work)
> >  			    (wl->tx_results_count & 0xff))
> >  				wl1271_tx_complete(wl);
> >  
> > +			/* Check if any tx blocks were freed */
> > +			if ((wl->tx_blocks_available > prev_tx_blocks) &&
> > +					!skb_queue_empty(&wl->tx_queue)) {
> > +				/*
> > +				 * In order to avoid starvation of the TX path,
> > +				 * call the work function directly.
> > +				 */
> > +				cancel_work_sync(&wl->tx_work);
> 
> Hmm, isn't this causing a theoretical potential for a dead-lock? The
> tx_work could be waiting in mutex-lock already when cancel_work_sync is
> called, in which case cancel_work_sync would lock forever.
> 
> IIRC the irq_work and tx_work currently run in the same queue, so this
> may work with the current driver. Smells like a hazard anyway, and
> changing the workqueues for each work could easily lead to dead locks
> here. So at minimum I'd like to see it documented in the comment why
> this cannot cause a deadlock.
> 
> > +				wl1271_tx_work_locked(wl);
> > +			}
> > +
> >  			wl1271_rx(wl, wl->fw_status);
> >  		}
> >  
> > diff --git a/drivers/net/wireless/wl12xx/wl1271_tx.c b/drivers/net/wireless/wl12xx/wl1271_tx.c
> > index 63bc52c..90a8909 100644
> 
> 
> 

[PATCH 3/4] wl1271: Allocate TX descriptors more efficiently

[Ido Yariv]

On each TX descriptor allocation, a free entry is found by traversing the TX
descriptors array.

Improve this by holding a bitmap of all TX descriptors, and using efficient
bit operations to search for free entries.
The bitmap is 32 bits long, assuming that the maximum number of
descriptors (ACX_TX_DESCRIPTORS) will never exceed 32.

Signed-off-by: Ido Yariv <ido@wizery.com>
---
 drivers/net/wireless/wl12xx/wl1271.h      |    1 +
 drivers/net/wireless/wl12xx/wl1271_main.c |    1 +
 drivers/net/wireless/wl12xx/wl1271_tx.c   |   38 ++++++++++++++++------------
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
index 8a4cd76..650fc00 100644
--- a/drivers/net/wireless/wl12xx/wl1271.h
+++ b/drivers/net/wireless/wl12xx/wl1271.h
@@ -397,6 +397,7 @@ struct wl1271 {
 	struct work_struct tx_work;
 
 	/* Pending TX frames */
+	unsigned long tx_frames_map;
 	struct sk_buff *tx_frames[ACX_TX_DESCRIPTORS];
 	int tx_frames_cnt;
 
diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index 5643834..59f1aee 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -2528,6 +2528,7 @@ struct ieee80211_hw *wl1271_alloc_hw(void)
 	wl->sg_enabled = true;
 	wl->hw_pg_ver = -1;
 
+	wl->tx_frames_map = 0;
 	for (i = 0; i < ACX_TX_DESCRIPTORS; i++)
 		wl->tx_frames[i] = NULL;
 
diff --git a/drivers/net/wireless/wl12xx/wl1271_tx.c b/drivers/net/wireless/wl12xx/wl1271_tx.c
index 90a8909..e2ba5a7 100644
--- a/drivers/net/wireless/wl12xx/wl1271_tx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_tx.c
@@ -30,17 +30,26 @@
 #include "wl1271_ps.h"
 #include "wl1271_tx.h"
 
-static int wl1271_tx_id(struct wl1271 *wl, struct sk_buff *skb)
+static int wl1271_alloc_tx_id(struct wl1271 *wl, struct sk_buff *skb)
 {
-	int i;
-	for (i = 0; i < ACX_TX_DESCRIPTORS; i++)
-		if (wl->tx_frames[i] == NULL) {
-			wl->tx_frames[i] = skb;
-			wl->tx_frames_cnt++;
-			return i;
-		}
+	int id;
+
+	id = find_first_zero_bit(&wl->tx_frames_map, ACX_TX_DESCRIPTORS);
+	if (id >= ACX_TX_DESCRIPTORS)
+		return -EBUSY;
+
+	set_bit(id, &wl->tx_frames_map);
+	wl->tx_frames[id] = skb;
+	wl->tx_frames_cnt++;
+	return id;
+}
 
-	return -EBUSY;
+static void wl1271_free_tx_id(struct wl1271 *wl, int id)
+{
+	if (test_and_clear_bit(id, &wl->tx_frames_map)) {
+		wl->tx_frames[id] = NULL;
+		wl->tx_frames_cnt--;
+	}
 }
 
 static int wl1271_tx_allocate(struct wl1271 *wl, struct sk_buff *skb, u32 extra,
@@ -55,7 +64,7 @@ static int wl1271_tx_allocate(struct wl1271 *wl, struct sk_buff *skb, u32 extra,
 		return -EAGAIN;
 
 	/* allocate free identifier for the packet */
-	id = wl1271_tx_id(wl, skb);
+	id = wl1271_alloc_tx_id(wl, skb);
 	if (id < 0)
 		return id;
 
@@ -79,8 +88,7 @@ static int wl1271_tx_allocate(struct wl1271 *wl, struct sk_buff *skb, u32 extra,
 			     "tx_allocate: size: %d, blocks: %d, id: %d",
 			     total_len, total_blocks, id);
 	} else {
-		wl->tx_frames[id] = NULL;
-		wl->tx_frames_cnt--;
+		wl1271_free_tx_id(wl, id);
 	}
 
 	return ret;
@@ -353,8 +361,7 @@ static void wl1271_tx_complete_packet(struct wl1271 *wl,
 
 	/* return the packet to the stack */
 	ieee80211_tx_status(wl->hw, skb);
-	wl->tx_frames[result->id] = NULL;
-	wl->tx_frames_cnt--;
+	wl1271_free_tx_id(wl, result->id);
 }
 
 /* Called upon reception of a TX complete interrupt */
@@ -423,11 +430,10 @@ void wl1271_tx_reset(struct wl1271 *wl)
 	for (i = 0; i < ACX_TX_DESCRIPTORS; i++)
 		if (wl->tx_frames[i] != NULL) {
 			skb = wl->tx_frames[i];
-			wl->tx_frames[i] = NULL;
+			wl1271_free_tx_id(wl, i);
 			wl1271_debug(DEBUG_TX, "freeing skb 0x%p", skb);
 			ieee80211_tx_status(wl->hw, skb);
 		}
-	wl->tx_frames_cnt = 0;
 }
 
 #define WL1271_TX_FLUSH_TIMEOUT 500000
-- 
1.7.0.4

[PATCH 4/4] wl1271: Fix TX queue low watermark handling

[Ido Yariv]

The number of entries in the TX queue is compared to the low watermark
value each time TX completion interrupts are handled.
However, the fact that a TX completion arrived does not necessarily mean
there are any less skbs in the TX queue.

In addition, a TX completion interrupt does not necessarily mean that there
are any new available TX blocks. Thus, queuing TX work when the low
watermark is reached might not be needed.

Fix this by moving the low watermark handling to the TX work function,
and avoid queuing TX work in this case.

Signed-off-by: Ido Yariv <ido@wizery.com>
---
 drivers/net/wireless/wl12xx/wl1271_tx.c |   36 +++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_tx.c b/drivers/net/wireless/wl12xx/wl1271_tx.c
index e2ba5a7..a22ebb2 100644
--- a/drivers/net/wireless/wl12xx/wl1271_tx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_tx.c
@@ -212,6 +212,20 @@ u32 wl1271_tx_enabled_rates_get(struct wl1271 *wl, u32 rate_set)
 	return enabled_rates;
 }
 
+static void handle_tx_low_watermark(struct wl1271 *wl)
+{
+	unsigned long flags;
+
+	if (test_bit(WL1271_FLAG_TX_QUEUE_STOPPED, &wl->flags) &&
+	    skb_queue_len(&wl->tx_queue) <= WL1271_TX_QUEUE_LOW_WATERMARK) {
+		/* firmware buffer has space, restart queues */
+		spin_lock_irqsave(&wl->wl_lock, flags);
+		ieee80211_wake_queues(wl->hw);
+		clear_bit(WL1271_FLAG_TX_QUEUE_STOPPED, &wl->flags);
+		spin_unlock_irqrestore(&wl->wl_lock, flags);
+	}
+}
+
 void wl1271_tx_work_locked(struct wl1271 *wl)
 {
 	struct sk_buff *skb;
@@ -225,6 +239,7 @@ void wl1271_tx_work_locked(struct wl1271 *wl)
 	if (unlikely(test_and_clear_bit(WL1271_FLAG_STA_RATES_CHANGED,
 					&wl->flags))) {
 		unsigned long flags;
+
 		spin_lock_irqsave(&wl->wl_lock, flags);
 		sta_rates = wl->sta_rate_set;
 		spin_unlock_irqrestore(&wl->wl_lock, flags);
@@ -289,6 +304,8 @@ out_ack:
 	}
 
 out:
+	handle_tx_low_watermark(wl);
+
 	if (woken_up)
 		wl1271_ps_elp_sleep(wl);
 }
@@ -400,19 +417,6 @@ void wl1271_tx_complete(struct wl1271 *wl)
 
 		wl->tx_results_count++;
 	}
-
-	if (test_bit(WL1271_FLAG_TX_QUEUE_STOPPED, &wl->flags) &&
-	    skb_queue_len(&wl->tx_queue) <= WL1271_TX_QUEUE_LOW_WATERMARK) {
-		unsigned long flags;
-
-		/* firmware buffer has space, restart queues */
-		wl1271_debug(DEBUG_TX, "tx_complete: waking queues");
-		spin_lock_irqsave(&wl->wl_lock, flags);
-		ieee80211_wake_queues(wl->hw);
-		clear_bit(WL1271_FLAG_TX_QUEUE_STOPPED, &wl->flags);
-		spin_unlock_irqrestore(&wl->wl_lock, flags);
-		ieee80211_queue_work(wl->hw, &wl->tx_work);
-	}
 }
 
 /* caller must hold wl->mutex */
@@ -427,6 +431,12 @@ void wl1271_tx_reset(struct wl1271 *wl)
 		ieee80211_tx_status(wl->hw, skb);
 	}
 
+	/*
+	 * Make sure the driver is at a consistent state, in case this
+	 * function is called from a context other than interface removal.
+	 */
+	handle_tx_low_watermark(wl);
+
 	for (i = 0; i < ACX_TX_DESCRIPTORS; i++)
 		if (wl->tx_frames[i] != NULL) {
 			skb = wl->tx_frames[i];
-- 
1.7.0.4