Skip to contentSkip to conversation

rt2x00usb: initialize the read value in case of failure

PATCH In review Sebastian Andrzej Siewior 3 participants 12 messages Mar 12, 2015 – Mar 30, 2015 Networking drivers (wireless)

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 rt2x00usb: initialize the read value in case of failure

My understanding ist that rt2x00usb_register_read() is void and so the reader is unaware of read errors and assumes that whatever was on the stack as it was about to read.

---
 drivers/net/wireless/rt2x00/rt2x00usb.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/wireless/rt2x00/rt2x00usb.h +2 -2
index 8f85fbd5f237..569363da00a2 100644
@@ -199,7 +199,7 @@ static inline void rt2x00usb_register_read(struct rt2x00_dev *rt2x00dev,
 					   const unsigned int offset,
 					   u32 *value)
 {
-	__le32 reg;
+	__le32 reg = 0;
 	rt2x00usb_vendor_request_buff(rt2x00dev, USB_MULTI_READ,
 				      USB_VENDOR_REQUEST_IN, offset,
 				      &reg, sizeof(reg));
@@ -219,7 +219,7 @@ static inline void rt2x00usb_register_read_lock(struct rt2x00_dev *rt2x00dev,
 						const unsigned int offset,
 						u32 *value)
 {
-	__le32 reg;
+	__le32 reg = 0;
 	rt2x00usb_vendor_req_buff_lock(rt2x00dev, USB_MULTI_READ,
 				       USB_VENDOR_REQUEST_IN, offset,
 				       &reg, sizeof(reg), REGISTER_TIMEOUT);
Signature · 1 line
2.1.4
#1 lore raw

PATCH 2/3 rt2x00usb: check USB's request error code in rt2800usb_autorun_detect()

rt2800usb_autorun_detect() blindly assumes assumes that it worked while it could have failed. Check the error code here.

---
 drivers/net/wireless/rt2x00/rt2800usb.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/net/wireless/rt2x00/rt2800usb.c +7 -3
index 8444313eabe2..e692e5fe4e01 100644
@@ -233,6 +233,7 @@ static int rt2800usb_autorun_detect(struct rt2x00_dev *rt2x00dev)
 {
 	__le32 *reg;
 	u32 fw_mode;
+	int ret;
 
 	reg = kmalloc(sizeof(*reg), GFP_KERNEL);
 	if (reg == NULL)
@@ -242,11 +243,14 @@ static int rt2800usb_autorun_detect(struct rt2x00_dev *rt2x00dev)
 	 * magic value USB_MODE_AUTORUN (0x11) to the device, thus the
 	 * returned value would be invalid.
 	 */
-	rt2x00usb_vendor_request(rt2x00dev, USB_DEVICE_MODE,
-				 USB_VENDOR_REQUEST_IN, 0, USB_MODE_AUTORUN,
-				 reg, sizeof(*reg), REGISTER_TIMEOUT_FIRMWARE);
+	ret = rt2x00usb_vendor_request(rt2x00dev, USB_DEVICE_MODE,
+				       USB_VENDOR_REQUEST_IN, 0,
+				       USB_MODE_AUTORUN, reg, sizeof(*reg),
+				       REGISTER_TIMEOUT_FIRMWARE);
 	fw_mode = le32_to_cpu(*reg);
 	kfree(reg);
+	if (ret < 0)
+		return ret;
 
 	if ((fw_mode & 0x00000003) == 2)
 		return 1;
Signature · 1 line
2.1.4
#2 lore raw

PATCH 3/3 rt2x00usb: drop rt2x00usb_disable_radio() from rt2800usb_disable_radio()

In reply to Sebastian Andrzej Siewior

I have here FRITZ!WLAN USB Stick N v2 / idVendor=057c, idProduct=8501 and every single of this requests ends up with:

|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x0c failed for offset 0x0000 with error -32

I was browsing the the vedor code and I haven't seen such request so I remove it with this patch. If I wasn't look enough or if this error is expected then please excuse :)

---
 drivers/net/wireless/rt2x00/rt2800usb.c | 1 -
 1 file changed, 1 deletion(-)
drivers/net/wireless/rt2x00/rt2800usb.c +0 -1
index e692e5fe4e01..9a2f44a54d94 100644
@@ -378,7 +378,6 @@ static int rt2800usb_enable_radio(struct rt2x00_dev *rt2x00dev)
 static void rt2800usb_disable_radio(struct rt2x00_dev *rt2x00dev)
 {
 	rt2800_disable_radio(rt2x00dev);
-	rt2x00usb_disable_radio(rt2x00dev);
 }
 
 static int rt2800usb_set_state(struct rt2x00_dev *rt2x00dev,
Signature · 1 line
2.1.4
#3 lore raw
In reply to Sebastian Andrzej Siewior
Quoted 6 lines

My understanding ist that rt2x00usb_register_read() is void and so the reader is unaware of read errors and assumes that whatever was on the stack as it was about to read.

Thanks, 3 patches applied to wireless-drivers-next.git:

e4fcfaf802bf rt2x00usb: initialize the read value in case of failure 92d5e2456d39 rt2x00usb: check USB's request error code in rt2800usb_autorun_detect() 7daa54b747dd rt2x00usb: drop rt2x00usb_disable_radio() from rt2800usb_disable_radio()

Kalle Valo

#4 lore raw
Quoted 1 line · * Kalle Valo | 2015-03-16 16:06:52 [+0000]:

Thanks, 3 patches applied to wireless-drivers-next.git:

Thanks. Do you guys have any kind of further documentation? Where did you get the firmware from? Is it the vendor driver in better shape and firmware excracted from header files?
The reason I'm asking is that I have a small script that switches between AP & Managed mode and after a while the show ends with:

|wlan0: authenticate with XX
|wlan0: send auth to XX (try 1/3)
|wlan0: authenticated
|wlan0: associate with XX (try 1/3)
|wlan0: RX AssocResp fromXX (capab=0x1411 status=0 aid=3)
|IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
|wlan0: associated
|wlan0: dropped frame to XX (unauthorized port)

Managed mode complete.

|wlan0: deauthenticating from XX by local choice (Reason: 3=DEAUTH_LEAVING)
|cfg80211: Calling CRDA to update world regulatory domain
|ieee80211 phy0: rt2800usb_write_firmware: Info - Firmware loading not required - NIC in AutoRun mode
|IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
|IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready

switch to AP mode done.

And this should be where it switches back to managed mode

|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x3200 with error -110 val: 0 type 40
|ieee80211 phy0: rt2800usb_write_firmware: Info - Firmware is written.
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x7014 with error -110 val: 0 type 40
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x701c with error -110 val: 0 type 40
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x01 failed for offset 0x0000 with error -110 val: 8 type 40
|ieee80211 phy0: rt2800usb_write_firmware: Error - Failed to write Firmware to device
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x0400 with error -110 val: 0 type c0
… mode of these…
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x0400 with error -110 val: 0 type c0
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x0400 with error -110 val: 0 type c0
|ieee80211 phy0: rt2800_load_firmware: Error - PBF system register not ready
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x7010 with error -110 val: 0 type c0
… mode of these…
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x7010 with error -110 val: 0 type 40
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x0404 with error -110 val: 0 type 40
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x1000 with error -110 val: 0 type c0
|ieee80211 phy0: rt2800_wait_csr_ready: Error - Unstable hardware 
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x07 failed for offset 0x7010 with error -110 val: 0 type c0
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x7010 with error -110 val: 0 type 40

at this point the stick is dead. Dead as in I think I get -110 for every USB reqeust. The only thing that helps is to replug the USB stick since it does not even accept a new device number after a USB-reset. Since the error happens by the time the firmware is written could it be that the firmware was not yet ready to receive a new one? Since it does not answer at all I think it blue screened :)

Quoted 2 lines

Kalle Valo

Sebastian

#5 lore raw

* Sebastian Andrzej Siewior | 2015-03-16 17:41:00 [+0100]:

Just an update:
…

Quoted 8 lines
|ieee80211 phy0: rt2800usb_write_firmware: Info - Firmware loading not required - NIC in AutoRun mode
|IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
|IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready

switch to AP mode done.

And this should be where it switches back to managed mode
|ieee80211 phy0: rt2x00usb_vendor_request: Error - Vendor Request 0x06 failed for offset 0x3200 with error -110 val: 0 type 40 |ieee80211 phy0: rt2800usb_write_firmware: Info - Firmware is written.

so the difference is that I don't see "NIC in AutoRun mode". So rt2800usb_autorun_detect() does not return 1 for some reason but 0 instead and whatever comes next kills the stick. Maybe it was tired of of the wpa <-> hostapd mode switch for no reason.

So I told rt2800usb_autorun_detect() to always return 1 and not to care what the stick says. The stick now runs for 12.5h in test without a problem (that means it is able to connect to the AP and hostapd does not complain, too). After around 11h I saw the message that it would write the firmware (but it got ignored instead). So the workaround seems to work.

I have no idea what AutoRun mode is (it seems that the stick already has a firmware and is happy with it) and I have no clue why the firmware decides to lie about it. I looked at the vendor driver I found at github (which seems to be from 2012-10-22, DPO_RT5572_LinuxSTA_2.6.1.3) and well, I look again if someone says it is worth looking… They seem to do some kind "firmware is comming" magic which might be already done in the current driver at a different spot but I think the main question is why request is answered wrong. I *think* the vendor driver sends the USB_MODE_AUTORUN request only once but it is hard to tell…

Quoted 1 line
Quoted 1 line

Kalle Valo

Sebastian

#6 lore raw
Quoted 9 lines · Sebastian Andrzej Siewior

I have no idea what AutoRun mode is (it seems that the stick already has a firmware and is happy with it) and I have no clue why the firmware decides to lie about it. I looked at the vendor driver I found at github (which seems to be from 2012-10-22, DPO_RT5572_LinuxSTA_2.6.1.3) and well, I look again if someone says it is worth looking… They seem to do some kind "firmware is comming" magic which might be already done in the current driver at a different spot but I think the main question is why request is answered wrong. I *think* the vendor driver sends the USB_MODE_AUTORUN request only once but it is hard to tell…

Does the below patch solve the issue ?

drivers/net/wireless/rt2x00/rt2800usb.c +1 -0
index 9a2f44a..3a6cae8 100644
@@ -293,6 +293,7 @@ static int rt2800usb_write_firmware(struct rt2x00_dev *rt2x00dev,
 	if (retval) {
 		rt2x00_info(rt2x00dev,
 			    "Firmware loading not required - NIC in AutoRun mode\n");
+		__clear_bit(REQUIRE_FIRMWARE, &rt2x00dev->cap_flags);
 	} else {
 		rt2x00usb_register_multiwrite(rt2x00dev, FIRMWARE_IMAGE_BASE,
 					      data + offset, length);
#7 lore raw
Quoted 11 lines · Stanislaw Gruszka
Quoted 9 lines · Sebastian Andrzej Siewior

I have no idea what AutoRun mode is (it seems that the stick already has a firmware and is happy with it) and I have no clue why the firmware decides to lie about it. I looked at the vendor driver I found at github (which seems to be from 2012-10-22, DPO_RT5572_LinuxSTA_2.6.1.3) and well, I look again if someone says it is worth looking… They seem to do some kind "firmware is comming" magic which might be already done in the current driver at a different spot but I think the main question is why request is answered wrong. I *think* the vendor driver sends the USB_MODE_AUTORUN request only once but it is hard to tell…

Quoted 1 line

Does the below patch solve the issue ?

So you kill the please-load-the-firware bit and avoid further questions regarding its state. Yes, it helps as in my test-case is now running for ~15h. I see the "NIC in AutoRun mode" just once. Thanks.

I am still curious why it answers wrong but lets hope the first answer is correct :)

Quoted 7 lines
drivers/net/wireless/rt2x00/rt2800usb.c +0 -0
index 9a2f44a..3a6cae8 100644

drivers/net/wireless/rt2x00/rt2800usb.c static int rt2800usb_write_firmware(struct rt2x00_dev *rt2x00dev, · line 299

Quoted 7 lines
drivers/net/wireless/rt2x00/rt2800usb.c +1 -0
@@ -293,6 +293,7 @@ static int rt2800usb_write_firmware(struct rt2x00_dev *rt2x00dev,
 	if (retval) {
 		rt2x00_info(rt2x00dev,
 			    "Firmware loading not required - NIC in AutoRun mode\n");
+		__clear_bit(REQUIRE_FIRMWARE, &rt2x00dev->cap_flags);
 	} else {
 		rt2x00usb_register_multiwrite(rt2x00dev, FIRMWARE_IMAGE_BASE,
 					      data + offset, length);

Sebastian

#8 lore raw
Quoted 12 lines · Sebastian Andrzej Siewior
Quoted 11 lines · Stanislaw Gruszka
Quoted 9 lines · Sebastian Andrzej Siewior

I have no idea what AutoRun mode is (it seems that the stick already has a firmware and is happy with it) and I have no clue why the firmware decides to lie about it. I looked at the vendor driver I found at github (which seems to be from 2012-10-22, DPO_RT5572_LinuxSTA_2.6.1.3) and well, I look again if someone says it is worth looking… They seem to do some kind "firmware is comming" magic which might be already done in the current driver at a different spot but I think the main question is why request is answered wrong. I *think* the vendor driver sends the USB_MODE_AUTORUN request only once but it is hard to tell…

Quoted 8 lines
Quoted 1 line

Does the below patch solve the issue ?

So you kill the please-load-the-firware bit and avoid further questions regarding its state. Yes, it helps as in my test-case is now running for ~15h. I see the "NIC in AutoRun mode" just once. Thanks.

I am still curious why it answers wrong but lets hope the first answer is correct :)

I don't know, perhaps this value is only valid at initialization stage. Looks that vendor driver reads it only once and saves it as flag in the memory.

Stanislaw

#9 lore raw

PATCH 1/1 rt2800usb: check Autorun mode on FW load only once

In reply to Sebastian Andrzej Siewior

Seems H/W report correctly firmware Autorun value only at initialization stage. When we close interface and open it again, Autorun value is 0 and we try to load firmware what kills the device. To fix clear REQUIRE_FIRMWARE firmware flag, to do not load firmware again, once we discover Autorun mode.

---
 drivers/net/wireless/rt2x00/rt2800usb.c | 1 +
 1 file changed, 1 insertion(+)
drivers/net/wireless/rt2x00/rt2800usb.c +1 -0
index 9a2f44a..3a6cae8 100644
@@ -293,6 +293,7 @@ static int rt2800usb_write_firmware(struct rt2x00_dev *rt2x00dev,
 	if (retval) {
 		rt2x00_info(rt2x00dev,
 			    "Firmware loading not required - NIC in AutoRun mode\n");
+		__clear_bit(REQUIRE_FIRMWARE, &rt2x00dev->cap_flags);
 	} else {
 		rt2x00usb_register_multiwrite(rt2x00dev, FIRMWARE_IMAGE_BASE,
 					      data + offset, length);
Signature · 1 line
1.8.3.1
Applied
#10 lore raw

Re: rt2800usb: check Autorun mode on FW load only once

Quoted 9 lines

Seems H/W report correctly firmware Autorun value only at initialization stage. When we close interface and open it again, Autorun value is 0 and we try to load firmware what kills the device. To fix clear REQUIRE_FIRMWARE firmware flag, to do not load firmware again, once we discover Autorun mode.

Thanks, applied to wireless-drivers-next.git.

Kalle Valo

#11 lore raw
In reply to Sebastian Andrzej Siewior
Quoted 5 lines · Sebastian Andrzej Siewior
Quoted 1 line · * Kalle Valo | 2015-03-16 16:06:52 [+0000]:

Thanks, 3 patches applied to wireless-drivers-next.git:

Thanks. Do you guys have any kind of further documentation? Where did

There is doc describing MAC registers, it was provided under NDA to rt2x00 developers. However you can google for "MT7620_ProgrammingGuide.pdf" and get similar doc. It does not describe radio/broadband registers, we only have vendor driver as source for those.

Quoted 1 line

you get the firmware from?

linux-firmware contains firmware posed by Ralink developers. Vendor driver ships new firmware versions, it is binnary file common/rt2870.bin

Quoted 2 lines

Is it the vendor driver in better shape and
firmware excracted from header files?

For some users vendor driver works better.

Stanislaw