Re: [PATCH] Revert "wcn36xx: Enable firmware link monitoring"

[Kalle Valo <kvalo@codeaurora.org>]

Loic Poulain <loic.poulain@linaro.org> writes:

> Hi Bryan,
>
> On Tue, 31 Aug 2021 at 03:13, Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
>
>  On 30/08/2021 18:09, Loic Poulain wrote:
>  > This reverts commit 8def9ec46a5fafc0abcf34489a9e8a787bca984d.
>  > 
>  > The firmware keep-alive does not cause any event in case of error
>  > such as non acked. It's just a basic keep alive to prevent the AP
>  > to kick-off the station due to inactivity. So let mac80211 submit
>  > its own monitoring packet (probe/null) and disconnect on timeout.
>  > 
>  > Note: We want to keep firmware keep alive to prevent kick-off
>  > when host is in suspend-to-mem (no mac80211 monitor packet).
>  > Ideally fw keep alive should be enabled in suspend path and disabled
>  > in resume path to prevent having both firmware and mac80211 submitting
>  > periodic null packets.
>  > 
>  > This fixes non detected AP leaving issues in active mode (nothing
>  > monitors beacon or connection).
>  > 
>  > Cc: stable@vger.kernel.org
>  > Fixes: 8def9ec46a5f ("wcn36xx: Enable firmware link monitoring")
>  > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
>  > ---
>  >   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 216bc34..128d25d 100644
>  > --- a/drivers/net/wireless/ath/wcn36xx/main.c
>  > +++ b/drivers/net/wireless/ath/wcn36xx/main.c
>  > @@ -1362,7 +1362,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) |
>  > 
>
>  But why is BSS heartbeat offload not running, it should be.
>
>  I agree we should switch this bit off for now since its obviously not 
>  working as intended.
>
>  But we need to root cause _why_
>
> I think it has just not be designed as a connection tracking mechanism but as a simple
> keep alive, which is submitted every 30s unconditionally.
>
>  In suspend absent a working heartbeat monitor - if the AP goes away we 
>  stay in suspend indefinitely.
>
> We shouldn't because the firmware is monitoring beacons and would cause a beacon miss
> indication, waking up the host.

No HTML please, our lists drop emails using HTML.

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

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