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
On 2015/11/9 18:02, Yin, Fengwei wrote:
> From: Andy Green <[email protected]>
>
> From: Andy Green <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
On Mon, Nov 09, 2015 at 05:02:40AM -0500, Yin, Fengwei wrote:
>
> From: Andy Green <[email protected]>
>
> 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/
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.
>
From: Andy Green <[email protected]>
From: Andy Green <[email protected]>
On wcn3620, firmware response to remove_bsskey uses the new, larger
"v2" format
Signed-off-by: Andy Green <[email protected]>
---
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
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
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
On Mon, Nov 09, 2015 at 05:02:41AM -0500, Yin, Fengwei wrote:
> From: Andy Green <[email protected]>
>
> From: Andy Green <[email protected]>
>
> 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/
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 <[email protected]>
> ---
^^^
... should go here (after the ---), so that it doesn't appear in the final
changelog.
--
Bob Copeland %% http://bobcopeland.com/
From: Andy Green <[email protected]>
From: Andy Green <[email protected]>
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 <[email protected]>
---
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
From: Andy Green <[email protected]>
From: Andy Green <[email protected]>
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 <[email protected]>
---
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
From: Andy Green <[email protected]>
From: Andy Green <[email protected]>
On wcn3620, firmware response to trigger_ba uses the new, larger
"v2" format
Signed-off-by: Andy Green <[email protected]>
---
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
On Mon, Nov 9, 2015 at 7:40 AM, Bob Copeland <[email protected]> wrote:
> On Mon, Nov 09, 2015 at 05:02:41AM -0500, Yin, Fengwei wrote:
>> From: Andy Green <[email protected]>
>>
>> From: Andy Green <[email protected]>
>>
>> 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
On Mon, Nov 09, 2015 at 05:02:38AM -0500, Yin, Fengwei wrote:
> From: Andy Green <[email protected]>
>
> From: Andy Green <[email protected]>
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/
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/
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
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
From: Andy Green <[email protected]>
From: Andy Green <[email protected]>
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 <[email protected]>
---
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
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 <[email protected]>
>>
>> From: Andy Green <[email protected]>
>>
>> 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
>
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
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/
Hi Bjorn,
On 2015/11/12 12:50, Bjorn Andersson wrote:
> On Mon, Nov 9, 2015 at 7:40 AM, Bob Copeland <[email protected]> wrote:
>> On Mon, Nov 09, 2015 at 05:02:41AM -0500, Yin, Fengwei wrote:
>>> From: Andy Green <[email protected]>
>>>
>>> From: Andy Green <[email protected]>
>>>
>>> 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
>
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/
Hi Yin,
Yes, it seems like response is FW specific other than chip spesific.
2015-11-12 6:37 GMT+00:00 fengwei.yin <[email protected]>:
> Hi Bjorn,
>
> On 2015/11/12 12:50, Bjorn Andersson wrote:
>>
>> On Mon, Nov 9, 2015 at 7:40 AM, Bob Copeland <[email protected]> wrote:
>>>
>>> On Mon, Nov 09, 2015 at 05:02:41AM -0500, Yin, Fengwei wrote:
>>>>
>>>> From: Andy Green <[email protected]>
>>>>
>>>> From: Andy Green <[email protected]>
>>>>
>>>> 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