This patchset enables various firmware offload features for
- Link keepalive
- Link monitoring
Keepalive is a necessary precursor for an upcoming series on
wake on wlan, since we need to inform the firmware to keep the link up in
suspend.
Testing shows that LINK_FAIL_TX_CNT is a link monitor enable field. Once
set to non-zero link monitoring becomes active. This series activates
CONNECTION_MONITOR after enabling LINK_FAIL_TX_CNT thus offloading link
monitoring to the firmware.
Bryan O'Donoghue (3):
wcn36xx: Set LINK_FAIL_TX_CNT non zero on wcn3620/wcn3660
wcn36xx: Enable firmware link monitoring
wcn36xx: Enable firmware offloaded keepalive
drivers/net/wireless/ath/wcn36xx/main.c | 3 +++
drivers/net/wireless/ath/wcn36xx/smd.c | 1 +
2 files changed, 4 insertions(+)
--
2.28.0
This patch switches on CONNECTION_MONITOR. Once done it is up to the
firmware to send keep alive and to monitor the link state.
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
drivers/net/wireless/ath/wcn36xx/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 706728fba72d..e924cc4acde0 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1246,6 +1246,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
ieee80211_hw_set(wcn->hw, HAS_RATE_CONTROL);
ieee80211_hw_set(wcn->hw, SINGLE_SCAN_ON_ALL_BANDS);
ieee80211_hw_set(wcn->hw, REPORTS_TX_ACK_STATUS);
+ ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR);
wcn->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
BIT(NL80211_IFTYPE_AP) |
--
2.28.0
This patch calls wcn36xx_smd_keep_alive_req() on the STA patch immediately
after associating with an AP.
This will cause the firmware to send a NULL packet out to the AP every 30
seconds, thus offloading keep-alive processing from the SoC to the
firmware.
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
drivers/net/wireless/ath/wcn36xx/main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index e924cc4acde0..b514a7b952df 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -910,6 +910,8 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw,
* place where AID is available.
*/
wcn36xx_smd_config_sta(wcn, vif, sta);
+ wcn36xx_smd_keep_alive_req(wcn, vif,
+ WCN36XX_HAL_KEEP_ALIVE_NULL_PKT);
} else {
wcn36xx_dbg(WCN36XX_DBG_MAC,
"disassociated bss %pM vif %pM AID=%d\n",
--
2.28.0
The firmware parameter LINK_FAIL_TX_CNT maps to the prima configuration
file parameter gLinkFailTxCnt and is described as:
quote: " If within gLinkFailTimeout period(values is mentioned in msec) if
FW doesn't receive acks for gLinkFailTxCnt number of packets, then
link will be disconnected."
The downstream description sets a minimum value of 1000 a maximum value of
60000 and a default value of 6000, however it appears that unless we
actually set this value deliberately firmware defaults it to 0.
Setting this value to non-zero results in the firmware doing link
monitoring.
In conjunction with ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR); this
change effects offload of link monitoring to the firmware.
Tested with:
'CNSS-PR-2-0-1-2-c1-74-130449-3' wcn3620
'CNSS-PR-2-0-1-2-c1-00083' wcn3680
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
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 766400f7b61c..262978371c1f 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -78,6 +78,7 @@ static struct wcn36xx_cfg_val wcn36xx_cfg_vals[] = {
WCN36XX_CFG_VAL(MAX_ASSOC_LIMIT, 10),
WCN36XX_CFG_VAL(ENABLE_MCC_ADAPTIVE_SCHEDULER, 0),
WCN36XX_CFG_VAL(ENABLE_DYNAMIC_RA_START_RATE, 133), /* MCS 5 */
+ WCN36XX_CFG_VAL(LINK_FAIL_TX_CNT, 200),
};
static struct wcn36xx_cfg_val wcn3680_cfg_vals[] = {
--
2.28.0
HI Bryan,
On Sat, 31 Oct 2020 at 03:22, Bryan O'Donoghue
<[email protected]> wrote:
>
> The firmware parameter LINK_FAIL_TX_CNT maps to the prima configuration
> file parameter gLinkFailTxCnt and is described as:
>
> quote: " If within gLinkFailTimeout period(values is mentioned in msec) if
> FW doesn't receive acks for gLinkFailTxCnt number of packets, then
> link will be disconnected."
>
> The downstream description sets a minimum value of 1000 a maximum value of
> 60000 and a default value of 6000, however it appears that unless we
> actually set this value deliberately firmware defaults it to 0.
>
> Setting this value to non-zero results in the firmware doing link
> monitoring.
>
> In conjunction with ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR); this
> change effects offload of link monitoring to the firmware.
>
> Tested with:
> 'CNSS-PR-2-0-1-2-c1-74-130449-3' wcn3620
> 'CNSS-PR-2-0-1-2-c1-00083' wcn3680
>
> Signed-off-by: Bryan O'Donoghue <[email protected]>
> ---
> 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 766400f7b61c..262978371c1f 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -78,6 +78,7 @@ static struct wcn36xx_cfg_val wcn36xx_cfg_vals[] = {
> WCN36XX_CFG_VAL(MAX_ASSOC_LIMIT, 10),
> WCN36XX_CFG_VAL(ENABLE_MCC_ADAPTIVE_SCHEDULER, 0),
> WCN36XX_CFG_VAL(ENABLE_DYNAMIC_RA_START_RATE, 133), /* MCS 5 */
> + WCN36XX_CFG_VAL(LINK_FAIL_TX_CNT, 200),
Could you set the value to the minimum value described in the
downstream driver (i.e 1000)? Not sure a lower value is supported on
all wcn36xx firmware versions. So better to align with the
recommendations.
> };
>
> static struct wcn36xx_cfg_val wcn3680_cfg_vals[] = {
> --
> 2.28.0
>
On Sat, 31 Oct 2020 at 03:22, Bryan O'Donoghue
<[email protected]> wrote:
>
> This patch calls wcn36xx_smd_keep_alive_req() on the STA patch immediately
> after associating with an AP.
>
> This will cause the firmware to send a NULL packet out to the AP every 30
> seconds, thus offloading keep-alive processing from the SoC to the
> firmware.
>
> Signed-off-by: Bryan O'Donoghue <[email protected]>
> ---
> drivers/net/wireless/ath/wcn36xx/main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index e924cc4acde0..b514a7b952df 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -910,6 +910,8 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw,
> * place where AID is available.
> */
> wcn36xx_smd_config_sta(wcn, vif, sta);
> + wcn36xx_smd_keep_alive_req(wcn, vif,
> + WCN36XX_HAL_KEEP_ALIVE_NULL_PKT);
There is the wcn36xx_enable_keep_alive_null_packet function (from
pmc.c) that you can use.
> } else {
> wcn36xx_dbg(WCN36XX_DBG_MAC,
> "disassociated bss %pM vif %pM AID=%d\n",
> --
> 2.28.0
>
On Sat, 31 Oct 2020 at 03:22, Bryan O'Donoghue
<[email protected]> wrote:
>
> This patch switches on CONNECTION_MONITOR. Once done it is up to the
> firmware to send keep alive and to monitor the link state.
>
> Signed-off-by: Bryan O'Donoghue <[email protected]>
> ---
> drivers/net/wireless/ath/wcn36xx/main.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index 706728fba72d..e924cc4acde0 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -1246,6 +1246,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
> ieee80211_hw_set(wcn->hw, HAS_RATE_CONTROL);
> ieee80211_hw_set(wcn->hw, SINGLE_SCAN_ON_ALL_BANDS);
> ieee80211_hw_set(wcn->hw, REPORTS_TX_ACK_STATUS);
> + ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR);
The problem could be that when connection monitor is enabled, mac80211
stop sending regular null/probe packet to the AP (as expected), but
also stop monitoring beacon miss:
https://elixir.bootlin.com/linux/v5.10-rc1/source/net/mac80211/mlme.c#L115
That's not a big problem, but that would mean that in active mode
(power_save disabled, non PS), the mac80211 will not detect if the AP
has left immediately, and in worst case, only after 30 seconds. Note
that in PS mode, beacon monitoring is well done by the firmware.
>
> wcn->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
> BIT(NL80211_IFTYPE_AP) |
> --
> 2.28.0
>
On 31/10/2020 09:33, Loic Poulain wrote:
> Could you set the value to the minimum value described in the
> downstream driver (i.e 1000)?
Agreed.
Seems reasonable
On 31/10/2020 09:39, Loic Poulain wrote:
> On Sat, 31 Oct 2020 at 03:22, Bryan O'Donoghue
> <[email protected]> wrote:
>> + wcn36xx_smd_keep_alive_req(wcn, vif,
>> + WCN36XX_HAL_KEEP_ALIVE_NULL_PKT);
>
> There is the wcn36xx_enable_keep_alive_null_packet function (from
> pmc.c) that you can use.
Sure
On 31/10/2020 09:57, Loic Poulain wrote:
> On Sat, 31 Oct 2020 at 03:22, Bryan O'Donoghue
> <[email protected]> wrote:
>>
>> This patch switches on CONNECTION_MONITOR. Once done it is up to the
>> firmware to send keep alive and to monitor the link state.
>>
>> Signed-off-by: Bryan O'Donoghue <[email protected]>
>> ---
>> drivers/net/wireless/ath/wcn36xx/main.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
>> index 706728fba72d..e924cc4acde0 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/main.c
>> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
>> @@ -1246,6 +1246,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>> ieee80211_hw_set(wcn->hw, HAS_RATE_CONTROL);
>> ieee80211_hw_set(wcn->hw, SINGLE_SCAN_ON_ALL_BANDS);
>> ieee80211_hw_set(wcn->hw, REPORTS_TX_ACK_STATUS);
>> + ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR);
>
> The problem could be that when connection monitor is enabled, mac80211
> stop sending regular null/probe packet to the AP (as expected), but
> also stop monitoring beacon miss:
> https://elixir.bootlin.com/linux/v5.10-rc1/source/net/mac80211/mlme.c#L115
>
> That's not a big problem, but that would mean that in active mode
> (power_save disabled, non PS), the mac80211 will not detect if the AP
> has left immediately, and in worst case, only after 30 seconds. Note
> that in PS mode, beacon monitoring is well done by the firmware.
>
If you pull the plug out of the AP it can take up to 30 seconds to see
it agreed.
On the flip side, the amount of NULL data packets produced drops off
significantly once we delegate this completely to the firmware.
IMO you gain more by reducing the regular runtime noise than you loose
with the timing out of an gone away AP.
On Sat, 31 Oct 2020 at 14:01, Bryan O'Donoghue
<[email protected]> wrote:
>
> On 31/10/2020 09:57, Loic Poulain wrote:
> > On Sat, 31 Oct 2020 at 03:22, Bryan O'Donoghue
> > <[email protected]> wrote:
> >>
> >> This patch switches on CONNECTION_MONITOR. Once done it is up to the
> >> firmware to send keep alive and to monitor the link state.
> >>
> >> Signed-off-by: Bryan O'Donoghue <[email protected]>
> >> ---
> >> drivers/net/wireless/ath/wcn36xx/main.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> >> index 706728fba72d..e924cc4acde0 100644
> >> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> >> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> >> @@ -1246,6 +1246,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
> >> ieee80211_hw_set(wcn->hw, HAS_RATE_CONTROL);
> >> ieee80211_hw_set(wcn->hw, SINGLE_SCAN_ON_ALL_BANDS);
> >> ieee80211_hw_set(wcn->hw, REPORTS_TX_ACK_STATUS);
> >> + ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR);
> >
> > The problem could be that when connection monitor is enabled, mac80211
> > stop sending regular null/probe packet to the AP (as expected), but
> > also stop monitoring beacon miss:
> > https://elixir.bootlin.com/linux/v5.10-rc1/source/net/mac80211/mlme.c#L115
> >
> > That's not a big problem, but that would mean that in active mode
> > (power_save disabled, non PS), the mac80211 will not detect if the AP
> > has left immediately, and in worst case, only after 30 seconds. Note
> > that in PS mode, beacon monitoring is well done by the firmware.
> >
>
> If you pull the plug out of the AP it can take up to 30 seconds to see
> it agreed.
>
> On the flip side, the amount of NULL data packets produced drops off
> significantly once we delegate this completely to the firmware.
>
> IMO you gain more by reducing the regular runtime noise than you loose
> with the timing out of an gone away AP.
OK, fair enough!
Loic