Re: [PATCH] ath10k: use configured nss instead of max nss.

[Ben Greear <greearb@candelatech.com>]

On 09/24/2014 12:09 AM, Michal Kazior wrote:
> On 23 September 2014 18:48, Ben Greear <greearb@candelatech.com> wrote:
>> On 09/23/2014 01:59 AM, Michal Kazior wrote:
>>> On 23 September 2014 01:00,  <greearb@candelatech.com> wrote:
>>>> From: Ben Greear <greearb@candelatech.com>
> [...]
>>>> @@ -4086,6 +4086,10 @@ ath10k_default_bitrate_mask(struct ath10k *ar,
>>>>         u32 legacy = 0x00ff;
>>>>         u8 ht = 0xff, i;
>>>>         u16 vht = 0x3ff;
>>>> +       u16 nrf = ar->num_rf_chains;
>>>> +
>>>> +       if (ar->cfg_tx_chainmask)
>>>> +               nrf = get_nss_from_chainmask(ar->cfg_tx_chainmask);
> [...]
>>> I think it might be a good idea to convey the limitation of tx/rx
>>> chainmask to the user: you can't change the tx/rx chainmask on the fly
>>> easily (while connected/have associated stations). Or do you plan to
>>> schedule peer reassoc in __ath10k_set_antenna() in a follow up
>>> patch(es) later?
>>
>> My user-space tools assume you have to re-connect (requested from user-space)
>> after changing the chainmask.  I was under the impression that this
>> is how ath9k works, though I could be wrong.
> 
> ath9k doesn't have firmware and it doesn't have a blackbox rate
> control so maybe it doesn't need to worry about this as much as ath10k
> needs to.
> 
> But then this is not something you normally use except some very
> specific use cases so I think it might not be worth the hassle to
> implement full-blown reconfig after chainmask is updated.

See this in net/mac80211/cfg.c:

static int ieee80211_set_antenna(struct wiphy *wiphy, u32 tx_ant, u32 rx_ant)
{
	struct ieee80211_local *local = wiphy_priv(wiphy);

	if (local->started)
		return -EOPNOTSUPP;

	return drv_set_antenna(local, tx_ant, rx_ant);
}


It ensures there are no active vdevs when this change is made, so
you should not have to worry about block-ack or anything similar.


> But it's still probably a good idea to comment the quoted code chunk
> above explaining why nrf is overriden by chainmask (i.e. due to
> firmware rate control issues, right?).

I am not certain it is a bug in the firmware.  The driver should not configure
nss incorrectly as it was doing previous to my recent patches.

Thanks,
Ben

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