[PATCH rtw-next] wifi: rtw89: use separated function to set RX filter

[PATCH rtw-next] wifi: rtw89: use separated function to set RX filter

[Ping-Ke Shih]

Normally use rtw89_write32_mask() with a consecutive bit mask, but mask of
RX filter B_AX_RX_FLTR_CFG_MASK is bits 31-22 and 15-0, which excludes bits
of B_AX_RX_MPDU_MAX_LEN_MASK (bits 21-16).

Though the original logic is well to set RX filter, change it to a separate
function to avoid the tricky design.

Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
Hi Geert,

I plan to have this change in next release cycle, and then you can send
your patches without errors caused by this.
---
 drivers/net/wireless/realtek/rtw89/core.c     |  5 +----
 drivers/net/wireless/realtek/rtw89/fw.c       | 10 ++--------
 drivers/net/wireless/realtek/rtw89/mac.c      | 14 ++++++++++++++
 drivers/net/wireless/realtek/rtw89/mac.h      |  1 +
 drivers/net/wireless/realtek/rtw89/mac80211.c | 11 ++---------
 5 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
index a15a911484bb..b6b8500a0028 100644
--- a/drivers/net/wireless/realtek/rtw89/core.c
+++ b/drivers/net/wireless/realtek/rtw89/core.c
@@ -4169,12 +4169,10 @@ void rtw89_roc_start(struct rtw89_dev *rtwdev, struct rtw89_vif *rtwvif)
 
 void rtw89_roc_end(struct rtw89_dev *rtwdev, struct rtw89_vif *rtwvif)
 {
-	const struct rtw89_mac_gen_def *mac = rtwdev->chip->mac_def;
 	struct ieee80211_hw *hw = rtwdev->hw;
 	struct rtw89_roc *roc = &rtwvif->roc;
 	struct rtw89_vif_link *rtwvif_link;
 	struct rtw89_vif *tmp_vif;
-	u32 reg;
 	int ret;
 
 	lockdep_assert_wiphy(hw->wiphy);
@@ -4191,8 +4189,7 @@ void rtw89_roc_end(struct rtw89_dev *rtwdev, struct rtw89_vif *rtwvif)
 		return;
 	}
 
-	reg = rtw89_mac_reg_by_idx(rtwdev, mac->rx_fltr, rtwvif_link->mac_idx);
-	rtw89_write32_mask(rtwdev, reg, B_AX_RX_FLTR_CFG_MASK, rtwdev->hal.rx_fltr);
+	rtw89_mac_set_rx_fltr(rtwdev, rtwvif_link->mac_idx, rtwdev->hal.rx_fltr);
 
 	roc->state = RTW89_ROC_IDLE;
 	rtw89_config_roc_chandef(rtwdev, rtwvif_link, NULL);
diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
index 515e9f098380..e96c3725a1ea 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.c
+++ b/drivers/net/wireless/realtek/rtw89/fw.c
@@ -8068,7 +8068,6 @@ int rtw89_hw_scan_start(struct rtw89_dev *rtwdev,
 			struct rtw89_vif_link *rtwvif_link,
 			struct ieee80211_scan_request *scan_req)
 {
-	const struct rtw89_mac_gen_def *mac = rtwdev->chip->mac_def;
 	enum rtw89_entity_mode mode = rtw89_get_entity_mode(rtwdev);
 	struct cfg80211_scan_request *req = &scan_req->req;
 	const struct rtw89_chan *chan = rtw89_chan_get(rtwdev,
@@ -8080,7 +8079,6 @@ int rtw89_hw_scan_start(struct rtw89_dev *rtwdev,
 	};
 	u32 rx_fltr = rtwdev->hal.rx_fltr;
 	u8 mac_addr[ETH_ALEN];
-	u32 reg;
 	int ret;
 
 	/* clone op and keep it during scan */
@@ -8120,8 +8118,7 @@ int rtw89_hw_scan_start(struct rtw89_dev *rtwdev,
 	rx_fltr &= ~B_AX_A_BC;
 	rx_fltr &= ~B_AX_A_A1_MATCH;
 
-	reg = rtw89_mac_reg_by_idx(rtwdev, mac->rx_fltr, rtwvif_link->mac_idx);
-	rtw89_write32_mask(rtwdev, reg, B_AX_RX_FLTR_CFG_MASK, rx_fltr);
+	rtw89_mac_set_rx_fltr(rtwdev, rtwvif_link->mac_idx, rx_fltr);
 
 	rtw89_chanctx_pause(rtwdev, &pause_parm);
 	rtw89_phy_dig_suspend(rtwdev);
@@ -8139,20 +8136,17 @@ struct rtw89_hw_scan_complete_cb_data {
 
 static int rtw89_hw_scan_complete_cb(struct rtw89_dev *rtwdev, void *data)
 {
-	const struct rtw89_mac_gen_def *mac = rtwdev->chip->mac_def;
 	enum rtw89_entity_mode mode = rtw89_get_entity_mode(rtwdev);
 	struct rtw89_hw_scan_complete_cb_data *cb_data = data;
 	struct rtw89_vif_link *rtwvif_link = cb_data->rtwvif_link;
 	struct cfg80211_scan_info info = {
 		.aborted = cb_data->aborted,
 	};
-	u32 reg;
 
 	if (!rtwvif_link)
 		return -EINVAL;
 
-	reg = rtw89_mac_reg_by_idx(rtwdev, mac->rx_fltr, rtwvif_link->mac_idx);
-	rtw89_write32_mask(rtwdev, reg, B_AX_RX_FLTR_CFG_MASK, rtwdev->hal.rx_fltr);
+	rtw89_mac_set_rx_fltr(rtwdev, rtwvif_link->mac_idx, rtwdev->hal.rx_fltr);
 
 	rtw89_core_scan_complete(rtwdev, rtwvif_link, true);
 	ieee80211_scan_completed(rtwdev->hw, &info);
diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
index dab0a76556c9..0786a9e32548 100644
--- a/drivers/net/wireless/realtek/rtw89/mac.c
+++ b/drivers/net/wireless/realtek/rtw89/mac.c
@@ -2543,6 +2543,20 @@ static int rtw89_mac_typ_fltr_opt_ax(struct rtw89_dev *rtwdev,
 	return 0;
 }
 
+void rtw89_mac_set_rx_fltr(struct rtw89_dev *rtwdev, u8 mac_idx, u32 rx_fltr)
+{
+	const struct rtw89_mac_gen_def *mac = rtwdev->chip->mac_def;
+	u32 reg;
+	u32 val;
+
+	reg = rtw89_mac_reg_by_idx(rtwdev, mac->rx_fltr, mac_idx);
+
+	val = rtw89_read32(rtwdev, reg);
+	/* B_AX_RX_FLTR_CFG_MASK is not a consecutive bit mask */
+	val = (val & ~B_AX_RX_FLTR_CFG_MASK) | (rx_fltr & B_AX_RX_FLTR_CFG_MASK);
+	rtw89_write32(rtwdev, reg, val);
+}
+
 static int rx_fltr_init_ax(struct rtw89_dev *rtwdev, u8 mac_idx)
 {
 	int ret, i;
diff --git a/drivers/net/wireless/realtek/rtw89/mac.h b/drivers/net/wireless/realtek/rtw89/mac.h
index 3cc97fd0c0ec..98cfd5c3cb3f 100644
--- a/drivers/net/wireless/realtek/rtw89/mac.h
+++ b/drivers/net/wireless/realtek/rtw89/mac.h
@@ -1339,6 +1339,7 @@ int rtw89_mac_cfg_ppdu_status_bands(struct rtw89_dev *rtwdev, bool enable)
 	return rtw89_mac_cfg_ppdu_status(rtwdev, RTW89_MAC_1, enable);
 }
 
+void rtw89_mac_set_rx_fltr(struct rtw89_dev *rtwdev, u8 mac_idx, u32 rx_fltr);
 void rtw89_mac_update_rts_threshold(struct rtw89_dev *rtwdev);
 void rtw89_mac_flush_txq(struct rtw89_dev *rtwdev, u32 queues, bool drop);
 int rtw89_mac_coex_init(struct rtw89_dev *rtwdev, const struct rtw89_mac_ax_coex *coex);
diff --git a/drivers/net/wireless/realtek/rtw89/mac80211.c b/drivers/net/wireless/realtek/rtw89/mac80211.c
index 6f7571c7c274..a335a04f413d 100644
--- a/drivers/net/wireless/realtek/rtw89/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw89/mac80211.c
@@ -307,7 +307,6 @@ static void rtw89_ops_configure_filter(struct ieee80211_hw *hw,
 				       u64 multicast)
 {
 	struct rtw89_dev *rtwdev = hw->priv;
-	const struct rtw89_mac_gen_def *mac = rtwdev->chip->mac_def;
 	u32 rx_fltr;
 
 	lockdep_assert_wiphy(hw->wiphy);
@@ -369,16 +368,10 @@ static void rtw89_ops_configure_filter(struct ieee80211_hw *hw,
 		rx_fltr &= ~B_AX_A_A1_MATCH;
 	}
 
-	rtw89_write32_mask(rtwdev,
-			   rtw89_mac_reg_by_idx(rtwdev, mac->rx_fltr, RTW89_MAC_0),
-			   B_AX_RX_FLTR_CFG_MASK,
-			   rx_fltr);
+	rtw89_mac_set_rx_fltr(rtwdev, RTW89_MAC_0, rx_fltr);
 	if (!rtwdev->dbcc_en)
 		return;
-	rtw89_write32_mask(rtwdev,
-			   rtw89_mac_reg_by_idx(rtwdev, mac->rx_fltr, RTW89_MAC_1),
-			   B_AX_RX_FLTR_CFG_MASK,
-			   rx_fltr);
+	rtw89_mac_set_rx_fltr(rtwdev, RTW89_MAC_1, rx_fltr);
 }
 
 static const u8 ac_to_fw_idx[IEEE80211_NUM_ACS] = {
-- 
2.25.1

Re: [PATCH rtw-next] wifi: rtw89: use separated function to set RX filter

[Geert Uytterhoeven]

Hi Ping-ke,

On Thu, 13 Nov 2025 at 06:35, Ping-Ke Shih <pkshih@realtek.com> wrote:
> Normally use rtw89_write32_mask() with a consecutive bit mask, but mask of
> RX filter B_AX_RX_FLTR_CFG_MASK is bits 31-22 and 15-0, which excludes bits
> of B_AX_RX_MPDU_MAX_LEN_MASK (bits 21-16).
>
> Though the original logic is well to set RX filter, change it to a separate
> function to avoid the tricky design.
>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> I plan to have this change in next release cycle, and then you can send
> your patches without errors caused by this.

I found two more classes of issues:

  1. drivers/net/wireless/realtek/rtw89/rtw8852a_rfk.c has twice:

          rtw89_phy_write32_mask(rtwdev, R_ADC_FIFO, B_ADC_FIFO_RST, 0x0303);

     B_ADC_FIFO_RST is only 8 bits wide, so 0x0303 does not fit.
     Other call sites write the magic value 0x0303 to B_ADC_FIFO_RXK,
     so perhaps that field was intended?

         #define B_ADC_FIFO_RST GENMASK(31, 24)
         #define B_ADC_FIFO_RXK GENMASK(31, 16)

  2. drivers/net/wireless/realtek/rtw89/rtw8851b.c
     drivers/net/wireless/realtek/rtw89/rtw8852b.c
     drivers/net/wireless/realtek/rtw89/rtw8852c.c

           __write_ctrl(R_AX_PWR_COEXT_CTRL, B_AX_TXAGC_BT_MASK, val,
                        B_AX_TXAGC_BT_EN, arg.ctrl_gnt_bt != 0xffff);

     B_AX_TXAGC_BT_MASK and B_AX_TXAGC_BT_EN are combined,
     thus forming a non-consecutive mask.

         #define B_AX_TXAGC_BT_EN BIT(1)
         #define B_AX_TXAGC_BT_MASK GENMASK(11, 3)

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH rtw-next] wifi: rtw89: use separated function to set RX filter

[Ping-Ke Shih]

Hi Geert,

Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Ping-ke,
> 
> On Thu, 13 Nov 2025 at 06:35, Ping-Ke Shih <pkshih@realtek.com> wrote:
> > Normally use rtw89_write32_mask() with a consecutive bit mask, but mask of
> > RX filter B_AX_RX_FLTR_CFG_MASK is bits 31-22 and 15-0, which excludes bits
> > of B_AX_RX_MPDU_MAX_LEN_MASK (bits 21-16).
> >
> > Though the original logic is well to set RX filter, change it to a separate
> > function to avoid the tricky design.
> >
> > Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > I plan to have this change in next release cycle, and then you can send
> > your patches without errors caused by this.
> 
> I found two more classes of issues:
> 
>   1. drivers/net/wireless/realtek/rtw89/rtw8852a_rfk.c has twice:
> 
>           rtw89_phy_write32_mask(rtwdev, R_ADC_FIFO, B_ADC_FIFO_RST, 0x0303);
> 
>      B_ADC_FIFO_RST is only 8 bits wide, so 0x0303 does not fit.
>      Other call sites write the magic value 0x0303 to B_ADC_FIFO_RXK,
>      so perhaps that field was intended?
> 
>          #define B_ADC_FIFO_RST GENMASK(31, 24)
>          #define B_ADC_FIFO_RXK GENMASK(31, 16)

By vendor driver, it should be B_ADC_FIFO_RXK. I will correct it.

> 
>   2. drivers/net/wireless/realtek/rtw89/rtw8851b.c
>      drivers/net/wireless/realtek/rtw89/rtw8852b.c
>      drivers/net/wireless/realtek/rtw89/rtw8852c.c
> 
>            __write_ctrl(R_AX_PWR_COEXT_CTRL, B_AX_TXAGC_BT_MASK, val,
>                         B_AX_TXAGC_BT_EN, arg.ctrl_gnt_bt != 0xffff);
> 
>      B_AX_TXAGC_BT_MASK and B_AX_TXAGC_BT_EN are combined,
>      thus forming a non-consecutive mask.
> 
>          #define B_AX_TXAGC_BT_EN BIT(1)
>          #define B_AX_TXAGC_BT_MASK GENMASK(11, 3)

I will convert this to something like the style of this patch.

Re: [PATCH rtw-next] wifi: rtw89: use separated function to set RX filter

[Ping-Ke Shih]

Ping-Ke Shih <pkshih@realtek.com> wrote:

> Normally use rtw89_write32_mask() with a consecutive bit mask, but mask of
> RX filter B_AX_RX_FLTR_CFG_MASK is bits 31-22 and 15-0, which excludes bits
> of B_AX_RX_MPDU_MAX_LEN_MASK (bits 21-16).
> 
> Though the original logic is well to set RX filter, change it to a separate
> function to avoid the tricky design.
> 
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

1 patch(es) applied to rtw-next branch of rtw.git, thanks.

c8d212fa81c5 wifi: rtw89: use separated function to set RX filter

---
https://github.com/pkshih/rtw.git