rt2x00: Hardcode TX ack timeout and consume time

PATCH Ivo van Doorn 3 participants 5 messages Sep 6, 2009 – Sep 10, 2009 Networking drivers (wireless)

Share & tools

Ivo van Doorn

author

2009-09-06 13:14 UTC #0 lore raw

rt2x00: Hardcode TX ack timeout and consume time

The calculated values for the ACK timeout and ACK
consume time are different then the values as
used by the Legacy drivers.

After testing from James Ledwith it appeared that the calculated values caused a high amount of TX failures, and the values from the Legacy drivers were the most optimal to prevent TX failure due to excessive retries.

The symptoms of this problem:
- Rate control module always falls back to 1Mbs
- Low throughput when bitrate was fixed

Possible side-effects (not confirmed but highly likely)
- Problems with DHCP
- Broken connections due to lack of probe response

This should fix at least:
Kernel bugzilla reports: [13362], [13009], [9273]
Fedora bugzilla reports: [443203]
but possible some additional bugs as well.

Signed-off-by Ivo van Doorn <[email protected]>

---
John, this patch has not yet been confirmed by any of the above bugzilla reports, but I do think this should be merged as soon as possible to 2.6.31 since this issue has been marked as regression for quite some time now. :)
Thanks.
---

drivers/net/wireless/rt2x00/rt2400pci.c +2 -3

index 164df93..798f625 100644
@@ -331,9 +331,8 @@ static void rt2400pci_config_erp(struct rt2x00_dev *rt2x00dev,
 	preamble_mask = erp->short_preamble << 3;
 
 	rt2x00pci_register_read(rt2x00dev, TXCSR1, &reg);
-	rt2x00_set_field32(&reg, TXCSR1_ACK_TIMEOUT, erp->ack_timeout);
-	rt2x00_set_field32(&reg, TXCSR1_ACK_CONSUME_TIME,
-			   erp->ack_consume_time);
+	rt2x00_set_field32(&reg, TXCSR1_ACK_TIMEOUT, 0x1ff);
+	rt2x00_set_field32(&reg, TXCSR1_ACK_CONSUME_TIME, 0x13a);
 	rt2x00_set_field32(&reg, TXCSR1_TSF_OFFSET, IEEE80211_HEADER);
 	rt2x00_set_field32(&reg, TXCSR1_AUTORESPONDER, 1);
 	rt2x00pci_register_write(rt2x00dev, TXCSR1, reg);

drivers/net/wireless/rt2x00/rt2500pci.c +2 -3

index 4186582..2e872ac 100644
@@ -337,9 +337,8 @@ static void rt2500pci_config_erp(struct rt2x00_dev *rt2x00dev,
 	preamble_mask = erp->short_preamble << 3;
 
 	rt2x00pci_register_read(rt2x00dev, TXCSR1, &reg);
-	rt2x00_set_field32(&reg, TXCSR1_ACK_TIMEOUT, erp->ack_timeout);
-	rt2x00_set_field32(&reg, TXCSR1_ACK_CONSUME_TIME,
-			   erp->ack_consume_time);
+	rt2x00_set_field32(&reg, TXCSR1_ACK_TIMEOUT, 0x162);
+	rt2x00_set_field32(&reg, TXCSR1_ACK_CONSUME_TIME, 0xa2);
 	rt2x00_set_field32(&reg, TXCSR1_TSF_OFFSET, IEEE80211_HEADER);
 	rt2x00_set_field32(&reg, TXCSR1_AUTORESPONDER, 1);
 	rt2x00pci_register_write(rt2x00dev, TXCSR1, reg);

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

index b04f59b..22dd6d9 100644
@@ -488,10 +488,6 @@ static void rt2500usb_config_erp(struct rt2x00_dev *rt2x00dev,
 {
 	u16 reg;
 
-	rt2500usb_register_read(rt2x00dev, TXRX_CSR1, &reg);
-	rt2x00_set_field16(&reg, TXRX_CSR1_ACK_TIMEOUT, erp->ack_timeout);
-	rt2500usb_register_write(rt2x00dev, TXRX_CSR1, reg);
-
 	rt2500usb_register_read(rt2x00dev, TXRX_CSR10, &reg);
 	rt2x00_set_field16(&reg, TXRX_CSR10_AUTORESPOND_PREAMBLE,
 			   !!erp->short_preamble);

drivers/net/wireless/rt2x00/rt2800usb.c +1 -2

index 1307873..9fe770f 100644
@@ -580,8 +580,7 @@ static void rt2800usb_config_erp(struct rt2x00_dev *rt2x00dev,
 	u32 reg;
 
 	rt2x00usb_register_read(rt2x00dev, TX_TIMEOUT_CFG, &reg);
-	rt2x00_set_field32(&reg, TX_TIMEOUT_CFG_RX_ACK_TIMEOUT,
-			   DIV_ROUND_UP(erp->ack_timeout, erp->slot_time));
+	rt2x00_set_field32(&reg, TX_TIMEOUT_CFG_RX_ACK_TIMEOUT, 0x20);
 	rt2x00usb_register_write(rt2x00dev, TX_TIMEOUT_CFG, reg);
 
 	rt2x00usb_register_read(rt2x00dev, AUTO_RSP_CFG, &reg);

drivers/net/wireless/rt2x00/rt2x00.h +0 -3

index 7bede90..196de8a 100644
@@ -424,9 +424,6 @@ struct rt2x00lib_erp {
 	int short_preamble;
 	int cts_protection;
 
-	int ack_timeout;
-	int ack_consume_time;
-
 	u32 basic_rates;
 
 	int slot_time;

drivers/net/wireless/rt2x00/rt2x00config.c +0 ~~-11~~

index 3501788..40a201e 100644
@@ -94,17 +94,6 @@ void rt2x00lib_config_erp(struct rt2x00_dev *rt2x00dev,
 	erp.difs = bss_conf->use_short_slot ? SHORT_DIFS : DIFS;
 	erp.eifs = bss_conf->use_short_slot ? SHORT_EIFS : EIFS;
 
-	erp.ack_timeout = PLCP + erp.difs + GET_DURATION(ACK_SIZE, 10);
-	erp.ack_consume_time = SIFS + PLCP + GET_DURATION(ACK_SIZE, 10);
-
-	if (bss_conf->use_short_preamble) {
-		erp.ack_timeout += SHORT_PREAMBLE;
-		erp.ack_consume_time += SHORT_PREAMBLE;
-	} else {
-		erp.ack_timeout += PREAMBLE;
-		erp.ack_consume_time += PREAMBLE;
-	}
-
 	erp.basic_rates = bss_conf->basic_rates;
 	erp.beacon_int = bss_conf->beacon_int;

drivers/net/wireless/rt2x00/rt61pci.c +1 -1

index f4b4b86..b20e3ea 100644
@@ -598,7 +598,7 @@ static void rt61pci_config_erp(struct rt2x00_dev *rt2x00dev,
 	u32 reg;
 
 	rt2x00pci_register_read(rt2x00dev, TXRX_CSR0, &reg);
-	rt2x00_set_field32(&reg, TXRX_CSR0_RX_ACK_TIMEOUT, erp->ack_timeout);
+	rt2x00_set_field32(&reg, TXRX_CSR0_RX_ACK_TIMEOUT, 0x32);
 	rt2x00_set_field32(&reg, TXRX_CSR0_TSF_OFFSET, IEEE80211_HEADER);
 	rt2x00pci_register_write(rt2x00dev, TXRX_CSR0, reg);

drivers/net/wireless/rt2x00/rt73usb.c +1 -1

index 90e1172..1cbd9b4 100644
@@ -561,7 +561,7 @@ static void rt73usb_config_erp(struct rt2x00_dev *rt2x00dev,
 	u32 reg;
 
 	rt2x00usb_register_read(rt2x00dev, TXRX_CSR0, &reg);
-	rt2x00_set_field32(&reg, TXRX_CSR0_RX_ACK_TIMEOUT, erp->ack_timeout);
+	rt2x00_set_field32(&reg, TXRX_CSR0_RX_ACK_TIMEOUT, 0x32);
 	rt2x00_set_field32(&reg, TXRX_CSR0_TSF_OFFSET, IEEE80211_HEADER);
 	rt2x00usb_register_write(rt2x00dev, TXRX_CSR0, reg);

John W. Linville

2009-09-08 18:45 UTC #1 lore raw

Quoted 5 lines · Ivo van Doorn

rt2x00: Hardcode TX ack timeout and consume time
John, this patch has not yet been confirmed by any of the above bugzilla reports, but I do think this should be merged as soon as possible to 2.6.31 since this issue has been marked as regression for quite some time now. :)

Well, given how late it is in the 2.6.31 cycle and the fact that this problem has persisted for a while I think I'll just let this slip to 2.6.32 -- good enough?

John

Signature · 2 lines

John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.

Ivo van Doorn

author

2009-09-10 22:19 UTC #2 lore raw

Quoted 10 lines · John W. Linville

Quoted 5 lines · Ivo van Doorn
rt2x00: Hardcode TX ack timeout and consume time
John, this patch has not yet been confirmed by any of the above bugzilla reports, but I do think this should be merged as soon as possible to 2.6.31 since this issue has been marked as regression for quite some time now. :)
Well, given how late it is in the 2.6.31 cycle and the fact that this problem has persisted for a while I think I'll just let this slip to 2.6.32 -- good enough?

Well preferably not, but I can sit it out and try to push it into -stable for 2.6.31.1. That might be the easiest solution which worked the last time when I had an urgent fix late in the release cycle as well. :)

Ivo

Richard Zidlicky

2009-09-08 19:45 UTC #3 lore raw

In reply to Ivo van Doorn

Quoted 1 line · Ivo van Doorn

rt2x00: Hardcode TX ack timeout and consume time

Hi,

it does significantly improve rt73usb performance and stability of ping times.

I can still get almost the double performance and much better ping times when setting the rate manually.

Quoted 3 lines

John, this patch has not yet been confirmed by any of the above bugzilla reports, but I do think this should be merged as soon as possible to 2.6.31 since this issue has been marked as regression for quite some time now. :)

imho it should be merged even if it does not fully fix every imaginable problem.

Actually I would wish critical fixes would be merged much more aggressively, eg rt73usb was critically broken before Pavel Roskin' recent off-by-1 register access fix - causing all kinds of weird problems probably including filesystem damage.

Seems some other fixes are also critical - it would be nice to have all important fixes in eg in 2.6.30.6 (if it ever comes) as it is not quite trivial to cherrypick them from the list when the code diverges too much from mainline.

Richard

Richard Zidlicky

2009-09-08 19:34 UTC #4 lore raw

Quoted 2 lines · Richard Zidlicky

it does significantly improve rt73usb performance and stability of ping times.

further data.. downloads improve significantly but uploads still require manual rate setting else they fall back to 1MBit immediately.

Richard