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

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

[Sebastian Andrzej Siewior]

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.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/wireless/rt2x00/rt2x00usb.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.h b/drivers/net/wireless/rt2x00/rt2x00usb.h
index 8f85fbd5f237..569363da00a2 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.h
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.h
@@ -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);
-- 
2.1.4

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

[Sebastian Andrzej Siewior]

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

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/wireless/rt2x00/rt2800usb.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 8444313eabe2..e692e5fe4e01 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -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;
-- 
2.1.4

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

[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 :)

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/wireless/rt2x00/rt2800usb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index e692e5fe4e01..9a2f44a54d94 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -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,
-- 
2.1.4

Re: [1/3] rt2x00usb: initialize the read value in case of failure

[Kalle Valo]

> 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.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

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

Re: [1/3] rt2x00usb: initialize the read value in case of failure

[Sebastian Andrzej Siewior]

* 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 :)

>
>Kalle Valo

Sebastian

Re: [1/3] rt2x00usb: initialize the read value in case of failure

[Sebastian Andrzej Siewior]

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

Just an update:
…
>|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…

>>Kalle Valo

Sebastian

Re: [1/3] rt2x00usb: initialize the read value in case of failure

[Stanislaw Gruszka]

On Tue, Mar 17, 2015 at 09:34:14AM +0100, Sebastian Andrzej Siewior wrote:
> 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 ?

diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 9a2f44a..3a6cae8 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -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);

Re: [1/3] rt2x00usb: initialize the read value in case of failure

[Sebastian Andrzej Siewior]

On 03/18/2015 02:11 PM, Stanislaw Gruszka wrote:
> On Tue, Mar 17, 2015 at 09:34:14AM +0100, Sebastian Andrzej Siewior wrote:
>> 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 ?

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

> 
> diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
> index 9a2f44a..3a6cae8 100644
> --- a/drivers/net/wireless/rt2x00/rt2800usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2800usb.c
> @@ -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

Re: [1/3] rt2x00usb: initialize the read value in case of failure

[Stanislaw Gruszka]

On Thu, Mar 19, 2015 at 02:24:02PM +0100, Sebastian Andrzej Siewior wrote:
> On 03/18/2015 02:11 PM, Stanislaw Gruszka wrote:
> > On Tue, Mar 17, 2015 at 09:34:14AM +0100, Sebastian Andrzej Siewior wrote:
> >> 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 ?
> 
> 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

[PATCH] rt2800usb: check Autorun mode on FW load only once

[Stanislaw Gruszka]

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.

Reported-and-tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/rt2x00/rt2800usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 9a2f44a..3a6cae8 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -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);
-- 
1.8.3.1

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

[Kalle Valo]

> 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.
> 
> Reported-and-tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>

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

Kalle Valo

Re: [1/3] rt2x00usb: initialize the read value in case of failure

[Stanislaw Gruszka]

On Mon, Mar 16, 2015 at 05:41:00PM +0100, Sebastian Andrzej Siewior wrote:
> * 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.

> 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

> Is it the vendor driver in better shape and
> firmware excracted from header files?
For some users vendor driver works better.

Stanislaw