* [PATCH] wcn36xx: Don't run scan_init multiple times
@ 2020-11-20 2:14 Bryan O'Donoghue
2020-11-20 2:14 ` [PATCH] wcn36xx: Send NULL data packet when exiting BMPS Bryan O'Donoghue
2020-11-20 8:12 ` [PATCH] wcn36xx: Don't run scan_init multiple times Loic Poulain
0 siblings, 2 replies; 7+ messages in thread
From: Bryan O'Donoghue @ 2020-11-20 2:14 UTC (permalink / raw)
To: kvalo, wcn36xx, linux-wireless
Cc: bryan.odonoghue, shawn.guo, loic.poulain, benl
Run scan_init only once. There's no need to run this command multiple times
if it has already been run once.
The software scan algorithm can end up repeatedly calling scan_init on each
loop resulting in between four and eight milliseconds of lost time on each
callout.
Subtract the overhead now.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/net/wireless/ath/wcn36xx/smd.c | 6 ++++++
drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index acf533fae46a..ec082cf3ab09 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -706,6 +706,10 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode,
int ret;
mutex_lock(&wcn->hal_mutex);
+ if (wcn->scan_init) {
+ ret = 0;
+ goto out;
+ }
INIT_HAL_MSG(msg_body, WCN36XX_HAL_INIT_SCAN_REQ);
msg_body.mode = mode;
@@ -731,6 +735,7 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode,
wcn36xx_err("hal_init_scan response failed err=%d\n", ret);
goto out;
}
+ wcn->scan_init = true;
out:
mutex_unlock(&wcn->hal_mutex);
return ret;
@@ -761,6 +766,7 @@ int wcn36xx_smd_start_scan(struct wcn36xx *wcn, u8 scan_channel)
wcn36xx_err("hal_start_scan response failed err=%d\n", ret);
goto out;
}
+ wcn->scan_init = false;
out:
mutex_unlock(&wcn->hal_mutex);
return ret;
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index 71fa9992b118..156df6d184c8 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -235,6 +235,7 @@ struct wcn36xx {
struct ieee80211_vif *sw_scan_vif;
struct mutex scan_lock;
bool scan_aborted;
+ bool scan_init;
/* DXE channels */
struct wcn36xx_dxe_ch dxe_tx_l_ch; /* TX low */
--
2.28.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] wcn36xx: Send NULL data packet when exiting BMPS
2020-11-20 2:14 [PATCH] wcn36xx: Don't run scan_init multiple times Bryan O'Donoghue
@ 2020-11-20 2:14 ` Bryan O'Donoghue
2020-11-20 8:15 ` Loic Poulain
2020-12-02 18:33 ` Kalle Valo
2020-11-20 8:12 ` [PATCH] wcn36xx: Don't run scan_init multiple times Loic Poulain
1 sibling, 2 replies; 7+ messages in thread
From: Bryan O'Donoghue @ 2020-11-20 2:14 UTC (permalink / raw)
To: kvalo, wcn36xx, linux-wireless
Cc: bryan.odonoghue, shawn.guo, loic.poulain, benl
This commit updates the BMPS exit path to be consistent with downstream in
terms of exiting BMPS mode. Downstream sets the flag to send a NULL data
frame to the host on exiting BMPS.
This will tell the AP to send any queued frames to the STA immediately.
Verified the relevant bit toggle in wireshark.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/net/wireless/ath/wcn36xx/smd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 8ff1eda8f942..acf533fae46a 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2176,6 +2176,7 @@ int wcn36xx_smd_exit_bmps(struct wcn36xx *wcn, struct ieee80211_vif *vif)
INIT_HAL_MSG(msg_body, WCN36XX_HAL_EXIT_BMPS_REQ);
msg_body.bss_index = vif_priv->bss_index;
+ msg_body.send_data_null = 1;
PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
--
2.28.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] wcn36xx: Don't run scan_init multiple times
2020-11-20 2:14 [PATCH] wcn36xx: Don't run scan_init multiple times Bryan O'Donoghue
2020-11-20 2:14 ` [PATCH] wcn36xx: Send NULL data packet when exiting BMPS Bryan O'Donoghue
@ 2020-11-20 8:12 ` Loic Poulain
2020-11-20 11:45 ` Bryan O'Donoghue
1 sibling, 1 reply; 7+ messages in thread
From: Loic Poulain @ 2020-11-20 8:12 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Kalle Valo, wcn36xx, linux-wireless, Shawn Guo, Benjamin Li
On Fri, 20 Nov 2020 at 03:13, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> Run scan_init only once. There's no need to run this command multiple times
> if it has already been run once.
>
> The software scan algorithm can end up repeatedly calling scan_init on each
> loop resulting in between four and eight milliseconds of lost time on each
> callout.
>
> Subtract the overhead now.
This command defines parameters like the BSSID we want to inform,
etc... So this can change depending on the scan is done while
connected or not. Moreover in the connected case, the scans are
interleaved with normal data listening period, and AFAIU, init/stop
scan allow to submit a null data packet with PS/non-PS bit when
mac80211 leaves the operating channel to scanning another one (so that
AP does no submit packet to it). So at first glance, this patch would
break that, right?
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> drivers/net/wireless/ath/wcn36xx/smd.c | 6 ++++++
> drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index acf533fae46a..ec082cf3ab09 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -706,6 +706,10 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode,
> int ret;
>
> mutex_lock(&wcn->hal_mutex);
> + if (wcn->scan_init) {
> + ret = 0;
> + goto out;
> + }
> INIT_HAL_MSG(msg_body, WCN36XX_HAL_INIT_SCAN_REQ);
>
> msg_body.mode = mode;
> @@ -731,6 +735,7 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode,
> wcn36xx_err("hal_init_scan response failed err=%d\n", ret);
> goto out;
> }
> + wcn->scan_init = true;
> out:
> mutex_unlock(&wcn->hal_mutex);
> return ret;
> @@ -761,6 +766,7 @@ int wcn36xx_smd_start_scan(struct wcn36xx *wcn, u8 scan_channel)
> wcn36xx_err("hal_start_scan response failed err=%d\n", ret);
> goto out;
> }
> + wcn->scan_init = false;
> out:
> mutex_unlock(&wcn->hal_mutex);
> return ret;
> diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> index 71fa9992b118..156df6d184c8 100644
> --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> @@ -235,6 +235,7 @@ struct wcn36xx {
> struct ieee80211_vif *sw_scan_vif;
> struct mutex scan_lock;
> bool scan_aborted;
> + bool scan_init;
>
> /* DXE channels */
> struct wcn36xx_dxe_ch dxe_tx_l_ch; /* TX low */
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] wcn36xx: Send NULL data packet when exiting BMPS
2020-11-20 2:14 ` [PATCH] wcn36xx: Send NULL data packet when exiting BMPS Bryan O'Donoghue
@ 2020-11-20 8:15 ` Loic Poulain
2020-11-20 11:58 ` Bryan O'Donoghue
2020-12-02 18:33 ` Kalle Valo
1 sibling, 1 reply; 7+ messages in thread
From: Loic Poulain @ 2020-11-20 8:15 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Kalle Valo, wcn36xx, linux-wireless, Shawn Guo, Benjamin Li
On Fri, 20 Nov 2020 at 03:13, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> This commit updates the BMPS exit path to be consistent with downstream in
> terms of exiting BMPS mode. Downstream sets the flag to send a NULL data
> frame to the host on exiting BMPS.
>
> This will tell the AP to send any queued frames to the STA immediately.
> Verified the relevant bit toggle in wireshark.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> drivers/net/wireless/ath/wcn36xx/smd.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index 8ff1eda8f942..acf533fae46a 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -2176,6 +2176,7 @@ int wcn36xx_smd_exit_bmps(struct wcn36xx *wcn, struct ieee80211_vif *vif)
> INIT_HAL_MSG(msg_body, WCN36XX_HAL_EXIT_BMPS_REQ);
>
> msg_body.bss_index = vif_priv->bss_index;
> + msg_body.send_data_null = 1;
I'm quite sure I've seen null data packet wakeup (PS=0) when sniffing
wcn3620, but maybe it was submitted by mac80211, have you then checked
you do not end with double null packets with that patch (one from
firmware and one from mac layer)?
>
> PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
>
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] wcn36xx: Don't run scan_init multiple times
2020-11-20 8:12 ` [PATCH] wcn36xx: Don't run scan_init multiple times Loic Poulain
@ 2020-11-20 11:45 ` Bryan O'Donoghue
0 siblings, 0 replies; 7+ messages in thread
From: Bryan O'Donoghue @ 2020-11-20 11:45 UTC (permalink / raw)
To: Loic Poulain; +Cc: Kalle Valo, wcn36xx, linux-wireless, Shawn Guo, Benjamin Li
On 20/11/2020 08:12, Loic Poulain wrote:
> On Fri, 20 Nov 2020 at 03:13, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> Run scan_init only once. There's no need to run this command multiple times
>> if it has already been run once.
>>
>> The software scan algorithm can end up repeatedly calling scan_init on each
>> loop resulting in between four and eight milliseconds of lost time on each
>> callout.
>>
>> Subtract the overhead now.
>
> This command defines parameters like the BSSID we want to inform,
> etc... So this can change depending on the scan is done while
> connected or not.
So you're saying a scan is started and our connection state toggles from
non-connected to connected.
Possible I guess.
> Moreover in the connected case, the scans are
> interleaved with normal data listening period, and AFAIU, init/stop
> scan allow to submit a null data packet with PS/non-PS bit when
> mac80211 leaves the operating channel to scanning another one (so that
> AP does no submit packet to it). So at first glance, this patch would
> break that, right?
I agree with that logic, and actually looking at downstream - we see
that downstream doesn't set the notification byte before starting a scan
on a new channel
connected:
[ 63.475897] BOD WDI_SendMsg/23794 message = 0x04 version = 0x01 len
0x00000030 // WLAN_HAL_INIT_SCAN_REQ
[ 63.475902] SMD <<< 00000000: 04 00 01 00 30 00 00 00 02 00 00 00 00
00 00 00 00 00 00 00 00 ff ff ff c8 9f 7a 21 c0 ff ff ff
[ 63.475907] SMD <<< 00000020: d4 2e 7c 00 c0 ff ff ff 00 54 93 6d c0
22 22 00
[ 63.478242] SMD >>> 00000000: 05 00 00 00 0c 00 00 00 00 00 00 00
type=04 00 version=01 00 length=30 00 00 00
mode=02 00 00 00
bssid=00 000 00 00 00 00
notify=00
...
which I accept is actually a bug downstream, conceptually at any rate.
I need to ensure a scan isn't in process when we go into suspend but,
there's no reason to skip the scan_init() command for that.
flagging != skipping
Let's forget this one.
---
bod
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] wcn36xx: Send NULL data packet when exiting BMPS
2020-11-20 8:15 ` Loic Poulain
@ 2020-11-20 11:58 ` Bryan O'Donoghue
0 siblings, 0 replies; 7+ messages in thread
From: Bryan O'Donoghue @ 2020-11-20 11:58 UTC (permalink / raw)
To: Loic Poulain; +Cc: Kalle Valo, wcn36xx, linux-wireless, Shawn Guo, Benjamin Li
On 20/11/2020 08:15, Loic Poulain wrote:
> On Fri, 20 Nov 2020 at 03:13, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> This commit updates the BMPS exit path to be consistent with downstream in
>> terms of exiting BMPS mode. Downstream sets the flag to send a NULL data
>> frame to the host on exiting BMPS.
>>
>> This will tell the AP to send any queued frames to the STA immediately.
>> Verified the relevant bit toggle in wireshark.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>> drivers/net/wireless/ath/wcn36xx/smd.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
>> index 8ff1eda8f942..acf533fae46a 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
>> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
>> @@ -2176,6 +2176,7 @@ int wcn36xx_smd_exit_bmps(struct wcn36xx *wcn, struct ieee80211_vif *vif)
>> INIT_HAL_MSG(msg_body, WCN36XX_HAL_EXIT_BMPS_REQ);
>>
>> msg_body.bss_index = vif_priv->bss_index;
>> + msg_body.send_data_null = 1;
>
> I'm quite sure I've seen null data packet wakeup (PS=0) when sniffing
> wcn3620, but maybe it was submitted by mac80211, have you then checked
> you do not end with double null packets with that patch (one from
> firmware and one from mac layer)?
Just did.
Definitely not double nulling
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] wcn36xx: Send NULL data packet when exiting BMPS
2020-11-20 2:14 ` [PATCH] wcn36xx: Send NULL data packet when exiting BMPS Bryan O'Donoghue
2020-11-20 8:15 ` Loic Poulain
@ 2020-12-02 18:33 ` Kalle Valo
1 sibling, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2020-12-02 18:33 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: wcn36xx, linux-wireless, bryan.odonoghue, shawn.guo, loic.poulain, benl
Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> This commit updates the BMPS exit path to be consistent with downstream in
> terms of exiting BMPS mode. Downstream sets the flag to send a NULL data
> frame to the host on exiting BMPS.
>
> This will tell the AP to send any queued frames to the STA immediately.
> Verified the relevant bit toggle in wireshark.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
9bc3a55f4ae5 wcn36xx: Send NULL data packet when exiting BMPS
--
https://patchwork.kernel.org/project/linux-wireless/patch/20201120021403.2646574-2-bryan.odonoghue@linaro.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-02 18:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 2:14 [PATCH] wcn36xx: Don't run scan_init multiple times Bryan O'Donoghue
2020-11-20 2:14 ` [PATCH] wcn36xx: Send NULL data packet when exiting BMPS Bryan O'Donoghue
2020-11-20 8:15 ` Loic Poulain
2020-11-20 11:58 ` Bryan O'Donoghue
2020-12-02 18:33 ` Kalle Valo
2020-11-20 8:12 ` [PATCH] wcn36xx: Don't run scan_init multiple times Loic Poulain
2020-11-20 11:45 ` Bryan O'Donoghue
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.