2009-11-05 06:09:47

by Larry Finger

[permalink] [raw]
Subject: [PATCH] rtl8187: Remove deprecated 'qual' from returned RX status

The qual member of ieee80211_rx_status is deprecated. As a result, this
driver no longer needs to calculate a quality value.

Signed-off-by: Larry Finger <[email protected]>
---

John,

This is 2.6.33 material.

Larry
---

Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_dev.c
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
@@ -320,7 +320,6 @@ static void rtl8187_rx_cb(struct urb *ur
struct ieee80211_rx_status rx_status = { 0 };
int rate, signal;
u32 flags;
- u32 quality;
unsigned long f;

spin_lock_irqsave(&priv->rx_queue.lock, f);
@@ -338,10 +337,9 @@ static void rtl8187_rx_cb(struct urb *ur
(typeof(hdr))(skb_tail_pointer(skb) - sizeof(*hdr));
flags = le32_to_cpu(hdr->flags);
/* As with the RTL8187B below, the AGC is used to calculate
- * signal strength and quality. In this case, the scaling
+ * signal strength. In this case, the scaling
* constants are derived from the output of p54usb.
*/
- quality = 130 - ((41 * hdr->agc) >> 6);
signal = -4 - ((27 * hdr->agc) >> 6);
rx_status.antenna = (hdr->signal >> 7) & 1;
rx_status.mactime = le64_to_cpu(hdr->mac_time);
@@ -354,23 +352,18 @@ static void rtl8187_rx_cb(struct urb *ur
* In testing, none of these quantities show qualitative
* agreement with AP signal strength, except for the AGC,
* which is inversely proportional to the strength of the
- * signal. In the following, the quality and signal strength
- * are derived from the AGC. The arbitrary scaling constants
+ * signal. In the following, the signal strength
+ * is derived from the AGC. The arbitrary scaling constants
* are chosen to make the results close to the values obtained
* for a BCM4312 using b43 as the driver. The noise is ignored
* for now.
*/
flags = le32_to_cpu(hdr->flags);
- quality = 170 - hdr->agc;
signal = 14 - hdr->agc / 2;
rx_status.antenna = (hdr->rssi >> 7) & 1;
rx_status.mactime = le64_to_cpu(hdr->mac_time);
}

- if (quality > 100)
- quality = 100;
- rx_status.qual = quality;
- priv->quality = quality;
rx_status.signal = signal;
priv->signal = signal;
rate = (flags >> 20) & 0xF;
Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187.h
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187.h
@@ -119,7 +119,6 @@ struct rtl8187_priv {
} hw_rev;
struct sk_buff_head rx_queue;
u8 signal;
- u8 quality;
u8 noise;
u8 slot_time;
u8 aifsn[4];


2009-11-05 19:43:11

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Remove deprecated 'qual' from returned RX status

On Thu, Nov 5, 2009 at 7:24 PM, Michael Buesch <[email protected]> wrote:
> On Thursday 05 November 2009 20:21:39 Hin-Tak Leung wrote:
>> > + * is derived from the AGC. The arbitrary scaling constants
>> > * are chosen to make the results close to the values obtained
>> > * for a BCM4312 using b43 as the driver. The noise is ignored
>
> It's not really a good idea to take b43 for reference, because the values/scales
> are chosen pretty arbitrarily in that driver. So you basically end up
> with an arbitrary^2 value.
>
> --
> Greetings, Michael.
>

Well, onee needs to start from something... many users, bug reporters
have often quoted 'I used driver/device X and it shows 90% strength,
yours shows 60%..." and vice versa. They don't know the numbers are
made up in both drivers, just differently :-).

Hin-Tak

2009-11-06 00:56:29

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Remove deprecated 'qual' from returned RX status

On 11/05/2009 01:24 PM, Michael Buesch wrote:
> On Thursday 05 November 2009 20:21:39 Hin-Tak Leung wrote:
>>> + * is derived from the AGC. The arbitrary scaling constants
>>> * are chosen to make the results close to the values obtained
>>> * for a BCM4312 using b43 as the driver. The noise is ignored
>
> It's not really a good idea to take b43 for reference, because the values/scales
> are chosen pretty arbitrarily in that driver. So you basically end up
> with an arbitrary^2 value.

When I did that, b43 was the only other device that I had available.
As Hin-Tak said, one has to start somewhere. As we all know, only
relative values are significant, and the absolute value has little
meaning.

Larry

2009-11-05 19:24:58

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Remove deprecated 'qual' from returned RX status

On Thursday 05 November 2009 20:21:39 Hin-Tak Leung wrote:
> > + * is derived from the AGC. The arbitrary scaling constants
> > * are chosen to make the results close to the values obtained
> > * for a BCM4312 using b43 as the driver. The noise is ignored

It's not really a good idea to take b43 for reference, because the values/scales
are chosen pretty arbitrarily in that driver. So you basically end up
with an arbitrary^2 value.

--
Greetings, Michael.

2009-11-05 19:21:35

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Remove deprecated 'qual' from returned RX status

On Thu, Nov 5, 2009 at 6:09 AM, Larry Finger <[email protected]> wrote:
> The qual member of ieee80211_rx_status is deprecated. As a result, this
> driver no longer needs to calculate a quality value.
>
> Signed-off-by: Larry Finger <[email protected]>

Acked-by: Hin-Tak Leung <[email protected]>

a bit busy lately. I'l give this a try soonest. BTW, well-done with
the oops on unplug.

Hin-Tak

> ---
>
> John,
>
> This is 2.6.33 material.
>
> Larry
> ---
>
> Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_dev.c
> +++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
> @@ -320,7 +320,6 @@ static void rtl8187_rx_cb(struct urb *ur
> struct ieee80211_rx_status rx_status = { 0 };
> int rate, signal;
> u32 flags;
> - u32 quality;
> unsigned long f;
>
> spin_lock_irqsave(&priv->rx_queue.lock, f);
> @@ -338,10 +337,9 @@ static void rtl8187_rx_cb(struct urb *ur
> (typeof(hdr))(skb_tail_pointer(skb) - sizeof(*hdr));
> flags = le32_to_cpu(hdr->flags);
> /* As with the RTL8187B below, the AGC is used to calculate
> - * signal strength and quality. In this case, the scaling
> + * signal strength. In this case, the scaling
> * constants are derived from the output of p54usb.
> */
> - quality = 130 - ((41 * hdr->agc) >> 6);
> signal = -4 - ((27 * hdr->agc) >> 6);
> rx_status.antenna = (hdr->signal >> 7) & 1;
> rx_status.mactime = le64_to_cpu(hdr->mac_time);
> @@ -354,23 +352,18 @@ static void rtl8187_rx_cb(struct urb *ur
> * In testing, none of these quantities show qualitative
> * agreement with AP signal strength, except for the AGC,
> * which is inversely proportional to the strength of the
> - * signal. In the following, the quality and signal strength
> - * are derived from the AGC. The arbitrary scaling constants
> + * signal. In the following, the signal strength
> + * is derived from the AGC. The arbitrary scaling constants
> * are chosen to make the results close to the values obtained
> * for a BCM4312 using b43 as the driver. The noise is ignored
> * for now.
> */
> flags = le32_to_cpu(hdr->flags);
> - quality = 170 - hdr->agc;
> signal = 14 - hdr->agc / 2;
> rx_status.antenna = (hdr->rssi >> 7) & 1;
> rx_status.mactime = le64_to_cpu(hdr->mac_time);
> }
>
> - if (quality > 100)
> - quality = 100;
> - rx_status.qual = quality;
> - priv->quality = quality;
> rx_status.signal = signal;
> priv->signal = signal;
> rate = (flags >> 20) & 0xF;
> Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187.h
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187.h
> +++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187.h
> @@ -119,7 +119,6 @@ struct rtl8187_priv {
> } hw_rev;
> struct sk_buff_head rx_queue;
> u8 signal;
> - u8 quality;
> u8 noise;
> u8 slot_time;
> u8 aifsn[4];
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>