2013-08-26 13:18:43

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 3.11] 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

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 1f80ea5..a0119d3 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -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.
*
--
1.7.11.7



2013-08-27 09:08:57

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 3.11] rt2800: fix wrong TX power compensation

On Tue, Aug 27, 2013 at 10:22:45AM +0200, Paul Menzel wrote:
> > 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.

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



2013-08-27 08:15:32

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 3.11 v2] 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

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 1f80ea5..a0119d3 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -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.
*
--
1.7.11.7


2013-08-27 08:23:06

by Paul Menzel

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 3.11] rt2800: fix wrong TX power compensation

Am Dienstag, den 27.08.2013, 10:00 +0200 schrieb Stanislaw Gruszka:
> On Mon, Aug 26, 2013 at 10:19:58PM +0200, Paul Menzel wrote:
> > 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.

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

That would be great!

> 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


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part

2013-08-28 15:15:18

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 3.11 v2] rt2800: fix wrong TX power compensation

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

On Tue, Aug 27, 2013 at 10:13:40AM +0200, Stanislaw Gruszka wrote:
> 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
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index 1f80ea5..a0119d3 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -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.
> *
> --
> 1.7.11.7
>
>

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

2013-08-27 08:26:18

by Paul Menzel

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 3.11 v2] rt2800: fix wrong TX power compensation

Dear Stanislaw,


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


Am Dienstag, den 27.08.2013, 10:13 +0200 schrieb 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
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index 1f80ea5..a0119d3 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -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.

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

Thanks,

Paul


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part

2013-08-26 19:04:25

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 3.11] rt2800: fix wrong TX power compensation



Sent from my iPad

On 26 aug. 2013, at 15:18, Stanislaw Gruszka <[email protected]> wrote:

> 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]>

Acked-by: Gertjan van Wingerde <[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
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index 1f80ea5..a0119d3 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -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.
> *
> --
> 1.7.11.7
>
>
> _______________________________________________
> users mailing list
> [email protected]
> http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com

2013-08-27 09:15:26

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 3.11 v2] rt2800: fix wrong TX power compensation

On Tue, Aug 27, 2013 at 10:26:07AM +0200, Paul Menzel wrote:
> > /*
> > + * 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


2013-08-26 20:20:25

by Paul Menzel

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 3.11] rt2800: fix wrong TX power compensation

Am Montag, den 26.08.2013, 15:18 +0200 schrieb 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

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

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

• fixes

> some Ralink chips/devices.

Could you please add which ones you have tested?

Is there a bug report about this?

> 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
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index 1f80ea5..a0119d3 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -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.
> *

With the improvements to the commit message,

Acked-by: Paul Menzel <[email protected]>


Thanks,

Paul


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part

2013-08-27 08:28:22

by Daniel Golle

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 3.11] rt2800: fix wrong TX power compensation

On 08/27/2013 10:00 AM, Stanislaw Gruszka wrote:
> On Mon, Aug 26, 2013 at 10:19:58PM +0200, Paul Menzel wrote:
>> 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.


2013-08-27 08:01:55

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 3.11] rt2800: fix wrong TX power compensation

On Mon, Aug 26, 2013 at 10:19:58PM +0200, Paul Menzel wrote:
> 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.

> 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