2023-12-31 22:19:15

by Bitterblue Smith

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 <[email protected]>
---
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

2024-01-01 13:12:59

by Jes Sorensen

Subject: Re: [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate

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 <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Signed-off-by: Jes Sorensen <[email protected]>

2024-01-02 00:46:02

by Ping-Ke Shih

Subject: RE: [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate

> -----Original Message-----
> From: Bitterblue Smith <[email protected]>
> Sent: Monday, January 1, 2024 6:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Ping-Ke Shih <[email protected]>
> 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 <[email protected]>
> ---
> 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

2024-01-02 12:22:17

by Bitterblue Smith

Subject: Re: [PATCH] wifi: rtl8xxxu: Fix off by one initial RTS rate

On 02/01/2024 02:45, Ping-Ke Shih wrote:
>
>
>> -----Original Message-----
>> From: Bitterblue Smith <[email protected]>
>> Sent: Monday, January 1, 2024 6:19 AM
>> To: [email protected]
>> Cc: Jes Sorensen <[email protected]>; Ping-Ke Shih <[email protected]>
>> 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 <[email protected]>
>> ---
>> 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.