2009-03-30 13:57:29

by Ivan Kuten

[permalink] [raw]
Subject: rtl8187 diversity

Hello!

Anybody is working on adding antenna diversity feature to rtl8187 driver?

Regards,
Ivan



2009-03-30 19:08:11

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: rtl8187 diversity

On Mon, Mar 30, 2009 at 1:38 PM, Ivan Kuten <[email protected]> wr=
ote:
> Hello!
>
> Anybody is working on adding antenna diversity feature to rtl8187 dri=
ver?

Which of rtl8187 comes with multiple antenna, etc which would benefit
from this feature?

>
> Regards,
> Ivan
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wirel=
ess" in
> the body of a message to [email protected]
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>

2009-03-31 09:35:32

by Ivan Kuten

[permalink] [raw]
Subject: Re: rtl8187 diversity

Hin-Tak Leung wrote:
> On Mon, Mar 30, 2009 at 1:38 PM, Ivan Kuten <[email protected]> wrote:
>> Hello!
>>
>> Anybody is working on adding antenna diversity feature to rtl8187 driver?
>
> Which of rtl8187 comes with multiple antenna, etc which would benefit
> from this feature?
>

We are using WMG2503 module (with 2 antenna) from Abocom on our Blackfin DSP based board.
We did some preliminary work based on rtl8187L_linux_26.1036.1105.2008, however there are too
many unclear points.

Regards,
Ivan

>> Regards,
>> Ivan
>>
>> --
>> 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
>>
> --
> 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
>
>
>


2009-04-08 23:37:00

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: rtl8187 diversity

First of all, thanks a lot for posting the patch.

The use of compat-wireless-old is presumably for older kernel
side-by-side comparison on an older kernel?
In any case, starting from somewhere is better than not-starting :-).

>> However while testing, the switching between antennas doesn't give any
>> download speed improvements.
>> May be we missed some point.

You are confusing throughput with signal quality - unless the
improvement with signal quality is dramatic enough to affect the
rate-controlling mechanism (to bump the driver up to a higher rate),
it probably will just result in more "steady" download (i.e. with less
fluctuations), and you probably need to be moving around a bit to
notice this.

> I don't like the idea of having the antenna diversity as a compile-time option.
> That works OK for a single user, but it fails when distros are considered. They
> would have to have it on for all cases even though very few users would need it.
> Does the EEPROM for your device have special encoding that indicates the
> presence of multiple antennas? You should be able to compile the antenna
> diversity code unconditionally and use such a special EEPROM value.
> Alternatively, the USB ID's (13d1:abe6) could be used to select its execution.

I agree this should not be a compile-time option; it should either
auto-detect -

- is based on USB vid/pid's reliable? enumeration of the number of antennas
(presumably there is a way of asking the device itself how many
antennas it is attached to)
and the code simply cycles/alternates between all antennas; this might
actually punnish the
majority of the single-antenna users -

or a run-time configurable option, i.e. via iwconfig/iwpriv/iw . It
might be as simple as
making the antenna switching no-op in the case of single antenna; but
whatever needs to be done between single and multiple antennas should
be require a module recompile.

On the patch itself - I suggest to separate the procfs related stuff
as a separate patch, or even removing it(?) it is probably only useful
for devel? and hard coding "#define VAL_ARRAY_SIZE 10"?

2009-04-22 19:36:08

by Larry Finger

[permalink] [raw]
Subject: Re: rtl8187 diversity

Eugene Sobol wrote:
>
> This is reworked patch for antenna diversity in rtl8187 compat wireless.
> - removed proc file system functionality
> - reworked calculation of agc
> - fixed codestyle
> Next step I will create for 2.6.30 RC2

It would be better if the patch were made for wireless-testing, which contains
some changes that will not appear in mainline until 2.6.31.

--snip--

{USB_DEVICE(0x03f0, 0xca02), .driver_info = DEVICE_RTL8187},
/* Sitecom */
{USB_DEVICE(0x0df6, 0x000d), .driver_info = DEVICE_RTL8187},
+

What is the vendor for your device? At http://www.linux-usb.org/usb.ids, this ID
is shown as belonging to A-MAX Technology Macao Commercial Offshore Co. Ltd.
That is quite a mouthful, and the above line might say "A-Max Technology",
unless you have a different vendor.

+ {USB_DEVICE(0x13d1, 0xabe6), .driver_info = DEVICE_RTL8187},
+

There should not be a blank line here.

--snip--

@@ -724,6 +738,8 @@
u32 reg;
int ret;

+ priv->hw = dev;

This will not be needed. See the comment in the patch for rtl8187.h.

--snip--

@@ -970,6 +995,85 @@
rtl818x_iowrite32_async(priv, &priv->map->RX_CONF, priv->rx_conf);
}

+#define ANTENNA_DIVERSITY_TIMER_PERIOD 1000
+#define ANTENNA_SWITCH_TIMER_PERIOD 300
+#define ANTENNA_MAX_AGC_DIFF 15
+void sw_antenna_diversity_timer_callback(struct rtl8187_priv *priv)
+{
+ static int average_old;
+ int average_current = 0;
+ int i, sub;
+
+ if (!priv->ant_diversity_enabled)
+ return;
+ for (i = 0; i < VAL_ARRAY_SIZE; ++i)
+ average_current += priv->ant_diversity.agc_array[i];
+
+ average_current /= VAL_ARRAY_SIZE;
+ sub = average_current - average_old;
+ if (sub > ANTENNA_MAX_AGC_DIFF) {
+ priv->ant_diversity.switch_to ^= 1;
+ queue_delayed_work(priv->hw->workqueue, &priv->antenna_work,
+ msecs_to_jiffies(ANTENNA_SWITCH_TIMER_PERIOD));

Should be priv->dev->workqueue. Everywhere you have priv->hw, it should be
priv->dev.

--snip--

@@ -1123,6 +1229,7 @@
break;
default:
chip_name = "RTL8187vB (default)";
+ priv->ant_diversity_enabled = 1;

Is the default branch correct here? What is the value read from 0xFFE1? I'm
concerned that antenna diversity might be turned on for some unit that doesn't
support it.

The rtl8187se vendor driver in drivers/staging does the equivalent of:

rtl818x_iowrite8(priv, &priv->map->EEPROM_CMD, \
RTL818X_EEPROM_CMD_CONFIG);
if (rtl818x_ioread8(priv, &priv->map->CONFIG2) & 1 << 6)
priv->ant_diversity_enabled = 1;
else
priv->ant_diversity_enabled = 0;

This method should be tested to see if it works for USB devices in general, and
yours in particular.

--snip--

struct rtl8187_priv {
/* common between rtl818x drivers */
struct rtl818x_csr *map;
const struct rtl818x_rf_ops *rf;
struct ieee80211_vif *vif;
+ struct ieee80211_hw *hw;

Look about 15 lines down and see the "struct ieee80211hw *dev". It should be
used. It is badly named, but changing it would be disruptive.

Good work. This version is much improved.

Larry

2009-04-08 18:05:06

by Larry Finger

[permalink] [raw]
Subject: Re: rtl8187 diversity

eugene.sobol wrote:
> Hello!
>
> We are using RTL8187L module (with 2 antenna) from Abocom on our Blackfin DSP
> based board.
> We have to enable antenna diversity for that module.
>
> We have done some workaround realted with it:
> I got compat-wireless archive from:
> http://wireless.kernel.org/download/compat-wireless-2.6/compat-wireless-old.tar.bz2
> I added switch-antenna related code from rtl8187L_linux_26.1036.1105.2008
> driver.
> I added simple logic how to evaluate quality of signal by AGC and switch
> between antennas.
>
> Following code shows how does swiching between antennas work (from
> rtl8187L_linux_26.1036.1105.2008 ) :
> + write_nic_byte(priv, 0x7f, ((0x01009b90 & 0xff000000) >> 24));
> + write_nic_byte(priv, 0x7e, ((0x01009b90 & 0x00ff0000) >> 16));
> + write_nic_byte(priv, 0x7d, ((0x01009b90 & 0x0000ff00) >> 8));
> + write_nic_byte(priv, 0x7c, ((0x01009b90 & 0x000000ff) >> 0));
> +
> + // Rx OFDM.
> + /* original code from realtek driver */
> + write_nic_byte(priv, 0x7f, ((0x000090a6 & 0xff000000) >> 24));
> + write_nic_byte(priv, 0x7e, ((0x000090a6 & 0x00ff0000) >> 16));
> + write_nic_byte(priv, 0x7d, ((0x000090a6 & 0x0000ff00) >> 8));
> + write_nic_byte(priv, 0x7c, ((0x000090a6 & 0x000000ff) >> 0));
> +
> + // Tx Antenna.
> + /* original code from realtek driver */
> + write_nic_byte(priv, ANTSEL, 0x03); // Config TX antenna.
>
> This is code example shows switching to one of two antennas. Other code you
> can see in attached patch.
>
> During handling every packet, the AGC value recording to an array. Every
> second driver calculate average value from AGC's array and makes decision
> have to do switch to other antenna or not.
>
> Compat drivers for chip on our board shows better results than
> rtl8187L_linux_26.1036.1105.2008 driver,
> compat driver ~ 800-1000Kbytes/sec
> rtl8187L_linux_26.1036.1105.2008 driver ~ 200Kbytes/sec
>
> Please correct me if I chose not correct parameter for evaluating signal
> quality information.
>
> However while testing, the switching between antennas doesn't give any
> download speed improvements.
> May be we missed some point.
>

My first comment is that the patch should be in-line rather than as an
attachment. Reviewing is easier.

You should be using the wireless-testing tree to derive your patches. Otherwise,
they cannot be applied. For example, the following hunk is already in w-t:

@@ -257,6 +261,7 @@

usb_fill_bulk_urb(urb, priv->udev, usb_sndbulkpipe(priv->udev, ep),
buf, skb->len, rtl8187_tx_cb, skb);
+ urb->transfer_flags |= URB_ZERO_PACKET;
rc = usb_submit_urb(urb, GFP_ATOMIC);
if (rc < 0) {
usb_free_urb(urb);


I don't like the idea of having the antenna diversity as a compile-time option.
That works OK for a single user, but it fails when distros are considered. They
would have to have it on for all cases even though very few users would need it.
Does the EEPROM for your device have special encoding that indicates the
presence of multiple antennas? You should be able to compile the antenna
diversity code unconditionally and use such a special EEPROM value.
Alternatively, the USB ID's (13d1:abe6) could be used to select its execution.

You should run the patch through the scripts/checkpatch.pl routine. Yours shows
55 errors and 34 warnings.

Please split out the part that adds the new USB_DEVICE and submit it now. That
will be needed no matter what happens to the antenna diversity code.

Larry

2009-04-22 23:29:01

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: rtl8187 diversity

Thanks a lot for working on the patch; I see Larry has already given
some feedback - more below.

On Wed, Apr 22, 2009 at 5:53 PM, Eugene Sobol <[email protected]=
> wrote:

> diff -urN compat-wireless-2.6-old.orig/drivers/net/wireless/rtl8187_d=
ev.c compat-wireless-2.6-old/drivers/net/wireless/rtl8187_dev.c
> --- compat-wireless-2.6-old.orig/drivers/net/wireless/rtl8187_dev.c =A0=
=A0 2008-09-15 19:57:24.000000000 +0300
> +++ compat-wireless-2.6-old/drivers/net/wireless/rtl8187_dev.c =A0200=
9-04-22 18:04:01.000000000 +0300
> @@ -45,6 +45,9 @@
> =A0 =A0 =A0 =A0{USB_DEVICE(0x03f0, 0xca02), .driver_info =3D DEVICE_R=
TL8187},
> =A0 =A0 =A0 =A0/* Sitecom */
> =A0 =A0 =A0 =A0{USB_DEVICE(0x0df6, 0x000d), .driver_info =3D DEVICE_R=
TL8187},
> +
> + =A0 =A0 =A0 {USB_DEVICE(0x13d1, 0xabe6), .driver_info =3D DEVICE_RT=
L8187},
> +
> =A0 =A0 =A0 =A0{}
> =A0};
>

Support for new hardware should/could be a separate patch; also some
notes about make/vendor would be nice, about the device.


> @@ -1123,6 +1229,7 @@
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0default:
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0chip_name =3D "RTL8187=
vB (default)";
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 priv->ant_diversity_ena=
bled =3D 1;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0} else {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/*

Switching on antenna diversity for all 8187L? I don't know what
side-effect there might be - (particularly for device which hasn't got
double-antenna). It is probably better to move this out of the loop
and conditioning on the vid/pid - i.e. specific to your device, for
the time being.

I am just a little concerned that the code should not have side effect
on devices not having double-antenna - for general testing/reviewing
is fine, but I think we need some way of auto-detection or runtime
switching (via iwpriv, etc), rather than compile-time change.

> diff -urN compat-wireless-2.6-old.orig/drivers/net/wireless/rtl8187.h=
compat-wireless-2.6-old/drivers/net/wireless/rtl8187.h
> --- compat-wireless-2.6-old.orig/drivers/net/wireless/rtl8187.h 2008-=
09-04 00:04:20.000000000 +0300
> +++ compat-wireless-2.6-old/drivers/net/wireless/rtl8187.h =A0 =A0 =A0=
2009-04-22 16:51:13.000000000 +0300
> @@ -82,11 +82,24 @@
> =A0 =A0 =A0 =A0DEVICE_RTL8187B
> =A0};
>
> +#define VAL_ARRAY_SIZE 10
> +#define ANTSEL 0xFF9F
> +
> +struct rtl8187_ant_diversity {
> +
> + =A0 =A0int count;
> + =A0 =A0 =A0 u8 agc_array[VAL_ARRAY_SIZE];
> +
> + =A0 =A0u8 switch_to;
> + =A0 =A0u8 curr_ant_index;
> +} __attribute__ ((packed));
> +
> =A0struct rtl8187_priv {
> =A0 =A0 =A0 =A0/* common between rtl818x drivers */
> =A0 =A0 =A0 =A0struct rtl818x_csr *map;
> =A0 =A0 =A0 =A0const struct rtl818x_rf_ops *rf;
> =A0 =A0 =A0 =A0struct ieee80211_vif *vif;
> + =A0 =A0 =A0 struct ieee80211_hw *hw;
> =A0 =A0 =A0 =A0int mode;
> =A0 =A0 =A0 =A0/* The mutex protects the TX loopback state.
> =A0 =A0 =A0 =A0 * Any attempt to set channels concurrently locks the =
device.

This looks wrong - there are existing means for accessing the
hardware, should not need to add another struct there.

Thanks for the work.

2009-04-08 12:57:56

by Eugene Sobol

[permalink] [raw]
Subject: Re: rtl8187 diversity

Hello!

We are using RTL8187L module (with 2 antenna) from Abocom on our Blackfin DSP
based board.
We have to enable antenna diversity for that module.

We have done some workaround realted with it:
I got compat-wireless archive from:
http://wireless.kernel.org/download/compat-wireless-2.6/compat-wireless-old.tar.bz2
I added switch-antenna related code from rtl8187L_linux_26.1036.1105.2008
driver.
I added simple logic how to evaluate quality of signal by AGC and switch
between antennas.

Following code shows how does swiching between antennas work (from
rtl8187L_linux_26.1036.1105.2008 ) :
+ write_nic_byte(priv, 0x7f, ((0x01009b90 & 0xff000000) >> 24));
+ write_nic_byte(priv, 0x7e, ((0x01009b90 & 0x00ff0000) >> 16));
+ write_nic_byte(priv, 0x7d, ((0x01009b90 & 0x0000ff00) >> 8));
+ write_nic_byte(priv, 0x7c, ((0x01009b90 & 0x000000ff) >> 0));
+
+ // Rx OFDM.
+ /* original code from realtek driver */
+ write_nic_byte(priv, 0x7f, ((0x000090a6 & 0xff000000) >> 24));
+ write_nic_byte(priv, 0x7e, ((0x000090a6 & 0x00ff0000) >> 16));
+ write_nic_byte(priv, 0x7d, ((0x000090a6 & 0x0000ff00) >> 8));
+ write_nic_byte(priv, 0x7c, ((0x000090a6 & 0x000000ff) >> 0));
+
+ // Tx Antenna.
+ /* original code from realtek driver */
+ write_nic_byte(priv, ANTSEL, 0x03); // Config TX antenna.

This is code example shows switching to one of two antennas. Other code you
can see in attached patch.

During handling every packet, the AGC value recording to an array. Every
second driver calculate average value from AGC's array and makes decision
have to do switch to other antenna or not.

Compat drivers for chip on our board shows better results than
rtl8187L_linux_26.1036.1105.2008 driver,
compat driver ~ 800-1000Kbytes/sec
rtl8187L_linux_26.1036.1105.2008 driver ~ 200Kbytes/sec

Please correct me if I chose not correct parameter for evaluating signal
quality information.

However while testing, the switching between antennas doesn't give any
download speed improvements.
May be we missed some point.

Thank you.

Best Regards,
Eugene Sobol


Attachments:
(No filename) (2.18 kB)
rtl8187_antenna_diversity.patch (11.20 kB)
Download all attachments

2009-04-08 21:07:25

by Larry Finger

[permalink] [raw]
Subject: Re: rtl8187 diversity

Ivan Kuten wrote:
>
> Hello Larry, thanks for comments! Surely Eugene will fix patch issues and
> resubmit patch inline. Actually in its current stage this patch just show
> an
> diversity idea and will be cleaned-up for submission with Signed-off-by.
> Currently the question was about correctness of calculation of signal
> quality
> and making decision when switching between antennas should be done. Anyway
> it will be easier to say about this when patch will be inline.
>
> Btw, USB_DEVICE is already added: http://lkml.org/lkml/2008/11/17/39

I think there is a logic error in the following:

+void sw_antenna_diversity_timer_callback(struct rtl8187_priv *priv)
+{
+ static int tick_without_switch = 0;
+ static int old_value = 0;
+ int average = 0;
+ int i;
+ for ( i = 0; i < VAL_ARRAY_SIZE; ++i)
+ {
+ average += priv->ant_diversity.agc_array[i];
+ }
+ average /= VAL_ARRAY_SIZE;
+ average /= 10; // cut 1 chipher
+ printk("%d->%d\n", old_value, average);
+
+ int sub = average - old_value;
+ if (sub < 0)
+ sub *= -1;
+ if (average > old_value)
+ {
+ priv->ant_diversity.switch_to ^= 1;
+ queue_delayed_work(priv->hw->workqueue, &priv->antenna_work,
+ msecs_to_jiffies(ANTENNA_SWITCH_TIMER_PERIOD));
+ }
+ old_value = average;

The AGC value is inversely proportional to the signal strength. I think you need
to select the antenna with the smaller, not the larger, value for the average.

Larry


2009-04-22 16:53:20

by Eugene Sobol

[permalink] [raw]
Subject: Re: rtl8187 diversity

Hello.

This is reworked patch for antenna diversity in rtl8187 compat wireless.
- removed proc file system functionality
- reworked calculation of agc
- fixed codestyle
Next step I will create for 2.6.30 RC2

--
BR, Eugene Sobol

diff -urN compat-wireless-2.6-old.orig/drivers/net/wireless/rtl8187_dev.c compat-wireless-2.6-old/drivers/net/wireless/rtl8187_dev.c
--- compat-wireless-2.6-old.orig/drivers/net/wireless/rtl8187_dev.c 2008-09-15 19:57:24.000000000 +0300
+++ compat-wireless-2.6-old/drivers/net/wireless/rtl8187_dev.c 2009-04-22 18:04:01.000000000 +0300
@@ -45,6 +45,9 @@
{USB_DEVICE(0x03f0, 0xca02), .driver_info = DEVICE_RTL8187},
/* Sitecom */
{USB_DEVICE(0x0df6, 0x000d), .driver_info = DEVICE_RTL8187},
+
+ {USB_DEVICE(0x13d1, 0xabe6), .driver_info = DEVICE_RTL8187},
+
{}
};

@@ -257,6 +260,7 @@

usb_fill_bulk_urb(urb, priv->udev, usb_sndbulkpipe(priv->udev, ep),
buf, skb->len, rtl8187_tx_cb, skb);
+ urb->transfer_flags |= URB_ZERO_PACKET;
rc = usb_submit_urb(urb, GFP_ATOMIC);
if (rc < 0) {
usb_free_urb(urb);
@@ -320,6 +324,9 @@
}
rx_status.signal = signal;
priv->signal = signal;
+
+ if (priv->ant_diversity_enabled)
+ add_to_average_array(priv, hdr->agc);
} else {
struct rtl8187b_rx_hdr *hdr =
(typeof(hdr))(skb_tail_pointer(skb) - sizeof(*hdr));
@@ -555,6 +562,13 @@
rtl818x_iowrite8(priv, (u8 *)0xFFFF, 0x60);
rtl818x_iowrite8(priv, &priv->map->PGSELECT, reg);

+ if (priv->ant_diversity_enabled) {
+ INIT_DELAYED_WORK(&priv->antenna_work,
+ sw_antenna_work_item_callback);
+ priv->ant_diversity.switch_to = 0;
+ switch_antenna(priv);
+ }
+
return 0;
}

@@ -724,6 +738,8 @@
u32 reg;
int ret;

+ priv->hw = dev;
+
ret = (!priv->is_rtl8187b) ? rtl8187_init_hw(dev) :
rtl8187b_init_hw(dev);
if (ret)
@@ -836,6 +852,9 @@
struct rtl8187_priv *priv = dev->priv;
int i;

+ if (priv->ant_diversity_enabled)
+ sw_antenna_diversity_timer_callback(priv);
+
if (priv->mode != NL80211_IFTYPE_MONITOR)
return -EOPNOTSUPP;

@@ -867,6 +886,12 @@
mutex_lock(&priv->conf_mutex);
priv->mode = NL80211_IFTYPE_MONITOR;
priv->vif = NULL;
+
+ if (priv->ant_diversity_enabled) {
+ del_timer_sync(&priv->sw_antenna_diversity_timer);
+ cancel_delayed_work(&priv->antenna_work);
+ }
+
mutex_unlock(&priv->conf_mutex);
}

@@ -970,6 +995,85 @@
rtl818x_iowrite32_async(priv, &priv->map->RX_CONF, priv->rx_conf);
}

+#define ANTENNA_DIVERSITY_TIMER_PERIOD 1000
+#define ANTENNA_SWITCH_TIMER_PERIOD 300
+#define ANTENNA_MAX_AGC_DIFF 15
+void sw_antenna_diversity_timer_callback(struct rtl8187_priv *priv)
+{
+ static int average_old;
+ int average_current = 0;
+ int i, sub;
+
+ if (!priv->ant_diversity_enabled)
+ return;
+ for (i = 0; i < VAL_ARRAY_SIZE; ++i)
+ average_current += priv->ant_diversity.agc_array[i];
+
+ average_current /= VAL_ARRAY_SIZE;
+ sub = average_current - average_old;
+ if (sub > ANTENNA_MAX_AGC_DIFF) {
+ priv->ant_diversity.switch_to ^= 1;
+ queue_delayed_work(priv->hw->workqueue, &priv->antenna_work,
+ msecs_to_jiffies(ANTENNA_SWITCH_TIMER_PERIOD));
+ }
+
+ average_old = average_current;
+ priv->sw_antenna_diversity_timer.expires = jiffies +
+ msecs_to_jiffies(ANTENNA_DIVERSITY_TIMER_PERIOD);
+ add_timer(&priv->sw_antenna_diversity_timer);
+}
+
+/*
+ * Change Antenna Switch.
+ */
+u8 switch_antenna(struct rtl8187_priv *priv)
+{
+ u8 b_antenna_switched = 0;
+ u8 u1b_antenna_index = priv->ant_diversity.switch_to;
+
+ switch (u1b_antenna_index) {
+ case 0:
+ /* main antenna */
+ /* Rx CCK. */
+ rtl8187_write_phy(priv->hw, 0x10, 0x0001009b);
+ /* Rx OFDM. */
+ rtl8187_write_phy(priv->hw, 0x26, 0x00000090);
+ /* Tx Antenna. */
+ rtl818x_iowrite16(priv, (__le16 *)ANTSEL, 0x0003);
+
+ b_antenna_switched = 1;
+ printk(KERN_INFO "switch to ANT1 antenna\n");
+ break;
+ case 1: /* switch(u1bAntennaIndex) */
+ /* Rx CCK. */
+ rtl8187_write_phy(priv->hw, 0x10, 0x000100db);
+ /* Rx OFDM. */
+ rtl8187_write_phy(priv->hw, 0x26, 0x00000010);
+ /* Tx Antenna. */
+ rtl818x_iowrite16(priv, (__le16 *)ANTSEL, 0x0000);
+
+ b_antenna_switched = 1;
+ printk(KERN_INFO "switch to AUX antenna\n");
+ break;
+ default:
+ printk(KERN_WARNING "[%s] unkown u_1b_antenna_index(%d)\n",
+ __func__, u1b_antenna_index);
+ break;
+ } /* switch(u1bAntennaIndex) */
+
+ if (b_antenna_switched)
+ priv->ant_diversity.curr_ant_index = u1b_antenna_index;
+
+ return b_antenna_switched;
+}
+
+void sw_antenna_work_item_callback(struct work_struct *work)
+{
+ struct rtl8187_priv *priv = container_of(work,
+ struct rtl8187_priv, antenna_work.work);
+ switch_antenna(priv);
+}
+
static const struct ieee80211_ops rtl8187_ops = {
.tx = rtl8187_tx,
.start = rtl8187_start,
@@ -1034,6 +1138,8 @@
priv = dev->priv;
priv->is_rtl8187b = (id->driver_info == DEVICE_RTL8187B);

+ priv->ant_diversity_enabled = 0;
+
SET_IEEE80211_DEV(dev, &intf->dev);
usb_set_intfdata(intf, dev);
priv->udev = udev;
@@ -1123,6 +1229,7 @@
break;
default:
chip_name = "RTL8187vB (default)";
+ priv->ant_diversity_enabled = 1;
}
} else {
/*
@@ -1210,6 +1317,15 @@
wiphy_name(dev->wiphy), print_mac(mac, dev->wiphy->perm_addr),
chip_name, priv->asic_rev, priv->rf->name);

+ if (priv->ant_diversity_enabled) {
+ init_timer(&priv->sw_antenna_diversity_timer);
+ memset(priv->ant_diversity.agc_array, 0,
+ sizeof priv->ant_diversity);
+ priv->sw_antenna_diversity_timer.data = (unsigned long)priv;
+ priv->sw_antenna_diversity_timer.function =
+ (void *)sw_antenna_diversity_timer_callback;
+ }
+
return 0;

err_free_dev:
diff -urN compat-wireless-2.6-old.orig/drivers/net/wireless/rtl8187.h compat-wireless-2.6-old/drivers/net/wireless/rtl8187.h
--- compat-wireless-2.6-old.orig/drivers/net/wireless/rtl8187.h 2008-09-04 00:04:20.000000000 +0300
+++ compat-wireless-2.6-old/drivers/net/wireless/rtl8187.h 2009-04-22 16:51:13.000000000 +0300
@@ -82,11 +82,24 @@
DEVICE_RTL8187B
};

+#define VAL_ARRAY_SIZE 10
+#define ANTSEL 0xFF9F
+
+struct rtl8187_ant_diversity {
+
+ int count;
+ u8 agc_array[VAL_ARRAY_SIZE];
+
+ u8 switch_to;
+ u8 curr_ant_index;
+} __attribute__ ((packed));
+
struct rtl8187_priv {
/* common between rtl818x drivers */
struct rtl818x_csr *map;
const struct rtl818x_rf_ops *rf;
struct ieee80211_vif *vif;
+ struct ieee80211_hw *hw;
int mode;
/* The mutex protects the TX loopback state.
* Any attempt to set channels concurrently locks the device.
@@ -112,8 +125,21 @@
u8 signal;
u8 quality;
u8 noise;
+
+ u8 ant_diversity_enabled;
+ struct timer_list sw_antenna_diversity_timer;
+ struct rtl8187_ant_diversity ant_diversity;
+ struct delayed_work antenna_work;
+
};

+static inline void add_to_average_array(struct rtl8187_priv *priv, u8 val)
+{
+ priv->ant_diversity.agc_array[priv->ant_diversity.count++] = val;
+ if (priv->ant_diversity.count >= VAL_ARRAY_SIZE)
+ priv->ant_diversity.count = 0;
+}
+
void rtl8187_write_phy(struct ieee80211_hw *dev, u8 addr, u32 data);

static inline u8 rtl818x_ioread8_idx(struct rtl8187_priv *priv,
@@ -218,4 +244,10 @@
rtl818x_iowrite32_idx(priv, addr, val, 0);
}

+void sw_antenna_diversity_timer_callback(struct rtl8187_priv *dev);
+
+u8 switch_antenna(struct rtl8187_priv *priv);
+
+void sw_antenna_work_item_callback(struct work_struct *work);
+
#endif /* RTL8187_H */


2009-03-31 22:26:24

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: rtl8187 diversity

On Tue, Mar 31, 2009 at 10:35 AM, Ivan Kuten <[email protected]> w=
rote:
> Hin-Tak Leung wrote:
>>
>> On Mon, Mar 30, 2009 at 1:38 PM, Ivan Kuten <[email protected]>
>> wrote:
>>>
>>> Hello!
>>>
>>> Anybody is working on adding antenna diversity feature to rtl8187 d=
river?
>>
>> Which of rtl8187 comes with multiple antenna, etc which would benefi=
t
>> from this feature?
>>
>
> We are using WMG2503 module (with 2 antenna) from Abocom on our Black=
fin DSP
> based board.
> We did some preliminary work based on rtl8187L_linux_26.1036.1105.200=
8,
> however there are too
> many unclear points.

If you have already thought about this, and how to go about it, care
to share? Some of us have spent some time
looking at rtl8187L_linux_26.1036.1105.2008 so may be able to chip in.

>
> Regards,
> Ivan
>
>>> Regards,
>>> Ivan
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-wir=
eless"
>>> in
>>> the body of a message to [email protected]
>>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.htm=
l
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wire=
less"
>> in
>> the body of a message to [email protected]
>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>
>

2009-04-08 19:44:05

by Ivan Kuten

[permalink] [raw]
Subject: Re: rtl8187 diversity

>
> My first comment is that the patch should be in-line rather than as an
> attachment. Reviewing is easier.
>
> You should be using the wireless-testing tree to derive your patches.
> Otherwise,
> they cannot be applied. For example, the following hunk is already in
w-t:
>
> @@ -257,6 +261,7 @@
>
> usb_fill_bulk_urb(urb, priv->udev, usb_sndbulkpipe(priv->udev,
> ep),
> buf, skb->len, rtl8187_tx_cb, skb);
> + urb->transfer_flags |= URB_ZERO_PACKET;
> rc = usb_submit_urb(urb, GFP_ATOMIC);
> if (rc < 0) {
> usb_free_urb(urb);
>
>
> I don't like the idea of having the antenna diversity as a compile-time
> option.
> That works OK for a single user, but it fails when distros are
considered.
> They
> would have to have it on for all cases even though very few users would
> need it.
> Does the EEPROM for your device have special encoding that indicates the
> presence of multiple antennas? You should be able to compile the antenna
> diversity code unconditionally and use such a special EEPROM value.
> Alternatively, the USB ID's (13d1:abe6) could be used to select its
> execution.
>
> You should run the patch through the scripts/checkpatch.pl routine. Yours
> shows
> 55 errors and 34 warnings.
>
> Please split out the part that adds the new USB_DEVICE and submit it now.
> That
> will be needed no matter what happens to the antenna diversity code.
>
> Larry
> --

Hello Larry, thanks for comments! Surely Eugene will fix patch issues and
resubmit patch inline. Actually in its current stage this patch just show
an
diversity idea and will be cleaned-up for submission with Signed-off-by.
Currently the question was about correctness of calculation of signal
quality
and making decision when switching between antennas should be done. Anyway
it will be easier to say about this when patch will be inline.

Btw, USB_DEVICE is already added: http://lkml.org/lkml/2008/11/17/39

Best regards,
Ivan