2008-12-04 01:50:03

by Larry Finger

[permalink] [raw]
Subject: [PATCH] rtl8187: Improve wireless statistics for RTL8187

The current wireless statistics for the RTL8187 poorly indicate the sig=
nal
strength and quality. With testing, I found that the AGC value is inver=
sely
correlated with the strength as in the RTL8187B. By implementing a simi=
lar
calculation, much more code becomes common to the two devices.

Signed-off-by: Larry Finger <[email protected]>
Tested by: Mart=C3=ADn Ernesto Barreyro <[email protected]>
---

John,

This is 2.6.29 material.

Larry
---

Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_dev.c
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
@@ -313,29 +313,17 @@ static void rtl8187_rx_cb(struct urb *ur
struct rtl8187_rx_hdr *hdr =3D
(typeof(hdr))(skb_tail_pointer(skb) - sizeof(*hdr));
flags =3D le32_to_cpu(hdr->flags);
- signal =3D hdr->signal & 0x7f;
+ /* As with the RTL8187B below, the AGC is used to calculate
+ * signal strength and quality. In this case, the scaling
+ * constants are derived from the output of p54usb.
+ */
+ quality =3D 130 - ((41 * hdr->agc) >> 6);
+ if (quality > 100)
+ quality =3D 100;
+ signal =3D -4 - ((27 * hdr->agc) >> 6);
+ rx_status.qual =3D quality;
rx_status.antenna =3D (hdr->signal >> 7) & 1;
- rx_status.noise =3D hdr->noise;
rx_status.mactime =3D le64_to_cpu(hdr->mac_time);
- priv->quality =3D signal;
- rx_status.qual =3D priv->quality;
- priv->noise =3D hdr->noise;
- rate =3D (flags >> 20) & 0xF;
- if (rate > 3) { /* OFDM rate */
- if (signal > 90)
- signal =3D 90;
- else if (signal < 25)
- signal =3D 25;
- signal =3D 90 - signal;
- } else { /* CCK rate */
- if (signal > 95)
- signal =3D 95;
- else if (signal < 30)
- signal =3D 30;
- signal =3D 95 - signal;
- }
- rx_status.signal =3D signal;
- priv->signal =3D signal;
} else {
struct rtl8187b_rx_hdr *hdr =3D
(typeof(hdr))(skb_tail_pointer(skb) - sizeof(*hdr));
@@ -356,15 +344,15 @@ static void rtl8187_rx_cb(struct urb *ur
if (quality > 100)
quality =3D 100;
signal =3D 14 - hdr->agc / 2;
- rx_status.qual =3D quality;
- priv->quality =3D quality;
rx_status.signal =3D signal;
- priv->signal =3D signal;
rx_status.antenna =3D (hdr->rssi >> 7) & 1;
rx_status.mactime =3D le64_to_cpu(hdr->mac_time);
- rate =3D (flags >> 20) & 0xF;
}
=20
+ priv->quality =3D quality;
+ rx_status.signal =3D signal;
+ priv->signal =3D signal;
+ rate =3D (flags >> 20) & 0xF;
skb_trim(skb, flags & 0x0FFF);
rx_status.rate_idx =3D rate;
rx_status.freq =3D dev->conf.channel->center_freq;
@@ -1294,6 +1282,7 @@ static int __devinit rtl8187_probe(struc
=20
priv->mode =3D NL80211_IFTYPE_MONITOR;
dev->flags =3D IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
+ IEEE80211_HW_SIGNAL_DBM |
IEEE80211_HW_RX_INCLUDES_FCS;
=20
eeprom.data =3D dev;
@@ -1409,13 +1398,8 @@ static int __devinit rtl8187_probe(struc
(*channel++).hw_value =3D txpwr >> 8;
}
=20
- if (priv->is_rtl8187b) {
+ if (priv->is_rtl8187b)
printk(KERN_WARNING "rtl8187: 8187B chip detected.\n");
- dev->flags |=3D IEEE80211_HW_SIGNAL_DBM;
- } else {
- dev->flags |=3D IEEE80211_HW_SIGNAL_UNSPEC;
- dev->max_signal =3D 65;
- }
=20
/*
* XXX: Once this driver supports anything that requires


2008-12-04 03:20:54

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Improve wireless statistics for RTL8187

Hin-Tak Leung wrote:
> --- On Thu, 4/12/08, Larry Finger <[email protected]> wrote:
>=20
>> The current wireless statistics for the RTL8187 poorly
>> indicate the signal
>> strength and quality. With testing, I found that the AGC
>> value is inversely
>> correlated with the strength as in the RTL8187B. By
>> implementing a similar
>> calculation, much more code becomes common to the two
>> devices.
>>
>> Signed-off-by: Larry Finger
>> <[email protected]>
>> Tested by: Mart=EDn Ernesto Barreyro
>> <[email protected]>
>> ---
> Acked-by: Hin-Tak Leung <[email protected]>
>=20
> Acually eveything to do with "quality" can be moved out of the if/the=
n.
>=20
> I mean these 3 lines:
>=20
> + if (quality > 100)
> + quality =3D 100;
> + ....
> + rx_status.qual =3D quality;
>=20
> You have removed "rx_status.qual =3D quality;" from the "then" part, =
but didn't put it back in the common portion afterwards?
>=20
> Also, what happened to "dev->max_signal =3D 65;" at the end?

Thanks for the comments. I'll fix up the code and resubmit.

The dev->max_signal is only needed if the IEEE80211_HW_SIGNAL_UNSPEC fl=
ag is
used. Once we switch to IEEE80211_HW_SIGNAL_DBM, then the maximum is kn=
own.

Larry

2008-12-15 20:15:14

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Improve wireless statistics for RTL8187

On Mon, Dec 15, 2008 at 02:01:19PM -0600, Larry Finger wrote:
> John W. Linville wrote:
> > On Wed, Dec 03, 2008 at 09:20:46PM -0600, Larry Finger wrote:

> >> Thanks for the comments. I'll fix up the code and resubmit.
> >>
> >> The dev->max_signal is only needed if the IEEE80211_HW_SIGNAL_UNSPEC flag is
> >> used. Once we switch to IEEE80211_HW_SIGNAL_DBM, then the maximum is known.
> >
> > I'm dropping this patch from my mailbox. Is a new version of this
> > still coming?
>
> It is already in wireless-testing as commit b95e4bc3da1167bba718bc5. It was sent
> to you as "[PATCH V2] rtl8187: Improve wireless statistics for RTL8187" on Dec
> 3. The original should be dropped.

Ah, good...thanks!

John
--
John W. Linville Linux should be at the core
[email protected] of your literate lifestyle.

2008-12-15 20:01:26

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Improve wireless statistics for RTL8187

John W. Linville wrote:
> On Wed, Dec 03, 2008 at 09:20:46PM -0600, Larry Finger wrote:
>> Hin-Tak Leung wrote:
>>> --- On Thu, 4/12/08, Larry Finger <[email protected]> wrote=
:
>>>
>>>> The current wireless statistics for the RTL8187 poorly
>>>> indicate the signal
>>>> strength and quality. With testing, I found that the AGC
>>>> value is inversely
>>>> correlated with the strength as in the RTL8187B. By
>>>> implementing a similar
>>>> calculation, much more code becomes common to the two
>>>> devices.
>>>>
>>>> Signed-off-by: Larry Finger
>>>> <[email protected]>
>>>> Tested by: Mart=EDn Ernesto Barreyro
>>>> <[email protected]>
>>>> ---
>>> Acked-by: Hin-Tak Leung <[email protected]>
>>>
>>> Acually eveything to do with "quality" can be moved out of the if/t=
hen.
>>>
>>> I mean these 3 lines:
>>>
>>> + if (quality > 100)
>>> + quality =3D 100;
>>> + ....
>>> + rx_status.qual =3D quality;
>>>
>>> You have removed "rx_status.qual =3D quality;" from the "then" part=
, but didn't put it back in the common portion afterwards?
>>>
>>> Also, what happened to "dev->max_signal =3D 65;" at the end?
>> Thanks for the comments. I'll fix up the code and resubmit.
>>
>> The dev->max_signal is only needed if the IEEE80211_HW_SIGNAL_UNSPEC=
flag is
>> used. Once we switch to IEEE80211_HW_SIGNAL_DBM, then the maximum is=
known.
>=20
> I'm dropping this patch from my mailbox. Is a new version of this
> still coming?

It is already in wireless-testing as commit b95e4bc3da1167bba718bc5. It=
was sent
to you as "[PATCH V2] rtl8187: Improve wireless statistics for RTL8187"=
on Dec
3. The original should be dropped.

Larry

2008-12-15 19:45:16

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Improve wireless statistics for RTL8187

On Wed, Dec 03, 2008 at 09:20:46PM -0600, Larry Finger wrote:
> Hin-Tak Leung wrote:
> > --- On Thu, 4/12/08, Larry Finger <[email protected]> wrote=
:
> >=20
> >> The current wireless statistics for the RTL8187 poorly
> >> indicate the signal
> >> strength and quality. With testing, I found that the AGC
> >> value is inversely
> >> correlated with the strength as in the RTL8187B. By
> >> implementing a similar
> >> calculation, much more code becomes common to the two
> >> devices.
> >>
> >> Signed-off-by: Larry Finger
> >> <[email protected]>
> >> Tested by: Mart=EDn Ernesto Barreyro
> >> <[email protected]>
> >> ---
> > Acked-by: Hin-Tak Leung <[email protected]>
> >=20
> > Acually eveything to do with "quality" can be moved out of the if/t=
hen.
> >=20
> > I mean these 3 lines:
> >=20
> > + if (quality > 100)
> > + quality =3D 100;
> > + ....
> > + rx_status.qual =3D quality;
> >=20
> > You have removed "rx_status.qual =3D quality;" from the "then" part=
, but didn't put it back in the common portion afterwards?
> >=20
> > Also, what happened to "dev->max_signal =3D 65;" at the end?
>=20
> Thanks for the comments. I'll fix up the code and resubmit.
>=20
> The dev->max_signal is only needed if the IEEE80211_HW_SIGNAL_UNSPEC =
flag is
> used. Once we switch to IEEE80211_HW_SIGNAL_DBM, then the maximum is =
known.

I'm dropping this patch from my mailbox. Is a new version of this
still coming?

John
--=20
John W. Linville Linux should be at the core
[email protected] of your literate lifestyle.

2008-12-04 02:50:38

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: [PATCH] rtl8187: Improve wireless statistics for RTL8187

--- On Thu, 4/12/08, Larry Finger <[email protected]> wrote:

> The current wireless statistics for the RTL8187 poorly
> indicate the signal
> strength and quality. With testing, I found that the AGC
> value is inversely
> correlated with the strength as in the RTL8187B. By
> implementing a similar
> calculation, much more code becomes common to the two
> devices.
>=20
> Signed-off-by: Larry Finger
> <[email protected]>
> Tested by: Mart=EDn Ernesto Barreyro
> <[email protected]>
> ---
Acked-by: Hin-Tak Leung <[email protected]>

Acually eveything to do with "quality" can be moved out of the if/then.

I mean these 3 lines:

+ if (quality > 100)
+ quality =3D 100;
+ ....
+ rx_status.qual =3D quality;

You have removed "rx_status.qual =3D quality;" from the "then" part, bu=
t didn't put it back in the common portion afterwards?

Also, what happened to "dev->max_signal =3D 65;" at the end?



>=20
> John,
>=20
> This is 2.6.29 material.
>=20
> Larry
> ---
>=20
> Index:
> wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> ---
> wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_dev.c
> +++
> wireless-testing/drivers/net/wireless/rtl818x/rtl8187_dev.c
> @@ -313,29 +313,17 @@ static void rtl8187_rx_cb(struct urb
> *ur
> struct rtl8187_rx_hdr *hdr =3D
> (typeof(hdr))(skb_tail_pointer(skb) - sizeof(*hdr));
> flags =3D le32_to_cpu(hdr->flags);
> - signal =3D hdr->signal & 0x7f;
> + /* As with the RTL8187B below, the AGC is used to
> calculate
> + * signal strength and quality. In this case, the
> scaling
> + * constants are derived from the output of p54usb.
> + */
> + quality =3D 130 - ((41 * hdr->agc) >> 6);
> + if (quality > 100)
> + quality =3D 100;
> + signal =3D -4 - ((27 * hdr->agc) >> 6);
> + rx_status.qual =3D quality;
> rx_status.antenna =3D (hdr->signal >> 7) & 1;
> - rx_status.noise =3D hdr->noise;
> rx_status.mactime =3D le64_to_cpu(hdr->mac_time);
> - priv->quality =3D signal;
> - rx_status.qual =3D priv->quality;
> - priv->noise =3D hdr->noise;
> - rate =3D (flags >> 20) & 0xF;
> - if (rate > 3) { /* OFDM rate */
> - if (signal > 90)
> - signal =3D 90;
> - else if (signal < 25)
> - signal =3D 25;
> - signal =3D 90 - signal;
> - } else { /* CCK rate */
> - if (signal > 95)
> - signal =3D 95;
> - else if (signal < 30)
> - signal =3D 30;
> - signal =3D 95 - signal;
> - }
> - rx_status.signal =3D signal;
> - priv->signal =3D signal;
> } else {
> struct rtl8187b_rx_hdr *hdr =3D
> (typeof(hdr))(skb_tail_pointer(skb) - sizeof(*hdr));
> @@ -356,15 +344,15 @@ static void rtl8187_rx_cb(struct urb
> *ur
> if (quality > 100)
> quality =3D 100;
> signal =3D 14 - hdr->agc / 2;
> - rx_status.qual =3D quality;
> - priv->quality =3D quality;
> rx_status.signal =3D signal;
> - priv->signal =3D signal;
> rx_status.antenna =3D (hdr->rssi >> 7) & 1;
> rx_status.mactime =3D le64_to_cpu(hdr->mac_time);
> - rate =3D (flags >> 20) & 0xF;
> }
> =20
> + priv->quality =3D quality;
> + rx_status.signal =3D signal;
> + priv->signal =3D signal;
> + rate =3D (flags >> 20) & 0xF;
> skb_trim(skb, flags & 0x0FFF);
> rx_status.rate_idx =3D rate;
> rx_status.freq =3D dev->conf.channel->center_freq;
> @@ -1294,6 +1282,7 @@ static int __devinit
> rtl8187_probe(struc
> =20
> priv->mode =3D NL80211_IFTYPE_MONITOR;
> dev->flags =3D IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING
> |
> + IEEE80211_HW_SIGNAL_DBM |
> IEEE80211_HW_RX_INCLUDES_FCS;
> =20
> eeprom.data =3D dev;
> @@ -1409,13 +1398,8 @@ static int __devinit
> rtl8187_probe(struc
> (*channel++).hw_value =3D txpwr >> 8;
> }
> =20
> - if (priv->is_rtl8187b) {
> + if (priv->is_rtl8187b)
> printk(KERN_WARNING "rtl8187: 8187B chip
> detected.\n");
> - dev->flags |=3D IEEE80211_HW_SIGNAL_DBM;
> - } else {
> - dev->flags |=3D IEEE80211_HW_SIGNAL_UNSPEC;
> - dev->max_signal =3D 65;
> - }
> =20
> /*
> * XXX: Once this driver supports anything that requires


=20