[PATCH v2 0/3] staging: rtl8723bs: refactor PHY_ConvertTxPowerLimitToPowerIndex()

[PATCH v2 0/3] staging: rtl8723bs: refactor PHY_ConvertTxPowerLimitToPowerIndex()

[Leonardo Martins Martins]

The function PHY_ConvertTxPowerLimitToPowerIndex() has several style
problems, such as mixed-case names, deeply nested code, and redundant
bounds checking.

Patch 1 renames mixed-case variables that are on the same scope as this
function, struct fields such as TxPwrLimit_2_4G were left as is. Patch 2
removes redundant parts of the code, and patch 3 solves the nesting
issue by extracting the inner loop into an auxiliary function like it is
done in rtw_phy_tx_power_limit_config(), at
drivers/net/wireless/realtek/rtw88/phy.c

Checkpatch issues a few warnings and checks for deep nesting and long
lines, but these are solved by the last patch.

Compile tested only.

Signed-off-by: Leonardo Martins Martins <dev.lmmrtns@gmail.com>
---
Changes in v2:
- Rebase on staging-testing and drop the patch that was already applied:
  "staging: rtl8723bs: rename PHY_ConvertTxPowerLimitToPowerIndex()"
- Dropped these two name changes, keeping the original names:
	regulation -> reg
	channel -> ch
- Link to v1: https://lore.kernel.org/r/20260926-rtl8723bs-tx-pwr-limit-v1-0-989dcbc113fd@gmail.com

---
Leonardo Martins Martins (3):
      staging: rtl8723bs: rename local vars in phy_tx_power_limit_to_index()
      staging: rtl8723bs: access TxPowerByRateBase2_4G directly
      staging: rtl8723bs: extract inner loop in phy_tx_power_limit_to_index()

 drivers/staging/rtl8723bs/hal/hal_com_phycfg.c     | 87 ++++++++--------------
 drivers/staging/rtl8723bs/include/hal_com_phycfg.h |  3 -
 2 files changed, 30 insertions(+), 60 deletions(-)
---
base-commit: dbc2fa996f44602af72ea56efa68c043407eaaac
change-id: 20260922-rtl8723bs-tx-pwr-limit-9a19bb9f901e

Best regards,
-- 
Leonardo Martins Martins <dev.lmmrtns@gmail.com>

[PATCH v2 1/3] staging: rtl8723bs: rename local vars in phy_tx_power_limit_to_index()

[Leonardo Martins Martins]

In phy_tx_power_limit_to_index(), there are many mixed-case local
variables, rename them to adhere to the linux kernel coding style.

Signed-off-by: Leonardo Martins Martins <dev.lmmrtns@gmail.com>
---
Renamed Variables:
	pHalData -> hal_data
	Adapter -> adapter
	BW40PwrBasedBm2_4G -> pwr_base
	rateSection -> rs
	tempValue -> tmp
	tempPwrLmt -> tmp_pwr_lmt
	rfPath -> path

Changes in v2:
- Dropped these two name changes:
	regulation -> reg
	channel -> ch
---
 drivers/staging/rtl8723bs/hal/hal_com_phycfg.c | 44 +++++++++++++-------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
index 3712b38cd37d5d5a923f54cdeb6675223549f740..e98e8a936630cbb6c336ad923234781e718b5c10 100644
--- a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
+++ b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
@@ -739,35 +739,35 @@ s8 phy_get_tx_pwr_lmt(struct adapter *adapter, u32 reg_pwr_tbl_sel,
 	return pwr_lmt;
 }
 
-void phy_tx_power_limit_to_index(struct adapter *Adapter)
+void phy_tx_power_limit_to_index(struct adapter *adapter)
 {
-	struct hal_com_data	*pHalData = GET_HAL_DATA(Adapter);
-	struct registry_priv *r = &Adapter->registrypriv;
-	u8 BW40PwrBasedBm2_4G = 0x2E;
-	u8 regulation, bw, channel, rateSection;
-	s8 tempValue = 0, tempPwrLmt = 0;
-	u8 rfPath = 0;
+	struct hal_com_data *hal_data = GET_HAL_DATA(adapter);
+	struct registry_priv *r = &adapter->registrypriv;
+	u8 pwr_base = 0x2E;
+	u8 regulation, bw, channel, rs;
+	s8 tmp = 0, tmp_pwr_lmt = 0;
+	u8 path = 0;
 
 	for (regulation = 0; regulation < MAX_REGULATION_NUM; ++regulation) {
 		for (bw = 0; bw < MAX_2_4G_BANDWIDTH_NUM; ++bw) {
 			for (channel = 0; channel < CHANNEL_MAX_NUMBER_2G; ++channel) {
-				for (rateSection = 0; rateSection < MAX_RATE_SECTION_NUM; ++rateSection) {
-					tempPwrLmt = pHalData->TxPwrLimit_2_4G[regulation][bw][rateSection][channel][RF_PATH_A];
-
-					for (rfPath = RF_PATH_A; rfPath < MAX_RF_PATH_NUM; ++rfPath) {
-						if (pHalData->odmpriv.PhyRegPgValueType == PHY_REG_PG_EXACT_VALUE) {
-							if (rateSection == 2) /*  HT 1T */
-								BW40PwrBasedBm2_4G = PHY_GetTxPowerByRateBase(Adapter, rfPath, HT_MCS0_MCS7);
-							else if (rateSection == 1) /*  OFDM */
-								BW40PwrBasedBm2_4G = PHY_GetTxPowerByRateBase(Adapter, rfPath, OFDM);
-							else if (rateSection == 0) /*  CCK */
-								BW40PwrBasedBm2_4G = PHY_GetTxPowerByRateBase(Adapter, rfPath, CCK);
+				for (rs = 0; rs < MAX_RATE_SECTION_NUM; ++rs) {
+					tmp_pwr_lmt = hal_data->TxPwrLimit_2_4G[regulation][bw][rs][channel][RF_PATH_A];
+
+					for (path = RF_PATH_A; path < MAX_RF_PATH_NUM; ++path) {
+						if (hal_data->odmpriv.PhyRegPgValueType == PHY_REG_PG_EXACT_VALUE) {
+							if (rs == 2) /*  HT 1T */
+								pwr_base = PHY_GetTxPowerByRateBase(adapter, path, HT_MCS0_MCS7);
+							else if (rs == 1) /*  OFDM */
+								pwr_base = PHY_GetTxPowerByRateBase(adapter, path, OFDM);
+							else if (rs == 0) /*  CCK */
+								pwr_base = PHY_GetTxPowerByRateBase(adapter, path, CCK);
 						} else
-							BW40PwrBasedBm2_4G = r->reg_power_base * 2;
+							pwr_base = r->reg_power_base * 2;
 
-						if (tempPwrLmt != MAX_POWER_INDEX) {
-							tempValue = tempPwrLmt - BW40PwrBasedBm2_4G;
-							pHalData->TxPwrLimit_2_4G[regulation][bw][rateSection][channel][rfPath] = tempValue;
+						if (tmp_pwr_lmt != MAX_POWER_INDEX) {
+							tmp = tmp_pwr_lmt - pwr_base;
+							hal_data->TxPwrLimit_2_4G[regulation][bw][rs][channel][path] = tmp;
 						}
 					}
 				}

-- 
2.47.3

Re: [PATCH v2 1/3] staging: rtl8723bs: rename local vars in phy_tx_power_limit_to_index()

[Greg Kroah-Hartman]

On Fri, Oct 02, 2026 at 01:07:35AM -0300, Leonardo Martins Martins wrote:
> In phy_tx_power_limit_to_index(), there are many mixed-case local
> variables, rename them to adhere to the linux kernel coding style.
> 
> Signed-off-by: Leonardo Martins Martins <dev.lmmrtns@gmail.com>
> ---
> Renamed Variables:
> 	pHalData -> hal_data
> 	Adapter -> adapter
> 	BW40PwrBasedBm2_4G -> pwr_base

"power_base"?

> 	rateSection -> rs

What's wrong with "rate_selection"?  We have lots of characters you can
use :)

> 	tempValue -> tmp
> 	tempPwrLmt -> tmp_pwr_lmt
> 	rfPath -> path

Why isn't this in the changelog text?

thanks,

greg k-h

Re: [PATCH v2 1/3] staging: rtl8723bs: rename local vars in phy_tx_power_limit_to_index()

[Leonardo Martins Martins]

On Fri, Oct 02, 2026 at 08:09:28AM +0200, Greg Kroah-Hartman wrote:
> On Fri, Oct 02, 2026 at 01:07:35AM -0300, Leonardo Martins Martins wrote:
> > In phy_tx_power_limit_to_index(), there are many mixed-case local
> > variables, rename them to adhere to the linux kernel coding style.
> > 
> > Signed-off-by: Leonardo Martins Martins <dev.lmmrtns@gmail.com>
> > ---
> > Renamed Variables:
> > 	pHalData -> hal_data
> > 	Adapter -> adapter
> > 	BW40PwrBasedBm2_4G -> pwr_base
> 
> "power_base"?

Yeah power_base does seem better.

> 
> > 	rateSection -> rs
> 
> What's wrong with "rate_selection"?  We have lots of characters you can
> use :)

Here I used rs instead of rate_section because I saw it being used
like that in rtw88, and I also saw the "Naming" section of
Documentation/process/coding-style.rst where it says local variable
names should be short and to the point.

Regardless, I don't have any strong opinions on these variable names,
rate_section is also fine, so I'll just use that in v3.

> 
> > 	tempValue -> tmp
> > 	tempPwrLmt -> tmp_pwr_lmt
> > 	rfPath -> path
> 
> Why isn't this in the changelog text?

Oh it really should be, my bad.

Best Regards,
Leonardo Martins

[PATCH v2 2/3] staging: rtl8723bs: access TxPowerByRateBase2_4G directly

[Leonardo Martins Martins]

In phy_tx_power_limit_to_index(), pwr_base is acquired from
TxPowerByRateBase2_4G using rs, which itself is an iterator between
0 and 2, but it's accessed by using an if-else chain that then passes
the equivalent enum constant to PHY_GetTxPowerByRateBase(), which
checks RfPath against RF_PATH_MAX (which is already done by the loop),
then uses a switch-case statement on the enum to access
TxPowerByRateBase2_4G with the original value of rs.

Remove the redundant if-else, the call to PHY_GetTxPowerByRateBase() and
access it directly. Also remove PHY_GetTxPowerByRateBase() since those
were its only usages.

Signed-off-by: Leonardo Martins Martins <dev.lmmrtns@gmail.com>
---
 drivers/staging/rtl8723bs/hal/hal_com_phycfg.c     | 37 ++--------------------
 drivers/staging/rtl8723bs/include/hal_com_phycfg.h |  3 --
 2 files changed, 3 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
index e98e8a936630cbb6c336ad923234781e718b5c10..937441aa7dfee50857d37e0d0aa1415037b8cc60 100644
--- a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
+++ b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
@@ -10,32 +10,6 @@
 #include <linux/kernel.h>
 #include <linux/string.h>
 
-u8 PHY_GetTxPowerByRateBase(struct adapter *Adapter, u8 RfPath,
-			    enum rate_section RateSection)
-{
-	struct hal_com_data	*pHalData = GET_HAL_DATA(Adapter);
-	u8	value = 0;
-
-	if (RfPath >= RF_PATH_MAX)
-		return 0;
-
-	switch (RateSection) {
-	case CCK:
-		value = pHalData->TxPwrByRateBase2_4G[RfPath][0];
-		break;
-	case OFDM:
-		value = pHalData->TxPwrByRateBase2_4G[RfPath][1];
-		break;
-	case HT_MCS0_MCS7:
-		value = pHalData->TxPwrByRateBase2_4G[RfPath][2];
-		break;
-	default:
-		break;
-	}
-
-	return value;
-}
-
 static void
 phy_SetTxPowerByRateBase(struct adapter *Adapter, u8 RfPath,
 			 enum rate_section RateSection, u8 Value)
@@ -755,14 +729,9 @@ void phy_tx_power_limit_to_index(struct adapter *adapter)
 					tmp_pwr_lmt = hal_data->TxPwrLimit_2_4G[regulation][bw][rs][channel][RF_PATH_A];
 
 					for (path = RF_PATH_A; path < MAX_RF_PATH_NUM; ++path) {
-						if (hal_data->odmpriv.PhyRegPgValueType == PHY_REG_PG_EXACT_VALUE) {
-							if (rs == 2) /*  HT 1T */
-								pwr_base = PHY_GetTxPowerByRateBase(adapter, path, HT_MCS0_MCS7);
-							else if (rs == 1) /*  OFDM */
-								pwr_base = PHY_GetTxPowerByRateBase(adapter, path, OFDM);
-							else if (rs == 0) /*  CCK */
-								pwr_base = PHY_GetTxPowerByRateBase(adapter, path, CCK);
-						} else
+						if (hal_data->odmpriv.PhyRegPgValueType == PHY_REG_PG_EXACT_VALUE)
+							pwr_base = hal_data->TxPwrByRateBase2_4G[path][rs];
+						else
 							pwr_base = r->reg_power_base * 2;
 
 						if (tmp_pwr_lmt != MAX_POWER_INDEX) {
diff --git a/drivers/staging/rtl8723bs/include/hal_com_phycfg.h b/drivers/staging/rtl8723bs/include/hal_com_phycfg.h
index a20b4c233de8128fe20e41dee2b303a4662f4de4..ec17e093d8832460ada94878433600063a2b4eab 100644
--- a/drivers/staging/rtl8723bs/include/hal_com_phycfg.h
+++ b/drivers/staging/rtl8723bs/include/hal_com_phycfg.h
@@ -54,9 +54,6 @@ struct bb_register_def {
 
 };
 
-u8 PHY_GetTxPowerByRateBase(struct adapter *Adapter, u8 RfPath,
-			    enum rate_section RateSection);
-
 u8 PHY_GetRateSectionIndexOfTxPowerByRate(struct adapter *padapter, u32	RegAddr,
 					  u32 BitMask);
 

-- 
2.47.3

[PATCH v2 3/3] staging: rtl8723bs: extract inner loop in phy_tx_power_limit_to_index()

[Leonardo Martins Martins]

phy_tx_power_limit_to_index() iterates over an array with 5 indexes,
making the code deeply nested, move the inner loop into an auxiliary
static function to reduce nesting and improve readability.

Signed-off-by: Leonardo Martins Martins <dev.lmmrtns@gmail.com>
---
 drivers/staging/rtl8723bs/hal/hal_com_phycfg.c | 48 ++++++++++++++------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
index 937441aa7dfee50857d37e0d0aa1415037b8cc60..f9f79bead66dcf3df155b505156f2ff73ea5fbed 100644
--- a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
+++ b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
@@ -713,35 +713,39 @@ s8 phy_get_tx_pwr_lmt(struct adapter *adapter, u32 reg_pwr_tbl_sel,
 	return pwr_lmt;
 }
 
+static void __phy_tx_power_limit_to_index(struct hal_com_data *hal_data, struct registry_priv *r,
+					  u8 regulation, u8 bw, u8 channel, u8 rs)
+{
+	s8 tmp_pwr_lmt = hal_data->TxPwrLimit_2_4G[regulation][bw][rs][channel][RF_PATH_A];
+	s8 tmp;
+	u8 path;
+	u8 pwr_base;
+
+	for (path = RF_PATH_A; path < MAX_RF_PATH_NUM; ++path) {
+		if (hal_data->odmpriv.PhyRegPgValueType == PHY_REG_PG_EXACT_VALUE)
+			pwr_base = hal_data->TxPwrByRateBase2_4G[path][rs];
+		else
+			pwr_base = r->reg_power_base * 2;
+
+		if (tmp_pwr_lmt != MAX_POWER_INDEX) {
+			tmp = tmp_pwr_lmt - pwr_base;
+			hal_data->TxPwrLimit_2_4G[regulation][bw][rs][channel][path] = tmp;
+		}
+	}
+}
+
 void phy_tx_power_limit_to_index(struct adapter *adapter)
 {
 	struct hal_com_data *hal_data = GET_HAL_DATA(adapter);
 	struct registry_priv *r = &adapter->registrypriv;
-	u8 pwr_base = 0x2E;
 	u8 regulation, bw, channel, rs;
-	s8 tmp = 0, tmp_pwr_lmt = 0;
-	u8 path = 0;
 
 	for (regulation = 0; regulation < MAX_REGULATION_NUM; ++regulation) {
-		for (bw = 0; bw < MAX_2_4G_BANDWIDTH_NUM; ++bw) {
-			for (channel = 0; channel < CHANNEL_MAX_NUMBER_2G; ++channel) {
-				for (rs = 0; rs < MAX_RATE_SECTION_NUM; ++rs) {
-					tmp_pwr_lmt = hal_data->TxPwrLimit_2_4G[regulation][bw][rs][channel][RF_PATH_A];
-
-					for (path = RF_PATH_A; path < MAX_RF_PATH_NUM; ++path) {
-						if (hal_data->odmpriv.PhyRegPgValueType == PHY_REG_PG_EXACT_VALUE)
-							pwr_base = hal_data->TxPwrByRateBase2_4G[path][rs];
-						else
-							pwr_base = r->reg_power_base * 2;
-
-						if (tmp_pwr_lmt != MAX_POWER_INDEX) {
-							tmp = tmp_pwr_lmt - pwr_base;
-							hal_data->TxPwrLimit_2_4G[regulation][bw][rs][channel][path] = tmp;
-						}
-					}
-				}
-			}
-		}
+		for (bw = 0; bw < MAX_2_4G_BANDWIDTH_NUM; ++bw)
+			for (channel = 0; channel < CHANNEL_MAX_NUMBER_2G; ++channel)
+				for (rs = 0; rs < MAX_RATE_SECTION_NUM; ++rs)
+					__phy_tx_power_limit_to_index(hal_data, r, regulation, bw,
+								      channel, rs);
 	}
 }
 

-- 
2.47.3

Re: [PATCH v2 0/3] staging: rtl8723bs: refactor PHY_ConvertTxPowerLimitToPowerIndex()

[Greg Kroah-Hartman]

On Fri, Oct 02, 2026 at 01:07:34AM -0300, Leonardo Martins Martins wrote:
> The function PHY_ConvertTxPowerLimitToPowerIndex() has several style
> problems, such as mixed-case names, deeply nested code, and redundant
> bounds checking.
> 
> Patch 1 renames mixed-case variables that are on the same scope as this
> function, struct fields such as TxPwrLimit_2_4G were left as is. Patch 2
> removes redundant parts of the code, and patch 3 solves the nesting
> issue by extracting the inner loop into an auxiliary function like it is
> done in rtw_phy_tx_power_limit_config(), at
> drivers/net/wireless/realtek/rtw88/phy.c
> 
> Checkpatch issues a few warnings and checks for deep nesting and long
> lines, but these are solved by the last patch.
> 
> Compile tested only.

When doing logic refactoring, testing on the real hardware is best.

thanks,

greg k-h

Re: [PATCH v2 0/3] staging: rtl8723bs: refactor PHY_ConvertTxPowerLimitToPowerIndex()

[Leonardo Martins Martins]

> When doing logic refactoring, testing on the real hardware is best.

Makes sense, so should I be dropping patch 2 where the logic is
refactored, but still do patch 3 where the inner loop is extracted into
__phy_tx_power_limit_to_index() to reduce all the indentations?

Best Regards,
Leonardo Martins

Re: [PATCH v2 0/3] staging: rtl8723bs: refactor PHY_ConvertTxPowerLimitToPowerIndex()

[Greg Kroah-Hartman]

On Fri, Oct 02, 2026 at 04:39:46AM -0300, Leonardo Martins Martins wrote:
> > When doing logic refactoring, testing on the real hardware is best.
> 
> Makes sense, so should I be dropping patch 2 where the logic is
> refactored, but still do patch 3 where the inner loop is extracted into
> __phy_tx_power_limit_to_index() to reduce all the indentations?

Yes, that's probably best as it's easiest to "prove" it's correct by
just looking at it.

thanks,

greg k-h