[PATCH] staging: rtl8723bs: remove copy function

Bera Yüzlü posted 1 patch 2 weeks, 1 day ago
drivers/staging/rtl8723bs/hal/hal_com.c       | 19 -------------------
.../staging/rtl8723bs/hal/hal_com_phycfg.c    | 14 ++++++++++----
drivers/staging/rtl8723bs/include/hal_com.h   |  2 --
3 files changed, 10 insertions(+), 25 deletions(-)
[PATCH] staging: rtl8723bs: remove copy function
Posted by Bera Yüzlü 2 weeks, 1 day ago
GetU1ByteIntegerFromStringInDecimal() is a copy of kstrtou8().
Remove its usages to kstrtou8() and check the return value.

Signed-off-by: Bera Yüzlü <b9788213@gmail.com>
---
 drivers/staging/rtl8723bs/hal/hal_com.c       | 19 -------------------
 .../staging/rtl8723bs/hal/hal_com_phycfg.c    | 14 ++++++++++----
 drivers/staging/rtl8723bs/include/hal_com.h   |  2 --
 3 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/hal_com.c b/drivers/staging/rtl8723bs/hal/hal_com.c
index 50370b14ce7c..4f4a430c9f87 100644
--- a/drivers/staging/rtl8723bs/hal/hal_com.c
+++ b/drivers/staging/rtl8723bs/hal/hal_com.c
@@ -751,25 +751,6 @@ void SetHalODMVar(
 	}
 }
 
-
-bool GetU1ByteIntegerFromStringInDecimal(char *Str, u8 *pInt)
-{
-	u16 i = 0;
-	*pInt = 0;
-
-	while (Str[i] != '\0') {
-		if (Str[i] >= '0' && Str[i] <= '9') {
-			*pInt *= 10;
-			*pInt += (Str[i] - '0');
-		} else
-			return false;
-
-		++i;
-	}
-
-	return true;
-}
-
 void rtw_hal_check_rxfifo_full(struct adapter *adapter)
 {
 	/* switch counter to RX fifo */
diff --git a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
index bdd595a99b98..447b72954804 100644
--- a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
+++ b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
@@ -816,11 +816,17 @@ void PHY_SetTxPowerLimit(
 )
 {
 	struct hal_com_data	*pHalData = GET_HAL_DATA(Adapter);
-	u8 regulation = 0, bandwidth = 0, rateSection = 0, channel;
-	s8 powerLimit = 0, prevPowerLimit, channelIndex;
+	u8 regulation = 0, bandwidth = 0, rateSection = 0, channel, powerLimit;
+	s8 prevPowerLimit, channelIndex;
+	int ret;
 
-	GetU1ByteIntegerFromStringInDecimal((s8 *)Channel, &channel);
-	GetU1ByteIntegerFromStringInDecimal((s8 *)PowerLimit, &powerLimit);
+	ret = kstrtou8((const char *)Channel, 10, &channel);
+	if (ret)
+		return;
+
+	ret = kstrtou8((const char *)PowerLimit, 10, &powerLimit);
+	if (ret)
+		return;
 
 	powerLimit = powerLimit > MAX_POWER_INDEX ? MAX_POWER_INDEX : powerLimit;
 
diff --git a/drivers/staging/rtl8723bs/include/hal_com.h b/drivers/staging/rtl8723bs/include/hal_com.h
index 483f0390addc..7c67fee148fa 100644
--- a/drivers/staging/rtl8723bs/include/hal_com.h
+++ b/drivers/staging/rtl8723bs/include/hal_com.h
@@ -141,8 +141,6 @@ void rtw_hal_check_rxfifo_full(struct adapter *adapter);
 u8 GetHalDefVar(struct adapter *adapter, enum hal_def_variable variable,
 		void *value);
 
-bool GetU1ByteIntegerFromStringInDecimal(char *str, u8 *in);
-
 #define		HWSET_MAX_SIZE			512
 
 void rtw_bb_rf_gain_offset(struct adapter *padapter);
-- 
2.53.0

Re: [PATCH] staging: rtl8723bs: remove copy function
Posted by Andy Shevchenko 2 weeks ago
On Fri, Mar 20, 2026 at 10:18:31AM +0300, Bera Yüzlü wrote:
> GetU1ByteIntegerFromStringInDecimal() is a copy of kstrtou8().
> Remove its usages to kstrtou8() and check the return value.

...

> void PHY_SetTxPowerLimit( ... )
>  {
>  	struct hal_com_data	*pHalData = GET_HAL_DATA(Adapter);
> -	u8 regulation = 0, bandwidth = 0, rateSection = 0, channel;
> -	s8 powerLimit = 0, prevPowerLimit, channelIndex;
> +	u8 regulation = 0, bandwidth = 0, rateSection = 0, channel, powerLimit;
> +	s8 prevPowerLimit, channelIndex;
> +	int ret;
>  
> -	GetU1ByteIntegerFromStringInDecimal((s8 *)Channel, &channel);
> -	GetU1ByteIntegerFromStringInDecimal((s8 *)PowerLimit, &powerLimit);
> +	ret = kstrtou8((const char *)Channel, 10, &channel);
> +	if (ret)
> +		return;
> +
> +	ret = kstrtou8((const char *)PowerLimit, 10, &powerLimit);
> +	if (ret)
> +		return;

This will change behaviour on the invalid data. Before it was just partially
converted here and continue, now it breaks an execution. The commit message
does not explain if it's safe to do or not (i.o.w. if there is a guarantee
that in current state the input will be always in the correct form).

Otherwise, you probably want to use one of simple_strtou*().

>  	powerLimit = powerLimit > MAX_POWER_INDEX ? MAX_POWER_INDEX : powerLimit;

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] staging: rtl8723bs: remove copy function
Posted by Bera Yüzlü 2 weeks ago
On Fri, 20 Mar 2026 08:27:22 +0000, Andy Shevchenko wrote:
> > void PHY_SetTxPowerLimit( ... )
> >  {
> >  	struct hal_com_data	*pHalData = GET_HAL_DATA(Adapter);
> > -	u8 regulation = 0, bandwidth = 0, rateSection = 0, channel;
> > -	s8 powerLimit = 0, prevPowerLimit, channelIndex;
> > +	u8 regulation = 0, bandwidth = 0, rateSection = 0, channel, powerLimit;
> > +	s8 prevPowerLimit, channelIndex;
> > +	int ret;
> >  
> > -	GetU1ByteIntegerFromStringInDecimal((s8 *)Channel, &channel);
> > -	GetU1ByteIntegerFromStringInDecimal((s8 *)PowerLimit, &powerLimit);
> > +	ret = kstrtou8((const char *)Channel, 10, &channel);
> > +	if (ret)
> > +		return;
> > +
> > +	ret = kstrtou8((const char *)PowerLimit, 10, &powerLimit);
> > +	if (ret)
> > +		return;
> 
> This will change behaviour on the invalid data. Before it was just partially
> converted here and continue, now it breaks an execution. The commit message
> does not explain if it's safe to do or not (i.o.w. if there is a guarantee
> that in current state the input will be always in the correct form).

> Otherwise, you probably want to use one of simple_strtou*().

It only indirectly called from ODM_ReadAndConfig_MP_8723B_TXPWR_LMT() with constant values.
Maybe we can convert this strings to ints so we don't need to do it at runtime.
We can also remove the wrapper odm_ConfigBB_TXPWR_LMT_8723B(). This should be a seperate
patch, right?

Thanks,
Bera
Re: [PATCH] staging: rtl8723bs: remove copy function
Posted by Bera Yüzlü 2 weeks ago
On Fri, 20 Mar 2026 11:25:08 -0700, Bera Yüzlü wrote:
> On Fri, 20 Mar 2026 08:27:22 +0000, Andy Shevchenko wrote:
> > > void PHY_SetTxPowerLimit( ... )
> > >  {
> > >  	struct hal_com_data	*pHalData = GET_HAL_DATA(Adapter);
> > > -	u8 regulation = 0, bandwidth = 0, rateSection = 0, channel;
> > > -	s8 powerLimit = 0, prevPowerLimit, channelIndex;
> > > +	u8 regulation = 0, bandwidth = 0, rateSection = 0, channel, powerLimit;
> > > +	s8 prevPowerLimit, channelIndex;
> > > +	int ret;
> > >  
> > > -	GetU1ByteIntegerFromStringInDecimal((s8 *)Channel, &channel);
> > > -	GetU1ByteIntegerFromStringInDecimal((s8 *)PowerLimit, &powerLimit);
> > > +	ret = kstrtou8((const char *)Channel, 10, &channel);
> > > +	if (ret)
> > > +		return;
> > > +
> > > +	ret = kstrtou8((const char *)PowerLimit, 10, &powerLimit);
> > > +	if (ret)
> > > +		return;
> > 
> > This will change behaviour on the invalid data. Before it was just partially
> > converted here and continue, now it breaks an execution. The commit message
> > does not explain if it's safe to do or not (i.o.w. if there is a guarantee
> > that in current state the input will be always in the correct form).
>
> > Otherwise, you probably want to use one of simple_strtou*().
>
> It only indirectly called from ODM_ReadAndConfig_MP_8723B_TXPWR_LMT() with constant values.
> Maybe we can convert this strings to ints so we don't need to do it at runtime.
> We can also remove the wrapper odm_ConfigBB_TXPWR_LMT_8723B(). This should be a seperate
> patch, right?
> 
> Thanks,
> Bera

Just noticed ODM_ReadAndConfig_MP_8723B_TXPWR_LMT() didn't called anywhere.
I will remove:
GetU1ByteIntegerFromStringInDecimal()
PHY_SetTxPowerLimit()
odm_ConfigBB_TXPWR_LMT_8723B()
ODM_ReadAndConfig_MP_8723B_TXPWR_LMT()
in v2. They are all unusued.

Thanks,
Bera.
Re: [PATCH] staging: rtl8723bs: remove copy function
Posted by Andy Shevchenko 2 weeks ago
On Fri, Mar 20, 2026 at 10:19:59PM +0300, Bera Yüzlü wrote:
> On Fri, 20 Mar 2026 11:25:08 -0700, Bera Yüzlü wrote:
> > On Fri, 20 Mar 2026 08:27:22 +0000, Andy Shevchenko wrote:

...

> > > > -	GetU1ByteIntegerFromStringInDecimal((s8 *)Channel, &channel);
> > > > -	GetU1ByteIntegerFromStringInDecimal((s8 *)PowerLimit, &powerLimit);
> > > > +	ret = kstrtou8((const char *)Channel, 10, &channel);
> > > > +	if (ret)
> > > > +		return;
> > > > +
> > > > +	ret = kstrtou8((const char *)PowerLimit, 10, &powerLimit);
> > > > +	if (ret)
> > > > +		return;
> > > 
> > > This will change behaviour on the invalid data. Before it was just partially
> > > converted here and continue, now it breaks an execution. The commit message
> > > does not explain if it's safe to do or not (i.o.w. if there is a guarantee
> > > that in current state the input will be always in the correct form).
> >
> > > Otherwise, you probably want to use one of simple_strtou*().
> >
> > It only indirectly called from ODM_ReadAndConfig_MP_8723B_TXPWR_LMT() with constant values.
> > Maybe we can convert this strings to ints so we don't need to do it at runtime.
> > We can also remove the wrapper odm_ConfigBB_TXPWR_LMT_8723B(). This should be a seperate
> > patch, right?
> 
> Just noticed ODM_ReadAndConfig_MP_8723B_TXPWR_LMT() didn't called anywhere.
> I will remove:
> GetU1ByteIntegerFromStringInDecimal()
> PHY_SetTxPowerLimit()
> odm_ConfigBB_TXPWR_LMT_8723B()
> ODM_ReadAndConfig_MP_8723B_TXPWR_LMT()
> in v2. They are all unusued.

Sounds good to me.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] staging: rtl8723bs: remove copy function
Posted by Dan Carpenter 2 weeks ago
On Fri, Mar 20, 2026 at 10:18:31AM +0300, Bera Yüzlü wrote:
> GetU1ByteIntegerFromStringInDecimal() is a copy of kstrtou8().
> Remove its usages to kstrtou8() and check the return value.
> 
> Signed-off-by: Bera Yüzlü <b9788213@gmail.com>
> ---

Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter