2010-10-11 15:44:42

by Shahar Levi

[permalink] [raw]
Subject: [PATCH v3 00/03] wl1271: 11n Support, add support to 80211n spec

Series that add 80211n spec support to wl1271: scan, connection via MCS rates
Wl12xx driver add HT support:
- Scan includes HT IE
- Connection to HT devices
- HT protection
- Traffic via MCS rates
- A-MSDU in RX path

Missing functionality is:
- BA session as Initiator and Receiver

Changes from v2:
- Add logic on wl1271_op_bss_info_changed()
- add CONFIG_WL1271_HT in tx & rx
- Rebase to latest wl12xx git
- Clean to Linux code style.

Shahar Levi (3):
wl1271: 11n Support, Add Definitions
wl1271: 11n Support, ACX Commands
wl1271: 11n Support, functionality and configuration ability

drivers/net/wireless/wl12xx/Kconfig | 10 +++
drivers/net/wireless/wl12xx/wl1271.h | 11 +++-
drivers/net/wireless/wl12xx/wl1271_acx.c | 85 ++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1271_acx.h | 88 +++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1271_main.c | 108 ++++++++++++++++++++++++-----
drivers/net/wireless/wl12xx/wl1271_rx.c | 6 ++
drivers/net/wireless/wl12xx/wl1271_tx.c | 11 +++
7 files changed, 301 insertions(+), 18 deletions(-)



2010-10-11 15:44:45

by Shahar Levi

[permalink] [raw]
Subject: [PATCH v3 01/03] wl1271: 11n Support, Add Definitions

Two acx commands: ht_capabilities & ht_information, 11n sta capabilities macro.

Signed-off-by: Shahar Levi <[email protected]>
---
drivers/net/wireless/wl12xx/wl1271.h | 11 ++++-
drivers/net/wireless/wl12xx/wl1271_acx.h | 83 +++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1271_main.c | 15 +++++
3 files changed, 108 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
index 8a4cd76..45a2583 100644
--- a/drivers/net/wireless/wl12xx/wl1271.h
+++ b/drivers/net/wireless/wl12xx/wl1271.h
@@ -432,7 +432,12 @@ struct wl1271 {
/* Our association ID */
u16 aid;

- /* currently configured rate set */
+ /*
+ * currently configured rate set:
+ * bits 0-15 - 802.11abg rates
+ * bits 16-23 - 802.11n MCS index mask
+ * support only 1 stream, thus only 8 bits for the MCS rates (0-7).
+ */
u32 sta_rate_set;
u32 basic_rate_set;
u32 basic_rate;
@@ -509,4 +514,8 @@ int wl1271_plt_stop(struct wl1271 *wl);
#define WL1271_PRE_POWER_ON_SLEEP 20 /* in miliseconds */
#define WL1271_POWER_ON_SLEEP 200 /* in miliseconds */

+/* Macros to handle wl1271.sta_rate_set */
+#define HW_BG_RATES_MASK 0xffff
+#define HW_HT_RATES_OFFSET 16
+
#endif
diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
index ebb341d..90082c5 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.h
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
@@ -964,6 +964,89 @@ struct wl1271_acx_rssi_snr_avg_weights {
u8 snr_data;
};

+/*
+ * ACX_PEER_HT_CAP
+ * Configure HT capabilities - declare the capabilities of the peer
+ * we are connected to.
+ */
+struct wl1271_acx_ht_capabilities {
+ struct acx_header header;
+
+ /*
+ * bit 0 - Allow HT Operation
+ * bit 1 - Allow Greenfield format in TX
+ * bit 2 - Allow Short GI in TX
+ * bit 3 - Allow L-SIG TXOP Protection in TX
+ * bit 4 - Allow HT Control fields in TX.
+ * Note, driver will still leave space for HT control in packets
+ * regardless of the value of this field. FW will be responsible
+ * to drop the HT field from any frame when this Bit set to 0.
+ * bit 5 - Allow RD initiation in TXOP. FW is allowed to initate RD.
+ * Exact policy setting for this feature is TBD.
+ * Note, this bit can only be set to 1 if bit 3 is set to 1.
+ */
+ __le32 ht_capabilites;
+
+ /*
+ * Indicates to which peer these capabilities are relevant.
+ * Note, currently this value will be set to FFFFFFFFFFFF to indicate it
+ * is relevant for all peers since we only support HT in infrastructure
+ * mode. Later on this field will be relevant to IBSS/DLS operation
+ */
+ u8 mac_address[ETH_ALEN];
+
+ /*
+ * This the maximum A-MPDU length supported by the AP. The FW may not
+ * exceed this length when sending A-MPDUs
+ */
+ u8 ampdu_max_length;
+
+ /*
+ * This is the minimal spacing required when sending A-MPDUs to the AP
+ */
+ u8 ampdu_min_spacing;
+} __packed;
+
+/* HT Capabilites Fw Bit Mask Mapping */
+#define WL1271_ACX_FW_CAP_BIT_MASK_HT_OPERATION BIT(0)
+#define WL1271_ACX_FW_CAP_BIT_MASK_GREENFIELD_FRAME_FORMAT BIT(1)
+#define WL1271_ACX_FW_CAP_BIT_MASK_SHORT_GI_FOR_20MHZ_PACKETS BIT(2)
+#define WL1271_ACX_FW_CAP_BIT_MASK_LSIG_TXOP_PROTECTION BIT(3)
+#define WL1271_ACX_FW_CAP_BIT_MASK_HT_CONTROL_FIELDS BIT(4)
+#define WL1271_ACX_FW_CAP_BIT_MASK_RD_INITIATION BIT(5)
+
+
+/*
+ * ACX_HT_BSS_OPERATION
+ * Configure HT capabilities - AP rules for behavior in the BSS.
+ */
+struct wl1271_acx_ht_information {
+ struct acx_header header;
+
+ /* Values: 0 - RIFS not allowed, 1 - RIFS allowed */
+ u8 rifs_mode;
+
+ /* Values: 0 - 3 like in spec */
+ u8 ht_protection;
+
+ /* Values: 0 - GF protection not required, 1 - GF protection required */
+ u8 gf_protection;
+
+ /*Values: 0 - TX Burst limit not required, 1 - TX Burst Limit required*/
+ u8 ht_tx_burst_limit;
+
+ /*
+ * Values: 0 - Dual CTS protection not required,
+ * 1 Dual CTS Protection required
+ * Note: When this value is set to 1 FW will protect all TXOP with RTS
+ * frame and will not use CTS-to-self regardless of the value of the
+ * ACX_CTS_PROTECTION information element
+ */
+ u8 dual_cts_protection;
+
+ u8 padding[3];
+} __packed;
+
struct wl1271_acx_fw_tsf_information {
struct acx_header header;

diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index 48a4b99..1b4c970 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -2134,6 +2134,21 @@ static const u8 wl1271_rate_to_idx_2ghz[] = {
0 /* CONF_HW_RXTX_RATE_1 */
};

+/* 11n STA capabilities */
+#define HW_RX_HIGHEST_RATE 65
+
+#define WL1271_HT_CAP { \
+ .cap = IEEE80211_HT_CAP_GRN_FLD | IEEE80211_HT_CAP_SGI_20, \
+ .ht_supported = true, \
+ .ampdu_factor = IEEE80211_HT_MAX_AMPDU_8K, \
+ .ampdu_density = IEEE80211_HT_MPDU_DENSITY_8, \
+ .mcs = { \
+ .rx_mask = { 0xff, 0, 0, 0, 0, 0, 0, 0, 0, 0, }, \
+ .rx_highest = cpu_to_le16(HW_RX_HIGHEST_RATE), \
+ .tx_params = IEEE80211_HT_MCS_TX_DEFINED, \
+ }, \
+}
+
/* can't be const, mac80211 writes to this */
static struct ieee80211_supported_band wl1271_band_2ghz = {
.channels = wl1271_channels,
--
1.6.0.4


2010-10-13 06:17:33

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v3 03/03] wl1271: 11n Support, functionality and configuration ability

Hi Shahar,

Shahar Levi <[email protected]> writes:

> Thanks for the review. All below will be fix in v4.

Please don't top post and also trim your quotes. That makes a lot
easier to read your emails. More info here:

http://idallen.com/topposting.html

People here deal with hundreds of emails per day from different
mailing lists and it really matter how the messages are formatted. So
please be kind to us and try to follow the etiquette. Thanks.

--
Kalle Valo

2010-10-12 16:00:50

by Shahar Levi

[permalink] [raw]
Subject: Re: [PATCH v3 03/03] wl1271: 11n Support, functionality and configuration ability

Thanks for the review. All below will be fix in v4.
Cheers,
Shahar

Luciano Coelho wrote:
> On Mon, 2010-10-11 at 17:47 +0200, ext Shahar Levi wrote:
>> Add 11n ability in scan, connection and using MCS rates.
>> The configuration is temporary due to the code incomplete and
>> still in testing process. That plans to be remove in the future.
>>
>> Signed-off-by: Shahar Levi <[email protected]>
>> ---
>
> ...
>
>> @@ -1709,6 +1721,7 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
>> {
>> enum wl1271_cmd_ps_mode mode;
>> struct wl1271 *wl = hw->priv;
>> + struct ieee80211_sta *sta = ieee80211_find_sta(vif, bss_conf->bssid);
>> bool do_join = false;
>> bool set_assoc = false;
>> int ret;
>> @@ -1927,6 +1940,45 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
>> }
>> }
>>
>> + if (sta) {
>> + /*
>> + * Takes care of: New association with HT enable,
>> + * HT information change in beacon.
>> + */
>> + if ((changed & BSS_CHANGED_HT) &&
>> + (bss_conf->channel_type != NL80211_CHAN_NO_HT)) {
>
> You could move the if (sta) into the same if, as I proposed before:
>
> if (sta && changed & BSS_CHANGED_HT) &&
> (bss_conf->channel_type != NL80211_CHAN_NO_HT)) {
>
>
>
>> + ret = wl1271_acx_set_ht_capabilities(wl,
>> + &sta->ht_cap,
>> + true);
>
> ...then you don't need to break this line so much...
>
>
>> + if (ret < 0) {
>> + wl1271_warning("Set ht cap true failed %d",
>> + ret);
>
> ...nor this...
>
>> + goto out_sleep;
>> + }
>> +
>> + ret = wl1271_acx_set_ht_information(wl,
>> + bss_conf->ht_operation_mode);
>> + if (ret < 0) {
>> + wl1271_warning("Set ht information failed %d",
>> + ret);
>
> ...nor this...
>
>
>> + /*
>> + * Takes care of: New association without HT,
>> + * Disassociate.
>> + */
>
> Disassociation.
>
>
>> + else if (changed & BSS_CHANGED_ASSOC) {
>
> Same in this block about the if (sta) thingy:
>
> if (sta && changed & BSS_CHANGED_ASSOC) {
>
>
>> @@ -2107,14 +2158,14 @@ static struct ieee80211_channel wl1271_channels[] = {
>> /* mapping to indexes for wl1271_rates */
>> static const u8 wl1271_rate_to_idx_2ghz[] = {
>> /* MCS rates are used only with 11n */
>> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS7 */
>> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS6 */
>> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS5 */
>> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS4 */
>> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS3 */
>> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS2 */
>> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS1 */
>> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS0 */
>> + 7, /* CONF_HW_RXTX_RATE_MCS7 */
>> + 6, /* CONF_HW_RXTX_RATE_MCS6 */
>> + 5, /* CONF_HW_RXTX_RATE_MCS5 */
>> + 4, /* CONF_HW_RXTX_RATE_MCS4 */
>> + 3, /* CONF_HW_RXTX_RATE_MCS3 */
>> + 2, /* CONF_HW_RXTX_RATE_MCS2 */
>> + 1, /* CONF_HW_RXTX_RATE_MCS1 */
>> + 0, /* CONF_HW_RXTX_RATE_MCS0 */
>>
>> 11, /* CONF_HW_RXTX_RATE_54 */
>> 10, /* CONF_HW_RXTX_RATE_48 */
>> @@ -2137,6 +2188,7 @@ static const u8 wl1271_rate_to_idx_2ghz[] = {
>> /* 11n STA capabilities */
>> #define HW_RX_HIGHEST_RATE 65
>>
>> +#ifdef CONFIG_WL1271_HT
>> #define WL1271_HT_CAP { \
>> .cap = IEEE80211_HT_CAP_GRN_FLD | IEEE80211_HT_CAP_SGI_20, \
>> .ht_supported = true, \
>> @@ -2148,6 +2200,11 @@ static const u8 wl1271_rate_to_idx_2ghz[] = {
>> .tx_params = IEEE80211_HT_MCS_TX_DEFINED, \
>> }, \
>> }
>> +#else
>> +#define WL1271_HT_CAP { \
>> + .ht_supported = false, \
>> +}
>> +#endif
>
> Would be nicer to have the ifdefs already when you create this macro
> (ie. patch 01/03), but you don't have the CONFIG flag at that point
> yet... but well, never mind. No need to change it.
>
>


2010-10-11 15:44:46

by Shahar Levi

[permalink] [raw]
Subject: [PATCH v3 02/03] wl1271: 11n Support, ACX Commands

Added ACX command to the FW for 11n support.

Signed-off-by: Shahar Levi <[email protected]>
---
drivers/net/wireless/wl12xx/wl1271_acx.c | 85 ++++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1271_acx.h | 5 ++
2 files changed, 90 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.c b/drivers/net/wireless/wl12xx/wl1271_acx.c
index 6189934..cd89482 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.c
@@ -1226,6 +1226,91 @@ out:
return ret;
}

+int wl1271_acx_set_ht_capabilities(struct wl1271 *wl,
+ struct ieee80211_sta_ht_cap *ht_cap,
+ bool allow_ht_operation)
+{
+ struct wl1271_acx_ht_capabilities *acx;
+ u8 mac_address[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+ int ret = 0;
+
+ wl1271_debug(DEBUG_ACX, "acx ht capabilities setting");
+
+ acx = kzalloc(sizeof(*acx), GFP_KERNEL);
+ if (!acx) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ /* Allow HT Operation ? */
+ if (allow_ht_operation) {
+ acx->ht_capabilites =
+ WL1271_ACX_FW_CAP_BIT_MASK_HT_OPERATION;
+ acx->ht_capabilites |=
+ ((ht_cap->cap & IEEE80211_HT_CAP_GRN_FLD) ?
+ WL1271_ACX_FW_CAP_BIT_MASK_GREENFIELD_FRAME_FORMAT : 0);
+ acx->ht_capabilites |=
+ ((ht_cap->cap & IEEE80211_HT_CAP_SGI_20) ?
+ WL1271_ACX_FW_CAP_BIT_MASK_SHORT_GI_FOR_20MHZ_PACKETS : 0);
+ acx->ht_capabilites |=
+ ((ht_cap->cap & IEEE80211_HT_CAP_LSIG_TXOP_PROT) ?
+ WL1271_ACX_FW_CAP_BIT_MASK_LSIG_TXOP_PROTECTION : 0);
+
+ /* get data from A-MPDU parameters field */
+ acx->ampdu_max_length = ht_cap->ampdu_factor;
+ acx->ampdu_min_spacing = ht_cap->ampdu_density;
+
+ memcpy(acx->mac_address, mac_address, ETH_ALEN);
+ }
+ /* HT operations are not allowed */
+ else {
+ acx->ht_capabilites = 0;
+ }
+
+ ret = wl1271_cmd_configure(wl, ACX_PEER_HT_CAP, acx, sizeof(*acx));
+ if (ret < 0) {
+ wl1271_warning("acx ht capabilities setting failed: %d", ret);
+ goto out;
+ }
+
+out:
+ kfree(acx);
+ return ret;
+}
+
+int wl1271_acx_set_ht_information(struct wl1271 *wl,
+ u16 ht_operation_mode)
+{
+ struct wl1271_acx_ht_information *acx;
+ int ret = 0;
+
+ wl1271_debug(DEBUG_ACX, "acx ht information setting");
+
+ acx = kzalloc(sizeof(*acx), GFP_KERNEL);
+ if (!acx) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ acx->ht_protection =
+ (u8)(ht_operation_mode & IEEE80211_HT_OP_MODE_PROTECTION);
+ acx->rifs_mode = 0;
+ acx->gf_protection = 0;
+ acx->ht_tx_burst_limit = 0;
+ acx->dual_cts_protection = 0;
+
+ ret = wl1271_cmd_configure(wl, ACX_HT_BSS_OPERATION, acx, sizeof(*acx));
+
+ if (ret < 0) {
+ wl1271_warning("acx ht information setting failed: %d", ret);
+ goto out;
+ }
+
+out:
+ kfree(acx);
+ return ret;
+}
+
int wl1271_acx_tsf_info(struct wl1271 *wl, u64 *mactime)
{
struct wl1271_acx_fw_tsf_information *tsf_info;
diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
index 90082c5..57d503b 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.h
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
@@ -1176,6 +1176,11 @@ int wl1271_acx_keep_alive_config(struct wl1271 *wl, u8 index, u8 tpl_valid);
int wl1271_acx_rssi_snr_trigger(struct wl1271 *wl, bool enable,
s16 thold, u8 hyst);
int wl1271_acx_rssi_snr_avg_weights(struct wl1271 *wl);
+int wl1271_acx_set_ht_capabilities(struct wl1271 *wl,
+ struct ieee80211_sta_ht_cap *ht_cap,
+ bool allow_ht_operation);
+int wl1271_acx_set_ht_information(struct wl1271 *wl,
+ u16 ht_operation_mode);
int wl1271_acx_tsf_info(struct wl1271 *wl, u64 *mactime);

#endif /* __WL1271_ACX_H__ */
--
1.6.0.4


2010-10-11 15:44:46

by Shahar Levi

[permalink] [raw]
Subject: [PATCH v3 03/03] wl1271: 11n Support, functionality and configuration ability

Add 11n ability in scan, connection and using MCS rates.
The configuration is temporary due to the code incomplete and
still in testing process. That plans to be remove in the future.

Signed-off-by: Shahar Levi <[email protected]>
---
drivers/net/wireless/wl12xx/Kconfig | 10 +++
drivers/net/wireless/wl12xx/wl1271_main.c | 95 +++++++++++++++++++++++-----
drivers/net/wireless/wl12xx/wl1271_rx.c | 6 ++
drivers/net/wireless/wl12xx/wl1271_tx.c | 11 ++++
4 files changed, 103 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/Kconfig b/drivers/net/wireless/wl12xx/Kconfig
index 4a8bb25..36deb6e 100644
--- a/drivers/net/wireless/wl12xx/Kconfig
+++ b/drivers/net/wireless/wl12xx/Kconfig
@@ -52,6 +52,16 @@ config WL1271
If you choose to build a module, it'll be called wl1271. Say N if
unsure.

+config WL1271_HT
+ bool "TI wl1271 802.11 HT support (EXPERIMENTAL)"
+ depends on WL1271 && EXPERIMENTAL
+ default n
+ ---help---
+ This will enable 802.11 HT support for TI wl1271 chipset.
+
+ That configuration is temporary due to the code incomplete and
+ still in testing process.
+
config WL1271_SPI
tristate "TI wl1271 SPI support"
depends on WL1271 && SPI_MASTER
diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index 1b4c970..88a1113 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -853,10 +853,23 @@ static int wl1271_op_tx(struct ieee80211_hw *hw, struct sk_buff *skb)

/* peek into the rates configured in the STA entry */
spin_lock_irqsave(&wl->wl_lock, flags);
- if (sta && sta->supp_rates[conf->channel->band] != wl->sta_rate_set) {
+ if (sta &&
+ (sta->supp_rates[conf->channel->band] !=
+ (wl->sta_rate_set & HW_BG_RATES_MASK))) {
wl->sta_rate_set = sta->supp_rates[conf->channel->band];
set_bit(WL1271_FLAG_STA_RATES_CHANGED, &wl->flags);
}
+
+ if (sta &&
+ sta->ht_cap.ht_supported &&
+ ((wl->sta_rate_set >> HW_HT_RATES_OFFSET) !=
+ sta->ht_cap.mcs.rx_mask[0])) {
+ /* Clean MCS bits before setting them */
+ wl->sta_rate_set &= HW_BG_RATES_MASK;
+ wl->sta_rate_set |=
+ (sta->ht_cap.mcs.rx_mask[0] << HW_HT_RATES_OFFSET);
+ set_bit(WL1271_FLAG_STA_RATES_CHANGED, &wl->flags);
+ }
spin_unlock_irqrestore(&wl->wl_lock, flags);

/* queue the packet */
@@ -1709,6 +1721,7 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
{
enum wl1271_cmd_ps_mode mode;
struct wl1271 *wl = hw->priv;
+ struct ieee80211_sta *sta = ieee80211_find_sta(vif, bss_conf->bssid);
bool do_join = false;
bool set_assoc = false;
int ret;
@@ -1927,6 +1940,45 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
}
}

+ if (sta) {
+ /*
+ * Takes care of: New association with HT enable,
+ * HT information change in beacon.
+ */
+ if ((changed & BSS_CHANGED_HT) &&
+ (bss_conf->channel_type != NL80211_CHAN_NO_HT)) {
+ ret = wl1271_acx_set_ht_capabilities(wl,
+ &sta->ht_cap,
+ true);
+ if (ret < 0) {
+ wl1271_warning("Set ht cap true failed %d",
+ ret);
+ goto out_sleep;
+ }
+
+ ret = wl1271_acx_set_ht_information(wl,
+ bss_conf->ht_operation_mode);
+ if (ret < 0) {
+ wl1271_warning("Set ht information failed %d",
+ ret);
+ goto out_sleep;
+ }
+ }
+ /*
+ * Takes care of: New association without HT,
+ * Disassociate.
+ */
+ else if (changed & BSS_CHANGED_ASSOC) {
+ ret = wl1271_acx_set_ht_capabilities(wl,
+ &sta->ht_cap,
+ false);
+ if (ret < 0) {
+ wl1271_warning("Set ht cap false failed %d",
+ ret);
+ goto out_sleep;
+ }
+ }
+ }
+
if (changed & BSS_CHANGED_ARP_FILTER) {
__be32 addr = bss_conf->arp_addr_list[0];
WARN_ON(wl->bss_type != BSS_TYPE_STA_BSS);
@@ -2107,14 +2158,14 @@ static struct ieee80211_channel wl1271_channels[] = {
/* mapping to indexes for wl1271_rates */
static const u8 wl1271_rate_to_idx_2ghz[] = {
/* MCS rates are used only with 11n */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS7 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS6 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS5 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS4 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS3 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS2 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS1 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS0 */
+ 7, /* CONF_HW_RXTX_RATE_MCS7 */
+ 6, /* CONF_HW_RXTX_RATE_MCS6 */
+ 5, /* CONF_HW_RXTX_RATE_MCS5 */
+ 4, /* CONF_HW_RXTX_RATE_MCS4 */
+ 3, /* CONF_HW_RXTX_RATE_MCS3 */
+ 2, /* CONF_HW_RXTX_RATE_MCS2 */
+ 1, /* CONF_HW_RXTX_RATE_MCS1 */
+ 0, /* CONF_HW_RXTX_RATE_MCS0 */

11, /* CONF_HW_RXTX_RATE_54 */
10, /* CONF_HW_RXTX_RATE_48 */
@@ -2137,6 +2188,7 @@ static const u8 wl1271_rate_to_idx_2ghz[] = {
/* 11n STA capabilities */
#define HW_RX_HIGHEST_RATE 65

+#ifdef CONFIG_WL1271_HT
#define WL1271_HT_CAP { \
.cap = IEEE80211_HT_CAP_GRN_FLD | IEEE80211_HT_CAP_SGI_20, \
.ht_supported = true, \
@@ -2148,6 +2200,11 @@ static const u8 wl1271_rate_to_idx_2ghz[] = {
.tx_params = IEEE80211_HT_MCS_TX_DEFINED, \
}, \
}
+#else
+#define WL1271_HT_CAP { \
+ .ht_supported = false, \
+}
+#endif

/* can't be const, mac80211 writes to this */
static struct ieee80211_supported_band wl1271_band_2ghz = {
@@ -2155,6 +2212,7 @@ static struct ieee80211_supported_band wl1271_band_2ghz = {
.n_channels = ARRAY_SIZE(wl1271_channels),
.bitrates = wl1271_rates,
.n_bitrates = ARRAY_SIZE(wl1271_rates),
+ .ht_cap = WL1271_HT_CAP,
};

/* 5 GHz data rates for WL1273 */
@@ -2237,14 +2295,14 @@ static struct ieee80211_channel wl1271_channels_5ghz[] = {
/* mapping to indexes for wl1271_rates_5ghz */
static const u8 wl1271_rate_to_idx_5ghz[] = {
/* MCS rates are used only with 11n */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS7 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS6 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS5 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS4 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS3 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS2 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS1 */
- CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS0 */
+ 7, /* CONF_HW_RXTX_RATE_MCS7 */
+ 6, /* CONF_HW_RXTX_RATE_MCS6 */
+ 5, /* CONF_HW_RXTX_RATE_MCS5 */
+ 4, /* CONF_HW_RXTX_RATE_MCS4 */
+ 3, /* CONF_HW_RXTX_RATE_MCS3 */
+ 2, /* CONF_HW_RXTX_RATE_MCS2 */
+ 1, /* CONF_HW_RXTX_RATE_MCS1 */
+ 0, /* CONF_HW_RXTX_RATE_MCS0 */

7, /* CONF_HW_RXTX_RATE_54 */
6, /* CONF_HW_RXTX_RATE_48 */
@@ -2269,6 +2327,7 @@ static struct ieee80211_supported_band wl1271_band_5ghz = {
.n_channels = ARRAY_SIZE(wl1271_channels_5ghz),
.bitrates = wl1271_rates_5ghz,
.n_bitrates = ARRAY_SIZE(wl1271_rates_5ghz),
+ .ht_cap = WL1271_HT_CAP,
};

static const u8 *wl1271_band_rate_to_idx[] = {
diff --git a/drivers/net/wireless/wl12xx/wl1271_rx.c b/drivers/net/wireless/wl12xx/wl1271_rx.c
index bea133b..ac13f7d 100644
--- a/drivers/net/wireless/wl12xx/wl1271_rx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_rx.c
@@ -53,6 +53,12 @@ static void wl1271_rx_status(struct wl1271 *wl,
status->band = wl->band;
status->rate_idx = wl1271_rate_to_idx(wl, desc->rate);

+#ifdef CONFIG_WL1271_HT
+ /* 11n support */
+ if (desc->rate <= CONF_HW_RXTX_RATE_MCS0)
+ status->flag |= RX_FLAG_HT;
+#endif
+
status->signal = desc->rssi;

/*
diff --git a/drivers/net/wireless/wl12xx/wl1271_tx.c b/drivers/net/wireless/wl12xx/wl1271_tx.c
index e3dc13c..6a87633 100644
--- a/drivers/net/wireless/wl12xx/wl1271_tx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_tx.c
@@ -201,6 +201,17 @@ u32 wl1271_tx_enabled_rates_get(struct wl1271 *wl, u32 rate_set)
rate_set >>= 1;
}

+#ifdef CONFIG_WL1271_HT
+ /* MCS rates indication are on bits 16 - 23 */
+ rate_set >>= HW_HT_RATES_OFFSET - band->n_bitrates;
+
+ for (bit = 0; bit < 8; bit++) {
+ if (rate_set & 0x1)
+ enabled_rates |= (CONF_HW_BIT_RATE_MCS_0 << bit);
+ rate_set >>= 1;
+ }
+#endif
+
return enabled_rates;
}

--
1.6.0.4


2010-10-12 13:46:14

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v3 02/03] wl1271: 11n Support, ACX Commands

On Mon, 2010-10-11 at 17:47 +0200, ext Shahar Levi wrote:
> Added ACX command to the FW for 11n support.
>
> Signed-off-by: Shahar Levi <[email protected]>
> ---

More minors.


> diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.c b/drivers/net/wireless/wl12xx/wl1271_acx.c
> index 6189934..cd89482 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_acx.c
> +++ b/drivers/net/wireless/wl12xx/wl1271_acx.c
> @@ -1226,6 +1226,91 @@ out:
> return ret;
> }
>
> +int wl1271_acx_set_ht_capabilities(struct wl1271 *wl,
> + struct ieee80211_sta_ht_cap *ht_cap,
> + bool allow_ht_operation)
> +{
> + struct wl1271_acx_ht_capabilities *acx;
> + u8 mac_address[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};

As I said in my comment to 01/03, it's better to have the comment about
this here and not in the header.


> + acx = kzalloc(sizeof(*acx), GFP_KERNEL);
> + if (!acx) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + /* Allow HT Operation ? */
> + if (allow_ht_operation) {
> + acx->ht_capabilites =
> + WL1271_ACX_FW_CAP_BIT_MASK_HT_OPERATION;
> + acx->ht_capabilites |=
> + ((ht_cap->cap & IEEE80211_HT_CAP_GRN_FLD) ?
> + WL1271_ACX_FW_CAP_BIT_MASK_GREENFIELD_FRAME_FORMAT : 0);
> + acx->ht_capabilites |=
> + ((ht_cap->cap & IEEE80211_HT_CAP_SGI_20) ?
> + WL1271_ACX_FW_CAP_BIT_MASK_SHORT_GI_FOR_20MHZ_PACKETS : 0);
> + acx->ht_capabilites |=
> + ((ht_cap->cap & IEEE80211_HT_CAP_LSIG_TXOP_PROT) ?
> + WL1271_ACX_FW_CAP_BIT_MASK_LSIG_TXOP_PROTECTION : 0);

We use one full TAB for indentation on the next line after an = sign.
Some of this won't fit in 80 chars width, but that's because they're too
long anyway.

So two things to change: first, you can remove the _BIT_MASK part of all
this macros, so they're shorter; second, add a full TAB in the
indentation, like this:

acx->ht_capabilites =
WL1271_ACX_FW_CAP_HT_OPERATION;

> +
> + /* get data from A-MPDU parameters field */
> + acx->ampdu_max_length = ht_cap->ampdu_factor;
> + acx->ampdu_min_spacing = ht_cap->ampdu_density;
> +
> + memcpy(acx->mac_address, mac_address, ETH_ALEN);
> + }
> + /* HT operations are not allowed */
> + else {
> + acx->ht_capabilites = 0;
> + }

} else {
acx->ht_capabilities = 0;
}

If you still want to add the "HT operations are not allowed" comment,
add it in the same line where "} else {" is.


--
Cheers,
Luca.


2010-10-12 14:18:21

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v3 03/03] wl1271: 11n Support, functionality and configuration ability

On Mon, 2010-10-11 at 17:47 +0200, ext Shahar Levi wrote:
> Add 11n ability in scan, connection and using MCS rates.
> The configuration is temporary due to the code incomplete and
> still in testing process. That plans to be remove in the future.
>
> Signed-off-by: Shahar Levi <[email protected]>
> ---

...

> @@ -1709,6 +1721,7 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
> {
> enum wl1271_cmd_ps_mode mode;
> struct wl1271 *wl = hw->priv;
> + struct ieee80211_sta *sta = ieee80211_find_sta(vif, bss_conf->bssid);
> bool do_join = false;
> bool set_assoc = false;
> int ret;
> @@ -1927,6 +1940,45 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
> }
> }
>
> + if (sta) {
> + /*
> + * Takes care of: New association with HT enable,
> + * HT information change in beacon.
> + */
> + if ((changed & BSS_CHANGED_HT) &&
> + (bss_conf->channel_type != NL80211_CHAN_NO_HT)) {

You could move the if (sta) into the same if, as I proposed before:

if (sta && changed & BSS_CHANGED_HT) &&
(bss_conf->channel_type != NL80211_CHAN_NO_HT)) {



> + ret = wl1271_acx_set_ht_capabilities(wl,
> + &sta->ht_cap,
> + true);

...then you don't need to break this line so much...


> + if (ret < 0) {
> + wl1271_warning("Set ht cap true failed %d",
> + ret);

...nor this...

> + goto out_sleep;
> + }
> +
> + ret = wl1271_acx_set_ht_information(wl,
> + bss_conf->ht_operation_mode);
> + if (ret < 0) {
> + wl1271_warning("Set ht information failed %d",
> + ret);

...nor this...


> + /*
> + * Takes care of: New association without HT,
> + * Disassociate.
> + */

Disassociation.


> + else if (changed & BSS_CHANGED_ASSOC) {

Same in this block about the if (sta) thingy:

if (sta && changed & BSS_CHANGED_ASSOC) {


> @@ -2107,14 +2158,14 @@ static struct ieee80211_channel wl1271_channels[] = {
> /* mapping to indexes for wl1271_rates */
> static const u8 wl1271_rate_to_idx_2ghz[] = {
> /* MCS rates are used only with 11n */
> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS7 */
> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS6 */
> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS5 */
> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS4 */
> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS3 */
> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS2 */
> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS1 */
> - CONF_HW_RXTX_RATE_UNSUPPORTED, /* CONF_HW_RXTX_RATE_MCS0 */
> + 7, /* CONF_HW_RXTX_RATE_MCS7 */
> + 6, /* CONF_HW_RXTX_RATE_MCS6 */
> + 5, /* CONF_HW_RXTX_RATE_MCS5 */
> + 4, /* CONF_HW_RXTX_RATE_MCS4 */
> + 3, /* CONF_HW_RXTX_RATE_MCS3 */
> + 2, /* CONF_HW_RXTX_RATE_MCS2 */
> + 1, /* CONF_HW_RXTX_RATE_MCS1 */
> + 0, /* CONF_HW_RXTX_RATE_MCS0 */
>
> 11, /* CONF_HW_RXTX_RATE_54 */
> 10, /* CONF_HW_RXTX_RATE_48 */
> @@ -2137,6 +2188,7 @@ static const u8 wl1271_rate_to_idx_2ghz[] = {
> /* 11n STA capabilities */
> #define HW_RX_HIGHEST_RATE 65
>
> +#ifdef CONFIG_WL1271_HT
> #define WL1271_HT_CAP { \
> .cap = IEEE80211_HT_CAP_GRN_FLD | IEEE80211_HT_CAP_SGI_20, \
> .ht_supported = true, \
> @@ -2148,6 +2200,11 @@ static const u8 wl1271_rate_to_idx_2ghz[] = {
> .tx_params = IEEE80211_HT_MCS_TX_DEFINED, \
> }, \
> }
> +#else
> +#define WL1271_HT_CAP { \
> + .ht_supported = false, \
> +}
> +#endif

Would be nicer to have the ifdefs already when you create this macro
(ie. patch 01/03), but you don't have the CONFIG flag at that point
yet... but well, never mind. No need to change it.


--
Cheers,
Luca.


2010-10-12 13:34:14

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v3 01/03] wl1271: 11n Support, Add Definitions

On Mon, 2010-10-11 at 17:47 +0200, ext Shahar Levi wrote:
> Two acx commands: ht_capabilities & ht_information, 11n sta capabilities macro.
>
> Signed-off-by: Shahar Levi <[email protected]>
> ---

A few more minor comments.


> diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
> index ebb341d..90082c5 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_acx.h
> +++ b/drivers/net/wireless/wl12xx/wl1271_acx.h

[...]

> + /*
> + * Indicates to which peer these capabilities are relevant.
> + * Note, currently this value will be set to FFFFFFFFFFFF to indicate it
> + * is relevant for all peers since we only support HT in infrastructure
> + * mode. Later on this field will be relevant to IBSS/DLS operation
> + */
> + u8 mac_address[ETH_ALEN];

Okay, I was not very clear, but in my comment to patch 02/03, I said:
"[...]you could actually remove the Note from the wl1271_acx.h file and
leave it here[...]". The reasoning to leave it in the C file is that it
is there that you actually set the mac_address to FFFFFFFFFFFF. So,
remove from the wl1271_acx.h file and put it back on the wl1271_acx.c
file.


> +
> + /*
> + * This the maximum A-MPDU length supported by the AP. The FW may not
> + * exceed this length when sending A-MPDUs
> + */
> + u8 ampdu_max_length;
> +
> + /*
> + * This is the minimal spacing required when sending A-MPDUs to the AP
> + */

No need to have this comment in multiple lines. You could write just
"Minimal spacing..." and then it will fit nicely in one line ;)


> +/* HT Capabilites Fw Bit Mask Mapping */
> +#define WL1271_ACX_FW_CAP_BIT_MASK_HT_OPERATION BIT(0)
> +#define WL1271_ACX_FW_CAP_BIT_MASK_GREENFIELD_FRAME_FORMAT BIT(1)
> +#define WL1271_ACX_FW_CAP_BIT_MASK_SHORT_GI_FOR_20MHZ_PACKETS BIT(2)
> +#define WL1271_ACX_FW_CAP_BIT_MASK_LSIG_TXOP_PROTECTION BIT(3)
> +#define WL1271_ACX_FW_CAP_BIT_MASK_HT_CONTROL_FIELDS BIT(4)
> +#define WL1271_ACX_FW_CAP_BIT_MASK_RD_INITIATION BIT(5)

Repeating: No need for the TABs after the #defines.


> +/*
> + * ACX_HT_BSS_OPERATION
> + * Configure HT capabilities - AP rules for behavior in the BSS.
> + */
> +struct wl1271_acx_ht_information {
> + struct acx_header header;
> +
> + /* Values: 0 - RIFS not allowed, 1 - RIFS allowed */
> + u8 rifs_mode;
> +
> + /* Values: 0 - 3 like in spec */
> + u8 ht_protection;
> +
> + /* Values: 0 - GF protection not required, 1 - GF protection required */
> + u8 gf_protection;
> +
> + /*Values: 0 - TX Burst limit not required, 1 - TX Burst Limit required*/
> + u8 ht_tx_burst_limit;
> +
> + /*
> + * Values: 0 - Dual CTS protection not required,
> + * 1 Dual CTS Protection required

Alignment.


--
Cheers,
Luca.