Re: [PATCH] wcn36xx: Set BTLE coexistence related configuration values to defaults

[kbuild test robot]

Hi Eyal,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on v4.14 next-20171114]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Ramon-Fried/wcn36xx-Set-BTLE-coexistence-related-configuration-values-to-defaults/20171114-194715
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git 
master
config: i386-randconfig-x019-201746 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/net/wireless/ath/wcn36xx/main.c:30:33: error: 'WCN36XX_DBG_NONE' 
>> undeclared here (not in a function)
unsigned int wcn36xx_dbg_mask = WCN36XX_DBG_NONE;
^~~~

vim +/WCN36XX_DBG_NONE +30 drivers/net/wireless/ath/wcn36xx/main.c

29  
  > 30  unsigned int wcn36xx_dbg_mask = WCN36XX_DBG_NONE;
31  module_param_named(debug_mask, wcn36xx_dbg_mask, uint, 0644);
32  MODULE_PARM_DESC(debug_mask, "Debugging mask");
33  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] wcn36xx: Set BTLE coexistence related configuration values to defaults

[Kalle Valo]

Ramon Fried  writes:

> From: Eyal Ilsar 
>
> If the value for the firmware configuration parameters
> BTC_STATIC_LEN_LE_BT and BTC_STATIC_LEN_LE_WLAN are not set the duty
> cycle between BT and WLAN is such that if BT (including BLE) is active
> WLAN gets 0 bandwidth. When tuning these parameters having a too high
> value for WLAN means that BLE performance degrades. The "sweet" point
> of roughly half of the maximal values was empirically found to achieve
> a balance between BLE and Wi-Fi coexistence performance.
>
> Signed-off-by: Eyal Ilsar 
> Signed-off-by: Ramon Fried 

Then submit a new version of the patch then please include the version
number:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#patch_version_missing

So after fixing Bjorn's comments the next version should be v3.

-- 
Kalle Valo

Re: [PATCH] wcn36xx: Set BTLE coexistence related configuration values to defaults

[Bjorn Andersson]

On Sun 12 Nov 06:21 PST 2017, Ramon Fried wrote:

> From: Eyal Ilsar 
> 
> If the value for the firmware configuration parameters BTC_STATIC_LEN_LE_BT
> and BTC_STATIC_LEN_LE_WLAN are not set the duty cycle between BT and WLAN
> is such that if BT (including BLE) is active WLAN gets 0 bandwidth.
> When tuning these parameters having a too high value for WLAN means that BLE 
> performance degrades.
> The "sweet" point of roughly half of the maximal values was empirically found 
> to achieve
> a balance between BLE and Wi-Fi coexistence performance.
> 

Thanks for the patch! Just some minor comments.

Please limit subject to 50 chars and wrap body at 72 chars.

> Signed-off-by: Eyal Ilsar 
> Signed-off-by: Ramon Fried 
> ---
>  drivers/net/wireless/ath/wcn36xx/smd.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c 
> b/drivers/net/wireless/ath/wcn36xx/smd.c
> index 9c6590d..1c75987 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -72,8 +72,10 @@ struct wcn36xx_cfg_val {
>   WCN36XX_CFG_VAL(DYNAMIC_PS_POLL_VALUE, 0),
>   WCN36XX_CFG_VAL(TX_PWR_CTRL_ENABLE, 1),
>   WCN36XX_CFG_VAL(ENABLE_CLOSE_LOOP, 1),
> - WCN36XX_CFG_VAL(ENABLE_LPWR_IMG_TRANSITION, 0),

I don't see a need for moving this line.

> + WCN36XX_CFG_VAL(BTC_STATIC_LEN_LE_BT, 12),
> + WCN36XX_CFG_VAL(BTC_STATIC_LEN_LE_WLAN, 3),

These looks reasonable, are we okay leaving the other coexistence
properties at their preconfigured values?

>   WCN36XX_CFG_VAL(MAX_ASSOC_LIMIT, 10),
> + WCN36XX_CFG_VAL(ENABLE_LPWR_IMG_TRANSITION, 0),
>   WCN36XX_CFG_VAL(ENABLE_MCC_ADAPTIVE_SCHEDULER, 0),

Regards,
Bjorn

[PATCH] wcn36xx: Set BTLE coexistence related configuration values to defaults

[Ramon Fried]

From: Eyal Ilsar 

If the value for the firmware configuration parameters BTC_STATIC_LEN_LE_BT
and BTC_STATIC_LEN_LE_WLAN are not set the duty cycle between BT and WLAN
is such that if BT (including BLE) is active WLAN gets 0 bandwidth.
When tuning these parameters having a too high value for WLAN means that BLE 
performance degrades.
The "sweet" point of roughly half of the maximal values was empirically found 
to achieve
a balance between BLE and Wi-Fi coexistence performance.

Signed-off-by: Eyal Ilsar 
Signed-off-by: Ramon Fried 
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c 
b/drivers/net/wireless/ath/wcn36xx/smd.c
index 9c6590d..1c75987 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -72,8 +72,10 @@ struct wcn36xx_cfg_val {
WCN36XX_CFG_VAL(DYNAMIC_PS_POLL_VALUE, 0),
WCN36XX_CFG_VAL(TX_PWR_CTRL_ENABLE, 1),
WCN36XX_CFG_VAL(ENABLE_CLOSE_LOOP, 1),
-   WCN36XX_CFG_VAL(ENABLE_LPWR_IMG_TRANSITION, 0),
+   WCN36XX_CFG_VAL(BTC_STATIC_LEN_LE_BT, 12),
+   WCN36XX_CFG_VAL(BTC_STATIC_LEN_LE_WLAN, 3),
WCN36XX_CFG_VAL(MAX_ASSOC_LIMIT, 10),
+   WCN36XX_CFG_VAL(ENABLE_LPWR_IMG_TRANSITION, 0),
WCN36XX_CFG_VAL(ENABLE_MCC_ADAPTIVE_SCHEDULER, 0),
 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] wcn36xx: Set BTLE coexistence related configuration values to defaults

[Kalle Valo]

Ramon Fried  writes:

> From: Eyal Ilsar 
>
> Signed-off-by: Eyal Ilsar 
> Signed-off-by: Ramon Fried 
> ---
>  drivers/net/wireless/ath/wcn36xx/main.c | 2 +-
>  drivers/net/wireless/ath/wcn36xx/smd.c  | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c 
> b/drivers/net/wireless/ath/wcn36xx/main.c
> index b83f01d..0d4ed41 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -27,7 +27,7 @@
>  #include 
>  #include "wcn36xx.h"
>  
> -unsigned int wcn36xx_dbg_mask;
> +unsigned int wcn36xx_dbg_mask = WCN36XX_DBG_NONE;

And this looks unrelated to the title?

-- 
Kalle Valo

Re: [PATCH] wcn36xx: Set BTLE coexistence related configuration values to defaults

[Kalle Valo]

Ramon Fried  writes:

> From: Eyal Ilsar 
>
> Signed-off-by: Eyal Ilsar 
> Signed-off-by: Ramon Fried 

Why? No empty commit logs, please.

-- 
Kalle Valo

[PATCH] wcn36xx: Set BTLE coexistence related configuration values to defaults

[Ramon Fried]

From: Eyal Ilsar 

Signed-off-by: Eyal Ilsar 
Signed-off-by: Ramon Fried 
---
 drivers/net/wireless/ath/wcn36xx/main.c | 2 +-
 drivers/net/wireless/ath/wcn36xx/smd.c  | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c 
b/drivers/net/wireless/ath/wcn36xx/main.c
index b83f01d..0d4ed41 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -27,7 +27,7 @@
 #include 
 #include "wcn36xx.h"
 
-unsigned int wcn36xx_dbg_mask;
+unsigned int wcn36xx_dbg_mask = WCN36XX_DBG_NONE;
 module_param_named(debug_mask, wcn36xx_dbg_mask, uint, 0644);
 MODULE_PARM_DESC(debug_mask, "Debugging mask");
 
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c 
b/drivers/net/wireless/ath/wcn36xx/smd.c
index 9c6590d..1c75987 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -72,8 +72,10 @@ struct wcn36xx_cfg_val {
WCN36XX_CFG_VAL(DYNAMIC_PS_POLL_VALUE, 0),
WCN36XX_CFG_VAL(TX_PWR_CTRL_ENABLE, 1),
WCN36XX_CFG_VAL(ENABLE_CLOSE_LOOP, 1),
-   WCN36XX_CFG_VAL(ENABLE_LPWR_IMG_TRANSITION, 0),
+   WCN36XX_CFG_VAL(BTC_STATIC_LEN_LE_BT, 12),
+   WCN36XX_CFG_VAL(BTC_STATIC_LEN_LE_WLAN, 3),
WCN36XX_CFG_VAL(MAX_ASSOC_LIMIT, 10),
+   WCN36XX_CFG_VAL(ENABLE_LPWR_IMG_TRANSITION, 0),
WCN36XX_CFG_VAL(ENABLE_MCC_ADAPTIVE_SCHEDULER, 0),
 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project