Skip to contentSkip to conversation

wifi: rtw88: Don't use static local variable in rtw8822b_set_tx_power_index_by_rate

PATCH In review Bitterblue Smith 2 participants 6 messages Jan 26, 2025 – Feb 3, 2025 Realtek wireless driver (rtw88)

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 wifi: rtw88: Don't use static local variable in rtw8822b_set_tx_power_index_by_rate

Some users want to plug two identical USB devices at the same time. This static variable could theoretically cause them to use incorrect TX power values.

Move the variable to the caller and pass a pointer to it to rtw8822b_set_tx_power_index_by_rate().

---
 drivers/net/wireless/realtek/rtw88/rtw8822b.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/net/wireless/realtek/rtw88/rtw8822b.c +8 -6
index 7f03903ddf4b..23a29019752d 100644
@@ -935,11 +935,11 @@ static void query_phy_status(struct rtw_dev *rtwdev, u8 *phy_status,
 }
 
 static void
-rtw8822b_set_tx_power_index_by_rate(struct rtw_dev *rtwdev, u8 path, u8 rs)
+rtw8822b_set_tx_power_index_by_rate(struct rtw_dev *rtwdev, u8 path,
+				    u8 rs, u32 *phy_pwr_idx)
 {
 	struct rtw_hal *hal = &rtwdev->hal;
 	static const u32 offset_txagc[2] = {0x1d00, 0x1d80};
-	static u32 phy_pwr_idx;
 	u8 rate, rate_idx, pwr_index, shift;
 	int j;
 
@@ -947,12 +947,12 @@ rtw8822b_set_tx_power_index_by_rate(struct rtw_dev *rtwdev, u8 path, u8 rs)
 		rate = rtw_rate_section[rs][j];
 		pwr_index = hal->tx_pwr_tbl[path][rate];
 		shift = rate & 0x3;
-		phy_pwr_idx |= ((u32)pwr_index << (shift * 8));
+		*phy_pwr_idx |= ((u32)pwr_index << (shift * 8));
 		if (shift == 0x3) {
 			rate_idx = rate & 0xfc;
 			rtw_write32(rtwdev, offset_txagc[path] + rate_idx,
-				    phy_pwr_idx);
-			phy_pwr_idx = 0;
+				    *phy_pwr_idx);
+			*phy_pwr_idx = 0;
 		}
 	}
 }
@@ -960,11 +960,13 @@ rtw8822b_set_tx_power_index_by_rate(struct rtw_dev *rtwdev, u8 path, u8 rs)
 static void rtw8822b_set_tx_power_index(struct rtw_dev *rtwdev)
 {
 	struct rtw_hal *hal = &rtwdev->hal;
+	u32 phy_pwr_idx = 0;
 	int rs, path;
 
 	for (path = 0; path < hal->rf_path_num; path++) {
 		for (rs = 0; rs < RTW_RATE_SECTION_MAX; rs++)
-			rtw8822b_set_tx_power_index_by_rate(rtwdev, path, rs);
+			rtw8822b_set_tx_power_index_by_rate(rtwdev, path, rs,
+							    &phy_pwr_idx);
 	}
 }
 
Signature · 1 line
2.48.1
#1 lore raw

PATCH 2/2 wifi: rtw88: Don't use static local variable in rtw8821c_set_tx_power_index_by_rate

Some users want to plug two identical USB devices at the same time. This static variable could theoretically cause them to use incorrect TX power values.

Move the variable to the caller and pass a pointer to it to rtw8821c_set_tx_power_index_by_rate().

---
 drivers/net/wireless/realtek/rtw88/rtw8821c.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/net/wireless/realtek/rtw88/rtw8821c.c +8 -6
index eb7e34c545d0..cc152248407c 100644
@@ -680,11 +680,11 @@ static void query_phy_status(struct rtw_dev *rtwdev, u8 *phy_status,
 }
 
 static void
-rtw8821c_set_tx_power_index_by_rate(struct rtw_dev *rtwdev, u8 path, u8 rs)
+rtw8821c_set_tx_power_index_by_rate(struct rtw_dev *rtwdev, u8 path,
+				    u8 rs, u32 *phy_pwr_idx)
 {
 	struct rtw_hal *hal = &rtwdev->hal;
 	static const u32 offset_txagc[2] = {0x1d00, 0x1d80};
-	static u32 phy_pwr_idx;
 	u8 rate, rate_idx, pwr_index, shift;
 	int j;
 
@@ -692,12 +692,12 @@ rtw8821c_set_tx_power_index_by_rate(struct rtw_dev *rtwdev, u8 path, u8 rs)
 		rate = rtw_rate_section[rs][j];
 		pwr_index = hal->tx_pwr_tbl[path][rate];
 		shift = rate & 0x3;
-		phy_pwr_idx |= ((u32)pwr_index << (shift * 8));
+		*phy_pwr_idx |= ((u32)pwr_index << (shift * 8));
 		if (shift == 0x3 || rate == DESC_RATEVHT1SS_MCS9) {
 			rate_idx = rate & 0xfc;
 			rtw_write32(rtwdev, offset_txagc[path] + rate_idx,
-				    phy_pwr_idx);
-			phy_pwr_idx = 0;
+				    *phy_pwr_idx);
+			*phy_pwr_idx = 0;
 		}
 	}
 }
@@ -705,6 +705,7 @@ rtw8821c_set_tx_power_index_by_rate(struct rtw_dev *rtwdev, u8 path, u8 rs)
 static void rtw8821c_set_tx_power_index(struct rtw_dev *rtwdev)
 {
 	struct rtw_hal *hal = &rtwdev->hal;
+	u32 phy_pwr_idx = 0;
 	int rs, path;
 
 	for (path = 0; path < hal->rf_path_num; path++) {
@@ -712,7 +713,8 @@ static void rtw8821c_set_tx_power_index(struct rtw_dev *rtwdev)
 			if (rs == RTW_RATE_SECTION_HT_2S ||
 			    rs == RTW_RATE_SECTION_VHT_2S)
 				continue;
-			rtw8821c_set_tx_power_index_by_rate(rtwdev, path, rs);
+			rtw8821c_set_tx_power_index_by_rate(rtwdev, path, rs,
+							    &phy_pwr_idx);
 		}
 	}
 }
Signature · 1 line
2.48.1
maintainer Acked-by on v1 2/2
#2 lore raw

Re: [PATCH 2/2] wifi: rtw88: Don't use static local variable in rtw8821c_set_tx_power_index_by_rate

Bitterblue Smith <[email protected]> worte:

Quoted 8 lines

Some users want to plug two identical USB devices at the same time. This static variable could theoretically cause them to use incorrect TX power values.

Move the variable to the caller and pass a pointer to it to rtw8821c_set_tx_power_index_by_rate().

maintainer Acked-by on v1 1/2
#3 lore raw
In reply to Bitterblue Smith
Quoted 9 lines · Bitterblue Smith

Some users want to plug two identical USB devices at the same time. This static variable could theoretically cause them to use incorrect TX power values.

Move the variable to the caller and pass a pointer to it to rtw8822b_set_tx_power_index_by_rate().

By the way, if you interest to cleanup rtw88 and avoid this kind of limits, we can check symbols of output of "objdump -t *.o | grep "\.data". Patches are welcome.

#4 lore raw
Quoted 9 lines · Ping-Ke Shih
Quoted 8 lines · Bitterblue Smith

Some users want to plug two identical USB devices at the same time. This static variable could theoretically cause them to use incorrect TX power values.

Move the variable to the caller and pass a pointer to it to rtw8822b_set_tx_power_index_by_rate().

Quoted 8 lines
Quoted 1 line

By the way, if you interest to cleanup rtw88 and avoid this kind of limits, we can check symbols of output of "objdump -t *.o | grep "\.data". Patches are welcome.

I see some tables that should be const. I will send a patch.

maintainer
#5 lore raw
In reply to Bitterblue Smith
Quoted 9 lines · Bitterblue Smith

Some users want to plug two identical USB devices at the same time. This static variable could theoretically cause them to use incorrect TX power values.

Move the variable to the caller and pass a pointer to it to rtw8822b_set_tx_power_index_by_rate().

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

00451eb3bec7 wifi: rtw88: Don't use static local variable in rtw8822b_set_tx_power_index_by_rate 0d1d165eff9d wifi: rtw88: Don't use static local variable in rtw8821c_set_tx_power_index_by_rate

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