rt2500usb: fix powersaving random failures

PATCH In review Ondrej Zary 4 participants 20 messages Mar 29, 2010 – Apr 3, 2010 Networking drivers (wireless)

Share & tools

Ondrej Zary

author

2010-03-29 07:56 UTC #0 lore raw

PATCH 1/1 rt2500usb: fix powersaving random failures

This patch seems to fix random powersaving failure where the device state is stuck at 1 (STATE_SLEEP) and does not change to 3 (STATE_AWAKE), resulting in to associate and these error messages:
phy0 -> rt2500usb_set_device_state: Error - Device failed to enter state 3 (-16). No probe response from AP xx:xx:xx:xx:xx:xx after 500ms, disconnecting.

Signed-off-by Ondrej Zary <[email protected]>

drivers/net/wireless/rt2x00/rt2500usb.c +4 -0

@@ -648,6 +648,10 @@
 
 		rt2x00_set_field16(&reg, MAC_CSR18_AUTO_WAKE, 1);
 		rt2500usb_register_write(rt2x00dev, MAC_CSR18, reg);
+	} else {
+		rt2500usb_register_read(rt2x00dev, MAC_CSR18, &reg);
+		rt2x00_set_field16(&reg, MAC_CSR18_AUTO_WAKE, 0);
+		rt2500usb_register_write(rt2x00dev, MAC_CSR18, reg);
 	}
 
 	rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);

Signature · 1 line

Ondrej Zary

Gertjan van Wingerde

2010-03-29 19:47 UTC #1 lore raw

Quoted 7 lines · Ondrej Zary

This patch seems to fix random powersaving failure where the device state is stuck at 1 (STATE_SLEEP) and does not change to 3 (STATE_AWAKE), resulting in to associate and these error messages:
phy0 -> rt2500usb_set_device_state: Error - Device failed to enter state 3 (-16). No probe response from AP xx:xx:xx:xx:xx:xx after 500ms, disconnecting.
Signed-off-by Ondrej Zary <[email protected]>

Thanks. This looks good, but I'll rehash it against the latest state of affairs of rt2x00, so that the disabling of powersaving is undone at the same time.

John, I'll send you the rehashed patch tomorrow.

---
Gertjan.

Quoted 15 lines

drivers/net/wireless/rt2x00/rt2500usb.c +4 -0

@@ -648,6 +648,10 @@
 
 		rt2x00_set_field16(&reg, MAC_CSR18_AUTO_WAKE, 1);
 		rt2500usb_register_write(rt2x00dev, MAC_CSR18, reg);
+	} else {
+		rt2500usb_register_read(rt2x00dev, MAC_CSR18, &reg);
+		rt2x00_set_field16(&reg, MAC_CSR18_AUTO_WAKE, 0);
+		rt2500usb_register_write(rt2x00dev, MAC_CSR18, reg);
 	}
 
 	rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);

Ivo van Doorn

Acked-by

2010-03-29 21:00 UTC #2 lore raw

Quoted 7 lines · Gertjan van Wingerde

Quoted 6 lines · Ondrej Zary
This patch seems to fix random powersaving failure where the device state is stuck at 1 (STATE_SLEEP) and does not change to 3 (STATE_AWAKE), resulting in to associate and these error messages:
phy0 -> rt2500usb_set_device_state: Error - Device failed to enter state 3 (-16). No probe response from AP xx:xx:xx:xx:xx:xx after 500ms, disconnecting.

Quoted 6 lines

Quoted 1 line
Signed-off-by Ondrej Zary <[email protected]>
Thanks. This looks good, but I'll rehash it against the latest state of affairs of rt2x00, so that the disabling of powersaving is undone at the same time.
John, I'll send you the rehashed patch tomorrow.

Is this change interesting for the other drivers as well? Anyway, this one gets my Ack:

Acked-by Ivo van Doorn <[email protected]>

Quoted 25 lines

---
Gertjan.
Quoted 5 lines
drivers/net/wireless/rt2x00/rt2500usb.c line 657
Quoted 10 lines
drivers/net/wireless/rt2x00/rt2500usb.c +4 -0
@@ -648,6 +648,10 @@
 
 		rt2x00_set_field16(&reg, MAC_CSR18_AUTO_WAKE, 1);
 		rt2500usb_register_write(rt2x00dev, MAC_CSR18, reg);
+	} else {
+		rt2500usb_register_read(rt2x00dev, MAC_CSR18, &reg);
+		rt2x00_set_field16(&reg, MAC_CSR18_AUTO_WAKE, 0);
+		rt2500usb_register_write(rt2x00dev, MAC_CSR18, reg);
 	}
 
 	rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);
_______________________________________________
users mailing list
[email protected]
http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com

Gertjan van Wingerde

2010-03-30 05:11 UTC #3 lore raw

Quoted 13 lines · Ivo van Doorn

Quoted 12 lines · Gertjan van Wingerde
Quoted 7 lines · Ondrej Zary
This patch seems to fix random powersaving failure where the device state is stuck at 1 (STATE_SLEEP) and does not change to 3 (STATE_AWAKE), resulting in to associate and these error messages:
phy0 -> rt2500usb_set_device_state: Error - Device failed to enter state 3 (-16). No probe response from AP xx:xx:xx:xx:xx:xx after 500ms, disconnecting.
Signed-off-by Ondrej Zary <[email protected]>
Thanks. This looks good, but I'll rehash it against the latest state of affairs of rt2x00, so that the disabling of powersaving is undone at the same time.

Quoted 6 lines

Quoted 1 line
John, I'll send you the rehashed patch tomorrow.
Is this change interesting for the other drivers as well? Anyway, this one gets my Ack:
Acked-by Ivo van Doorn <[email protected]>

Correct. That is the other reason to rehash. At least rt2400pci and rt2500pci can use the same change.

Quoted 29 lines

Quoted 26 lines
---
Gertjan.
Quoted 5 lines
drivers/net/wireless/rt2x00/rt2500usb.c line 657
Quoted 10 lines
drivers/net/wireless/rt2x00/rt2500usb.c +4 -0
@@ -648,6 +648,10 @@
 
 		rt2x00_set_field16(&reg, MAC_CSR18_AUTO_WAKE, 1);
 		rt2500usb_register_write(rt2x00dev, MAC_CSR18, reg);
+	} else {
+		rt2500usb_register_read(rt2x00dev, MAC_CSR18, &reg);
+		rt2x00_set_field16(&reg, MAC_CSR18_AUTO_WAKE, 0);
+		rt2500usb_register_write(rt2x00dev, MAC_CSR18, reg);
 	}
 
 	rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);
_______________________________________________
users mailing list
[email protected]
http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com

Ondrej Zary

author

2010-03-30 06:01 UTC #4 lore raw

Quoted 18 lines · Gertjan van Wingerde

Quoted 17 lines · Ivo van Doorn
Quoted 15 lines · Gertjan van Wingerde
Quoted 8 lines · Ondrej Zary
This patch seems to fix random powersaving failure where the device state is stuck at 1 (STATE_SLEEP) and does not change to 3 (STATE_AWAKE), resulting in to associate and these error messages:
phy0 -> rt2500usb_set_device_state: Error - Device failed to enter state 3 (-16). No probe response from AP xx:xx:xx:xx:xx:xx after 500ms, disconnecting.
Signed-off-by Ondrej Zary <[email protected]>
Thanks. This looks good, but I'll rehash it against the latest state of affairs of rt2x00, so that the disabling of powersaving is undone at the same time.
John, I'll send you the rehashed patch tomorrow.

Quoted 7 lines

Quoted 4 lines
Is this change interesting for the other drivers as well? Anyway, this one gets my Ack:
Acked-by Ivo van Doorn <[email protected]>
Correct. That is the other reason to rehash. At least rt2400pci and rt2500pci can use the same change.

I also have a PCI card with RT2560F (MSI MS-6834) but haven't tested it yet.

Signature · 1 line

Ondrej Zary

Ondrej Zary

author

2010-03-30 12:34 UTC #5 lore raw

In reply to Gertjan van Wingerde

Quoted 18 lines · Gertjan van Wingerde

Quoted 17 lines · Ivo van Doorn
Quoted 15 lines · Gertjan van Wingerde
Quoted 8 lines · Ondrej Zary
This patch seems to fix random powersaving failure where the device state is stuck at 1 (STATE_SLEEP) and does not change to 3 (STATE_AWAKE), resulting in to associate and these error messages:
phy0 -> rt2500usb_set_device_state: Error - Device failed to enter state 3 (-16). No probe response from AP xx:xx:xx:xx:xx:xx after 500ms, disconnecting.
Signed-off-by Ondrej Zary <[email protected]>
Thanks. This looks good, but I'll rehash it against the latest state of affairs of rt2x00, so that the disabling of powersaving is undone at the same time.
John, I'll send you the rehashed patch tomorrow.

Quoted 7 lines

Quoted 4 lines
Is this change interesting for the other drivers as well? Anyway, this one gets my Ack:
Acked-by Ivo van Doorn <[email protected]>
Correct. That is the other reason to rehash. At least rt2400pci and rt2500pci can use the same change.

rt2500pci does not seem to need this change. At least my card never stucks in state 3. But it does in state 3 - this message appears probably on each attempt to enter state 1 (after each packet with "ping" and after terminating "ping -f"):
phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 1 (-16).

Otherwise it works fine. Increasing REGISTER_BUSY_COUNT to 20 (and msleep in rt2500pci_set_state() to 30) does not help.

Signature · 1 line

Ondrej Zary

Ondrej Zary

author

2010-03-30 12:38 UTC #6 lore raw

Quoted 2 lines

rt2500pci does not seem to need this change. At least my card never stucks in state 3. But it does in state 3 - this message appears probably on each

It never stucks in state 1 but it does in state 3.

Signature · 1 line

Ondrej Zary

Matthijs Kooijman

2010-03-30 13:30 UTC #7 lore raw

In reply to Ondrej Zary

Hi all,

Quoted 1 line

phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 1 (-16).

I'm seeing this one on my rt2500pci as well (and also works ok otherwise).

Gr.

Matthijs

Ondrej Zary

author

2010-03-30 20:09 UTC #8 lore raw

RFC PATCH 1/1 rt2500pci: fix powersaving

Quoted 6 lines · Matthijs Kooijman

Hi all,
Quoted 2 lines
phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 1 (-16).
I'm seeing this one on my rt2500pci as well (and also works ok otherwise).

OK, so let's try to fix it.

The first patch below fixes this problem. And reveals two other problems. Now the device has problems entering states 3 and 4 (remains stuck in state 1) - the driver seems to not like this and oopses.
The second patch fixes the "stuck state 1" problem (same fix as in rt2500usb). The oops does not appear anymore with the second patch - but I think that it should be fixed anyway separately.
(I'm testing this with "while true; do ifdown wlan0; ifup wlan0; done")

oops:

phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 3 (-16). phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 3 (-16). phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 3 (-16).
wlan0: deauthenticating from 00:13:d4:0f:f3:19 by local choice (reason=3) phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 3 (-16).
cfg80211: Calling CRDA to update world regulatory domain
phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 3 (-16). phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 4 (-5).

BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<c8949726>] rt2x00queue_init_queues+0x25/0x64 [rt2x00lib]
*pde = 00000000
Oops: 0002 [#1] SMP
last sysfs file: /sys/devices/virtual/input/mice/uevent
Modules linked in: aes_generic arc4 ecb rt2500pci rt2x00pci rt2x00lib mac80211 cfg80211 rfkill ns558 pcspkr eeprom_93cx6 gameport 8139too

8139cp mii [last
unloaded: scsi_wait_scan]

Pid: 2051, comm: ifconfig Not tainted 2.6.34-rc2 #8 i815-W83627/
EIP: 0060:[<c8949726>] EFLAGS: 00010202 CPU: 0
EIP is at rt2x00queue_init_queues+0x25/0x64 [rt2x00lib]
EAX: 00000000 EBX: c703b100 ECX: c703b10c EDX: 00000000
ESI: c72b2ac0 EDI: 00000001 EBP: c7293df4 ESP: c7293de4
DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
Process ifconfig (pid: 2051, ti=c7293000 task=c7253280 task.ti=c7293000)

Stack:
 c703b130 c72b2ac0 00000000 c72b2884 c7293e04 c894868a c72b2ac0 00000000
<0> c7293e14 c8948773 c72b2240 c72b53c0 c7293e1c c8949039 c7293e48 c890cd83
<0> c72b2240 c72b2884 c72b5000 00000000 00005d31 00000000 c72b5000 c891e46c
Call Trace:
 [<c894868a>] ? rt2x00lib_stop+0x54/0xb4 [rt2x00lib]
 [<c8948773>] ? rt2x00lib_start+0x89/0xa6 [rt2x00lib]
 [<c8949039>] ? rt2x00mac_start+0x18/0x1a [rt2x00lib]
 [<c890cd83>] ? ieee80211_open+0x24a/0x567 [mac80211]
 [<c05f73dd>] ? __dev_open+0x7b/0xa4
 [<c05f5a81>] ? __dev_change_flags+0x93/0x108
 [<c05f7336>] ? dev_change_flags+0x13/0x3f
 [<c062aa81>] ? devinet_ioctl+0x21e/0x497
 [<c062bcce>] ? inet_ioctl+0x8e/0xa7
 [<c05eac3e>] ? sock_ioctl+0x1c0/0x1e4
 [<c04a762d>] ? vfs_ioctl+0x27/0x91
 [<c05eaa7e>] ? sock_ioctl+0x0/0x1e4
 [<c04a7bce>] ? do_vfs_ioctl+0x48e/0x4cc
 [<c048a13f>] ? handle_mm_fault+0x3fd/0x86a
 [<c046cff1>] ? call_rcu_sched+0xd/0xf
 [<c04a7c3a>] ? sys_ioctl+0x2e/0x48
 [<c0402ed8>] ? sysenter_do_call+0x12/0x28
Code: f7 5b 5e 5f 5d c3 55 89 e5 57 56 89 c6 53 83 ec 04 8b 98 c8 02 00 00 eb 30 89 d8 31 ff e8 a9 ff ff ff eb 1a 6b c7 14 8b 53 04 47 <c7>

04 02 00 00 00
00 8b 56 04 03 43 04 8b 52 2c ff 52 20 0f b7
EIP: [<c8949726>] rt2x00queue_init_queues+0x25/0x64 [rt2x00lib] SS:ESP 0068:c7293de4
CR2: 0000000000000000

---[ end trace 65bcf981f552d652 ]---

drivers/net/wireless/rt2x00/rt2500pci.c +5 -4

@@ -1079,7 +1079,7 @@
 static int rt2500pci_set_state(struct rt2x00_dev *rt2x00dev,
 			       enum dev_state state)
 {
-	u32 reg;
+	u32 reg, reg2;
 	unsigned int i;
 	char put_to_sleep;
 	char bbp_state;

@@ -1100,11 +1100,12 @@
 	 * device has entered the correct state.
 	 */
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
-		rt2x00pci_register_read(rt2x00dev, PWRCSR1, &reg);
-		bbp_state = rt2x00_get_field32(reg, PWRCSR1_BBP_CURR_STATE);
-		rf_state = rt2x00_get_field32(reg, PWRCSR1_RF_CURR_STATE);
+		rt2x00pci_register_read(rt2x00dev, PWRCSR1, &reg2);
+		bbp_state = rt2x00_get_field32(reg2, PWRCSR1_BBP_CURR_STATE);
+		rf_state = rt2x00_get_field32(reg2, PWRCSR1_RF_CURR_STATE);
 		if (bbp_state == state && rf_state == state)
 			return 0;
+		rt2x00pci_register_write(rt2x00dev, PWRCSR1, reg);
 		msleep(10);
 	}

drivers/net/wireless/rt2x00/rt2500pci.c 2010-03-30 21:15:03.000000000 +0200 renamed from drivers/net/wireless/rt2x00/rt2500pci.c 2010-03-20 02:17:57.000000000 +0100 +4 -0

@@ -573,6 +573,10 @@
 
 		rt2x00_set_field32(&reg, CSR20_AUTOWAKE, 1);
 		rt2x00pci_register_write(rt2x00dev, CSR20, reg);
+	} else {
+		rt2x00pci_register_read(rt2x00dev, CSR20, &reg);
+		rt2x00_set_field32(&reg, CSR20_AUTOWAKE, 0);
+		rt2x00pci_register_write(rt2x00dev, CSR20, reg);
 	}
 
 	rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);

Signature · 1 line

Ondrej Zary

Matthijs Kooijman

2010-03-30 20:32 UTC #9 lore raw

Re: [PATCH] [RFC] rt2500pci: fix powersaving

Hi Ondrej,

do you need testing of your patches? If so, let me know and I'll give them a go.

Gr.

Matthijs

Gertjan van Wingerde

2010-03-30 20:38 UTC #10 lore raw

Re: [rt2x00-users] [PATCH] [RFC] rt2500pci: fix powersaving

Quoted 5 lines · Matthijs Kooijman

Hi Ondrej,
do you need testing of your patches? If so, let me know and I'll give them a go.

Matthijs,

It would be good if you could test these patches (I can't really test them on rt2500pci). As I'm including them in a bigger powersave patch-set, it would be good to see if Ondrej's patches have any unwanted side effects.

---
Gertjan.

Gertjan van Wingerde

2010-03-30 21:02 UTC #11 lore raw

Re: [rt2x00-users] [PATCH] [RFC] rt2500pci: fix powersaving

In reply to Ondrej Zary

Quoted 10 lines · Ondrej Zary

Quoted 6 lines · Matthijs Kooijman
Hi all,
Quoted 2 lines
phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 1 (-16).
I'm seeing this one on my rt2500pci as well (and also works ok otherwise).
OK, so let's try to fix it.

Quoted 8 lines

The first patch below fixes this problem. And reveals two other problems. Now the device has problems entering states 3 and 4 (remains stuck in state 1) - the driver seems to not like this and oopses.
The second patch fixes the "stuck state 1" problem (same fix as in rt2500usb). The oops does not appear anymore with the second patch - but I think that it should be fixed anyway separately.
(I'm testing this with "while true; do ifdown wlan0; ifup wlan0; done")

Thanks again for your hard and persistent work on this.

Quoted 75 lines

oops:
phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 3 (-16). phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 3 (-16). phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 3 (-16).
wlan0: deauthenticating from 00:13:d4:0f:f3:19 by local choice (reason=3) phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 3 (-16).
cfg80211: Calling CRDA to update world regulatory domain
phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 3 (-16). phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 4 (-5).
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<c8949726>] rt2x00queue_init_queues+0x25/0x64 [rt2x00lib]
*pde = 00000000
Oops: 0002 [#1] SMP
last sysfs file: /sys/devices/virtual/input/mice/uevent
Modules linked in: aes_generic arc4 ecb rt2500pci rt2x00pci rt2x00lib mac80211 cfg80211 rfkill ns558 pcspkr eeprom_93cx6 gameport 8139too 
8139cp mii [last
unloaded: scsi_wait_scan]
Pid: 2051, comm: ifconfig Not tainted 2.6.34-rc2 #8 i815-W83627/
EIP: 0060:[<c8949726>] EFLAGS: 00010202 CPU: 0
EIP is at rt2x00queue_init_queues+0x25/0x64 [rt2x00lib]
EAX: 00000000 EBX: c703b100 ECX: c703b10c EDX: 00000000
ESI: c72b2ac0 EDI: 00000001 EBP: c7293df4 ESP: c7293de4
DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
Process ifconfig (pid: 2051, ti=c7293000 task=c7253280 task.ti=c7293000)
Stack:
 c703b130 c72b2ac0 00000000 c72b2884 c7293e04 c894868a c72b2ac0 00000000
<0> c7293e14 c8948773 c72b2240 c72b53c0 c7293e1c c8949039 c7293e48 c890cd83
<0> c72b2240 c72b2884 c72b5000 00000000 00005d31 00000000 c72b5000 c891e46c
Call Trace:
 [<c894868a>] ? rt2x00lib_stop+0x54/0xb4 [rt2x00lib]
 [<c8948773>] ? rt2x00lib_start+0x89/0xa6 [rt2x00lib]
 [<c8949039>] ? rt2x00mac_start+0x18/0x1a [rt2x00lib]
 [<c890cd83>] ? ieee80211_open+0x24a/0x567 [mac80211]
 [<c05f73dd>] ? __dev_open+0x7b/0xa4
 [<c05f5a81>] ? __dev_change_flags+0x93/0x108
 [<c05f7336>] ? dev_change_flags+0x13/0x3f
 [<c062aa81>] ? devinet_ioctl+0x21e/0x497
 [<c062bcce>] ? inet_ioctl+0x8e/0xa7
 [<c05eac3e>] ? sock_ioctl+0x1c0/0x1e4
 [<c04a762d>] ? vfs_ioctl+0x27/0x91
 [<c05eaa7e>] ? sock_ioctl+0x0/0x1e4
 [<c04a7bce>] ? do_vfs_ioctl+0x48e/0x4cc
 [<c048a13f>] ? handle_mm_fault+0x3fd/0x86a
 [<c046cff1>] ? call_rcu_sched+0xd/0xf
 [<c04a7c3a>] ? sys_ioctl+0x2e/0x48
 [<c0402ed8>] ? sysenter_do_call+0x12/0x28
Code: f7 5b 5e 5f 5d c3 55 89 e5 57 56 89 c6 53 83 ec 04 8b 98 c8 02 00 00 eb 30 89 d8 31 ff e8 a9 ff ff ff eb 1a 6b c7 14 8b 53 04 47 <c7> 
04 02 00 00 00
00 8b 56 04 03 43 04 8b 52 2c ff 52 20 0f b7
EIP: [<c8949726>] rt2x00queue_init_queues+0x25/0x64 [rt2x00lib] SS:ESP 0068:c7293de4
CR2: 0000000000000000
---[ end trace 65bcf981f552d652 ]---
drivers/net/wireless/rt2x00/rt2500pci.c +1 -3
@@ -1079,7 +1079,7 @@
 static int rt2500pci_set_state(struct rt2x00_dev *rt2x00dev,
 			       enum dev_state state)
 {
-	u32 reg;
+	u32 reg, reg2;
 	unsigned int i;
 	char put_to_sleep;
 	char bbp_state;
@@ -1100,11 +1100,12 @@
 	 * device has entered the correct state.
 	 */
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
-		rt2x00pci_register_read(rt2x00dev, PWRCSR1, &reg);
-		bbp_state = rt2x00_get_field32(reg, PWRCSR1_BBP_CURR_STATE);

drivers/net/wireless/rt2x00/rt2500pci.c line 1111

Quoted 10 lines

drivers/net/wireless/rt2x00/rt2500pci.c +4 -1

-		rf_state = rt2x00_get_field32(reg, PWRCSR1_RF_CURR_STATE);
+		rt2x00pci_register_read(rt2x00dev, PWRCSR1, &reg2);
+		bbp_state = rt2x00_get_field32(reg2, PWRCSR1_BBP_CURR_STATE);
+		rf_state = rt2x00_get_field32(reg2, PWRCSR1_RF_CURR_STATE);
 		if (bbp_state == state && rf_state == state)
 			return 0;
+		rt2x00pci_register_write(rt2x00dev, PWRCSR1, reg);
 		msleep(10);
 	}

Hmmm, checking the old legacy Ralink rt2500 driver it shows that the legacy driver never waits and checks whether the device has come in the right power state. What does happen to the stability of the connection if you simply remove this entire for-loop?

Quoted 5 lines

drivers/net/wireless/rt2x00/rt2500pci.c 2010-03-30 21:15:03.000000000 +0200 line 582

Quoted 10 lines

drivers/net/wireless/rt2x00/rt2500pci.c 2010-03-30 21:15:03.000000000 +0200 renamed from drivers/net/wireless/rt2x00/rt2500pci.c 2010-03-20 02:17:57.000000000 +0100 +4 -0
@@ -573,6 +573,10 @@
 
 		rt2x00_set_field32(&reg, CSR20_AUTOWAKE, 1);
 		rt2x00pci_register_write(rt2x00dev, CSR20, reg);
+	} else {
+		rt2x00pci_register_read(rt2x00dev, CSR20, &reg);
+		rt2x00_set_field32(&reg, CSR20_AUTOWAKE, 0);
+		rt2x00pci_register_write(rt2x00dev, CSR20, reg);
 	}
 
 	rt2x00dev->ops->lib->set_device_state(rt2x00dev, state);

OK. Got this queued up already (based upon your patch to rt2500usb).

---
Gertjan.

Ondrej Zary

author

2010-03-30 21:36 UTC #12 lore raw

Re: [rt2x00-users] [PATCH] [RFC] rt2500pci: fix powersaving

Quoted 50 lines · On Tuesday 30 March 2010 23:02:33 Gertjan van Wingerde wrote:

Quoted 11 lines · Ondrej Zary
Quoted 7 lines · Matthijs Kooijman
Hi all,
Quoted 2 lines
phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 1 (-16).
I'm seeing this one on my rt2500pci as well (and also works ok otherwise).
OK, so let's try to fix it.
Quoted 7 lines
The first patch below fixes this problem. And reveals two other problems. Now the device has problems entering states 3 and 4 (remains stuck in state 1) - the driver seems to not like this and oopses.
The second patch fixes the "stuck state 1" problem (same fix as in rt2500usb). The oops does not appear anymore with the second patch - but I think that it should be fixed anyway separately.
(I'm testing this with "while true; do ifdown wlan0; ifup wlan0; done")
Thanks again for your hard and persistent work on this.
Quoted 28 lines
oops removed
---
linux-2.6.34-rc2-orig/drivers/net/wireless/rt2x00/rt2500pci.c 2010-03-20
02:17:57.000000000 +0100 +++
linux-2.6.34-rc2/drivers/net/wireless/rt2x00/rt2500pci.c 2010-03-30
15:04:50.000000000 +0200 @@ -1079,7 +1079,7 @@
static int rt2500pci_set_state(struct rt2x00_dev *rt2x00dev,
drivers/net/wireless/rt2x00/rt2500pci.c +4 -4
 			       enum dev_state state)
 {
-	u32 reg;
+	u32 reg, reg2;
 	unsigned int i;
 	char put_to_sleep;
 	char bbp_state;
@@ -1100,11 +1100,12 @@
 	 * device has entered the correct state.
 	 */
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
-		rt2x00pci_register_read(rt2x00dev, PWRCSR1, &reg);
-		bbp_state = rt2x00_get_field32(reg, PWRCSR1_BBP_CURR_STATE);
-		rf_state = rt2x00_get_field32(reg, PWRCSR1_RF_CURR_STATE);
+		rt2x00pci_register_read(rt2x00dev, PWRCSR1, &reg2);
+		bbp_state = rt2x00_get_field32(reg2, PWRCSR1_BBP_CURR_STATE);
+		rf_state = rt2x00_get_field32(reg2, PWRCSR1_RF_CURR_STATE);
 		if (bbp_state == state && rf_state == state)
 			return 0;

Quoted 8 lines

drivers/net/wireless/rt2x00/rt2500pci.c line 1110
Quoted 3 lines
drivers/net/wireless/rt2x00/rt2500pci.c +1 -0
+		rt2x00pci_register_write(rt2x00dev, PWRCSR1, reg);
 		msleep(10);
 	}
Hmmm, checking the old legacy Ralink rt2500 driver it shows that the legacy driver never waits and checks whether the device has come in the right power state. What does happen to the stability of the connection if you simply remove this entire for-loop?

It works without the loop - But I suspect that the device never enters STATE_SLEEP. With the loop present, it needs 3 loop passes to enter STATE_SLEEP (with that register write added). It never enters STATE_SLEEP without that added register write (I took it from rt2500usb) and that's why the error message was printed in log.

Signature · 1 line

Ondrej Zary

Gertjan van Wingerde

2010-03-30 21:44 UTC #13 lore raw

Re: [rt2x00-users] [PATCH] [RFC] rt2500pci: fix powersaving

Quoted 60 lines · Ondrej Zary

Quoted 50 lines · On Tuesday 30 March 2010 23:02:33 Gertjan van Wingerde wrote:
Quoted 11 lines · Ondrej Zary
Quoted 7 lines · Matthijs Kooijman
Hi all,
Quoted 2 lines
phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 1 (-16).
I'm seeing this one on my rt2500pci as well (and also works ok otherwise).
OK, so let's try to fix it.
Quoted 7 lines
The first patch below fixes this problem. And reveals two other problems. Now the device has problems entering states 3 and 4 (remains stuck in state 1) - the driver seems to not like this and oopses.
The second patch fixes the "stuck state 1" problem (same fix as in rt2500usb). The oops does not appear anymore with the second patch - but I think that it should be fixed anyway separately.
(I'm testing this with "while true; do ifdown wlan0; ifup wlan0; done")
Thanks again for your hard and persistent work on this.
Quoted 28 lines
oops removed
---
linux-2.6.34-rc2-orig/drivers/net/wireless/rt2x00/rt2500pci.c 2010-03-20
02:17:57.000000000 +0100 +++
linux-2.6.34-rc2/drivers/net/wireless/rt2x00/rt2500pci.c 2010-03-30
15:04:50.000000000 +0200 @@ -1079,7 +1079,7 @@
static int rt2500pci_set_state(struct rt2x00_dev *rt2x00dev,
drivers/net/wireless/rt2x00/rt2500pci.c +4 -4
 			       enum dev_state state)
 {
-	u32 reg;
+	u32 reg, reg2;
 	unsigned int i;
 	char put_to_sleep;
 	char bbp_state;
@@ -1100,11 +1100,12 @@
 	 * device has entered the correct state.
 	 */
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
-		rt2x00pci_register_read(rt2x00dev, PWRCSR1, &reg);
-		bbp_state = rt2x00_get_field32(reg, PWRCSR1_BBP_CURR_STATE);
-		rf_state = rt2x00_get_field32(reg, PWRCSR1_RF_CURR_STATE);
+		rt2x00pci_register_read(rt2x00dev, PWRCSR1, &reg2);
+		bbp_state = rt2x00_get_field32(reg2, PWRCSR1_BBP_CURR_STATE);
+		rf_state = rt2x00_get_field32(reg2, PWRCSR1_RF_CURR_STATE);
 		if (bbp_state == state && rf_state == state)
 			return 0;
Quoted 8 lines
drivers/net/wireless/rt2x00/rt2500pci.c line 1110
Quoted 3 lines
drivers/net/wireless/rt2x00/rt2500pci.c +1 -0
+		rt2x00pci_register_write(rt2x00dev, PWRCSR1, reg);
 		msleep(10);
 	}
Hmmm, checking the old legacy Ralink rt2500 driver it shows that the legacy driver never waits and checks whether the device has come in the right power state. What does happen to the stability of the connection if you simply remove this entire for-loop?

Quoted 6 lines

It works without the loop - But I suspect that the device never enters STATE_SLEEP. With the loop present, it needs 3 loop passes to enter STATE_SLEEP (with that register write added). It never enters STATE_SLEEP without that added register write (I took it from rt2500usb) and that's why the error message was printed in log.

OK. Thanks. I'll go with your patch then.
I'll make a similar change to rt2400pci as well, as it uses exactly the same code, and I'll audit the other drivers in this respect.

---
Gertjan.

Matthijs Kooijman

2010-03-31 17:41 UTC #14 lore raw

Re: [PATCH] [RFC] rt2500pci: fix powersaving

In reply to Ondrej Zary

Hi Ondrej,

Quoted 1 line

phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 1 (-16).

I've been running with your patches (applied to 2.6.32.7) for a day of normal work, and the steady stream of these errors has stopped. During the entire day, I've seen the error once, perhaps a corner case here or there?

Overall, I have the feeling that connectivity is a bit more unstable with these two patches, however. On a few occasions, my SSH connections would hang for a bit. Sometimes pinging would be ok (and sometimes it wouldn't), so I haven't quite figured out if these problems were actually caused by the driver, or some other cause). Once I seemd to solve the problem by turning off powersaving, but that might have been a coincidence. Other times the problems went away by themselves.

Is there anything else I can do at the moment I'm seeing connection problems? Make a register dump or check some command?

I'll do some more testing, tonight on another (probably more stable) connection.

Gr.

Matthijs

Gertjan van Wingerde

2010-03-31 18:38 UTC #15 lore raw

Re: [PATCH] [RFC] rt2500pci: fix powersaving

Quoted 15 lines · Matthijs Kooijman

Hi Ondrej,
Quoted 1 line
phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 1 (-16).
I've been running with your patches (applied to 2.6.32.7) for a day of normal work, and the steady stream of these errors has stopped. During the entire day, I've seen the error once, perhaps a corner case here or there?
Overall, I have the feeling that connectivity is a bit more unstable with these two patches, however. On a few occasions, my SSH connections would hang for a bit. Sometimes pinging would be ok (and sometimes it wouldn't), so I haven't quite figured out if these problems were actually caused by the driver, or some other cause). Once I seemd to solve the problem by turning off powersaving, but that might have been a coincidence. Other times the problems went away by themselves.

Quoted 5 lines

Is there anything else I can do at the moment I'm seeing connection problems? Make a register dump or check some command?
I'll do some more testing, tonight on another (probably more stable) connection.

Could you try the attached patch on top of Ondrej's?

The legacy Ralink drivers do not go to sleep when there are still active entries in the TX queues. This patch mimics that behavior by refusing to go to sleep if there still are active entries in any of the TX queues.

---
Gertjan

drivers/net/wireless/rt2x00/rt2500usb.c +8 -0

index c6d3f2c..a7ff006 100644
@@ -633,9 +633,17 @@ static void rt2500usb_config_ps(struct rt2x00_dev *rt2x00dev,
 	enum dev_state state =
 	    (libconf->conf->flags & IEEE80211_CONF_PS) ?
 		STATE_SLEEP : STATE_AWAKE;
+	struct data_queue *queue;
 	u16 reg;
 
 	if (state == STATE_SLEEP) {
+		/*
+		 * Don't go to sleep when the TX queues are not empty.
+		 */
+		tx_queue_for_each(rt2x00dev, queue)
+			if (!rt2x00queue_empty(queue))
+				return;
+
 		rt2500usb_register_read(rt2x00dev, MAC_CSR18, &reg);
 		rt2x00_set_field16(&reg, MAC_CSR18_DELAY_AFTER_BEACON,
 				   rt2x00dev->beacon_int - 20);

Gertjan van Wingerde

2010-03-31 18:46 UTC #16 lore raw

Re: [PATCH] [RFC] rt2500pci: fix powersaving

Quoted 19 lines · Gertjan van Wingerde

Quoted 18 lines · Matthijs Kooijman
Hi Ondrej,
Quoted 1 line
phy0 -> rt2500pci_set_device_state: Error - Device failed to enter state 1 (-16).
I've been running with your patches (applied to 2.6.32.7) for a day of normal work, and the steady stream of these errors has stopped. During the entire day, I've seen the error once, perhaps a corner case here or there?
Overall, I have the feeling that connectivity is a bit more unstable with these two patches, however. On a few occasions, my SSH connections would hang for a bit. Sometimes pinging would be ok (and sometimes it wouldn't), so I haven't quite figured out if these problems were actually caused by the driver, or some other cause). Once I seemd to solve the problem by turning off powersaving, but that might have been a coincidence. Other times the problems went away by themselves.
Is there anything else I can do at the moment I'm seeing connection problems? Make a register dump or check some command?

Quoted 8 lines

Quoted 2 lines
I'll do some more testing, tonight on another (probably more stable) connection.
Could you try the attached patch on top of Ondrej's?
The legacy Ralink drivers do not go to sleep when there are still active entries in the TX queues. This patch mimics that behavior by refusing to go to sleep if there still are active entries in any of the TX queues.

Sorry, I got confused on which driver we were working on. Find attached the correct patch, now for the rt2500pci driver instead of the rt2500usb driver.

---
Gertjan

drivers/net/wireless/rt2x00/rt2500pci.c +8 -0

index b0c9502..a0441fa 100644
@@ -558,9 +558,17 @@ static void rt2500pci_config_ps(struct rt2x00_dev *rt2x00dev,
 	enum dev_state state =
 	    (libconf->conf->flags & IEEE80211_CONF_PS) ?
 		STATE_SLEEP : STATE_AWAKE;
+	struct data_queue *queue;
 	u32 reg;
 
 	if (state == STATE_SLEEP) {
+		/*
+		 * Don't go to sleep when the TX queues are not empty.
+		 */
+		tx_queue_for_each(rt2x00dev, queue)
+			if (!rt2x00queue_empty(queue))
+				return;
+
 		rt2x00pci_register_read(rt2x00dev, CSR20, &reg);
 		rt2x00_set_field32(&reg, CSR20_DELAY_AFTER_TBCN,
 				   (rt2x00dev->beacon_int - 20) * 16);

Matthijs Kooijman

2010-03-31 19:00 UTC #17 lore raw

Re: [rt2x00-users] [PATCH] [RFC] rt2500pci: fix powersaving

Hi Gertjan,

Quoted 2 lines

Sorry, I got confused on which driver we were working on. Find attached the correct patch, now for the rt2500pci driver instead of the rt2500usb driver.

I had already come up with that patch myself. Rebooting in a minute, I'll get back to you when I have something to tell :-)

Gr.

Matthijs

Gertjan van Wingerde

2010-04-03 14:11 UTC #18 lore raw

Re: [rt2x00-users] [PATCH] [RFC] rt2500pci: fix powersaving

Quoted 7 lines · Matthijs Kooijman

Hi Gertjan,
Quoted 2 lines
Sorry, I got confused on which driver we were working on. Find attached the correct patch, now for the rt2500pci driver instead of the rt2500usb driver.
I had already come up with that patch myself. Rebooting in a minute, I'll get back to you when I have something to tell :-)

Hi Matthijs,

Did you have the chance to test the patch and do you have some feedback if the patch helps improve the rt2500pci situation?

---
Gertjan.

Matthijs Kooijman

2010-04-03 15:28 UTC #19 lore raw

Re: [rt2x00-users] [PATCH] [RFC] rt2500pci: fix powersaving

Hi Gertjan,

Quoted 2 lines

Did you have the chance to test the patch and do you have some feedback if the patch helps improve the rt2500pci situation?

I've been running with all three patches applied for a while, and it does indeed seem more stable. I haven't seen the powersaving error anymore either (though I've not continuously checked for it). I haven't gotten any new problems.

Gr.

Matthijs