[PATCH 3.11] rt2800: fix wrong TX power compensation

[PATCH 3.11] rt2800: fix wrong TX power compensation

[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.

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

Reported-and-tested-by: Fabien ADAM <id2ndr@crocobox.org>
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 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

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

[Gertjan van Wingerde]

Sent from my iPad

On 26 aug. 2013, at 15:18, Stanislaw Gruszka <sgruszka@redhat.com> 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 <id2ndr@crocobox.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>

Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>

> ---
> 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
> users@rt2x00.serialmonkey.com
> http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com

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

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]

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 <id2ndr@crocobox.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  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 <paulepanter@users.sourceforge.net>


Thanks,

Paul

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[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.

> 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

[PATCH 3.11 v2] rt2800: fix wrong TX power compensation

[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 <id2ndr@crocobox.org>
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>
Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
 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

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

[Daniel Golle]

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.

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

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]

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 <id2ndr@crocobox.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>
> Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
> ---
>  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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Stanislaw Gruszka]

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

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

[Stanislaw Gruszka]

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 

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

[John W. Linville]

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 <id2ndr@crocobox.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>
> Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
> ---
>  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
linville@tuxdriver.com			might be all we have.  Be ready.