* [PATCH] staging: rtl8723bs: remove copy function
@ 2026-03-20 7:18 Bera Yüzlü
2026-03-20 8:25 ` Dan Carpenter
2026-03-20 8:27 ` Andy Shevchenko
0 siblings, 2 replies; 6+ messages in thread
From: Bera Yüzlü @ 2026-03-20 7:18 UTC (permalink / raw)
To: gregkh
Cc: straube.linux, andriy.shevchenko, ethantidmore06, hansg, Yeking,
rayfraytech, linux-staging, linux-kernel, Bera Yüzlü
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: rtl8723bs: remove copy function
2026-03-20 7:18 [PATCH] staging: rtl8723bs: remove copy function Bera Yüzlü
@ 2026-03-20 8:25 ` Dan Carpenter
2026-03-20 8:27 ` Andy Shevchenko
1 sibling, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2026-03-20 8:25 UTC (permalink / raw)
To: Bera Yüzlü
Cc: gregkh, straube.linux, andriy.shevchenko, ethantidmore06, hansg,
Yeking, rayfraytech, linux-staging, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: rtl8723bs: remove copy function
2026-03-20 7:18 [PATCH] staging: rtl8723bs: remove copy function Bera Yüzlü
2026-03-20 8:25 ` Dan Carpenter
@ 2026-03-20 8:27 ` Andy Shevchenko
2026-03-20 18:24 ` Bera Yüzlü
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2026-03-20 8:27 UTC (permalink / raw)
To: Bera Yüzlü
Cc: gregkh, straube.linux, ethantidmore06, hansg, Yeking, rayfraytech,
linux-staging, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: rtl8723bs: remove copy function
2026-03-20 8:27 ` Andy Shevchenko
@ 2026-03-20 18:24 ` Bera Yüzlü
2026-03-20 19:19 ` Bera Yüzlü
0 siblings, 1 reply; 6+ messages in thread
From: Bera Yüzlü @ 2026-03-20 18:24 UTC (permalink / raw)
To: andriy.shevchenko
Cc: Yeking, b9788213, ethantidmore06, gregkh, hansg, linux-kernel,
linux-staging, rayfraytech, straube.linux
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: rtl8723bs: remove copy function
2026-03-20 18:24 ` Bera Yüzlü
@ 2026-03-20 19:19 ` Bera Yüzlü
2026-03-20 19:26 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Bera Yüzlü @ 2026-03-20 19:19 UTC (permalink / raw)
To: b9788213
Cc: Yeking, andriy.shevchenko, ethantidmore06, gregkh, hansg,
linux-kernel, linux-staging, rayfraytech, straube.linux
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: rtl8723bs: remove copy function
2026-03-20 19:19 ` Bera Yüzlü
@ 2026-03-20 19:26 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2026-03-20 19:26 UTC (permalink / raw)
To: Bera Yüzlü
Cc: Yeking, ethantidmore06, gregkh, hansg, linux-kernel,
linux-staging, rayfraytech, straube.linux
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-03-20 19:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-20 7:18 [PATCH] staging: rtl8723bs: remove copy function Bera Yüzlü
2026-03-20 8:25 ` Dan Carpenter
2026-03-20 8:27 ` Andy Shevchenko
2026-03-20 18:24 ` Bera Yüzlü
2026-03-20 19:19 ` Bera Yüzlü
2026-03-20 19:26 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox