RE: [PATCH wireless 2/2] wifi: rtw88: restore the control frame filter when the device is started

[Ping-Ke Shih <pkshih@realtek.com>]

Jeremy Fareau <jeremy.fareau@gmail.com> wrote:
> Commit ed51a86b787f ("wifi: rtw88: Enable receiving control frames in
> monitor mode") writes REG_RXFLTMAP1 from configure_filter(),

Using LLM is okay, but please reword yourself. I don't think this sentence
is necessary.

> but that
> register is reset to the chip default by the chip's mac_init() during
> power_on().

Maybe the default value of rtwdev->hal.rxfltmap1 should be set at
rtw_core_init()? But I don't have clear thought how to switch between
normal and monitor modes (or even sta + monitor mode). 

>  mac80211 only calls ieee80211_ops::configure_filter() when
> its own filter flags change, so after a stop/start cycle the flags are
> unchanged and the control frame filter silently reverts: monitor mode
> stops delivering control frames again.

This problem is also only encountered in monitor mode, right?
Let's do it under monitor mode condition?

> 
> Keep track of the value configure_filter() last asked for and restore
> it in rtw_core_start(), next to the RCR restore, which exists for the
> same reason.
> 
> Measured on an RTL8814AU and an RTL8812AU in monitor mode on channel 6,
> 40 s captures taken after a down/up cycle.  Control frames go from 0 to
> 23184 and from 0 to 483 respectively; without this change both are 0.
> 
> Fixes: ed51a86b787f ("wifi: rtw88: Enable receiving control frames in monitor mode")
> Assisted-by: LLM
> Signed-off-by: Jeremy Fareau <jeremy.fareau@gmail.com>
> ---
>  drivers/net/wireless/realtek/rtw88/mac80211.c | 5 +++--
>  drivers/net/wireless/realtek/rtw88/main.c     | 8 ++++++++
>  drivers/net/wireless/realtek/rtw88/main.h     | 1 +
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c
> b/drivers/net/wireless/realtek/rtw88/mac80211.c
> index 827f38390969..84d869f1efe0 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac80211.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
> @@ -300,9 +300,10 @@ static void rtw_ops_configure_filter(struct ieee80211_hw *hw,
> 
>         if (changed_flags & FIF_CONTROL) {
>                 if (*new_flags & FIF_CONTROL)
> -                       rtw_write16(rtwdev, REG_RXFLTMAP1, 0xffff);

REG_RXFLTMAP1 should be 0xffff in monitor mode, right?
Maybe, just set the value in monitor mode. No need rxfltmap1_cur to store
current value. Maintaining two variables is hard and confusing...

> +                       rtwdev->hal.rxfltmap1_cur = 0xffff;
>                 else
> -                       rtw_write16(rtwdev, REG_RXFLTMAP1, rtwdev->hal.rxfltmap1);
> +                       rtwdev->hal.rxfltmap1_cur = rtwdev->hal.rxfltmap1;
> +               rtw_write16(rtwdev, REG_RXFLTMAP1, rtwdev->hal.rxfltmap1_cur);
>         }
>         if (changed_flags & FIF_ALLMULTI) {
>                 if (*new_flags & FIF_ALLMULTI)