This series intends to bring AP mode support to the rtl8xxxu driver,
more specifically for the 8188f, because this is the HW I have.
The work is based on the vendor driver as I do not have access to
datasheets.
This is an RFC, so that there can be a discussion first before
potentially implementing support for the other chips in this driver, if
required.
Also while doing some measurements with iperf3 to compare with the
vendor driver, I saw, that TCP traffic from AP to STA is slower than in
the vendor driver. For UDP it looks fine. I hope I can get some help to
fix this.
* vendor driver:
without 802.11n:
UDP (AP -> STA): 27 Mbits/sec
UDP (STA -> AP): 33 Mbits/sec
TCP (AP -> STA): 24 Mbits/sec
TCP (STA -> AP): 26 Mbits/sec
with 802.11n:
UDP (AP -> STA): 51 Mbits/sec
UDP (STA -> AP): 35 Mbits/sec
TCP (AP -> STA): 40 Mbits/sec
TCP (STA -> AP): 36 Mbits/sec
* rtl8xxxu:
without 802.11n:
UDP (AP -> STA): 25 Mbits/sec
UDP (STA -> AP): 31 Mbits/sec
TCP (AP -> STA): 3 Mbits/sec !
TCP (STA -> AP): 25 Mbits/sec
with 802.11n:
UDP (AP -> STA): 41 Mbits/sec
UDP (STA -> AP): 36 Mbits/sec
TCP (AP -> STA): 3 Mbits/sec !
TCP (STA -> AP): 32 Mbits/sec
Thanks,
Martin
Martin Kaistra (14):
wifi: rtl8xxxu: Add start_ap() callback
wifi: rtl8xxxu: Select correct queue for beacon frames
wifi: rtl8xxxu: Add beacon functions
wifi: rtl8xxxu: Add set_tim() callback
wifi: rtl8xxxu: Allow setting rts threshold to -1
wifi: rtl8xxxu: Allow creating interface in AP mode
wifi: rtl8xxxu: Add parameter macid to update_rate_mask
wifi: rtl8xxxu: Actually use macid in rtl8xxxu_gen2_report_connect
wifi: rtl8xxxu: Add parameter role to report_connect
wifi: rtl8xxxu: Add sta_add() callback
wifi: rtl8xxxu: Put the macid in txdesc
wifi: rtl8xxxu: Enable hw seq for all non-qos frames
wifi: rtl8xxxu: Clean up filter configuration
wifi: rtl8xxxu: Declare AP mode support for 8188f
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 28 ++-
.../realtek/rtl8xxxu/rtl8xxxu_8188e.c | 3 +-
.../realtek/rtl8xxxu/rtl8xxxu_8188f.c | 1 +
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 226 +++++++++++++++---
.../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h | 2 +
5 files changed, 222 insertions(+), 38 deletions(-)
--
2.30.2
The report_connect function has had a macid parameter from the
beginning, but it has not been used, because in STA mode, the value was
always zero.
As it can now have different values in AP mode, actually wire it up to
the H2C command.
Signed-off-by: Martin Kaistra <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index b5cb15e472f1c..4209880d724be 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4557,6 +4557,8 @@ void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
else
h2c.media_status_rpt.parm &= ~BIT(0);
+ h2c.media_status_rpt.macid = macid;
+
rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
}
--
2.30.2
The HW maintains a rate_mask for each connection, referenced by the
macid. Add a parameter to update_rate_mask and add the macid to the
h2c call in the gen2 implementation.
Also extend refresh_rate_mask to generate the macid in AP mode from
sta->aid.
Signed-off-by: Martin Kaistra <[email protected]>
---
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 7 ++++---
.../wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c | 3 ++-
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 17 +++++++++++++----
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index cac985271628c..c06ad33645974 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1905,7 +1905,8 @@ struct rtl8xxxu_fileops {
void (*set_tx_power) (struct rtl8xxxu_priv *priv, int channel,
bool ht40);
void (*update_rate_mask) (struct rtl8xxxu_priv *priv,
- u32 ramask, u8 rateid, int sgi, int txbw_40mhz);
+ u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
+ u8 macid);
void (*report_connect) (struct rtl8xxxu_priv *priv,
u8 macid, bool connect);
void (*report_rssi) (struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
@@ -2007,9 +2008,9 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw);
void rtl8xxxu_gen1_usb_quirks(struct rtl8xxxu_priv *priv);
void rtl8xxxu_gen2_usb_quirks(struct rtl8xxxu_priv *priv);
void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
- u32 ramask, u8 rateid, int sgi, int txbw_40mhz);
+ u32 ramask, u8 rateid, int sgi, int txbw_40mhz, u8 macid);
void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
- u32 ramask, u8 rateid, int sgi, int txbw_40mhz);
+ u32 ramask, u8 rateid, int sgi, int txbw_40mhz, u8 macid);
void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
u8 macid, bool connect);
void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
index 6a82ec47568ee..c3dc5130c9f37 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
@@ -1798,7 +1798,8 @@ static void rtl8188e_arfb_refresh(struct rtl8xxxu_ra_info *ra)
static void
rtl8188e_update_rate_mask(struct rtl8xxxu_priv *priv,
- u32 ramask, u8 rateid, int sgi, int txbw_40mhz)
+ u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
+ u8 macid)
{
struct rtl8xxxu_ra_info *ra = &priv->ra_info;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index b20ff8bc40870..b5cb15e472f1c 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4471,7 +4471,8 @@ static void rtl8xxxu_sw_scan_complete(struct ieee80211_hw *hw,
}
void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
- u32 ramask, u8 rateid, int sgi, int txbw_40mhz)
+ u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
+ u8 macid)
{
struct h2c_cmd h2c;
@@ -4491,7 +4492,8 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
}
void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
- u32 ramask, u8 rateid, int sgi, int txbw_40mhz)
+ u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
+ u8 macid)
{
struct h2c_cmd h2c;
u8 bw;
@@ -4508,6 +4510,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
h2c.b_macid_cfg.ramask1 = (ramask >> 8) & 0xff;
h2c.b_macid_cfg.ramask2 = (ramask >> 16) & 0xff;
h2c.b_macid_cfg.ramask3 = (ramask >> 24) & 0xff;
+ h2c.b_macid_cfg.macid = macid;
h2c.b_macid_cfg.data1 = rateid;
if (sgi)
@@ -4870,7 +4873,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
priv->vif = vif;
priv->rssi_level = RTL8XXXU_RATR_STA_INIT;
- priv->fops->update_rate_mask(priv, ramask, 0, sgi, bw == RATE_INFO_BW_40);
+ priv->fops->update_rate_mask(priv, ramask, 0, sgi,
+ bw == RATE_INFO_BW_40, 0);
rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
@@ -6772,6 +6776,7 @@ static void rtl8xxxu_refresh_rate_mask(struct rtl8xxxu_priv *priv,
u8 txbw_40mhz;
u8 snr, snr_thresh_high, snr_thresh_low;
u8 go_up_gap = 5;
+ u8 macid = 0;
rssi_level = priv->rssi_level;
snr = rtl8xxxu_signal_to_snr(signal);
@@ -6891,7 +6896,11 @@ static void rtl8xxxu_refresh_rate_mask(struct rtl8xxxu_priv *priv,
}
priv->rssi_level = rssi_level;
- priv->fops->update_rate_mask(priv, rate_bitmap, ratr_idx, sgi, txbw_40mhz);
+
+ if (priv->vif->type == NL80211_IFTYPE_AP)
+ macid = sta->aid + 1;
+
+ priv->fops->update_rate_mask(priv, rate_bitmap, ratr_idx, sgi, txbw_40mhz, macid);
}
}
--
2.30.2
Use the sequence from the vendor driver for setting up the beacon
related registers.
Also set the MAC address register.
Signed-off-by: Martin Kaistra <[email protected]>
---
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 31 ++++++++++++++++---
.../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h | 2 ++
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index b233c66a7a5a8..b20ff8bc40870 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6408,18 +6408,39 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
int ret;
u8 val8;
+ if (!priv->vif)
+ priv->vif = vif;
+ else
+ return -EOPNOTSUPP;
+
switch (vif->type) {
case NL80211_IFTYPE_STATION:
- if (!priv->vif)
- priv->vif = vif;
- else
- return -EOPNOTSUPP;
rtl8xxxu_stop_tx_beacon(priv);
val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
val8 |= BEACON_ATIM | BEACON_FUNCTION_ENABLE |
BEACON_DISABLE_TSF_UPDATE;
rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
+ ret = 0;
+ break;
+ case NL80211_IFTYPE_AP:
+ rtl8xxxu_write8(priv, REG_BEACON_CTRL,
+ BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
+ rtl8xxxu_write8(priv, REG_ATIMWND, 0x0c); /* 12ms */
+ rtl8xxxu_write16(priv, REG_TSFTR_SYN_OFFSET, 0x7fff); /* ~32ms */
+ rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, DUAL_TSF_RESET_TSF0);
+
+ /* enable BCN0 function */
+ rtl8xxxu_write8(priv, REG_BEACON_CTRL,
+ BEACON_DISABLE_TSF_UPDATE |
+ BEACON_FUNCTION_ENABLE | BEACON_CTRL_MBSSID |
+ BEACON_CTRL_TX_BEACON_RPT);
+
+ /* select BCN on port 0 */
+ val8 = rtl8xxxu_read8(priv, REG_CCK_CHECK);
+ val8 &= ~BIT_BCN_PORT_SEL;
+ rtl8xxxu_write8(priv, REG_CCK_CHECK, val8);
+
ret = 0;
break;
default:
@@ -6427,6 +6448,8 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
}
rtl8xxxu_set_linktype(priv, vif->type);
+ ether_addr_copy(priv->mac_addr, vif->addr);
+ rtl8xxxu_set_mac(priv);
return ret;
}
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
index 4dffbab494c3b..83e7f8fd82c0a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
@@ -572,6 +572,8 @@
#define REG_ARFR1 0x0448
#define REG_ARFR2 0x044c
#define REG_ARFR3 0x0450
+#define REG_CCK_CHECK 0x0454
+#define BIT_BCN_PORT_SEL BIT(5)
#define REG_AMPDU_MAX_TIME_8723B 0x0456
#define REG_AGGLEN_LMT 0x0458
#define REG_AMPDU_MIN_SPACE 0x045c
--
2.30.2
This function gets called in AP mode, when a new STA gets associated to
us. Call rtl8xxxu_refresh_rate_mask() to set a rate mask for the newly
connected STA (referenced by the macid) and then send a media connnect
report.
Signed-off-by: Martin Kaistra <[email protected]>
---
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 5e36fddbbb488..d74a3c6452507 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -7159,6 +7159,19 @@ static void rtl8xxxu_stop(struct ieee80211_hw *hw)
rtl8xxxu_free_tx_resources(priv);
}
+static int rtl8xxxu_sta_add(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_sta *sta)
+{
+ struct rtl8xxxu_priv *priv = hw->priv;
+
+ if (sta) {
+ rtl8xxxu_refresh_rate_mask(priv, 0, sta);
+ priv->fops->report_connect(priv, sta->aid + 1, H2C_ROLE_STA, true);
+ }
+ return 0;
+}
+
static const struct ieee80211_ops rtl8xxxu_ops = {
.tx = rtl8xxxu_tx,
.wake_tx_queue = ieee80211_handle_wake_tx_queue,
@@ -7179,6 +7192,7 @@ static const struct ieee80211_ops rtl8xxxu_ops = {
.sta_statistics = rtl8xxxu_sta_statistics,
.get_antenna = rtl8xxxu_get_antenna,
.set_tim = rtl8xxxu_set_tim,
+ .sta_add = rtl8xxxu_sta_add,
};
static int rtl8xxxu_parse_usb(struct rtl8xxxu_priv *priv,
--
2.30.2
This allows to tell the HW if a connection is made to a STA or an AP.
Add the implementation for the gen2 version.
Signed-off-by: Martin Kaistra <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 9 ++++++---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 11 ++++++-----
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index c06ad33645974..e78e0bbd23354 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1280,6 +1280,9 @@ struct rtl8xxxu_rfregs {
#define H2C_JOIN_BSS_DISCONNECT 0
#define H2C_JOIN_BSS_CONNECT 1
+#define H2C_ROLE_STA 1
+#define H2C_ROLE_AP 2
+
/*
* H2C (firmware) commands differ between the older generation chips
* 8188[cr]u, 819[12]cu, and 8723au, and the more recent chips 8723bu,
@@ -1908,7 +1911,7 @@ struct rtl8xxxu_fileops {
u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
u8 macid);
void (*report_connect) (struct rtl8xxxu_priv *priv,
- u8 macid, bool connect);
+ u8 macid, u8 role, bool connect);
void (*report_rssi) (struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
struct ieee80211_tx_info *tx_info,
@@ -2012,9 +2015,9 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
u32 ramask, u8 rateid, int sgi, int txbw_40mhz, u8 macid);
void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
- u8 macid, bool connect);
+ u8 macid, u8 role, bool connect);
void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
- u8 macid, bool connect);
+ u8 macid, u8 role, bool connect);
void rtl8xxxu_gen1_report_rssi(struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
void rtl8xxxu_gen2_report_rssi(struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv);
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 4209880d724be..5e36fddbbb488 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4524,7 +4524,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
}
void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
- u8 macid, bool connect)
+ u8 macid, u8 role, bool connect)
{
struct h2c_cmd h2c;
@@ -4541,7 +4541,7 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
}
void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
- u8 macid, bool connect)
+ u8 macid, u8 role, bool connect)
{
/*
* The firmware turns on the rate control when it knows it's
@@ -4557,6 +4557,7 @@ void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
else
h2c.media_status_rpt.parm &= ~BIT(0);
+ h2c.media_status_rpt.parm |= ((role << 4) & 0xf0);
h2c.media_status_rpt.macid = macid;
rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
@@ -4886,13 +4887,13 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
rtl8xxxu_write16(priv, REG_BCN_PSR_RPT,
0xc000 | vif->cfg.aid);
- priv->fops->report_connect(priv, 0, true);
+ priv->fops->report_connect(priv, 0, H2C_ROLE_AP, true);
} else {
val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
val8 |= BEACON_DISABLE_TSF_UPDATE;
rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
- priv->fops->report_connect(priv, 0, false);
+ priv->fops->report_connect(priv, 0, H2C_ROLE_AP, false);
}
}
@@ -4953,7 +4954,7 @@ static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
dev_dbg(dev, "Start AP mode\n");
rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
- priv->fops->report_connect(priv, 0, true);
+ priv->fops->report_connect(priv, 0, 0, true);
return 0;
}
--
2.30.2
Beacon frames are generated by the HW and therefore contain a HW
generated seq number. Enable HW sequence number for other frames to
match that.
Signed-off-by: Martin Kaistra <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c232de1d47173..82fbe778fc5ec 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5275,6 +5275,9 @@ rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
tx_desc40->txdw4 |= cpu_to_le32(TXDESC40_RETRY_LIMIT_ENABLE);
}
+ if (!ieee80211_is_data_qos(hdr->frame_control))
+ tx_desc40->txdw8 |= cpu_to_le32(TXDESC40_HW_SEQ_ENABLE);
+
if (short_preamble)
tx_desc40->txdw5 |= cpu_to_le32(TXDESC40_SHORT_PREAMBLE);
--
2.30.2
Add a parameter macid to fill_txdesc(), implement setting it for the
gen2 version.
This is used to tell the HW who the recipient of the packet is, so that
the appropriate data rate can be selected.
Signed-off-by: Martin Kaistra <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 8 ++++----
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 16 ++++++++++++----
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index e78e0bbd23354..20304b0bd68a3 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1917,7 +1917,7 @@ struct rtl8xxxu_fileops {
struct ieee80211_tx_info *tx_info,
struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
bool short_preamble, bool ampdu_enable,
- u32 rts_rate);
+ u32 rts_rate, u8 macid);
void (*set_crystal_cap) (struct rtl8xxxu_priv *priv, u8 crystal_cap);
s8 (*cck_rssi) (struct rtl8xxxu_priv *priv, struct rtl8723au_phy_stats *phy_stats);
int (*led_classdev_brightness_set) (struct led_classdev *led_cdev,
@@ -2046,17 +2046,17 @@ void rtl8xxxu_fill_txdesc_v1(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
struct ieee80211_tx_info *tx_info,
struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
bool short_preamble, bool ampdu_enable,
- u32 rts_rate);
+ u32 rts_rate, u8 macid);
void rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
struct ieee80211_tx_info *tx_info,
struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
bool short_preamble, bool ampdu_enable,
- u32 rts_rate);
+ u32 rts_rate, u8 macid);
void rtl8xxxu_fill_txdesc_v3(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
struct ieee80211_tx_info *tx_info,
struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
bool short_preamble, bool ampdu_enable,
- u32 rts_rate);
+ u32 rts_rate, u8 macid);
void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
u8 arg1, u8 arg2, u8 arg3, u8 arg4, u8 arg5);
void rtl8723bu_phy_init_antenna_selection(struct rtl8xxxu_priv *priv);
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index d74a3c6452507..c232de1d47173 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5152,7 +5152,8 @@ void
rtl8xxxu_fill_txdesc_v1(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
struct ieee80211_tx_info *tx_info,
struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
- bool short_preamble, bool ampdu_enable, u32 rts_rate)
+ bool short_preamble, bool ampdu_enable, u32 rts_rate,
+ u8 macid)
{
struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
struct rtl8xxxu_priv *priv = hw->priv;
@@ -5224,7 +5225,8 @@ void
rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
struct ieee80211_tx_info *tx_info,
struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
- bool short_preamble, bool ampdu_enable, u32 rts_rate)
+ bool short_preamble, bool ampdu_enable, u32 rts_rate,
+ u8 macid)
{
struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
struct rtl8xxxu_priv *priv = hw->priv;
@@ -5248,6 +5250,8 @@ rtl8xxxu_fill_txdesc_v2(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_desc40->pkt_size));
+ tx_desc40->txdw1 |= cpu_to_le32(macid << TXDESC40_MACID_SHIFT);
+
seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
tx_desc40->txdw4 = cpu_to_le32(rate);
@@ -5299,7 +5303,8 @@ void
rtl8xxxu_fill_txdesc_v3(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
struct ieee80211_tx_info *tx_info,
struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
- bool short_preamble, bool ampdu_enable, u32 rts_rate)
+ bool short_preamble, bool ampdu_enable, u32 rts_rate,
+ u8 macid)
{
struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
struct rtl8xxxu_priv *priv = hw->priv;
@@ -5398,6 +5403,7 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
u16 pktlen = skb->len;
u16 rate_flag = tx_info->control.rates[0].flags;
int tx_desc_size = priv->fops->tx_desc_size;
+ u8 macid = 0;
int ret;
bool ampdu_enable, sgi = false, short_preamble = false;
@@ -5497,9 +5503,11 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
else
rts_rate = 0;
+ if (vif->type == NL80211_IFTYPE_AP && sta)
+ macid = sta->aid + 1;
priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
- ampdu_enable, rts_rate);
+ ampdu_enable, rts_rate, macid);
rtl8xxxu_calc_tx_desc_csum(tx_desc);
--
2.30.2
Everything is in place now for AP mode, we can tell the system that we
support it. Put the feature behind a flag in priv->fops, because it is
not (yet) implemented for all chips.
Signed-off-by: Martin Kaistra <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 1 +
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c | 1 +
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 ++
3 files changed, 4 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 20304b0bd68a3..31f9cf9e558d7 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1930,6 +1930,7 @@ struct rtl8xxxu_fileops {
u8 has_tx_report:1;
u8 gen2_thermal_meter:1;
u8 needs_full_init:1;
+ u8 supports_ap:1;
u32 adda_1t_init;
u32 adda_1t_path_on;
u32 adda_2t_path_on_a;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
index 82dee1fed4779..c4c1f015a7fd9 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
@@ -1746,6 +1746,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
.has_tx_report = 1,
.gen2_thermal_meter = 1,
.needs_full_init = 1,
+ .supports_ap = 1,
.adda_1t_init = 0x03c00014,
.adda_1t_path_on = 0x03c00014,
.trxff_boundary = 0x3f7f,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index b6f811ad01333..31bd1f2711aed 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -7449,6 +7449,8 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
hw->wiphy->max_scan_ssids = 1;
hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
+ if (priv->fops->supports_ap)
+ hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP);
hw->queues = 4;
sband = &rtl8xxxu_supported_band;
--
2.30.2
In AP mode, RCR_CHECK_BSSID_MATCH should not be set. Rearrange RCR bits
to filter flags to match other realtek drivers and don't set
RCR_CHECK_BSSID_BEACON and RCR_CHECK_BSSID_MATCH in AP mode.
Signed-off-by: Martin Kaistra <[email protected]>
---
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 82fbe778fc5ec..b6f811ad01333 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6597,23 +6597,24 @@ static void rtl8xxxu_configure_filter(struct ieee80211_hw *hw,
* FIF_PLCPFAIL not supported?
*/
- if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
- rcr &= ~RCR_CHECK_BSSID_BEACON;
- else
- rcr |= RCR_CHECK_BSSID_BEACON;
+ if (priv->vif->type != NL80211_IFTYPE_AP) {
+ if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
+ rcr &= ~(RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH);
+ else
+ rcr |= RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH;
+ } else {
+ rcr &= ~RCR_CHECK_BSSID_MATCH;
+ }
if (*total_flags & FIF_CONTROL)
rcr |= RCR_ACCEPT_CTRL_FRAME;
else
rcr &= ~RCR_ACCEPT_CTRL_FRAME;
- if (*total_flags & FIF_OTHER_BSS) {
+ if (*total_flags & FIF_OTHER_BSS)
rcr |= RCR_ACCEPT_AP;
- rcr &= ~RCR_CHECK_BSSID_MATCH;
- } else {
+ else
rcr &= ~RCR_ACCEPT_AP;
- rcr |= RCR_CHECK_BSSID_MATCH;
- }
if (*total_flags & FIF_PSPOLL)
rcr |= RCR_ACCEPT_PM;
--
2.30.2
This gets called at the start of AP mode operation. Set bssid, beacon
interval and send a connect report to the HW.
Signed-off-by: Martin Kaistra <[email protected]>
---
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c152b228606f1..90b98b9dcbd9d 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4899,6 +4899,20 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
return;
}
+static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+ struct ieee80211_bss_conf *link_conf)
+{
+ struct rtl8xxxu_priv *priv = hw->priv;
+ struct device *dev = &priv->udev->dev;
+
+ dev_dbg(dev, "Start AP mode\n");
+ rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
+ rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
+ priv->fops->report_connect(priv, 0, true);
+
+ return 0;
+}
+
static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
{
u32 rtlqueue;
@@ -7026,6 +7040,7 @@ static const struct ieee80211_ops rtl8xxxu_ops = {
.config = rtl8xxxu_config,
.conf_tx = rtl8xxxu_conf_tx,
.bss_info_changed = rtl8xxxu_bss_info_changed,
+ .start_ap = rtl8xxxu_start_ap,
.configure_filter = rtl8xxxu_configure_filter,
.set_rts_threshold = rtl8xxxu_set_rts_threshold,
.start = rtl8xxxu_start,
--
2.30.2
Add a workqueue to update the beacon contents asynchronously and
implement downloading the beacon to the HW and starting beacon tx like
the vendor driver.
Signed-off-by: Martin Kaistra <[email protected]>
---
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 3 +
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 81 +++++++++++++++++++
2 files changed, 84 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 9d48c69ffece1..cac985271628c 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1744,6 +1744,8 @@ struct rtl8xxxu_priv {
bool shutdown;
struct work_struct rx_urb_wq;
+ bool beacon_enabled;
+
u8 mac_addr[ETH_ALEN];
char chip_name[8];
char chip_vendor[8];
@@ -1850,6 +1852,7 @@ struct rtl8xxxu_priv {
struct delayed_work ra_watchdog;
struct work_struct c2hcmd_work;
struct sk_buff_head c2hcmd_queue;
+ struct work_struct update_beacon_work;
struct rtl8xxxu_btcoex bt_coex;
struct rtl8xxxu_ra_report ra_report;
struct rtl8xxxu_cfo_tracking cfo_tracking;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index daeaa7d6864f9..404fa6e322f58 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1104,6 +1104,24 @@ static void rtl8xxxu_stop_tx_beacon(struct rtl8xxxu_priv *priv)
val8 = rtl8xxxu_read8(priv, REG_TBTT_PROHIBIT + 2);
val8 &= ~BIT(0);
rtl8xxxu_write8(priv, REG_TBTT_PROHIBIT + 2, val8);
+
+ priv->beacon_enabled = false;
+}
+
+static void rtl8xxxu_start_tx_beacon(struct rtl8xxxu_priv *priv)
+{
+ u8 val8;
+
+ val8 = rtl8xxxu_read8(priv, REG_FWHW_TXQ_CTRL + 2);
+ val8 |= BIT(6);
+ rtl8xxxu_write8(priv, REG_FWHW_TXQ_CTRL + 2, val8);
+
+ rtl8xxxu_write8(priv, REG_TBTT_PROHIBIT + 1, 0x80);
+ val8 = rtl8xxxu_read8(priv, REG_TBTT_PROHIBIT + 2);
+ val8 &= 0xF0;
+ rtl8xxxu_write8(priv, REG_TBTT_PROHIBIT + 2, val8);
+
+ priv->beacon_enabled = true;
}
@@ -4895,6 +4913,17 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
dev_dbg(dev, "Changed BASIC_RATES!\n");
rtl8xxxu_set_basic_rates(priv, bss_conf->basic_rates);
}
+
+ if (changed & BSS_CHANGED_BEACON ||
+ (changed & BSS_CHANGED_BEACON_ENABLED &&
+ bss_conf->enable_beacon)) {
+ if (!priv->beacon_enabled) {
+ dev_dbg(dev, "BSS_CHANGED_BEACON_ENABLED\n");
+ rtl8xxxu_start_tx_beacon(priv);
+ schedule_work(&priv->update_beacon_work);
+ }
+ }
+
error:
return;
}
@@ -5476,6 +5505,57 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
dev_kfree_skb(skb);
}
+static void rtl8xxxu_send_beacon_frame(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif)
+{
+ struct rtl8xxxu_priv *priv = hw->priv;
+ struct sk_buff *skb = ieee80211_beacon_get(hw, vif, 0);
+ struct device *dev = &priv->udev->dev;
+ int retry;
+ u8 val8;
+
+ /* BCN_VALID, BIT16 of REG_TDECTRL = BIT0 of REG_TDECTRL+2,
+ * write 1 to clear, cleared by SW.
+ */
+ val8 = rtl8xxxu_read8(priv, REG_TDECTRL + 2);
+ val8 |= BIT(0);
+ rtl8xxxu_write8(priv, REG_TDECTRL + 2, val8);
+
+ /* SW_BCN_SEL - Port0 */
+ val8 = rtl8xxxu_read8(priv, REG_DWBCN1_CTRL_8723B + 2);
+ val8 &= ~BIT(4);
+ rtl8xxxu_write8(priv, REG_DWBCN1_CTRL_8723B + 2, val8);
+
+ if (skb)
+ rtl8xxxu_tx(hw, NULL, skb);
+
+ retry = 100;
+ do {
+ val8 = rtl8xxxu_read8(priv, REG_TDECTRL + 2);
+ if (val8 & BIT(0))
+ break;
+ usleep_range(10, 20);
+ } while (retry--);
+
+ if (!retry)
+ dev_err(dev, "%s: Failed to read beacon valid bit\n", __func__);
+}
+
+static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work)
+{
+ struct rtl8xxxu_priv *priv =
+ container_of(work, struct rtl8xxxu_priv, update_beacon_work);
+ struct ieee80211_hw *hw = priv->hw;
+ struct ieee80211_vif *vif = priv->vif;
+
+ if (!vif) {
+ WARN_ONCE(true, "no vif to update beacon\n");
+ return;
+ }
+
+ rtl8xxxu_send_beacon_frame(hw, vif);
+}
+
void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
struct ieee80211_rx_status *rx_status,
struct rtl8723au_phy_stats *phy_stats,
@@ -7244,6 +7324,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
spin_lock_init(&priv->rx_urb_lock);
INIT_WORK(&priv->rx_urb_wq, rtl8xxxu_rx_urb_work);
INIT_DELAYED_WORK(&priv->ra_watchdog, rtl8xxxu_watchdog_callback);
+ INIT_WORK(&priv->update_beacon_work, rtl8xxxu_update_beacon_work_callback);
skb_queue_head_init(&priv->c2hcmd_queue);
usb_set_intfdata(interface, hw);
--
2.30.2
Use the special beacon queue for beacon frames instead of the management
frame queue, so that the HW sends them periodically instead of only
once.
Signed-off-by: Martin Kaistra <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 90b98b9dcbd9d..daeaa7d6864f9 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4941,7 +4941,9 @@ static u32 rtl8xxxu_queue_select(struct ieee80211_hdr *hdr, struct sk_buff *skb)
{
u32 queue;
- if (ieee80211_is_mgmt(hdr->frame_control))
+ if (unlikely(ieee80211_is_beacon(hdr->frame_control)))
+ queue = TXDESC_QUEUE_BEACON;
+ else if (ieee80211_is_mgmt(hdr->frame_control))
queue = TXDESC_QUEUE_MGNT;
else
queue = rtl8xxxu_80211_to_rtl_queue(skb_get_queue_mapping(skb));
--
2.30.2
The default setting in hostapd.conf for rts threshold is -1, which means
disabled. Allow to set it.
Signed-off-by: Martin Kaistra <[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 f8380a2d51ae2..b233c66a7a5a8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6592,7 +6592,7 @@ static void rtl8xxxu_configure_filter(struct ieee80211_hw *hw,
static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
{
- if (rts > 2347)
+ if (rts > 2347 && rts != (u32)-1)
return -EINVAL;
return 0;
--
2.30.2
On 22/03/2023 19:18, Martin Kaistra wrote:
> This series intends to bring AP mode support to the rtl8xxxu driver,
> more specifically for the 8188f, because this is the HW I have.
> The work is based on the vendor driver as I do not have access to
> datasheets.
>
> This is an RFC, so that there can be a discussion first before
> potentially implementing support for the other chips in this driver, if
> required.
>
Hi!
I ran into some problems while testing this.
First, a null pointer dereference in rtl8xxxu_config_filter() when
turning on the AP. priv->vif was NULL:
if (priv->vif->type != NL80211_IFTYPE_AP) {
I changed it like this:
if (priv->vif && priv->vif->type != NL80211_IFTYPE_AP) {
Then I was able to turn on the AP and connect my phone to it. However,
the system froze after a few seconds. I had `journalctl --follow`
running. The last thing printed before the system froze was the DHCP
stuff (discover, offer, request, ack). The phone said it was connected,
but speedtest.net didn't have time to load before the freeze.
My system is a laptop with RTL8822CE internal wifi card connected to my
ISP's router. The connections are managed by NetworkManager 1.42.4-1,
which uses wpa_supplicant 2:2.10-8 and dnsmasq 2.89-1. The operating
system is Arch Linux running kernel 6.2.5-arch1-1.
I used Plasma's NetworkManager applet to create a new "Wi-Fi (shared)"
connection with mode "Access Point", band 2.4 GHz, channel 1, no
encryption, and "IPv4 is required for this connection".
Unrelated to anything, just out of curiosity, what brand/model is your
RTL8188FU?
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 01/14] wifi: rtl8xxxu: Add start_ap() callback
>
> This gets called at the start of AP mode operation. Set bssid, beacon
> interval and send a connect report to the HW.
>
> Signed-off-by: Martin Kaistra <[email protected]>
Reviewed-by: Ping-Ke Shih <[email protected]>
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index c152b228606f1..90b98b9dcbd9d 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4899,6 +4899,20 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> return;
> }
>
> +static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> + struct ieee80211_bss_conf *link_conf)
> +{
> + struct rtl8xxxu_priv *priv = hw->priv;
> + struct device *dev = &priv->udev->dev;
> +
> + dev_dbg(dev, "Start AP mode\n");
> + rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
> + rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
> + priv->fops->report_connect(priv, 0, true);
> +
> + return 0;
> +}
> +
> static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
> {
> u32 rtlqueue;
> @@ -7026,6 +7040,7 @@ static const struct ieee80211_ops rtl8xxxu_ops = {
> .config = rtl8xxxu_config,
> .conf_tx = rtl8xxxu_conf_tx,
> .bss_info_changed = rtl8xxxu_bss_info_changed,
> + .start_ap = rtl8xxxu_start_ap,
> .configure_filter = rtl8xxxu_configure_filter,
> .set_rts_threshold = rtl8xxxu_set_rts_threshold,
> .start = rtl8xxxu_start,
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 02/14] wifi: rtl8xxxu: Select correct queue for beacon frames
>
> Use the special beacon queue for beacon frames instead of the management
> frame queue, so that the HW sends them periodically instead of only
> once.
Frames with beacon queue will be put in a special area called reserved page
where hardware will treat the frame as beacon frame and send out periodically
when net_type is configured as AP mode.
>
> Signed-off-by: Martin Kaistra <[email protected]>
Reviewed-by: Ping-Ke Shih <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 90b98b9dcbd9d..daeaa7d6864f9 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4941,7 +4941,9 @@ static u32 rtl8xxxu_queue_select(struct ieee80211_hdr *hdr, struct sk_buff *skb)
> {
> u32 queue;
>
> - if (ieee80211_is_mgmt(hdr->frame_control))
> + if (unlikely(ieee80211_is_beacon(hdr->frame_control)))
> + queue = TXDESC_QUEUE_BEACON;
> + else if (ieee80211_is_mgmt(hdr->frame_control))
> queue = TXDESC_QUEUE_MGNT;
> else
> queue = rtl8xxxu_80211_to_rtl_queue(skb_get_queue_mapping(skb));
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 05/14] wifi: rtl8xxxu: Allow setting rts threshold to -1
>
> The default setting in hostapd.conf for rts threshold is -1, which means
> disabled. Allow to set it.
>
> Signed-off-by: Martin Kaistra <[email protected]>
Reviewed-by: Ping-Ke Shih <[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 f8380a2d51ae2..b233c66a7a5a8 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6592,7 +6592,7 @@ static void rtl8xxxu_configure_filter(struct ieee80211_hw *hw,
>
> static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
> {
> - if (rts > 2347)
> + if (rts > 2347 && rts != (u32)-1)
> return -EINVAL;
>
> return 0;
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 03/14] wifi: rtl8xxxu: Add beacon functions
>
> Add a workqueue to update the beacon contents asynchronously and
> implement downloading the beacon to the HW and starting beacon tx like
> the vendor driver.
>
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 3 +
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 81 +++++++++++++++++++
> 2 files changed, 84 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 9d48c69ffece1..cac985271628c 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1744,6 +1744,8 @@ struct rtl8xxxu_priv {
> bool shutdown;
> struct work_struct rx_urb_wq;
>
> + bool beacon_enabled;
> +
> u8 mac_addr[ETH_ALEN];
> char chip_name[8];
> char chip_vendor[8];
> @@ -1850,6 +1852,7 @@ struct rtl8xxxu_priv {
> struct delayed_work ra_watchdog;
> struct work_struct c2hcmd_work;
> struct sk_buff_head c2hcmd_queue;
> + struct work_struct update_beacon_work;
> struct rtl8xxxu_btcoex bt_coex;
> struct rtl8xxxu_ra_report ra_report;
> struct rtl8xxxu_cfo_tracking cfo_tracking;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index daeaa7d6864f9..404fa6e322f58 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -1104,6 +1104,24 @@ static void rtl8xxxu_stop_tx_beacon(struct rtl8xxxu_priv *priv)
> val8 = rtl8xxxu_read8(priv, REG_TBTT_PROHIBIT + 2);
> val8 &= ~BIT(0);
> rtl8xxxu_write8(priv, REG_TBTT_PROHIBIT + 2, val8);
> +
> + priv->beacon_enabled = false;
> +}
> +
> +static void rtl8xxxu_start_tx_beacon(struct rtl8xxxu_priv *priv)
> +{
> + u8 val8;
> +
> + val8 = rtl8xxxu_read8(priv, REG_FWHW_TXQ_CTRL + 2);
> + val8 |= BIT(6);
#define EN_BCNQ_DL BIT(22)
val8 |= EN_BCNQ_DL >> 16;
> + rtl8xxxu_write8(priv, REG_FWHW_TXQ_CTRL + 2, val8);
> +
> + rtl8xxxu_write8(priv, REG_TBTT_PROHIBIT + 1, 0x80);
> + val8 = rtl8xxxu_read8(priv, REG_TBTT_PROHIBIT + 2);
> + val8 &= 0xF0;
> + rtl8xxxu_write8(priv, REG_TBTT_PROHIBIT + 2, val8);
> +
> + priv->beacon_enabled = true;
> }
>
>
> @@ -4895,6 +4913,17 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> dev_dbg(dev, "Changed BASIC_RATES!\n");
> rtl8xxxu_set_basic_rates(priv, bss_conf->basic_rates);
> }
> +
> + if (changed & BSS_CHANGED_BEACON ||
> + (changed & BSS_CHANGED_BEACON_ENABLED &&
> + bss_conf->enable_beacon)) {
> + if (!priv->beacon_enabled) {
> + dev_dbg(dev, "BSS_CHANGED_BEACON_ENABLED\n");
> + rtl8xxxu_start_tx_beacon(priv);
> + schedule_work(&priv->update_beacon_work);
> + }
> + }
> +
> error:
> return;
> }
> @@ -5476,6 +5505,57 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> dev_kfree_skb(skb);
> }
>
> +static void rtl8xxxu_send_beacon_frame(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif)
> +{
> + struct rtl8xxxu_priv *priv = hw->priv;
> + struct sk_buff *skb = ieee80211_beacon_get(hw, vif, 0);
> + struct device *dev = &priv->udev->dev;
> + int retry;
> + u8 val8;
> +
> + /* BCN_VALID, BIT16 of REG_TDECTRL = BIT0 of REG_TDECTRL+2,
> + * write 1 to clear, cleared by SW.
> + */
> + val8 = rtl8xxxu_read8(priv, REG_TDECTRL + 2);
> + val8 |= BIT(0);
#define BIT_BCN_VALID BIT(16)
val8 |= BIT_BCN_VALID >> 16;
> + rtl8xxxu_write8(priv, REG_TDECTRL + 2, val8);
> +
> + /* SW_BCN_SEL - Port0 */
> + val8 = rtl8xxxu_read8(priv, REG_DWBCN1_CTRL_8723B + 2);
> + val8 &= ~BIT(4);
#define BIT_SW_BCN_SEL BIT(20)
val8 &= ~(BIT_SW_BCN_SEL >> 20);
> + rtl8xxxu_write8(priv, REG_DWBCN1_CTRL_8723B + 2, val8);
> +
> + if (skb)
> + rtl8xxxu_tx(hw, NULL, skb);
> +
> + retry = 100;
> + do {
> + val8 = rtl8xxxu_read8(priv, REG_TDECTRL + 2);
> + if (val8 & BIT(0))
#define BIT_BCN_VALID BIT(16)
if (val8 & (BIT_BCN_VALID >> 16)
> + break;
> + usleep_range(10, 20);
> + } while (retry--);
> +
> + if (!retry)
> + dev_err(dev, "%s: Failed to read beacon valid bit\n", __func__);
> +}
> +
> +static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work)
> +{
> + struct rtl8xxxu_priv *priv =
> + container_of(work, struct rtl8xxxu_priv, update_beacon_work);
> + struct ieee80211_hw *hw = priv->hw;
> + struct ieee80211_vif *vif = priv->vif;
> +
> + if (!vif) {
> + WARN_ONCE(true, "no vif to update beacon\n");
> + return;
> + }
> +
> + rtl8xxxu_send_beacon_frame(hw, vif);
> +}
> +
> void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
> struct ieee80211_rx_status *rx_status,
> struct rtl8723au_phy_stats *phy_stats,
> @@ -7244,6 +7324,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> spin_lock_init(&priv->rx_urb_lock);
> INIT_WORK(&priv->rx_urb_wq, rtl8xxxu_rx_urb_work);
> INIT_DELAYED_WORK(&priv->ra_watchdog, rtl8xxxu_watchdog_callback);
> + INIT_WORK(&priv->update_beacon_work, rtl8xxxu_update_beacon_work_callback);
> skb_queue_head_init(&priv->c2hcmd_queue);
>
> usb_set_intfdata(interface, hw);
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 06/14] wifi: rtl8xxxu: Allow creating interface in AP mode
>
> Use the sequence from the vendor driver for setting up the beacon
> related registers.
> Also set the MAC address register.
>
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 31 ++++++++++++++++---
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h | 2 ++
> 2 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index b233c66a7a5a8..b20ff8bc40870 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6408,18 +6408,39 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
> int ret;
> u8 val8;
>
> + if (!priv->vif)
> + priv->vif = vif;
> + else
> + return -EOPNOTSUPP;
> +
> switch (vif->type) {
> case NL80211_IFTYPE_STATION:
> - if (!priv->vif)
> - priv->vif = vif;
> - else
> - return -EOPNOTSUPP;
> rtl8xxxu_stop_tx_beacon(priv);
>
> val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
> val8 |= BEACON_ATIM | BEACON_FUNCTION_ENABLE |
> BEACON_DISABLE_TSF_UPDATE;
> rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
> + ret = 0;
> + break;
> + case NL80211_IFTYPE_AP:
> + rtl8xxxu_write8(priv, REG_BEACON_CTRL,
> + BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
> + rtl8xxxu_write8(priv, REG_ATIMWND, 0x0c); /* 12ms */
> + rtl8xxxu_write16(priv, REG_TSFTR_SYN_OFFSET, 0x7fff); /* ~32ms */
> + rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, DUAL_TSF_RESET_TSF0);
> +
> + /* enable BCN0 function */
> + rtl8xxxu_write8(priv, REG_BEACON_CTRL,
> + BEACON_DISABLE_TSF_UPDATE |
> + BEACON_FUNCTION_ENABLE | BEACON_CTRL_MBSSID |
> + BEACON_CTRL_TX_BEACON_RPT);
> +
> + /* select BCN on port 0 */
> + val8 = rtl8xxxu_read8(priv, REG_CCK_CHECK);
> + val8 &= ~BIT_BCN_PORT_SEL;
> + rtl8xxxu_write8(priv, REG_CCK_CHECK, val8);
> +
> ret = 0;
> break;
> default:
> @@ -6427,6 +6448,8 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
> }
>
> rtl8xxxu_set_linktype(priv, vif->type);
> + ether_addr_copy(priv->mac_addr, vif->addr);
> + rtl8xxxu_set_mac(priv);
rtl8xxxu_set_mac() is already called by rtl8xxxu_init_device(). Is there
anything unexpected?
While I reviewed first patch, I would like to suggest to call calibration:
fops->phy_lc_calibrate(priv);
fops->phy_iq_calibrate(priv);
I traced rtl8xxxu and saw they present in rtl8xxxu_init_device() and rtl8xxxu
doesn't implement idle power saving to turn off power of wifi card. Then, I
think the calibration will be fine if rtl8xxxu only does it once.
So, I would like to know if something I miss.
>
> return ret;
> }
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> index 4dffbab494c3b..83e7f8fd82c0a 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> @@ -572,6 +572,8 @@
> #define REG_ARFR1 0x0448
> #define REG_ARFR2 0x044c
> #define REG_ARFR3 0x0450
> +#define REG_CCK_CHECK 0x0454
> +#define BIT_BCN_PORT_SEL BIT(5)
> #define REG_AMPDU_MAX_TIME_8723B 0x0456
> #define REG_AGGLEN_LMT 0x0458
> #define REG_AMPDU_MIN_SPACE 0x045c
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 07/14] wifi: rtl8xxxu: Add parameter macid to update_rate_mask
>
> The HW maintains a rate_mask for each connection, referenced by the
> macid. Add a parameter to update_rate_mask and add the macid to the
> h2c call in the gen2 implementation.
>
> Also extend refresh_rate_mask to generate the macid in AP mode from
> sta->aid.
Firmware can support 32 mac_id (station instance) at most, so it will be a
problem if hostapd assigns aid more than 32. Though I'm not clear how
hostpad assigns the aid, it would be always safe that rtl8xxxu maintains
mac_id by a bitmap in driver.
>
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 7 ++++---
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c | 3 ++-
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 17 +++++++++++++----
> 3 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index cac985271628c..c06ad33645974 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1905,7 +1905,8 @@ struct rtl8xxxu_fileops {
> void (*set_tx_power) (struct rtl8xxxu_priv *priv, int channel,
> bool ht40);
> void (*update_rate_mask) (struct rtl8xxxu_priv *priv,
> - u32 ramask, u8 rateid, int sgi, int txbw_40mhz);
> + u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
> + u8 macid);
> void (*report_connect) (struct rtl8xxxu_priv *priv,
> u8 macid, bool connect);
> void (*report_rssi) (struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
> @@ -2007,9 +2008,9 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw);
> void rtl8xxxu_gen1_usb_quirks(struct rtl8xxxu_priv *priv);
> void rtl8xxxu_gen2_usb_quirks(struct rtl8xxxu_priv *priv);
> void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> - u32 ramask, u8 rateid, int sgi, int txbw_40mhz);
> + u32 ramask, u8 rateid, int sgi, int txbw_40mhz, u8 macid);
> void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> - u32 ramask, u8 rateid, int sgi, int txbw_40mhz);
> + u32 ramask, u8 rateid, int sgi, int txbw_40mhz, u8 macid);
> void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> u8 macid, bool connect);
> void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
> index 6a82ec47568ee..c3dc5130c9f37 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
> @@ -1798,7 +1798,8 @@ static void rtl8188e_arfb_refresh(struct rtl8xxxu_ra_info *ra)
>
> static void
> rtl8188e_update_rate_mask(struct rtl8xxxu_priv *priv,
> - u32 ramask, u8 rateid, int sgi, int txbw_40mhz)
> + u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
> + u8 macid)
> {
> struct rtl8xxxu_ra_info *ra = &priv->ra_info;
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index b20ff8bc40870..b5cb15e472f1c 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4471,7 +4471,8 @@ static void rtl8xxxu_sw_scan_complete(struct ieee80211_hw *hw,
> }
>
> void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> - u32 ramask, u8 rateid, int sgi, int txbw_40mhz)
> + u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
> + u8 macid)
> {
> struct h2c_cmd h2c;
>
> @@ -4491,7 +4492,8 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> }
>
> void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> - u32 ramask, u8 rateid, int sgi, int txbw_40mhz)
> + u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
> + u8 macid)
> {
> struct h2c_cmd h2c;
> u8 bw;
> @@ -4508,6 +4510,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> h2c.b_macid_cfg.ramask1 = (ramask >> 8) & 0xff;
> h2c.b_macid_cfg.ramask2 = (ramask >> 16) & 0xff;
> h2c.b_macid_cfg.ramask3 = (ramask >> 24) & 0xff;
> + h2c.b_macid_cfg.macid = macid;
>
> h2c.b_macid_cfg.data1 = rateid;
> if (sgi)
> @@ -4870,7 +4873,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> priv->vif = vif;
> priv->rssi_level = RTL8XXXU_RATR_STA_INIT;
>
> - priv->fops->update_rate_mask(priv, ramask, 0, sgi, bw == RATE_INFO_BW_40);
> + priv->fops->update_rate_mask(priv, ramask, 0, sgi,
> + bw == RATE_INFO_BW_40, 0);
>
> rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
>
> @@ -6772,6 +6776,7 @@ static void rtl8xxxu_refresh_rate_mask(struct rtl8xxxu_priv *priv,
> u8 txbw_40mhz;
> u8 snr, snr_thresh_high, snr_thresh_low;
> u8 go_up_gap = 5;
> + u8 macid = 0;
>
> rssi_level = priv->rssi_level;
> snr = rtl8xxxu_signal_to_snr(signal);
> @@ -6891,7 +6896,11 @@ static void rtl8xxxu_refresh_rate_mask(struct rtl8xxxu_priv *priv,
> }
>
> priv->rssi_level = rssi_level;
> - priv->fops->update_rate_mask(priv, rate_bitmap, ratr_idx, sgi, txbw_40mhz);
> +
> + if (priv->vif->type == NL80211_IFTYPE_AP)
> + macid = sta->aid + 1;
We should reserve a special mac_id for broadcast packets, because rate adaptive
algorithm in firmware also uses mac_id as instance ID of rate selection.
> +
> + priv->fops->update_rate_mask(priv, rate_bitmap, ratr_idx, sgi, txbw_40mhz, macid);
> }
> }
>
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 08/14] wifi: rtl8xxxu: Actually use macid in rtl8xxxu_gen2_report_connect
>
> The report_connect function has had a macid parameter from the
> beginning, but it has not been used, because in STA mode, the value was
> always zero.
> As it can now have different values in AP mode, actually wire it up to
> the H2C command.
>
> Signed-off-by: Martin Kaistra <[email protected]>
Reviewed-by: Ping-Ke Shih <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index b5cb15e472f1c..4209880d724be 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4557,6 +4557,8 @@ void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> else
> h2c.media_status_rpt.parm &= ~BIT(0);
>
> + h2c.media_status_rpt.macid = macid;
> +
> rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
> }
>
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 10/14] wifi: rtl8xxxu: Add sta_add() callback
>
> This function gets called in AP mode, when a new STA gets associated to
> us. Call rtl8xxxu_refresh_rate_mask() to set a rate mask for the newly
> connected STA (referenced by the macid) and then send a media connnect
> report.
>
> Signed-off-by: Martin Kaistra <[email protected]>
Reviewed-by: Ping-Ke Shih <[email protected]>
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 5e36fddbbb488..d74a3c6452507 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -7159,6 +7159,19 @@ static void rtl8xxxu_stop(struct ieee80211_hw *hw)
> rtl8xxxu_free_tx_resources(priv);
> }
>
> +static int rtl8xxxu_sta_add(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + struct ieee80211_sta *sta)
> +{
> + struct rtl8xxxu_priv *priv = hw->priv;
> +
> + if (sta) {
> + rtl8xxxu_refresh_rate_mask(priv, 0, sta);
> + priv->fops->report_connect(priv, sta->aid + 1, H2C_ROLE_STA, true);
> + }
> + return 0;
> +}
> +
> static const struct ieee80211_ops rtl8xxxu_ops = {
> .tx = rtl8xxxu_tx,
> .wake_tx_queue = ieee80211_handle_wake_tx_queue,
> @@ -7179,6 +7192,7 @@ static const struct ieee80211_ops rtl8xxxu_ops = {
> .sta_statistics = rtl8xxxu_sta_statistics,
> .get_antenna = rtl8xxxu_get_antenna,
> .set_tim = rtl8xxxu_set_tim,
> + .sta_add = rtl8xxxu_sta_add,
> };
>
> static int rtl8xxxu_parse_usb(struct rtl8xxxu_priv *priv,
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 09/14] wifi: rtl8xxxu: Add parameter role to report_connect
>
> This allows to tell the HW if a connection is made to a STA or an AP.
> Add the implementation for the gen2 version.
>
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 9 ++++++---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 11 ++++++-----
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index c06ad33645974..e78e0bbd23354 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1280,6 +1280,9 @@ struct rtl8xxxu_rfregs {
> #define H2C_JOIN_BSS_DISCONNECT 0
> #define H2C_JOIN_BSS_CONNECT 1
>
> +#define H2C_ROLE_STA 1
> +#define H2C_ROLE_AP 2
> +
> /*
> * H2C (firmware) commands differ between the older generation chips
> * 8188[cr]u, 819[12]cu, and 8723au, and the more recent chips 8723bu,
> @@ -1908,7 +1911,7 @@ struct rtl8xxxu_fileops {
> u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
> u8 macid);
> void (*report_connect) (struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect);
> + u8 macid, u8 role, bool connect);
> void (*report_rssi) (struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
> void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> @@ -2012,9 +2015,9 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> u32 ramask, u8 rateid, int sgi, int txbw_40mhz, u8 macid);
> void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect);
> + u8 macid, u8 role, bool connect);
> void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect);
> + u8 macid, u8 role, bool connect);
> void rtl8xxxu_gen1_report_rssi(struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
> void rtl8xxxu_gen2_report_rssi(struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
> void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv);
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 4209880d724be..5e36fddbbb488 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4524,7 +4524,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> }
>
> void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect)
> + u8 macid, u8 role, bool connect)
> {
> struct h2c_cmd h2c;
>
> @@ -4541,7 +4541,7 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> }
>
> void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect)
> + u8 macid, u8 role, bool connect)
> {
> /*
> * The firmware turns on the rate control when it knows it's
> @@ -4557,6 +4557,7 @@ void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> else
> h2c.media_status_rpt.parm &= ~BIT(0);
>
> + h2c.media_status_rpt.parm |= ((role << 4) & 0xf0);
> h2c.media_status_rpt.macid = macid;
>
> rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
> @@ -4886,13 +4887,13 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> rtl8xxxu_write16(priv, REG_BCN_PSR_RPT,
> 0xc000 | vif->cfg.aid);
>
> - priv->fops->report_connect(priv, 0, true);
> + priv->fops->report_connect(priv, 0, H2C_ROLE_AP, true);
Should it be called only AP mode? Because STA mode can run into this too.
> } else {
> val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
> val8 |= BEACON_DISABLE_TSF_UPDATE;
> rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>
> - priv->fops->report_connect(priv, 0, false);
> + priv->fops->report_connect(priv, 0, H2C_ROLE_AP, false);
> }
> }
>
> @@ -4953,7 +4954,7 @@ static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> dev_dbg(dev, "Start AP mode\n");
> rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
> rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
> - priv->fops->report_connect(priv, 0, true);
> + priv->fops->report_connect(priv, 0, 0, true);
>
> return 0;
> }
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 12/14] wifi: rtl8xxxu: Enable hw seq for all non-qos frames
>
> Beacon frames are generated by the HW and therefore contain a HW
> generated seq number. Enable HW sequence number for other frames to
> match that.
>
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index c232de1d47173..82fbe778fc5ec 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -5275,6 +5275,9 @@ rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> tx_desc40->txdw4 |= cpu_to_le32(TXDESC40_RETRY_LIMIT_ENABLE);
> }
>
> + if (!ieee80211_is_data_qos(hdr->frame_control))
Since beacon is management frame, should it be ieee80211_is_mgmt() ?
> + tx_desc40->txdw8 |= cpu_to_le32(TXDESC40_HW_SEQ_ENABLE);
> +
> if (short_preamble)
> tx_desc40->txdw5 |= cpu_to_le32(TXDESC40_SHORT_PREAMBLE);
>
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 11/14] wifi: rtl8xxxu: Put the macid in txdesc
>
> Add a parameter macid to fill_txdesc(), implement setting it for the
> gen2 version.
> This is used to tell the HW who the recipient of the packet is, so that
> the appropriate data rate can be selected.
>
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 8 ++++----
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 16 ++++++++++++----
> 2 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index e78e0bbd23354..20304b0bd68a3 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1917,7 +1917,7 @@ struct rtl8xxxu_fileops {
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
> bool short_preamble, bool ampdu_enable,
> - u32 rts_rate);
> + u32 rts_rate, u8 macid);
> void (*set_crystal_cap) (struct rtl8xxxu_priv *priv, u8 crystal_cap);
> s8 (*cck_rssi) (struct rtl8xxxu_priv *priv, struct rtl8723au_phy_stats *phy_stats);
> int (*led_classdev_brightness_set) (struct led_classdev *led_cdev,
> @@ -2046,17 +2046,17 @@ void rtl8xxxu_fill_txdesc_v1(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
> bool short_preamble, bool ampdu_enable,
> - u32 rts_rate);
> + u32 rts_rate, u8 macid);
> void rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
> bool short_preamble, bool ampdu_enable,
> - u32 rts_rate);
> + u32 rts_rate, u8 macid);
> void rtl8xxxu_fill_txdesc_v3(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
> bool short_preamble, bool ampdu_enable,
> - u32 rts_rate);
> + u32 rts_rate, u8 macid);
> void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
> u8 arg1, u8 arg2, u8 arg3, u8 arg4, u8 arg5);
> void rtl8723bu_phy_init_antenna_selection(struct rtl8xxxu_priv *priv);
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index d74a3c6452507..c232de1d47173 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -5152,7 +5152,8 @@ void
> rtl8xxxu_fill_txdesc_v1(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
> - bool short_preamble, bool ampdu_enable, u32 rts_rate)
> + bool short_preamble, bool ampdu_enable, u32 rts_rate,
> + u8 macid)
> {
> struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
> struct rtl8xxxu_priv *priv = hw->priv;
> @@ -5224,7 +5225,8 @@ void
> rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
> - bool short_preamble, bool ampdu_enable, u32 rts_rate)
> + bool short_preamble, bool ampdu_enable, u32 rts_rate,
> + u8 macid)
> {
> struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
> struct rtl8xxxu_priv *priv = hw->priv;
> @@ -5248,6 +5250,8 @@ rtl8xxxu_fill_txdesc_v2(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_desc40->pkt_size));
>
> + tx_desc40->txdw1 |= cpu_to_le32(macid << TXDESC40_MACID_SHIFT);
> +
> seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
>
> tx_desc40->txdw4 = cpu_to_le32(rate);
> @@ -5299,7 +5303,8 @@ void
> rtl8xxxu_fill_txdesc_v3(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
> - bool short_preamble, bool ampdu_enable, u32 rts_rate)
> + bool short_preamble, bool ampdu_enable, u32 rts_rate,
> + u8 macid)
> {
> struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
> struct rtl8xxxu_priv *priv = hw->priv;
> @@ -5398,6 +5403,7 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> u16 pktlen = skb->len;
> u16 rate_flag = tx_info->control.rates[0].flags;
> int tx_desc_size = priv->fops->tx_desc_size;
> + u8 macid = 0;
> int ret;
> bool ampdu_enable, sgi = false, short_preamble = false;
>
> @@ -5497,9 +5503,11 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> else
> rts_rate = 0;
>
> + if (vif->type == NL80211_IFTYPE_AP && sta)
> + macid = sta->aid + 1;
As suggestion before, please make sure hostapd never assigns a big number like 100 as aid.
>
> priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
> - ampdu_enable, rts_rate);
> + ampdu_enable, rts_rate, macid);
>
> rtl8xxxu_calc_tx_desc_csum(tx_desc);
>
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 13/14] wifi: rtl8xxxu: Clean up filter configuration
>
> In AP mode, RCR_CHECK_BSSID_MATCH should not be set. Rearrange RCR bits
> to filter flags to match other realtek drivers and don't set
> RCR_CHECK_BSSID_BEACON and RCR_CHECK_BSSID_MATCH in AP mode.
>
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 82fbe778fc5ec..b6f811ad01333 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6597,23 +6597,24 @@ static void rtl8xxxu_configure_filter(struct ieee80211_hw *hw,
> * FIF_PLCPFAIL not supported?
> */
>
> - if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
> - rcr &= ~RCR_CHECK_BSSID_BEACON;
> - else
> - rcr |= RCR_CHECK_BSSID_BEACON;
> + if (priv->vif->type != NL80211_IFTYPE_AP) {
I think mac80211 configure filters depends on operating conditions, so it would
be possible to avoid checking vif->type.
> + if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
> + rcr &= ~(RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH);
> + else
> + rcr |= RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH;
> + } else {
> + rcr &= ~RCR_CHECK_BSSID_MATCH;
> + }
>
> if (*total_flags & FIF_CONTROL)
> rcr |= RCR_ACCEPT_CTRL_FRAME;
> else
> rcr &= ~RCR_ACCEPT_CTRL_FRAME;
>
> - if (*total_flags & FIF_OTHER_BSS) {
> + if (*total_flags & FIF_OTHER_BSS)
> rcr |= RCR_ACCEPT_AP;
> - rcr &= ~RCR_CHECK_BSSID_MATCH;
> - } else {
> + else
> rcr &= ~RCR_ACCEPT_AP;
> - rcr |= RCR_CHECK_BSSID_MATCH;
> - }
>
> if (*total_flags & FIF_PSPOLL)
> rcr |= RCR_ACCEPT_PM;
> --
> 2.30.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Thursday, March 23, 2023 1:19 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: [RFC PATCH 14/14] wifi: rtl8xxxu: Declare AP mode support for 8188f
>
> Everything is in place now for AP mode, we can tell the system that we
> support it. Put the feature behind a flag in priv->fops, because it is
> not (yet) implemented for all chips.
>
> Signed-off-by: Martin Kaistra <[email protected]>
Reviewed-by: Ping-Ke Shih <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 1 +
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c | 1 +
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 ++
> 3 files changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 20304b0bd68a3..31f9cf9e558d7 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1930,6 +1930,7 @@ struct rtl8xxxu_fileops {
> u8 has_tx_report:1;
> u8 gen2_thermal_meter:1;
> u8 needs_full_init:1;
> + u8 supports_ap:1;
> u32 adda_1t_init;
> u32 adda_1t_path_on;
> u32 adda_2t_path_on_a;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> index 82dee1fed4779..c4c1f015a7fd9 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> @@ -1746,6 +1746,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
> .has_tx_report = 1,
> .gen2_thermal_meter = 1,
> .needs_full_init = 1,
> + .supports_ap = 1,
> .adda_1t_init = 0x03c00014,
> .adda_1t_path_on = 0x03c00014,
> .trxff_boundary = 0x3f7f,
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index b6f811ad01333..31bd1f2711aed 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -7449,6 +7449,8 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> hw->wiphy->max_scan_ssids = 1;
> hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
> hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
> + if (priv->fops->supports_ap)
> + hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP);
> hw->queues = 4;
>
> sband = &rtl8xxxu_supported_band;
> --
> 2.30.2
Hi,
Am 23.03.23 um 18:12 schrieb Bitterblue Smith:
> On 22/03/2023 19:18, Martin Kaistra wrote:
>> This series intends to bring AP mode support to the rtl8xxxu driver,
>> more specifically for the 8188f, because this is the HW I have.
>> The work is based on the vendor driver as I do not have access to
>> datasheets.
>>
>> This is an RFC, so that there can be a discussion first before
>> potentially implementing support for the other chips in this driver, if
>> required.
>>
>
> Hi!
>
> I ran into some problems while testing this.
>
> First, a null pointer dereference in rtl8xxxu_config_filter() when
> turning on the AP. priv->vif was NULL:
>
> if (priv->vif->type != NL80211_IFTYPE_AP) {
>
> I changed it like this:
>
> if (priv->vif && priv->vif->type != NL80211_IFTYPE_AP) {
>
> Then I was able to turn on the AP and connect my phone to it. However,
> the system froze after a few seconds. I had `journalctl --follow`
> running. The last thing printed before the system froze was the DHCP
> stuff (discover, offer, request, ack). The phone said it was connected,
> but speedtest.net didn't have time to load before the freeze.
>
> My system is a laptop with RTL8822CE internal wifi card connected to my
> ISP's router. The connections are managed by NetworkManager 1.42.4-1,
> which uses wpa_supplicant 2:2.10-8 and dnsmasq 2.89-1. The operating
> system is Arch Linux running kernel 6.2.5-arch1-1.
>
> I used Plasma's NetworkManager applet to create a new "Wi-Fi (shared)"
> connection with mode "Access Point", band 2.4 GHz, channel 1, no
> encryption, and "IPv4 is required for this connection".
Thanks for your bug report.
I did use hostapd for testing, I will try to see, if I can reproduce any
problems when using NetworkManager.
>
>
> Unrelated to anything, just out of curiosity, what brand/model is your
> RTL8188FU?
This should be the one I have:
https://www.fn-link.com/6188S-UF-Wi-Fi-Module-pd41531470.html (Fn-Link
6188S-UF)
Martin
Ping-Ke Shih <[email protected]> writes:
>> -----Original Message-----
>> From: Martin Kaistra <[email protected]>
>> Sent: Thursday, March 23, 2023 1:19 AM
>> To: [email protected]
>> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
>> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
>> <[email protected]>
>> Subject: [RFC PATCH 07/14] wifi: rtl8xxxu: Add parameter macid to update_rate_mask
>>
>> The HW maintains a rate_mask for each connection, referenced by the
>> macid. Add a parameter to update_rate_mask and add the macid to the
>> h2c call in the gen2 implementation.
>>
>> Also extend refresh_rate_mask to generate the macid in AP mode from
>> sta->aid.
>
> Firmware can support 32 mac_id (station instance) at most, so it will be a
> problem if hostapd assigns aid more than 32. Though I'm not clear how
> hostpad assigns the aid, it would be always safe that rtl8xxxu maintains
> mac_id by a bitmap in driver.
Does rtlw8xxxu set struct wiphy::max_ap_assoc_sta? It would be good to
advertise the user space the maximum number of stations.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
> -----Original Message-----
> From: Kalle Valo <[email protected]>
> Sent: Monday, March 27, 2023 4:42 PM
> To: Ping-Ke Shih <[email protected]>
> Cc: Martin Kaistra <[email protected]>; [email protected]; Jes Sorensen
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> Subject: Re: [RFC PATCH 07/14] wifi: rtl8xxxu: Add parameter macid to update_rate_mask
>
> Ping-Ke Shih <[email protected]> writes:
>
> >> -----Original Message-----
> >> From: Martin Kaistra <[email protected]>
> >> Sent: Thursday, March 23, 2023 1:19 AM
> >> To: [email protected]
> >> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> >> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> >> <[email protected]>
> >> Subject: [RFC PATCH 07/14] wifi: rtl8xxxu: Add parameter macid to update_rate_mask
> >>
> >> The HW maintains a rate_mask for each connection, referenced by the
> >> macid. Add a parameter to update_rate_mask and add the macid to the
> >> h2c call in the gen2 implementation.
> >>
> >> Also extend refresh_rate_mask to generate the macid in AP mode from
> >> sta->aid.
> >
> > Firmware can support 32 mac_id (station instance) at most, so it will be a
> > problem if hostapd assigns aid more than 32. Though I'm not clear how
> > hostpad assigns the aid, it would be always safe that rtl8xxxu maintains
> > mac_id by a bitmap in driver.
>
> Does rtlw8xxxu set struct wiphy::max_ap_assoc_sta? It would be good to
> advertise the user space the maximum number of stations.
>
Thanks for this information, Kalle.
Martin, please add this. I think we can preserve at least one mac_id for
broadcast/multicast frames. In fact, I'm not absolutely sure we can
support up to 32 mac_id, so set wiphy::max_ap_assoc_sta = 16 -1 or -2
would be safer.
Ping-Ke
On 22/03/2023 19:18, Martin Kaistra wrote:
> This gets called at the start of AP mode operation. Set bssid, beacon
> interval and send a connect report to the HW.
>
Hmm, but why send a connect report when you don't have anything
connected yet?
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index c152b228606f1..90b98b9dcbd9d 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4899,6 +4899,20 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> return;
> }
>
> +static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> + struct ieee80211_bss_conf *link_conf)
> +{
> + struct rtl8xxxu_priv *priv = hw->priv;
> + struct device *dev = &priv->udev->dev;
> +
> + dev_dbg(dev, "Start AP mode\n");
> + rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
> + rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
> + priv->fops->report_connect(priv, 0, true);
> +
> + return 0;
> +}
> +
> static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
> {
> u32 rtlqueue;
> @@ -7026,6 +7040,7 @@ static const struct ieee80211_ops rtl8xxxu_ops = {
> .config = rtl8xxxu_config,
> .conf_tx = rtl8xxxu_conf_tx,
> .bss_info_changed = rtl8xxxu_bss_info_changed,
> + .start_ap = rtl8xxxu_start_ap,
> .configure_filter = rtl8xxxu_configure_filter,
> .set_rts_threshold = rtl8xxxu_set_rts_threshold,
> .start = rtl8xxxu_start,
On 22/03/2023 19:18, Martin Kaistra wrote:
> Add a workqueue to update the beacon contents asynchronously and
> implement downloading the beacon to the HW and starting beacon tx like
> the vendor driver.
>
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 3 +
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 81 +++++++++++++++++++
> 2 files changed, 84 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 9d48c69ffece1..cac985271628c 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1744,6 +1744,8 @@ struct rtl8xxxu_priv {
> bool shutdown;
> struct work_struct rx_urb_wq;
>
> + bool beacon_enabled;
> +
> u8 mac_addr[ETH_ALEN];
> char chip_name[8];
> char chip_vendor[8];
> @@ -1850,6 +1852,7 @@ struct rtl8xxxu_priv {
> struct delayed_work ra_watchdog;
> struct work_struct c2hcmd_work;
> struct sk_buff_head c2hcmd_queue;
> + struct work_struct update_beacon_work;
> struct rtl8xxxu_btcoex bt_coex;
> struct rtl8xxxu_ra_report ra_report;
> struct rtl8xxxu_cfo_tracking cfo_tracking;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index daeaa7d6864f9..404fa6e322f58 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -1104,6 +1104,24 @@ static void rtl8xxxu_stop_tx_beacon(struct rtl8xxxu_priv *priv)
> val8 = rtl8xxxu_read8(priv, REG_TBTT_PROHIBIT + 2);
> val8 &= ~BIT(0);
> rtl8xxxu_write8(priv, REG_TBTT_PROHIBIT + 2, val8);
> +
> + priv->beacon_enabled = false;
> +}
> +
> +static void rtl8xxxu_start_tx_beacon(struct rtl8xxxu_priv *priv)
> +{
> + u8 val8;
> +
> + val8 = rtl8xxxu_read8(priv, REG_FWHW_TXQ_CTRL + 2);
> + val8 |= BIT(6);
> + rtl8xxxu_write8(priv, REG_FWHW_TXQ_CTRL + 2, val8);
> +
> + rtl8xxxu_write8(priv, REG_TBTT_PROHIBIT + 1, 0x80);
> + val8 = rtl8xxxu_read8(priv, REG_TBTT_PROHIBIT + 2);
> + val8 &= 0xF0;
> + rtl8xxxu_write8(priv, REG_TBTT_PROHIBIT + 2, val8);
> +
> + priv->beacon_enabled = true;
> }
>
>
> @@ -4895,6 +4913,17 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> dev_dbg(dev, "Changed BASIC_RATES!\n");
> rtl8xxxu_set_basic_rates(priv, bss_conf->basic_rates);
> }
> +
> + if (changed & BSS_CHANGED_BEACON ||
> + (changed & BSS_CHANGED_BEACON_ENABLED &&
> + bss_conf->enable_beacon)) {
> + if (!priv->beacon_enabled) {
> + dev_dbg(dev, "BSS_CHANGED_BEACON_ENABLED\n");
> + rtl8xxxu_start_tx_beacon(priv);
> + schedule_work(&priv->update_beacon_work);
> + }
Is it not necessary to stop transmitting the beacons when
bss_conf->enable_beacon is false?
> + }
> +
> error:
> return;
> }
> @@ -5476,6 +5505,57 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> dev_kfree_skb(skb);
> }
>
> +static void rtl8xxxu_send_beacon_frame(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif)
> +{
> + struct rtl8xxxu_priv *priv = hw->priv;
> + struct sk_buff *skb = ieee80211_beacon_get(hw, vif, 0);> + struct device *dev = &priv->udev->dev;
> + int retry;
> + u8 val8;
> +
> + /* BCN_VALID, BIT16 of REG_TDECTRL = BIT0 of REG_TDECTRL+2,
> + * write 1 to clear, cleared by SW.
> + */
> + val8 = rtl8xxxu_read8(priv, REG_TDECTRL + 2);
> + val8 |= BIT(0);
> + rtl8xxxu_write8(priv, REG_TDECTRL + 2, val8);
> +
> + /* SW_BCN_SEL - Port0 */
> + val8 = rtl8xxxu_read8(priv, REG_DWBCN1_CTRL_8723B + 2);
> + val8 &= ~BIT(4);
> + rtl8xxxu_write8(priv, REG_DWBCN1_CTRL_8723B + 2, val8);
> +
> + if (skb)
> + rtl8xxxu_tx(hw, NULL, skb);
> +
> + retry = 100;
> + do {
> + val8 = rtl8xxxu_read8(priv, REG_TDECTRL + 2);
> + if (val8 & BIT(0))
> + break;
> + usleep_range(10, 20);
> + } while (retry--);
> +
> + if (!retry)
> + dev_err(dev, "%s: Failed to read beacon valid bit\n", __func__);
> +}
> +
> +static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work)
> +{
> + struct rtl8xxxu_priv *priv =
> + container_of(work, struct rtl8xxxu_priv, update_beacon_work);
> + struct ieee80211_hw *hw = priv->hw;
> + struct ieee80211_vif *vif = priv->vif;
> +
> + if (!vif) {
> + WARN_ONCE(true, "no vif to update beacon\n");
> + return;
> + }
> +
> + rtl8xxxu_send_beacon_frame(hw, vif);
> +}
> +
> void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
> struct ieee80211_rx_status *rx_status,
> struct rtl8723au_phy_stats *phy_stats,
> @@ -7244,6 +7324,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> spin_lock_init(&priv->rx_urb_lock);
> INIT_WORK(&priv->rx_urb_wq, rtl8xxxu_rx_urb_work);
> INIT_DELAYED_WORK(&priv->ra_watchdog, rtl8xxxu_watchdog_callback);
> + INIT_WORK(&priv->update_beacon_work, rtl8xxxu_update_beacon_work_callback);
> skb_queue_head_init(&priv->c2hcmd_queue);
>
> usb_set_intfdata(interface, hw);
On 22/03/2023 19:19, Martin Kaistra wrote:
> This allows to tell the HW if a connection is made to a STA or an AP.
> Add the implementation for the gen2 version.
>
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 9 ++++++---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 11 ++++++-----
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index c06ad33645974..e78e0bbd23354 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1280,6 +1280,9 @@ struct rtl8xxxu_rfregs {
> #define H2C_JOIN_BSS_DISCONNECT 0
> #define H2C_JOIN_BSS_CONNECT 1
>
> +#define H2C_ROLE_STA 1
> +#define H2C_ROLE_AP 2
> +
They describe the role of a macid, so maybe call them H2C_MACID_ROLE_*.
> /*
> * H2C (firmware) commands differ between the older generation chips
> * 8188[cr]u, 819[12]cu, and 8723au, and the more recent chips 8723bu,
> @@ -1908,7 +1911,7 @@ struct rtl8xxxu_fileops {
> u32 ramask, u8 rateid, int sgi, int txbw_40mhz,
> u8 macid);
> void (*report_connect) (struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect);
> + u8 macid, u8 role, bool connect);
> void (*report_rssi) (struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
> void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> @@ -2012,9 +2015,9 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> u32 ramask, u8 rateid, int sgi, int txbw_40mhz, u8 macid);
> void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect);
> + u8 macid, u8 role, bool connect);
> void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect);
> + u8 macid, u8 role, bool connect);
> void rtl8xxxu_gen1_report_rssi(struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
> void rtl8xxxu_gen2_report_rssi(struct rtl8xxxu_priv *priv, u8 macid, u8 rssi);
> void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv);
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 4209880d724be..5e36fddbbb488 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4524,7 +4524,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> }
>
> void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect)
> + u8 macid, u8 role, bool connect)
> {
> struct h2c_cmd h2c;
>
> @@ -4541,7 +4541,7 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> }
>
> void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect)
> + u8 macid, u8 role, bool connect)
> {
> /*
> * The firmware turns on the rate control when it knows it's
> @@ -4557,6 +4557,7 @@ void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> else
> h2c.media_status_rpt.parm &= ~BIT(0);
>
> + h2c.media_status_rpt.parm |= ((role << 4) & 0xf0);
> h2c.media_status_rpt.macid = macid;
>
> rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
> @@ -4886,13 +4887,13 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> rtl8xxxu_write16(priv, REG_BCN_PSR_RPT,
> 0xc000 | vif->cfg.aid);
>
> - priv->fops->report_connect(priv, 0, true);
> + priv->fops->report_connect(priv, 0, H2C_ROLE_AP, true);
> } else {
> val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
> val8 |= BEACON_DISABLE_TSF_UPDATE;
> rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>
> - priv->fops->report_connect(priv, 0, false);
> + priv->fops->report_connect(priv, 0, H2C_ROLE_AP, false);
> }
> }
>
> @@ -4953,7 +4954,7 @@ static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> dev_dbg(dev, "Start AP mode\n");
> rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
> rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
> - priv->fops->report_connect(priv, 0, true);
> + priv->fops->report_connect(priv, 0, 0, true);
>
> return 0;
> }
On 22/03/2023 19:19, Martin Kaistra wrote:
> Add a parameter macid to fill_txdesc(), implement setting it for the
> gen2 version.
> This is used to tell the HW who the recipient of the packet is, so that
> the appropriate data rate can be selected.
>
> Signed-off-by: Martin Kaistra <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 8 ++++----
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 16 ++++++++++++----
> 2 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index e78e0bbd23354..20304b0bd68a3 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1917,7 +1917,7 @@ struct rtl8xxxu_fileops {
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
> bool short_preamble, bool ampdu_enable,
> - u32 rts_rate);
> + u32 rts_rate, u8 macid);
> void (*set_crystal_cap) (struct rtl8xxxu_priv *priv, u8 crystal_cap);
> s8 (*cck_rssi) (struct rtl8xxxu_priv *priv, struct rtl8723au_phy_stats *phy_stats);
> int (*led_classdev_brightness_set) (struct led_classdev *led_cdev,
> @@ -2046,17 +2046,17 @@ void rtl8xxxu_fill_txdesc_v1(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
> bool short_preamble, bool ampdu_enable,
> - u32 rts_rate);
> + u32 rts_rate, u8 macid);
> void rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
> bool short_preamble, bool ampdu_enable,
> - u32 rts_rate);
> + u32 rts_rate, u8 macid);
> void rtl8xxxu_fill_txdesc_v3(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
> bool short_preamble, bool ampdu_enable,
> - u32 rts_rate);
> + u32 rts_rate, u8 macid);
> void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
> u8 arg1, u8 arg2, u8 arg3, u8 arg4, u8 arg5);
> void rtl8723bu_phy_init_antenna_selection(struct rtl8xxxu_priv *priv);
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index d74a3c6452507..c232de1d47173 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -5152,7 +5152,8 @@ void
> rtl8xxxu_fill_txdesc_v1(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
> - bool short_preamble, bool ampdu_enable, u32 rts_rate)
> + bool short_preamble, bool ampdu_enable, u32 rts_rate,
> + u8 macid)
> {
> struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
> struct rtl8xxxu_priv *priv = hw->priv;
> @@ -5224,7 +5225,8 @@ void
> rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
> - bool short_preamble, bool ampdu_enable, u32 rts_rate)
> + bool short_preamble, bool ampdu_enable, u32 rts_rate,
> + u8 macid)
> {
> struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
> struct rtl8xxxu_priv *priv = hw->priv;
> @@ -5248,6 +5250,8 @@ rtl8xxxu_fill_txdesc_v2(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_desc40->pkt_size));
>
> + tx_desc40->txdw1 |= cpu_to_le32(macid << TXDESC40_MACID_SHIFT);
> +
> seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
>
> tx_desc40->txdw4 = cpu_to_le32(rate);
> @@ -5299,7 +5303,8 @@ void
> rtl8xxxu_fill_txdesc_v3(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> struct ieee80211_tx_info *tx_info,
> struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
> - bool short_preamble, bool ampdu_enable, u32 rts_rate)
> + bool short_preamble, bool ampdu_enable, u32 rts_rate,
> + u8 macid)
> {
> struct ieee80211_rate *tx_rate = ieee80211_get_tx_rate(hw, tx_info);
> struct rtl8xxxu_priv *priv = hw->priv;
> @@ -5398,6 +5403,7 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> u16 pktlen = skb->len;
> u16 rate_flag = tx_info->control.rates[0].flags;
> int tx_desc_size = priv->fops->tx_desc_size;
> + u8 macid = 0;
> int ret;
> bool ampdu_enable, sgi = false, short_preamble = false;
>
> @@ -5497,9 +5503,11 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> else
> rts_rate = 0;
>
> + if (vif->type == NL80211_IFTYPE_AP && sta)
> + macid = sta->aid + 1;
>
You should have a function which calculates the macid instead of copying
the calculation everywhere.
> priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
> - ampdu_enable, rts_rate);
> + ampdu_enable, rts_rate, macid);
>
> rtl8xxxu_calc_tx_desc_csum(tx_desc);
>
On 27/03/2023 12:19, Ping-Ke Shih wrote:
>
>
>> -----Original Message-----
>> From: Kalle Valo <[email protected]>
>> Sent: Monday, March 27, 2023 4:42 PM
>> To: Ping-Ke Shih <[email protected]>
>> Cc: Martin Kaistra <[email protected]>; [email protected]; Jes Sorensen
>> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
>> <[email protected]>
>> Subject: Re: [RFC PATCH 07/14] wifi: rtl8xxxu: Add parameter macid to update_rate_mask
>>
>> Ping-Ke Shih <[email protected]> writes:
>>
>>>> -----Original Message-----
>>>> From: Martin Kaistra <[email protected]>
>>>> Sent: Thursday, March 23, 2023 1:19 AM
>>>> To: [email protected]
>>>> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
>>>> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
>>>> <[email protected]>
>>>> Subject: [RFC PATCH 07/14] wifi: rtl8xxxu: Add parameter macid to update_rate_mask
>>>>
>>>> The HW maintains a rate_mask for each connection, referenced by the
>>>> macid. Add a parameter to update_rate_mask and add the macid to the
>>>> h2c call in the gen2 implementation.
>>>>
>>>> Also extend refresh_rate_mask to generate the macid in AP mode from
>>>> sta->aid.
>>>
>>> Firmware can support 32 mac_id (station instance) at most, so it will be a
>>> problem if hostapd assigns aid more than 32. Though I'm not clear how
>>> hostpad assigns the aid, it would be always safe that rtl8xxxu maintains
>>> mac_id by a bitmap in driver.
>>
>> Does rtlw8xxxu set struct wiphy::max_ap_assoc_sta? It would be good to
>> advertise the user space the maximum number of stations.
>>
>
> Thanks for this information, Kalle.
>
> Martin, please add this. I think we can preserve at least one mac_id for
> broadcast/multicast frames. In fact, I'm not absolutely sure we can
> support up to 32 mac_id, so set wiphy::max_ap_assoc_sta = 16 -1 or -2
> would be safer.
>
> Ping-Ke
>
>
Indeed, the RTL8188FU driver has hal_spec->macid_num = 16. I think that's
the maximum for this chip.
RTL8710BU: 16
RTL8188EU: 64
RTL8192EU: 128
RTL8723BU: 128
Am 27.03.23 um 03:39 schrieb Ping-Ke Shih:
>
>
>> -----Original Message-----
>> From: Martin Kaistra <[email protected]>
>> Sent: Thursday, March 23, 2023 1:19 AM
>> To: [email protected]
>> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
>> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
>> <[email protected]>
>> Subject: [RFC PATCH 06/14] wifi: rtl8xxxu: Allow creating interface in AP mode
>>
>> Use the sequence from the vendor driver for setting up the beacon
>> related registers.
>> Also set the MAC address register.
>>
>> Signed-off-by: Martin Kaistra <[email protected]>
>> ---
>> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 31 ++++++++++++++++---
>> .../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h | 2 ++
>> 2 files changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index b233c66a7a5a8..b20ff8bc40870 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -6408,18 +6408,39 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>> int ret;
>> u8 val8;
>>
>> + if (!priv->vif)
>> + priv->vif = vif;
>> + else
>> + return -EOPNOTSUPP;
>> +
>> switch (vif->type) {
>> case NL80211_IFTYPE_STATION:
>> - if (!priv->vif)
>> - priv->vif = vif;
>> - else
>> - return -EOPNOTSUPP;
>> rtl8xxxu_stop_tx_beacon(priv);
>>
>> val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
>> val8 |= BEACON_ATIM | BEACON_FUNCTION_ENABLE |
>> BEACON_DISABLE_TSF_UPDATE;
>> rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>> + ret = 0;
>> + break;
>> + case NL80211_IFTYPE_AP:
>> + rtl8xxxu_write8(priv, REG_BEACON_CTRL,
>> + BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
>> + rtl8xxxu_write8(priv, REG_ATIMWND, 0x0c); /* 12ms */
>> + rtl8xxxu_write16(priv, REG_TSFTR_SYN_OFFSET, 0x7fff); /* ~32ms */
>> + rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, DUAL_TSF_RESET_TSF0);
>> +
>> + /* enable BCN0 function */
>> + rtl8xxxu_write8(priv, REG_BEACON_CTRL,
>> + BEACON_DISABLE_TSF_UPDATE |
>> + BEACON_FUNCTION_ENABLE | BEACON_CTRL_MBSSID |
>> + BEACON_CTRL_TX_BEACON_RPT);
>> +
>> + /* select BCN on port 0 */
>> + val8 = rtl8xxxu_read8(priv, REG_CCK_CHECK);
>> + val8 &= ~BIT_BCN_PORT_SEL;
>> + rtl8xxxu_write8(priv, REG_CCK_CHECK, val8);
>> +
>> ret = 0;
>> break;
>> default:
>> @@ -6427,6 +6448,8 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>> }
>>
>> rtl8xxxu_set_linktype(priv, vif->type);
>> + ether_addr_copy(priv->mac_addr, vif->addr);
>> + rtl8xxxu_set_mac(priv);
>
> rtl8xxxu_set_mac() is already called by rtl8xxxu_init_device(). Is there
> anything unexpected?
If the hostapd config has the option "bssid" set, this will be set as
the MAC address for the interface in AP mode. If the MAC address
registers are not updated, then I don't see any ack frames being send.
The same could probably happen if the MAC address is set via
ip link set dev <wlan_name> address <MAC address>
Maybe the call to rtl8xxxu_set_mac() in rtl8xxxu_init_device() can be
removed, if it is also called here.
>
> While I reviewed first patch, I would like to suggest to call calibration:
> fops->phy_lc_calibrate(priv);
> fops->phy_iq_calibrate(priv);
> I traced rtl8xxxu and saw they present in rtl8xxxu_init_device() and rtl8xxxu
> doesn't implement idle power saving to turn off power of wifi card. Then, I
> think the calibration will be fine if rtl8xxxu only does it once.
>
> So, I would like to know if something I miss.
>
Am 27.03.23 um 15:10 schrieb Bitterblue Smith:
> On 22/03/2023 19:18, Martin Kaistra wrote:
>> This gets called at the start of AP mode operation. Set bssid, beacon
>> interval and send a connect report to the HW.
>>
>
> Hmm, but why send a connect report when you don't have anything
> connected yet?
I tried following the vendor driver here, I don't know what exactly
happens in the firmware.
I can test, though, if there is any difference, if I remove it.
>
>> Signed-off-by: Martin Kaistra <[email protected]>
>> ---
>> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index c152b228606f1..90b98b9dcbd9d 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -4899,6 +4899,20 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>> return;
>> }
>>
>> +static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>> + struct ieee80211_bss_conf *link_conf)
>> +{
>> + struct rtl8xxxu_priv *priv = hw->priv;
>> + struct device *dev = &priv->udev->dev;
>> +
>> + dev_dbg(dev, "Start AP mode\n");
>> + rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
>> + rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
>> + priv->fops->report_connect(priv, 0, true);
>> +
>> + return 0;
>> +}
>> +
>> static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
>> {
>> u32 rtlqueue;
>> @@ -7026,6 +7040,7 @@ static const struct ieee80211_ops rtl8xxxu_ops = {
>> .config = rtl8xxxu_config,
>> .conf_tx = rtl8xxxu_conf_tx,
>> .bss_info_changed = rtl8xxxu_bss_info_changed,
>> + .start_ap = rtl8xxxu_start_ap,
>> .configure_filter = rtl8xxxu_configure_filter,
>> .set_rts_threshold = rtl8xxxu_set_rts_threshold,
>> .start = rtl8xxxu_start,
>
On 27/03/2023 19:08, Martin Kaistra wrote:
> Am 27.03.23 um 15:10 schrieb Bitterblue Smith:
>> On 22/03/2023 19:18, Martin Kaistra wrote:
>>> This gets called at the start of AP mode operation. Set bssid, beacon
>>> interval and send a connect report to the HW.
>>>
>>
>> Hmm, but why send a connect report when you don't have anything
>> connected yet?
>
> I tried following the vendor driver here, I don't know what exactly happens in the firmware.
> I can test, though, if there is any difference, if I remove it.
>
Ah, okay. I was just wondering.
>>
>>> Signed-off-by: Martin Kaistra <[email protected]>
>>> ---
>>> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 15 +++++++++++++++
>>> 1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> index c152b228606f1..90b98b9dcbd9d 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> @@ -4899,6 +4899,20 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>>> return;
>>> }
>>> +static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>>> + struct ieee80211_bss_conf *link_conf)
>>> +{
>>> + struct rtl8xxxu_priv *priv = hw->priv;
>>> + struct device *dev = &priv->udev->dev;
>>> +
>>> + dev_dbg(dev, "Start AP mode\n");
>>> + rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
>>> + rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
>>> + priv->fops->report_connect(priv, 0, true);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
>>> {
>>> u32 rtlqueue;
>>> @@ -7026,6 +7040,7 @@ static const struct ieee80211_ops rtl8xxxu_ops = {
>>> .config = rtl8xxxu_config,
>>> .conf_tx = rtl8xxxu_conf_tx,
>>> .bss_info_changed = rtl8xxxu_bss_info_changed,
>>> + .start_ap = rtl8xxxu_start_ap,
>>> .configure_filter = rtl8xxxu_configure_filter,
>>> .set_rts_threshold = rtl8xxxu_set_rts_threshold,
>>> .start = rtl8xxxu_start,
>>
Am 27.03.23 um 04:06 schrieb Ping-Ke Shih:
>
>
>> -----Original Message-----
>> From: Martin Kaistra <[email protected]>
>> Sent: Thursday, March 23, 2023 1:19 AM
>> To: [email protected]
>> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
>> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
>> <[email protected]>
>> Subject: [RFC PATCH 13/14] wifi: rtl8xxxu: Clean up filter configuration
>>
>> In AP mode, RCR_CHECK_BSSID_MATCH should not be set. Rearrange RCR bits
>> to filter flags to match other realtek drivers and don't set
>> RCR_CHECK_BSSID_BEACON and RCR_CHECK_BSSID_MATCH in AP mode.
>>
>> Signed-off-by: Martin Kaistra <[email protected]>
>> ---
>> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 19 ++++++++++---------
>> 1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 82fbe778fc5ec..b6f811ad01333 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -6597,23 +6597,24 @@ static void rtl8xxxu_configure_filter(struct ieee80211_hw *hw,
>> * FIF_PLCPFAIL not supported?
>> */
>>
>> - if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
>> - rcr &= ~RCR_CHECK_BSSID_BEACON;
>> - else
>> - rcr |= RCR_CHECK_BSSID_BEACON;
>> + if (priv->vif->type != NL80211_IFTYPE_AP) {
>
> I think mac80211 configure filters depends on operating conditions, so it would
> be possible to avoid checking vif->type.
It should be possible to remove the vif->type check from
FIF_BCN_PRBRESP_PROMISC check, but I would still need it to remove the
CHECK_BSSID_MATCH bit in the AP mode case. Otherwise I seem to receive
no data frames.
if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
rcr &= ~(RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH);
else
rcr |= RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH;
if (priv->vif && priv->vif->type == NL80211_IFTYPE_AP)
rcr &= ~RCR_CHECK_BSSID_MATCH;
Another way would be like in the rtw88 driver, where the BIT_CBSSID_DATA
is not set again in the else case, but I am not sure, that is the right way.
>
>> + if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
>> + rcr &= ~(RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH);
>> + else
>> + rcr |= RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH;
>> + } else {
>> + rcr &= ~RCR_CHECK_BSSID_MATCH;
>> + }
>>
>> if (*total_flags & FIF_CONTROL)
>> rcr |= RCR_ACCEPT_CTRL_FRAME;
>> else
>> rcr &= ~RCR_ACCEPT_CTRL_FRAME;
>>
>> - if (*total_flags & FIF_OTHER_BSS) {
>> + if (*total_flags & FIF_OTHER_BSS)
>> rcr |= RCR_ACCEPT_AP;
>> - rcr &= ~RCR_CHECK_BSSID_MATCH;
>> - } else {
>> + else
>> rcr &= ~RCR_ACCEPT_AP;
>> - rcr |= RCR_CHECK_BSSID_MATCH;
>> - }
>>
>> if (*total_flags & FIF_PSPOLL)
>> rcr |= RCR_ACCEPT_PM;
>> --
>> 2.30.2
>
> -----Original Message-----
> From: Bitterblue Smith <[email protected]>
> Sent: Tuesday, March 28, 2023 2:29 AM
> To: Martin Kaistra <[email protected]>; [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Sebastian Andrzej Siewior <[email protected]>
> Subject: Re: [RFC PATCH 01/14] wifi: rtl8xxxu: Add start_ap() callback
>
> On 27/03/2023 19:08, Martin Kaistra wrote:
> > Am 27.03.23 um 15:10 schrieb Bitterblue Smith:
> >> On 22/03/2023 19:18, Martin Kaistra wrote:
> >>> This gets called at the start of AP mode operation. Set bssid, beacon
> >>> interval and send a connect report to the HW.
> >>>
> >>
> >> Hmm, but why send a connect report when you don't have anything
> >> connected yet?
> >
> > I tried following the vendor driver here, I don't know what exactly happens in the firmware.
> > I can test, though, if there is any difference, if I remove it.
> >
> Ah, okay. I was just wondering.
This is to tell firmware we are going to use the entry of mac_id = 0,
please allocate this entry and set it as valid.
>
> >>
> >>> Signed-off-by: Martin Kaistra <[email protected]>
> >>> ---
> >>> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 15 +++++++++++++++
> >>> 1 file changed, 15 insertions(+)
> >>>
> >>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >>> index c152b228606f1..90b98b9dcbd9d 100644
> >>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >>> @@ -4899,6 +4899,20 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> >>> return;
> >>> }
> >>> +static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> >>> + struct ieee80211_bss_conf *link_conf)
> >>> +{
> >>> + struct rtl8xxxu_priv *priv = hw->priv;
> >>> + struct device *dev = &priv->udev->dev;
> >>> +
> >>> + dev_dbg(dev, "Start AP mode\n");
> >>> + rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
> >>> + rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
> >>> + priv->fops->report_connect(priv, 0, true);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> static u32 rtl8xxxu_80211_to_rtl_queue(u32 queue)
> >>> {
> >>> u32 rtlqueue;
> >>> @@ -7026,6 +7040,7 @@ static const struct ieee80211_ops rtl8xxxu_ops = {
> >>> .config = rtl8xxxu_config,
> >>> .conf_tx = rtl8xxxu_conf_tx,
> >>> .bss_info_changed = rtl8xxxu_bss_info_changed,
> >>> + .start_ap = rtl8xxxu_start_ap,
> >>> .configure_filter = rtl8xxxu_configure_filter,
> >>> .set_rts_threshold = rtl8xxxu_set_rts_threshold,
> >>> .start = rtl8xxxu_start,
> >>
>
>
> ------Please consider the environment before printing this e-mail.
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Tuesday, March 28, 2023 10:47 PM
> To: Ping-Ke Shih <[email protected]>; [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Bitterblue Smith
> <[email protected]>; Sebastian Andrzej Siewior <[email protected]>
> Subject: Re: [RFC PATCH 13/14] wifi: rtl8xxxu: Clean up filter configuration
>
> Am 27.03.23 um 04:06 schrieb Ping-Ke Shih:
> >
> >
> >> -----Original Message-----
> >> From: Martin Kaistra <[email protected]>
> >> Sent: Thursday, March 23, 2023 1:19 AM
> >> To: [email protected]
> >> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> >> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> >> <[email protected]>
> >> Subject: [RFC PATCH 13/14] wifi: rtl8xxxu: Clean up filter configuration
> >>
> >> In AP mode, RCR_CHECK_BSSID_MATCH should not be set. Rearrange RCR bits
> >> to filter flags to match other realtek drivers and don't set
> >> RCR_CHECK_BSSID_BEACON and RCR_CHECK_BSSID_MATCH in AP mode.
> >>
> >> Signed-off-by: Martin Kaistra <[email protected]>
> >> ---
> >> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 19 ++++++++++---------
> >> 1 file changed, 10 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> index 82fbe778fc5ec..b6f811ad01333 100644
> >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> @@ -6597,23 +6597,24 @@ static void rtl8xxxu_configure_filter(struct ieee80211_hw *hw,
> >> * FIF_PLCPFAIL not supported?
> >> */
> >>
> >> - if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
> >> - rcr &= ~RCR_CHECK_BSSID_BEACON;
> >> - else
> >> - rcr |= RCR_CHECK_BSSID_BEACON;
> >> + if (priv->vif->type != NL80211_IFTYPE_AP) {
> >
> > I think mac80211 configure filters depends on operating conditions, so it would
> > be possible to avoid checking vif->type.
>
> It should be possible to remove the vif->type check from
> FIF_BCN_PRBRESP_PROMISC check, but I would still need it to remove the
> CHECK_BSSID_MATCH bit in the AP mode case. Otherwise I seem to receive
> no data frames.
>
>
> if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
> rcr &= ~(RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH);
> else
> rcr |= RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH;
>
> if (priv->vif && priv->vif->type == NL80211_IFTYPE_AP)
> rcr &= ~RCR_CHECK_BSSID_MATCH;
>
> Another way would be like in the rtw88 driver, where the BIT_CBSSID_DATA
> is not set again in the else case, but I am not sure, that is the right way.
>
rtl8xxxu support single one vif, so your proposal will be fine.
Without BIT_CBSSID_DATA, driver will receive unnecessary data frames, so
it will increase traffic of bus, but it will be fine for users.
Am 23.03.23 um 18:12 schrieb Bitterblue Smith:
> On 22/03/2023 19:18, Martin Kaistra wrote:
>> This series intends to bring AP mode support to the rtl8xxxu driver,
>> more specifically for the 8188f, because this is the HW I have.
>> The work is based on the vendor driver as I do not have access to
>> datasheets.
>>
>> This is an RFC, so that there can be a discussion first before
>> potentially implementing support for the other chips in this driver, if
>> required.
>>
>
> Hi!
>
> I ran into some problems while testing this.
>
> First, a null pointer dereference in rtl8xxxu_config_filter() when
> turning on the AP. priv->vif was NULL:
>
> if (priv->vif->type != NL80211_IFTYPE_AP) {
>
> I changed it like this:
>
> if (priv->vif && priv->vif->type != NL80211_IFTYPE_AP) {
>
> Then I was able to turn on the AP and connect my phone to it. However,
> the system froze after a few seconds. I had `journalctl --follow`
> running. The last thing printed before the system froze was the DHCP
> stuff (discover, offer, request, ack). The phone said it was connected,
> but speedtest.net didn't have time to load before the freeze.
Hi
I could reproduce a frozen system with my hostapd setup, though it
doesn't happen reliably and I don't have an error message when it happens.
What I can see on the other hand, are WARNING messages which happen
sometimes in include/net/mac80211.h:2936 (ieee80211_get_tx_rate()).
This might be unrelated, I am not sure.
Is this function even supposed to work in combination with
HAS_RATE_CONTROL set? Also, why are we putting rate into txdesc for all
packets (ie. also when USE_DRIVER_RATE is not set) when the firmware
sets the rate based on the rate_mask?
>
> My system is a laptop with RTL8822CE internal wifi card connected to my
> ISP's router. The connections are managed by NetworkManager 1.42.4-1,
> which uses wpa_supplicant 2:2.10-8 and dnsmasq 2.89-1. The operating
> system is Arch Linux running kernel 6.2.5-arch1-1.
>
> I used Plasma's NetworkManager applet to create a new "Wi-Fi (shared)"
> connection with mode "Access Point", band 2.4 GHz, channel 1, no
> encryption, and "IPv4 is required for this connection".
>
>
> Unrelated to anything, just out of curiosity, what brand/model is your
> RTL8188FU?
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Wednesday, April 5, 2023 11:31 PM
> To: Bitterblue Smith <[email protected]>
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Sebastian Andrzej Siewior <[email protected]>; [email protected]
> Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f
>
> Am 23.03.23 um 18:12 schrieb Bitterblue Smith:
> > On 22/03/2023 19:18, Martin Kaistra wrote:
> > Then I was able to turn on the AP and connect my phone to it. However,
> > the system froze after a few seconds. I had `journalctl --follow`
> > running. The last thing printed before the system froze was the DHCP
> > stuff (discover, offer, request, ack). The phone said it was connected,
> > but speedtest.net didn't have time to load before the freeze.
>
> Hi
>
> I could reproduce a frozen system with my hostapd setup, though it
> doesn't happen reliably and I don't have an error message when it happens.
>
Using netcat would help to capture useful messages when system gets frozen.
Ping-Ke
Am 06.04.23 um 02:42 schrieb Ping-Ke Shih:
>
>
>> -----Original Message-----
>> From: Martin Kaistra <[email protected]>
>> Sent: Wednesday, April 5, 2023 11:31 PM
>> To: Bitterblue Smith <[email protected]>
>> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
>> <[email protected]>; Sebastian Andrzej Siewior <[email protected]>; [email protected]
>> Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f
>>
>> Am 23.03.23 um 18:12 schrieb Bitterblue Smith:
>>> On 22/03/2023 19:18, Martin Kaistra wrote:
>>> Then I was able to turn on the AP and connect my phone to it. However,
>>> the system froze after a few seconds. I had `journalctl --follow`
>>> running. The last thing printed before the system froze was the DHCP
>>> stuff (discover, offer, request, ack). The phone said it was connected,
>>> but speedtest.net didn't have time to load before the freeze.
>>
>> Hi
>>
>> I could reproduce a frozen system with my hostapd setup, though it
>> doesn't happen reliably and I don't have an error message when it happens.
>>
>
> Using netcat would help to capture useful messages when system gets frozen.
>
> Ping-Ke
>
Thanks. I got a trace by using netconsole and netcat. It is a NULL
pointer dereference in rtl8xxxu_fill_txdesc_v2():
if (rate_flags & IEEE80211_TX_RC_MCS &&
!ieee80211_is_mgmt(hdr->frame_control))
rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0;
else
rate = tx_rate->hw_value; <-- error happens here
if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX)
dev_info(dev, "%s: TX rate: %d, pkt size %u\n",
This happens when ieee80211_get_tx_rate() hits the WARN_ON_ONCE and
maybe also when c->control.rates[0].idx has another invalid value.
See my previous comments about HAS_RATE_CONTROL.
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Wednesday, April 5, 2023 11:31 PM
> To: Bitterblue Smith <[email protected]>
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Sebastian Andrzej Siewior <[email protected]>; [email protected]
> Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f
>
> Am 23.03.23 um 18:12 schrieb Bitterblue Smith:
> > On 22/03/2023 19:18, Martin Kaistra wrote:
> >> This series intends to bring AP mode support to the rtl8xxxu driver,
> >> more specifically for the 8188f, because this is the HW I have.
> >> The work is based on the vendor driver as I do not have access to
> >> datasheets.
> >>
> >> This is an RFC, so that there can be a discussion first before
> >> potentially implementing support for the other chips in this driver, if
> >> required.
> >>
> >
> > Hi!
> >
> > I ran into some problems while testing this.
> >
> > First, a null pointer dereference in rtl8xxxu_config_filter() when
> > turning on the AP. priv->vif was NULL:
> >
> > if (priv->vif->type != NL80211_IFTYPE_AP) {
> >
> > I changed it like this:
> >
> > if (priv->vif && priv->vif->type != NL80211_IFTYPE_AP) {
> >
> > Then I was able to turn on the AP and connect my phone to it. However,
> > the system froze after a few seconds. I had `journalctl --follow`
> > running. The last thing printed before the system froze was the DHCP
> > stuff (discover, offer, request, ack). The phone said it was connected,
> > but speedtest.net didn't have time to load before the freeze.
>
> Hi
>
> I could reproduce a frozen system with my hostapd setup, though it
> doesn't happen reliably and I don't have an error message when it happens.
>
> What I can see on the other hand, are WARNING messages which happen
> sometimes in include/net/mac80211.h:2936 (ieee80211_get_tx_rate()).
> This might be unrelated, I am not sure.
>
> Is this function even supposed to work in combination with
> HAS_RATE_CONTROL set?
I'm not familiar with rate control of mac80211, so I didn't have comment
about rate control and HAS_RATE_CONTROL before.
Simply checking rate_control_get_rate() that is to fill info->control.rates[],
but it doesn't do it if HAS_RATE_CONTROL is set:
if (ieee80211_hw_check(&sdata->local->hw, HAS_RATE_CONTROL))
return;
So, I think it will not work with HAS_RATE_CONTROL set. Fortunately,
rtl8xxxu only works on 2 GHz band, and only management frames use
info->control.rates[] to decide TX rate, so always TX management frames
with 1M CCK rate (hw_rate = 0) is fine.
> Also, why are we putting rate into txdesc for all
> packets (ie. also when USE_DRIVER_RATE is not set) when the firmware
> sets the rate based on the rate_mask?
Conceptually, if USE_DRIVER_RATE is set, rate info of txdesc is adopted.
Otherwise, rate register controlled by firmware is adopted. That means
if USE_DRIVER_RATE isn't set, rate info of txdesc is ignored.
rtl8xxxu_update_rate_mask() is to tell firmware the all supported rate mask,
and firmware choose proper TX and retry rate, and set to register.
Ping-Ke
On 06/04/2023 18:09, Martin Kaistra wrote:
> Am 06.04.23 um 02:42 schrieb Ping-Ke Shih:
>>
>>
>>> -----Original Message-----
>>> From: Martin Kaistra <[email protected]>
>>> Sent: Wednesday, April 5, 2023 11:31 PM
>>> To: Bitterblue Smith <[email protected]>
>>> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
>>> <[email protected]>; Sebastian Andrzej Siewior <[email protected]>; [email protected]
>>> Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f
>>>
>>> Am 23.03.23 um 18:12 schrieb Bitterblue Smith:
>>>> On 22/03/2023 19:18, Martin Kaistra wrote:
>>>> Then I was able to turn on the AP and connect my phone to it. However,
>>>> the system froze after a few seconds. I had `journalctl --follow`
>>>> running. The last thing printed before the system froze was the DHCP
>>>> stuff (discover, offer, request, ack). The phone said it was connected,
>>>> but speedtest.net didn't have time to load before the freeze.
>>>
>>> Hi
>>>
>>> I could reproduce a frozen system with my hostapd setup, though it
>>> doesn't happen reliably and I don't have an error message when it happens.
>>>
>>
>> Using netcat would help to capture useful messages when system gets frozen.
>>
>> Ping-Ke
>>
>
> Thanks. I got a trace by using netconsole and netcat. It is a NULL pointer dereference in rtl8xxxu_fill_txdesc_v2():
>
>
> if (rate_flags & IEEE80211_TX_RC_MCS &&
> !ieee80211_is_mgmt(hdr->frame_control))
> rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0;
> else
> rate = tx_rate->hw_value; <-- error happens here
>
> if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX)
> dev_info(dev, "%s: TX rate: %d, pkt size %u\n",
>
> This happens when ieee80211_get_tx_rate() hits the WARN_ON_ONCE and maybe also when c->control.rates[0].idx has another invalid value.
> See my previous comments about HAS_RATE_CONTROL.
Good job finding the problem!
ieee80211_get_tx_rate() is used since the initial commit, so only Jes
may know why. I assume we only ever need to use the rate from mac80211
for frame injection (IEEE80211_TX_CTRL_RATE_INJECT, currently ignored).
If c->control.rates[0].idx is negative, is c->control.rates[0].flags
also unusable?
ieee80211_get_rts_cts_rate() probably causes the same problem.
Am 09.04.23 um 14:41 schrieb Bitterblue Smith:
> On 06/04/2023 18:09, Martin Kaistra wrote:
>> Am 06.04.23 um 02:42 schrieb Ping-Ke Shih:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Martin Kaistra <[email protected]>
>>>> Sent: Wednesday, April 5, 2023 11:31 PM
>>>> To: Bitterblue Smith <[email protected]>
>>>> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
>>>> <[email protected]>; Sebastian Andrzej Siewior <[email protected]>; [email protected]
>>>> Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f
>>>>
>>>> Am 23.03.23 um 18:12 schrieb Bitterblue Smith:
>>>>> On 22/03/2023 19:18, Martin Kaistra wrote:
>>>>> Then I was able to turn on the AP and connect my phone to it. However,
>>>>> the system froze after a few seconds. I had `journalctl --follow`
>>>>> running. The last thing printed before the system froze was the DHCP
>>>>> stuff (discover, offer, request, ack). The phone said it was connected,
>>>>> but speedtest.net didn't have time to load before the freeze.
>>>>
>>>> Hi
>>>>
>>>> I could reproduce a frozen system with my hostapd setup, though it
>>>> doesn't happen reliably and I don't have an error message when it happens.
>>>>
>>>
>>> Using netcat would help to capture useful messages when system gets frozen.
>>>
>>> Ping-Ke
>>>
>>
>> Thanks. I got a trace by using netconsole and netcat. It is a NULL pointer dereference in rtl8xxxu_fill_txdesc_v2():
>>
>>
>> if (rate_flags & IEEE80211_TX_RC_MCS &&
>> !ieee80211_is_mgmt(hdr->frame_control))
>> rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0;
>> else
>> rate = tx_rate->hw_value; <-- error happens here
>>
>> if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX)
>> dev_info(dev, "%s: TX rate: %d, pkt size %u\n",
>>
>> This happens when ieee80211_get_tx_rate() hits the WARN_ON_ONCE and maybe also when c->control.rates[0].idx has another invalid value.
>> See my previous comments about HAS_RATE_CONTROL.
>
> Good job finding the problem!
>
> ieee80211_get_tx_rate() is used since the initial commit, so only Jes
> may know why. I assume we only ever need to use the rate from mac80211
> for frame injection (IEEE80211_TX_CTRL_RATE_INJECT, currently ignored).
>
> If c->control.rates[0].idx is negative, is c->control.rates[0].flags
> also unusable?
control.rates[0].flags seems to be 0 normally in all my tests when
HAS_RATE_CONTROL is enabled, and when control.rates[0].idx is negative,
I see some random values which do not look like real flags.
>
> ieee80211_get_rts_cts_rate() probably causes the same problem.
Yes, I agree.
How should proceed? I have a v2 of this patch series ready, do I need to
add a patch to remove the calls to ieee80211_get_tx_rate() and just set
rate in txdesc to 0 or should we handle this separately?
On 12/04/2023 13:02, Martin Kaistra wrote:
> Am 09.04.23 um 14:41 schrieb Bitterblue Smith:
>> On 06/04/2023 18:09, Martin Kaistra wrote:
>>> Am 06.04.23 um 02:42 schrieb Ping-Ke Shih:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Martin Kaistra <[email protected]>
>>>>> Sent: Wednesday, April 5, 2023 11:31 PM
>>>>> To: Bitterblue Smith <[email protected]>
>>>>> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
>>>>> <[email protected]>; Sebastian Andrzej Siewior <[email protected]>; [email protected]
>>>>> Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f
>>>>>
>>>>> Am 23.03.23 um 18:12 schrieb Bitterblue Smith:
>>>>>> On 22/03/2023 19:18, Martin Kaistra wrote:
>>>>>> Then I was able to turn on the AP and connect my phone to it. However,
>>>>>> the system froze after a few seconds. I had `journalctl --follow`
>>>>>> running. The last thing printed before the system froze was the DHCP
>>>>>> stuff (discover, offer, request, ack). The phone said it was connected,
>>>>>> but speedtest.net didn't have time to load before the freeze.
>>>>>
>>>>> Hi
>>>>>
>>>>> I could reproduce a frozen system with my hostapd setup, though it
>>>>> doesn't happen reliably and I don't have an error message when it happens.
>>>>>
>>>>
>>>> Using netcat would help to capture useful messages when system gets frozen.
>>>>
>>>> Ping-Ke
>>>>
>>>
>>> Thanks. I got a trace by using netconsole and netcat. It is a NULL pointer dereference in rtl8xxxu_fill_txdesc_v2():
>>>
>>>
>>> if (rate_flags & IEEE80211_TX_RC_MCS &&
>>> !ieee80211_is_mgmt(hdr->frame_control))
>>> rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0;
>>> else
>>> rate = tx_rate->hw_value; <-- error happens here
>>>
>>> if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX)
>>> dev_info(dev, "%s: TX rate: %d, pkt size %u\n",
>>>
>>> This happens when ieee80211_get_tx_rate() hits the WARN_ON_ONCE and maybe also when c->control.rates[0].idx has another invalid value.
>>> See my previous comments about HAS_RATE_CONTROL.
>>
>> Good job finding the problem!
>>
>> ieee80211_get_tx_rate() is used since the initial commit, so only Jes
>> may know why. I assume we only ever need to use the rate from mac80211
>> for frame injection (IEEE80211_TX_CTRL_RATE_INJECT, currently ignored).
>>
>> If c->control.rates[0].idx is negative, is c->control.rates[0].flags
>> also unusable?
>
> control.rates[0].flags seems to be 0 normally in all my tests when HAS_RATE_CONTROL is enabled, and when control.rates[0].idx is negative, I see some random values which do not look like real flags.
>
>>
>> ieee80211_get_rts_cts_rate() probably causes the same problem.
>
> Yes, I agree.
>
> How should proceed? I have a v2 of this patch series ready, do I need to add a patch to remove the calls to ieee80211_get_tx_rate() and just set rate in txdesc to 0 or should we handle this separately?
Adding a patch to the series sounds good to me. Remove the calls
to ieee80211_get_tx_rate() and ieee80211_get_rts_cts_rate(), remove
tx_info->control.rates[0].flags, send management and control frames
with 1M.
About ieee80211_get_rts_cts_rate(), we can go back to sending RTS
with the 24M rate, like the vendor drivers do. It's unclear why
commit a748a11038f8 ("rtl8xxxu: Obtain RTS rates from mac80211")
changed this.