2021-10-22 14:10:28

by Bryan O'Donoghue

[permalink] [raw]
Subject: [PATCH 0/1] wcn36xx: Revert firmware link monitoring

We can get this working with

- Secure APs PS on/off
- Open APs PS on

by setting firmware feature bit WLANACTIVE_OFFLOAD

Open APs PS off though is non-functional even with the above bit enabled
and three quaters of a wheel is useless.

So unfortunately for now zap it off.

Bryan O'Donoghue (1):
Revert "wcn36xx: Enable firmware link monitoring"

drivers/net/wireless/ath/wcn36xx/main.c | 1 -
1 file changed, 1 deletion(-)

--
2.33.0


2021-10-22 14:11:37

by Bryan O'Donoghue

[permalink] [raw]
Subject: [PATCH 1/1] Revert "wcn36xx: Enable firmware link monitoring"

This reverts commit c973fdad79f6eaf247d48b5fc77733e989eb01e1.

Signed-off-by: Bryan O'Donoghue <[email protected]>
Signed-off-by: Benjamin Li <[email protected]>
---
drivers/net/wireless/ath/wcn36xx/main.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index dd8810fd76a3d..fd8b2753da7dd 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1334,7 +1334,6 @@ 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.33.0

2021-10-22 14:55:12

by Loic Poulain

[permalink] [raw]
Subject: Re: [PATCH 1/1] Revert "wcn36xx: Enable firmware link monitoring"

On Fri, 22 Oct 2021 at 16:08, Bryan O'Donoghue
<[email protected]> wrote:
>
> This reverts commit c973fdad79f6eaf247d48b5fc77733e989eb01e1.
>
> Signed-off-by: Bryan O'Donoghue <[email protected]>
> Signed-off-by: Benjamin Li <[email protected]>

Should it get a 'Fixes' tag?

Regards,
Loic

> ---
> drivers/net/wireless/ath/wcn36xx/main.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index dd8810fd76a3d..fd8b2753da7dd 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -1334,7 +1334,6 @@ 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.33.0
>

2021-10-25 08:54:28

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/1] wcn36xx: Revert firmware link monitoring

Bryan O'Donoghue <[email protected]> writes:

> We can get this working with
>
> - Secure APs PS on/off
> - Open APs PS on
>
> by setting firmware feature bit WLANACTIVE_OFFLOAD
>
> Open APs PS off though is non-functional even with the above bit enabled
> and three quaters of a wheel is useless.
>
> So unfortunately for now zap it off.

Avoid using acronyms as much as possible, PS meaning power save might
not be clear for everyone.

And please try to be more specific, I'm not sure what power save feature
you are meaning here. (802.11 protocol power save, BMPS or what?)

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-10-25 08:56:56

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/1] Revert "wcn36xx: Enable firmware link monitoring"

Loic Poulain <[email protected]> writes:

> On Fri, 22 Oct 2021 at 16:08, Bryan O'Donoghue
> <[email protected]> wrote:
>>
>> This reverts commit c973fdad79f6eaf247d48b5fc77733e989eb01e1.
>>
>> Signed-off-by: Bryan O'Donoghue <[email protected]>
>> Signed-off-by: Benjamin Li <[email protected]>
>
> Should it get a 'Fixes' tag?

But this patch is not really fixing anything, right? I wonder if there's
patch 2 missing?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-10-25 08:56:59

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/1] Revert "wcn36xx: Enable firmware link monitoring"

Bryan O'Donoghue <[email protected]> writes:

> This reverts commit c973fdad79f6eaf247d48b5fc77733e989eb01e1.
>
> Signed-off-by: Bryan O'Donoghue <[email protected]>

The commit log should answer to question "why?". Please move the
explanation from the cover email to the commit log.

> Signed-off-by: Benjamin Li <[email protected]>

Why Benjamin's s-o-b?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-10-25 09:09:51

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH 0/1] wcn36xx: Revert firmware link monitoring

On 25/10/2021 09:52, Kalle Valo wrote:
> Avoid using acronyms as much as possible, PS meaning power save might
> not be clear for everyone.
>
> And please try to be more specific, I'm not sure what power save feature
> you are meaning here. (802.11 protocol power save, BMPS or what?)

Yep - the issue is on an Open AP when we enter BMPS link monitoring
stops working.

I'll make that clear in a resend