2014-09-19 18:05:00

by Ben Greear

[permalink] [raw]
Subject: [PATCH 1/3] ath10k: use 64-bit vdev map.

From: Ben Greear <[email protected]>

This can allow more than 32 stations to be supported
without over-running the bitmap.

Signed-off-by: Ben Greear <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 4 ++--
drivers/net/wireless/ath/ath10k/core.h | 2 +-
drivers/net/wireless/ath/ath10k/mac.c | 21 ++++++++++++---------
3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index cee18c8..37e3166 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -846,9 +846,9 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode)
goto err_hif_stop;

if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
- ar->free_vdev_map = (1 << TARGET_10X_NUM_VDEVS) - 1;
+ 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 fe531ea..20c5e40 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -482,7 +482,7 @@ struct ath10k {
/* current operating channel definition */
struct cfg80211_chan_def chandef;

- int free_vdev_map;
+ unsigned long long free_vdev_map;
bool monitor;
int monitor_vdev_id;
bool monitor_started;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 4670930..e1ddac4 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -590,9 +590,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,
@@ -603,7 +603,7 @@ 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(ar, ATH10K_DBG_MAC, "mac monitor vdev %d created\n",
ar->monitor_vdev_id);

@@ -623,7 +623,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(ar, ATH10K_DBG_MAC, "mac monitor vdev %d deleted\n",
ar->monitor_vdev_id);
@@ -2772,9 +2772,12 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
ret = -EBUSY;
goto err;
}
- bit = ffs(ar->free_vdev_map);
+ bit = __ffs64(ar->free_vdev_map);

- arvif->vdev_id = bit - 1;
+ ath10k_warn(ar, "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)
@@ -2815,7 +2818,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;
@@ -2908,7 +2911,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:
@@ -2944,7 +2947,7 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
ath10k_warn(ar, "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) {
--
1.7.11.7



2014-09-19 18:05:01

by Ben Greear

[permalink] [raw]
Subject: [PATCH 2/3] ath10k: support CT firmware flag.

From: Ben Greear <[email protected]>

Add placeholder so CT firmware can more easily co-exist with upstream
kernel.

Signed-off-by: Ben Greear <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 20c5e40..3f14858 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -361,6 +361,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,
};
--
1.7.11.7


2014-09-23 12:54:55

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath10k: use 64-bit vdev map.

[email protected] writes:

> From: Ben Greear <[email protected]>
>
> This can allow more than 32 stations to be supported
> without over-running the bitmap.
>
> Signed-off-by: Ben Greear <[email protected]>

[...]

> - ar->monitor_vdev_id = bit - 1;
> + ar->monitor_vdev_id = bit;

[...]

> - arvif->vdev_id = bit - 1;
> + ath10k_warn(ar, "Creating vdev id: %i map: %llu\n",
> + bit, ar->free_vdev_map);
> +
> + arvif->vdev_id = bit;

Why remove the "- 1"? Are you sure that's not going to break any
assumptions somewhere?

--
Kalle Valo

2014-09-23 14:22:00

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath10k: use 64-bit vdev map.



On 09/23/2014 05:54 AM, Kalle Valo wrote:
> [email protected] writes:
>
>> From: Ben Greear <[email protected]>
>>
>> This can allow more than 32 stations to be supported
>> without over-running the bitmap.
>>
>> Signed-off-by: Ben Greear <[email protected]>
>
> [...]
>
>> - ar->monitor_vdev_id = bit - 1;
>> + ar->monitor_vdev_id = bit;
>
> [...]
>
>> - arvif->vdev_id = bit - 1;
>> + ath10k_warn(ar, "Creating vdev id: %i map: %llu\n",
>> + bit, ar->free_vdev_map);
>> +
>> + arvif->vdev_id = bit;
>
> Why remove the "- 1"? Are you sure that's not going to break any
> assumptions somewhere?

I have been testing this patch in one form or another for almost a year
and have not noticed any problems with it.

The return value is subtly different when using the 64-bit version,
so you don't need the -1.

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2014-09-23 09:04:22

by Michal Kazior

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath10k: use 64-bit vdev map.

On 19 September 2014 20:04, <[email protected]> wrote:
> From: Ben Greear <[email protected]>
>
> This can allow more than 32 stations to be supported
> without over-running the bitmap.
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/core.c | 4 ++--
> drivers/net/wireless/ath/ath10k/core.h | 2 +-
> drivers/net/wireless/ath/ath10k/mac.c | 21 ++++++++++++---------
> 3 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index cee18c8..37e3166 100644
[...]
> @@ -2772,9 +2772,12 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
> ret = -EBUSY;
> goto err;
> }
> - bit = ffs(ar->free_vdev_map);
> + bit = __ffs64(ar->free_vdev_map);
>
> - arvif->vdev_id = bit - 1;
> + ath10k_warn(ar, "Creating vdev id: %i map: %llu\n",
> + bit, ar->free_vdev_map);

Shouldn't this be a ath10k_dbg()? It probably makes sense to print the
map as hex instead of a decimal too. Prints should be lower case and
debug needs a prefix, i.e.

"mac create vdev %i map %llx"


MichaƂ

2014-09-19 18:05:03

by Ben Greear

[permalink] [raw]
Subject: [PATCH 3/3] ath10k: support 32+ stations.

From: Ben Greear <[email protected]>

Support up to 32 stations when using CT firmware.

Signed-off-by: Ben Greear <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 4 +++-
drivers/net/wireless/ath/ath10k/hw.h | 6 +++++
drivers/net/wireless/ath/ath10k/mac.c | 42 ++++++++++++++++++++++++++++++++--
drivers/net/wireless/ath/ath10k/wmi.c | 25 ++++++++++++++++----
4 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 37e3166..507f31a 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -845,7 +845,9 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode)
if (status)
goto err_hif_stop;

- if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
+ 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 = (1LL << TARGET_NUM_VDEVS) - 1;
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 3cf5702..9583555 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 e1ddac4..b517f71 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3497,7 +3497,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;
@@ -4601,6 +4603,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,
@@ -4627,6 +4645,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};
@@ -4863,7 +4897,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 2c42bd5..76abdd7 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2245,6 +2245,13 @@ static void ath10k_wmi_10x_service_ready_event_rx(struct ath10k *ar,
int ret;
struct wmi_service_ready_event_10x *ev = (void *)skb->data;
DECLARE_BITMAP(svc_bmap, WMI_SERVICE_MAX) = {};
+ 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(ar, "Service ready event was %d B but expected %zu B. Wrong firmware version?\n",
@@ -2309,9 +2316,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(ar, ATH10K_DBG_WMI,
"wmi mem_req_id %d num_units %d num_unit_info %d unit size %d actual units %d\n",
@@ -3057,12 +3064,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


2014-09-23 14:58:36

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath10k: support 32+ stations.



On 09/23/2014 05:59 AM, Kalle Valo wrote:
> [email protected] writes:
>
>> From: Ben Greear <[email protected]>
>>
>> Support up to 32 stations when using CT firmware.
>>
>> Signed-off-by: Ben Greear <[email protected]>
>
> [...]
>
>> - if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
>> + 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 = (1LL << TARGET_NUM_VDEVS) - 1;
>
> [...]
>
>> +/* 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)
>
> [...]
>
>> + if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
>> + max_num_peers = TARGET_10X_NUM_PEERS_CT - 1;
>
> Like I said before, this hardcoding of values using feature bits is not
> going to work in the long run. It's better that these values come
> directly from FW IEs as integers, that gives us a lot more flexibility
> between firmware versions.
>
> That's why I'll drop patches 2 and 3.

Please suggest the name and definition of the IE(s) that you want for
this feature so I don't have to guess.

I also have a pending patch that supports tx status when using CT
firmware. How would you prefer this feature be communicated to
the driver? IE flag?

Please also understand, it will be a bad idea to force the number of
stations based on IE, but we could note upper limits and let module
parameters tune the actual stations?

Note that my patch above, for instance, decreases number of peers, but
increases vdevs. That works well for my purposes, but others may
want more peers and less vdevs.

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2014-09-23 12:59:12

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath10k: support 32+ stations.

[email protected] writes:

> From: Ben Greear <[email protected]>
>
> Support up to 32 stations when using CT firmware.
>
> Signed-off-by: Ben Greear <[email protected]>

[...]

> - if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features))
> + 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 = (1LL << TARGET_NUM_VDEVS) - 1;

[...]

> +/* 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)

[...]

> + if (test_bit(ATH10K_FW_FEATURE_WMI_10X_CT, ar->fw_features))
> + max_num_peers = TARGET_10X_NUM_PEERS_CT - 1;

Like I said before, this hardcoding of values using feature bits is not
going to work in the long run. It's better that these values come
directly from FW IEs as integers, that gives us a lot more flexibility
between firmware versions.

That's why I'll drop patches 2 and 3.

--
Kalle Valo