[PATCH 0/5] wcn36xx: add some new firmware functionalities support

[PATCH 0/5] wcn36xx: add some new firmware functionalities support

[Yin, Fengwei]

Hi,

This patchset are from Andy Green. Which support to some functionalities
of latest wcn36xx firmware.

Thanks.

Andy Green (5):
  wcn36xx: introduce WCN36XX_HAL_AVOID_FREQ_RANGE_IND
  wcn36xx: swallow two wcn3620 IND messages
  wcn36xx: handle new hal response format
  wcn3620: use new response format for wcn3620 trigger_ba
  wcn3620: use new response format for wcn3620 remove_bsskey

 drivers/net/wireless/ath/wcn36xx/hal.h |  2 ++
 drivers/net/wireless/ath/wcn36xx/smd.c | 28 ++++++++++++++++++++++++++--
 drivers/net/wireless/ath/wcn36xx/smd.h |  9 +++++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

-- 
2.1.4

[PATCH 1/5] wcn36xx: introduce WCN36XX_HAL_AVOID_FREQ_RANGE_IND

[Yin, Fengwei]

From: Andy Green <andy.green@linaro.org>

From: Andy Green <andy.green@linaro.org>

WCN3620 firmware introduces a new async indication, we need to
add it as a known message type so we can accept it

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index a1f1127..b947de0 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -345,6 +345,8 @@ enum wcn36xx_hal_host_msg_type {
 	WCN36XX_HAL_DHCP_START_IND = 189,
 	WCN36XX_HAL_DHCP_STOP_IND = 190,
 
+	WCN36XX_HAL_AVOID_FREQ_RANGE_IND = 233,
+
 	WCN36XX_HAL_MSG_MAX = WCN36XX_HAL_MSG_TYPE_MAX_ENUM_SIZE
 };
 
-- 
2.1.4

Re: [PATCH 1/5] wcn36xx: introduce WCN36XX_HAL_AVOID_FREQ_RANGE_IND

[Bob Copeland]

On Mon, Nov 09, 2015 at 05:02:38AM -0500, Yin, Fengwei wrote:
> From: Andy Green <andy.green@linaro.org>
> 
> From: Andy Green <andy.green@linaro.org>

Hi Fengwei,

There are two "From" lines in this and a couple of the other patches,
can you remove one?  (Not sure if git-am automatically strips the second
or not.)

-- 
Bob Copeland %% http://bobcopeland.com/

[PATCH 2/5] wcn36xx: swallow two wcn3620 IND messages

[Yin, Fengwei]

From: Andy Green <andy.green@linaro.org>

From: Andy Green <andy.green@linaro.org>

WCN3620 can asynchronously send two new kinds of indication message,
since we can't handle them just accept them quietly.

v2: used one break for both in the second stanza

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index c9263e1..be317f4 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2128,6 +2128,8 @@ static void wcn36xx_smd_rsp_process(struct wcn36xx *wcn, void *buf, size_t len)
 		complete(&wcn->hal_rsp_compl);
 		break;
 
+	case WCN36XX_HAL_COEX_IND:
+	case WCN36XX_HAL_AVOID_FREQ_RANGE_IND:
 	case WCN36XX_HAL_OTA_TX_COMPL_IND:
 	case WCN36XX_HAL_MISSED_BEACON_IND:
 	case WCN36XX_HAL_DELETE_STA_CONTEXT_IND:
@@ -2174,6 +2176,9 @@ static void wcn36xx_ind_smd_work(struct work_struct *work)
 	msg_header = (struct wcn36xx_hal_msg_header *)hal_ind_msg->msg;
 
 	switch (msg_header->msg_type) {
+	case WCN36XX_HAL_COEX_IND:
+	case WCN36XX_HAL_AVOID_FREQ_RANGE_IND:
+		break;
 	case WCN36XX_HAL_OTA_TX_COMPL_IND:
 		wcn36xx_smd_tx_compl_ind(wcn,
 					 hal_ind_msg->msg,
-- 
2.1.4

Re: [PATCH 2/5] wcn36xx: swallow two wcn3620 IND messages

[Bob Copeland]

On Mon, Nov 09, 2015 at 05:02:39AM -0500, Yin, Fengwei wrote:
> v2: used one break for both in the second stanza

^^^ this

> 
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---

  ^^^
... should go here (after the ---), so that it doesn't appear in the final
changelog.

-- 
Bob Copeland %% http://bobcopeland.com/

[PATCH 3/5] wcn36xx: handle new hal response format

[Yin, Fengwei]

From: Andy Green <andy.green@linaro.org>

From: Andy Green <andy.green@linaro.org>

wcn3620 has a new message structure for the reply to some hal
commands.  This patch adds the struct and helper routine that
uses it if the chip is wcn3620, or falls back to the old
helper routine.

We don't know what to do with the candidate list he sends back,
but we can at least accept and ignore it nicely instead of dying.

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 17 +++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h |  9 +++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index be317f4..a84c2cc 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -302,6 +302,23 @@ static int wcn36xx_smd_rsp_status_check(void *buf, size_t len)
 	return 0;
 }
 
+static int wcn36xx_smd_rsp_status_check_v2(struct wcn36xx *wcn, void *buf,
+					     size_t len)
+{
+	struct wcn36xx_fw_msg_status_rsp_v2 *rsp;
+
+	if (wcn->chip_version != WCN36XX_CHIP_3620 ||
+	    len < sizeof(struct wcn36xx_hal_msg_header) + sizeof(*rsp))
+		return wcn36xx_smd_rsp_status_check(buf, len);
+
+	rsp = buf + sizeof(struct wcn36xx_hal_msg_header);
+
+	if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status)
+		return rsp->status;
+
+	return 0;
+}
+
 int wcn36xx_smd_load_nv(struct wcn36xx *wcn)
 {
 	struct nv_data *nv_d;
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index 008d034..8361f9e 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -44,6 +44,15 @@ struct wcn36xx_fw_msg_status_rsp {
 	u32	status;
 } __packed;
 
+/* wcn3620 returns this for tigger_ba */
+
+struct wcn36xx_fw_msg_status_rsp_v2 {
+	u8	bss_id[6];
+	u32	status __packed;
+	u16	count_following_candidates __packed;
+	/* candidate list follows */
+};
+
 struct wcn36xx_hal_ind_msg {
 	struct list_head list;
 	u8 *msg;
-- 
2.1.4

Re: [PATCH 3/5] wcn36xx: handle new hal response format

[kbuild test robot]

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

Hi Andy,

[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on v4.3 next-20151109]

url:    https://github.com/0day-ci/linux/commits/Yin-Fengwei/wcn36xx-add-some-new-firmware-functionalities-support/20151109-170444
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: sparc-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc 

All errors (new ones prefixed by >>):

   drivers/net/wireless/ath/wcn36xx/smd.c: In function 'wcn36xx_smd_rsp_status_check_v2':
>> drivers/net/wireless/ath/wcn36xx/smd.c:310:27: error: 'WCN36XX_CHIP_3620' undeclared (first use in this function)
     if (wcn->chip_version != WCN36XX_CHIP_3620 ||
                              ^
   drivers/net/wireless/ath/wcn36xx/smd.c:310:27: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/wireless/ath/wcn36xx/smd.c: At top level:
   drivers/net/wireless/ath/wcn36xx/smd.c:305:12: warning: 'wcn36xx_smd_rsp_status_check_v2' defined but not used [-Wunused-function]
    static int wcn36xx_smd_rsp_status_check_v2(struct wcn36xx *wcn, void *buf,
               ^

vim +/WCN36XX_CHIP_3620 +310 drivers/net/wireless/ath/wcn36xx/smd.c

   304	
   305	static int wcn36xx_smd_rsp_status_check_v2(struct wcn36xx *wcn, void *buf,
   306						     size_t len)
   307	{
   308		struct wcn36xx_fw_msg_status_rsp_v2 *rsp;
   309	
 > 310		if (wcn->chip_version != WCN36XX_CHIP_3620 ||
   311		    len < sizeof(struct wcn36xx_hal_msg_header) + sizeof(*rsp))
   312			return wcn36xx_smd_rsp_status_check(buf, len);
   313	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 43564 bytes --]

Re: [PATCH 3/5] wcn36xx: handle new hal response format

[fengwei.yin]

On 2015/11/9 18:02, Yin, Fengwei wrote:
> From: Andy Green <andy.green@linaro.org>
>
> From: Andy Green <andy.green@linaro.org>
>
> wcn3620 has a new message structure for the reply to some hal
> commands.  This patch adds the struct and helper routine that
> uses it if the chip is wcn3620, or falls back to the old
> helper routine.
>
> We don't know what to do with the candidate list he sends back,
> but we can at least accept and ignore it nicely instead of dying.
>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>   drivers/net/wireless/ath/wcn36xx/smd.c | 17 +++++++++++++++++
>   drivers/net/wireless/ath/wcn36xx/smd.h |  9 +++++++++
>   2 files changed, 26 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index be317f4..a84c2cc 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -302,6 +302,23 @@ static int wcn36xx_smd_rsp_status_check(void *buf, size_t len)
>   	return 0;
>   }
>
> +static int wcn36xx_smd_rsp_status_check_v2(struct wcn36xx *wcn, void *buf,
> +					     size_t len)
> +{
> +	struct wcn36xx_fw_msg_status_rsp_v2 *rsp;
> +
> +	if (wcn->chip_version != WCN36XX_CHIP_3620 ||
> +	    len < sizeof(struct wcn36xx_hal_msg_header) + sizeof(*rsp))
> +		return wcn36xx_smd_rsp_status_check(buf, len);
> +
> +	rsp = buf + sizeof(struct wcn36xx_hal_msg_header);
> +
> +	if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status)
> +		return rsp->status;
> +
> +	return 0;
> +}
> +
>   int wcn36xx_smd_load_nv(struct wcn36xx *wcn)
>   {
>   	struct nv_data *nv_d;
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
> index 008d034..8361f9e 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.h
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.h
> @@ -44,6 +44,15 @@ struct wcn36xx_fw_msg_status_rsp {
>   	u32	status;
>   } __packed;
>
> +/* wcn3620 returns this for tigger_ba */
> +
> +struct wcn36xx_fw_msg_status_rsp_v2 {
> +	u8	bss_id[6];
> +	u32	status __packed;
> +	u16	count_following_candidates __packed;
> +	/* candidate list follows */
> +};
> +
>   struct wcn36xx_hal_ind_msg {
>   	struct list_head list;
>   	u8 *msg;
>

I got message from kbuild test robot that this patch has build error. Please
go ahead to review the other patches. I will send out v2 which have missed
patch included. Sorry for this.

Regards
Yin, Fengwei

Re: [PATCH 3/5] wcn36xx: handle new hal response format

[kbuild test robot]

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

Hi Andy,

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on v4.3 next-20151109]

url:    https://github.com/0day-ci/linux/commits/Yin-Fengwei/wcn36xx-add-some-new-firmware-functionalities-support/20151109-170444
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: x86_64-randconfig-s4-11091756 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/linkage.h:4:0,
                    from include/linux/kernel.h:6,
                    from include/linux/skbuff.h:17,
                    from include/linux/if_ether.h:23,
                    from include/linux/etherdevice.h:25,
                    from drivers/net/wireless/ath/wcn36xx/smd.c:19:
   drivers/net/wireless/ath/wcn36xx/smd.c: In function 'wcn36xx_smd_rsp_status_check_v2':
   drivers/net/wireless/ath/wcn36xx/smd.c:310:27: error: 'WCN36XX_CHIP_3620' undeclared (first use in this function)
     if (wcn->chip_version != WCN36XX_CHIP_3620 ||
                              ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
>> drivers/net/wireless/ath/wcn36xx/smd.c:310:2: note: in expansion of macro 'if'
     if (wcn->chip_version != WCN36XX_CHIP_3620 ||
     ^
   drivers/net/wireless/ath/wcn36xx/smd.c:310:27: note: each undeclared identifier is reported only once for each function it appears in
     if (wcn->chip_version != WCN36XX_CHIP_3620 ||
                              ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
>> drivers/net/wireless/ath/wcn36xx/smd.c:310:2: note: in expansion of macro 'if'
     if (wcn->chip_version != WCN36XX_CHIP_3620 ||
     ^
   drivers/net/wireless/ath/wcn36xx/smd.c: At top level:
   drivers/net/wireless/ath/wcn36xx/smd.c:305:12: warning: 'wcn36xx_smd_rsp_status_check_v2' defined but not used [-Wunused-function]
    static int wcn36xx_smd_rsp_status_check_v2(struct wcn36xx *wcn, void *buf,
               ^

vim +/if +310 drivers/net/wireless/ath/wcn36xx/smd.c

   294			return -EIO;
   295	
   296		rsp = (struct wcn36xx_fw_msg_status_rsp *)
   297			(buf + sizeof(struct wcn36xx_hal_msg_header));
   298	
   299		if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status)
   300			return rsp->status;
   301	
   302		return 0;
   303	}
   304	
   305	static int wcn36xx_smd_rsp_status_check_v2(struct wcn36xx *wcn, void *buf,
   306						     size_t len)
   307	{
   308		struct wcn36xx_fw_msg_status_rsp_v2 *rsp;
   309	
 > 310		if (wcn->chip_version != WCN36XX_CHIP_3620 ||
   311		    len < sizeof(struct wcn36xx_hal_msg_header) + sizeof(*rsp))
   312			return wcn36xx_smd_rsp_status_check(buf, len);
   313	
   314		rsp = buf + sizeof(struct wcn36xx_hal_msg_header);
   315	
   316		if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status)
   317			return rsp->status;
   318	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 25396 bytes --]

Re: [PATCH 3/5] wcn36xx: handle new hal response format

[Bob Copeland]

On Mon, Nov 09, 2015 at 05:02:40AM -0500, Yin, Fengwei wrote:
> 
> From: Andy Green <andy.green@linaro.org>
> 
> wcn3620 has a new message structure for the reply to some hal
> commands.  This patch adds the struct and helper routine that
> uses it if the chip is wcn3620, or falls back to the old
> helper routine.

> +	if (wcn->chip_version != WCN36XX_CHIP_3620 ||
> +	    len < sizeof(struct wcn36xx_hal_msg_header) + sizeof(*rsp))
> +		return wcn36xx_smd_rsp_status_check(buf, len);
> +

Ok, disregard my question on the other patch, for some reason I
read them out of order.  It looks like this should work ok with
non-3620 chips.

-- 
Bob Copeland %% http://bobcopeland.com/

[PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[Yin, Fengwei]

From: Andy Green <andy.green@linaro.org>

From: Andy Green <andy.green@linaro.org>

On wcn3620, firmware response to trigger_ba uses the new, larger
"v2" format

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index a84c2cc..f2443a0 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -1968,7 +1968,8 @@ int wcn36xx_smd_trigger_ba(struct wcn36xx *wcn, u8 sta_index)
 		wcn36xx_err("Sending hal_trigger_ba failed\n");
 		goto out;
 	}
-	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
+	ret = wcn36xx_smd_rsp_status_check_v2(wcn, wcn->hal_buf,
+						wcn->hal_rsp_len);
 	if (ret) {
 		wcn36xx_err("hal_trigger_ba response failed err=%d\n", ret);
 		goto out;
-- 
2.1.4

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[Bob Copeland]

On Mon, Nov 09, 2015 at 05:02:41AM -0500, Yin, Fengwei wrote:
> From: Andy Green <andy.green@linaro.org>
> 
> From: Andy Green <andy.green@linaro.org>
> 
> On wcn3620, firmware response to trigger_ba uses the new, larger
> "v2" format

> -	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
> +	ret = wcn36xx_smd_rsp_status_check_v2(wcn, wcn->hal_buf,
> +						wcn->hal_rsp_len);

It's unclear from the changelog -- is it safe to call
wcn36xx_smd_rsp_status_check_v2 on the 3660/3680 as well?

Is wcn36xx_smd_rsp_status_check() still needed?

-- 
Bob Copeland %% http://bobcopeland.com/

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[fengwei.yin]

Hi Bob,

On 2015/11/9 23:40, Bob Copeland wrote:
> On Mon, Nov 09, 2015 at 05:02:41AM -0500, Yin, Fengwei wrote:
>> From: Andy Green <andy.green@linaro.org>
>>
>> From: Andy Green <andy.green@linaro.org>
>>
>> On wcn3620, firmware response to trigger_ba uses the new, larger
>> "v2" format
>
>> -	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
>> +	ret = wcn36xx_smd_rsp_status_check_v2(wcn, wcn->hal_buf,
>> +						wcn->hal_rsp_len);
>
> It's unclear from the changelog -- is it safe to call
> wcn36xx_smd_rsp_status_check_v2 on the 3660/3680 as well?
>
> Is wcn36xx_smd_rsp_status_check() still needed?
Even on 3620, not all cmds switch to wcn36xx_smd_rsp_status_check_v2.
And I don't think wcn36xx_smd_rsp_status_check_v2 should bind to 3620 (it
may be related with firmware).

In patch v2, the wcn36xx_smd_rsp_status_check_v2 is not bind to 3620. If
you could test on none-3620 platform (I don't have none-3620 plafrom), that
will be great.

I will send out patch v2 soon which drive the build error and the comments
from you. Thanks a lot for reviewing.

Regards
Yin, Fengwei

>

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[Bob Copeland]

On Tue, Nov 10, 2015 at 03:08:53PM +0800, fengwei.yin wrote:
> Even on 3620, not all cmds switch to wcn36xx_smd_rsp_status_check_v2.
> And I don't think wcn36xx_smd_rsp_status_check_v2 should bind to 3620 (it
> may be related with firmware).
> 
> In patch v2, the wcn36xx_smd_rsp_status_check_v2 is not bind to 3620. If
> you could test on none-3620 platform (I don't have none-3620 plafrom), that
> will be great.

Sure, I'll take a look today or tomorrow on my hardware.

-- 
Bob Copeland %% http://bobcopeland.com/

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[fengwei.yin]

Hi Bob,

On 2015/11/10 22:13, Bob Copeland wrote:
> On Tue, Nov 10, 2015 at 03:08:53PM +0800, fengwei.yin wrote:
>> Even on 3620, not all cmds switch to wcn36xx_smd_rsp_status_check_v2.
>> And I don't think wcn36xx_smd_rsp_status_check_v2 should bind to 3620 (it
>> may be related with firmware).
>>
>> In patch v2, the wcn36xx_smd_rsp_status_check_v2 is not bind to 3620. If
>> you could test on none-3620 platform (I don't have none-3620 plafrom), that
>> will be great.
>
> Sure, I'll take a look today or tomorrow on my hardware.
Thanks a lot.

>

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[yfw]

Hi Bob,

On 2015年11月10日 22:13, Bob Copeland wrote:
> On Tue, Nov 10, 2015 at 03:08:53PM +0800, fengwei.yin wrote:
>> Even on 3620, not all cmds switch to wcn36xx_smd_rsp_status_check_v2.
>> And I don't think wcn36xx_smd_rsp_status_check_v2 should bind to 3620 (it
>> may be related with firmware).
>>
>> In patch v2, the wcn36xx_smd_rsp_status_check_v2 is not bind to 3620. If
>> you could test on none-3620 platform (I don't have none-3620 plafrom), that
>> will be great.
>
> Sure, I'll take a look today or tomorrow on my hardware.
>

Ping...

Regards
Yin, Fengwei

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[Bob Copeland]

On Thu, Nov 19, 2015 at 11:20:14AM +0800, yfw wrote:
> Hi Bob,
> 
> On 2015年11月10日 22:13, Bob Copeland wrote:
> >On Tue, Nov 10, 2015 at 03:08:53PM +0800, fengwei.yin wrote:
> >>Even on 3620, not all cmds switch to wcn36xx_smd_rsp_status_check_v2.
> >>And I don't think wcn36xx_smd_rsp_status_check_v2 should bind to 3620 (it
> >>may be related with firmware).
> >>
> >>In patch v2, the wcn36xx_smd_rsp_status_check_v2 is not bind to 3620. If
> >>you could test on none-3620 platform (I don't have none-3620 plafrom), that
> >>will be great.
> >
> >Sure, I'll take a look today or tomorrow on my hardware.
> >
> 
> Ping...

Oops, sorry.  I can confirm my hardware uses this format.

Turns out, in my backports snapshot I was already carrying a patch for it:

https://github.com/KrasnikovEugene/wcn36xx/commit/7d41270b8c3eee0f72713b4553ca047807c0a00e

-- 
Bob Copeland %% http://bobcopeland.com/

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[fengwei.yin]

Hi Bob,

On 2015/11/20 1:41, Bob Copeland wrote:
> On Thu, Nov 19, 2015 at 11:20:14AM +0800, yfw wrote:
>> Hi Bob,
>>
>> On 2015年11月10日 22:13, Bob Copeland wrote:
>>> On Tue, Nov 10, 2015 at 03:08:53PM +0800, fengwei.yin wrote:
>>>> Even on 3620, not all cmds switch to wcn36xx_smd_rsp_status_check_v2.
>>>> And I don't think wcn36xx_smd_rsp_status_check_v2 should bind to 3620 (it
>>>> may be related with firmware).
>>>>
>>>> In patch v2, the wcn36xx_smd_rsp_status_check_v2 is not bind to 3620. If
>>>> you could test on none-3620 platform (I don't have none-3620 plafrom), that
>>>> will be great.
>>>
>>> Sure, I'll take a look today or tomorrow on my hardware.
>>>
>>
>> Ping...
>
> Oops, sorry.  I can confirm my hardware uses this format.
>
> Turns out, in my backports snapshot I was already carrying a patch for it:
>
> https://github.com/KrasnikovEugene/wcn36xx/commit/7d41270b8c3eee0f72713b4553ca047807c0a00e
>

Actually, this patch was using the v1 and DragonBoard needs v2 for trigger_ba.
And I want to know whether the change from Andy could work on the platform
which is using v1.

Can you try this change on your platform? Thanks.

Regards
Yin, Fengwei

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[Bob Copeland]

On Fri, Nov 20, 2015 at 09:40:30AM +0800, fengwei.yin wrote:
> Actually, this patch was using the v1 and DragonBoard needs v2 for trigger_ba.
> And I want to know whether the change from Andy could work on the platform
> which is using v1.
> 
> Can you try this change on your platform? Thanks.

I'm not sure I understand -- if you look at the right side of the diff
for the patch "wcn36xx: Parse trigger_ba response properly", it is changing
wcn36xx_smd_rsp_status_check() to wcn36xx_smd_trigger_ba_rsp(), whose
definition is the same as wcn36xx_smd_rsp_station_check_v2() as far as I can
tell.  Or am I missing something?

-- 
Bob Copeland %% http://bobcopeland.com/

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[fengwei.yin]

On 2015/11/20 9:52, Bob Copeland wrote:
> On Fri, Nov 20, 2015 at 09:40:30AM +0800, fengwei.yin wrote:
>> Actually, this patch was using the v1 and DragonBoard needs v2 for trigger_ba.
>> And I want to know whether the change from Andy could work on the platform
>> which is using v1.
>>
>> Can you try this change on your platform? Thanks.
>
> I'm not sure I understand -- if you look at the right side of the diff
> for the patch "wcn36xx: Parse trigger_ba response properly", it is changing
> wcn36xx_smd_rsp_status_check() to wcn36xx_smd_trigger_ba_rsp(), whose
> definition is the same as wcn36xx_smd_rsp_station_check_v2() as far as I can
> tell.  Or am I missing something?
>
Yes. Exactly. So your platform is using the v2 format as well. We may not need
to care the v1 format too much. Thanks.

Regards
Yin, Fengwei

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[Bjorn Andersson]

On Mon, Nov 9, 2015 at 7:40 AM, Bob Copeland <me@bobcopeland.com> wrote:
> On Mon, Nov 09, 2015 at 05:02:41AM -0500, Yin, Fengwei wrote:
>> From: Andy Green <andy.green@linaro.org>
>>
>> From: Andy Green <andy.green@linaro.org>
>>
>> On wcn3620, firmware response to trigger_ba uses the new, larger
>> "v2" format
>
>> -     ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
>> +     ret = wcn36xx_smd_rsp_status_check_v2(wcn, wcn->hal_buf,
>> +                                             wcn->hal_rsp_len);
>
> It's unclear from the changelog -- is it safe to call
> wcn36xx_smd_rsp_status_check_v2 on the 3660/3680 as well?
>
> Is wcn36xx_smd_rsp_status_check() still needed?
>

I had to introduce this on one of my 3680 devices recently to silence
the error described originally by Andy. So it not only seems safe but
seems required. But still, based on how the code was written this
doesn't seem to be the case on all versions of the firmware or all
chips(?)

Regards,
Bjorn

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[fengwei.yin]

Hi Bjorn,

On 2015/11/12 12:50, Bjorn Andersson wrote:
> On Mon, Nov 9, 2015 at 7:40 AM, Bob Copeland <me@bobcopeland.com> wrote:
>> On Mon, Nov 09, 2015 at 05:02:41AM -0500, Yin, Fengwei wrote:
>>> From: Andy Green <andy.green@linaro.org>
>>>
>>> From: Andy Green <andy.green@linaro.org>
>>>
>>> On wcn3620, firmware response to trigger_ba uses the new, larger
>>> "v2" format
>>
>>> -     ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
>>> +     ret = wcn36xx_smd_rsp_status_check_v2(wcn, wcn->hal_buf,
>>> +                                             wcn->hal_rsp_len);
>>
>> It's unclear from the changelog -- is it safe to call
>> wcn36xx_smd_rsp_status_check_v2 on the 3660/3680 as well?
>>
>> Is wcn36xx_smd_rsp_status_check() still needed?
>>
>
> I had to introduce this on one of my 3680 devices recently to silence
> the error described originally by Andy. So it not only seems safe but
> seems required. But still, based on how the code was written this
> doesn't seem to be the case on all versions of the firmware or all
> chips(?)
>
Thanks for the information. It confirm my thought that the change sticks
to new firmware instead of specific platform.

But we couldn't tell which version of firmware need this new format. Andy's
original change has two conditions to use the new format:
1. The platform is 3620. - this should be removed because you need the same
    change for 3680. And patch v2 already remove it.

2. The packet size from firmware is larger than old response size. I suppose
    this one works in most case.

Regards
Yin, Fengwei

> Regards,
> Bjorn
>

Re: [PATCH 4/5] wcn3620: use new response format for wcn3620 trigger_ba

[Eugene Krasnikov]

Hi Yin,

Yes, it seems like response is FW specific other than chip spesific.

2015-11-12 6:37 GMT+00:00 fengwei.yin <fengwei.yin@linaro.org>:
> Hi Bjorn,
>
> On 2015/11/12 12:50, Bjorn Andersson wrote:
>>
>> On Mon, Nov 9, 2015 at 7:40 AM, Bob Copeland <me@bobcopeland.com> wrote:
>>>
>>> On Mon, Nov 09, 2015 at 05:02:41AM -0500, Yin, Fengwei wrote:
>>>>
>>>> From: Andy Green <andy.green@linaro.org>
>>>>
>>>> From: Andy Green <andy.green@linaro.org>
>>>>
>>>> On wcn3620, firmware response to trigger_ba uses the new, larger
>>>> "v2" format
>>>
>>>
>>>> -     ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf,
>>>> wcn->hal_rsp_len);
>>>> +     ret = wcn36xx_smd_rsp_status_check_v2(wcn, wcn->hal_buf,
>>>> +                                             wcn->hal_rsp_len);
>>>
>>>
>>> It's unclear from the changelog -- is it safe to call
>>> wcn36xx_smd_rsp_status_check_v2 on the 3660/3680 as well?
>>>
>>> Is wcn36xx_smd_rsp_status_check() still needed?
>>>
>>
>> I had to introduce this on one of my 3680 devices recently to silence
>> the error described originally by Andy. So it not only seems safe but
>> seems required. But still, based on how the code was written this
>> doesn't seem to be the case on all versions of the firmware or all
>> chips(?)
>>
> Thanks for the information. It confirm my thought that the change sticks
> to new firmware instead of specific platform.
>
> But we couldn't tell which version of firmware need this new format. Andy's
> original change has two conditions to use the new format:
> 1. The platform is 3620. - this should be removed because you need the same
>    change for 3680. And patch v2 already remove it.
>
> 2. The packet size from firmware is larger than old response size. I suppose
>    this one works in most case.
>
> Regards
> Yin, Fengwei
>
>> Regards,
>> Bjorn
>>
>



-- 
Best regards,
Eugene

[PATCH 5/5] wcn3620: use new response format for wcn3620 remove_bsskey

[Yin, Fengwei]

From: Andy Green <andy.green@linaro.org>

From: Andy Green <andy.green@linaro.org>

On wcn3620, firmware response to remove_bsskey uses the new, larger
"v2" format

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index f2443a0..0e22bae 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -1599,7 +1599,8 @@ int wcn36xx_smd_remove_bsskey(struct wcn36xx *wcn,
 		wcn36xx_err("Sending hal_remove_bsskey failed\n");
 		goto out;
 	}
-	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
+	ret = wcn36xx_smd_rsp_status_check_v2(wcn, wcn->hal_buf,
+					      wcn->hal_rsp_len);
 	if (ret) {
 		wcn36xx_err("hal_remove_bsskey response failed err=%d\n", ret);
 		goto out;
-- 
2.1.4