[PATCH v3 0/7] wcn36xx: Add a set of helpers to enable VHT parameter passing

[PATCH v3 0/7] wcn36xx: Add a set of helpers to enable VHT parameter passing

[Bryan O'Donoghue]

This series is two in a set of seven to add support for wcn3680 at 802.11ac
data-rates.

V3:
- Enables MU-MIMO bits in wcn36xx_smd_set_sta_vht_params()
- Adds a set of helper functions. In the previous set these unused
  declarations would cause a warning during compile because they were
  declared static with the intention to add code later on to use them.

  To resolve that, I do not declare the functions static. Static
  declaration is done in a later patch after the functions have been used.

- There are two good reasons for this:
  1. Granularity. I think its important and useful to have a patch per
     function that clearly sets out what it does and why.
  2. Patch size. Declaring seven functions at the time of their first
     use is not good practice.

I think maintaining patch granularity is a good enough reason to not squash
these commits down. It is pretty trivial to declare the functions static
later on without generating compile warnings in the interregnum.

V2:
This set makes a series of modifications to enable passing of an extended
V1 data-structure to the firmware.

A series of of helper functions are added to facilitate the setup.

The final step in the process is enabling the extended data-structure
parameter passing for the wcn3680 only.

Only a firmware that is capable of 80211.ac will accept the larger V1
messages and only after explicitly enabling the DOT11AC firmware feature
bit.

- No functional difference from V1 for these patches
  Breaking into smaller chunks to enable easier review/merging

V1:

https://lore.kernel.org/linux-wireless/87eensldhi.fsf@codeaurora.org/T/#t

Bryan O'Donoghue (7):
  wcn36xx: Add wcn36xx_set_default_rates_v1
  wcn36xx: Add wcn36xx_smd_set_sta_default_vht_params()
  wcn36xx: Add wcn36xx_smd_set_sta_default_ht_ldpc_params()
  wcn36xx: Add wcn36xx_smd_set_sta_vht_params()
  wcn36xx: Add wcn36xx_smd_set_sta_ht_ldpc_params()
  wcn36xx: Add wcn36xx_smd_set_bss_vht_params()
  wcn36xx: Add wrapper function wcn36xx_smd_set_sta_params_v1()

 drivers/net/wireless/ath/wcn36xx/main.c    |  8 ++
 drivers/net/wireless/ath/wcn36xx/smd.c     | 88 ++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h |  1 +
 3 files changed, 97 insertions(+)

-- 
2.27.0

[PATCH v3 1/7] wcn36xx: Add wcn36xx_set_default_rates_v1

[Bryan O'Donoghue]

Add a routine to set some additional default parameters associated with the
V1 data structure.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c    | 8 ++++++++
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index eb518bd88870..ba37439352a7 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -793,6 +793,14 @@ void wcn36xx_set_default_rates(struct wcn36xx_hal_supported_rates *rates)
 		sizeof(*ofdm_rates) * WCN36XX_HAL_NUM_OFDM_RATES);
 	rates->supported_mcs_set[0] = 0xFF;
 }
+
+void wcn36xx_set_default_rates_v1(struct wcn36xx_hal_supported_rates_v1 *rates)
+{
+	rates->op_rate_mode = STA_11ac;
+	rates->vht_rx_mcs_map = IEEE80211_VHT_MCS_SUPPORT_0_9;
+	rates->vht_tx_mcs_map = IEEE80211_VHT_MCS_SUPPORT_0_9;
+}
+
 static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw,
 				     struct ieee80211_vif *vif,
 				     struct ieee80211_bss_conf *bss_conf,
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index d7d349de20e6..2da81d9926c4 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -271,6 +271,7 @@ static inline bool wcn36xx_is_fw_version(struct wcn36xx *wcn,
 		wcn->fw_revision == revision);
 }
 void wcn36xx_set_default_rates(struct wcn36xx_hal_supported_rates *rates);
+void wcn36xx_set_default_rates_v1(struct wcn36xx_hal_supported_rates_v1 *rates);
 
 static inline
 struct ieee80211_sta *wcn36xx_priv_to_sta(struct wcn36xx_sta *sta_priv)
-- 
2.27.0

[PATCH v3 2/7] wcn36xx: Add wcn36xx_smd_set_sta_default_vht_params()

[Bryan O'Donoghue]

This commit adds support for setting default VHT parameters, which are
exposed by the extended version 1 STA parameter type.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index d5ca9907af86..bb31c4d4d9a8 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -190,6 +190,22 @@ static void wcn36xx_smd_set_sta_default_ht_params(
 	sta_params->dsss_cck_mode_40mhz = 1;
 }
 
+void wcn36xx_smd_set_sta_default_vht_params(struct wcn36xx *wcn,
+		struct wcn36xx_hal_config_sta_params_v1 *sta_params)
+{
+	if (wcn->rf_id == RF_IRIS_WCN3680) {
+		sta_params->vht_capable = 1;
+		sta_params->vht_tx_mu_beamformee_capable = 1;
+	} else {
+		sta_params->vht_capable = 0;
+		sta_params->vht_tx_mu_beamformee_capable = 0;
+	}
+
+	sta_params->vht_ldpc_enabled = 0;
+	sta_params->vht_tx_channel_width_set = 0;
+	sta_params->vht_tx_bf_enabled = 0;
+}
+
 static void wcn36xx_smd_set_sta_params(struct wcn36xx *wcn,
 		struct ieee80211_vif *vif,
 		struct ieee80211_sta *sta,
-- 
2.27.0

[PATCH v3 3/7] wcn36xx: Add wcn36xx_smd_set_sta_default_ht_ldpc_params()

[Bryan O'Donoghue]

Toggling the LDPC enabled bit is possible only via the extended V1
data-structure. This function provides a means of setting the default
depending on chip-type.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index bb31c4d4d9a8..3f57acfe11bb 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -206,6 +206,15 @@ void wcn36xx_smd_set_sta_default_vht_params(struct wcn36xx *wcn,
 	sta_params->vht_tx_bf_enabled = 0;
 }
 
+void wcn36xx_smd_set_sta_default_ht_ldpc_params(struct wcn36xx *wcn,
+		struct wcn36xx_hal_config_sta_params_v1 *sta_params)
+{
+	if (wcn->rf_id == RF_IRIS_WCN3680)
+		sta_params->ht_ldpc_enabled = 1;
+	else
+		sta_params->ht_ldpc_enabled = 0;
+}
+
 static void wcn36xx_smd_set_sta_params(struct wcn36xx *wcn,
 		struct ieee80211_vif *vif,
 		struct ieee80211_sta *sta,
-- 
2.27.0

[PATCH v3 4/7] wcn36xx: Add wcn36xx_smd_set_sta_vht_params()

[Bryan O'Donoghue]

This commit adds support for setting VHT parameters based on the declared
VHT capability bits in the VHT capability structure.

We cannot do 160MHz so VHT Channel width set should be zero.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 3f57acfe11bb..a563a30c3a0c 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -174,6 +174,28 @@ static void wcn36xx_smd_set_sta_ht_params(struct ieee80211_sta *sta,
 	}
 }
 
+void wcn36xx_smd_set_sta_vht_params(struct wcn36xx *wcn,
+		struct ieee80211_sta *sta,
+		struct wcn36xx_hal_config_sta_params_v1 *sta_params)
+{
+	if (sta->vht_cap.vht_supported) {
+		unsigned long caps = sta->vht_cap.cap;
+
+		sta_params->vht_capable = sta->vht_cap.vht_supported;
+		sta_params->vht_ldpc_enabled =
+			is_cap_supported(caps, IEEE80211_VHT_CAP_RXLDPC);
+		if (get_feat_caps(wcn->fw_feat_caps, MU_MIMO)) {
+			sta_params->vht_tx_mu_beamformee_capable =
+				is_cap_supported(caps, IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE);
+			if (sta_params->vht_tx_mu_beamformee_capable)
+			       sta_params->vht_tx_bf_enabled = 1;
+		} else {
+			sta_params->vht_tx_mu_beamformee_capable = 0;
+		}
+		sta_params->vht_tx_channel_width_set = 0;
+	}
+}
+
 static void wcn36xx_smd_set_sta_default_ht_params(
 		struct wcn36xx_hal_config_sta_params *sta_params)
 {
-- 
2.27.0

[PATCH v3 5/7] wcn36xx: Add wcn36xx_smd_set_sta_ht_ldpc_params()

[Bryan O'Donoghue]

Adds a routine to allow setting the LDPC bit for HT parameter passing
inside the version 1 STA parameters data structure.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index a563a30c3a0c..14af98af42f3 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -196,6 +196,15 @@ void wcn36xx_smd_set_sta_vht_params(struct wcn36xx *wcn,
 	}
 }
 
+void wcn36xx_smd_set_sta_ht_ldpc_params(struct ieee80211_sta *sta,
+		struct wcn36xx_hal_config_sta_params_v1 *sta_params)
+{
+	if (sta->ht_cap.ht_supported) {
+		sta_params->ht_ldpc_enabled =
+			is_cap_supported(sta->ht_cap.cap, IEEE80211_HT_CAP_LDPC_CODING);
+	}
+}
+
 static void wcn36xx_smd_set_sta_default_ht_params(
 		struct wcn36xx_hal_config_sta_params *sta_params)
 {
-- 
2.27.0

[PATCH v3 6/7] wcn36xx: Add wcn36xx_smd_set_bss_vht_params()

[Bryan O'Donoghue]

This commit adds wcn36xx_smd_set_bss_vht_params(). The job of this function
is to decide if the BSS is VHT capable and if so set the appropriate bit
in the BSS parameter structure for passing to the firmware.

VHT Channel width set is not set since we don't support 160MHz.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 14af98af42f3..fa6f5943a43d 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -146,6 +146,15 @@ static void wcn36xx_smd_set_bss_ht_params(struct ieee80211_vif *vif,
 	}
 }
 
+void
+wcn36xx_smd_set_bss_vht_params(struct ieee80211_vif *vif,
+			       struct ieee80211_sta *sta,
+			       struct wcn36xx_hal_config_bss_params_v1 *bss)
+{
+	if (sta && sta->vht_cap.vht_supported)
+		bss->vht_capable = 1;
+}
+
 static void wcn36xx_smd_set_sta_ht_params(struct ieee80211_sta *sta,
 		struct wcn36xx_hal_config_sta_params *sta_params)
 {
-- 
2.27.0

[PATCH v3 7/7] wcn36xx: Add wrapper function wcn36xx_smd_set_sta_params_v1()

[Bryan O'Donoghue]

This commit adds a wrapper function wcn36xx_smd_set_sta_params_v1() which
calls into wcn36xx_smd_set_sta_params() and then subsequently sets
version-1 specific parameters.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index fa6f5943a43d..f434c355d32d 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -1249,6 +1249,31 @@ static void wcn36xx_smd_convert_sta_to_v1(struct wcn36xx *wcn,
 	v1->p2p = orig->p2p;
 }
 
+void
+wcn36xx_smd_set_sta_params_v1(struct wcn36xx *wcn,
+			      struct ieee80211_vif *vif,
+			      struct ieee80211_sta *sta,
+			      struct wcn36xx_hal_config_sta_params_v1 *sta_par)
+{
+	struct wcn36xx_sta *sta_priv = NULL;
+	struct wcn36xx_hal_config_sta_params sta_par_v0;
+
+	wcn36xx_smd_set_sta_params(wcn, vif, sta, &sta_par_v0);
+	wcn36xx_smd_convert_sta_to_v1(wcn, &sta_par_v0, sta_par);
+
+	if (sta) {
+		sta_priv = wcn36xx_sta_to_priv(sta);
+		wcn36xx_smd_set_sta_vht_params(wcn, sta, sta_par);
+		wcn36xx_smd_set_sta_ht_ldpc_params(sta, sta_par);
+		memcpy(&sta_par->supported_rates, &sta_priv->supported_rates,
+		       sizeof(sta_par->supported_rates));
+	} else {
+		wcn36xx_set_default_rates_v1(&sta_par->supported_rates);
+		wcn36xx_smd_set_sta_default_vht_params(wcn, sta_par);
+		wcn36xx_smd_set_sta_default_ht_ldpc_params(wcn, sta_par);
+	}
+}
+
 static int wcn36xx_smd_config_sta_rsp(struct wcn36xx *wcn,
 				      struct ieee80211_sta *sta,
 				      void *buf,
-- 
2.27.0

Re: [PATCH v3 2/7] wcn36xx: Add wcn36xx_smd_set_sta_default_vht_params()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5175 bytes --]

Hi Bryan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on wireless-drivers/master ath6kl/ath-next v5.9-rc4 next-20200908]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bryan-O-Donoghue/wcn36xx-Add-a-set-of-helpers-to-enable-VHT-parameter-passing/20200909-115630
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/net/wireless/ath/wcn36xx/smd.h:20,
                    from drivers/net/wireless/ath/wcn36xx/smd.c:23:
   drivers/net/wireless/ath/wcn36xx/wcn36xx.h:272:42: warning: 'struct wcn36xx_hal_supported_rates_v1' declared inside parameter list will not be visible outside of this definition or declaration
     272 | void wcn36xx_set_default_rates_v1(struct wcn36xx_hal_supported_rates_v1 *rates);
         |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/wireless/ath/wcn36xx/smd.c:192:6: warning: no previous prototype for 'wcn36xx_smd_set_sta_default_vht_params' [-Wmissing-prototypes]
     192 | void wcn36xx_smd_set_sta_default_vht_params(struct wcn36xx *wcn,
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/wireless/ath/wcn36xx/smd.c: In function 'wcn36xx_smd_set_sta_default_vht_params':
   drivers/net/wireless/ath/wcn36xx/smd.c:195:20: error: 'RF_IRIS_WCN3680' undeclared (first use in this function); did you mean 'RF_IRIS_WCN3620'?
     195 |  if (wcn->rf_id == RF_IRIS_WCN3680) {
         |                    ^~~~~~~~~~~~~~~
         |                    RF_IRIS_WCN3620
   drivers/net/wireless/ath/wcn36xx/smd.c:195:20: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/wireless/ath/wcn36xx/smd.c:196:15: error: 'struct wcn36xx_hal_config_sta_params_v1' has no member named 'vht_capable'; did you mean 'ht_capable'?
     196 |   sta_params->vht_capable = 1;
         |               ^~~~~~~~~~~
         |               ht_capable
   drivers/net/wireless/ath/wcn36xx/smd.c:197:13: error: 'struct wcn36xx_hal_config_sta_params_v1' has no member named 'vht_tx_mu_beamformee_capable'
     197 |   sta_params->vht_tx_mu_beamformee_capable = 1;
         |             ^~
   drivers/net/wireless/ath/wcn36xx/smd.c:199:15: error: 'struct wcn36xx_hal_config_sta_params_v1' has no member named 'vht_capable'; did you mean 'ht_capable'?
     199 |   sta_params->vht_capable = 0;
         |               ^~~~~~~~~~~
         |               ht_capable
   drivers/net/wireless/ath/wcn36xx/smd.c:200:13: error: 'struct wcn36xx_hal_config_sta_params_v1' has no member named 'vht_tx_mu_beamformee_capable'
     200 |   sta_params->vht_tx_mu_beamformee_capable = 0;
         |             ^~
   drivers/net/wireless/ath/wcn36xx/smd.c:203:12: error: 'struct wcn36xx_hal_config_sta_params_v1' has no member named 'vht_ldpc_enabled'
     203 |  sta_params->vht_ldpc_enabled = 0;
         |            ^~
   drivers/net/wireless/ath/wcn36xx/smd.c:204:14: error: 'struct wcn36xx_hal_config_sta_params_v1' has no member named 'vht_tx_channel_width_set'; did you mean 'tx_channel_width_set'?
     204 |  sta_params->vht_tx_channel_width_set = 0;
         |              ^~~~~~~~~~~~~~~~~~~~~~~~
         |              tx_channel_width_set
   drivers/net/wireless/ath/wcn36xx/smd.c:205:12: error: 'struct wcn36xx_hal_config_sta_params_v1' has no member named 'vht_tx_bf_enabled'
     205 |  sta_params->vht_tx_bf_enabled = 0;
         |            ^~

# https://github.com/0day-ci/linux/commit/0b840a269eba144bf378cdbd20160813582915a2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Bryan-O-Donoghue/wcn36xx-Add-a-set-of-helpers-to-enable-VHT-parameter-passing/20200909-115630
git checkout 0b840a269eba144bf378cdbd20160813582915a2
vim +/wcn36xx_smd_set_sta_default_vht_params +192 drivers/net/wireless/ath/wcn36xx/smd.c

   191	
 > 192	void wcn36xx_smd_set_sta_default_vht_params(struct wcn36xx *wcn,
   193			struct wcn36xx_hal_config_sta_params_v1 *sta_params)
   194	{
   195		if (wcn->rf_id == RF_IRIS_WCN3680) {
   196			sta_params->vht_capable = 1;
   197			sta_params->vht_tx_mu_beamformee_capable = 1;
   198		} else {
   199			sta_params->vht_capable = 0;
   200			sta_params->vht_tx_mu_beamformee_capable = 0;
   201		}
   202	
   203		sta_params->vht_ldpc_enabled = 0;
   204		sta_params->vht_tx_channel_width_set = 0;
   205		sta_params->vht_tx_bf_enabled = 0;
   206	}
   207	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 74730 bytes --]

Re: [PATCH v3 2/7] wcn36xx: Add wcn36xx_smd_set_sta_default_vht_params()

[Bryan O'Donoghue]

On 09/09/2020 08:23, kernel test robot wrote:
> Hi Bryan,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on wireless-drivers-next/master]
> [also build test WARNING on wireless-drivers/master ath6kl/ath-next v5.9-rc4 next-20200908]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]

Applies against

ath.git	git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
160b351d75cb - (tag: ath-202009090652, ath.git/master)

but, I don't mind resending after figuring out how to include --base