* [PATCH 01/04] wl1271: 11n Support, Add Definitions
2010-09-15 18:31 [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec Shahar Levi
@ 2010-09-15 18:31 ` Shahar Levi
2010-09-15 19:15 ` Luciano Coelho
2010-09-15 18:31 ` [PATCH 02/04] wl1271: 11n Support, ACX Commands Shahar Levi
` (3 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Shahar Levi @ 2010-09-15 18:31 UTC (permalink / raw)
To: linux-wireless; +Cc: Luciano Coelho
Added all definitions needed for 11n support.
Signed-off-by: Shahar Levi <shahar_levi@ti.com>
---
drivers/net/wireless/wl12xx/wl1271.h | 2 +-
drivers/net/wireless/wl12xx/wl1271_acx.h | 91 +++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1271_conf.h | 3 +
drivers/net/wireless/wl12xx/wl1271_main.c | 13 ++++
4 files changed, 108 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
index 4134f44..fd1c2fc 100644
--- a/drivers/net/wireless/wl12xx/wl1271.h
+++ b/drivers/net/wireless/wl12xx/wl1271.h
@@ -423,7 +423,7 @@ struct wl1271 {
/* Our association ID */
u16 aid;
- /* currently configured rate set */
+ /* currently configured rate set: bits0-15 BG, bits 16-23 MCS */
u32 sta_rate_set;
u32 basic_rate_set;
u32 basic_rate;
diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
index 4235bc5..2a8fc43 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.h
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
@@ -993,6 +993,97 @@ struct wl1271_acx_rssi_snr_avg_weights {
u8 snr_data;
};
+/* 11n command to the FW */
+/* Name: ACX_PEER_HT_CAP
+* Desc: Configure HT capabilities - declare the capabilities of the peer
+* we are connected to.
+* Type: Configuration
+* Access: Write Only
+* Length:
+*/
+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 is set to 0.
+ * 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.
+ */
+ u32 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 */
+enum acx_ht_capabilites_fw {
+ WL1271_ACX_FW_CAP_BIT_MASK_HT_OPERATION = BIT(0),
+ WL1271_ACX_FW_CAP_BIT_MASK_GREENFIELD_FRAME_FORMAT = BIT(1),
+ WL1271_ACX_FW_CAP_BIT_MASK_SHORT_GI_FOR_20MHZ_PACKETS = BIT(2),
+ WL1271_ACX_FW_CAP_BIT_MASK_LSIG_TXOP_PROTECTION = BIT(3),
+ WL1271_ACX_FW_CAP_BIT_MASK_HT_CONTROL_FIELDS = BIT(4),
+ WL1271_ACX_FW_CAP_BIT_MASK_RD_INITIATION = BIT(5)
+};
+
+/* Name: ACX_HT_BSS_OPERATION
+ * Desc: Configure HT capabilities - AP rules for behavior in the BSS.
+ * Type: Configuration
+ * Access: Write Only
+ * Length:
+ */
+struct wl1271_acx_ht_information {
+ struct acx_header header;
+
+ u8 rifs_mode; /* Values: 0 - RIFS not allowed, 1 - RIFS allowed */
+
+ u8 ht_protection; /* Values: 0 - 3 like in spec */
+
+ /*
+ * 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_conf.h b/drivers/net/wireless/wl12xx/wl1271_conf.h
index 0435ffd..0e22d41 100644
--- a/drivers/net/wireless/wl12xx/wl1271_conf.h
+++ b/drivers/net/wireless/wl12xx/wl1271_conf.h
@@ -91,6 +91,9 @@ enum {
CONF_HW_RXTX_RATE_UNSUPPORTED = 0xff
};
+#define HW_BG_RATES_MASK 0xffff
+#define HW_HT_RATES_OFFSET 16
+
enum {
CONF_SG_DISABLE = 0,
CONF_SG_PROTECTIVE,
diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index af26150..ae69397 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -2041,6 +2041,19 @@ static const u8 wl1271_rate_to_idx_2ghz[] = {
0 /* CONF_HW_RXTX_RATE_1 */
};
+/* 11n sta capabilities */
+#define WL12xx_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(65), \
+ .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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 01/04] wl1271: 11n Support, Add Definitions
2010-09-15 18:31 ` [PATCH 01/04] wl1271: 11n Support, Add Definitions Shahar Levi
@ 2010-09-15 19:15 ` Luciano Coelho
0 siblings, 0 replies; 21+ messages in thread
From: Luciano Coelho @ 2010-09-15 19:15 UTC (permalink / raw)
To: ext Shahar Levi; +Cc: linux-wireless@vger.kernel.org
On Wed, 2010-09-15 at 20:31 +0200, ext Shahar Levi wrote:
> Added all definitions needed for 11n support.
This could be a bit more descriptive...
> Signed-off-by: Shahar Levi <shahar_levi@ti.com>
> ---
> diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.h b/drivers/net/wireless/wl12xx/wl1271_acx.h
> index 4235bc5..2a8fc43 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_acx.h
> +++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
> @@ -993,6 +993,97 @@ struct wl1271_acx_rssi_snr_avg_weights {
> u8 snr_data;
> };
>
> +/* 11n command to the FW */
> +/* Name: ACX_PEER_HT_CAP
> +* Desc: Configure HT capabilities - declare the capabilities of the peer
> +* we are connected to.
> +* Type: Configuration
> +* Access: Write Only
> +* Length:
> +*/
> +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 is set to 0.
> + * 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.
> + */
> + u32 ht_capabilites;
This should be __le32 instead.
> +/* HT Capabilites Fw Bit Mask Mapping */
> +enum acx_ht_capabilites_fw {
> + WL1271_ACX_FW_CAP_BIT_MASK_HT_OPERATION = BIT(0),
> + WL1271_ACX_FW_CAP_BIT_MASK_GREENFIELD_FRAME_FORMAT = BIT(1),
> + WL1271_ACX_FW_CAP_BIT_MASK_SHORT_GI_FOR_20MHZ_PACKETS = BIT(2),
> + WL1271_ACX_FW_CAP_BIT_MASK_LSIG_TXOP_PROTECTION = BIT(3),
> + WL1271_ACX_FW_CAP_BIT_MASK_HT_CONTROL_FIELDS = BIT(4),
> + WL1271_ACX_FW_CAP_BIT_MASK_RD_INITIATION = BIT(5)
> +};
I have a slight preference to use #defines for this kind of definition,
but we use it in both ways currently, so no need to change.
It is common practice to add a , after the last value in the
enumeration, so that if any new lines are added in the future, the
previous one doesn't need to be changed. So please change to this:
+ WL1271_ACX_FW_CAP_BIT_MASK_RD_INITIATION = BIT(5),
> diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
> index af26150..ae69397 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_main.c
> +++ b/drivers/net/wireless/wl12xx/wl1271_main.c
> @@ -2041,6 +2041,19 @@ static const u8 wl1271_rate_to_idx_2ghz[] = {
> 0 /* CONF_HW_RXTX_RATE_1 */
> };
>
> +/* 11n sta capabilities */
> +#define WL12xx_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(65), \
> + .tx_params = IEEE80211_HT_MCS_TX_DEFINED, \
> + }, \
> +}
I don't like this macro. I'd rather have this explicitly defined
(despite twice) in wl1271_band_2ghz and wl1271_band_5ghz.
Also, it would be nice to avoid using magic numbers (ie. 65). Is it
possible to define a descriptive macro for instead?
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 02/04] wl1271: 11n Support, ACX Commands
2010-09-15 18:31 [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec Shahar Levi
2010-09-15 18:31 ` [PATCH 01/04] wl1271: 11n Support, Add Definitions Shahar Levi
@ 2010-09-15 18:31 ` Shahar Levi
2010-09-15 18:31 ` [PATCH 03/04] wl1271: 11n Support, Scan, Connection, MCS rates Shahar Levi
` (2 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Shahar Levi @ 2010-09-15 18:31 UTC (permalink / raw)
To: linux-wireless; +Cc: Luciano Coelho
Added ACX command to the FW for 11n support.
Signed-off-by: Shahar Levi <shahar_levi@ti.com>
---
drivers/net/wireless/wl12xx/wl1271_acx.c | 91 ++++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1271_acx.h | 5 ++
2 files changed, 96 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.c b/drivers/net/wireless/wl12xx/wl1271_acx.c
index f03ad08..449079e 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.c
@@ -1260,6 +1260,97 @@ 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;
+ /*
+ * 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] = {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 (true == 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 date 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 2a8fc43..1f85799 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.h
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.h
@@ -1215,6 +1215,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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 03/04] wl1271: 11n Support, Scan, Connection, MCS rates
2010-09-15 18:31 [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec Shahar Levi
2010-09-15 18:31 ` [PATCH 01/04] wl1271: 11n Support, Add Definitions Shahar Levi
2010-09-15 18:31 ` [PATCH 02/04] wl1271: 11n Support, ACX Commands Shahar Levi
@ 2010-09-15 18:31 ` Shahar Levi
2010-09-15 18:31 ` [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable Shahar Levi
2010-09-15 18:58 ` [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec Luciano Coelho
4 siblings, 0 replies; 21+ messages in thread
From: Shahar Levi @ 2010-09-15 18:31 UTC (permalink / raw)
To: linux-wireless; +Cc: Luciano Coelho
11n support add to scan, connection via MCS rates
Signed-off-by: Shahar Levi <shahar_levi@ti.com>
---
drivers/net/wireless/wl12xx/wl1271_main.c | 63 +++++++++++++++++++++--------
drivers/net/wireless/wl12xx/wl1271_rx.c | 4 ++
drivers/net/wireless/wl12xx/wl1271_tx.c | 9 ++++
3 files changed, 59 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index ae69397..e89e574 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -782,10 +782,21 @@ 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])) {
+ 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 */
@@ -1622,6 +1633,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;
@@ -1837,6 +1849,21 @@ static void wl1271_op_bss_info_changed(struct ieee80211_hw *hw,
}
}
+ if (sta) {
+ if (changed & BSS_CHANGED_HT) {
+ ret = wl1271_acx_set_ht_capabilities(wl,
+ &sta->ht_cap,
+ true);
+ ret = wl1271_acx_set_ht_information(wl,
+ bss_conf->ht_operation_mode);
+ }
+ else
+ if (changed & BSS_CHANGED_ASSOC)
+ ret = wl1271_acx_set_ht_capabilities(wl,
+ &sta->ht_cap,
+ false);
+ }
+
if (changed & BSS_CHANGED_ARP_FILTER) {
__be32 addr = bss_conf->arp_addr_list[0];
WARN_ON(wl->bss_type != BSS_TYPE_STA_BSS);
@@ -2014,14 +2041,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 */
@@ -2060,6 +2087,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 = WL12xx_HT_CAP,
};
/* 5 GHz data rates for WL1273 */
@@ -2139,14 +2167,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 */
@@ -2171,6 +2199,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 = WL12xx_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 019aa79..e752215 100644
--- a/drivers/net/wireless/wl12xx/wl1271_rx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_rx.c
@@ -53,6 +53,10 @@ static void wl1271_rx_status(struct wl1271 *wl,
status->band = wl->band;
status->rate_idx = wl1271_rate_to_idx(wl, desc->rate);
+ /* 11n support */
+ if (desc->rate <= CONF_HW_RXTX_RATE_MCS0)
+ status->flag |= RX_FLAG_HT;
+
status->signal = desc->rssi;
/*
diff --git a/drivers/net/wireless/wl12xx/wl1271_tx.c b/drivers/net/wireless/wl12xx/wl1271_tx.c
index dc0b46c..3dd9766 100644
--- a/drivers/net/wireless/wl12xx/wl1271_tx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_tx.c
@@ -236,6 +236,15 @@ u32 wl1271_tx_enabled_rates_get(struct wl1271 *wl, u32 rate_set)
rate_set >>= 1;
}
+ /* 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;
+ }
+
return enabled_rates;
}
--
1.6.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
2010-09-15 18:31 [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec Shahar Levi
` (2 preceding siblings ...)
2010-09-15 18:31 ` [PATCH 03/04] wl1271: 11n Support, Scan, Connection, MCS rates Shahar Levi
@ 2010-09-15 18:31 ` Shahar Levi
2010-09-15 18:53 ` Johannes Berg
2010-09-15 19:21 ` Luciano Coelho
2010-09-15 18:58 ` [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec Luciano Coelho
4 siblings, 2 replies; 21+ messages in thread
From: Shahar Levi @ 2010-09-15 18:31 UTC (permalink / raw)
To: linux-wireless; +Cc: Luciano Coelho
Add support to configurable 11n
Signed-off-by: Shahar Levi <shahar_levi@ti.com>
---
drivers/net/wireless/wl12xx/Kconfig | 7 +++++++
drivers/net/wireless/wl12xx/Makefile | 4 ++++
drivers/net/wireless/wl12xx/wl1271_main.c | 2 ++
3 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/Kconfig b/drivers/net/wireless/wl12xx/Kconfig
index 4a8bb25..1d5e5fd 100644
--- a/drivers/net/wireless/wl12xx/Kconfig
+++ b/drivers/net/wireless/wl12xx/Kconfig
@@ -52,6 +52,13 @@ config WL1271
If you choose to build a module, it'll be called wl1271. Say N if
unsure.
+config WL1271_80211_HT
+ bool "TI wl1271 802.11 HT support (EXPERIMENTAL)"
+ depends on WL1271 && EXPERIMENTAL
+ default y
+ ---help---
+ This will enable 802.11 HT support for TI wl1271 chipset.
+
config WL1271_SPI
tristate "TI wl1271 SPI support"
depends on WL1271 && SPI_MASTER
diff --git a/drivers/net/wireless/wl12xx/Makefile b/drivers/net/wireless/wl12xx/Makefile
index 078b439..e0db9db 100644
--- a/drivers/net/wireless/wl12xx/Makefile
+++ b/drivers/net/wireless/wl12xx/Makefile
@@ -16,3 +16,7 @@ wl1271-$(CONFIG_NL80211_TESTMODE) += wl1271_testmode.o
obj-$(CONFIG_WL1271) += wl1271.o
obj-$(CONFIG_WL1271_SPI) += wl1271_spi.o
obj-$(CONFIG_WL1271_SDIO) += wl1271_sdio.o
+
+ifeq ($(CONFIG_WL1271_80211_HT),y)
+EXTRA_CFLAGS += -DCONFIG_WL1271_HT
+endif
diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index e89e574..8f2cea9 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -2087,7 +2087,9 @@ static struct ieee80211_supported_band wl1271_band_2ghz = {
.n_channels = ARRAY_SIZE(wl1271_channels),
.bitrates = wl1271_rates,
.n_bitrates = ARRAY_SIZE(wl1271_rates),
+#ifdef CONFIG_WL1271_HT
.ht_cap = WL12xx_HT_CAP,
+#endif
};
/* 5 GHz data rates for WL1273 */
--
1.6.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
2010-09-15 18:31 ` [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable Shahar Levi
@ 2010-09-15 18:53 ` Johannes Berg
2010-09-15 18:56 ` Luciano Coelho
2010-09-15 19:21 ` Luciano Coelho
1 sibling, 1 reply; 21+ messages in thread
From: Johannes Berg @ 2010-09-15 18:53 UTC (permalink / raw)
To: Shahar Levi; +Cc: linux-wireless, Luciano Coelho
On Wed, 2010-09-15 at 20:31 +0200, Shahar Levi wrote:
> Add support to configurable 11n
This really makes no sense.
johannes
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
2010-09-15 18:53 ` Johannes Berg
@ 2010-09-15 18:56 ` Luciano Coelho
0 siblings, 0 replies; 21+ messages in thread
From: Luciano Coelho @ 2010-09-15 18:56 UTC (permalink / raw)
To: ext Johannes Berg; +Cc: Shahar Levi, linux-wireless@vger.kernel.org
On Wed, 2010-09-15 at 20:53 +0200, ext Johannes Berg wrote:
> On Wed, 2010-09-15 at 20:31 +0200, Shahar Levi wrote:
> > Add support to configurable 11n
>
> This really makes no sense.
Yes, that's true. In real life it makes no sense, but we required this
temporarily because we want to keep this completely disable until it's
proven to work correctly (and we have tested it more thoroughly). There
are still a few missing bits in this implementation...
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
2010-09-15 18:31 ` [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable Shahar Levi
2010-09-15 18:53 ` Johannes Berg
@ 2010-09-15 19:21 ` Luciano Coelho
2010-09-16 0:39 ` Julian Calaby
1 sibling, 1 reply; 21+ messages in thread
From: Luciano Coelho @ 2010-09-15 19:21 UTC (permalink / raw)
To: ext Shahar Levi; +Cc: linux-wireless@vger.kernel.org
On Wed, 2010-09-15 at 20:31 +0200, ext Shahar Levi wrote:
> Add support to configurable 11n
Please rephrase and make it more descriptive. Explain that this is
temporary because the code is incomplete and not tested enough.
> diff --git a/drivers/net/wireless/wl12xx/Kconfig b/drivers/net/wireless/wl12xx/Kconfig
> index 4a8bb25..1d5e5fd 100644
> --- a/drivers/net/wireless/wl12xx/Kconfig
> +++ b/drivers/net/wireless/wl12xx/Kconfig
> @@ -52,6 +52,13 @@ config WL1271
> If you choose to build a module, it'll be called wl1271. Say N if
> unsure.
>
> +config WL1271_80211_HT
> + bool "TI wl1271 802.11 HT support (EXPERIMENTAL)"
> + depends on WL1271 && EXPERIMENTAL
> + default y
For now, I'd prefer to have this as default n.
> + ---help---
> + This will enable 802.11 HT support for TI wl1271 chipset.
> +
This needs to be much more descriptive in order to avoid comments like
Johannes's. ;)
Please make it clear why the option exist, that it is temporary because
the code is not complete and not thoroughly tested etc.
> diff --git a/drivers/net/wireless/wl12xx/Makefile b/drivers/net/wireless/wl12xx/Makefile
> index 078b439..e0db9db 100644
> --- a/drivers/net/wireless/wl12xx/Makefile
> +++ b/drivers/net/wireless/wl12xx/Makefile
> @@ -16,3 +16,7 @@ wl1271-$(CONFIG_NL80211_TESTMODE) += wl1271_testmode.o
> obj-$(CONFIG_WL1271) += wl1271.o
> obj-$(CONFIG_WL1271_SPI) += wl1271_spi.o
> obj-$(CONFIG_WL1271_SDIO) += wl1271_sdio.o
> +
> +ifeq ($(CONFIG_WL1271_80211_HT),y)
> +EXTRA_CFLAGS += -DCONFIG_WL1271_HT
> +endif
Please don't do this. Just use the CONFIG_WL1271_80211_HT flag directly
in the code (see below).
> diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
> index e89e574..8f2cea9 100644
> --- a/drivers/net/wireless/wl12xx/wl1271_main.c
> +++ b/drivers/net/wireless/wl12xx/wl1271_main.c
> @@ -2087,7 +2087,9 @@ static struct ieee80211_supported_band wl1271_band_2ghz = {
> .n_channels = ARRAY_SIZE(wl1271_channels),
> .bitrates = wl1271_rates,
> .n_bitrates = ARRAY_SIZE(wl1271_rates),
> +#ifdef CONFIG_WL1271_HT
> .ht_cap = WL12xx_HT_CAP,
> +#endif
Here you can use #ifdef CONFIG_WL1271_80211_HT directly, no need to
duplicate it into a new flag.
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
2010-09-15 19:21 ` Luciano Coelho
@ 2010-09-16 0:39 ` Julian Calaby
2010-09-16 15:54 ` Levi, Shahar
0 siblings, 1 reply; 21+ messages in thread
From: Julian Calaby @ 2010-09-16 0:39 UTC (permalink / raw)
To: Luciano Coelho; +Cc: ext Shahar Levi, linux-wireless@vger.kernel.org
On Thu, Sep 16, 2010 at 05:21, Luciano Coelho <luciano.coelho@nokia.com> wrote:
> On Wed, 2010-09-15 at 20:31 +0200, ext Shahar Levi wrote:
>> diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
>> index e89e574..8f2cea9 100644
>> --- a/drivers/net/wireless/wl12xx/wl1271_main.c
>> +++ b/drivers/net/wireless/wl12xx/wl1271_main.c
>> @@ -2087,7 +2087,9 @@ static struct ieee80211_supported_band wl1271_band_2ghz = {
>> .n_channels = ARRAY_SIZE(wl1271_channels),
>> .bitrates = wl1271_rates,
>> .n_bitrates = ARRAY_SIZE(wl1271_rates),
>> +#ifdef CONFIG_WL1271_HT
>> .ht_cap = WL12xx_HT_CAP,
>> +#endif
>
> Here you can use #ifdef CONFIG_WL1271_80211_HT directly, no need to
> duplicate it into a new flag.
Another thing you might want to do could be to adjust the order of the
patches so you introduce this config option *before* you introduce the
actual code that is protected by it. At the moment, this functionality
is enabled unconditionally if this patch isn't applied, which could
potentially occur during bisection, given that John doesn't squash the
patches into one when he applies them.
Thanks,
--
Julian Calaby
Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
^ permalink raw reply [flat|nested] 21+ messages in thread* RE: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
2010-09-16 0:39 ` Julian Calaby
@ 2010-09-16 15:54 ` Levi, Shahar
2010-09-16 17:16 ` Julian Calaby
0 siblings, 1 reply; 21+ messages in thread
From: Levi, Shahar @ 2010-09-16 15:54 UTC (permalink / raw)
To: Julian Calaby, Luciano Coelho; +Cc: linux-wireless@vger.kernel.org
> -----Original Message-----
> From: Julian Calaby [mailto:julian.calaby@gmail.com]
> Sent: Thursday, September 16, 2010 2:39 AM
> To: Luciano Coelho
> Cc: Levi, Shahar; linux-wireless@vger.kernel.org
> Subject: Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
>
> On Thu, Sep 16, 2010 at 05:21, Luciano Coelho <luciano.coelho@nokia.com>
> wrote:
> > On Wed, 2010-09-15 at 20:31 +0200, ext Shahar Levi wrote:
> >> diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c
> b/drivers/net/wireless/wl12xx/wl1271_main.c
> >> index e89e574..8f2cea9 100644
> >> --- a/drivers/net/wireless/wl12xx/wl1271_main.c
> >> +++ b/drivers/net/wireless/wl12xx/wl1271_main.c
> >> @@ -2087,7 +2087,9 @@ static struct ieee80211_supported_band
> wl1271_band_2ghz = {
> >> .n_channels = ARRAY_SIZE(wl1271_channels),
> >> .bitrates = wl1271_rates,
> >> .n_bitrates = ARRAY_SIZE(wl1271_rates),
> >> +#ifdef CONFIG_WL1271_HT
> >> .ht_cap = WL12xx_HT_CAP,
> >> +#endif
> >
> > Here you can use #ifdef CONFIG_WL1271_80211_HT directly, no need to
> > duplicate it into a new flag.
>
> Another thing you might want to do could be to adjust the order of the
> patches so you introduce this config option *before* you introduce the
> actual code that is protected by it. At the moment, this functionality
> is enabled unconditionally if this patch isn't applied, which could
> potentially occur during bisection, given that John doesn't squash the
> patches into one when he applies them.
>
> Thanks,
>
> --
>
> Julian Calaby
>
> Email: julian.calaby@gmail.com
> Profile: http://www.google.com/profiles/julian.calaby/
> .Plan: http://sites.google.com/site/juliancalaby/
Hi Julian,
It is a good point. ".ht_cap = WL12xx_HT_CAP" in wl1271_main.c adds only on patch 03/04. I believe that adding "#ifdef CONFIG_WL1271_HT" in wl1271_main.c with early patch on code that not exist is less clear. Squashing the patches 03 and 04 to one in v2 could prevent that but it will not emphasize the configuration option. Is there are cases that the some patches from series not applies?
Thanks for your review,
Shahar
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
2010-09-16 15:54 ` Levi, Shahar
@ 2010-09-16 17:16 ` Julian Calaby
2010-09-18 17:56 ` Levi, Shahar
0 siblings, 1 reply; 21+ messages in thread
From: Julian Calaby @ 2010-09-16 17:16 UTC (permalink / raw)
To: Levi, Shahar; +Cc: Luciano Coelho, linux-wireless@vger.kernel.org
2010/9/17 Levi, Shahar <shahar_levi@ti.com>:
>> -----Original Message-----
>> From: Julian Calaby [mailto:julian.calaby@gmail.com]
>> Sent: Thursday, September 16, 2010 2:39 AM
>> To: Luciano Coelho
>> Cc: Levi, Shahar; linux-wireless@vger.kernel.org
>> Subject: Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
>>
>> On Thu, Sep 16, 2010 at 05:21, Luciano Coelho <luciano.coelho@nokia.com>
>> wrote:
>> > On Wed, 2010-09-15 at 20:31 +0200, ext Shahar Levi wrote:
>> >> diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c
>> b/drivers/net/wireless/wl12xx/wl1271_main.c
>> >> index e89e574..8f2cea9 100644
>> >> --- a/drivers/net/wireless/wl12xx/wl1271_main.c
>> >> +++ b/drivers/net/wireless/wl12xx/wl1271_main.c
>> >> @@ -2087,7 +2087,9 @@ static struct ieee80211_supported_band
>> wl1271_band_2ghz = {
>> >> .n_channels = ARRAY_SIZE(wl1271_channels),
>> >> .bitrates = wl1271_rates,
>> >> .n_bitrates = ARRAY_SIZE(wl1271_rates),
>> >> +#ifdef CONFIG_WL1271_HT
>> >> .ht_cap = WL12xx_HT_CAP,
>> >> +#endif
>> >
>> > Here you can use #ifdef CONFIG_WL1271_80211_HT directly, no need to
>> > duplicate it into a new flag.
>>
>> Another thing you might want to do could be to adjust the order of the
>> patches so you introduce this config option *before* you introduce the
>> actual code that is protected by it. At the moment, this functionality
>> is enabled unconditionally if this patch isn't applied, which could
>> potentially occur during bisection, given that John doesn't squash the
>> patches into one when he applies them.
>>
>> Thanks,
>>
>> --
>>
>> Julian Calaby
>>
>> Email: julian.calaby@gmail.com
>> Profile: http://www.google.com/profiles/julian.calaby/
>> .Plan: http://sites.google.com/site/juliancalaby/
>
> Hi Julian,
> It is a good point. ".ht_cap = WL12xx_HT_CAP" in wl1271_main.c adds only on patch 03/04. I believe that adding "#ifdef CONFIG_WL1271_HT" in wl1271_main.c with early patch on code that not exist is less clear. Squashing the patches 03 and 04 to one in v2 could prevent that but it will not emphasize the configuration option. Is there are cases that the some patches from series not applies?
Bisection (which seems to be my watch word, at the moment) could jump
into the middle of random patch sets like this.
Personally, I'd squash 3 and 4 into one patch: I.e. you're saying
"here's the functionality and it's protected by a Kconfig variable."
The fact that it's touching the Kconfig file is probably enough
emphasis in and of itself.
> Thanks for your review,
No problem,
--
Julian Calaby
Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
^ permalink raw reply [flat|nested] 21+ messages in thread* RE: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
2010-09-16 17:16 ` Julian Calaby
@ 2010-09-18 17:56 ` Levi, Shahar
2010-09-19 1:19 ` Julian Calaby
0 siblings, 1 reply; 21+ messages in thread
From: Levi, Shahar @ 2010-09-18 17:56 UTC (permalink / raw)
To: Julian Calaby; +Cc: Luciano Coelho, linux-wireless@vger.kernel.org
> -----Original Message-----
> From: Julian Calaby [mailto:julian.calaby@gmail.com]
> Sent: Thursday, September 16, 2010 7:17 PM
> To: Levi, Shahar
> Cc: Luciano Coelho; linux-wireless@vger.kernel.org
> Subject: Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
>
> 2010/9/17 Levi, Shahar <shahar_levi@ti.com>:
> >> -----Original Message-----
> >> From: Julian Calaby [mailto:julian.calaby@gmail.com]
> >> Sent: Thursday, September 16, 2010 2:39 AM
> >> To: Luciano Coelho
> >> Cc: Levi, Shahar; linux-wireless@vger.kernel.org
> >> Subject: Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
> >>
> >> On Thu, Sep 16, 2010 at 05:21, Luciano Coelho <luciano.coelho@nokia.com>
> >> wrote:
> >> > On Wed, 2010-09-15 at 20:31 +0200, ext Shahar Levi wrote:
> >> >> diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c
> >> b/drivers/net/wireless/wl12xx/wl1271_main.c
> >> >> index e89e574..8f2cea9 100644
> >> >> --- a/drivers/net/wireless/wl12xx/wl1271_main.c
> >> >> +++ b/drivers/net/wireless/wl12xx/wl1271_main.c
> >> >> @@ -2087,7 +2087,9 @@ static struct ieee80211_supported_band
> >> wl1271_band_2ghz = {
> >> >> .n_channels = ARRAY_SIZE(wl1271_channels),
> >> >> .bitrates = wl1271_rates,
> >> >> .n_bitrates = ARRAY_SIZE(wl1271_rates),
> >> >> +#ifdef CONFIG_WL1271_HT
> >> >> .ht_cap = WL12xx_HT_CAP,
> >> >> +#endif
> >> >
> >> > Here you can use #ifdef CONFIG_WL1271_80211_HT directly, no need to
> >> > duplicate it into a new flag.
> >>
> >> Another thing you might want to do could be to adjust the order of the
> >> patches so you introduce this config option *before* you introduce the
> >> actual code that is protected by it. At the moment, this functionality
> >> is enabled unconditionally if this patch isn't applied, which could
> >> potentially occur during bisection, given that John doesn't squash the
> >> patches into one when he applies them.
> >>
> >> Thanks,
> >>
> >> --
> >>
> >> Julian Calaby
> >>
> >
> > Hi Julian,
> > It is a good point. ".ht_cap = WL12xx_HT_CAP" in wl1271_main.c adds only on
> patch 03/04. I believe that adding "#ifdef CONFIG_WL1271_HT" in wl1271_main.c
> with early patch on code that not exist is less clear. Squashing the patches
> 03 and 04 to one in v2 could prevent that but it will not emphasize the
> configuration option. Is there are cases that the some patches from series not
> applies?
>
> Bisection (which seems to be my watch word, at the moment) could jump
> into the middle of random patch sets like this.
>
> Personally, I'd squash 3 and 4 into one patch: I.e. you're saying
> "here's the functionality and it's protected by a Kconfig variable."
> The fact that it's touching the Kconfig file is probably enough
> emphasis in and of itself.
>
> > Thanks for your review,
>
> No problem,
>
> --
>
> Julian Calaby
>
> Email: julian.calaby@gmail.com
> Profile: http://www.google.com/profiles/julian.calaby/
> .Plan: http://sites.google.com/site/juliancalaby/
I am confuse. If there option that patch isn't applied from a series how we could trust any patches series we send to that list. I would greatly appreciate if you help me to understand and elaborate what it bisection danger. Am I missing something?
I really appreciate your willingness to help.
Regards,
Shahar
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
2010-09-18 17:56 ` Levi, Shahar
@ 2010-09-19 1:19 ` Julian Calaby
2010-09-19 12:23 ` Levi, Shahar
0 siblings, 1 reply; 21+ messages in thread
From: Julian Calaby @ 2010-09-19 1:19 UTC (permalink / raw)
To: Levi, Shahar; +Cc: Luciano Coelho, linux-wireless@vger.kernel.org
2010/9/19 Levi, Shahar <shahar_levi@ti.com>:
> I am confuse. If there option that patch isn't applied from a series how we could trust any patches series we send to that list. I would greatly appreciate if you help me to understand and elaborate what it bisection danger. Am I missing something?
> I really appreciate your willingness to help.
Unless people dislike your patches, there's no danger that they won't
all be applied to the kernel.
That said, there are still circumstances when only part of a patch
series like your one will be applied to a kernel. One of these is when
a user is using git bisection to find exactly which commit caused a
particular bug.
So, let's say you upgrade your kernel from 2.6.35 to 2.6.38 and your
wl1271 device now won't connect to an AP running on some random
proprietary hardware. The simplest way to determine exactly which
patch caused this issue is to use git bisect to do a binary search
over the changes between these two kernel versions to determine the
exact patch that broke it. Now, let's assume that, this particular
piece of hardware won't connect to *anything* when 802.11n support is
added, so you've already excluded that from your kernel. If git bisect
decides to leap into the middle of your series, so that 802.11n
support is added before the Kconfig option is added, then 802.11n
support will be unconditionally enabled, and you'll see the symptoms
of that bug, and won't be able to quickly determine whether the bug
existed before the 802.11n patches or after, and git bisect might
wrongly assume that these patches were the cause of the bug.
As such, it's generally regarded as a good idea to introduce Kconfig
options with the code they guard.
Thanks,
--
Julian Calaby
Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
2010-09-19 1:19 ` Julian Calaby
@ 2010-09-19 12:23 ` Levi, Shahar
0 siblings, 0 replies; 21+ messages in thread
From: Levi, Shahar @ 2010-09-19 12:23 UTC (permalink / raw)
To: Julian Calaby; +Cc: Luciano Coelho, linux-wireless@vger.kernel.org
> -----Original Message-----
> From: Julian Calaby [mailto:julian.calaby@gmail.com]
> Sent: Sunday, September 19, 2010 3:19 AM
> To: Levi, Shahar
> Cc: Luciano Coelho; linux-wireless@vger.kernel.org
> Subject: Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable
>
> Unless people dislike your patches, there's no danger that they won't
> all be applied to the kernel.
>
> That said, there are still circumstances when only part of a patch
> series like your one will be applied to a kernel. One of these is when
> a user is using git bisection to find exactly which commit caused a
> particular bug.
>
> So, let's say you upgrade your kernel from 2.6.35 to 2.6.38 and your
> wl1271 device now won't connect to an AP running on some random
> proprietary hardware. The simplest way to determine exactly which
> patch caused this issue is to use git bisect to do a binary search
> over the changes between these two kernel versions to determine the
> exact patch that broke it. Now, let's assume that, this particular
> piece of hardware won't connect to *anything* when 802.11n support is
> added, so you've already excluded that from your kernel. If git bisect
> decides to leap into the middle of your series, so that 802.11n
> support is added before the Kconfig option is added, then 802.11n
> support will be unconditionally enabled, and you'll see the symptoms
> of that bug, and won't be able to quickly determine whether the bug
> existed before the 802.11n patches or after, and git bisect might
> wrongly assume that these patches were the cause of the bug.
>
> As such, it's generally regarded as a good idea to introduce Kconfig
> options with the code they guard.
>
> Thanks,
>
> --
>
> Julian Calaby
>
> Email: julian.calaby@gmail.com
> Profile: http://www.google.com/profiles/julian.calaby/
> .Plan: http://sites.google.com/site/juliancalaby/
Hi Julian,
You are right. I will squash 3 and 4 into one patch in v2.
I greatly appreciate your elaboration and help to understand.
Thanks you,
Shahar
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec
2010-09-15 18:31 [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec Shahar Levi
` (3 preceding siblings ...)
2010-09-15 18:31 ` [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable Shahar Levi
@ 2010-09-15 18:58 ` Luciano Coelho
2010-09-15 19:25 ` Luciano Coelho
4 siblings, 1 reply; 21+ messages in thread
From: Luciano Coelho @ 2010-09-15 18:58 UTC (permalink / raw)
To: ext Shahar Levi; +Cc: linux-wireless@vger.kernel.org
Hi Shahar,
On Wed, 2010-09-15 at 20:31 +0200, ext Shahar Levi wrote:
> Series that add 80211n spec support to wl1271: scan, connection via MCS rates
Very nice! Thank you for this implementation!
It would be nice to describe this a bit more thoroughly, what is already
implemented, what is still missing etc. So that people who want to use
can know the status of the implementation.
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec
2010-09-15 18:58 ` [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec Luciano Coelho
@ 2010-09-15 19:25 ` Luciano Coelho
2010-09-15 19:34 ` Levi, Shahar
0 siblings, 1 reply; 21+ messages in thread
From: Luciano Coelho @ 2010-09-15 19:25 UTC (permalink / raw)
To: ext Shahar Levi; +Cc: linux-wireless@vger.kernel.org
On Wed, 2010-09-15 at 20:58 +0200, Luciano Coelho wrote:
> Hi Shahar,
>
> On Wed, 2010-09-15 at 20:31 +0200, ext Shahar Levi wrote:
> > Series that add 80211n spec support to wl1271: scan, connection via MCS rates
>
> Very nice! Thank you for this implementation!
>
> It would be nice to describe this a bit more thoroughly, what is already
> implemented, what is still missing etc. So that people who want to use
> can know the status of the implementation.
I had a few comments to your patches, but I'm really happy you have sent
these patches upstream! Just some minor non-functional comments. Please
make the changes I proposed and send the series again as v2.
I owe you a drink, if we meet some day ;)
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec
2010-09-15 19:25 ` Luciano Coelho
@ 2010-09-15 19:34 ` Levi, Shahar
2010-09-15 20:20 ` Luis R. Rodriguez
0 siblings, 1 reply; 21+ messages in thread
From: Levi, Shahar @ 2010-09-15 19:34 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless@vger.kernel.org
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBMdWNpYW5vIENvZWxobyBbbWFp
bHRvOmx1Y2lhbm8uY29lbGhvQG5va2lhLmNvbV0NCj4gU2VudDogV2VkbmVzZGF5LCBTZXB0ZW1i
ZXIgMTUsIDIwMTAgOToyNiBQTQ0KPiBUbzogTGV2aSwgU2hhaGFyDQo+IENjOiBsaW51eC13aXJl
bGVzc0B2Z2VyLmtlcm5lbC5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAwMC8wNF0gd2wxMjcx
OiAxMW4gU3VwcG9ydCwgYWRkIHN1cHBvcnQgdG8gODAyMTFuIHNwZWMNCj4gDQo+IEkgaGFkIGEg
ZmV3IGNvbW1lbnRzIHRvIHlvdXIgcGF0Y2hlcywgYnV0IEknbSByZWFsbHkgaGFwcHkgeW91IGhh
dmUgc2VudA0KPiB0aGVzZSBwYXRjaGVzIHVwc3RyZWFtISBKdXN0IHNvbWUgbWlub3Igbm9uLWZ1
bmN0aW9uYWwgY29tbWVudHMuICBQbGVhc2UNCj4gbWFrZSB0aGUgY2hhbmdlcyBJIHByb3Bvc2Vk
IGFuZCBzZW5kIHRoZSBzZXJpZXMgYWdhaW4gYXMgdjIuDQo+IA0KPiBJIG93ZSB5b3UgYSBkcmlu
aywgaWYgd2UgbWVldCBzb21lIGRheSA7KQ0KPiANCj4gLS0NCj4gQ2hlZXJzLA0KPiBMdWNhLg0K
DQpIaSBMdWNhLA0KSSBncmVhdGx5IGFwcHJlY2lhdGUgeW91ciByZXZpZXcgYW5kIGNvbW1lbnRz
Lg0KSSB3aWxsIGZpeCBlYXJseSBuZXh0IHdlZWsgYW5kIHNlbmQuDQoNCkNoZWVycywNClNoYWhh
cg0K
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec
2010-09-15 19:34 ` Levi, Shahar
@ 2010-09-15 20:20 ` Luis R. Rodriguez
2010-09-15 20:40 ` Luciano Coelho
0 siblings, 1 reply; 21+ messages in thread
From: Luis R. Rodriguez @ 2010-09-15 20:20 UTC (permalink / raw)
To: Levi, Shahar; +Cc: Luciano Coelho, linux-wireless@vger.kernel.org
On Wed, Sep 15, 2010 at 12:34 PM, Levi, Shahar <shahar_levi@ti.com> wrote:
>> -----Original Message-----
>> From: Luciano Coelho [mailto:luciano.coelho@nokia.com]
>> Sent: Wednesday, September 15, 2010 9:26 PM
>> To: Levi, Shahar
>> Cc: linux-wireless@vger.kernel.org
>> Subject: Re: [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec
>>
>> I had a few comments to your patches, but I'm really happy you have sent
>> these patches upstream! Just some minor non-functional comments. Please
>> make the changes I proposed and send the series again as v2.
>>
>> I owe you a drink, if we meet some day ;)
>>
>> --
>> Cheers,
>> Luca.
>
> Hi Luca,
> I greatly appreciate your review and comments.
> I will fix early next week and send.
Might be good to add details of what is supported to the wiki too.
Luis
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec
2010-09-15 20:20 ` Luis R. Rodriguez
@ 2010-09-15 20:40 ` Luciano Coelho
2010-09-16 15:24 ` Levi, Shahar
0 siblings, 1 reply; 21+ messages in thread
From: Luciano Coelho @ 2010-09-15 20:40 UTC (permalink / raw)
To: ext Luis R. Rodriguez; +Cc: Levi, Shahar, linux-wireless@vger.kernel.org
On Wed, 2010-09-15 at 22:20 +0200, ext Luis R. Rodriguez wrote:
> On Wed, Sep 15, 2010 at 12:34 PM, Levi, Shahar <shahar_levi@ti.com> wrote:
> >> -----Original Message-----
> >> From: Luciano Coelho [mailto:luciano.coelho@nokia.com]
> >> Sent: Wednesday, September 15, 2010 9:26 PM
> >> To: Levi, Shahar
> >> Cc: linux-wireless@vger.kernel.org
> >> Subject: Re: [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec
> >>
> >> I had a few comments to your patches, but I'm really happy you have sent
> >> these patches upstream! Just some minor non-functional comments. Please
> >> make the changes I proposed and send the series again as v2.
> >>
> >> I owe you a drink, if we meet some day ;)
> >>
> >> --
> >> Cheers,
> >> Luca.
> >
> > Hi Luca,
> > I greatly appreciate your review and comments.
> > I will fix early next week and send.
>
> Might be good to add details of what is supported to the wiki too.
Good point! Actually the wiki is quite outdated as nobody has been
taking care of it for a very long time. If anyone wants to volunteer to
update it, this is the location:
http://wireless.kernel.org/en/users/Drivers/wl12xx
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 00/04] wl1271: 11n Support, add support to 80211n spec
2010-09-15 20:40 ` Luciano Coelho
@ 2010-09-16 15:24 ` Levi, Shahar
0 siblings, 0 replies; 21+ messages in thread
From: Levi, Shahar @ 2010-09-16 15:24 UTC (permalink / raw)
To: Luciano Coelho, ext Luis R. Rodriguez; +Cc: linux-wireless@vger.kernel.org
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogTHVjaWFubyBDb2VsaG8g
W21haWx0bzpsdWNpYW5vLmNvZWxob0Bub2tpYS5jb21dDQo+IFNlbnQ6IFdlZG5lc2RheSwgU2Vw
dGVtYmVyIDE1LCAyMDEwIDEwOjQwIFBNDQo+IFRvOiBleHQgTHVpcyBSLiBSb2RyaWd1ZXoNCj4g
Q2M6IExldmksIFNoYWhhcjsgbGludXgtd2lyZWxlc3NAdmdlci5rZXJuZWwub3JnDQo+IFN1Ympl
Y3Q6IFJlOiBbUEFUQ0ggMDAvMDRdIHdsMTI3MTogMTFuIFN1cHBvcnQsIGFkZCBzdXBwb3J0IHRv
IDgwMjExbiBzcGVjDQo+IA0KPiA+DQo+ID4gTWlnaHQgYmUgZ29vZCB0byBhZGQgZGV0YWlscyBv
ZiB3aGF0IGlzIHN1cHBvcnRlZCB0byB0aGUgd2lraSB0b28uDQo+IA0KPiBHb29kIHBvaW50ISBB
Y3R1YWxseSB0aGUgd2lraSBpcyBxdWl0ZSBvdXRkYXRlZCBhcyBub2JvZHkgaGFzIGJlZW4NCj4g
dGFraW5nIGNhcmUgb2YgaXQgZm9yIGEgdmVyeSBsb25nIHRpbWUuICBJZiBhbnlvbmUgd2FudHMg
dG8gdm9sdW50ZWVyIHRvDQo+IHVwZGF0ZSBpdCwgdGhpcyBpcyB0aGUgbG9jYXRpb246DQo+IA0K
PiBodHRwOi8vd2lyZWxlc3Mua2VybmVsLm9yZy9lbi91c2Vycy9Ecml2ZXJzL3dsMTJ4eA0KPiAN
Cj4gLS0NCj4gQ2hlZXJzLA0KPiBMdWNhLg0KSSB3aWxsIHVwZGF0ZSB0aGUgd2lraSBuZXh0IHdl
ZWsgb24gdGhlIG5ldyBzdXBwb3J0Lg0KUmVnYXJkcywNClNoYWhhcg0K
^ permalink raw reply [flat|nested] 21+ messages in thread