linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] rt2x00: use timeout in rt2x00usb_vendor_request
@ 2014-11-26 14:29 Stanislaw Gruszka
  2014-11-26 14:29 ` [PATCH 2/4] rt2x00: change REGISTER_BUSY_COUNT for USB Stanislaw Gruszka
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Stanislaw Gruszka @ 2014-11-26 14:29 UTC (permalink / raw)
  To: linux-wireless; +Cc: users, Richard Genoud

Use provided timeout value in rt2x00usb_vendor_request() instead
of iterating REGISTER_BUSY_COUNT times.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/rt2x00/rt2x00usb.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index dc85d3e..258e2a8 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -42,31 +42,27 @@ int rt2x00usb_vendor_request(struct rt2x00_dev *rt2x00dev,
 {
 	struct usb_device *usb_dev = to_usb_device_intf(rt2x00dev->dev);
 	int status;
-	unsigned int i;
 	unsigned int pipe =
 	    (requesttype == USB_VENDOR_REQUEST_IN) ?
 	    usb_rcvctrlpipe(usb_dev, 0) : usb_sndctrlpipe(usb_dev, 0);
+	unsigned long expire = jiffies + msecs_to_jiffies(timeout);
 
 	if (!test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
 		return -ENODEV;
 
-	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
+	do {
 		status = usb_control_msg(usb_dev, pipe, request, requesttype,
 					 value, offset, buffer, buffer_length,
-					 timeout);
+					 timeout / 2);
 		if (status >= 0)
 			return 0;
 
-		/*
-		 * Check for errors
-		 * -ENODEV: Device has disappeared, no point continuing.
-		 * All other errors: Try again.
-		 */
-		else if (status == -ENODEV) {
+		if (status == -ENODEV) {
+			/* Device has disappeared. */
 			clear_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags);
 			break;
 		}
-	}
+	} while (time_before(jiffies, expire));
 
 	/* If the port is powered down, we get a -EPROTO error, and this
 	 * leads to a endless loop. So just say that the device is gone.
-- 
1.8.3.1


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

* [PATCH 2/4] rt2x00: change REGISTER_BUSY_COUNT for USB
  2014-11-26 14:29 [PATCH 1/4] rt2x00: use timeout in rt2x00usb_vendor_request Stanislaw Gruszka
@ 2014-11-26 14:29 ` Stanislaw Gruszka
  2014-11-26 14:29 ` [PATCH 3/4] rt2x00: change REGISTER_TIMEOUT Stanislaw Gruszka
  2014-11-26 14:29 ` [PATCH 4/4] Revert "rt2x00: Endless loop on hub port power down" Stanislaw Gruszka
  2 siblings, 0 replies; 7+ messages in thread
From: Stanislaw Gruszka @ 2014-11-26 14:29 UTC (permalink / raw)
  To: linux-wireless; +Cc: users, Richard Genoud

Because of delays on USB we do not have to iterate so many times on
USB hardware when waiting for H/W register become valid.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/rt2x00/rt2500usb.c | 8 ++++----
 drivers/net/wireless/rt2x00/rt2x00.h    | 5 ++++-
 drivers/net/wireless/rt2x00/rt2x00usb.c | 2 +-
 drivers/net/wireless/rt2x00/rt73usb.c   | 2 +-
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index c878e3f..05c6459 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -47,7 +47,7 @@ MODULE_PARM_DESC(nohwcrypt, "Disable hardware encryption.");
  * BBP and RF register require indirect register access,
  * and use the CSR registers BBPCSR and RFCSR to achieve this.
  * These indirect registers work with busy bits,
- * and we will try maximal REGISTER_BUSY_COUNT times to access
+ * and we will try maximal REGISTER_USB_BUSY_COUNT times to access
  * the register while taking a REGISTER_BUSY_DELAY us delay
  * between each attampt. When the busy bit is still set at that time,
  * the access attempt is considered to have failed,
@@ -122,7 +122,7 @@ static int rt2500usb_regbusy_read(struct rt2x00_dev *rt2x00dev,
 {
 	unsigned int i;
 
-	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
+	for (i = 0; i < REGISTER_USB_BUSY_COUNT; i++) {
 		rt2500usb_register_read_lock(rt2x00dev, offset, reg);
 		if (!rt2x00_get_field16(*reg, field))
 			return 1;
@@ -904,7 +904,7 @@ static int rt2500usb_wait_bbp_ready(struct rt2x00_dev *rt2x00dev)
 	unsigned int i;
 	u8 value;
 
-	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
+	for (i = 0; i < REGISTER_USB_BUSY_COUNT; i++) {
 		rt2500usb_bbp_read(rt2x00dev, 0, &value);
 		if ((value != 0xff) && (value != 0x00))
 			return 0;
@@ -1023,7 +1023,7 @@ static int rt2500usb_set_state(struct rt2x00_dev *rt2x00dev,
 	 * We must wait until the register indicates that the
 	 * device has entered the correct state.
 	 */
-	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
+	for (i = 0; i < REGISTER_USB_BUSY_COUNT; i++) {
 		rt2500usb_register_read(rt2x00dev, MAC_CSR17, &reg2);
 		bbp_state = rt2x00_get_field16(reg2, MAC_CSR17_BBP_CURR_STATE);
 		rf_state = rt2x00_get_field16(reg2, MAC_CSR17_RF_CURR_STATE);
diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index d13f25c..361fcad 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -1019,9 +1019,12 @@ struct rt2x00_bar_list_entry {
  * Register defines.
  * Some registers require multiple attempts before success,
  * in those cases REGISTER_BUSY_COUNT attempts should be
- * taken with a REGISTER_BUSY_DELAY interval.
+ * taken with a REGISTER_BUSY_DELAY interval. Due to USB
+ * bus delays, we do not have to loop so many times to wait
+ * for valid register value on that bus.
  */
 #define REGISTER_BUSY_COUNT	100
+#define REGISTER_USB_BUSY_COUNT 20
 #define REGISTER_BUSY_DELAY	100
 
 /*
diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index 258e2a8..c2346f8 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -150,7 +150,7 @@ int rt2x00usb_regbusy_read(struct rt2x00_dev *rt2x00dev,
 	if (!test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
 		return -ENODEV;
 
-	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
+	for (i = 0; i < REGISTER_USB_BUSY_COUNT; i++) {
 		rt2x00usb_register_read_lock(rt2x00dev, offset, reg);
 		if (!rt2x00_get_field32(*reg, field))
 			return 1;
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index 95724ff..a5458cf 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -1295,7 +1295,7 @@ static int rt73usb_wait_bbp_ready(struct rt2x00_dev *rt2x00dev)
 	unsigned int i;
 	u8 value;
 
-	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
+	for (i = 0; i < REGISTER_USB_BUSY_COUNT; i++) {
 		rt73usb_bbp_read(rt2x00dev, 0, &value);
 		if ((value != 0xff) && (value != 0x00))
 			return 0;
-- 
1.8.3.1


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

* [PATCH 3/4] rt2x00: change REGISTER_TIMEOUT
  2014-11-26 14:29 [PATCH 1/4] rt2x00: use timeout in rt2x00usb_vendor_request Stanislaw Gruszka
  2014-11-26 14:29 ` [PATCH 2/4] rt2x00: change REGISTER_BUSY_COUNT for USB Stanislaw Gruszka
@ 2014-11-26 14:29 ` Stanislaw Gruszka
  2014-11-26 14:29 ` [PATCH 4/4] Revert "rt2x00: Endless loop on hub port power down" Stanislaw Gruszka
  2 siblings, 0 replies; 7+ messages in thread
From: Stanislaw Gruszka @ 2014-11-26 14:29 UTC (permalink / raw)
  To: linux-wireless; +Cc: users, Richard Genoud

Waiting 500ms for register access is too long, decrease this value
to 100ms.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/rt2x00/rt2x00usb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.h b/drivers/net/wireless/rt2x00/rt2x00usb.h
index 819690e..8f85fbd 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.h
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.h
@@ -38,7 +38,7 @@
  * a higher value is required. In that case we use the REGISTER_TIMEOUT_FIRMWARE
  * and EEPROM_TIMEOUT.
  */
-#define REGISTER_TIMEOUT		500
+#define REGISTER_TIMEOUT		100
 #define REGISTER_TIMEOUT_FIRMWARE	1000
 #define EEPROM_TIMEOUT			2000
 
-- 
1.8.3.1


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

* [PATCH 4/4] Revert "rt2x00: Endless loop on hub port power down"
  2014-11-26 14:29 [PATCH 1/4] rt2x00: use timeout in rt2x00usb_vendor_request Stanislaw Gruszka
  2014-11-26 14:29 ` [PATCH 2/4] rt2x00: change REGISTER_BUSY_COUNT for USB Stanislaw Gruszka
  2014-11-26 14:29 ` [PATCH 3/4] rt2x00: change REGISTER_TIMEOUT Stanislaw Gruszka
@ 2014-11-26 14:29 ` Stanislaw Gruszka
  2014-12-02 11:17   ` Richard Genoud
  2 siblings, 1 reply; 7+ messages in thread
From: Stanislaw Gruszka @ 2014-11-26 14:29 UTC (permalink / raw)
  To: linux-wireless; +Cc: users, Richard Genoud

This reverts commit 2ad69ac5976191e9bb7dc4044204a504653ad1bb. It
causes wireless device disappear when we get -EPROTO error form USB
request. I encounter such situation occasionally when resume form
suspend with RT3070 adapter:

[  289.619985] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x0404 with error -71
[  289.639368] ieee80211 phy0: rt2800_wait_bbp_ready: Error - BBP register access failed, aborting
[  289.639374] ieee80211 phy0: rt2800usb_set_device_state: Error - Device failed to enter state 4 (-5)

Without the patch, except printing error, device works just fine after
resume.

Currently after timeouts and REGISTER_BUSY_COUNT tuning, we should
not have any "endless loop", though we can wait quite long when driver
is trying to communicate with the device through non functioning USB
connection. Generally the problem that commit 2ad69ac597619 solves
is kinda artificial.

Cc: Richard Genoud <richard.genoud@gmail.com>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/rt2x00/rt2x00usb.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index c2346f8..892270d 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -64,12 +64,6 @@ int rt2x00usb_vendor_request(struct rt2x00_dev *rt2x00dev,
 		}
 	} while (time_before(jiffies, expire));
 
-	/* If the port is powered down, we get a -EPROTO error, and this
-	 * leads to a endless loop. So just say that the device is gone.
-	 */
-	if (status == -EPROTO)
-		clear_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags);
-
 	rt2x00_err(rt2x00dev,
 		   "Vendor Request 0x%02x failed for offset 0x%04x with error %d\n",
 		   request, offset, status);
-- 
1.8.3.1


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

* Re: [PATCH 4/4] Revert "rt2x00: Endless loop on hub port power down"
  2014-11-26 14:29 ` [PATCH 4/4] Revert "rt2x00: Endless loop on hub port power down" Stanislaw Gruszka
@ 2014-12-02 11:17   ` Richard Genoud
  2014-12-02 12:15     ` Stanislaw Gruszka
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Genoud @ 2014-12-02 11:17 UTC (permalink / raw)
  To: Stanislaw Gruszka, linux-wireless; +Cc: users

On 26/11/2014 15:29, Stanislaw Gruszka wrote:
> This reverts commit 2ad69ac5976191e9bb7dc4044204a504653ad1bb. It
> causes wireless device disappear when we get -EPROTO error form USB
> request. I encounter such situation occasionally when resume form
> suspend with RT3070 adapter:
> 
> [  289.619985] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x0404 with error -71
> [  289.639368] ieee80211 phy0: rt2800_wait_bbp_ready: Error - BBP register access failed, aborting
> [  289.639374] ieee80211 phy0: rt2800usb_set_device_state: Error - Device failed to enter state 4 (-5)
> 
> Without the patch, except printing error, device works just fine after
> resume.
> 
> Currently after timeouts and REGISTER_BUSY_COUNT tuning, we should
> not have any "endless loop", though we can wait quite long when driver
> is trying to communicate with the device through non functioning USB
> connection. Generally the problem that commit 2ad69ac597619 solves
> is kinda artificial.
> 
> Cc: Richard Genoud <richard.genoud@gmail.com>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/rt2x00/rt2x00usb.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
> index c2346f8..892270d 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
> @@ -64,12 +64,6 @@ int rt2x00usb_vendor_request(struct rt2x00_dev *rt2x00dev,
>  		}
>  	} while (time_before(jiffies, expire));
>  
> -	/* If the port is powered down, we get a -EPROTO error, and this
> -	 * leads to a endless loop. So just say that the device is gone.
> -	 */
> -	if (status == -EPROTO)
> -		clear_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags);
> -
>  	rt2x00_err(rt2x00dev,
>  		   "Vendor Request 0x%02x failed for offset 0x%04x with error %d\n",
>  		   request, offset, status);
> 
It tested this serie but unfortunately, reverting this still caused an infinite loop.
(cf https://lkml.org/lkml/2014/4/3/492 to reproduce)

[  642.007812] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.023437] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.031250] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.046875] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.054687] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.062500] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.078125] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.085937] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.101562] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.117187] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.125000] ieee80211 phy0: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 4 in queue 0
[  642.132812] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.250000] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x1700 with error -71
[  642.359375] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x0438 with error -71
[  642.468750] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x0438 with error -71
[  642.476562] ieee80211 phy0: rt2800usb_watchdog: Warning - TX HW queue 1 timed out, invoke forced kick
[  642.585937] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x0408 with error -71
[  642.695312] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x0408 with error -71
[  642.718750] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.734375] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.742187] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.757812] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.765625] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.773437] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.789062] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.796875] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.812500] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  642.820312] ieee80211 phy0: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 5 in queue 0
[  642.835937] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[...]
So it seems the "forced kick" is not done 

With patches 1-3 applied but not this one, I've got:
[  154.015625] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  154.031250] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  154.039062] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  154.054687] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  154.062500] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  154.070312] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  154.085937] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  154.093750] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  154.109375] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  154.117187] ieee80211 phy1: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 8 in queue 0
[  154.132812] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
[  154.242187] ieee80211 phy1: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x0438 with error -71
[  154.250000] ieee80211 phy1: rt2800usb_watchdog: Warning - TX HW queue 0 timed out, invoke forced kick
[  154.765625] cfg80211: Calling CRDA to update world regulatory domain
[  155.710937] wlan0: authenticate with 00:17:9a:84:fb:94
[  155.718750] wlan0: send auth to 00:17:9a:84:fb:94 (try 1/3)
[  155.726562] wlan0: send auth to 00:17:9a:84:fb:94 (try 2/3)
[  155.734375] wlan0: send auth to 00:17:9a:84:fb:94 (try 3/3)
[  155.742187] wlan0: authentication with 00:17:9a:84:fb:94 timed out
Here, the forced kick is done.

Richard



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

* Re: [PATCH 4/4] Revert "rt2x00: Endless loop on hub port power down"
  2014-12-02 11:17   ` Richard Genoud
@ 2014-12-02 12:15     ` Stanislaw Gruszka
  2014-12-02 16:35       ` Richard Genoud
  0 siblings, 1 reply; 7+ messages in thread
From: Stanislaw Gruszka @ 2014-12-02 12:15 UTC (permalink / raw)
  To: Richard Genoud; +Cc: linux-wireless, users

On Tue, Dec 02, 2014 at 12:17:57PM +0100, Richard Genoud wrote:
> It tested this serie but unfortunately, reverting this still caused an infinite loop.
> (cf https://lkml.org/lkml/2014/4/3/492 to reproduce)

It is possible to disable internal hub? It fails here, but perhaps I do
not have compiled proper options in the kernel:

[stasiu@localhost Downloads]$ lsusb -t
/:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/3p, 480M
    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/8p, 480M
/:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/3p, 480M
    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/6p, 480M
        |__ Port 3: Dev 3, If 0, Class=Vendor Specific Class, Driver=, 12M
        |__ Port 4: Dev 4, If 0, Class=Vendor Specific Class, Driver=btusb, 12M
        |__ Port 4: Dev 4, If 1, Class=Vendor Specific Class, Driver=btusb, 12M
        |__ Port 4: Dev 4, If 2, Class=Vendor Specific Class, Driver=, 12M
        |__ Port 4: Dev 4, If 3, Class=Application Specific Interface, Driver=, 12M
        |__ Port 6: Dev 5, If 0, Class=Video, Driver=uvcvideo, 480M
        |__ Port 6: Dev 5, If 1, Class=Video, Driver=uvcvideo, 480M
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 480M
    |__ Port 2: Dev 2, If 0, Class=Vendor Specific Class, Driver=rt2800usb, 480M
    |__ Port 3: Dev 3, If 0, Class=Human Interface Device, Driver=usbhid, 12M
[stasiu@localhost Downloads]$ sudo ./hub-ctrl -b 1 -d 1 -P 1 -p 0
Device not found.
[stasiu@localhost Downloads]$ sudo ./hub-ctrl -b 1 -d 2 -P 2 -p 0
Device not found.

> [  642.476562] ieee80211 phy0: rt2800usb_watchdog: Warning - TX HW queue 1 timed out, invoke forced kick
> [  642.585937] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x0408 with error -71
> [  642.695312] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x0408 with error -71
> [  642.796875] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
> [...]
> So it seems the "forced kick" is not done 

"Forced kick" does not mean to remove device, it means restarting
hardware queue. It is not done, because it requires write to PBF_CFG
register (0x0408), which is not possible.

I do not see an "infinite" loop. What I can see is continues failures
when sending requests to to the hardware. I consider this as proper
behaviour, taking that USB layer continuously return -EPROTO error. If
for example there will be not possible to down interface or remove
rt2800usb module in such condition, I would consider this as a minor
bug, but I'm not sure if that happen or not.

Stanislaw

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

* Re: [PATCH 4/4] Revert "rt2x00: Endless loop on hub port power down"
  2014-12-02 12:15     ` Stanislaw Gruszka
@ 2014-12-02 16:35       ` Richard Genoud
  0 siblings, 0 replies; 7+ messages in thread
From: Richard Genoud @ 2014-12-02 16:35 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: linux-wireless, users

2014-12-02 13:15 GMT+01:00 Stanislaw Gruszka <sgruszka@redhat.com>:
> On Tue, Dec 02, 2014 at 12:17:57PM +0100, Richard Genoud wrote:
>> It tested this serie but unfortunately, reverting this still caused an infinite loop.
>> (cf https://lkml.org/lkml/2014/4/3/492 to reproduce)
>
> It is possible to disable internal hub? It fails here, but perhaps I do
> not have compiled proper options in the kernel:
>
> [stasiu@localhost Downloads]$ lsusb -t
> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/3p, 480M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/8p, 480M
> /:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/3p, 480M
>     |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/6p, 480M
>         |__ Port 3: Dev 3, If 0, Class=Vendor Specific Class, Driver=, 12M
>         |__ Port 4: Dev 4, If 0, Class=Vendor Specific Class, Driver=btusb, 12M
>         |__ Port 4: Dev 4, If 1, Class=Vendor Specific Class, Driver=btusb, 12M
>         |__ Port 4: Dev 4, If 2, Class=Vendor Specific Class, Driver=, 12M
>         |__ Port 4: Dev 4, If 3, Class=Application Specific Interface, Driver=, 12M
>         |__ Port 6: Dev 5, If 0, Class=Video, Driver=uvcvideo, 480M
>         |__ Port 6: Dev 5, If 1, Class=Video, Driver=uvcvideo, 480M
> /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
> /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 480M
>     |__ Port 2: Dev 2, If 0, Class=Vendor Specific Class, Driver=rt2800usb, 480M
>     |__ Port 3: Dev 3, If 0, Class=Human Interface Device, Driver=usbhid, 12M
> [stasiu@localhost Downloads]$ sudo ./hub-ctrl -b 1 -d 1 -P 1 -p 0
> Device not found.
> [stasiu@localhost Downloads]$ sudo ./hub-ctrl -b 1 -d 2 -P 2 -p 0
> Device not found.
I've just tried on my machine, and it doesn't work on root_hub.
root@lnx-rg:~$ lsusb -t
/:  Bus 08.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/6p, 480M
    |__ Port 3: Dev 2, If 0, Class=hub, Driver=hub/4p, 480M
        |__ Port 1: Dev 3, If 0, Class=HID, Driver=usbhid, 1.5M
        |__ Port 4: Dev 4, If 0, Class=vend., Driver=ftdi_sio, 12M
/:  Bus 07.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/6p, 480M
    |__ Port 4: Dev 2, If 0, Class=stor., Driver=usb-storage, 480M
    |__ Port 5: Dev 10, If 0, Class=hub, Driver=hub/4p, 480M
        |__ Port 1: Dev 11, If 0, Class=vend., Driver=rt2800usb, 480M
    |__ Port 6: Dev 12, If 0, Class=vend., Driver=rt2800usb, 480M
/:  Bus 06.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 05.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
root@lnx-rg:~$ /home/rgenoud/bin/hub-ctrl -b 7 -d 1 -P 6 -p 0
Device not found.

but with an external hub: (not every usb hubs have individual port
power management)
root@lnx-rg:~$ lsusb -t
/:  Bus 08.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/6p, 480M
    |__ Port 3: Dev 2, If 0, Class=hub, Driver=hub/4p, 480M
        |__ Port 1: Dev 3, If 0, Class=HID, Driver=usbhid, 1.5M
        |__ Port 4: Dev 4, If 0, Class=vend., Driver=ftdi_sio, 12M
/:  Bus 07.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/6p, 480M
    |__ Port 4: Dev 2, If 0, Class=stor., Driver=usb-storage, 480M
    |__ Port 5: Dev 10, If 0, Class=hub, Driver=hub/4p, 480M
        |__ Port 1: Dev 11, If 0, Class=vend., Driver=rt2800usb, 480M
/:  Bus 06.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 05.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=uhci_hcd/2p, 12M
root@lnx-rg:~$ /home/rgenoud/bin/hub-ctrl -b 7 -d 10 -P 1 -p 0


>> [  642.476562] ieee80211 phy0: rt2800usb_watchdog: Warning - TX HW queue 1 timed out, invoke forced kick
>> [  642.585937] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x0408 with error -71
>> [  642.695312] ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x0408 with error -71
>> [  642.796875] ieee80211 phy0: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71
>> [...]
>> So it seems the "forced kick" is not done
>
> "Forced kick" does not mean to remove device, it means restarting
> hardware queue. It is not done, because it requires write to PBF_CFG
> register (0x0408), which is not possible.
>
> I do not see an "infinite" loop. What I can see is continues failures
> when sending requests to to the hardware. I consider this as proper
> behaviour, taking that USB layer continuously return -EPROTO error. If
> for example there will be not possible to down interface or remove
> rt2800usb module in such condition, I would consider this as a minor
> bug, but I'm not sure if that happen or not.
>
> Stanislaw
Understood.
With a:
ip link set wlan0 down
just before shuting down the usb port, I haven't got any more complains.

With the other way around,
the error messages stops when the link is set down:
# strace -r ip link set wlan0 down
     0.000000 execve("/sbin/ip", ["ip", "link", "set", "wlan0",
"down"], [/* 18 vars */]) = 0
     0.010134 uname({sys="Linux", node="LNS", ...}) = 0
     0.010741 brk(0)                    = 0x152000
     0.019857 brk(0x152d20)             = 0x152d20
     0.004385 set_tls(0x1524c0, 0x14c07c, 0, 0x152d20, 0x14d200) = 0
     0.004250 readlink("/proc/self/exe", "/bin/busybox", 4096) = 12
     0.005069 brk(0x173d20)             = 0x173d20
     0.003573 brk(0x174000)             = 0x174000
     0.003200 getuid32()                = 0
     0.001635 socket(PF_INET, SOCK_DGRAM, IPPROTO_IP) = 3
     0.002335 ioctl(3, SIOCGIFFLAGS, {ifr_name="wlan0",
ifr_flags=IFF_UP|IFF_BROADCAST|IFF_MULTICAST}) = 0
     0.004430 ioctl(3, SIOCSIFFLAGS, {ifr_name="wlan0",
ifr_flags=IFF_BROADCAST|IFF_MULTICAST}) = 0
     6.629790 close(3)                  = 0
     0.004894 exit_group(0)             = ?
     0.002010 +++ exited with 0 +++

So I agree with you, it seems like a normal behaviour.

you can add my
Tested-by: Richard Genoud <richard.genoud@gmail.com>

Tanks!
Richard.

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

end of thread, other threads:[~2014-12-02 16:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-26 14:29 [PATCH 1/4] rt2x00: use timeout in rt2x00usb_vendor_request Stanislaw Gruszka
2014-11-26 14:29 ` [PATCH 2/4] rt2x00: change REGISTER_BUSY_COUNT for USB Stanislaw Gruszka
2014-11-26 14:29 ` [PATCH 3/4] rt2x00: change REGISTER_TIMEOUT Stanislaw Gruszka
2014-11-26 14:29 ` [PATCH 4/4] Revert "rt2x00: Endless loop on hub port power down" Stanislaw Gruszka
2014-12-02 11:17   ` Richard Genoud
2014-12-02 12:15     ` Stanislaw Gruszka
2014-12-02 16:35       ` Richard Genoud

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).