linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] rt2x00: Queue flush fix
@ 2019-08-20 22:13 Balakrishna Bandi
  2019-08-21  8:34 ` Stanislaw Gruszka
  0 siblings, 1 reply; 2+ messages in thread
From: Balakrishna Bandi @ 2019-08-20 22:13 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Balakrishna Bandi

Added rt2x00 queue flush fix and beacon frames checks.

Signed-off-by: Balakrishna Bandi <b.balakrishna@globaledgesoft.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800mmio.c  |  2 +-
 drivers/net/wireless/ralink/rt2x00/rt2x00dev.c   |  7 ++--
 drivers/net/wireless/ralink/rt2x00/rt2x00mac.c   | 22 ++++++++++--
 drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 44 +++++++++++++++++++-----
 4 files changed, 61 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
index 110bb39..9964371 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
@@ -566,7 +566,7 @@ void rt2800mmio_queue_init(struct data_queue *queue)
 
 	switch (queue->qid) {
 	case QID_RX:
-		queue->limit = 128;
+		queue->limit = 512;
 		queue->data_size = AGGREGATION_SIZE;
 		queue->desc_size = RXD_DESC_SIZE;
 		queue->winfo_size = rxwi_size;
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index 35414f9..085a41e 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -128,7 +128,8 @@ static void rt2x00lib_intf_scheduled_iter(void *data, u8 *mac,
 
 	if (test_and_clear_bit(DELAYED_UPDATE_BEACON, &intf->delayed_flags)) {
 		mutex_lock(&intf->beacon_skb_mutex);
-		rt2x00queue_update_beacon(rt2x00dev, vif);
+		if (intf->enable_beacon)
+			rt2x00queue_update_beacon(rt2x00dev, vif);
 		mutex_unlock(&intf->beacon_skb_mutex);
 	}
 }
@@ -191,6 +192,7 @@ static void rt2x00lib_beaconupdate_iter(void *data, u8 *mac,
 					struct ieee80211_vif *vif)
 {
 	struct rt2x00_dev *rt2x00dev = data;
+	struct rt2x00_intf *intf = vif_to_intf(vif);
 
 	if (vif->type != NL80211_IFTYPE_AP &&
 	    vif->type != NL80211_IFTYPE_ADHOC &&
@@ -204,7 +206,8 @@ static void rt2x00lib_beaconupdate_iter(void *data, u8 *mac,
 	 * never be called for USB devices.
 	 */
 	WARN_ON(rt2x00_is_usb(rt2x00dev));
-	rt2x00queue_update_beacon(rt2x00dev, vif);
+	if (intf->enable_beacon)
+		rt2x00queue_update_beacon(rt2x00dev, vif);
 }
 
 void rt2x00lib_beacondone(struct rt2x00_dev *rt2x00dev)
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c
index beb20c5..5c424da 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c
@@ -103,6 +103,25 @@ void rt2x00mac_tx(struct ieee80211_hw *hw,
 	 */
 	if (!test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
 		goto exit_free_skb;
+	/* Dirty hack for queue overrun protection,
+	 * if AC_VO/AC_VI/AC_BE is full, use next queue.
+	 * if AC_BK is full use previous queue.
+	 */
+	if (qid < 4) {
+		queue = rt2x00queue_get_tx_queue(rt2x00dev,qid);
+		if (unlikely(rt2x00queue_full(queue))) {
+			switch(qid) {
+				case 0: /* QID_AC_VO */
+				case 1: /* QID_AC_VI */
+				case 2: /* QID_AC_BE */
+					qid++;
+					break;
+				case 3: /* QID_AC_BK */
+					qid--;
+					break;
+			}
+		}
+	}
 
 	/*
 	 * Use the ATIM queue if appropriate and present.
@@ -602,8 +621,7 @@ void rt2x00mac_bss_info_changed(struct ieee80211_hw *hw,
 			 * Upload beacon to the H/W. This is only required on
 			 * USB devices. PCI devices fetch beacons periodically.
 			 */
-			if (rt2x00_is_usb(rt2x00dev))
-				rt2x00queue_update_beacon(rt2x00dev, vif);
+			rt2x00queue_update_beacon(rt2x00dev, vif);
 
 			if (rt2x00dev->intf_beaconing == 1) {
 				/*
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
index 3b6100e..4254811 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
@@ -986,25 +986,37 @@ void rt2x00queue_stop_queue(struct data_queue *queue)
 
 void rt2x00queue_flush_queue(struct data_queue *queue, bool drop)
 {
+	unsigned int i;
+	bool started;
 	bool tx_queue =
 		(queue->qid == QID_AC_VO) ||
 		(queue->qid == QID_AC_VI) ||
 		(queue->qid == QID_AC_BE) ||
 		(queue->qid == QID_AC_BK);
+	mutex_lock(&queue->status_lock);
+	/* If the queue has been started, we must stop it temporarily
+	 * to prevent any new frames to be queued on the device. If
+	 * we are not dropping the pending frames, the queue must
+	 * only be stopped in the software and not the hardware,
+	 * otherwise the queue will never become empty on its own.
+	 */
+	started = test_bit(QUEUE_STARTED, &queue->flags);
+	if (started) {
+		// pause the queue.
+		rt2x00queue_pause_queue(queue);
 
+		/* If we are not supposed to drop any pending
+		 * frames, this means we must force a start (=kick)
+		 * to the queue to make sure the hardware will
+		 * start transmitting.
+		 */
+		if (!drop && tx_queue)
+			queue->rt2x00dev->ops->lib->kick_queue(queue);
+	}
 	if (rt2x00queue_empty(queue))
 		return;
 
 	/*
-	 * If we are not supposed to drop any pending
-	 * frames, this means we must force a start (=kick)
-	 * to the queue to make sure the hardware will
-	 * start transmitting.
-	 */
-	if (!drop && tx_queue)
-		queue->rt2x00dev->ops->lib->kick_queue(queue);
-
-	/*
 	 * Check if driver supports flushing, if that is the case we can
 	 * defer the flushing to the driver. Otherwise we must use the
 	 * alternative which just waits for the queue to become empty.
@@ -1013,11 +1025,25 @@ void rt2x00queue_flush_queue(struct data_queue *queue, bool drop)
 		queue->rt2x00dev->ops->lib->flush_queue(queue, drop);
 
 	/*
+	 * When we don't want to drop any frames, or when
+	 * the driver doesn't fully flush the queue correcly,
+	 * we must wait for the queue to become empty.
+	 */
+	for (i = 0; !rt2x00queue_empty(queue) && i < 10; i++)
+		msleep(10);
+
+	/*
 	 * The queue flush has failed...
 	 */
 	if (unlikely(!rt2x00queue_empty(queue)))
 		rt2x00_warn(queue->rt2x00dev, "Queue %d failed to flush\n",
 			    queue->qid);
+	/*
+	 * Restore the queue to the previous status.
+	 */
+	if (started)
+		rt2x00queue_unpause_queue(queue);
+	mutex_unlock(&queue->status_lock);
 }
 EXPORT_SYMBOL_GPL(rt2x00queue_flush_queue);
 
-- 
1.9.1

Disclaimer:- The information contained in this electronic message and any attachments to this message are intended for the exclusive use of the addressee(s) and may contain proprietary, confidential or privileged information. If you are not the intended recipient, you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately and destroy all copies of this message and any attachments. The views expressed in this E-mail message (including the enclosure/(s) or attachment/(s) if any) are those of the individual sender, except where the sender expressly, and with authority, states them to be the views of GlobalEdge. Before opening any mail and attachments please check them for viruses .GlobalEdge does not accept any liability for virus infected mails.


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH 1/1] rt2x00: Queue flush fix
  2019-08-20 22:13 [PATCH 1/1] rt2x00: Queue flush fix Balakrishna Bandi
@ 2019-08-21  8:34 ` Stanislaw Gruszka
  0 siblings, 0 replies; 2+ messages in thread
From: Stanislaw Gruszka @ 2019-08-21  8:34 UTC (permalink / raw)
  To: Balakrishna Bandi; +Cc: kvalo, linux-wireless

On Wed, Aug 21, 2019 at 03:43:05AM +0530, Balakrishna Bandi wrote:
> Added rt2x00 queue flush fix and beacon frames checks.

Please post separate patch for each issue and provide
more descriptive information about the changes, especially
what problems are intended to solve.

> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
> index 110bb39..9964371 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
> @@ -566,7 +566,7 @@ void rt2800mmio_queue_init(struct data_queue *queue)
>  
>  	switch (queue->qid) {
>  	case QID_RX:
> -		queue->limit = 128;
> +		queue->limit = 512;

How this is related with flush or beaconing ?

At this point of rt2x00 driver development I'm pretty reluctant to
increase queue size. But maybe we can do this for some particular
chip if things were tested on this chip and improve or fix something.

>  	if (!test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
>  		goto exit_free_skb;
> +	/* Dirty hack for queue overrun protection,
> +	 * if AC_VO/AC_VI/AC_BE is full, use next queue.
> +	 * if AC_BK is full use previous queue.
> +	 */

No dirty hacks please.

>  void rt2x00queue_flush_queue(struct data_queue *queue, bool drop)
>  {
> +	unsigned int i;
> +	bool started;
>  	bool tx_queue =
>  		(queue->qid == QID_AC_VO) ||
>  		(queue->qid == QID_AC_VI) ||
>  		(queue->qid == QID_AC_BE) ||
>  		(queue->qid == QID_AC_BK);
> +	mutex_lock(&queue->status_lock);
> +	/* If the queue has been started, we must stop it temporarily
> +	 * to prevent any new frames to be queued on the device. If
> +	 * we are not dropping the pending frames, the queue must
> +	 * only be stopped in the software and not the hardware,
> +	 * otherwise the queue will never become empty on its own.
> +	 */

Since linux 5.2 there is rework done on related area. So maybe flush
issue you are trying to fix by this patch is already fixed. If not let
me know and provide description what is the problem.

> Disclaimer:- The information contained in this electronic message and any attachments to this message are intended for the exclusive use of the addressee(s) and may contain proprietary, confidential or privileged information. If you are not the intended recipient, you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately and destroy all copies of this message and any attachments. The views expressed in this E-mail message (including the enclosure/(s) or attachment/(s) if any) are those of the individual sender, except where the sender expressly, and with authority, states them to be the views of GlobalEdge. Before opening any mail and attachments please check them for viruses .GlobalEdge does not accept any liability for virus infected mails.
> 

You should not sent this disclaimer to open mailing list.
Fix this or use different email service to post patches.

Stanislaw

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-08-21  8:34 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-20 22:13 [PATCH 1/1] rt2x00: Queue flush fix Balakrishna Bandi
2019-08-21  8:34 ` Stanislaw Gruszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).