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 <eugene.sobol@xxxxxxxxxxx> wrote:
> 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},
> +
> {}
> };
>
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 @@
> break;
> default:
> chip_name = "RTL8187vB (default)";
> + priv->ant_diversity_enabled = 1;
> }
> } else {
> /*
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 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.
This looks wrong - there are existing means for accessing the
hardware, should not need to add another struct there.
Thanks for the work.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
