mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Luka Gejak" <luka.gejak@linux.dev>
To: "Jeremy Fareau" <jeremy.fareau@gmail.com>,
	"Ping-Ke Shih" <pkshih@realtek.com>
Cc: <linux-wireless@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	"Kalle Valo" <kvalo@kernel.org>,
	"Brian Norris" <briannorris@chromium.org>
Subject: Re: [PATCH wireless 2/2] wifi: rtw88: restore the control frame filter when the device is started
Date: Sun, 11 Oct 2026 13:08:39 +0000	[thread overview]
Message-ID: <DM216XAEDYAN.IZDPPZ4YWPLG@linux.dev> (raw)
In-Reply-To: <20261007091225.413-3-jeremy.fareau@gmail.com>

On Wed Oct 7, 2026 at 9:12 AM UTC, Jeremy Fareau wrote:
> Commit ed51a86b787f ("wifi: rtw88: Enable receiving control frames in
> monitor mode") writes REG_RXFLTMAP1 from configure_filter(), but that
> register is reset to the chip default by the chip's mac_init() during
> power_on().  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.
> 
> 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")

Please add Cc: stable@vger.kernel.org

> 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);
> +			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) {

I would change the new rxfltmap1_cur because hal->rxfltmap1 is not a constant.
The beamforming code edits it and writes the register with the
edited value:

          bf.c:140        rtwdev->hal.rxfltmap1 &= ~BIT_RXFLTMAP1_BF_REPORT_POLL;
          bf.c:141        rtw_write16(rtwdev, REG_RXFLTMAP1, rtwdev->hal.rxfltmap1);
          bf.c:273        rtwdev->hal.rxfltmap1 |= BIT_RXFLTMAP1_BF;
          bf.c:274        rtw_write16(rtwdev, REG_RXFLTMAP1, rtwdev->hal.rxfltmap1);

So after this patch the driver keeps two cached copies of one register. On an
8822C in monitor mode, configure_filter() caches 0xffff while bf.c has cleared
the report poll bit in hal->rxfltmap1, so the next stop/start restores 0xffff
and undoes that. The reverse happens too: a request with FIF_CONTROL off
caches hal->rxfltmap1 as it stands at that moment, and later bf.c edits are
lost at the next restart.

I would keep one variable and add one flag instead of a second shadow value.
It would remember whether control frames were requested, and then on restore
it would rebuild the register from that flag and hal->rxfltmap1.

> diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
> index cd9254370fcc..ea61020a0622 100644
> --- a/drivers/net/wireless/realtek/rtw88/main.c
> +++ b/drivers/net/wireless/realtek/rtw88/main.c
> @@ -1528,6 +1528,14 @@ int rtw_core_start(struct rtw_dev *rtwdev)
>  	/* rcr reset after powered on */
>  	rtw_write32(rtwdev, REG_RCR, rtwdev->hal.rcr);
>  
> +	/* REG_RXFLTMAP1 is reset by the chip's mac_init() during power_on(),
> +	 * and mac80211 only calls ieee80211_ops::configure_filter() when its
> +	 * own filter flags change.  After a stop/start cycle the flags are
> +	 * unchanged, so restore the value configure_filter() last asked for.
> +	 */

Comment style, same as in patch 1.

> +	if (rtwdev->hal.rxfltmap1_cur)
> +		rtw_write16(rtwdev, REG_RXFLTMAP1, rtwdev->hal.rxfltmap1_cur);
> +

A nit: this line doubles as "configure_filter has run". It only works because
no chip has a zero default (0x400, 0x0FFF, 0xFFFF), so an explicit flag would
not depend on that.

Best regards,
Luka Gejak

  reply	other threads:[~2026-10-11 13:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-07  9:12 [PATCH wireless 0/2] wifi: rtw88: channel and control frame filter lost on interface restart Jeremy Fareau
2026-10-07  9:12 ` [PATCH wireless 1/2] wifi: rtw88: program the channel when the device is started Jeremy Fareau
2026-10-09 12:12   ` Luka Gejak
2026-10-12  2:07   ` Ping-Ke Shih
2026-10-07  9:12 ` [PATCH wireless 2/2] wifi: rtw88: restore the control frame filter " Jeremy Fareau
2026-10-11 13:08   ` Luka Gejak [this message]
2026-10-12  2:28   ` Ping-Ke Shih

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM216XAEDYAN.IZDPPZ4YWPLG@linux.dev \
    --to=luka.gejak@linux.dev \
    --cc=briannorris@chromium.org \
    --cc=jeremy.fareau@gmail.com \
    --cc=kvalo@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=pkshih@realtek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®