[PATCH] ath10k: Support 32+ stations.

[PATCH] ath10k: Support 32+ stations.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Implement 64-bit vdev map to support larger numbers
of virtual devices.  Support up to 32 stations
(actual limit will be determined when loading firmware,
as different firmwares have different limits)

Enable larger limits when using Candela Technologies
firmware.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

NOTE:  The 10.2 firmware uses the bit that used to be used
by CT firmware, so existing CT firmware will not actually
work with this patch (or any kernel with the 10.2 firmware patch).
If I can get acceptance of this patch or even a simpler patch
that just adds the enum for CT firmware feature,
I can immediately re-do CT firmware with the proper bit
set.  If there is absolutely no way that even minimal
CT firmware support can be accepted upstream, then I
will just choose a large feature flag and hope it is not
re-used by upstream code any time soon.

 drivers/net/wireless/ath/ath10k/core.c |  8 +++--
 drivers/net/wireless/ath/ath10k/core.h |  5 ++-
 drivers/net/wireless/ath/ath10k/hw.h   |  6 ++++
 drivers/net/wireless/ath/ath10k/mac.c  | 65 ++++++++++++++++++++++++++++------
 drivers/net/wireless/ath/ath10k/wmi.c  | 25 ++++++++++---
 5 files changed, 89 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 27d027d..030bef1 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -844,10 +844,12 @@ int ath10k_core_start(struct ath10k *ar)
 	if (status)
 		goto err_htc_stop;
 
-	if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
-		ar->free_vdev_map = (1 << TARGET_10X_NUM_VDEVS) - 1;
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
+		ar->free_vdev_map = (1LL << TARGET_10X_NUM_VDEVS_CT) - 1;
+	else if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
+		ar->free_vdev_map = (1LL << TARGET_10X_NUM_VDEVS) - 1;
 	else
-		ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
+		ar->free_vdev_map = (1LL << TARGET_NUM_VDEVS) - 1;
 
 	INIT_LIST_HEAD(&ar->arvifs);
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 58c6348..777957e 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -425,6 +425,9 @@ enum ath10k_fw_features {
 	 */
 	ATH10K_FW_FEATURE_WMI_10_2 = 4,
 
+	/* Firmware from Candela Technologies, enables more VIFs, etc */
+	ATH10K_FW_FEATURE_WMI_10X_CT = 5,
+
 	/* keep last */
 	ATH10K_FW_FEATURE_COUNT,
 };
@@ -526,7 +529,7 @@ struct ath10k {
 	/* current operating channel definition */
 	struct cfg80211_chan_def chandef;
 
-	int free_vdev_map;
+	unsigned long long free_vdev_map;
 	bool promisc;
 	bool monitor;
 	int monitor_vdev_id;
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 291ad23..0bb3744 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -122,6 +122,12 @@ enum ath10k_mcast2ucast_mode {
 #define TARGET_10X_AST_SKID_LIMIT		16
 #define TARGET_10X_NUM_PEERS			(128 + (TARGET_10X_NUM_VDEVS))
 #define TARGET_10X_NUM_PEERS_MAX		128
+
+/* Over-rides for Candela Technologies firmware */
+#define TARGET_10X_NUM_VDEVS_CT			32
+#define TARGET_10X_NUM_PEERS_CT			(32 + (TARGET_10X_NUM_VDEVS_CT))
+#define TARGET_10X_AST_SKID_LIMIT_CT		(TARGET_10X_NUM_PEERS_CT * TARGET_10X_NUM_PEER_AST)
+
 #define TARGET_10X_NUM_OFFLOAD_PEERS		0
 #define TARGET_10X_NUM_OFFLOAD_REORDER_BUFS	0
 #define TARGET_10X_NUM_PEER_KEYS		2
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 33a66b9..e67137a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -602,9 +602,9 @@ static int ath10k_monitor_vdev_create(struct ath10k *ar)
 		return -ENOMEM;
 	}
 
-	bit = ffs(ar->free_vdev_map);
+	bit = __ffs64(ar->free_vdev_map);
 
-	ar->monitor_vdev_id = bit - 1;
+	ar->monitor_vdev_id = bit;
 
 	ret = ath10k_wmi_vdev_create(ar, ar->monitor_vdev_id,
 				     WMI_VDEV_TYPE_MONITOR,
@@ -615,7 +615,8 @@ static int ath10k_monitor_vdev_create(struct ath10k *ar)
 		return ret;
 	}
 
-	ar->free_vdev_map &= ~(1 << ar->monitor_vdev_id);
+	ar->free_vdev_map &= ~(1LL << ar->monitor_vdev_id);
+
 	ath10k_dbg(ATH10K_DBG_MAC, "mac monitor vdev %d created\n",
 		   ar->monitor_vdev_id);
 
@@ -635,7 +636,7 @@ static int ath10k_monitor_vdev_delete(struct ath10k *ar)
 		return ret;
 	}
 
-	ar->free_vdev_map |= 1 << ar->monitor_vdev_id;
+	ar->free_vdev_map |= 1LL << ar->monitor_vdev_id;
 
 	ath10k_dbg(ATH10K_DBG_MAC, "mac monitor vdev %d deleted\n",
 		   ar->monitor_vdev_id);
@@ -2742,9 +2743,13 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 		ret = -EBUSY;
 		goto err;
 	}
-	bit = ffs(ar->free_vdev_map);
 
-	arvif->vdev_id = bit - 1;
+	bit = __ffs64(ar->free_vdev_map);
+
+	ath10k_warn("Creating vdev id: %i  map: %llu\n",
+		    bit, ar->free_vdev_map);
+
+	arvif->vdev_id = bit;
 	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
 
 	if (ar->p2p)
@@ -2785,7 +2790,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 		goto err;
 	}
 
-	ar->free_vdev_map &= ~(1 << arvif->vdev_id);
+	ar->free_vdev_map &= ~(1LL << arvif->vdev_id);
 	list_add(&arvif->list, &ar->arvifs);
 
 	vdev_param = ar->wmi.vdev_param->def_keyid;
@@ -2878,7 +2883,7 @@ err_peer_delete:
 
 err_vdev_delete:
 	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
-	ar->free_vdev_map |= 1 << arvif->vdev_id;
+	ar->free_vdev_map |= 1LL << arvif->vdev_id;
 	list_del(&arvif->list);
 
 err:
@@ -2914,7 +2919,7 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
 		ath10k_warn("failed to stop spectral for vdev %i: %d\n",
 			    arvif->vdev_id, ret);
 
-	ar->free_vdev_map |= 1 << arvif->vdev_id;
+	ar->free_vdev_map |= 1LL << arvif->vdev_id;
 	list_del(&arvif->list);
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
@@ -3465,7 +3470,9 @@ static int ath10k_sta_state(struct ieee80211_hw *hw,
 		/*
 		 * New station addition.
 		 */
-		if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
+		if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
+			max_num_peers = TARGET_10X_NUM_PEERS_CT - 1;
+		else if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
 			max_num_peers = TARGET_10X_NUM_PEERS_MAX - 1;
 		else
 			max_num_peers = TARGET_NUM_PEERS;
@@ -4552,6 +4559,22 @@ static const struct ieee80211_iface_limit ath10k_10x_if_limits[] = {
 	},
 };
 
+static const struct ieee80211_iface_limit ath10k_10x_ct_if_limits[] = {
+	{
+	.max	= TARGET_10X_NUM_VDEVS_CT,
+	.types	= BIT(NL80211_IFTYPE_STATION)
+		| BIT(NL80211_IFTYPE_P2P_CLIENT)
+	},
+	{
+	.max	= 3,
+	.types	= BIT(NL80211_IFTYPE_P2P_GO)
+	},
+	{
+	.max	= 7,
+	.types	= BIT(NL80211_IFTYPE_AP)
+	},
+};
+
 static const struct ieee80211_iface_combination ath10k_if_comb[] = {
 	{
 		.limits = ath10k_if_limits,
@@ -4578,6 +4601,22 @@ static const struct ieee80211_iface_combination ath10k_10x_if_comb[] = {
 	},
 };
 
+static const struct ieee80211_iface_combination ath10k_10x_ct_if_comb[] = {
+	{
+		.limits = ath10k_10x_ct_if_limits,
+		.n_limits = ARRAY_SIZE(ath10k_10x_ct_if_limits),
+		.max_interfaces = TARGET_10X_NUM_VDEVS_CT,
+		.num_different_channels = 1,
+		.beacon_int_infra_match = true,
+#ifdef CONFIG_ATH10K_DFS_CERTIFIED
+		.radar_detect_widths =	BIT(NL80211_CHAN_WIDTH_20_NOHT) |
+					BIT(NL80211_CHAN_WIDTH_20) |
+					BIT(NL80211_CHAN_WIDTH_40) |
+					BIT(NL80211_CHAN_WIDTH_80),
+#endif
+	},
+};
+
 static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
 {
 	struct ieee80211_sta_vht_cap vht_cap = {0};
@@ -4814,7 +4853,11 @@ int ath10k_mac_register(struct ath10k *ar)
 	 */
 	ar->hw->queues = 4;
 
-	if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features)) {
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features)) {
+		ar->hw->wiphy->iface_combinations = ath10k_10x_ct_if_comb;
+		ar->hw->wiphy->n_iface_combinations =
+			ARRAY_SIZE(ath10k_10x_ct_if_comb);
+	} else if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features)) {
 		ar->hw->wiphy->iface_combinations = ath10k_10x_if_comb;
 		ar->hw->wiphy->n_iface_combinations =
 			ARRAY_SIZE(ath10k_10x_if_comb);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index a1037f7..0aee639 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2160,6 +2160,13 @@ static void ath10k_wmi_10x_service_ready_event_rx(struct ath10k *ar,
 	u32 num_units, req_id, unit_size, num_mem_reqs, num_unit_info, i;
 	int ret;
 	struct wmi_service_ready_event_10x *ev = (void *)skb->data;
+	int my_num_peers = TARGET_10X_NUM_PEERS;
+	int my_num_vdevs = TARGET_10X_NUM_VDEVS;
+
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features)) {
+		my_num_peers = TARGET_10X_NUM_PEERS_CT;
+		my_num_vdevs = TARGET_10X_NUM_VDEVS_CT;
+	}
 
 	if (skb->len < sizeof(*ev)) {
 		ath10k_warn("Service ready event was %d B but expected %zu B. Wrong firmware version?\n",
@@ -2222,9 +2229,9 @@ static void ath10k_wmi_10x_service_ready_event_rx(struct ath10k *ar,
 			 * peers, 1 extra for self peer on target */
 			/* this needs to be tied, host and target
 			 * can get out of sync */
-			num_units = TARGET_10X_NUM_PEERS + 1;
+			num_units = my_num_peers + 1;
 		else if (num_unit_info & NUM_UNITS_IS_NUM_VDEVS)
-			num_units = TARGET_10X_NUM_VDEVS + 1;
+			num_units = my_num_vdevs + 1;
 
 		ath10k_dbg(ATH10K_DBG_WMI,
 			   "wmi mem_req_id %d num_units %d num_unit_info %d unit size %d actual units %d\n",
@@ -2952,12 +2959,20 @@ static int ath10k_wmi_10x_cmd_init(struct ath10k *ar)
 	struct wmi_resource_config_10x config = {};
 	u32 len, val;
 	int i;
+	u32 skid_limit;
 
-	config.num_vdevs = __cpu_to_le32(TARGET_10X_NUM_VDEVS);
-	config.num_peers = __cpu_to_le32(TARGET_10X_NUM_PEERS);
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features)) {
+		config.num_vdevs = __cpu_to_le32(TARGET_10X_NUM_VDEVS_CT);
+		config.num_peers = __cpu_to_le32(TARGET_10X_NUM_PEERS_CT);
+		skid_limit = TARGET_10X_AST_SKID_LIMIT_CT;
+	} else {
+		config.num_vdevs = __cpu_to_le32(TARGET_10X_NUM_VDEVS);
+		config.num_peers = __cpu_to_le32(TARGET_10X_NUM_PEERS);
+		skid_limit = TARGET_10X_AST_SKID_LIMIT;
+	}
+	config.ast_skid_limit = __cpu_to_le32(skid_limit);
 	config.num_peer_keys = __cpu_to_le32(TARGET_10X_NUM_PEER_KEYS);
 	config.num_tids = __cpu_to_le32(TARGET_10X_NUM_TIDS);
-	config.ast_skid_limit = __cpu_to_le32(TARGET_10X_AST_SKID_LIMIT);
 	config.tx_chain_mask = __cpu_to_le32(TARGET_10X_TX_CHAIN_MASK);
 	config.rx_chain_mask = __cpu_to_le32(TARGET_10X_RX_CHAIN_MASK);
 	config.rx_timeout_pri_vo = __cpu_to_le32(TARGET_10X_RX_TIMEOUT_LO_PRI);
-- 
1.7.11.7

[PATCH] ath10k: Support 32+ stations.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Implement 64-bit vdev map to support larger numbers
of virtual devices.  Support up to 32 stations
(actual limit will be determined when loading firmware,
as different firmwares have different limits)

Enable larger limits when using Candela Technologies
firmware.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

NOTE:  The 10.2 firmware uses the bit that used to be used
by CT firmware, so existing CT firmware will not actually
work with this patch (or any kernel with the 10.2 firmware patch).
If I can get acceptance of this patch or even a simpler patch
that just adds the enum for CT firmware feature,
I can immediately re-do CT firmware with the proper bit
set.  If there is absolutely no way that even minimal
CT firmware support can be accepted upstream, then I
will just choose a large feature flag and hope it is not
re-used by upstream code any time soon.

 drivers/net/wireless/ath/ath10k/core.c |  8 +++--
 drivers/net/wireless/ath/ath10k/core.h |  5 ++-
 drivers/net/wireless/ath/ath10k/hw.h   |  6 ++++
 drivers/net/wireless/ath/ath10k/mac.c  | 65 ++++++++++++++++++++++++++++------
 drivers/net/wireless/ath/ath10k/wmi.c  | 25 ++++++++++---
 5 files changed, 89 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 27d027d..030bef1 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -844,10 +844,12 @@ int ath10k_core_start(struct ath10k *ar)
 	if (status)
 		goto err_htc_stop;
 
-	if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
-		ar->free_vdev_map = (1 << TARGET_10X_NUM_VDEVS) - 1;
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
+		ar->free_vdev_map = (1LL << TARGET_10X_NUM_VDEVS_CT) - 1;
+	else if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
+		ar->free_vdev_map = (1LL << TARGET_10X_NUM_VDEVS) - 1;
 	else
-		ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
+		ar->free_vdev_map = (1LL << TARGET_NUM_VDEVS) - 1;
 
 	INIT_LIST_HEAD(&ar->arvifs);
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 58c6348..777957e 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -425,6 +425,9 @@ enum ath10k_fw_features {
 	 */
 	ATH10K_FW_FEATURE_WMI_10_2 = 4,
 
+	/* Firmware from Candela Technologies, enables more VIFs, etc */
+	ATH10K_FW_FEATURE_WMI_10X_CT = 5,
+
 	/* keep last */
 	ATH10K_FW_FEATURE_COUNT,
 };
@@ -526,7 +529,7 @@ struct ath10k {
 	/* current operating channel definition */
 	struct cfg80211_chan_def chandef;
 
-	int free_vdev_map;
+	unsigned long long free_vdev_map;
 	bool promisc;
 	bool monitor;
 	int monitor_vdev_id;
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 291ad23..0bb3744 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -122,6 +122,12 @@ enum ath10k_mcast2ucast_mode {
 #define TARGET_10X_AST_SKID_LIMIT		16
 #define TARGET_10X_NUM_PEERS			(128 + (TARGET_10X_NUM_VDEVS))
 #define TARGET_10X_NUM_PEERS_MAX		128
+
+/* Over-rides for Candela Technologies firmware */
+#define TARGET_10X_NUM_VDEVS_CT			32
+#define TARGET_10X_NUM_PEERS_CT			(32 + (TARGET_10X_NUM_VDEVS_CT))
+#define TARGET_10X_AST_SKID_LIMIT_CT		(TARGET_10X_NUM_PEERS_CT * TARGET_10X_NUM_PEER_AST)
+
 #define TARGET_10X_NUM_OFFLOAD_PEERS		0
 #define TARGET_10X_NUM_OFFLOAD_REORDER_BUFS	0
 #define TARGET_10X_NUM_PEER_KEYS		2
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 33a66b9..e67137a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -602,9 +602,9 @@ static int ath10k_monitor_vdev_create(struct ath10k *ar)
 		return -ENOMEM;
 	}
 
-	bit = ffs(ar->free_vdev_map);
+	bit = __ffs64(ar->free_vdev_map);
 
-	ar->monitor_vdev_id = bit - 1;
+	ar->monitor_vdev_id = bit;
 
 	ret = ath10k_wmi_vdev_create(ar, ar->monitor_vdev_id,
 				     WMI_VDEV_TYPE_MONITOR,
@@ -615,7 +615,8 @@ static int ath10k_monitor_vdev_create(struct ath10k *ar)
 		return ret;
 	}
 
-	ar->free_vdev_map &= ~(1 << ar->monitor_vdev_id);
+	ar->free_vdev_map &= ~(1LL << ar->monitor_vdev_id);
+
 	ath10k_dbg(ATH10K_DBG_MAC, "mac monitor vdev %d created\n",
 		   ar->monitor_vdev_id);
 
@@ -635,7 +636,7 @@ static int ath10k_monitor_vdev_delete(struct ath10k *ar)
 		return ret;
 	}
 
-	ar->free_vdev_map |= 1 << ar->monitor_vdev_id;
+	ar->free_vdev_map |= 1LL << ar->monitor_vdev_id;
 
 	ath10k_dbg(ATH10K_DBG_MAC, "mac monitor vdev %d deleted\n",
 		   ar->monitor_vdev_id);
@@ -2742,9 +2743,13 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 		ret = -EBUSY;
 		goto err;
 	}
-	bit = ffs(ar->free_vdev_map);
 
-	arvif->vdev_id = bit - 1;
+	bit = __ffs64(ar->free_vdev_map);
+
+	ath10k_warn("Creating vdev id: %i  map: %llu\n",
+		    bit, ar->free_vdev_map);
+
+	arvif->vdev_id = bit;
 	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
 
 	if (ar->p2p)
@@ -2785,7 +2790,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 		goto err;
 	}
 
-	ar->free_vdev_map &= ~(1 << arvif->vdev_id);
+	ar->free_vdev_map &= ~(1LL << arvif->vdev_id);
 	list_add(&arvif->list, &ar->arvifs);
 
 	vdev_param = ar->wmi.vdev_param->def_keyid;
@@ -2878,7 +2883,7 @@ err_peer_delete:
 
 err_vdev_delete:
 	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
-	ar->free_vdev_map |= 1 << arvif->vdev_id;
+	ar->free_vdev_map |= 1LL << arvif->vdev_id;
 	list_del(&arvif->list);
 
 err:
@@ -2914,7 +2919,7 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
 		ath10k_warn("failed to stop spectral for vdev %i: %d\n",
 			    arvif->vdev_id, ret);
 
-	ar->free_vdev_map |= 1 << arvif->vdev_id;
+	ar->free_vdev_map |= 1LL << arvif->vdev_id;
 	list_del(&arvif->list);
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
@@ -3465,7 +3470,9 @@ static int ath10k_sta_state(struct ieee80211_hw *hw,
 		/*
 		 * New station addition.
 		 */
-		if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
+		if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
+			max_num_peers = TARGET_10X_NUM_PEERS_CT - 1;
+		else if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
 			max_num_peers = TARGET_10X_NUM_PEERS_MAX - 1;
 		else
 			max_num_peers = TARGET_NUM_PEERS;
@@ -4552,6 +4559,22 @@ static const struct ieee80211_iface_limit ath10k_10x_if_limits[] = {
 	},
 };
 
+static const struct ieee80211_iface_limit ath10k_10x_ct_if_limits[] = {
+	{
+	.max	= TARGET_10X_NUM_VDEVS_CT,
+	.types	= BIT(NL80211_IFTYPE_STATION)
+		| BIT(NL80211_IFTYPE_P2P_CLIENT)
+	},
+	{
+	.max	= 3,
+	.types	= BIT(NL80211_IFTYPE_P2P_GO)
+	},
+	{
+	.max	= 7,
+	.types	= BIT(NL80211_IFTYPE_AP)
+	},
+};
+
 static const struct ieee80211_iface_combination ath10k_if_comb[] = {
 	{
 		.limits = ath10k_if_limits,
@@ -4578,6 +4601,22 @@ static const struct ieee80211_iface_combination ath10k_10x_if_comb[] = {
 	},
 };
 
+static const struct ieee80211_iface_combination ath10k_10x_ct_if_comb[] = {
+	{
+		.limits = ath10k_10x_ct_if_limits,
+		.n_limits = ARRAY_SIZE(ath10k_10x_ct_if_limits),
+		.max_interfaces = TARGET_10X_NUM_VDEVS_CT,
+		.num_different_channels = 1,
+		.beacon_int_infra_match = true,
+#ifdef CONFIG_ATH10K_DFS_CERTIFIED
+		.radar_detect_widths =	BIT(NL80211_CHAN_WIDTH_20_NOHT) |
+					BIT(NL80211_CHAN_WIDTH_20) |
+					BIT(NL80211_CHAN_WIDTH_40) |
+					BIT(NL80211_CHAN_WIDTH_80),
+#endif
+	},
+};
+
 static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
 {
 	struct ieee80211_sta_vht_cap vht_cap = {0};
@@ -4814,7 +4853,11 @@ int ath10k_mac_register(struct ath10k *ar)
 	 */
 	ar->hw->queues = 4;
 
-	if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features)) {
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features)) {
+		ar->hw->wiphy->iface_combinations = ath10k_10x_ct_if_comb;
+		ar->hw->wiphy->n_iface_combinations =
+			ARRAY_SIZE(ath10k_10x_ct_if_comb);
+	} else if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features)) {
 		ar->hw->wiphy->iface_combinations = ath10k_10x_if_comb;
 		ar->hw->wiphy->n_iface_combinations =
 			ARRAY_SIZE(ath10k_10x_if_comb);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index a1037f7..0aee639 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2160,6 +2160,13 @@ static void ath10k_wmi_10x_service_ready_event_rx(struct ath10k *ar,
 	u32 num_units, req_id, unit_size, num_mem_reqs, num_unit_info, i;
 	int ret;
 	struct wmi_service_ready_event_10x *ev = (void *)skb->data;
+	int my_num_peers = TARGET_10X_NUM_PEERS;
+	int my_num_vdevs = TARGET_10X_NUM_VDEVS;
+
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features)) {
+		my_num_peers = TARGET_10X_NUM_PEERS_CT;
+		my_num_vdevs = TARGET_10X_NUM_VDEVS_CT;
+	}
 
 	if (skb->len < sizeof(*ev)) {
 		ath10k_warn("Service ready event was %d B but expected %zu B. Wrong firmware version?\n",
@@ -2222,9 +2229,9 @@ static void ath10k_wmi_10x_service_ready_event_rx(struct ath10k *ar,
 			 * peers, 1 extra for self peer on target */
 			/* this needs to be tied, host and target
 			 * can get out of sync */
-			num_units = TARGET_10X_NUM_PEERS + 1;
+			num_units = my_num_peers + 1;
 		else if (num_unit_info & NUM_UNITS_IS_NUM_VDEVS)
-			num_units = TARGET_10X_NUM_VDEVS + 1;
+			num_units = my_num_vdevs + 1;
 
 		ath10k_dbg(ATH10K_DBG_WMI,
 			   "wmi mem_req_id %d num_units %d num_unit_info %d unit size %d actual units %d\n",
@@ -2952,12 +2959,20 @@ static int ath10k_wmi_10x_cmd_init(struct ath10k *ar)
 	struct wmi_resource_config_10x config = {};
 	u32 len, val;
 	int i;
+	u32 skid_limit;
 
-	config.num_vdevs = __cpu_to_le32(TARGET_10X_NUM_VDEVS);
-	config.num_peers = __cpu_to_le32(TARGET_10X_NUM_PEERS);
+	if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features)) {
+		config.num_vdevs = __cpu_to_le32(TARGET_10X_NUM_VDEVS_CT);
+		config.num_peers = __cpu_to_le32(TARGET_10X_NUM_PEERS_CT);
+		skid_limit = TARGET_10X_AST_SKID_LIMIT_CT;
+	} else {
+		config.num_vdevs = __cpu_to_le32(TARGET_10X_NUM_VDEVS);
+		config.num_peers = __cpu_to_le32(TARGET_10X_NUM_PEERS);
+		skid_limit = TARGET_10X_AST_SKID_LIMIT;
+	}
+	config.ast_skid_limit = __cpu_to_le32(skid_limit);
 	config.num_peer_keys = __cpu_to_le32(TARGET_10X_NUM_PEER_KEYS);
 	config.num_tids = __cpu_to_le32(TARGET_10X_NUM_TIDS);
-	config.ast_skid_limit = __cpu_to_le32(TARGET_10X_AST_SKID_LIMIT);
 	config.tx_chain_mask = __cpu_to_le32(TARGET_10X_TX_CHAIN_MASK);
 	config.rx_chain_mask = __cpu_to_le32(TARGET_10X_RX_CHAIN_MASK);
 	config.rx_timeout_pri_vo = __cpu_to_le32(TARGET_10X_RX_TIMEOUT_LO_PRI);
-- 
1.7.11.7


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

WMI version handling

[Kalle Valo]

Hi,

This was "[PATCH] ath10k: Support 32+ stations." but I'll change the
subject as this a bigger discussion. I'll also drop linux-wireless for
now, I assume they are not interested about our discussion how to manage
firmware interfaces.

greearb@candelatech.com writes:

> From: Ben Greear <greearb@candelatech.com>
>
> Implement 64-bit vdev map to support larger numbers
> of virtual devices.  Support up to 32 stations
> (actual limit will be determined when loading firmware,
> as different firmwares have different limits)
>
> Enable larger limits when using Candela Technologies
> firmware.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>

So what this firmware does is that it changes these values:

> +/* Over-rides for Candela Technologies firmware */
> +#define TARGET_10X_NUM_VDEVS_CT			32
> +#define TARGET_10X_NUM_PEERS_CT			(32 + (TARGET_10X_NUM_VDEVS_CT))
> +#define TARGET_10X_AST_SKID_LIMIT_CT		(TARGET_10X_NUM_PEERS_CT * TARGET_10X_NUM_PEER_AST)

I think it would be better to provide these through FW IEs, one u32 for
each. I guess we need one also for the skid limit? That way it's easier
to maintain this in ath10k.

Of course that would mean we have to create struct ieee80211_iface_limit
and struct ieee80211_iface_combination runtime, instead of statically
how it's done now. But shouldn't be a problem, right?

Also I have been thinking that using firmware feature bits (for example
ATH10K_FW_FEATURE_WMI_10_2 and ATH10K_FW_FEATURE_WMI_10X) for WMI
version is not the best way. I think it's easier to manage all this if
we have a u32 FW IE to provide WMI version. IIRC Ben was suggesting this
at some point.

Example:

enum ath10k_fw_wmi_version {
	ATH10K_FW_WMI_VERSION_MAIN = 0,
	ATH10K_FW_WMI_VERSION_10_1 = 1,
	ATH10K_FW_WMI_VERSION_10_2 = 2,
}

And then wmi.c would set correct interface based on this version.

We would still use feature bits to enable and disable smaller changes
like ATH10K_FW_FEATURE_HAS_WMI_MGMT_TX does. But for bigger WMI changes
we would change the WMI version, for example if we have a new firmware
branch with significant changes or similar.

And for backwards comptability we need to do so that
ATH10K_FW_FEATURE_WMI_10X sets ATH10K_FW_WMI_VERSION_10_1 and
ATH10K_FW_FEATURE_WMI_10_2 sets ATH10K_FW_WMI_VERSION_10_2.

Thoughts?

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: WMI version handling

[Ben Greear]

On 09/02/2014 11:14 PM, Kalle Valo wrote:
> Hi,
> 
> This was "[PATCH] ath10k: Support 32+ stations." but I'll change the
> subject as this a bigger discussion. I'll also drop linux-wireless for
> now, I assume they are not interested about our discussion how to manage
> firmware interfaces.
> 
> greearb@candelatech.com writes:
> 
>> From: Ben Greear <greearb@candelatech.com>
>>
>> Implement 64-bit vdev map to support larger numbers
>> of virtual devices.  Support up to 32 stations
>> (actual limit will be determined when loading firmware,
>> as different firmwares have different limits)
>>
>> Enable larger limits when using Candela Technologies
>> firmware.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
> 
> So what this firmware does is that it changes these values:
> 
>> +/* Over-rides for Candela Technologies firmware */
>> +#define TARGET_10X_NUM_VDEVS_CT			32
>> +#define TARGET_10X_NUM_PEERS_CT			(32 + (TARGET_10X_NUM_VDEVS_CT))
>> +#define TARGET_10X_AST_SKID_LIMIT_CT		(TARGET_10X_NUM_PEERS_CT * TARGET_10X_NUM_PEER_AST)

My firmware really makes better use of resources and is able to deal better with
a wider range of configured values.

For a full list of ath10k patches that I hope to someday submit, see:

http://dmz2.candelatech.com/git/gitweb.cgi?p=linux.ath/.git;a=summary

Towards the end, I have patches to configure number of stations with
kernel module variable.  I think at a minimum, should be able to configure
peer count as well.

> I think it would be better to provide these through FW IEs, one u32 for
> each. I guess we need one also for the skid limit? That way it's easier
> to maintain this in ath10k.

Skid limit should depend on the others.  If you get unlucky with hashes,
or an attacker figures out how to do so artificially, then you can crash
upstream ath10k firmware by associating more than skid-limit stations.

> Of course that would mean we have to create struct ieee80211_iface_limit
> and struct ieee80211_iface_combination runtime, instead of statically
> how it's done now. But shouldn't be a problem, right?

Yes, I do that later in my patch series.  It works, my patches as written
may need some changes as I had to remove some const markings that might not
be acceptable upstream.

> Also I have been thinking that using firmware feature bits (for example
> ATH10K_FW_FEATURE_WMI_10_2 and ATH10K_FW_FEATURE_WMI_10X) for WMI
> version is not the best way. I think it's easier to manage all this if
> we have a u32 FW IE to provide WMI version. IIRC Ben was suggesting this
> at some point.
> 
> Example:
> 
> enum ath10k_fw_wmi_version {
> 	ATH10K_FW_WMI_VERSION_MAIN = 0,
> 	ATH10K_FW_WMI_VERSION_10_1 = 1,
> 	ATH10K_FW_WMI_VERSION_10_2 = 2,
> }
> 
> And then wmi.c would set correct interface based on this version.

I am happy with the current flags, it seems to work well enough.
I'd leave policy out of the IEs as much as possible, as normal users
cannot modify it, and certainly it would be a problem if you wanted
different policy with different NICs in same system.
IEs should describe the minimum needed for the
ath10k to enforce policy as best as it can.  We have to assume that
users tweaking station and peer count can deal with fw crashes until
they get the settings right.

My CT firmware will print out available RAM once booted, so it is
fairly easy to utilize maximum amount of resources with a bit of
trial and error.  It would be trivial to make upstream firmware do
the same, of course, but upstream firmware may crash for other
reasons as you change resource config...

> We would still use feature bits to enable and disable smaller changes
> like ATH10K_FW_FEATURE_HAS_WMI_MGMT_TX does. But for bigger WMI changes
> we would change the WMI version, for example if we have a new firmware
> branch with significant changes or similar.
> 
> And for backwards comptability we need to do so that
> ATH10K_FW_FEATURE_WMI_10X sets ATH10K_FW_WMI_VERSION_10_1 and
> ATH10K_FW_FEATURE_WMI_10_2 sets ATH10K_FW_WMI_VERSION_10_2.

I was able to do all of my changes w/out needing new a new WMI version,
and I think that is the right approach.  I do not like large amounts of
duplicated code and table lookups, which is what new WMI versions appear
to imply.

I'd prefer to keep IEs generally as they are now (in my firmware,
at least), and allow changing number of peers, stations, etc through debugfs
and/or kernel modules.  Just restart the firmware when users have completed
the config changes.  Don't assume ath10k can properly limit all configurables
to keep firmware from crashing, but give users enough info (ie, RAM usage),
that they can deal with it themselves.  Normal users will never need to know
about or change the configurables anyway, but those building APs can tweak
as needed.

If QCA goes to more of a rolling release for firmware, like I do for CT
firmware, then we may want to add IEs a bit more often, to notify things like
"can-support-feature-foo".  10.2 v/s 10.1 can still determine the
over-all WMI api, perhaps, but at specific points we can take advantage of
specific firmware features based on IE flag.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: WMI version handling

[Michal Kazior]

On 3 September 2014 17:00, Ben Greear <greearb@candelatech.com> wrote:
> On 09/02/2014 11:14 PM, Kalle Valo wrote:
>> Hi,
[...]
>> Also I have been thinking that using firmware feature bits (for example
>> ATH10K_FW_FEATURE_WMI_10_2 and ATH10K_FW_FEATURE_WMI_10X) for WMI
>> version is not the best way. I think it's easier to manage all this if
>> we have a u32 FW IE to provide WMI version. IIRC Ben was suggesting this
>> at some point.
>>
>> Example:
>>
>> enum ath10k_fw_wmi_version {
>>       ATH10K_FW_WMI_VERSION_MAIN = 0,
>>       ATH10K_FW_WMI_VERSION_10_1 = 1,
>>       ATH10K_FW_WMI_VERSION_10_2 = 2,
>> }
>>
>> And then wmi.c would set correct interface based on this version.
>
> I am happy with the current flags, it seems to work well enough.

I'm actually okay with this but I'd actually throw out the "wmi"
string from it and leave out just "ath10k_fw_version". I've recently
discovered some minor HTT discrepancies between fw branches so we
might want to use the version tag to pick HTT "backend" as well..


> I'd leave policy out of the IEs as much as possible, as normal users
> cannot modify it, and certainly it would be a problem if you wanted
> different policy with different NICs in same system.
> IEs should describe the minimum needed for the
> ath10k to enforce policy as best as it can.  We have to assume that
> users tweaking station and peer count can deal with fw crashes until
> they get the settings right.
>
> My CT firmware will print out available RAM once booted, so it is
> fairly easy to utilize maximum amount of resources with a bit of
> trial and error.  It would be trivial to make upstream firmware do
> the same, of course, but upstream firmware may crash for other
> reasons as you change resource config...

IOW You're okay if ath10k applies default/hardcoded configuration
values of 10.1 for your firmware (as your firmware is going to
advertise as a 10.1) - is that correct? You just want to be able to
tune these values per-device in runtime. I think debugfs is the best
candidate here then.

ath10k would need to have a separate debugfs entry for that then, i.e.
one that is independent from wiphy. You could have, e.g.
/sys/kernel/debug/ath10k/pci/0000:04:00.0/num_peers which re-registers
to mac80211 upon modification.


>> We would still use feature bits to enable and disable smaller changes
>> like ATH10K_FW_FEATURE_HAS_WMI_MGMT_TX does. But for bigger WMI changes
>> we would change the WMI version, for example if we have a new firmware
>> branch with significant changes or similar.
>>
>> And for backwards comptability we need to do so that
>> ATH10K_FW_FEATURE_WMI_10X sets ATH10K_FW_WMI_VERSION_10_1 and
>> ATH10K_FW_FEATURE_WMI_10_2 sets ATH10K_FW_WMI_VERSION_10_2.
>
> I was able to do all of my changes w/out needing new a new WMI version,
> and I think that is the right approach.  I do not like large amounts of
> duplicated code and table lookups, which is what new WMI versions appear
> to imply.

It's actually really simple to update WMI in an ABI-safe way.
Unfortunately official firmware updates break it fairly often and we
have to deal with that. Aaand there's going to be another WMI backend
soon most likely.


[...]
> If QCA goes to more of a rolling release for firmware, like I do for CT
> firmware, then we may want to add IEs a bit more often, to notify things like
> "can-support-feature-foo".  10.2 v/s 10.1 can still determine the
> over-all WMI api, perhaps, but at specific points we can take advantage of
> specific firmware features based on IE flag.

This is what WMI bitmap service is actually for. ath10k is ignoring
this now and exposes it via debugfs only. It's possible to, e.g. pick
"max num of stations" configuration values based on the "iram tids"
being set or not.


Michał

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: WMI version handling

[Ben Greear]

On 09/03/2014 11:56 PM, Michal Kazior wrote:
> On 3 September 2014 17:00, Ben Greear <greearb@candelatech.com> wrote:
>> On 09/02/2014 11:14 PM, Kalle Valo wrote:
>>> Hi,
> [...]
>>> Also I have been thinking that using firmware feature bits (for example
>>> ATH10K_FW_FEATURE_WMI_10_2 and ATH10K_FW_FEATURE_WMI_10X) for WMI
>>> version is not the best way. I think it's easier to manage all this if
>>> we have a u32 FW IE to provide WMI version. IIRC Ben was suggesting this
>>> at some point.
>>>
>>> Example:
>>>
>>> enum ath10k_fw_wmi_version {
>>>        ATH10K_FW_WMI_VERSION_MAIN = 0,
>>>        ATH10K_FW_WMI_VERSION_10_1 = 1,
>>>        ATH10K_FW_WMI_VERSION_10_2 = 2,
>>> }
>>>
>>> And then wmi.c would set correct interface based on this version.
>>
>> I am happy with the current flags, it seems to work well enough.
>
> I'm actually okay with this but I'd actually throw out the "wmi"
> string from it and leave out just "ath10k_fw_version". I've recently
> discovered some minor HTT discrepancies between fw branches so we
> might want to use the version tag to pick HTT "backend" as well..
>
>
>> I'd leave policy out of the IEs as much as possible, as normal users
>> cannot modify it, and certainly it would be a problem if you wanted
>> different policy with different NICs in same system.
>> IEs should describe the minimum needed for the
>> ath10k to enforce policy as best as it can.  We have to assume that
>> users tweaking station and peer count can deal with fw crashes until
>> they get the settings right.
>>
>> My CT firmware will print out available RAM once booted, so it is
>> fairly easy to utilize maximum amount of resources with a bit of
>> trial and error.  It would be trivial to make upstream firmware do
>> the same, of course, but upstream firmware may crash for other
>> reasons as you change resource config...
>
> IOW You're okay if ath10k applies default/hardcoded configuration
> values of 10.1 for your firmware (as your firmware is going to
> advertise as a 10.1) - is that correct? You just want to be able to
> tune these values per-device in runtime. I think debugfs is the best
> candidate here then.
>
> ath10k would need to have a separate debugfs entry for that then, i.e.
> one that is independent from wiphy. You could have, e.g.
> /sys/kernel/debug/ath10k/pci/0000:04:00.0/num_peers which re-registers
> to mac80211 upon modification.

That would be fine for the number of stations and peers, but there are other
features more specific to my firmware that should be enabled by default
if my firmware is loaded.  These include better tx-status, for instance.
I figured might as well also auto-tune the stations higher at the same time.

Also, some of the logic that auto-calculates skid-len, for instance, really
should be done for all firmware that can handle it to protect against malicious
hashing attacks.


>>> We would still use feature bits to enable and disable smaller changes
>>> like ATH10K_FW_FEATURE_HAS_WMI_MGMT_TX does. But for bigger WMI changes
>>> we would change the WMI version, for example if we have a new firmware
>>> branch with significant changes or similar.
>>>
>>> And for backwards comptability we need to do so that
>>> ATH10K_FW_FEATURE_WMI_10X sets ATH10K_FW_WMI_VERSION_10_1 and
>>> ATH10K_FW_FEATURE_WMI_10_2 sets ATH10K_FW_WMI_VERSION_10_2.
>>
>> I was able to do all of my changes w/out needing new a new WMI version,
>> and I think that is the right approach.  I do not like large amounts of
>> duplicated code and table lookups, which is what new WMI versions appear
>> to imply.
>
> It's actually really simple to update WMI in an ABI-safe way.
> Unfortunately official firmware updates break it fairly often and we
> have to deal with that. Aaand there's going to be another WMI backend
> soon most likely.
>
>
> [...]
>> If QCA goes to more of a rolling release for firmware, like I do for CT
>> firmware, then we may want to add IEs a bit more often, to notify things like
>> "can-support-feature-foo".  10.2 v/s 10.1 can still determine the
>> over-all WMI api, perhaps, but at specific points we can take advantage of
>> specific firmware features based on IE flag.
>
> This is what WMI bitmap service is actually for. ath10k is ignoring
> this now and exposes it via debugfs only. It's possible to, e.g. pick
> "max num of stations" configuration values based on the "iram tids"
> being set or not.

I guess I haven't looked into WMI bitmap, but I was hoping for minimal
code changes when adding my firmware, and I want to keep my firmware
backwards compat with existing kernels.  With regard to max-num-of-stations,
I don't think you will will be able to reliably calculate the exact resource
limits combinations from ath10k driver.
Every configurable (peers, tx-buffers, stations, skid-limit, etc)
affects RAM and/or IRAM and there are various
upper limits on various items in the firmware that may only be hit at
unlucky times during runtime.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: WMI version handling

[Kalle Valo]

Ben Greear <greearb@candelatech.com> writes:

>> Also I have been thinking that using firmware feature bits (for example
>> ATH10K_FW_FEATURE_WMI_10_2 and ATH10K_FW_FEATURE_WMI_10X) for WMI
>> version is not the best way. I think it's easier to manage all this if
>> we have a u32 FW IE to provide WMI version. IIRC Ben was suggesting this
>> at some point.
>> 
>> Example:
>> 
>> enum ath10k_fw_wmi_version {
>> 	ATH10K_FW_WMI_VERSION_MAIN = 0,
>> 	ATH10K_FW_WMI_VERSION_10_1 = 1,
>> 	ATH10K_FW_WMI_VERSION_10_2 = 2,
>> }
>> 
>> And then wmi.c would set correct interface based on this version.
>
> I am happy with the current flags, it seems to work well enough.

Yeah, there's no problem at the moment. But if we start adding more
interfaces handling them through feature flags will become more
difficult. That's why I think adding a separate enum will be better in
the long run.

> I'd leave policy out of the IEs as much as possible, as normal users
> cannot modify it, and certainly it would be a problem if you wanted
> different policy with different NICs in same system. IEs should
> describe the minimum needed for the ath10k to enforce policy as best
> as it can.

The firmware IEs are not about policy. They are about advertising the
capabilities of the firmware image so that ath10k can know how to
configure the firmare.

> We have to assume that users tweaking station and peer count can deal
> with fw crashes until they get the settings right.

No way. In no circumstances ath10k should configure firmware out of
bounds. If we do that, it's a bug in ath10k.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: WMI version handling

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 3 September 2014 17:00, Ben Greear <greearb@candelatech.com> wrote:
>> On 09/02/2014 11:14 PM, Kalle Valo wrote:
>>> Hi,
> [...]
>>> Also I have been thinking that using firmware feature bits (for example
>>> ATH10K_FW_FEATURE_WMI_10_2 and ATH10K_FW_FEATURE_WMI_10X) for WMI
>>> version is not the best way. I think it's easier to manage all this if
>>> we have a u32 FW IE to provide WMI version. IIRC Ben was suggesting this
>>> at some point.
>>>
>>> Example:
>>>
>>> enum ath10k_fw_wmi_version {
>>>       ATH10K_FW_WMI_VERSION_MAIN = 0,
>>>       ATH10K_FW_WMI_VERSION_10_1 = 1,
>>>       ATH10K_FW_WMI_VERSION_10_2 = 2,
>>> }
>>>
>>> And then wmi.c would set correct interface based on this version.
>>
>> I am happy with the current flags, it seems to work well enough.
>
> I'm actually okay with this but I'd actually throw out the "wmi"
> string from it and leave out just "ath10k_fw_version". I've recently
> discovered some minor HTT discrepancies between fw branches so we
> might want to use the version tag to pick HTT "backend" as well..

I'm again thinking that we should have a separate enum for the HTT
version. So that the firmware interface is defined with WMI version, HTT
version and feature flags (which define smaller changes in the
interface).

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: WMI version handling

[Ben Greear]

On 09/05/2014 06:23 AM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
>
>>> Also I have been thinking that using firmware feature bits (for example
>>> ATH10K_FW_FEATURE_WMI_10_2 and ATH10K_FW_FEATURE_WMI_10X) for WMI
>>> version is not the best way. I think it's easier to manage all this if
>>> we have a u32 FW IE to provide WMI version. IIRC Ben was suggesting this
>>> at some point.
>>>
>>> Example:
>>>
>>> enum ath10k_fw_wmi_version {
>>> 	ATH10K_FW_WMI_VERSION_MAIN = 0,
>>> 	ATH10K_FW_WMI_VERSION_10_1 = 1,
>>> 	ATH10K_FW_WMI_VERSION_10_2 = 2,
>>> }
>>>
>>> And then wmi.c would set correct interface based on this version.
>>
>> I am happy with the current flags, it seems to work well enough.
>
> Yeah, there's no problem at the moment. But if we start adding more
> interfaces handling them through feature flags will become more
> difficult. That's why I think adding a separate enum will be better in
> the long run.

Worry about the long run when it gets here.  Prematurely engineering for
possible scenarios just wastes time.  We can always go back and
reorganize code later as the re-use patterns become more clear.

>> I'd leave policy out of the IEs as much as possible, as normal users
>> cannot modify it, and certainly it would be a problem if you wanted
>> different policy with different NICs in same system. IEs should
>> describe the minimum needed for the ath10k to enforce policy as best
>> as it can.
>
> The firmware IEs are not about policy. They are about advertising the
> capabilities of the firmware image so that ath10k can know how to
> configure the firmare.

More stations means less peers, less tx-buffers means more of other things.
Disabling some firmware offload features frees ram for other things.
RAM usage per item increment is not even linear in some cases, and you
would have to have intimate knowledge of the firmware to understand the
limits.

Firmware can only tell you the maximum stations it can support, but if
you configure too many peers or too many tx-buffers, or enable roaming
offload, you would have to decrease the number of stations actually allocated.

Some users may want lots of stations, others lots of peers, other less tx-buffers,
and others more.  Some might want to ensure skid-length handles worst-case
scenarios at cost of fewer peers or stations.

>> We have to assume that users tweaking station and peer count can deal
>> with fw crashes until they get the settings right.
>
> No way. In no circumstances ath10k should configure firmware out of
> bounds. If we do that, it's a bug in ath10k.

You are either going to have to fix firmware, cripple your driver, or
accept some risk and let users configure settings that work best for them
if they care to.  Make sure the driver defaults work and do not crash,
but don't be so conservative that you decrease usability for those that
actually care to tinker.

If you are not swayed by my arguments, then just go ahead and implement
it how you want and I will attempt to make my patches work with your
methods.  I am frustrated with endless discussions and no upstream
patches.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k