[PATCH v1 0/3] wifi: rtl8xxxu: AP mode fixes (for gen1 devices)

[PATCH v1 0/3] wifi: rtl8xxxu: AP mode fixes (for gen1 devices)

[Ezra Buehler]

From: Ezra Buehler <ezra.buehler@husqvarnagroup.com>

As mentioned in the previous patch [1], especially the transmit rate is
rather low in AP mode with gen1 devices like the RTL8192CU. The
following changes seem to fix the issue.

When looking at the patches introducing the initial AP support [2], I
noticed that rtl8xxxu_gen2_report_connect() was altered too. So, now I
am wondering if we need to use `macid` and `role` in
rtl8xxxu_gen1_report_connect() as well...

[1]: https://lore.kernel.org/all/20250122071512.10165-1-ezra@easyb.ch/
[2]: https://lore.kernel.org/all/20230428150833.218605-1-martin.kaistra@linutronix.de/


Ezra Buehler (3):
  wifi: rtl8xxxu: Use macid in rtl8xxxu_update_rate_mask()
  wifi: rtl8xxxu: Use macid in rtl8xxxu_fill_txdesc_v1()
  wifi: rtl8xxxu: Make sure TX rate is reported in AP mode

 drivers/net/wireless/realtek/rtl8xxxu/core.c  | 28 ++++++++++++++++++-
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  1 +
 2 files changed, 28 insertions(+), 1 deletion(-)

--
2.43.0

[PATCH v1 1/3] wifi: rtl8xxxu: Use macid in rtl8xxxu_update_rate_mask()

[Ezra Buehler]

From: Ezra Buehler <ezra.buehler@husqvarnagroup.com>

This is needed in order for AP mode to work correctly on gen1 devices
like the RTL8192CU.

For more information, see the corresponding change for gen2 devices,
commit 769f326322e7 ("wifi: rtl8xxxu: Add parameter macid to
update_rate_mask").

Signed-off-by: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index 569856ca677f..cbd9efd22e3f 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -4606,7 +4606,7 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
 	h2c.ramask.mask_lo = cpu_to_le16(ramask & 0xffff);
 	h2c.ramask.mask_hi = cpu_to_le16(ramask >> 16);
 
-	h2c.ramask.arg = 0x80;
+	h2c.ramask.arg = macid | 0x80;
 	if (sgi)
 		h2c.ramask.arg |= 0x20;
 
-- 
2.43.0

RE: [PATCH v1 1/3] wifi: rtl8xxxu: Use macid in rtl8xxxu_update_rate_mask()

[Ping-Ke Shih]

Ezra Buehler <ezra@easyb.ch> wrote:
> From: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
> 
> This is needed in order for AP mode to work correctly on gen1 devices
> like the RTL8192CU.
> 
> For more information, see the corresponding change for gen2 devices,
> commit 769f326322e7 ("wifi: rtl8xxxu: Add parameter macid to
> update_rate_mask").
> 
> Signed-off-by: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> index 569856ca677f..cbd9efd22e3f 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> @@ -4606,7 +4606,7 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
>         h2c.ramask.mask_lo = cpu_to_le16(ramask & 0xffff);
>         h2c.ramask.mask_hi = cpu_to_le16(ramask >> 16);
> 
> -       h2c.ramask.arg = 0x80;
> +       h2c.ramask.arg = macid | 0x80;

Vendor driver fills macid and sgi as below: 

		arg = mac_id&0x1f;//MACID
		
		arg |= BIT(7);
		
		if (shortGIrate==_TRUE)
			arg |= BIT(5);

So, I suggest to define a mask and use u8_encode_bit() for macid.

[PATCH v1 2/3] wifi: rtl8xxxu: Use macid in rtl8xxxu_fill_txdesc_v1()

[Ezra Buehler]

From: Ezra Buehler <ezra.buehler@husqvarnagroup.com>

This is needed in order for AP mode to work correctly on gen1 devices
like the RTL8192CU.

For more information, see the corresponding change for gen2 devices,
commit 9aa45598d054 ("wifi: rtl8xxxu: Put the macid in txdesc").

Signed-off-by: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/core.c     | 2 ++
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index cbd9efd22e3f..2fa22d3145a4 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -5302,6 +5302,8 @@ rtl8xxxu_fill_txdesc_v1(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
 		dev_info(dev, "%s: TX rate: %d, pkt size %u\n",
 			 __func__, rate, le16_to_cpu(tx_desc->pkt_size));
 
+	tx_desc->txdw1 |= cpu_to_le32(macid & TXDESC32_MACID_MASK);
+
 	seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
 
 	tx_desc->txdw5 = cpu_to_le32(rate);
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index f42463e595cc..dde578f65d69 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -489,6 +489,7 @@ struct rtl8xxxu_txdesc40 {
 #define TXDESC_PKT_OFFSET_SZ		0
 #define TXDESC32_AGG_ENABLE		BIT(5)
 #define TXDESC32_AGG_BREAK		BIT(6)
+#define TXDESC32_MACID_MASK		GENMASK(4, 0)
 #define TXDESC40_MACID_SHIFT		0
 #define TXDESC40_MACID_MASK		0x00f0
 #define TXDESC_QUEUE_SHIFT		8
-- 
2.43.0

RE: [PATCH v1 2/3] wifi: rtl8xxxu: Use macid in rtl8xxxu_fill_txdesc_v1()

[Ping-Ke Shih]

Ezra Buehler <ezra@easyb.ch> wrote:
> From: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
> 
> This is needed in order for AP mode to work correctly on gen1 devices
> like the RTL8192CU.
> 
> For more information, see the corresponding change for gen2 devices,
> commit 9aa45598d054 ("wifi: rtl8xxxu: Put the macid in txdesc").
> 
> Signed-off-by: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/core.c     | 2 ++
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> index cbd9efd22e3f..2fa22d3145a4 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> @@ -5302,6 +5302,8 @@ rtl8xxxu_fill_txdesc_v1(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
>                 dev_info(dev, "%s: TX rate: %d, pkt size %u\n",
>                          __func__, rate, le16_to_cpu(tx_desc->pkt_size));
> 
> +       tx_desc->txdw1 |= cpu_to_le32(macid & TXDESC32_MACID_MASK);
> +

tx_desc->txdw1 |= le32_encode_bits(macid, TXDESC32_MACID_MASK);

[PATCH v1 3/3] wifi: rtl8xxxu: Make sure TX rate is reported in AP mode

[Ezra Buehler]

From: Ezra Buehler <ezra.buehler@husqvarnagroup.com>

Without this change, e.g., "iw station dump" will show "tx bitrate:
(unknown)" when in AP mode.

Signed-off-by: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
---
Note: The code was simply copied over from rtl8xxxu_bss_info_changed()
---
 drivers/net/wireless/realtek/rtl8xxxu/core.c | 24 ++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index 2fa22d3145a4..eb497301b625 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -7576,6 +7576,12 @@ static int rtl8xxxu_sta_add(struct ieee80211_hw *hw,
 	mutex_lock(&priv->sta_mutex);
 	ewma_rssi_init(&sta_info->avg_rssi);
 	if (vif->type == NL80211_IFTYPE_AP) {
+		struct rtl8xxxu_ra_report *rarpt = &priv->ra_report;
+		u32 ramask;
+		int sgi = 0;
+		u8 highest_rate;
+		u8 bw;
+
 		sta_info->rssi_level = RTL8XXXU_RATR_STA_INIT;
 		sta_info->macid = rtl8xxxu_acquire_macid(priv);
 		if (sta_info->macid >= RTL8XXXU_MAX_MAC_ID_NUM) {
@@ -7584,6 +7590,24 @@ static int rtl8xxxu_sta_add(struct ieee80211_hw *hw,
 		}

 		rtl8xxxu_refresh_rate_mask(priv, 0, sta, true);
+
+		/* TODO: Set bits 28-31 for rate adaptive id */
+		ramask = (sta->deflink.supp_rates[0] & 0xfff) |
+			sta->deflink.ht_cap.mcs.rx_mask[0] << 12 |
+			sta->deflink.ht_cap.mcs.rx_mask[1] << 20;
+		if (sta->deflink.ht_cap.cap &
+		    (IEEE80211_HT_CAP_SGI_40 | IEEE80211_HT_CAP_SGI_20))
+			sgi = 1;
+
+		highest_rate = fls(ramask) - 1;
+		if (rtl8xxxu_ht40_2g &&
+		    (sta->deflink.ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40))
+			bw = RATE_INFO_BW_40;
+		else
+			bw = RATE_INFO_BW_20;
+
+		rtl8xxxu_update_ra_report(rarpt, highest_rate, sgi, bw);
+
 		priv->fops->report_connect(priv, sta_info->macid, H2C_MACID_ROLE_STA, true);
 	} else {
 		switch (rtlvif->port_num) {
--
2.43.0

RE: [PATCH v1 3/3] wifi: rtl8xxxu: Make sure TX rate is reported in AP mode

[Ping-Ke Shih]

Ezra Buehler <ezra@easyb.ch> wrote:
> From: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
> 
> Without this change, e.g., "iw station dump" will show "tx bitrate:
> (unknown)" when in AP mode.
> 
> Signed-off-by: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
> ---
> Note: The code was simply copied over from rtl8xxxu_bss_info_changed()

The code seems to report the highest rate as initial (fake) TX rate.
Please add a patch ahead that moves the code into an individual function
but don't change existing logic. Then, this patch can use the function
to get your expectation. 

Re: [PATCH v1 0/3] wifi: rtl8xxxu: AP mode fixes (for gen1 devices)

[Bitterblue Smith]

On 19/02/2025 19:52, Ezra Buehler wrote:
> From: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
> 
> As mentioned in the previous patch [1], especially the transmit rate is
> rather low in AP mode with gen1 devices like the RTL8192CU. The
> following changes seem to fix the issue.
> 
> When looking at the patches introducing the initial AP support [2], I
> noticed that rtl8xxxu_gen2_report_connect() was altered too. So, now I
> am wondering if we need to use `macid` and `role` in
> rtl8xxxu_gen1_report_connect() as well...
> 

I don't think these chips have those fields in the "joinbss" command.

> [1]: https://lore.kernel.org/all/20250122071512.10165-1-ezra@easyb.ch/
> [2]: https://lore.kernel.org/all/20230428150833.218605-1-martin.kaistra@linutronix.de/
> 
> 
> Ezra Buehler (3):
>   wifi: rtl8xxxu: Use macid in rtl8xxxu_update_rate_mask()
>   wifi: rtl8xxxu: Use macid in rtl8xxxu_fill_txdesc_v1()
>   wifi: rtl8xxxu: Make sure TX rate is reported in AP mode
> 
>  drivers/net/wireless/realtek/rtl8xxxu/core.c  | 28 ++++++++++++++++++-
>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  1 +
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> --
> 2.43.0

Re: [PATCH v1 0/3] wifi: rtl8xxxu: AP mode fixes (for gen1 devices)

[Ezra Buehler]

> On 19 Feb 2025, at 18:52, Ezra Buehler <ezra@easyb.ch> wrote:
> 
> From: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
> 
> As mentioned in the previous patch [1], especially the transmit rate is
> rather low in AP mode with gen1 devices like the RTL8192CU. The
> following changes seem to fix the issue.
> 
> When looking at the patches introducing the initial AP support [2], I
> noticed that rtl8xxxu_gen2_report_connect() was altered too. So, now I
> am wondering if we need to use `macid` and `role` in
> rtl8xxxu_gen1_report_connect() as well...
> 
> [1]: https://lore.kernel.org/all/20250122071512.10165-1-ezra@easyb.ch/
> [2]: https://lore.kernel.org/all/20230428150833.218605-1-martin.kaistra@linutronix.de/
> 
> 
> Ezra Buehler (3):
>  wifi: rtl8xxxu: Use macid in rtl8xxxu_update_rate_mask()
>  wifi: rtl8xxxu: Use macid in rtl8xxxu_fill_txdesc_v1()
>  wifi: rtl8xxxu: Make sure TX rate is reported in AP mode
> 
> drivers/net/wireless/realtek/rtl8xxxu/core.c  | 28 ++++++++++++++++++-
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  1 +
> 2 files changed, 28 insertions(+), 1 deletion(-)
> 
> —
> 2.43.0

I apologize for the radio silence. It turned out that these changes do
not improve things. I must have made a mistake when measuring throughput
back then.

When testing the TX rate (with iperf3), I see the throughput constantely
dropping to 0 bits/sec. This makes AP mode unusable even for our
non-data-heavy use case.

Do you have any suggestions where to dig deeper to get to the bottom of
this?

Cheers,
Ezra.

RE: [PATCH v1 0/3] wifi: rtl8xxxu: AP mode fixes (for gen1 devices)

[Ping-Ke Shih]

Ezra Buehler <ezra@easyb.ch> wrote:
> > On 19 Feb 2025, at 18:52, Ezra Buehler <ezra@easyb.ch> wrote:
> >
> > From: Ezra Buehler <ezra.buehler@husqvarnagroup.com>
> >
> > As mentioned in the previous patch [1], especially the transmit rate is
> > rather low in AP mode with gen1 devices like the RTL8192CU. The
> > following changes seem to fix the issue.
> >
> > When looking at the patches introducing the initial AP support [2], I
> > noticed that rtl8xxxu_gen2_report_connect() was altered too. So, now I
> > am wondering if we need to use `macid` and `role` in
> > rtl8xxxu_gen1_report_connect() as well...
> >
> > [1]: https://lore.kernel.org/all/20250122071512.10165-1-ezra@easyb.ch/
> > [2]: https://lore.kernel.org/all/20230428150833.218605-1-martin.kaistra@linutronix.de/
> >
> >
> > Ezra Buehler (3):
> >  wifi: rtl8xxxu: Use macid in rtl8xxxu_update_rate_mask()
> >  wifi: rtl8xxxu: Use macid in rtl8xxxu_fill_txdesc_v1()
> >  wifi: rtl8xxxu: Make sure TX rate is reported in AP mode
> >
> > drivers/net/wireless/realtek/rtl8xxxu/core.c  | 28 ++++++++++++++++++-
> > .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  1 +
> > 2 files changed, 28 insertions(+), 1 deletion(-)
> >
> > —
> > 2.43.0
> 
> I apologize for the radio silence. It turned out that these changes do
> not improve things. I must have made a mistake when measuring throughput
> back then.
> 
> When testing the TX rate (with iperf3), I see the throughput constantely
> dropping to 0 bits/sec. This makes AP mode unusable even for our
> non-data-heavy use case.
> 
> Do you have any suggestions where to dig deeper to get to the bottom of
> this?

Testing with single one station connected would be easier to address problem,
and ensure mac_id is always 1 for AP mode transmitting packets to the
station. 

Also, you need a sniffer to check PHY rate if it can keep. As well as the
rate when throughput dropped. If rate is good, check any misbehavior of air
packets. 

Before all above, please ensure the PHY rate is good when it plays as a
station mode.