* [PATCH] wifi: rtl8xxxu: Fix gen1 rate mask command
@ 2024-04-15 21:10 Bitterblue Smith
2024-04-16 4:01 ` Ping-Ke Shih
0 siblings, 1 reply; 3+ messages in thread
From: Bitterblue Smith @ 2024-04-15 21:10 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org; +Cc: Jes Sorensen, Ping-Ke Shih
The H2C (host to card) command which tells the firmware which TX rates
it can use is slightly wrong. Fix the order of the bytes.
Also put the macid in the command (relevant for AP mode).
This was tested with RTL8192CU. It also affects the RTL8723AU.
Cc: stable@vger.kernel.org
Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 10 +++++++---
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 18 +++++++++++-------
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index fd92d23c43d9..ca44d82cb5aa 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1430,10 +1430,14 @@ struct h2c_cmd {
u8 data;
} __packed joinbss;
struct {
+#define RAID_MASK GENMASK(31, 28)
+#define RATE_MASK_MASK GENMASK(27, 0)
+#define MACID_MASK GENMASK(4, 0)
+#define SHORT_GI_MASK BIT(5)
+
u8 cmd;
- __le16 mask_hi;
- u8 arg;
- __le16 mask_lo;
+ __le32 rate_mask_and_raid;
+ u8 macid_and_short_gi;
} __packed ramask;
struct {
u8 cmd;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index fac7824ae727..acbafc25c6e0 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4641,15 +4641,19 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
memset(&h2c, 0, sizeof(struct h2c_cmd));
h2c.ramask.cmd = H2C_SET_RATE_MASK;
- h2c.ramask.mask_lo = cpu_to_le16(ramask & 0xffff);
- h2c.ramask.mask_hi = cpu_to_le16(ramask >> 16);
- h2c.ramask.arg = 0x80;
- if (sgi)
- h2c.ramask.arg |= 0x20;
+ le32p_replace_bits(&h2c.ramask.rate_mask_and_raid, rateid, RAID_MASK);
+ le32p_replace_bits(&h2c.ramask.rate_mask_and_raid, ramask, RATE_MASK_MASK);
+
+ u8p_replace_bits(&h2c.ramask.macid_and_short_gi, macid, MACID_MASK);
+ u8p_replace_bits(&h2c.ramask.macid_and_short_gi, sgi, SHORT_GI_MASK);
+ u8p_replace_bits(&h2c.ramask.macid_and_short_gi, 1, BIT(7));
+
+ dev_dbg(&priv->udev->dev,
+ "%s: rate mask %08x, rate id %02x, arg %02x, size %zi\n",
+ __func__, ramask, rateid, h2c.ramask.macid_and_short_gi,
+ sizeof(h2c.ramask));
- dev_dbg(&priv->udev->dev, "%s: rate mask %08x, arg %02x, size %zi\n",
- __func__, ramask, h2c.ramask.arg, sizeof(h2c.ramask));
rtl8xxxu_gen1_h2c_cmd(priv, &h2c, sizeof(h2c.ramask));
}
--
2.44.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* RE: [PATCH] wifi: rtl8xxxu: Fix gen1 rate mask command
2024-04-15 21:10 [PATCH] wifi: rtl8xxxu: Fix gen1 rate mask command Bitterblue Smith
@ 2024-04-16 4:01 ` Ping-Ke Shih
2024-04-16 11:58 ` Bitterblue Smith
0 siblings, 1 reply; 3+ messages in thread
From: Ping-Ke Shih @ 2024-04-16 4:01 UTC (permalink / raw)
To: Bitterblue Smith, linux-wireless@vger.kernel.org; +Cc: Jes Sorensen
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>
> The H2C (host to card) command which tells the firmware which TX rates
> it can use is slightly wrong. Fix the order of the bytes.
>
> Also put the macid in the command (relevant for AP mode).
>
> This was tested with RTL8192CU. It also affects the RTL8723AU.
Can you add test results before/after this patch?
I wonder if RTL8192CU and RTL8723AU use different command format, because
vendor driver of RTL8192CU seems use different command ID (w/o BIT(7)), see below.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 10 +++++++---
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 18 +++++++++++-------
> 2 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index fd92d23c43d9..ca44d82cb5aa 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1430,10 +1430,14 @@ struct h2c_cmd {
> u8 data;
> } __packed joinbss;
> struct {
> +#define RAID_MASK GENMASK(31, 28)
> +#define RATE_MASK_MASK GENMASK(27, 0)
> +#define MACID_MASK GENMASK(4, 0)
> +#define SHORT_GI_MASK BIT(5)
> +
> u8 cmd;
> - __le16 mask_hi;
> - u8 arg;
> - __le16 mask_lo;
> + __le32 rate_mask_and_raid;
> + u8 macid_and_short_gi;
> } __packed ramask;
> struct {
> u8 cmd;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index fac7824ae727..acbafc25c6e0 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4641,15 +4641,19 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> memset(&h2c, 0, sizeof(struct h2c_cmd));
>
> h2c.ramask.cmd = H2C_SET_RATE_MASK;
rtl8xxxu: H2C_SET_RATE_MASK = (6 | H2C_EXT)
vendor driver of 8192cu: MACID_CONFIG_EID=6
Can you confirm if command "(6 | BIT(7))" works on 8192cu?
Maybe, it only works on 8723au? (this chip is too old, I don't have more info about it)
> - h2c.ramask.mask_lo = cpu_to_le16(ramask & 0xffff);
> - h2c.ramask.mask_hi = cpu_to_le16(ramask >> 16);
>
> - h2c.ramask.arg = 0x80;
> - if (sgi)
> - h2c.ramask.arg |= 0x20;
> + le32p_replace_bits(&h2c.ramask.rate_mask_and_raid, rateid, RAID_MASK);
> + le32p_replace_bits(&h2c.ramask.rate_mask_and_raid, ramask, RATE_MASK_MASK);
> +
> + u8p_replace_bits(&h2c.ramask.macid_and_short_gi, macid, MACID_MASK);
> + u8p_replace_bits(&h2c.ramask.macid_and_short_gi, sgi, SHORT_GI_MASK);
> + u8p_replace_bits(&h2c.ramask.macid_and_short_gi, 1, BIT(7));
> +
> + dev_dbg(&priv->udev->dev,
> + "%s: rate mask %08x, rate id %02x, arg %02x, size %zi\n",
> + __func__, ramask, rateid, h2c.ramask.macid_and_short_gi,
> + sizeof(h2c.ramask));
>
> - dev_dbg(&priv->udev->dev, "%s: rate mask %08x, arg %02x, size %zi\n",
> - __func__, ramask, h2c.ramask.arg, sizeof(h2c.ramask));
> rtl8xxxu_gen1_h2c_cmd(priv, &h2c, sizeof(h2c.ramask));
> }
>
> --
> 2.44.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] wifi: rtl8xxxu: Fix gen1 rate mask command
2024-04-16 4:01 ` Ping-Ke Shih
@ 2024-04-16 11:58 ` Bitterblue Smith
0 siblings, 0 replies; 3+ messages in thread
From: Bitterblue Smith @ 2024-04-16 11:58 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless@vger.kernel.org; +Cc: Jes Sorensen
On 16/04/2024 07:01, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>>
>> The H2C (host to card) command which tells the firmware which TX rates
>> it can use is slightly wrong. Fix the order of the bytes.
>>
>> Also put the macid in the command (relevant for AP mode).
>>
>> This was tested with RTL8192CU. It also affects the RTL8723AU.
>
> Can you add test results before/after this patch?
>
> I wonder if RTL8192CU and RTL8723AU use different command format, because
> vendor driver of RTL8192CU seems use different command ID (w/o BIT(7)), see below.
>
They use the same format, the vendor drivers just add BIT(7)
in rtl8192c_FillH2CCmd function:
https://github.com/lwfinger/rtl8192cu/blob/52b20929a8f68d5fbae38e6e253f75b713258487/hal/rtl8192c_cmd.c#L114
Please ignore this patch. I made a mistake: the order of the
bytes in the struct is weird, but it is correct for rtl8xxxu.
rtl8xxxu_gen1_h2c_cmd is aware of the changed order.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> ---
>> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 10 +++++++---
>> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 18 +++++++++++-------
>> 2 files changed, 18 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> index fd92d23c43d9..ca44d82cb5aa 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> @@ -1430,10 +1430,14 @@ struct h2c_cmd {
>> u8 data;
>> } __packed joinbss;
>> struct {
>> +#define RAID_MASK GENMASK(31, 28)
>> +#define RATE_MASK_MASK GENMASK(27, 0)
>> +#define MACID_MASK GENMASK(4, 0)
>> +#define SHORT_GI_MASK BIT(5)
>> +
>> u8 cmd;
>> - __le16 mask_hi;
>> - u8 arg;
>> - __le16 mask_lo;
>> + __le32 rate_mask_and_raid;
>> + u8 macid_and_short_gi;
>> } __packed ramask;
>> struct {
>> u8 cmd;
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index fac7824ae727..acbafc25c6e0 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -4641,15 +4641,19 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
>> memset(&h2c, 0, sizeof(struct h2c_cmd));
>>
>> h2c.ramask.cmd = H2C_SET_RATE_MASK;
>
> rtl8xxxu: H2C_SET_RATE_MASK = (6 | H2C_EXT)
> vendor driver of 8192cu: MACID_CONFIG_EID=6
>
> Can you confirm if command "(6 | BIT(7))" works on 8192cu?
> Maybe, it only works on 8723au? (this chip is too old, I don't have more info about it)
>
>
>> - h2c.ramask.mask_lo = cpu_to_le16(ramask & 0xffff);
>> - h2c.ramask.mask_hi = cpu_to_le16(ramask >> 16);
>>
>> - h2c.ramask.arg = 0x80;
>> - if (sgi)
>> - h2c.ramask.arg |= 0x20;
>> + le32p_replace_bits(&h2c.ramask.rate_mask_and_raid, rateid, RAID_MASK);
>> + le32p_replace_bits(&h2c.ramask.rate_mask_and_raid, ramask, RATE_MASK_MASK);
>> +
>> + u8p_replace_bits(&h2c.ramask.macid_and_short_gi, macid, MACID_MASK);
>> + u8p_replace_bits(&h2c.ramask.macid_and_short_gi, sgi, SHORT_GI_MASK);
>> + u8p_replace_bits(&h2c.ramask.macid_and_short_gi, 1, BIT(7));
>> +
>> + dev_dbg(&priv->udev->dev,
>> + "%s: rate mask %08x, rate id %02x, arg %02x, size %zi\n",
>> + __func__, ramask, rateid, h2c.ramask.macid_and_short_gi,
>> + sizeof(h2c.ramask));
>>
>> - dev_dbg(&priv->udev->dev, "%s: rate mask %08x, arg %02x, size %zi\n",
>> - __func__, ramask, h2c.ramask.arg, sizeof(h2c.ramask));
>> rtl8xxxu_gen1_h2c_cmd(priv, &h2c, sizeof(h2c.ramask));
>> }
>>
>> --
>> 2.44.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-04-16 11:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-15 21:10 [PATCH] wifi: rtl8xxxu: Fix gen1 rate mask command Bitterblue Smith
2024-04-16 4:01 ` Ping-Ke Shih
2024-04-16 11:58 ` Bitterblue Smith
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox