rt2800: fix wrong TX power compensation

PATCH Superseded Stanislaw Gruszka 5 participants 11 messages Aug 26, 2013 – Aug 28, 2013 Networking drivers (wireless)

Share & tools

Stanislaw Gruszka

author

2013-08-26 13:18 UTC #0 lore raw

PATCH 1/1 rt2800: fix wrong TX power compensation

We should not do temperature compensation on devices without EXTERNAL_TX_ALC bit set (called DynamicTxAgcControl on vendor driver). Such devices can have totally bogus TSSI parameters on the EEPROM, but still threaded by us as valid and result doing wrong TX power calculations.

This fix inability to connect to AP on slightly longer distance on some Ralink chips/devices.

Reported-and-tested-by Fabien ADAM <[email protected]>

Cc [email protected]

Signed-off-by Stanislaw Gruszka <[email protected]>

---
 drivers/net/wireless/rt2x00/rt2800lib.c | 7 +++++++
 1 file changed, 7 insertions(+)

John,

If possible this should go to 3.11, -next & cc -stable is also fine as ususal.

Note that in -next version of the patch rt2x00_eeprom_read() should be changed to rt2800_eeprom_read() do to commit 3e38d3daf881a78ac13e93504a8ac5777040797e

drivers/net/wireless/rt2x00/rt2800lib.c +7 -0

index 1f80ea5..a0119d3 100644
@@ -2790,6 +2790,13 @@ static int rt2800_get_gain_calibration_delta(struct rt2x00_dev *rt2x00dev)
 	int i;
 
 	/*
+	 * First check if temperature compensation is supported.
+	 */
+	rt2x00_eeprom_read(rt2x00dev, EEPROM_NIC_CONF1, &eeprom);
+	if (!rt2x00_get_field16(eeprom, EEPROM_NIC_CONF1_EXTERNAL_TX_ALC))
+		return 0;
+
+	/*
 	 * Read TSSI boundaries for temperature compensation from
 	 * the EEPROM.
 	 *

Signature · 1 line

1.7.11.7

Gertjan van Wingerde

Acked-by on v1 1/1

2013-08-26 19:04 UTC #1 lore raw

Sent from my iPad

Quoted 6 lines · Stanislaw Gruszka

We should not do temperature compensation on devices without EXTERNAL_TX_ALC bit set (called DynamicTxAgcControl on vendor driver). Such devices can have totally bogus TSSI parameters on the EEPROM, but still threaded by us as valid and result doing wrong TX power calculations.

Quoted 6 lines

This fix inability to connect to AP on slightly longer distance on some Ralink chips/devices.
Reported-and-tested-by Fabien ADAM <[email protected]>
Cc [email protected]
Signed-off-by Stanislaw Gruszka <[email protected]>

Acked-by Gertjan van Wingerde <[email protected]>

Quoted 39 lines

---
drivers/net/wireless/rt2x00/rt2800lib.c | 7 +++++++

1 file changed, 7 insertions(+)

John,

If possible this should go to 3.11, -next & cc -stable is also fine as ususal.

Note that in -next version of the patch rt2x00_eeprom_read() should be changed to rt2800_eeprom_read() do to commit 3e38d3daf881a78ac13e93504a8ac5777040797e

drivers/net/wireless/rt2x00/rt2800lib.c +7 -0

index 1f80ea5..a0119d3 100644
@@ -2790,6 +2790,13 @@ static int rt2800_get_gain_calibration_delta(struct rt2x00_dev *rt2x00dev)
   int i;
 
   /*
+     * First check if temperature compensation is supported.
+     */
+    rt2x00_eeprom_read(rt2x00dev, EEPROM_NIC_CONF1, &eeprom);
+    if (!rt2x00_get_field16(eeprom, EEPROM_NIC_CONF1_EXTERNAL_TX_ALC))
+        return 0;
+
+    /*
    * Read TSSI boundaries for temperature compensation from
    * the EEPROM.
    *

Signature · 7 lines

1.7.11.7


_______________________________________________
users mailing list
[email protected]
http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com

Paul Menzel

Acked-by on v1 1/1

2013-08-26 20:20 UTC #2 lore raw

In reply to Stanislaw Gruszka

Am Montag, den 26.08.2013, 15:18 +0200 schrieb Stanislaw Gruszka:

Quoted 4 lines

We should not do temperature compensation on devices without EXTERNAL_TX_ALC bit set (called DynamicTxAgcControl on vendor driver). Such devices can have totally bogus TSSI parameters on the EEPROM, but still threaded by us as valid and result doing wrong TX power

• s,still threaded,are still treated,
• … and results in wrong TX power calculations.

Quoted 3 lines

calculations.
This fix inability to connect to AP on slightly longer distance on

• fixes

Quoted 1 line

some Ralink chips/devices.

Could you please add which ones you have tested?

Is there a bug report about this?

Quoted 24 lines

Reported-and-tested-by Fabien ADAM <[email protected]>
Cc [email protected]
Signed-off-by Stanislaw Gruszka <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2800lib.c | 7 +++++++
 1 file changed, 7 insertions(+)
John,
If possible this should go to 3.11, -next & cc -stable is also fine as ususal.
Note that in -next version of the patch rt2x00_eeprom_read() should be changed to rt2800_eeprom_read() do to commit 3e38d3daf881a78ac13e93504a8ac5777040797e
drivers/net/wireless/rt2x00/rt2800lib.c +0 -0
index 1f80ea5..a0119d3 100644
@@ -2790,6 +2790,13 @@ static int rt2800_get_gain_calibration_delta(struct rt2x00_dev *rt2x00dev)
 	int i;
 
 	/*

drivers/net/wireless/rt2x00/rt2800lib.c static int rt2800_get_gain_calibration_delta(struct rt2x00_dev *rt2x00dev) · line 2802

Quoted 10 lines

drivers/net/wireless/rt2x00/rt2800lib.c +7 -0

+	 * First check if temperature compensation is supported.
+	 */
+	rt2x00_eeprom_read(rt2x00dev, EEPROM_NIC_CONF1, &eeprom);
+	if (!rt2x00_get_field16(eeprom, EEPROM_NIC_CONF1_EXTERNAL_TX_ALC))
+		return 0;
+
+	/*
 	 * Read TSSI boundaries for temperature compensation from
 	 * the EEPROM.
 	 *

With the improvements to the commit message,

Acked-by Paul Menzel <[email protected]>

Thanks,

Paul

Stanislaw Gruszka

author

2013-08-27 08:01 UTC #3 lore raw

Quoted 1 line · Paul Menzel

Could you please add which ones you have tested?

I checked 2 devices (3071 & 3070) but they have EXTERNAL_TX_ALC bit set. I assume the fix is correct based on vendor driver and it fixes devices which have no EXTERNAL_TX_ACL, but I can not tell what chipset that are. Is possible that the same chipset can be configured with and without EXTERNAL_TX_ALC.

Quoted 1 line

Is there a bug report about this?

Fabien reported problem on mailing list, I can add that as reference. I hope it also fixes bug reported here:
https://bugzilla.redhat.com/show_bug.cgi?id=913631
but I haven't get confirmation about that from users yet.

Stanislaw

Stanislaw Gruszka

author

2013-08-27 08:15 UTC #4 lore raw

PATCH v2 1/1 rt2800: fix wrong TX power compensation

We should not do temperature compensation on devices without EXTERNAL_TX_ALC bit set (called DynamicTxAgcControl on vendor driver). Such devices can have totally bogus TSSI parameters on the EEPROM, but are still treated by us as valid and results in wrong TX power calculations.

This fixes inability to connect to AP on slightly longer distance on some Ralink chips/devices without EXTERNAL_TX_ALC configured.

Reference:
http://thread.gmane.org/gmane.linux.drivers.rt2x00.user/2263

Reported-and-tested-by Fabien ADAM <[email protected]>

Cc [email protected]

Signed-off-by Stanislaw Gruszka <[email protected]>

Acked-by Gertjan van Wingerde <[email protected]>

Acked-by Paul Menzel <[email protected]>

---
 drivers/net/wireless/rt2x00/rt2800lib.c | 7 +++++++
 1 file changed, 7 insertions(+)

v1 -> v2: fix changelog

John,

If possible this should go to 3.11, -next & cc -stable is also fine as usual.

Note that in -next version of the patch rt2x00_eeprom_read() should be changed to rt2800_eeprom_read() do to commit 3e38d3daf881a78ac13e93504a8ac5777040797e

drivers/net/wireless/rt2x00/rt2800lib.c +7 -0

index 1f80ea5..a0119d3 100644
@@ -2790,6 +2790,13 @@ static int rt2800_get_gain_calibration_delta(struct rt2x00_dev *rt2x00dev)
 	int i;
 
 	/*
+	 * First check if temperature compensation is supported.
+	 */
+	rt2x00_eeprom_read(rt2x00dev, EEPROM_NIC_CONF1, &eeprom);
+	if (!rt2x00_get_field16(eeprom, EEPROM_NIC_CONF1_EXTERNAL_TX_ALC))
+		return 0;
+
+	/*
 	 * Read TSSI boundaries for temperature compensation from
 	 * the EEPROM.
 	 *

Signature · 1 line

1.7.11.7

Paul Menzel

2013-08-27 08:26 UTC #5 lore raw

Dear Stanislaw,

thank you for the updated patch. One last suggestion inline.

Am Dienstag, den 27.08.2013, 10:13 +0200 schrieb Stanislaw Gruszka:

Quoted 37 lines

We should not do temperature compensation on devices without EXTERNAL_TX_ALC bit set (called DynamicTxAgcControl on vendor driver). Such devices can have totally bogus TSSI parameters on the EEPROM, but are still treated by us as valid and results in wrong TX power calculations.
This fixes inability to connect to AP on slightly longer distance on some Ralink chips/devices without EXTERNAL_TX_ALC configured.
Reference:
http://thread.gmane.org/gmane.linux.drivers.rt2x00.user/2263
Reported-and-tested-by Fabien ADAM <[email protected]>
Cc [email protected]
Signed-off-by Stanislaw Gruszka <[email protected]>
Acked-by Gertjan van Wingerde <[email protected]>
Acked-by Paul Menzel <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2800lib.c | 7 +++++++
 1 file changed, 7 insertions(+)
v1 -> v2: fix changelog
John,
If possible this should go to 3.11, -next & cc -stable is also fine as usual.
Note that in -next version of the patch rt2x00_eeprom_read() should be changed to rt2800_eeprom_read() do to commit 3e38d3daf881a78ac13e93504a8ac5777040797e
drivers/net/wireless/rt2x00/rt2800lib.c +0 -0
index 1f80ea5..a0119d3 100644

drivers/net/wireless/rt2x00/rt2800lib.c static int rt2800_get_gain_calibration_delta(struct rt2x00_dev *rt2x00dev) · line 2797

Quoted 8 lines

drivers/net/wireless/rt2x00/rt2800lib.c +5 -0

@@ -2790,6 +2790,13 @@ static int rt2800_get_gain_calibration_delta(struct rt2x00_dev *rt2x00dev)
 	int i;
 
 	/*
+	 * First check if temperature compensation is supported.
+	 */
+	rt2x00_eeprom_read(rt2x00dev, EEPROM_NIC_CONF1, &eeprom);
+	if (!rt2x00_get_field16(eeprom, EEPROM_NIC_CONF1_EXTERNAL_TX_ALC))
+		return 0;

Could a (debug) message be printed that temperature compensations is not supported? Not sure if that should be done in the library or the code calling this function.

drivers/net/wireless/rt2x00/rt2800lib.c static int rt2800_get_gain_calibration_delta(struct rt2x00_dev *rt2x00dev) · line 2802

Quoted 5 lines

drivers/net/wireless/rt2x00/rt2800lib.c +2 -0

+
+	/*
 	 * Read TSSI boundaries for temperature compensation from
 	 * the EEPROM.
 	 *

Thanks,

Paul

Stanislaw Gruszka

author

2013-08-27 09:15 UTC #6 lore raw

Quoted 10 lines · Paul Menzel

drivers/net/wireless/rt2x00/rt2800lib.c static int rt2800_get_gain_calibration_delta(struct rt2x00_dev *rt2x00dev) · line 2797
Quoted 6 lines
drivers/net/wireless/rt2x00/rt2800lib.c +5 -0
 	/*
+	 * First check if temperature compensation is supported.
+	 */
+	rt2x00_eeprom_read(rt2x00dev, EEPROM_NIC_CONF1, &eeprom);
+	if (!rt2x00_get_field16(eeprom, EEPROM_NIC_CONF1_EXTERNAL_TX_ALC))
+		return 0;
Could a (debug) message be printed that temperature compensations is not supported? Not sure if that should be done in the library or the code calling this function.

This is frequent path, so it will need to be a printk_once() based message, but perhaps would be better to print chip capabilities coded on the EEPROM together with "Chipset detected" message on device initialization.

Anyway, we can add debugging on separate patch.

Stanislaw

John W. Linville

2013-08-28 15:15 UTC #7 lore raw

In reply to Stanislaw Gruszka

I had already merged the first one, FWIW -- sorry...

Quoted 54 lines · Stanislaw Gruszka

We should not do temperature compensation on devices without EXTERNAL_TX_ALC bit set (called DynamicTxAgcControl on vendor driver). Such devices can have totally bogus TSSI parameters on the EEPROM, but are still treated by us as valid and results in wrong TX power calculations.
This fixes inability to connect to AP on slightly longer distance on some Ralink chips/devices without EXTERNAL_TX_ALC configured.
Reference:
http://thread.gmane.org/gmane.linux.drivers.rt2x00.user/2263
Reported-and-tested-by Fabien ADAM <[email protected]>
Cc [email protected]
Signed-off-by Stanislaw Gruszka <[email protected]>
Acked-by Gertjan van Wingerde <[email protected]>
Acked-by Paul Menzel <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2800lib.c | 7 +++++++
 1 file changed, 7 insertions(+)
v1 -> v2: fix changelog
John,
If possible this should go to 3.11, -next & cc -stable is also fine as usual.
Note that in -next version of the patch rt2x00_eeprom_read() should be changed to rt2800_eeprom_read() do to commit 3e38d3daf881a78ac13e93504a8ac5777040797e
drivers/net/wireless/rt2x00/rt2800lib.c +7 -0
index 1f80ea5..a0119d3 100644
@@ -2790,6 +2790,13 @@ static int rt2800_get_gain_calibration_delta(struct rt2x00_dev *rt2x00dev)
 	int i;
 
 	/*
+	 * First check if temperature compensation is supported.
+	 */
+	rt2x00_eeprom_read(rt2x00dev, EEPROM_NIC_CONF1, &eeprom);
+	if (!rt2x00_get_field16(eeprom, EEPROM_NIC_CONF1_EXTERNAL_TX_ALC))
+		return 0;
+
+	/*
 	 * Read TSSI boundaries for temperature compensation from
 	 * the EEPROM.
 	 *
Signature · 1 line
1.7.11.7

Signature · 2 lines

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

Paul Menzel

2013-08-27 08:23 UTC #8 lore raw

In reply to Stanislaw Gruszka

Am Dienstag, den 27.08.2013, 10:00 +0200 schrieb Stanislaw Gruszka:

Quoted 8 lines · Paul Menzel

Quoted 1 line
Could you please add which ones you have tested?
I checked 2 devices (3071 & 3070) but they have EXTERNAL_TX_ALC bit set. I assume the fix is correct based on vendor driver and it fixes devices which have no EXTERNAL_TX_ACL, but I can not tell what chipset that are. Is possible that the same chipset can be configured with and without EXTERNAL_TX_ALC.

I have the following device.

        $ lsusb -s 004
        Bus 002 Device 004: ID 148f:2870 Ralink Technology, Corp. RT2870 Wireless Adapter

How do I check if the bit is set? Sometimes the device disconnects when being 10 meters away from the access point. The connection is also slower than with an ath5k device right next to it.

Quoted 3 lines

Quoted 1 line
Is there a bug report about this?
Fabien reported problem on mailing list, I can add that as reference.

That would be great!

Quoted 3 lines

I hope it also fixes bug reported here:
https://bugzilla.redhat.com/show_bug.cgi?id=913631
but I haven't get confirmation about that from users yet.

Understood!

Thanks,

Paul

Stanislaw Gruszka

author

2013-08-27 09:08 UTC #9 lore raw

Quoted 6 lines · Paul Menzel

Quoted 5 lines
I checked 2 devices (3071 & 3070) but they have EXTERNAL_TX_ALC bit set. I assume the fix is correct based on vendor driver and it fixes devices which have no EXTERNAL_TX_ACL, but I can not tell what chipset that are. Is possible that the same chipset can be configured with and without EXTERNAL_TX_ALC.

Quoted 8 lines

I have the following device.
        $ lsusb -s 004
        Bus 002 Device 004: ID 148f:2870 Ralink Technology, Corp. RT2870 Wireless Adapter
How do I check if the bit is set? Sometimes the device disconnects when being 10 meters away from the access point. The connection is also slower than with an ath5k device right next to it.

If you compiled kernel with DEBUGFS support, you can check that by:

mount -t debugfs debugfs /sys/kernel/debug/
cd /sys/kernel/debug/ieee80211/phy0/rt2800usb/register/ echo "0x001b" > eeprom_offset
cat eeprom_value

Second less significant bit is EXTERNAL_TX_ALC .

I think your device has EXTERNAL_TX_ACL since it's older chip, but who knows ...

Stanislaw

Daniel Golle

2013-08-27 08:28 UTC #10 lore raw

In reply to Stanislaw Gruszka

Quoted 8 lines · Stanislaw Gruszka

Quoted 1 line · Paul Menzel
Could you please add which ones you have tested?
I checked 2 devices (3071 & 3070) but they have EXTERNAL_TX_ALC bit set. I assume the fix is correct based on vendor driver and it fixes devices which have no EXTERNAL_TX_ACL, but I can not tell what chipset that are. Is possible that the same chipset can be configured with and without EXTERNAL_TX_ALC.

At least for SoC (3352 and 5350) I know that there are devices with (e.g. DIR-615 rev.H1) and without EXTERNAL_TX_ALC (e.g. ALL5002). I guess the same applies for PCI modules.