From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail2.candelatech.com ([208.74.158.173]) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1XX94d-00041T-1t for ath10k@lists.infradead.org; Thu, 25 Sep 2014 13:27:08 +0000 Message-ID: <54241815.6030000@candelatech.com> Date: Thu, 25 Sep 2014 06:26:45 -0700 From: Ben Greear MIME-Version: 1.0 Subject: Re: [RFC 2/2] ath10k: re-config ht_caps when chainmask is modified. References: <1411518383-32634-1-git-send-email-greearb@candelatech.com> <1411518383-32634-2-git-send-email-greearb@candelatech.com> <5422D6CF.10408@candelatech.com> <5422F1C2.4090100@candelatech.com> In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: base64 Content-Type: text/plain; charset="utf-8"; Format="flowed" Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org To: Michal Kazior Cc: linux-wireless , "ath10k@lists.infradead.org" CgpPbiAwOS8yNC8yMDE0IDExOjIzIFBNLCBNaWNoYWwgS2F6aW9yIHdyb3RlOgo+IE9uIDI0IFNl cHRlbWJlciAyMDE0IDE4OjMwLCBCZW4gR3JlZWFyIDxncmVlYXJiQGNhbmRlbGF0ZWNoLmNvbT4g d3JvdGU6Cj4+IE9uIDA5LzI0LzIwMTQgMDg6MDUgQU0sIE1pY2hhbCBLYXppb3Igd3JvdGU6Cj4+ PiBPbiAyNCBTZXB0ZW1iZXIgMjAxNCAxNjozNSwgQmVuIEdyZWVhciA8Z3JlZWFyYkBjYW5kZWxh dGVjaC5jb20+IHdyb3RlOgo+Pj4+IE9uIDA5LzI0LzIwMTQgMTI6NTEgQU0sIE1pY2hhbCBLYXpp b3Igd3JvdGU6Cj4+Pj4+IE9uIDI0IFNlcHRlbWJlciAyMDE0IDAyOjI2LCAgPGdyZWVhcmJAY2Fu ZGVsYXRlY2guY29tPiB3cm90ZToKPj4+Pj4gWy4uLl0KPj4+Pj4+Cj4+Pj4+PiArc3RhdGljIHN0 cnVjdCBpZWVlODAyMTFfc3RhX3ZodF9jYXAgYXRoMTBrX2NyZWF0ZV92aHRfY2FwKHN0cnVjdCBh dGgxMGsKPj4+Pj4+ICphciwKPj4+Pj4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICBib29sCj4+Pj4+PiB1c2VfY2ZnX2NoYWlucykKPj4+ Pj4+ICAgIHsKPj4+Pj4+ICAgICAgICAgICBzdHJ1Y3QgaWVlZTgwMjExX3N0YV92aHRfY2FwIHZo dF9jYXAgPSB7MH07Cj4+Pj4+PiAgICAgICAgICAgdTE2IG1jc19tYXA7Cj4+Pj4+PiAgICAgICAg ICAgaW50IGk7Cj4+Pj4+PiArICAgICAgIGludCBucmYgPSBhci0+bnVtX3JmX2NoYWluczsKPj4+ Pj4+ICsKPj4+Pj4+ICsgICAgICAgaWYgKHVzZV9jZmdfY2hhaW5zICYmIGFyLT5jZmdfdHhfY2hh aW5tYXNrKQo+Pj4+Pj4gKyAgICAgICAgICAgICAgIG5yZiA9IGdldF9uc3NfZnJvbV9jaGFpbm1h c2soYXItPmNmZ190eF9jaGFpbm1hc2spOwo+Pj4+Pgo+Pj4+Pgo+Pj4+PiBJcyB1c2VfY2ZnX2No YWlucyByZWFsbHkgbmVjZXNzYXJ5IGhlcmU/IElzIHNldHRpbmcgdHgvcnggY2hhaW5tYXNrIHRv Cj4+Pj4+IDB4MCBtYWtlIGFueSBzZW5zZSBhdCBhbGw/IFNob3VsZG4ndCB3ZSBkZW55IGl0IG9y IG1ha2UgaXQgZmFsbGJhY2sgdG8KPj4+Pj4gdGhlIHN1cHBvcnRlZCB0eC9yeCBjaGFpbm1hc2sg dmFsdWVzPwo+Pj4+Cj4+Pj4gSXQgd291bGQgY2F1c2UgdGhlIGxvZ2ljIHRvIGZsaXAgYmFjayB0 byB0aGUgZGVmYXVsdHMsIHNvIHNlZW1zIG1pbGRseQo+Pj4+IHVzZWZ1bC4gIEknbSBub3Qgc3Vy ZQo+Pj4+IHVwcGVyIGxheWVycyB3b3VsZCBldmVyIGxldCBpdCBiZSA8IDEgdGhvdWdoLgo+Pj4K Pj4+IDAgaXMgYSB2YWxpZCBhcmd1bWVudCBhcyBmYXIgYXMgdXBwZXIgbGF5ZXJzIGFyZSBjb25j ZXJuZWQgYW5kIHNob3VsZAo+Pj4gYmUgdHJlYXRlZCBhcyAidXNlIGFsbCBhdmFpbGFibGUgYW50 ZW5uYXMiIChzZWUgYGl3IGxpc3RgIG91dHB1dAo+Pj4gYmVmb3JlIGV2ZXIgc2V0dGluZyBhbnRl bm5hLCBhZnRlciBzZXR0aW5nIHRvLCBlLmcuIDEgYW5kIHRoZW4gdG8gMCkuCj4+Pgo+Pj4gVGhp cyBpbXBsaWVzIGN1cnJlbnQgc2V0X2FudGVubmEoKSBpbXBsZW1lbnRhdGlvbiBpcyBhY3R1YWxs eSBidWdneQo+Pj4gKHBkZXYgcGFyYW0gc2hvdWxkIGludm9sdmUgdXNpbmcgc3VwcF90eC9yeF9j aGFpbm1hc2spLiBZb3VyCj4+PiBhc3N1bXB0aW9uIGluIHJlY2VudCBwYXRjaGVzIGlzIGFsc28g aW5jb3JyZWN0IGFzIGFudGVubmEgbWFzayA9IDAKPj4+IHNob3VsZCBpbXBseSBtYXggbnNzLCBu b3QgMS4KPj4KPj4gSSB0ZXN0ZWQgdGhpcyB1c2luZzoKPj4KPj4gaXcgcGh5IHdpcGh5MSBzZXQg YW50ZW5uYSAwIDAKPj4KPj4gVGhpcyBmbGlwcyBpdCBiYWNrIHRvIDN4MyAoSSBoYWQgcHJldmlv dXNseSBjb25maWd1cmVkIGl0IGZvciAyeDIpLAo+PiBzbyBJIHRoaW5rIHRoZSBwYXRjaGVzIGFy ZSB3b3JraW5nIHByb3Blcmx5Lgo+Cj4gTWVhIGN1bHBhLiBJdCB3aWxsIGZsaXAgYmFjayBpbmRl ZWQuCj4KPiBCdXQgSSBzdGlsbCBkb24ndCBzZWUgd2h5IHVzZV9jZmdfY2hhaW5zIGlzIG5lY2Vz c2FyeS4gSSBkb24ndCBzZWUgaG93Cj4gY2ZnX3R4X2NoYWlubWFzayBjb3VsZCBiZSBub24temVy byB3aGVuIGF0aDEwayBpcyByZWdpc3RlcmluZyB0byBtYWMuCgpJIHdhcyB0aGlua2luZyB3ZSBt aWdodCB3YW50IHRvIHJlLXJlZ2lzdGVyIHNvbWVkYXksIGxpa2UgYWZ0ZXIgbG9hZGluZwphIG5l dyBmaXJtd2FyZSwgb3IgdHVuaW5nIGZpcm13YXJlIGRpZmZlcmVudGx5IHNvIHRoYXQgdGhlIHZk ZXYgbGltaXRzCmNoYW5nZWQuCgpJZiB5b3UgYXJlIHN1cmUgd2UgY3VycmVudGx5IG9ubHkgcmVn aXN0ZXIgb25jZSBwZXIgbW9kdWxlIGxvYWQsIHRoZW4KSSBhZ3JlZSB0aGF0IHVzZV9jZmdfY2hh aW5zIHNob3VsZCBub3QgYmUgbmVlZGVkIGN1cnJlbnRseS4KCkJ1dCwgY29uc2lkZXJpbmcgbXkg ZGVzaXJlIHRvIGFsbG93IHRvIHJlLXJlZ2lzdGVyIGluIHRoZSBmdXR1cmUsIEknZApwcmVmZXIg dGhlIHBhdGNoIHRvIHJlbWFpbiBhcyBpcyB1bmxlc3MgeW91IGRpc2FncmVlLgoKVGhhbmtzLApC ZW4KCj4KPgo+IE1pY2hhxYIKPgoKLS0gCkJlbiBHcmVlYXIgPGdyZWVhcmJAY2FuZGVsYXRlY2gu Y29tPgpDYW5kZWxhIFRlY2hub2xvZ2llcyBJbmMgIGh0dHA6Ly93d3cuY2FuZGVsYXRlY2guY29t CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwphdGgxMGsg bWFpbGluZyBsaXN0CmF0aDEwa0BsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZy YWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vYXRoMTBrCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail2.candelatech.com ([208.74.158.173]:41672 "EHLO mail2.candelatech.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751284AbaIYN0r (ORCPT ); Thu, 25 Sep 2014 09:26:47 -0400 Message-ID: <54241815.6030000@candelatech.com> (sfid-20140925_152650_250648_FD9D3C8B) Date: Thu, 25 Sep 2014 06:26:45 -0700 From: Ben Greear MIME-Version: 1.0 To: Michal Kazior CC: linux-wireless , "ath10k@lists.infradead.org" Subject: Re: [RFC 2/2] ath10k: re-config ht_caps when chainmask is modified. References: <1411518383-32634-1-git-send-email-greearb@candelatech.com> <1411518383-32634-2-git-send-email-greearb@candelatech.com> <5422D6CF.10408@candelatech.com> <5422F1C2.4090100@candelatech.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: On 09/24/2014 11:23 PM, Michal Kazior wrote: > On 24 September 2014 18:30, Ben Greear wrote: >> On 09/24/2014 08:05 AM, Michal Kazior wrote: >>> On 24 September 2014 16:35, Ben Greear wrote: >>>> On 09/24/2014 12:51 AM, Michal Kazior wrote: >>>>> On 24 September 2014 02:26, 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 Candela Technologies Inc http://www.candelatech.com