diff mbox series

wifi: rtl8xxxu: Fix off by one initial RTS rate

Message ID be0427a7-f85f-449e-a6c8-b1c8371c39b0@gmail.com
State New
Headers show
Series wifi: rtl8xxxu: Fix off by one initial RTS rate | expand

Commit Message

Bitterblue Smith Dec. 31, 2023, 10:19 p.m. UTC
rtl8xxxu_set_basic_rates() sets the wrong initial RTS rate. It sets the
next higher rate than the one it should set, e.g. 36M instead of 24M.

The while loop is supposed to find the index of the most significant
bit which is 1.

Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jes Sorensen Jan. 1, 2024, 1:12 p.m. UTC | #1
On 12/31/23 17:19, Bitterblue Smith wrote:
> rtl8xxxu_set_basic_rates() sets the wrong initial RTS rate. It sets the
> next higher rate than the one it should set, e.g. 36M instead of 24M.
> 
> The while loop is supposed to find the index of the most significant
> bit which is 1.
> 
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Signed-off-by: Jes Sorensen <Jes.Sorensen@gmail.com>
Ping-Ke Shih Jan. 2, 2024, 12:45 a.m. UTC | #2
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Monday, January 1, 2024 6:19 AM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
> Subject: [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate
> 
> rtl8xxxu_set_basic_rates() sets the wrong initial RTS rate. It sets the
> next higher rate than the one it should set, e.g. 36M instead of 24M.
> 
> The while loop is supposed to find the index of the most significant
> bit which is 1.
> 
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 180907319e8c..b9f3382bd67c 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4839,7 +4839,7 @@ static void rtl8xxxu_set_basic_rates(struct rtl8xxxu_priv *priv, u32 rate_cfg)
> 
>         dev_dbg(&priv->udev->dev, "%s: rates %08x\n", __func__, rate_cfg);
> 
> -       while (rate_cfg) {
> +       while (rate_cfg > 1) {
>                 rate_cfg = (rate_cfg >> 1);
>                 rate_idx++;
>         }

How about using __fls()? 

if (rate_cfg)
	rate_idx = __fls(rate_cfg);

> --
> 2.43.0
Bitterblue Smith Jan. 2, 2024, 12:22 p.m. UTC | #3
On 02/01/2024 02:45, Ping-Ke Shih wrote:
> 
> 
>> -----Original Message-----
>> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> Sent: Monday, January 1, 2024 6:19 AM
>> To: linux-wireless@vger.kernel.org
>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
>> Subject: [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate
>>
>> rtl8xxxu_set_basic_rates() sets the wrong initial RTS rate. It sets the
>> next higher rate than the one it should set, e.g. 36M instead of 24M.
>>
>> The while loop is supposed to find the index of the most significant
>> bit which is 1.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> ---
>>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 180907319e8c..b9f3382bd67c 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -4839,7 +4839,7 @@ static void rtl8xxxu_set_basic_rates(struct rtl8xxxu_priv *priv, u32 rate_cfg)
>>
>>         dev_dbg(&priv->udev->dev, "%s: rates %08x\n", __func__, rate_cfg);
>>
>> -       while (rate_cfg) {
>> +       while (rate_cfg > 1) {
>>                 rate_cfg = (rate_cfg >> 1);
>>                 rate_idx++;
>>         }
> 
> How about using __fls()? 
> 
> if (rate_cfg)
> 	rate_idx = __fls(rate_cfg);
> 
Yes, that's nicer.
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 180907319e8c..b9f3382bd67c 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4839,7 +4839,7 @@  static void rtl8xxxu_set_basic_rates(struct rtl8xxxu_priv *priv, u32 rate_cfg)
 
 	dev_dbg(&priv->udev->dev, "%s: rates %08x\n", __func__,	rate_cfg);
 
-	while (rate_cfg) {
+	while (rate_cfg > 1) {
 		rate_cfg = (rate_cfg >> 1);
 		rate_idx++;
 	}