public inbox for linux-wireless@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate
@ 2023-12-31 22:19 Bitterblue Smith
  2024-01-01 13:12 ` Jes Sorensen
  2024-01-02  0:45 ` Ping-Ke Shih
  0 siblings, 2 replies; 4+ messages in thread
From: Bitterblue Smith @ 2023-12-31 22:19 UTC (permalink / raw)
  To: linux-wireless@vger.kernel.org; +Cc: Jes Sorensen, Ping-Ke Shih

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++;
 	}
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate
  2023-12-31 22:19 [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate Bitterblue Smith
@ 2024-01-01 13:12 ` Jes Sorensen
  2024-01-02  0:45 ` Ping-Ke Shih
  1 sibling, 0 replies; 4+ messages in thread
From: Jes Sorensen @ 2024-01-01 13:12 UTC (permalink / raw)
  To: Bitterblue Smith, linux-wireless@vger.kernel.org; +Cc: Ping-Ke Shih

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>



^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate
  2023-12-31 22:19 [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate Bitterblue Smith
  2024-01-01 13:12 ` Jes Sorensen
@ 2024-01-02  0:45 ` Ping-Ke Shih
  2024-01-02 12:22   ` Bitterblue Smith
  1 sibling, 1 reply; 4+ messages in thread
From: Ping-Ke Shih @ 2024-01-02  0:45 UTC (permalink / raw)
  To: Bitterblue Smith, linux-wireless@vger.kernel.org; +Cc: Jes Sorensen



> -----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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate
  2024-01-02  0:45 ` Ping-Ke Shih
@ 2024-01-02 12:22   ` Bitterblue Smith
  0 siblings, 0 replies; 4+ messages in thread
From: Bitterblue Smith @ 2024-01-02 12:22 UTC (permalink / raw)
  To: Ping-Ke Shih, linux-wireless@vger.kernel.org; +Cc: Jes Sorensen

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-01-02 12:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-31 22:19 [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate Bitterblue Smith
2024-01-01 13:12 ` Jes Sorensen
2024-01-02  0:45 ` Ping-Ke Shih
2024-01-02 12:22   ` Bitterblue Smith

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox