From: Ben Greear <[email protected]>
We will need to use these in set-antenna, so move them
so that we do not have to define the method signatures.
Signed-off-by: Ben Greear <[email protected]>
---
This appears to work, but needs more testing before it should
be applied. I'll do that testing tomorrow.
drivers/net/wireless/ath/ath10k/mac.c | 172 +++++++++++++++++-----------------
1 file changed, 86 insertions(+), 86 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index bde3a2f..dbef84a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2408,6 +2408,92 @@ void ath10k_halt(struct ath10k *ar)
spin_unlock_bh(&ar->data_lock);
}
+static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
+{
+ struct ieee80211_sta_vht_cap vht_cap = {0};
+ u16 mcs_map;
+ int i;
+
+ vht_cap.vht_supported = 1;
+ vht_cap.cap = ar->vht_cap_info;
+
+ mcs_map = 0;
+ for (i = 0; i < 8; i++) {
+ if (i < ar->num_rf_chains)
+ mcs_map |= IEEE80211_VHT_MCS_SUPPORT_0_9 << (i*2);
+ else
+ mcs_map |= IEEE80211_VHT_MCS_NOT_SUPPORTED << (i*2);
+ }
+
+ vht_cap.vht_mcs.rx_mcs_map = cpu_to_le16(mcs_map);
+ vht_cap.vht_mcs.tx_mcs_map = cpu_to_le16(mcs_map);
+
+ return vht_cap;
+}
+
+static struct ieee80211_sta_ht_cap ath10k_get_ht_cap(struct ath10k *ar)
+{
+ int i;
+ struct ieee80211_sta_ht_cap ht_cap = {0};
+
+ if (!(ar->ht_cap_info & WMI_HT_CAP_ENABLED))
+ return ht_cap;
+
+ ht_cap.ht_supported = 1;
+ ht_cap.ampdu_factor = IEEE80211_HT_MAX_AMPDU_64K;
+ ht_cap.ampdu_density = IEEE80211_HT_MPDU_DENSITY_8;
+ ht_cap.cap |= IEEE80211_HT_CAP_SUP_WIDTH_20_40;
+ ht_cap.cap |= IEEE80211_HT_CAP_DSSSCCK40;
+ ht_cap.cap |= WLAN_HT_CAP_SM_PS_STATIC << IEEE80211_HT_CAP_SM_PS_SHIFT;
+
+ if (ar->ht_cap_info & WMI_HT_CAP_HT20_SGI)
+ ht_cap.cap |= IEEE80211_HT_CAP_SGI_20;
+
+ if (ar->ht_cap_info & WMI_HT_CAP_HT40_SGI)
+ ht_cap.cap |= IEEE80211_HT_CAP_SGI_40;
+
+ if (ar->ht_cap_info & WMI_HT_CAP_DYNAMIC_SMPS) {
+ u32 smps;
+
+ smps = WLAN_HT_CAP_SM_PS_DYNAMIC;
+ smps <<= IEEE80211_HT_CAP_SM_PS_SHIFT;
+
+ ht_cap.cap |= smps;
+ }
+
+ if (ar->ht_cap_info & WMI_HT_CAP_TX_STBC)
+ ht_cap.cap |= IEEE80211_HT_CAP_TX_STBC;
+
+ if (ar->ht_cap_info & WMI_HT_CAP_RX_STBC) {
+ u32 stbc;
+
+ stbc = ar->ht_cap_info;
+ stbc &= WMI_HT_CAP_RX_STBC;
+ stbc >>= WMI_HT_CAP_RX_STBC_MASK_SHIFT;
+ stbc <<= IEEE80211_HT_CAP_RX_STBC_SHIFT;
+ stbc &= IEEE80211_HT_CAP_RX_STBC;
+
+ ht_cap.cap |= stbc;
+ }
+
+ if (ar->ht_cap_info & WMI_HT_CAP_LDPC)
+ ht_cap.cap |= IEEE80211_HT_CAP_LDPC_CODING;
+
+ if (ar->ht_cap_info & WMI_HT_CAP_L_SIG_TXOP_PROT)
+ ht_cap.cap |= IEEE80211_HT_CAP_LSIG_TXOP_PROT;
+
+ /* max AMSDU is implicitly taken from vht_cap_info */
+ if (ar->vht_cap_info & WMI_VHT_CAP_MAX_MPDU_LEN_MASK)
+ ht_cap.cap |= IEEE80211_HT_CAP_MAX_AMSDU;
+
+ for (i = 0; i < ar->num_rf_chains; i++)
+ ht_cap.mcs.rx_mask[i] = 0xFF;
+
+ ht_cap.mcs.tx_params |= IEEE80211_HT_MCS_TX_DEFINED;
+
+ return ht_cap;
+}
+
static int ath10k_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant)
{
struct ath10k *ar = hw->priv;
@@ -4735,92 +4821,6 @@ static const struct ieee80211_iface_combination ath10k_10x_if_comb[] = {
},
};
-static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
-{
- struct ieee80211_sta_vht_cap vht_cap = {0};
- u16 mcs_map;
- int i;
-
- vht_cap.vht_supported = 1;
- vht_cap.cap = ar->vht_cap_info;
-
- mcs_map = 0;
- for (i = 0; i < 8; i++) {
- if (i < ar->num_rf_chains)
- mcs_map |= IEEE80211_VHT_MCS_SUPPORT_0_9 << (i*2);
- else
- mcs_map |= IEEE80211_VHT_MCS_NOT_SUPPORTED << (i*2);
- }
-
- vht_cap.vht_mcs.rx_mcs_map = cpu_to_le16(mcs_map);
- vht_cap.vht_mcs.tx_mcs_map = cpu_to_le16(mcs_map);
-
- return vht_cap;
-}
-
-static struct ieee80211_sta_ht_cap ath10k_get_ht_cap(struct ath10k *ar)
-{
- int i;
- struct ieee80211_sta_ht_cap ht_cap = {0};
-
- if (!(ar->ht_cap_info & WMI_HT_CAP_ENABLED))
- return ht_cap;
-
- ht_cap.ht_supported = 1;
- ht_cap.ampdu_factor = IEEE80211_HT_MAX_AMPDU_64K;
- ht_cap.ampdu_density = IEEE80211_HT_MPDU_DENSITY_8;
- ht_cap.cap |= IEEE80211_HT_CAP_SUP_WIDTH_20_40;
- ht_cap.cap |= IEEE80211_HT_CAP_DSSSCCK40;
- ht_cap.cap |= WLAN_HT_CAP_SM_PS_STATIC << IEEE80211_HT_CAP_SM_PS_SHIFT;
-
- if (ar->ht_cap_info & WMI_HT_CAP_HT20_SGI)
- ht_cap.cap |= IEEE80211_HT_CAP_SGI_20;
-
- if (ar->ht_cap_info & WMI_HT_CAP_HT40_SGI)
- ht_cap.cap |= IEEE80211_HT_CAP_SGI_40;
-
- if (ar->ht_cap_info & WMI_HT_CAP_DYNAMIC_SMPS) {
- u32 smps;
-
- smps = WLAN_HT_CAP_SM_PS_DYNAMIC;
- smps <<= IEEE80211_HT_CAP_SM_PS_SHIFT;
-
- ht_cap.cap |= smps;
- }
-
- if (ar->ht_cap_info & WMI_HT_CAP_TX_STBC)
- ht_cap.cap |= IEEE80211_HT_CAP_TX_STBC;
-
- if (ar->ht_cap_info & WMI_HT_CAP_RX_STBC) {
- u32 stbc;
-
- stbc = ar->ht_cap_info;
- stbc &= WMI_HT_CAP_RX_STBC;
- stbc >>= WMI_HT_CAP_RX_STBC_MASK_SHIFT;
- stbc <<= IEEE80211_HT_CAP_RX_STBC_SHIFT;
- stbc &= IEEE80211_HT_CAP_RX_STBC;
-
- ht_cap.cap |= stbc;
- }
-
- if (ar->ht_cap_info & WMI_HT_CAP_LDPC)
- ht_cap.cap |= IEEE80211_HT_CAP_LDPC_CODING;
-
- if (ar->ht_cap_info & WMI_HT_CAP_L_SIG_TXOP_PROT)
- ht_cap.cap |= IEEE80211_HT_CAP_LSIG_TXOP_PROT;
-
- /* max AMSDU is implicitly taken from vht_cap_info */
- if (ar->vht_cap_info & WMI_VHT_CAP_MAX_MPDU_LEN_MASK)
- ht_cap.cap |= IEEE80211_HT_CAP_MAX_AMSDU;
-
- for (i = 0; i < ar->num_rf_chains; i++)
- ht_cap.mcs.rx_mask[i] = 0xFF;
-
- ht_cap.mcs.tx_params |= IEEE80211_HT_MCS_TX_DEFINED;
-
- return ht_cap;
-}
-
static void ath10k_get_arvif_iter(void *data, u8 *mac,
struct ieee80211_vif *vif)
{
--
1.7.11.7
On 09/24/2014 12:51 AM, Michal Kazior wrote:
> On 24 September 2014 02:26, <[email protected]> wrote:
> [...]
>> +static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar,
>> + bool use_cfg_chains)
>> {
>> struct ieee80211_sta_vht_cap vht_cap = {0};
>> u16 mcs_map;
>> int i;
>> + int nrf = ar->num_rf_chains;
>> +
>> + if (use_cfg_chains && ar->cfg_tx_chainmask)
>> + nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
>
> Is use_cfg_chains really necessary here? Is setting tx/rx chainmask to
> 0x0 make any sense at all? Shouldn't we deny it or make it fallback to
> the supported tx/rx chainmask values?
I was thinking we should register with supported values, instead of
configured values. That is the intention of the code. In case we
ever re-register after user has configured the system, this should
retain that functionality. If it is impossible to re-register the
wiphy, then this extra use_cfg_chains logic could go away.
On startup, before user ever configures anything (and most users never will),
the cfg-tx-chainmask is 0, so it would stay with chip's defaults.
Thanks,
Ben
>
>
> Michał
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 09/24/2014 08:05 AM, Michal Kazior wrote:
> On 24 September 2014 16:35, Ben Greear <[email protected]> wrote:
>> On 09/24/2014 12:51 AM, Michal Kazior wrote:
>>> On 24 September 2014 02:26, <[email protected]> wrote:
>>> [...]
>>>>
>>>> +static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k
>>>> *ar,
>>>> + bool
>>>> use_cfg_chains)
>>>> {
>>>> struct ieee80211_sta_vht_cap vht_cap = {0};
>>>> u16 mcs_map;
>>>> int i;
>>>> + int nrf = ar->num_rf_chains;
>>>> +
>>>> + if (use_cfg_chains && ar->cfg_tx_chainmask)
>>>> + nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
>>>
>>>
>>> Is use_cfg_chains really necessary here? Is setting tx/rx chainmask to
>>> 0x0 make any sense at all? Shouldn't we deny it or make it fallback to
>>> the supported tx/rx chainmask values?
>>
>> It would cause the logic to flip back to the defaults, so seems mildly
>> useful. I'm not sure
>> upper layers would ever let it be < 1 though.
>
> 0 is a valid argument as far as upper layers are concerned and should
> be treated as "use all available antennas" (see `iw list` output
> before ever setting antenna, after setting to, e.g. 1 and then to 0).
>
> This implies current set_antenna() implementation is actually buggy
> (pdev param should involve using supp_tx/rx_chainmask). Your
> assumption in recent patches is also incorrect as antenna mask = 0
> should imply max nss, not 1.
I tested this using:
iw phy wiphy1 set antenna 0 0
This flips it back to 3x3 (I had previously configured it for 2x2),
so I think the patches are working properly.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 09/24/2014 12:51 AM, Michal Kazior wrote:
> On 24 September 2014 02:26, <[email protected]> wrote:
> [...]
>> +static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar,
>> + bool use_cfg_chains)
>> {
>> struct ieee80211_sta_vht_cap vht_cap = {0};
>> u16 mcs_map;
>> int i;
>> + int nrf = ar->num_rf_chains;
>> +
>> + if (use_cfg_chains && ar->cfg_tx_chainmask)
>> + nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
>
> Is use_cfg_chains really necessary here? Is setting tx/rx chainmask to
> 0x0 make any sense at all? Shouldn't we deny it or make it fallback to
> the supported tx/rx chainmask values?
It would cause the logic to flip back to the defaults, so seems mildly useful. I'm not sure
upper layers would ever let it be < 1 though.
Thanks,
Ben
>
>
> Michał
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 09/24/2014 11:23 PM, Michal Kazior wrote:
> On 24 September 2014 18:30, Ben Greear <[email protected]> wrote:
>> On 09/24/2014 08:05 AM, Michal Kazior wrote:
>>> On 24 September 2014 16:35, Ben Greear <[email protected]> wrote:
>>>> On 09/24/2014 12:51 AM, Michal Kazior wrote:
>>>>> On 24 September 2014 02:26, <[email protected]> wrote:
>>>>> [...]
>>>>>>
>>>>>> +static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k
>>>>>> *ar,
>>>>>> + bool
>>>>>> use_cfg_chains)
>>>>>> {
>>>>>> struct ieee80211_sta_vht_cap vht_cap = {0};
>>>>>> u16 mcs_map;
>>>>>> int i;
>>>>>> + int nrf = ar->num_rf_chains;
>>>>>> +
>>>>>> + if (use_cfg_chains && ar->cfg_tx_chainmask)
>>>>>> + nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
>>>>>
>>>>>
>>>>> Is use_cfg_chains really necessary here? Is setting tx/rx chainmask to
>>>>> 0x0 make any sense at all? Shouldn't we deny it or make it fallback to
>>>>> the supported tx/rx chainmask values?
>>>>
>>>> It would cause the logic to flip back to the defaults, so seems mildly
>>>> useful. I'm not sure
>>>> upper layers would ever let it be < 1 though.
>>>
>>> 0 is a valid argument as far as upper layers are concerned and should
>>> be treated as "use all available antennas" (see `iw list` output
>>> before ever setting antenna, after setting to, e.g. 1 and then to 0).
>>>
>>> This implies current set_antenna() implementation is actually buggy
>>> (pdev param should involve using supp_tx/rx_chainmask). Your
>>> assumption in recent patches is also incorrect as antenna mask = 0
>>> should imply max nss, not 1.
>>
>> I tested this using:
>>
>> iw phy wiphy1 set antenna 0 0
>>
>> This flips it back to 3x3 (I had previously configured it for 2x2),
>> so I think the patches are working properly.
>
> Mea culpa. It will flip back indeed.
>
> But I still don't see why use_cfg_chains is necessary. I don't see how
> cfg_tx_chainmask could be non-zero when ath10k is registering to mac.
I was thinking we might want to re-register someday, like after loading
a new firmware, or tuning firmware differently so that the vdev limits
changed.
If you are sure we currently only register once per module load, then
I agree that use_cfg_chains should not be needed currently.
But, considering my desire to allow to re-register in the future, I'd
prefer the patch to remain as is unless you disagree.
Thanks,
Ben
>
>
> Michał
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 24 September 2014 16:35, Ben Greear <[email protected]> wrote:
> On 09/24/2014 12:51 AM, Michal Kazior wrote:
>> On 24 September 2014 02:26, <[email protected]> wrote:
>> [...]
>>>
>>> +static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k
>>> *ar,
>>> + bool
>>> use_cfg_chains)
>>> {
>>> struct ieee80211_sta_vht_cap vht_cap = {0};
>>> u16 mcs_map;
>>> int i;
>>> + int nrf = ar->num_rf_chains;
>>> +
>>> + if (use_cfg_chains && ar->cfg_tx_chainmask)
>>> + nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
>>
>>
>> Is use_cfg_chains really necessary here? Is setting tx/rx chainmask to
>> 0x0 make any sense at all? Shouldn't we deny it or make it fallback to
>> the supported tx/rx chainmask values?
>
> It would cause the logic to flip back to the defaults, so seems mildly
> useful. I'm not sure
> upper layers would ever let it be < 1 though.
0 is a valid argument as far as upper layers are concerned and should
be treated as "use all available antennas" (see `iw list` output
before ever setting antenna, after setting to, e.g. 1 and then to 0).
This implies current set_antenna() implementation is actually buggy
(pdev param should involve using supp_tx/rx_chainmask). Your
assumption in recent patches is also incorrect as antenna mask = 0
should imply max nss, not 1.
Michał
On 24 September 2014 02:26, <[email protected]> wrote:
[...]
> +static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar,
> + bool use_cfg_chains)
> {
> struct ieee80211_sta_vht_cap vht_cap = {0};
> u16 mcs_map;
> int i;
> + int nrf = ar->num_rf_chains;
> +
> + if (use_cfg_chains && ar->cfg_tx_chainmask)
> + nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
Is use_cfg_chains really necessary here? Is setting tx/rx chainmask to
0x0 make any sense at all? Shouldn't we deny it or make it fallback to
the supported tx/rx chainmask values?
Michał
From: Ben Greear <[email protected]>
This lets the mac80211 stack use the proper ht-caps when
negotiating with the peer. Note that all vdevs are guaranteed
to be down when antenna changes, so we can set the ht_caps
without worrying about messing up existing stations.
This patch also moves the get_nss_from_chainmask up in
the file.
Signed-off-by: Ben Greear <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 57 ++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index dbef84a..8bd47ea 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2408,18 +2408,34 @@ void ath10k_halt(struct ath10k *ar)
spin_unlock_bh(&ar->data_lock);
}
-static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
+static u32 get_nss_from_chainmask(u16 chain_mask)
+{
+ if ((chain_mask & 0x15) == 0x15)
+ return 4;
+ else if ((chain_mask & 0x7) == 0x7)
+ return 3;
+ else if ((chain_mask & 0x3) == 0x3)
+ return 2;
+ return 1;
+}
+
+static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar,
+ bool use_cfg_chains)
{
struct ieee80211_sta_vht_cap vht_cap = {0};
u16 mcs_map;
int i;
+ int nrf = ar->num_rf_chains;
+
+ if (use_cfg_chains && ar->cfg_tx_chainmask)
+ nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
vht_cap.vht_supported = 1;
vht_cap.cap = ar->vht_cap_info;
mcs_map = 0;
for (i = 0; i < 8; i++) {
- if (i < ar->num_rf_chains)
+ if (i < nrf)
mcs_map |= IEEE80211_VHT_MCS_SUPPORT_0_9 << (i*2);
else
mcs_map |= IEEE80211_VHT_MCS_NOT_SUPPORTED << (i*2);
@@ -2431,10 +2447,15 @@ static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
return vht_cap;
}
-static struct ieee80211_sta_ht_cap ath10k_get_ht_cap(struct ath10k *ar)
+static struct ieee80211_sta_ht_cap ath10k_get_ht_cap(struct ath10k *ar,
+ bool use_cfg_chains)
{
int i;
struct ieee80211_sta_ht_cap ht_cap = {0};
+ int nrf = ar->num_rf_chains;
+
+ if (use_cfg_chains && ar->cfg_tx_chainmask)
+ nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
if (!(ar->ht_cap_info & WMI_HT_CAP_ENABLED))
return ht_cap;
@@ -2486,7 +2507,7 @@ static struct ieee80211_sta_ht_cap ath10k_get_ht_cap(struct ath10k *ar)
if (ar->vht_cap_info & WMI_VHT_CAP_MAX_MPDU_LEN_MASK)
ht_cap.cap |= IEEE80211_HT_CAP_MAX_AMSDU;
- for (i = 0; i < ar->num_rf_chains; i++)
+ for (i = 0; i < nrf; i++)
ht_cap.mcs.rx_mask[i] = 0xFF;
ht_cap.mcs.tx_params |= IEEE80211_HT_MCS_TX_DEFINED;
@@ -2528,6 +2549,8 @@ static void ath10k_check_chain_mask(struct ath10k *ar, u32 cm, const char* dbg)
static int __ath10k_set_antenna(struct ath10k *ar, u32 tx_ant, u32 rx_ant)
{
int ret;
+ struct ieee80211_sta_vht_cap vht_cap;
+ struct ieee80211_sta_ht_cap ht_cap;
lockdep_assert_held(&ar->conf_mutex);
@@ -2537,6 +2560,17 @@ static int __ath10k_set_antenna(struct ath10k *ar, u32 tx_ant, u32 rx_ant)
ar->cfg_tx_chainmask = tx_ant;
ar->cfg_rx_chainmask = rx_ant;
+ ht_cap = ath10k_get_ht_cap(ar, true);
+ vht_cap = ath10k_create_vht_cap(ar, true);
+
+ if (ar->phy_capability & WHAL_WLAN_11G_CAPABILITY)
+ ar->mac.sbands[IEEE80211_BAND_2GHZ].ht_cap = ht_cap;
+
+ if (ar->phy_capability & WHAL_WLAN_11A_CAPABILITY) {
+ ar->mac.sbands[IEEE80211_BAND_5GHZ].ht_cap = ht_cap;
+ ar->mac.sbands[IEEE80211_BAND_5GHZ].vht_cap = vht_cap;
+ }
+
if ((ar->state != ATH10K_STATE_ON) &&
(ar->state != ATH10K_STATE_RESTARTED))
return 0;
@@ -2859,17 +2893,6 @@ static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
return ret;
}
-static u32 get_nss_from_chainmask(u16 chain_mask)
-{
- if ((chain_mask & 0x15) == 0x15)
- return 4;
- else if ((chain_mask & 0x7) == 0x7)
- return 3;
- else if ((chain_mask & 0x3) == 0x3)
- return 2;
- return 1;
-}
-
/*
* TODO:
* Figure out how to handle WMI_VDEV_SUBTYPE_P2P_DEVICE,
@@ -4864,8 +4887,8 @@ int ath10k_mac_register(struct ath10k *ar)
SET_IEEE80211_DEV(ar->hw, ar->dev);
- ht_cap = ath10k_get_ht_cap(ar);
- vht_cap = ath10k_create_vht_cap(ar);
+ ht_cap = ath10k_get_ht_cap(ar, false);
+ vht_cap = ath10k_create_vht_cap(ar, false);
if (ar->phy_capability & WHAL_WLAN_11G_CAPABILITY) {
channels = kmemdup(ath10k_2ghz_channels,
--
1.7.11.7
On 24 September 2014 18:30, Ben Greear <[email protected]> wrote:
> On 09/24/2014 08:05 AM, Michal Kazior wrote:
>> On 24 September 2014 16:35, Ben Greear <[email protected]> wrote:
>>> On 09/24/2014 12:51 AM, Michal Kazior wrote:
>>>> On 24 September 2014 02:26, <[email protected]> wrote:
>>>> [...]
>>>>>
>>>>> +static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k
>>>>> *ar,
>>>>> + bool
>>>>> use_cfg_chains)
>>>>> {
>>>>> struct ieee80211_sta_vht_cap vht_cap = {0};
>>>>> u16 mcs_map;
>>>>> int i;
>>>>> + int nrf = ar->num_rf_chains;
>>>>> +
>>>>> + if (use_cfg_chains && ar->cfg_tx_chainmask)
>>>>> + nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
>>>>
>>>>
>>>> Is use_cfg_chains really necessary here? Is setting tx/rx chainmask to
>>>> 0x0 make any sense at all? Shouldn't we deny it or make it fallback to
>>>> the supported tx/rx chainmask values?
>>>
>>> It would cause the logic to flip back to the defaults, so seems mildly
>>> useful. I'm not sure
>>> upper layers would ever let it be < 1 though.
>>
>> 0 is a valid argument as far as upper layers are concerned and should
>> be treated as "use all available antennas" (see `iw list` output
>> before ever setting antenna, after setting to, e.g. 1 and then to 0).
>>
>> This implies current set_antenna() implementation is actually buggy
>> (pdev param should involve using supp_tx/rx_chainmask). Your
>> assumption in recent patches is also incorrect as antenna mask = 0
>> should imply max nss, not 1.
>
> I tested this using:
>
> iw phy wiphy1 set antenna 0 0
>
> This flips it back to 3x3 (I had previously configured it for 2x2),
> so I think the patches are working properly.
Mea culpa. It will flip back indeed.
But I still don't see why use_cfg_chains is necessary. I don't see how
cfg_tx_chainmask could be non-zero when ath10k is registering to mac.
Michał
On 09/24/2014 08:05 AM, Michal Kazior wrote:
> On 24 September 2014 16:35, Ben Greear <[email protected]> wrote:
>> On 09/24/2014 12:51 AM, Michal Kazior wrote:
>>> On 24 September 2014 02:26, <[email protected]> wrote:
>>> [...]
>>>>
>>>> +static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k
>>>> *ar,
>>>> + bool
>>>> use_cfg_chains)
>>>> {
>>>> struct ieee80211_sta_vht_cap vht_cap = {0};
>>>> u16 mcs_map;
>>>> int i;
>>>> + int nrf = ar->num_rf_chains;
>>>> +
>>>> + if (use_cfg_chains && ar->cfg_tx_chainmask)
>>>> + nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
>>>
>>>
>>> Is use_cfg_chains really necessary here? Is setting tx/rx chainmask to
>>> 0x0 make any sense at all? Shouldn't we deny it or make it fallback to
>>> the supported tx/rx chainmask values?
>>
>> It would cause the logic to flip back to the defaults, so seems mildly
>> useful. I'm not sure
>> upper layers would ever let it be < 1 though.
>
> 0 is a valid argument as far as upper layers are concerned and should
> be treated as "use all available antennas" (see `iw list` output
> before ever setting antenna, after setting to, e.g. 1 and then to 0).
>
> This implies current set_antenna() implementation is actually buggy
> (pdev param should involve using supp_tx/rx_chainmask). Your
> assumption in recent patches is also incorrect as antenna mask = 0
> should imply max nss, not 1.
I will test, but I think you are mis-understanding the logic in my
patches. I should be using the max nss whenever configured value
is 0.
Thanks,
Ben
>
>
> Michał
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com