Return-path: Received: from mail-wg0-f49.google.com ([74.125.82.49]:54854 "EHLO mail-wg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751601AbaIYGXh convert rfc822-to-8bit (ORCPT ); Thu, 25 Sep 2014 02:23:37 -0400 Received: by mail-wg0-f49.google.com with SMTP id x12so7425012wgg.32 for ; Wed, 24 Sep 2014 23:23:36 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <5422F1C2.4090100@candelatech.com> 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> Date: Thu, 25 Sep 2014 08:23:35 +0200 Message-ID: (sfid-20140925_082340_263770_526FA2AA) Subject: Re: [RFC 2/2] ath10k: re-config ht_caps when chainmask is modified. From: Michal Kazior To: Ben Greear Cc: linux-wireless , "ath10k@lists.infradead.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: 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. Michał