* [PATCH v2 rtw-next 0/2] wifi: rtw89: adjust code to fit coming field_prep()
@ 2025-11-20 3:10 Ping-Ke Shih
2025-11-20 3:10 ` [PATCH v2 rtw-next 1/2] wifi: rtw89: 8852a: correct field mask of reset DAC/ADC FIFO Ping-Ke Shih
2025-11-20 3:10 ` [PATCH v2 rtw-next 2/2] wifi: rtw89: avoid to use not consecutive mask in __write_ctrl() Ping-Ke Shih
0 siblings, 2 replies; 5+ messages in thread
From: Ping-Ke Shih @ 2025-11-20 3:10 UTC (permalink / raw)
To: linux-wireless; +Cc: geert
As Geert mentioned [1], two more tricky cases are pointed out by
implementation of field_prep(). Change the code to resolve the tricks.
[1] https://lore.kernel.org/linux-wireless/fccadfe07e4244b993f44ac7315d3d52@realtek.com/T/#mbdfd08117f67b39fabb72dd71acb00f4b3c22bd8
v2:
- add Reported-by to patch 1/2
- use a macro to use u32_encode_bits() to generate mask before calling
inline function to prevent __bad_mask() warning.
Ping-Ke Shih (2):
wifi: rtw89: 8852a: correct field mask of reset DAC/ADC FIFO
wifi: rtw89: avoid to use not consecutive mask in __write_ctrl()
drivers/net/wireless/realtek/rtw89/mac.h | 26 +++++++++++++++++++
drivers/net/wireless/realtek/rtw89/rtw8851b.c | 26 +++++--------------
.../net/wireless/realtek/rtw89/rtw8852a_rfk.c | 8 +++---
drivers/net/wireless/realtek/rtw89/rtw8852b.c | 26 +++++--------------
drivers/net/wireless/realtek/rtw89/rtw8852c.c | 26 +++++--------------
5 files changed, 51 insertions(+), 61 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 rtw-next 1/2] wifi: rtw89: 8852a: correct field mask of reset DAC/ADC FIFO
2025-11-20 3:10 [PATCH v2 rtw-next 0/2] wifi: rtw89: adjust code to fit coming field_prep() Ping-Ke Shih
@ 2025-11-20 3:10 ` Ping-Ke Shih
2025-11-21 3:55 ` Ping-Ke Shih
2025-11-20 3:10 ` [PATCH v2 rtw-next 2/2] wifi: rtw89: avoid to use not consecutive mask in __write_ctrl() Ping-Ke Shih
1 sibling, 1 reply; 5+ messages in thread
From: Ping-Ke Shih @ 2025-11-20 3:10 UTC (permalink / raw)
To: linux-wireless; +Cc: geert
The field mask should be bits 16-31, but suddenly use wrong bits 24-31,
rarely causing a little performance degraded if DAC/DAC FIFO stays on
an unexpected state.
Found this by Geert who works on bit field functions.
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
v2: add Reported-by
---
drivers/net/wireless/realtek/rtw89/rtw8852a_rfk.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw89/rtw8852a_rfk.c b/drivers/net/wireless/realtek/rtw89/rtw8852a_rfk.c
index e74257d19412..463399413318 100644
--- a/drivers/net/wireless/realtek/rtw89/rtw8852a_rfk.c
+++ b/drivers/net/wireless/realtek/rtw89/rtw8852a_rfk.c
@@ -756,8 +756,8 @@ static void _iqk_rxk_setting(struct rtw89_dev *rtwdev, u8 path)
rtw89_phy_write32_mask(rtwdev, R_ANAPAR, B_ANAPAR_FLTRST, 0x1);
rtw89_phy_write32_mask(rtwdev, R_ANAPAR_PW15, B_ANAPAR_PW15_H2, 0x0);
udelay(1);
- rtw89_phy_write32_mask(rtwdev, R_ADC_FIFO, B_ADC_FIFO_RST, 0x0303);
- rtw89_phy_write32_mask(rtwdev, R_ADC_FIFO, B_ADC_FIFO_RST, 0x0000);
+ rtw89_phy_write32_mask(rtwdev, R_ADC_FIFO, B_ADC_FIFO_RXK, 0x0303);
+ rtw89_phy_write32_mask(rtwdev, R_ADC_FIFO, B_ADC_FIFO_RXK, 0x0000);
switch (iqk_info->iqk_band[path]) {
case RTW89_BAND_2G:
@@ -1239,8 +1239,8 @@ static void _iqk_txk_setting(struct rtw89_dev *rtwdev, u8 path)
udelay(1);
rtw89_phy_write32_mask(rtwdev, R_ANAPAR, B_ANAPAR_15, 0x0041);
udelay(1);
- rtw89_phy_write32_mask(rtwdev, R_ADC_FIFO, B_ADC_FIFO_RST, 0x0303);
- rtw89_phy_write32_mask(rtwdev, R_ADC_FIFO, B_ADC_FIFO_RST, 0x0000);
+ rtw89_phy_write32_mask(rtwdev, R_ADC_FIFO, B_ADC_FIFO_RXK, 0x0303);
+ rtw89_phy_write32_mask(rtwdev, R_ADC_FIFO, B_ADC_FIFO_RXK, 0x0000);
switch (iqk_info->iqk_band[path]) {
case RTW89_BAND_2G:
rtw89_write_rf(rtwdev, path, RR_XALNA2, RR_XALNA2_SW, 0x00);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 rtw-next 2/2] wifi: rtw89: avoid to use not consecutive mask in __write_ctrl()
2025-11-20 3:10 [PATCH v2 rtw-next 0/2] wifi: rtw89: adjust code to fit coming field_prep() Ping-Ke Shih
2025-11-20 3:10 ` [PATCH v2 rtw-next 1/2] wifi: rtw89: 8852a: correct field mask of reset DAC/ADC FIFO Ping-Ke Shih
@ 2025-11-20 3:10 ` Ping-Ke Shih
2025-11-21 3:59 ` Ping-Ke Shih
1 sibling, 1 reply; 5+ messages in thread
From: Ping-Ke Shih @ 2025-11-20 3:10 UTC (permalink / raw)
To: linux-wireless; +Cc: geert
The power value and enable bit fields can be not consecutive mask, but
normally we expect mask argument of rtw89_mac_txpwr_write32_mask() is
consecutive bit mask. Therefore, change the code accordingly.
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
v2: use a macro to use u32_encode_bits() to generate mask before calling
inline function to prevent __bad_mask() warning.
---
drivers/net/wireless/realtek/rtw89/mac.h | 26 +++++++++++++++++++
drivers/net/wireless/realtek/rtw89/rtw8851b.c | 26 +++++--------------
drivers/net/wireless/realtek/rtw89/rtw8852b.c | 26 +++++--------------
drivers/net/wireless/realtek/rtw89/rtw8852c.c | 26 +++++--------------
4 files changed, 47 insertions(+), 57 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw89/mac.h b/drivers/net/wireless/realtek/rtw89/mac.h
index 0007229d6753..64a7fd527f3e 100644
--- a/drivers/net/wireless/realtek/rtw89/mac.h
+++ b/drivers/net/wireless/realtek/rtw89/mac.h
@@ -1458,6 +1458,32 @@ static inline int rtw89_mac_txpwr_write32_mask(struct rtw89_dev *rtwdev,
return 0;
}
+static inline
+void __rtw89_mac_write_txpwr_ctrl(struct rtw89_dev *rtwdev, u32 reg, u32 mask,
+ u32 set, u32 mask_en, bool cond)
+{
+ u32 val32;
+ int ret;
+
+ ret = rtw89_mac_txpwr_read32(rtwdev, RTW89_PHY_0, reg, &val32);
+ if (ret)
+ return;
+
+ val32 &= ~(mask | mask_en);
+ val32 |= set;
+
+ if (cond)
+ val32 |= mask_en;
+
+ rtw89_mac_txpwr_write32(rtwdev, RTW89_PHY_0, reg, val32);
+}
+
+#define rtw89_mac_write_txpwr_ctrl(_rtwdev, _reg, _mask, _val, _mask_en, _cond) \
+do { \
+ u32 _set = u32_encode_bits(_val, _mask); \
+ __rtw89_mac_write_txpwr_ctrl(_rtwdev, _reg, _mask, _set, _mask_en, _cond); \
+} while (0)
+
static inline void rtw89_mac_ctrl_hci_dma_tx(struct rtw89_dev *rtwdev,
bool enable)
{
diff --git a/drivers/net/wireless/realtek/rtw89/rtw8851b.c b/drivers/net/wireless/realtek/rtw89/rtw8851b.c
index 84b628d23882..7134b88ccf76 100644
--- a/drivers/net/wireless/realtek/rtw89/rtw8851b.c
+++ b/drivers/net/wireless/realtek/rtw89/rtw8851b.c
@@ -2291,18 +2291,6 @@ rtw8851b_btc_set_wl_txpwr_ctrl(struct rtw89_dev *rtwdev, u32 txpwr_val)
union rtw8851b_btc_wl_txpwr_ctrl arg = { .txpwr_val = txpwr_val };
s32 val;
-#define __write_ctrl(_reg, _msk, _val, _en, _cond) \
-do { \
- u32 _wrt = FIELD_PREP(_msk, _val); \
- BUILD_BUG_ON(!!(_msk & _en)); \
- if (_cond) \
- _wrt |= _en; \
- else \
- _wrt &= ~_en; \
- rtw89_mac_txpwr_write32_mask(rtwdev, RTW89_PHY_0, _reg, \
- _msk | _en, _wrt); \
-} while (0)
-
switch (arg.ctrl_all_time) {
case 0xffff:
val = 0;
@@ -2312,9 +2300,10 @@ do { \
break;
}
- __write_ctrl(R_AX_PWR_RATE_CTRL, B_AX_FORCE_PWR_BY_RATE_VALUE_MASK,
- val, B_AX_FORCE_PWR_BY_RATE_EN,
- arg.ctrl_all_time != 0xffff);
+ rtw89_mac_write_txpwr_ctrl(rtwdev, R_AX_PWR_RATE_CTRL,
+ B_AX_FORCE_PWR_BY_RATE_VALUE_MASK,
+ val, B_AX_FORCE_PWR_BY_RATE_EN,
+ arg.ctrl_all_time != 0xffff);
switch (arg.ctrl_gnt_bt) {
case 0xffff:
@@ -2325,10 +2314,9 @@ do { \
break;
}
- __write_ctrl(R_AX_PWR_COEXT_CTRL, B_AX_TXAGC_BT_MASK, val,
- B_AX_TXAGC_BT_EN, arg.ctrl_gnt_bt != 0xffff);
-
-#undef __write_ctrl
+ rtw89_mac_write_txpwr_ctrl(rtwdev, R_AX_PWR_COEXT_CTRL,
+ B_AX_TXAGC_BT_MASK, val,
+ B_AX_TXAGC_BT_EN, arg.ctrl_gnt_bt != 0xffff);
}
static
diff --git a/drivers/net/wireless/realtek/rtw89/rtw8852b.c b/drivers/net/wireless/realtek/rtw89/rtw8852b.c
index 70fb05bc5e98..f88af57fac65 100644
--- a/drivers/net/wireless/realtek/rtw89/rtw8852b.c
+++ b/drivers/net/wireless/realtek/rtw89/rtw8852b.c
@@ -761,18 +761,6 @@ rtw8852b_btc_set_wl_txpwr_ctrl(struct rtw89_dev *rtwdev, u32 txpwr_val)
union rtw8852b_btc_wl_txpwr_ctrl arg = { .txpwr_val = txpwr_val };
s32 val;
-#define __write_ctrl(_reg, _msk, _val, _en, _cond) \
-do { \
- u32 _wrt = FIELD_PREP(_msk, _val); \
- BUILD_BUG_ON(!!(_msk & _en)); \
- if (_cond) \
- _wrt |= _en; \
- else \
- _wrt &= ~_en; \
- rtw89_mac_txpwr_write32_mask(rtwdev, RTW89_PHY_0, _reg, \
- _msk | _en, _wrt); \
-} while (0)
-
switch (arg.ctrl_all_time) {
case 0xffff:
val = 0;
@@ -782,9 +770,10 @@ do { \
break;
}
- __write_ctrl(R_AX_PWR_RATE_CTRL, B_AX_FORCE_PWR_BY_RATE_VALUE_MASK,
- val, B_AX_FORCE_PWR_BY_RATE_EN,
- arg.ctrl_all_time != 0xffff);
+ rtw89_mac_write_txpwr_ctrl(rtwdev, R_AX_PWR_RATE_CTRL,
+ B_AX_FORCE_PWR_BY_RATE_VALUE_MASK,
+ val, B_AX_FORCE_PWR_BY_RATE_EN,
+ arg.ctrl_all_time != 0xffff);
switch (arg.ctrl_gnt_bt) {
case 0xffff:
@@ -795,10 +784,9 @@ do { \
break;
}
- __write_ctrl(R_AX_PWR_COEXT_CTRL, B_AX_TXAGC_BT_MASK, val,
- B_AX_TXAGC_BT_EN, arg.ctrl_gnt_bt != 0xffff);
-
-#undef __write_ctrl
+ rtw89_mac_write_txpwr_ctrl(rtwdev, R_AX_PWR_COEXT_CTRL,
+ B_AX_TXAGC_BT_MASK, val,
+ B_AX_TXAGC_BT_EN, arg.ctrl_gnt_bt != 0xffff);
}
static const struct rtw89_chip_ops rtw8852b_chip_ops = {
diff --git a/drivers/net/wireless/realtek/rtw89/rtw8852c.c b/drivers/net/wireless/realtek/rtw89/rtw8852c.c
index db99450e9158..d2a91232ea27 100644
--- a/drivers/net/wireless/realtek/rtw89/rtw8852c.c
+++ b/drivers/net/wireless/realtek/rtw89/rtw8852c.c
@@ -2760,18 +2760,6 @@ rtw8852c_btc_set_wl_txpwr_ctrl(struct rtw89_dev *rtwdev, u32 txpwr_val)
union rtw8852c_btc_wl_txpwr_ctrl arg = { .txpwr_val = txpwr_val };
s32 val;
-#define __write_ctrl(_reg, _msk, _val, _en, _cond) \
-do { \
- u32 _wrt = FIELD_PREP(_msk, _val); \
- BUILD_BUG_ON((_msk & _en) != 0); \
- if (_cond) \
- _wrt |= _en; \
- else \
- _wrt &= ~_en; \
- rtw89_mac_txpwr_write32_mask(rtwdev, RTW89_PHY_0, _reg, \
- _msk | _en, _wrt); \
-} while (0)
-
switch (arg.ctrl_all_time) {
case 0xffff:
val = 0;
@@ -2781,9 +2769,10 @@ do { \
break;
}
- __write_ctrl(R_AX_PWR_RATE_CTRL, B_AX_FORCE_PWR_BY_RATE_VALUE_MASK,
- val, B_AX_FORCE_PWR_BY_RATE_EN,
- arg.ctrl_all_time != 0xffff);
+ rtw89_mac_write_txpwr_ctrl(rtwdev, R_AX_PWR_RATE_CTRL,
+ B_AX_FORCE_PWR_BY_RATE_VALUE_MASK,
+ val, B_AX_FORCE_PWR_BY_RATE_EN,
+ arg.ctrl_all_time != 0xffff);
switch (arg.ctrl_gnt_bt) {
case 0xffff:
@@ -2794,10 +2783,9 @@ do { \
break;
}
- __write_ctrl(R_AX_PWR_COEXT_CTRL, B_AX_TXAGC_BT_MASK, val,
- B_AX_TXAGC_BT_EN, arg.ctrl_gnt_bt != 0xffff);
-
-#undef __write_ctrl
+ rtw89_mac_write_txpwr_ctrl(rtwdev, R_AX_PWR_COEXT_CTRL,
+ B_AX_TXAGC_BT_MASK, val,
+ B_AX_TXAGC_BT_EN, arg.ctrl_gnt_bt != 0xffff);
}
static
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 rtw-next 1/2] wifi: rtw89: 8852a: correct field mask of reset DAC/ADC FIFO
2025-11-20 3:10 ` [PATCH v2 rtw-next 1/2] wifi: rtw89: 8852a: correct field mask of reset DAC/ADC FIFO Ping-Ke Shih
@ 2025-11-21 3:55 ` Ping-Ke Shih
0 siblings, 0 replies; 5+ messages in thread
From: Ping-Ke Shih @ 2025-11-21 3:55 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless; +Cc: geert
Ping-Ke Shih <pkshih@realtek.com> wrote:
> The field mask should be bits 16-31, but suddenly use wrong bits 24-31,
> rarely causing a little performance degraded if DAC/DAC FIFO stays on
> an unexpected state.
>
> Found this by Geert who works on bit field functions.
>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
1 patch(es) applied to rtw-next branch of rtw.git, thanks.
2a2aae365534 wifi: rtw89: 8852a: correct field mask of reset DAC/ADC FIFO
---
https://github.com/pkshih/rtw.git
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2 rtw-next 2/2] wifi: rtw89: avoid to use not consecutive mask in __write_ctrl()
2025-11-20 3:10 ` [PATCH v2 rtw-next 2/2] wifi: rtw89: avoid to use not consecutive mask in __write_ctrl() Ping-Ke Shih
@ 2025-11-21 3:59 ` Ping-Ke Shih
0 siblings, 0 replies; 5+ messages in thread
From: Ping-Ke Shih @ 2025-11-21 3:59 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org; +Cc: geert@linux-m68k.org
Hi Geert,
Ping-Ke Shih <pkshih@realtek.com> wrote:
> The power value and enable bit fields can be not consecutive mask, but
> normally we expect mask argument of rtw89_mac_txpwr_write32_mask() is
> consecutive bit mask. Therefore, change the code accordingly.
>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
> v2: use a macro to use u32_encode_bits() to generate mask before calling
> inline function to prevent __bad_mask() warning.
I'm still thinking if this change is good enough, so I'd defer it for now.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-11-21 3:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-20 3:10 [PATCH v2 rtw-next 0/2] wifi: rtw89: adjust code to fit coming field_prep() Ping-Ke Shih
2025-11-20 3:10 ` [PATCH v2 rtw-next 1/2] wifi: rtw89: 8852a: correct field mask of reset DAC/ADC FIFO Ping-Ke Shih
2025-11-21 3:55 ` Ping-Ke Shih
2025-11-20 3:10 ` [PATCH v2 rtw-next 2/2] wifi: rtw89: avoid to use not consecutive mask in __write_ctrl() Ping-Ke Shih
2025-11-21 3:59 ` Ping-Ke Shih
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).