diff mbox

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

Message ID 1447063362-27322-4-git-send-email-fengwei.yin@linaro.org (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show

Commit Message

Fengwei Yin Nov. 9, 2015, 10:02 a.m. UTC 
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(+)

Comments

kernel test robot Nov. 9, 2015, 9:25 a.m. UTC | #1 
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
Fengwei Yin Nov. 9, 2015, 9:29 a.m. UTC | #2 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
kernel test robot Nov. 9, 2015, 10:22 a.m. UTC | #3 
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
Bob Copeland Nov. 9, 2015, 3:43 p.m. UTC | #4 
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.
diff mbox

Patch

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;