[PATCH 00/20] wifi: rtl8xxxu: Add concurrent mode for 8188f

[PATCH 00/20] wifi: rtl8xxxu: Add concurrent mode for 8188f

[Martin Kaistra]

This series adds the possibility to use two virtual interfaces on the
same channel. Supported combinations are STA+STA and STA+AP. The
conversion of the driver to support multiple interfaces is split into
individual patches to hopefully make it easier to understand what is
going on.

Thanks,
  Martin

Martin Kaistra (20):
  wifi: rtl8xxxu: remove assignment of priv->vif in
    rtl8xxxu_bss_info_changed()
  wifi: rtl8xxxu: prepare supporting two virtual interfaces
  wifi: rtl8xxxu: support setting linktype for both interfaces
  wifi: rtl8xxxu: 8188e: convert usage of priv->vif to priv->vifs[0]
  wifi: rtl8xxxu: support setting mac address register for both
    interfaces
  wifi: rtl8xxxu: extend wifi connected check to both interfaces
  wifi: rtl8xxxu: extend check for matching bssid to both interfaces
  wifi: rtl8xxxu: support setting bssid register for multiple interfaces
  wifi: rtl8xxxu: support multiple interfaces in set_aifs()
  wifi: rtl8xxxu: support multiple interfaces in
    update_beacon_work_callback()
  wifi: rtl8xxxu: support multiple interfaces in configure_filter()
  wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()
  wifi: rtl8xxxu: support multiple interfaces in
    {add,remove}_interface()
  wifi: rtl8xxxu: support multiple interfaces in bss_info_changed()
  wifi: rtl8xxxu: support multiple interface in start_ap()
  wifi: rtl8xxxu: support multiple interfaces in get_macid()
  wifi: rtl8xxxu: remove obsolete priv->vif
  wifi: rtl8xxxu: add hw crypto support for AP mode
  wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
  wifi: rtl8xxxu: declare concurrent mode support for 8188f

 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  16 +-
 .../realtek/rtl8xxxu/rtl8xxxu_8188e.c         |   2 +-
 .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |   1 +
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 361 +++++++++++++-----
 4 files changed, 283 insertions(+), 97 deletions(-)

-- 
2.39.2

[PATCH 01/20] wifi: rtl8xxxu: remove assignment of priv->vif in rtl8xxxu_bss_info_changed()

[Martin Kaistra]

priv->vif gets already set in rtl8xxxu_add_interface, there is no need
to set it also in rtl8xxxu_bss_info_changed().

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 43ee7592bc6e1..2dbed43ef22a5 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5004,7 +5004,6 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 			rtl8xxxu_update_ra_report(rarpt, highest_rate, sgi, bw);
 
-			priv->vif = vif;
 			priv->rssi_level = RTL8XXXU_RATR_STA_INIT;
 
 			priv->fops->update_rate_mask(priv, ramask, 0, sgi,
-- 
2.39.2

[PATCH 02/20] wifi: rtl8xxxu: prepare supporting two virtual interfaces

[Martin Kaistra]

To prepare for concurrent mode, add an array ("vifs") to rtl8xxxu_priv
to keep track of both interfaces.

Keep the old priv->vif as long there are still users of it and let
priv->vifs[0] point to the same location.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h      |  2 ++
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 10 +++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 4695fb4e2d2db..b63fe084de92b 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1897,6 +1897,8 @@ struct rtl8xxxu_priv {
 	 * is supported and no iface_combinations are provided.
 	 */
 	struct ieee80211_vif *vif;
+
+	struct ieee80211_vif *vifs[2];
 	struct delayed_work ra_watchdog;
 	struct work_struct c2hcmd_work;
 	struct sk_buff_head c2hcmd_queue;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 2dbed43ef22a5..5b7c20970a973 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6569,10 +6569,12 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 	int ret;
 	u8 val8;
 
-	if (!priv->vif)
+	if (!priv->vif) {
 		priv->vif = vif;
-	else
+		priv->vifs[0] = vif;
+	} else {
 		return -EOPNOTSUPP;
+	}
 
 	switch (vif->type) {
 	case NL80211_IFTYPE_STATION:
@@ -6622,8 +6624,10 @@ static void rtl8xxxu_remove_interface(struct ieee80211_hw *hw,
 
 	dev_dbg(&priv->udev->dev, "%s\n", __func__);
 
-	if (priv->vif)
+	if (priv->vif) {
 		priv->vif = NULL;
+		priv->vifs[0] = NULL;
+	}
 }
 
 static int rtl8xxxu_config(struct ieee80211_hw *hw, u32 changed)
-- 
2.39.2

[PATCH 03/20] wifi: rtl8xxxu: support setting linktype for both interfaces

[Martin Kaistra]

To prepare for concurrent mode, enhance the set_linktype function to be
able to set the linktype in the MSR register for both hardware ports.

Until the users of set_linktype can handle multiple interfaces, use
port_num = 0.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 37 +++++++++++--------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 5b7c20970a973..305d6dd585dfa 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1633,33 +1633,41 @@ rtl8xxxu_gen1_set_tx_power(struct rtl8xxxu_priv *priv, int channel, bool ht40)
 }
 
 static void rtl8xxxu_set_linktype(struct rtl8xxxu_priv *priv,
-				  enum nl80211_iftype linktype)
+				  enum nl80211_iftype linktype, int port_num)
 {
-	u8 val8;
-
-	val8 = rtl8xxxu_read8(priv, REG_MSR);
-	val8 &= ~MSR_LINKTYPE_MASK;
+	u8 val8, type;
 
 	switch (linktype) {
 	case NL80211_IFTYPE_UNSPECIFIED:
-		val8 |= MSR_LINKTYPE_NONE;
+		type = MSR_LINKTYPE_NONE;
 		break;
 	case NL80211_IFTYPE_ADHOC:
-		val8 |= MSR_LINKTYPE_ADHOC;
+		type = MSR_LINKTYPE_ADHOC;
 		break;
 	case NL80211_IFTYPE_STATION:
-		val8 |= MSR_LINKTYPE_STATION;
+		type = MSR_LINKTYPE_STATION;
 		break;
 	case NL80211_IFTYPE_AP:
-		val8 |= MSR_LINKTYPE_AP;
+		type = MSR_LINKTYPE_AP;
 		break;
 	default:
-		goto out;
+		return;
+	}
+
+	switch (port_num) {
+	case 0:
+		val8 = rtl8xxxu_read8(priv, REG_MSR) & 0x0c;
+		val8 |= type;
+		break;
+	case 1:
+		val8 = rtl8xxxu_read8(priv, REG_MSR) & 0x03;
+		val8 |= type << 2;
+		break;
+	default:
+		return;
 	}
 
 	rtl8xxxu_write8(priv, REG_MSR, val8);
-out:
-	return;
 }
 
 static void
@@ -4236,7 +4244,6 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
 	}
 
 	rtl8xxxu_set_mac(priv);
-	rtl8xxxu_set_linktype(priv, NL80211_IFTYPE_STATION);
 
 	/*
 	 * Configure initial WMAC settings
@@ -4964,7 +4971,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	if (changed & BSS_CHANGED_ASSOC) {
 		dev_dbg(dev, "Changed ASSOC: %i!\n", vif->cfg.assoc);
 
-		rtl8xxxu_set_linktype(priv, vif->type);
+		rtl8xxxu_set_linktype(priv, vif->type, 0);
 
 		if (vif->cfg.assoc) {
 			u32 ramask;
@@ -6610,7 +6617,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 		ret = -EOPNOTSUPP;
 	}
 
-	rtl8xxxu_set_linktype(priv, vif->type);
+	rtl8xxxu_set_linktype(priv, vif->type, 0);
 	ether_addr_copy(priv->mac_addr, vif->addr);
 	rtl8xxxu_set_mac(priv);
 
-- 
2.39.2

[PATCH 04/20] wifi: rtl8xxxu: 8188e: convert usage of priv->vif to priv->vifs[0]

[Martin Kaistra]

The driver currently does not support AP or concurrent mode for 8188e,
so just use priv->vifs[0] instead of priv->vif for now.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
index 6d0f975f891b7..cbeac9386ae51 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
@@ -1699,7 +1699,7 @@ void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *s
 	/* We only use macid 0, so only the first item is relevant.
 	 * AP mode will use more of them if it's ever implemented.
 	 */
-	if (!priv->vif || priv->vif->type == NL80211_IFTYPE_STATION)
+	if (!priv->vifs[0] || priv->vifs[0]->type == NL80211_IFTYPE_STATION)
 		items = 1;
 
 	for (macid = 0; macid < items; macid++) {
-- 
2.39.2

[PATCH 05/20] wifi: rtl8xxxu: support setting mac address register for both interfaces

[Martin Kaistra]

To prepare for concurrent mode, enhance rtl8xxxu_set_mac() to write the
mac address of the respective interface to REG_MACID or REG_MACID1.

Remove the call to rtl8xxxu_set_mac() from the init function as we set
it in rtl8xxxu_add_interface() later anyway.

Until rtl8xxxu_add_interface() can handle both interfaces, call
rtl8xxxu_set_mac() with port_num = 0.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 305d6dd585dfa..c2ea8e92cd637 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3580,15 +3580,25 @@ void rtl8723a_phy_lc_calibrate(struct rtl8xxxu_priv *priv)
 		rtl8xxxu_write8(priv, REG_TXPAUSE, 0x00);
 }
 
-static int rtl8xxxu_set_mac(struct rtl8xxxu_priv *priv)
+static int rtl8xxxu_set_mac(struct rtl8xxxu_priv *priv, int port_num)
 {
 	int i;
 	u16 reg;
 
-	reg = REG_MACID;
+	switch (port_num) {
+	case 0:
+		reg = REG_MACID;
+		break;
+	case 1:
+		reg = REG_MACID1;
+		break;
+	default:
+		WARN_ONCE("%s: invalid port_num\n", __func__);
+		return -EINVAL;
+	}
 
 	for (i = 0; i < ETH_ALEN; i++)
-		rtl8xxxu_write8(priv, reg + i, priv->mac_addr[i]);
+		rtl8xxxu_write8(priv, reg + i, priv->vifs[port_num]->addr[i]);
 
 	return 0;
 }
@@ -4243,8 +4253,6 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
 		rtl8xxxu_write32(priv, REG_HIMR, 0xffffffff);
 	}
 
-	rtl8xxxu_set_mac(priv);
-
 	/*
 	 * Configure initial WMAC settings
 	 */
@@ -6619,7 +6627,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 
 	rtl8xxxu_set_linktype(priv, vif->type, 0);
 	ether_addr_copy(priv->mac_addr, vif->addr);
-	rtl8xxxu_set_mac(priv);
+	rtl8xxxu_set_mac(priv, 0);
 
 	return ret;
 }
-- 
2.39.2

[PATCH 06/20] wifi: rtl8xxxu: extend wifi connected check to both interfaces

[Martin Kaistra]

There are multiple places in the code where the current connection
status of wifi is checked. The driver will support two interfaces soon
and either one of them (or both) could be connected.

Convert all uses of (vif && vif->cfg.assoc) to a new helper
function rtl8xxxu_is_assoc() which checks both interfaces.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 20 +++++++++----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c2ea8e92cd637..fd6b6e2eba038 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6043,18 +6043,20 @@ void rtl8723bu_update_bt_link_info(struct rtl8xxxu_priv *priv, u8 bt_info)
 		btcoex->bt_busy = false;
 }
 
+static inline bool rtl8xxxu_is_assoc(struct rtl8xxxu_priv *priv)
+{
+	return (priv->vifs[0] && priv->vifs[0]->cfg.assoc) ||
+	       (priv->vifs[1] && priv->vifs[1]->cfg.assoc);
+}
+
 static
 void rtl8723bu_handle_bt_inquiry(struct rtl8xxxu_priv *priv)
 {
-	struct ieee80211_vif *vif;
 	struct rtl8xxxu_btcoex *btcoex;
-	bool wifi_connected;
 
-	vif = priv->vif;
 	btcoex = &priv->bt_coex;
-	wifi_connected = (vif && vif->cfg.assoc);
 
-	if (!wifi_connected) {
+	if (!rtl8xxxu_is_assoc(priv)) {
 		rtl8723bu_set_ps_tdma(priv, 0x8, 0x0, 0x0, 0x0, 0x0);
 		rtl8723bu_set_coex_with_type(priv, 0);
 	} else if (btcoex->has_sco || btcoex->has_hid || btcoex->has_a2dp) {
@@ -6072,15 +6074,11 @@ void rtl8723bu_handle_bt_inquiry(struct rtl8xxxu_priv *priv)
 static
 void rtl8723bu_handle_bt_info(struct rtl8xxxu_priv *priv)
 {
-	struct ieee80211_vif *vif;
 	struct rtl8xxxu_btcoex *btcoex;
-	bool wifi_connected;
 
-	vif = priv->vif;
 	btcoex = &priv->bt_coex;
-	wifi_connected = (vif && vif->cfg.assoc);
 
-	if (wifi_connected) {
+	if (rtl8xxxu_is_assoc(priv)) {
 		u32 val32 = 0;
 		u32 high_prio_tx = 0, high_prio_rx = 0;
 
@@ -7103,7 +7101,7 @@ static void rtl8xxxu_track_cfo(struct rtl8xxxu_priv *priv)
 	int cfo_khz_a, cfo_khz_b, cfo_average;
 	int crystal_cap;
 
-	if (!priv->vif || !priv->vif->cfg.assoc) {
+	if (!rtl8xxxu_is_assoc(priv)) {
 		/* Reset */
 		cfo->adjust = true;
 
-- 
2.39.2

[PATCH 07/20] wifi: rtl8xxxu: extend check for matching bssid to both interfaces

[Martin Kaistra]

The driver will support two interfaces soon, which both can be in
station mode, so extend the check, whether cfo information should be
parsed, to cover both interfaces.

For better code readability put the lines with priv->vifs[port_num] in a
separate function.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 22 ++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index fd6b6e2eba038..c3039049e9f5b 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5706,6 +5706,16 @@ static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work)
 	rtl8xxxu_send_beacon_frame(hw, vif);
 }
 
+static inline bool rtl8xxxu_is_packet_match_bssid(struct rtl8xxxu_priv *priv,
+						  struct ieee80211_hdr *hdr,
+						  int port_num)
+{
+	return priv->vifs[port_num] &&
+		priv->vifs[port_num]->type == NL80211_IFTYPE_STATION &&
+		priv->vifs[port_num]->cfg.assoc &&
+		ether_addr_equal(priv->vifs[port_num]->bss_conf.bssid, hdr->addr2);
+}
+
 void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
 				 struct ieee80211_rx_status *rx_status,
 				 struct rtl8723au_phy_stats *phy_stats,
@@ -5722,12 +5732,10 @@ void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
 		rx_status->signal = priv->fops->cck_rssi(priv, phy_stats);
 	} else {
 		bool parse_cfo = priv->fops->set_crystal_cap &&
-				 priv->vif &&
-				 priv->vif->type == NL80211_IFTYPE_STATION &&
-				 priv->vif->cfg.assoc &&
 				 !crc_icv_err &&
 				 !ieee80211_is_ctl(hdr->frame_control) &&
-				 ether_addr_equal(priv->vif->bss_conf.bssid, hdr->addr2);
+				 (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) ||
+				  rtl8xxxu_is_packet_match_bssid(priv, hdr, 1));
 
 		if (parse_cfo) {
 			priv->cfo_tracking.cfo_tail[0] = phy_stats->path_cfotail[0];
@@ -5762,12 +5770,10 @@ static void jaguar2_rx_parse_phystats_type1(struct rtl8xxxu_priv *priv,
 					    bool crc_icv_err)
 {
 	bool parse_cfo = priv->fops->set_crystal_cap &&
-			 priv->vif &&
-			 priv->vif->type == NL80211_IFTYPE_STATION &&
-			 priv->vif->cfg.assoc &&
 			 !crc_icv_err &&
 			 !ieee80211_is_ctl(hdr->frame_control) &&
-			 ether_addr_equal(priv->vif->bss_conf.bssid, hdr->addr2);
+			 (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) ||
+			  rtl8xxxu_is_packet_match_bssid(priv, hdr, 1));
 	u8 pwdb_max = 0;
 	int rx_path;
 
-- 
2.39.2

[PATCH 08/20] wifi: rtl8xxxu: support setting bssid register for multiple interfaces

[Martin Kaistra]

To prepare for concurrent mode, enhance rtl8xxxu_set_bssid() to write the
BSSID of the respective interface to REG_BSSID or REG_BSSID1.

Like done with rtl8xxxu_set_mac(), call rtl8xxxu_set_bssid() with
port_num = 0, until the callers also support multiple interfaces.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c3039049e9f5b..2b546bce01237 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3603,14 +3603,24 @@ static int rtl8xxxu_set_mac(struct rtl8xxxu_priv *priv, int port_num)
 	return 0;
 }
 
-static int rtl8xxxu_set_bssid(struct rtl8xxxu_priv *priv, const u8 *bssid)
+static int rtl8xxxu_set_bssid(struct rtl8xxxu_priv *priv, const u8 *bssid, int port_num)
 {
 	int i;
 	u16 reg;
 
 	dev_dbg(&priv->udev->dev, "%s: (%pM)\n", __func__, bssid);
 
-	reg = REG_BSSID;
+	switch (port_num) {
+	case 0:
+		reg = REG_BSSID;
+		break;
+	case 1:
+		reg = REG_BSSID1;
+		break;
+	default:
+		WARN_ONCE("%s: invalid port_num\n", __func__);
+		return -EINVAL;
+	}
 
 	for (i = 0; i < ETH_ALEN; i++)
 		rtl8xxxu_write8(priv, reg + i, bssid[i]);
@@ -5068,7 +5078,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 	if (changed & BSS_CHANGED_BSSID) {
 		dev_dbg(dev, "Changed BSSID!\n");
-		rtl8xxxu_set_bssid(priv, bss_conf->bssid);
+		rtl8xxxu_set_bssid(priv, bss_conf->bssid, 0);
 	}
 
 	if (changed & BSS_CHANGED_BASIC_RATES) {
@@ -5097,7 +5107,7 @@ static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	struct device *dev = &priv->udev->dev;
 
 	dev_dbg(dev, "Start AP mode\n");
-	rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
+	rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid, 0);
 	rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
 	priv->fops->report_connect(priv, RTL8XXXU_BC_MC_MACID, 0, true);
 
-- 
2.39.2

[PATCH 09/20] wifi: rtl8xxxu: support multiple interfaces in set_aifs()

[Martin Kaistra]

In concurrent mode supported by this driver, both interfaces will use
the same channel and same wireless mode.
It is therefore possible to get the wireless mode by checking the first
connected interface.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 2b546bce01237..827e715f0e585 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4913,14 +4913,18 @@ static void rtl8xxxu_set_aifs(struct rtl8xxxu_priv *priv, u8 slot_time)
 	u8 aifs, aifsn, sifs;
 	int i;
 
-	if (priv->vif) {
-		struct ieee80211_sta *sta;
+	for (i = 0; i < ARRAY_SIZE(priv->vifs); i++) {
+		if (priv->vifs[i]) {
+			struct ieee80211_sta *sta;
 
-		rcu_read_lock();
-		sta = ieee80211_find_sta(priv->vif, priv->vif->bss_conf.bssid);
-		if (sta)
-			wireless_mode = rtl8xxxu_wireless_mode(priv->hw, sta);
-		rcu_read_unlock();
+			rcu_read_lock();
+			sta = ieee80211_find_sta(priv->vifs[i], priv->vifs[i]->bss_conf.bssid);
+			if (sta)
+				wireless_mode = rtl8xxxu_wireless_mode(priv->hw, sta);
+			rcu_read_unlock();
+		}
+		if (wireless_mode)
+			break;
 	}
 
 	if (priv->hw->conf.chandef.chan->band == NL80211_BAND_5GHZ ||
-- 
2.39.2

[PATCH 10/20] wifi: rtl8xxxu: support multiple interfaces in update_beacon_work_callback()

[Martin Kaistra]

As we only want to support AP mode/sending beacons on port 0, it is
enough to replace priv->vif with priv->vifs[0].

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 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 827e715f0e585..6683ff0f4d8c7 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5710,7 +5710,7 @@ 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;
+	struct ieee80211_vif *vif = priv->vifs[0];
 
 	if (!vif) {
 		WARN_ONCE(true, "no vif to update beacon\n");
-- 
2.39.2

[PATCH 11/20] wifi: rtl8xxxu: support multiple interfaces in configure_filter()

[Martin Kaistra]

As we only want to support AP mode/sending beacons on port 0, change
from priv->vif to priv->vifs[0] in the check for AP mode.
Additionally, if we are in AP mode, don't filter RX beacon and probe
response frames to still allow working STATION mode on the other
interface.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 6683ff0f4d8c7..f54d7b1647792 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6782,8 +6782,8 @@ static void rtl8xxxu_configure_filter(struct ieee80211_hw *hw,
 	else
 		rcr |= RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH;
 
-	if (priv->vif && priv->vif->type == NL80211_IFTYPE_AP)
-		rcr &= ~RCR_CHECK_BSSID_MATCH;
+	if (priv->vifs[0] && priv->vifs[0]->type == NL80211_IFTYPE_AP)
+		rcr &= ~(RCR_CHECK_BSSID_MATCH | RCR_CHECK_BSSID_BEACON);
 
 	if (*total_flags & FIF_CONTROL)
 		rcr |= RCR_ACCEPT_CTRL_FRAME;
-- 
2.39.2

[PATCH 12/20] wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()

[Martin Kaistra]

Check first whether priv->vifs[0] exists and is of type STATION, otherwise
go to priv->vifs[1].

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 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 f54d7b1647792..0d171f61ac8e8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -7190,7 +7190,9 @@ static void rtl8xxxu_watchdog_callback(struct work_struct *work)
 	struct rtl8xxxu_priv *priv;
 
 	priv = container_of(work, struct rtl8xxxu_priv, ra_watchdog.work);
-	vif = priv->vif;
+	vif = priv->vifs[0];
+	if (vif && vif->type != NL80211_IFTYPE_STATION)
+		vif = priv->vifs[1];
 
 	if (vif && vif->type == NL80211_IFTYPE_STATION) {
 		int signal;
-- 
2.39.2

[PATCH 13/20] wifi: rtl8xxxu: support multiple interfaces in {add,remove}_interface()

[Martin Kaistra]

Add a custom struct to store in vif->drv_priv with a reference to
port_num and fill it when a new interface is added. Choose a free
port_num for the newly added interface.

As we only want to support AP mode/sending beacons on port 0, only change
the beacon settings if a new interface is actually assigned to port 0.

Call set_linktype() and set_mac() with the appropriate port_num.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  4 ++
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 52 +++++++++++--------
 2 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index b63fe084de92b..9d272da373a3c 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1921,6 +1921,10 @@ struct rtl8xxxu_sta_info {
 	u8 macid;
 };
 
+struct rtl8xxxu_vif {
+	int port_num;
+};
+
 struct rtl8xxxu_rx_urb {
 	struct urb urb;
 	struct ieee80211_hw *hw;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 0d171f61ac8e8..59dd50844f1ae 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6598,28 +6598,33 @@ static int rtl8xxxu_submit_int_urb(struct ieee80211_hw *hw)
 static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 				  struct ieee80211_vif *vif)
 {
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct rtl8xxxu_priv *priv = hw->priv;
-	int ret;
+	int port_num;
 	u8 val8;
 
-	if (!priv->vif) {
-		priv->vif = vif;
-		priv->vifs[0] = vif;
-	} else {
+	if (!priv->vifs[0])
+		port_num = 0;
+	else if (!priv->vifs[1])
+		port_num = 1;
+	else
 		return -EOPNOTSUPP;
-	}
 
 	switch (vif->type) {
 	case NL80211_IFTYPE_STATION:
-		rtl8xxxu_stop_tx_beacon(priv);
+		if (port_num == 0) {
+			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;
+			val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
+			val8 |= BEACON_ATIM | BEACON_FUNCTION_ENABLE |
+				BEACON_DISABLE_TSF_UPDATE;
+			rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
+		}
 		break;
 	case NL80211_IFTYPE_AP:
+		if (port_num == 1)
+			return -EOPNOTSUPP;
+
 		rtl8xxxu_write8(priv, REG_BEACON_CTRL,
 				BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
 		rtl8xxxu_write8(priv, REG_ATIMWND, 0x0c); /* 12ms */
@@ -6636,31 +6641,32 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 		val8 = rtl8xxxu_read8(priv, REG_CCK_CHECK);
 		val8 &= ~BIT_BCN_PORT_SEL;
 		rtl8xxxu_write8(priv, REG_CCK_CHECK, val8);
-
-		ret = 0;
 		break;
 	default:
-		ret = -EOPNOTSUPP;
+		return -EOPNOTSUPP;
 	}
 
-	rtl8xxxu_set_linktype(priv, vif->type, 0);
+	priv->vifs[port_num] = vif;
+	priv->vif = vif;
+	rtlvif->port_num = port_num;
+
+	rtl8xxxu_set_linktype(priv, vif->type, port_num);
 	ether_addr_copy(priv->mac_addr, vif->addr);
-	rtl8xxxu_set_mac(priv, 0);
+	rtl8xxxu_set_mac(priv, port_num);
 
-	return ret;
+	return 0;
 }
 
 static void rtl8xxxu_remove_interface(struct ieee80211_hw *hw,
 				      struct ieee80211_vif *vif)
 {
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct rtl8xxxu_priv *priv = hw->priv;
 
 	dev_dbg(&priv->udev->dev, "%s\n", __func__);
 
-	if (priv->vif) {
-		priv->vif = NULL;
-		priv->vifs[0] = NULL;
-	}
+	priv->vif = NULL;
+	priv->vifs[rtlvif->port_num] = NULL;
 }
 
 static int rtl8xxxu_config(struct ieee80211_hw *hw, u32 changed)
@@ -7649,6 +7655,8 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 	if (ret)
 		goto err_set_intfdata;
 
+	hw->vif_data_size = sizeof(struct rtl8xxxu_vif);
+
 	hw->wiphy->max_scan_ssids = 1;
 	hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
 	if (priv->fops->max_macid_num)
-- 
2.39.2

[PATCH 14/20] wifi: rtl8xxxu: support multiple interfaces in bss_info_changed()

[Martin Kaistra]

Call set_linktype now with correct port_num. Only react to beacon
changes if port_num == 0, as we only support AP mode on this port.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 26 +++++++++++--------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 59dd50844f1ae..f929b01615d00 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4981,6 +4981,7 @@ static void
 rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			  struct ieee80211_bss_conf *bss_conf, u64 changed)
 {
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct rtl8xxxu_priv *priv = hw->priv;
 	struct device *dev = &priv->udev->dev;
 	struct ieee80211_sta *sta;
@@ -4993,7 +4994,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	if (changed & BSS_CHANGED_ASSOC) {
 		dev_dbg(dev, "Changed ASSOC: %i!\n", vif->cfg.assoc);
 
-		rtl8xxxu_set_linktype(priv, vif->type, 0);
+		rtl8xxxu_set_linktype(priv, vif->type, rtlvif->port_num);
 
 		if (vif->cfg.assoc) {
 			u32 ramask;
@@ -5040,7 +5041,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 			rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
 
-			rtl8xxxu_stop_tx_beacon(priv);
+			if (rtlvif->port_num == 0)
+				rtl8xxxu_stop_tx_beacon(priv);
 
 			/* joinbss sequence */
 			rtl8xxxu_write16(priv, REG_BCN_PSR_RPT,
@@ -5082,7 +5084,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 	if (changed & BSS_CHANGED_BSSID) {
 		dev_dbg(dev, "Changed BSSID!\n");
-		rtl8xxxu_set_bssid(priv, bss_conf->bssid, 0);
+		rtl8xxxu_set_bssid(priv, bss_conf->bssid, rtlvif->port_num);
 	}
 
 	if (changed & BSS_CHANGED_BASIC_RATES) {
@@ -5090,16 +5092,18 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		rtl8xxxu_set_basic_rates(priv, bss_conf->basic_rates);
 	}
 
-	if (changed & BSS_CHANGED_BEACON_ENABLED) {
-		if (bss_conf->enable_beacon)
-			rtl8xxxu_start_tx_beacon(priv);
-		else
-			rtl8xxxu_stop_tx_beacon(priv);
+	/* beacon only supported for port_num = 0 */
+	if (rtlvif->port_num == 0) {
+		if (changed & BSS_CHANGED_BEACON_ENABLED) {
+			if (bss_conf->enable_beacon)
+				rtl8xxxu_start_tx_beacon(priv);
+			else
+				rtl8xxxu_stop_tx_beacon(priv);
+		}
+		if (changed & BSS_CHANGED_BEACON)
+			schedule_work(&priv->update_beacon_work);
 	}
 
-	if (changed & BSS_CHANGED_BEACON)
-		schedule_work(&priv->update_beacon_work);
-
 error:
 	return;
 }
-- 
2.39.2

[PATCH 15/20] wifi: rtl8xxxu: support multiple interface in start_ap()

[Martin Kaistra]

Call set_bssid() with the correct port_num now.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index f929b01615d00..3851fc90339e0 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5111,11 +5111,12 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			     struct ieee80211_bss_conf *link_conf)
 {
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	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, 0);
+	rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid, rtlvif->port_num);
 	rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
 	priv->fops->report_connect(priv, RTL8XXXU_BC_MC_MACID, 0, true);
 
-- 
2.39.2

[PATCH 16/20] wifi: rtl8xxxu: support multiple interfaces in get_macid()

[Martin Kaistra]

As sta_info->macid does not get set in station mode, we can simplify
this function by directly returning 0 if sta itself or sta_info is not
set.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 3851fc90339e0..ad76cddef81b2 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4053,10 +4053,13 @@ static inline u8 rtl8xxxu_get_macid(struct rtl8xxxu_priv *priv,
 {
 	struct rtl8xxxu_sta_info *sta_info;
 
-	if (!priv->vif || priv->vif->type == NL80211_IFTYPE_STATION || !sta)
+	if (!sta)
 		return 0;
 
 	sta_info = (struct rtl8xxxu_sta_info *)sta->drv_priv;
+	if (!sta_info)
+		return 0;
+
 	return sta_info->macid;
 }
 
-- 
2.39.2

[PATCH 17/20] wifi: rtl8xxxu: remove obsolete priv->vif

[Martin Kaistra]

Now that all uses of priv->vif have been converted to priv->vifs[]
remove the old attribute.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h      | 5 -----
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 --
 2 files changed, 7 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 9d272da373a3c..889ef7217f142 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1892,11 +1892,6 @@ struct rtl8xxxu_priv {
 	u8 rssi_level;
 	DECLARE_BITMAP(tx_aggr_started, IEEE80211_NUM_TIDS);
 	DECLARE_BITMAP(tid_tx_operational, IEEE80211_NUM_TIDS);
-	/*
-	 * Only one virtual interface permitted because only STA mode
-	 * is supported and no iface_combinations are provided.
-	 */
-	struct ieee80211_vif *vif;
 
 	struct ieee80211_vif *vifs[2];
 	struct delayed_work ra_watchdog;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index ad76cddef81b2..c2a577ebd061e 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6655,7 +6655,6 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 	}
 
 	priv->vifs[port_num] = vif;
-	priv->vif = vif;
 	rtlvif->port_num = port_num;
 
 	rtl8xxxu_set_linktype(priv, vif->type, port_num);
@@ -6673,7 +6672,6 @@ static void rtl8xxxu_remove_interface(struct ieee80211_hw *hw,
 
 	dev_dbg(&priv->udev->dev, "%s\n", __func__);
 
-	priv->vif = NULL;
 	priv->vifs[rtlvif->port_num] = NULL;
 }
 
-- 
2.39.2

[PATCH 18/20] wifi: rtl8xxxu: add hw crypto support for AP mode

[Martin Kaistra]

Add a custom function for allocating entries in the sec cam. This allows
us to store multiple keys with the same keyidx.

The maximum number of sec cam entries (for 8188f) is 16 according to the
vendor driver.

Set the bssid as mac address for group keys instead of just using the
ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
for differentiating them from pairwise keys like in the vendor driver.

Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
broadcast/multicast packets in AP mode.

Finally, allow the usage of rtl8xxxu_set_key() for AP mode.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  4 ++
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 889ef7217f142..a0222d2666000 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
 #define DESC_RATE_ID_SHIFT		16
 #define DESC_RATE_ID_MASK		0xf
 #define TXDESC_NAVUSEHDR		BIT(20)
+#define TXDESC_EN_DESC_ID		BIT(21)
 #define TXDESC_SEC_RC4			0x00400000
 #define TXDESC_SEC_AES			0x00c00000
 #define TXDESC_PKT_OFFSET_SHIFT		26
@@ -1774,6 +1775,7 @@ struct rtl8xxxu_cfo_tracking {
 #define RTL8XXXU_HW_LED_CONTROL	2
 #define RTL8XXXU_MAX_MAC_ID_NUM	128
 #define RTL8XXXU_BC_MC_MACID	0
+#define RTL8XXXU_MAX_SEC_CAM_NUM	16
 
 struct rtl8xxxu_priv {
 	struct ieee80211_hw *hw;
@@ -1907,6 +1909,7 @@ struct rtl8xxxu_priv {
 	char led_name[32];
 	struct led_classdev led_cdev;
 	DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
+	DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
 };
 
 struct rtl8xxxu_sta_info {
@@ -1918,6 +1921,7 @@ struct rtl8xxxu_sta_info {
 
 struct rtl8xxxu_vif {
 	int port_num;
+	u8 hw_key_idx;
 };
 
 struct rtl8xxxu_rx_urb {
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c2a577ebd061e..88730791091a7 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4558,8 +4558,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
 	 * This is a bit of a hack - the lower bits of the cipher
 	 * suite selector happens to match the cipher index in the CAM
 	 */
-	addr = key->keyidx << CAM_CMD_KEY_SHIFT;
+	addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
 	ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
+	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
+		ctrl |= BIT(6);
 
 	for (j = 5; j >= 0; j--) {
 		switch (j) {
@@ -5545,13 +5547,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 	struct rtl8xxxu_tx_urb *tx_urb;
 	struct ieee80211_sta *sta = NULL;
 	struct ieee80211_vif *vif = tx_info->control.vif;
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct device *dev = &priv->udev->dev;
 	u32 queue, rts_rate;
 	u16 pktlen = skb->len;
 	int tx_desc_size = priv->fops->tx_desc_size;
 	u8 macid;
 	int ret;
-	bool ampdu_enable, sgi = false, short_preamble = false;
+	bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
 
 	if (skb_headroom(skb) < tx_desc_size) {
 		dev_warn(dev,
@@ -5593,10 +5596,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 		tx_desc->txdw0 =
 			TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
 	if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
-	    is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
+	    is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
 		tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
+		bmc = true;
+	}
+
 
 	tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
+	macid = rtl8xxxu_get_macid(priv, sta);
 
 	if (tx_info->control.hw_key) {
 		switch (tx_info->control.hw_key->cipher) {
@@ -5611,6 +5618,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 		default:
 			break;
 		}
+		if (bmc && rtlvif->hw_key_idx != 0xff) {
+			tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
+			macid = rtlvif->hw_key_idx;
+		}
 	}
 
 	/* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
@@ -5654,7 +5665,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 	else
 		rts_rate = 0;
 
-	macid = rtl8xxxu_get_macid(priv, sta);
 	priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
 				ampdu_enable, rts_rate, macid);
 
@@ -6656,6 +6666,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 
 	priv->vifs[port_num] = vif;
 	rtlvif->port_num = port_num;
+	rtlvif->hw_key_idx = 0xff;
 
 	rtl8xxxu_set_linktype(priv, vif->type, port_num);
 	ether_addr_copy(priv->mac_addr, vif->addr);
@@ -6832,11 +6843,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
 	return 0;
 }
 
+static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
+{
+	struct rtl8xxxu_priv *priv = hw->priv;
+
+	return find_first_zero_bit(priv->cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
+}
+
 static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 			    struct ieee80211_vif *vif,
 			    struct ieee80211_sta *sta,
 			    struct ieee80211_key_conf *key)
 {
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct rtl8xxxu_priv *priv = hw->priv;
 	struct device *dev = &priv->udev->dev;
 	u8 mac_addr[ETH_ALEN];
@@ -6848,9 +6867,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
 		__func__, cmd, key->cipher, key->keyidx);
 
-	if (vif->type != NL80211_IFTYPE_STATION)
-		return -EOPNOTSUPP;
-
 	if (key->keyidx > 3)
 		return -EOPNOTSUPP;
 
@@ -6874,7 +6890,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 		ether_addr_copy(mac_addr, sta->addr);
 	} else {
 		dev_dbg(dev, "%s: group key\n", __func__);
-		eth_broadcast_addr(mac_addr);
+		ether_addr_copy(mac_addr, vif->bss_conf.bssid);
 	}
 
 	val16 = rtl8xxxu_read16(priv, REG_CR);
@@ -6888,16 +6904,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 
 	switch (cmd) {
 	case SET_KEY:
-		key->hw_key_idx = key->keyidx;
+
+		retval = rtl8xxxu_get_free_sec_cam(hw);
+		if (retval < 0)
+			return -EOPNOTSUPP;
+
+		key->hw_key_idx = retval;
+
+		if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
+			rtlvif->hw_key_idx = key->hw_key_idx;
+
 		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
 		rtl8xxxu_cam_write(priv, key, mac_addr);
+		set_bit(key->hw_key_idx, priv->cam_map);
 		retval = 0;
 		break;
 	case DISABLE_KEY:
 		rtl8xxxu_write32(priv, REG_CAM_WRITE, 0x00000000);
 		val32 = CAM_CMD_POLLING | CAM_CMD_WRITE |
-			key->keyidx << CAM_CMD_KEY_SHIFT;
+			key->hw_key_idx << CAM_CMD_KEY_SHIFT;
 		rtl8xxxu_write32(priv, REG_CAM_CMD, val32);
+		rtlvif->hw_key_idx = 0xff;
+		clear_bit(key->hw_key_idx, priv->cam_map);
 		retval = 0;
 		break;
 	default:
-- 
2.39.2

[PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent

[Martin Kaistra]

When the driver is used for concurrent mode, both virtual interfaces can
be set to station or AP mode, though only one can be in AP mode at the
same time.

In order to keep the code simple, use only hw port 0 for AP mode. When
an interface is added in AP mode which would be assigned to port 1, use
a switch_port function to transparently swap the mapping between virtual
interface and hw port.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 84 ++++++++++++++++++-
 1 file changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 88730791091a7..595f447874f4d 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6613,6 +6613,84 @@ static int rtl8xxxu_submit_int_urb(struct ieee80211_hw *hw)
 	return ret;
 }
 
+static void rtl8xxxu_switch_ports(struct rtl8xxxu_priv *priv)
+{
+	u8 macid[ETH_ALEN], bssid[ETH_ALEN], macid_1[ETH_ALEN], bssid_1[ETH_ALEN];
+	u8 msr, bcn_ctrl, bcn_ctrl_1, atimwnd[2], atimwnd_1[2];
+	struct rtl8xxxu_vif *rtlvif;
+	struct ieee80211_vif *vif;
+	u8 tsftr[8], tsftr_1[8];
+	int i;
+
+	msr = rtl8xxxu_read8(priv, REG_MSR);
+	bcn_ctrl = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
+	bcn_ctrl_1 = rtl8xxxu_read8(priv, REG_BEACON_CTRL_1);
+
+	for (i = 0; i < ARRAY_SIZE(atimwnd); i++)
+		atimwnd[i] = rtl8xxxu_read8(priv, REG_ATIMWND + i);
+	for (i = 0; i < ARRAY_SIZE(atimwnd_1); i++)
+		atimwnd_1[i] = rtl8xxxu_read8(priv, REG_ATIMWND_1 + i);
+
+	for (i = 0; i < ARRAY_SIZE(tsftr); i++)
+		tsftr[i] = rtl8xxxu_read8(priv, REG_TSFTR + i);
+	for (i = 0; i < ARRAY_SIZE(tsftr); i++)
+		tsftr_1[i] = rtl8xxxu_read8(priv, REG_TSFTR1 + i);
+
+	for (i = 0; i < ARRAY_SIZE(macid); i++)
+		macid[i] = rtl8xxxu_read8(priv, REG_MACID + i);
+
+	for (i = 0; i < ARRAY_SIZE(bssid); i++)
+		bssid[i] = rtl8xxxu_read8(priv, REG_BSSID + i);
+
+	for (i = 0; i < ARRAY_SIZE(macid_1); i++)
+		macid_1[i] = rtl8xxxu_read8(priv, REG_MACID1 + i);
+
+	for (i = 0; i < ARRAY_SIZE(bssid_1); i++)
+		bssid_1[i] = rtl8xxxu_read8(priv, REG_BSSID1 + i);
+
+	/* disable bcn function, disable update TSF */
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL, (bcn_ctrl &
+			(~BEACON_FUNCTION_ENABLE)) | BEACON_DISABLE_TSF_UPDATE);
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL_1, (bcn_ctrl_1 &
+			(~BEACON_FUNCTION_ENABLE)) | BEACON_DISABLE_TSF_UPDATE);
+
+	/* switch msr */
+	msr = (msr & 0xf0) | ((msr & 0x03) << 2) | ((msr & 0x0c) >> 2);
+	rtl8xxxu_write8(priv, REG_MSR, msr);
+
+	/* write port0 */
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL, bcn_ctrl_1 & ~BEACON_FUNCTION_ENABLE);
+	for (i = 0; i < ARRAY_SIZE(atimwnd_1); i++)
+		rtl8xxxu_write8(priv, REG_ATIMWND + i, atimwnd_1[i]);
+	for (i = 0; i < ARRAY_SIZE(tsftr_1); i++)
+		rtl8xxxu_write8(priv, REG_TSFTR + i, tsftr_1[i]);
+	for (i = 0; i < ARRAY_SIZE(macid_1); i++)
+		rtl8xxxu_write8(priv, REG_MACID + i, macid_1[i]);
+	for (i = 0; i < ARRAY_SIZE(bssid_1); i++)
+		rtl8xxxu_write8(priv, REG_BSSID + i, bssid_1[i]);
+
+	/* write port1 */
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL_1, bcn_ctrl & ~BEACON_FUNCTION_ENABLE);
+	for (i = 0; i < ARRAY_SIZE(atimwnd); i++)
+		rtl8xxxu_write8(priv, REG_ATIMWND_1 + i, atimwnd[i]);
+	for (i = 0; i < ARRAY_SIZE(tsftr); i++)
+		rtl8xxxu_write8(priv, REG_TSFTR1 + i, tsftr[i]);
+	for (i = 0; i < ARRAY_SIZE(macid); i++)
+		rtl8xxxu_write8(priv, REG_MACID1 + i, macid[i]);
+	for (i = 0; i < ARRAY_SIZE(bssid); i++)
+		rtl8xxxu_write8(priv, REG_BSSID1 + i, bssid[i]);
+
+	/* write bcn ctl */
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL, bcn_ctrl_1);
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL_1, bcn_ctrl);
+
+	vif = priv->vifs[0];
+	priv->vifs[0] = priv->vifs[1];
+	priv->vifs[1] = vif;
+	rtlvif = (struct rtl8xxxu_vif *)priv->vifs[1]->drv_priv;
+	rtlvif->port_num = 1;
+}
+
 static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 				  struct ieee80211_vif *vif)
 {
@@ -6640,8 +6718,10 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 		}
 		break;
 	case NL80211_IFTYPE_AP:
-		if (port_num == 1)
-			return -EOPNOTSUPP;
+		if (port_num == 1) {
+			rtl8xxxu_switch_ports(priv);
+			port_num = 0;
+		}
 
 		rtl8xxxu_write8(priv, REG_BEACON_CTRL,
 				BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
-- 
2.39.2

[PATCH 20/20] wifi: rtl8xxxu: declare concurrent mode support for 8188f

[Martin Kaistra]

Everything is in place now for concurrent mode, we can tell the system
that we support it.
We will allow a maximum of 2 virtual interfaces, one of them can be in
AP mode.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  1 +
 .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 19 +++++++++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index a0222d2666000..f0c21a41b4b85 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1991,6 +1991,7 @@ struct rtl8xxxu_fileops {
 	u8 init_reg_rxfltmap:1;
 	u8 init_reg_pkt_life_time:1;
 	u8 init_reg_hmtfr:1;
+	u8 supports_concurrent:1;
 	u8 ampdu_max_time;
 	u8 ustime_tsf_edca;
 	u16 max_aggr_num;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
index 1e1c8fa194cb8..80f3c39c7444a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
@@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
 	.max_aggr_num = 0x0c14,
 	.supports_ap = 1,
 	.max_macid_num = 16,
+	.supports_concurrent = 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 595f447874f4d..32fd723687e90 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -7634,6 +7634,20 @@ static void rtl8xxxu_deinit_led(struct rtl8xxxu_priv *priv)
 	led_classdev_unregister(led);
 }
 
+struct ieee80211_iface_limit rtl8xxxu_limits[] = {
+	{ .max = 2, .types = BIT(NL80211_IFTYPE_STATION), },
+	{ .max = 1, .types = BIT(NL80211_IFTYPE_AP), },
+};
+
+struct ieee80211_iface_combination rtl8xxxu_combinations[] = {
+	{
+		.limits = rtl8xxxu_limits,
+		.n_limits = ARRAY_SIZE(rtl8xxxu_limits),
+		.max_interfaces = 2,
+		.num_different_channels = 1,
+	},
+};
+
 static int rtl8xxxu_probe(struct usb_interface *interface,
 			  const struct usb_device_id *id)
 {
@@ -7780,6 +7794,11 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 		hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP);
 	hw->queues = 4;
 
+	if (priv->fops->supports_concurrent) {
+		hw->wiphy->iface_combinations = rtl8xxxu_combinations;
+		hw->wiphy->n_iface_combinations = ARRAY_SIZE(rtl8xxxu_combinations);
+	}
+
 	sband = &rtl8xxxu_supported_band;
 	sband->ht_cap.ht_supported = true;
 	sband->ht_cap.ampdu_factor = IEEE80211_HT_MAX_AMPDU_64K;
-- 
2.39.2

Re: [PATCH 00/20] wifi: rtl8xxxu: Add concurrent mode for 8188f

[Bitterblue Smith]

On 18/12/2023 16:36, Martin Kaistra wrote:
> This series adds the possibility to use two virtual interfaces on the
> same channel. Supported combinations are STA+STA and STA+AP. The
> conversion of the driver to support multiple interfaces is split into
> individual patches to hopefully make it easier to understand what is
> going on.
> 
> Thanks,
>   Martin
> 

Nice work! I'm glad to see this driver get more features.

Re: [PATCH 18/20] wifi: rtl8xxxu: add hw crypto support for AP mode

[Bitterblue Smith]

On 18/12/2023 16:36, Martin Kaistra wrote:
> Add a custom function for allocating entries in the sec cam. This allows
> us to store multiple keys with the same keyidx.
> 
> The maximum number of sec cam entries (for 8188f) is 16 according to the
> vendor driver.

The other chips seem to support more:

RTL8723BU, RTL8192EU, RTL8192FU: 64
RTL8188EU: 32
RTL8710BU: 32 (even though it supports only 16 macid)

I'm not sure about the RTL8723AU and RTL8192CU. Those drivers are
older and different.

Perhaps this should be a member of struct rtl8xxxu_fileops, like
max_macid_num?

> 
> Set the bssid as mac address for group keys instead of just using the
> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
> for differentiating them from pairwise keys like in the vendor driver.
> 
> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
> broadcast/multicast packets in AP mode.
> 
> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> ---
>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  4 ++
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
>  2 files changed, 42 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 889ef7217f142..a0222d2666000 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
>  #define DESC_RATE_ID_SHIFT		16
>  #define DESC_RATE_ID_MASK		0xf
>  #define TXDESC_NAVUSEHDR		BIT(20)
> +#define TXDESC_EN_DESC_ID		BIT(21)
>  #define TXDESC_SEC_RC4			0x00400000
>  #define TXDESC_SEC_AES			0x00c00000
>  #define TXDESC_PKT_OFFSET_SHIFT		26
> @@ -1774,6 +1775,7 @@ struct rtl8xxxu_cfo_tracking {
>  #define RTL8XXXU_HW_LED_CONTROL	2
>  #define RTL8XXXU_MAX_MAC_ID_NUM	128
>  #define RTL8XXXU_BC_MC_MACID	0
> +#define RTL8XXXU_MAX_SEC_CAM_NUM	16
>  
>  struct rtl8xxxu_priv {
>  	struct ieee80211_hw *hw;
> @@ -1907,6 +1909,7 @@ struct rtl8xxxu_priv {
>  	char led_name[32];
>  	struct led_classdev led_cdev;
>  	DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
> +	DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
>  };
>  
>  struct rtl8xxxu_sta_info {
> @@ -1918,6 +1921,7 @@ struct rtl8xxxu_sta_info {
>  
>  struct rtl8xxxu_vif {
>  	int port_num;
> +	u8 hw_key_idx;
>  };
>  
>  struct rtl8xxxu_rx_urb {
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index c2a577ebd061e..88730791091a7 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4558,8 +4558,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
>  	 * This is a bit of a hack - the lower bits of the cipher
>  	 * suite selector happens to match the cipher index in the CAM
>  	 */
> -	addr = key->keyidx << CAM_CMD_KEY_SHIFT;
> +	addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>  	ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
> +	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> +		ctrl |= BIT(6);
>  
>  	for (j = 5; j >= 0; j--) {
>  		switch (j) {
> @@ -5545,13 +5547,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>  	struct rtl8xxxu_tx_urb *tx_urb;
>  	struct ieee80211_sta *sta = NULL;
>  	struct ieee80211_vif *vif = tx_info->control.vif;
> +	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>  	struct device *dev = &priv->udev->dev;
>  	u32 queue, rts_rate;
>  	u16 pktlen = skb->len;
>  	int tx_desc_size = priv->fops->tx_desc_size;
>  	u8 macid;
>  	int ret;
> -	bool ampdu_enable, sgi = false, short_preamble = false;
> +	bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
>  
>  	if (skb_headroom(skb) < tx_desc_size) {
>  		dev_warn(dev,
> @@ -5593,10 +5596,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>  		tx_desc->txdw0 =
>  			TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
>  	if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
> -	    is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
> +	    is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
>  		tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
> +		bmc = true;
> +	}
> +
>  
>  	tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
> +	macid = rtl8xxxu_get_macid(priv, sta);
>  
>  	if (tx_info->control.hw_key) {
>  		switch (tx_info->control.hw_key->cipher) {
> @@ -5611,6 +5618,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>  		default:
>  			break;
>  		}
> +		if (bmc && rtlvif->hw_key_idx != 0xff) {
> +			tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
> +			macid = rtlvif->hw_key_idx;
> +		}

cpu_to_le32(TXDESC_EN_DESC_ID)

The vendor drivers only do this for data frames. Is it the same here?

>  	}
>  
>  	/* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
> @@ -5654,7 +5665,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>  	else
>  		rts_rate = 0;
>  
> -	macid = rtl8xxxu_get_macid(priv, sta);
>  	priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
>  				ampdu_enable, rts_rate, macid);
>  
> @@ -6656,6 +6666,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>  
>  	priv->vifs[port_num] = vif;
>  	rtlvif->port_num = port_num;
> +	rtlvif->hw_key_idx = 0xff;
>  
>  	rtl8xxxu_set_linktype(priv, vif->type, port_num);
>  	ether_addr_copy(priv->mac_addr, vif->addr);
> @@ -6832,11 +6843,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
>  	return 0;
>  }
>  
> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
> +{
> +	struct rtl8xxxu_priv *priv = hw->priv;
> +
> +	return find_first_zero_bit(priv->cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
> +}
> +
>  static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  			    struct ieee80211_vif *vif,
>  			    struct ieee80211_sta *sta,
>  			    struct ieee80211_key_conf *key)
>  {
> +	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>  	struct rtl8xxxu_priv *priv = hw->priv;
>  	struct device *dev = &priv->udev->dev;
>  	u8 mac_addr[ETH_ALEN];
> @@ -6848,9 +6867,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  	dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
>  		__func__, cmd, key->cipher, key->keyidx);
>  
> -	if (vif->type != NL80211_IFTYPE_STATION)
> -		return -EOPNOTSUPP;
> -
>  	if (key->keyidx > 3)
>  		return -EOPNOTSUPP;
>  
> @@ -6874,7 +6890,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  		ether_addr_copy(mac_addr, sta->addr);
>  	} else {
>  		dev_dbg(dev, "%s: group key\n", __func__);
> -		eth_broadcast_addr(mac_addr);
> +		ether_addr_copy(mac_addr, vif->bss_conf.bssid);
>  	}
>  
>  	val16 = rtl8xxxu_read16(priv, REG_CR);
> @@ -6888,16 +6904,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  
>  	switch (cmd) {
>  	case SET_KEY:
> -		key->hw_key_idx = key->keyidx;
> +
> +		retval = rtl8xxxu_get_free_sec_cam(hw);
> +		if (retval < 0)
> +			return -EOPNOTSUPP;
> +
> +		key->hw_key_idx = retval;
> +
> +		if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> +			rtlvif->hw_key_idx = key->hw_key_idx;
> +
>  		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
>  		rtl8xxxu_cam_write(priv, key, mac_addr);
> +		set_bit(key->hw_key_idx, priv->cam_map);
>  		retval = 0;
>  		break;
>  	case DISABLE_KEY:
>  		rtl8xxxu_write32(priv, REG_CAM_WRITE, 0x00000000);
>  		val32 = CAM_CMD_POLLING | CAM_CMD_WRITE |
> -			key->keyidx << CAM_CMD_KEY_SHIFT;
> +			key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>  		rtl8xxxu_write32(priv, REG_CAM_CMD, val32);
> +		rtlvif->hw_key_idx = 0xff;
> +		clear_bit(key->hw_key_idx, priv->cam_map);
>  		retval = 0;
>  		break;
>  	default:

RE: [PATCH 07/20] wifi: rtl8xxxu: extend check for matching bssid to both interfaces

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Monday, December 18, 2023 10:37 PM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
> <bigeasy@linutronix.de>
> Subject: [PATCH 07/20] wifi: rtl8xxxu: extend check for matching bssid to both interfaces
> 
> The driver will support two interfaces soon, which both can be in
> station mode, so extend the check, whether cfo information should be
> parsed, to cover both interfaces.
> 
> For better code readability put the lines with priv->vifs[port_num] in a
> separate function.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> ---
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 22 ++++++++++++-------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index fd6b6e2eba038..c3039049e9f5b 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -5706,6 +5706,16 @@ static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work)
>         rtl8xxxu_send_beacon_frame(hw, vif);
>  }
> 
> +static inline bool rtl8xxxu_is_packet_match_bssid(struct rtl8xxxu_priv *priv,
> +                                                 struct ieee80211_hdr *hdr,
> +                                                 int port_num)
> +{
> +       return priv->vifs[port_num] &&
> +               priv->vifs[port_num]->type == NL80211_IFTYPE_STATION &&
> +               priv->vifs[port_num]->cfg.assoc &&
> +               ether_addr_equal(priv->vifs[port_num]->bss_conf.bssid, hdr->addr2);

nit: coding style: align "priv->vifs", like

return priv->vifs[...]
       priv->vifs[port_num]....


> +}
> +
>  void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
>                                  struct ieee80211_rx_status *rx_status,
>                                  struct rtl8723au_phy_stats *phy_stats,
> @@ -5722,12 +5732,10 @@ void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
>                 rx_status->signal = priv->fops->cck_rssi(priv, phy_stats);
>         } else {
>                 bool parse_cfo = priv->fops->set_crystal_cap &&
> -                                priv->vif &&
> -                                priv->vif->type == NL80211_IFTYPE_STATION &&
> -                                priv->vif->cfg.assoc &&
>                                  !crc_icv_err &&
>                                  !ieee80211_is_ctl(hdr->frame_control) &&
> -                                ether_addr_equal(priv->vif->bss_conf.bssid, hdr->addr2);
> +                                (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) ||
> +                                 rtl8xxxu_is_packet_match_bssid(priv, hdr, 1));

I feel that driver can only track single one CFO (carrier frequency offset)
from AP. Considering STA+STA case with two different APs, it would cause
ping-pong CFO values between two APs. 

A simple way is just to ignore CFO for STA+STA case. Another way is to
reference the methods implemented by rtw89 where function name is
rtw89_phy_multi_sta_cfo_calc(). One method is to record CFO tail for each
mac_id and use the average as target CFO value to hardware.

Ping-Ke

RE: [PATCH 09/20] wifi: rtl8xxxu: support multiple interfaces in set_aifs()

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Monday, December 18, 2023 10:37 PM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
> <bigeasy@linutronix.de>
> Subject: [PATCH 09/20] wifi: rtl8xxxu: support multiple interfaces in set_aifs()
> 
> In concurrent mode supported by this driver, both interfaces will use
> the same channel and same wireless mode.
> It is therefore possible to get the wireless mode by checking the first
> connected interface.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> ---
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 2b546bce01237..827e715f0e585 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4913,14 +4913,18 @@ static void rtl8xxxu_set_aifs(struct rtl8xxxu_priv *priv, u8 slot_time)
>         u8 aifs, aifsn, sifs;
>         int i;
> 
> -       if (priv->vif) {
> -               struct ieee80211_sta *sta;
> +       for (i = 0; i < ARRAY_SIZE(priv->vifs); i++) {
> +               if (priv->vifs[i]) {

if (!priv->vifs[i])
        continue;

Then, you can only stir few code. 

Ping-Ke

RE: [PATCH 12/20] wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Monday, December 18, 2023 10:37 PM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
> <bigeasy@linutronix.de>
> Subject: [PATCH 12/20] wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()
> 
> Check first whether priv->vifs[0] exists and is of type STATION, otherwise
> go to priv->vifs[1].
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> ---
>  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 f54d7b1647792..0d171f61ac8e8 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -7190,7 +7190,9 @@ static void rtl8xxxu_watchdog_callback(struct work_struct *work)

This patch is fine. But, we need to extend this function to support 
multiple sta to rtl8xxxu_refresh_rate_mask(sta) at least. 

Ping-Ke

RE: [PATCH 14/20] wifi: rtl8xxxu: support multiple interfaces in bss_info_changed()

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Monday, December 18, 2023 10:37 PM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
> <bigeasy@linutronix.de>
> Subject: [PATCH 14/20] wifi: rtl8xxxu: support multiple interfaces in bss_info_changed()
> 
> Call set_linktype now with correct port_num. Only react to beacon
> changes if port_num == 0, as we only support AP mode on this port.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> ---
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 26 +++++++++++--------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 59dd50844f1ae..f929b01615d00 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c

[...]

> @@ -5090,16 +5092,18 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>                 rtl8xxxu_set_basic_rates(priv, bss_conf->basic_rates);
>         }
> 
> -       if (changed & BSS_CHANGED_BEACON_ENABLED) {
> -               if (bss_conf->enable_beacon)
> -                       rtl8xxxu_start_tx_beacon(priv);
> -               else
> -                       rtl8xxxu_stop_tx_beacon(priv);
> +       /* beacon only supported for port_num = 0 */
> +       if (rtlvif->port_num == 0) {

As your design, AP mode must play on port 0. Could mac80211 notify driver BEACON
changed on port 1? 

> +               if (changed & BSS_CHANGED_BEACON_ENABLED) {
> +                       if (bss_conf->enable_beacon)
> +                               rtl8xxxu_start_tx_beacon(priv);
> +                       else
> +                               rtl8xxxu_stop_tx_beacon(priv);
> +               }
> +               if (changed & BSS_CHANGED_BEACON)
> +                       schedule_work(&priv->update_beacon_work);
>         }
> 
> -       if (changed & BSS_CHANGED_BEACON)
> -               schedule_work(&priv->update_beacon_work);
> -
>  error:
>         return;
>  }

Ping-Ke

RE: [PATCH 16/20] wifi: rtl8xxxu: support multiple interfaces in get_macid()

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Monday, December 18, 2023 10:37 PM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
> <bigeasy@linutronix.de>
> Subject: [PATCH 16/20] wifi: rtl8xxxu: support multiple interfaces in get_macid()
> 
> As sta_info->macid does not get set in station mode, we can simplify
> this function by directly returning 0 if sta itself or sta_info is not
> set.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 3851fc90339e0..ad76cddef81b2 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4053,10 +4053,13 @@ static inline u8 rtl8xxxu_get_macid(struct rtl8xxxu_priv *priv,
>  {
>         struct rtl8xxxu_sta_info *sta_info;
> 
> -       if (!priv->vif || priv->vif->type == NL80211_IFTYPE_STATION || !sta)
> +       if (!sta)
>                 return 0;
> 
>         sta_info = (struct rtl8xxxu_sta_info *)sta->drv_priv;
> +       if (!sta_info)
> +               return 0;
> +
>         return sta_info->macid;

I checked where driver assign macid, and only

	if (vif->type == NL80211_IFTYPE_AP) {
		sta_info->macid = rtl8xxxu_acquire_macid(priv);

That means STA mode can be macid == 0 always, right?
This will be a problem. At least TX rate will be incorrect. 

Ping-Ke 

RE: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Monday, December 18, 2023 10:37 PM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
> <bigeasy@linutronix.de>
> Subject: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
> 
> When the driver is used for concurrent mode, both virtual interfaces can
> be set to station or AP mode, though only one can be in AP mode at the
> same time.
> 
> In order to keep the code simple, use only hw port 0 for AP mode. When
> an interface is added in AP mode which would be assigned to port 1, use
> a switch_port function to transparently swap the mapping between virtual
> interface and hw port.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> ---
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 84 ++++++++++++++++++-
>  1 file changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 88730791091a7..595f447874f4d 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6613,6 +6613,84 @@ static int rtl8xxxu_submit_int_urb(struct ieee80211_hw *hw)
>         return ret;
>  }
> 
> +static void rtl8xxxu_switch_ports(struct rtl8xxxu_priv *priv)
> +{

[...]

> +
> +       vif = priv->vifs[0];
> +       priv->vifs[0] = priv->vifs[1];
> +       priv->vifs[1] = vif;
> +       rtlvif = (struct rtl8xxxu_vif *)priv->vifs[1]->drv_priv;
> +       rtlvif->port_num = 1;

nit: Would it be better to swap port_num as well? Currently, port_num of vifs[0]
will be set to 0 by caller, but not sure if further people could misuse this
function.


> +}
> +
>  static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>                                   struct ieee80211_vif *vif)
>  {
> @@ -6640,8 +6718,10 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>                 }
>                 break;
>         case NL80211_IFTYPE_AP:
> -               if (port_num == 1)
> -                       return -EOPNOTSUPP;
> +               if (port_num == 1) {
> +                       rtl8xxxu_switch_ports(priv);
> +                       port_num = 0;
> +               }
> 
>                 rtl8xxxu_write8(priv, REG_BEACON_CTRL,
>                                 BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
> --
> 2.39.2

RE: [PATCH 00/20] wifi: rtl8xxxu: Add concurrent mode for 8188f

[Ping-Ke Shih]

> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Wednesday, December 20, 2023 6:03 AM
> To: Martin Kaistra <martin.kaistra@linutronix.de>; linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> <pkshih@realtek.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Subject: Re: [PATCH 00/20] wifi: rtl8xxxu: Add concurrent mode for 8188f
> 
> On 18/12/2023 16:36, Martin Kaistra wrote:
> > This series adds the possibility to use two virtual interfaces on the
> > same channel. Supported combinations are STA+STA and STA+AP. The
> > conversion of the driver to support multiple interfaces is split into
> > individual patches to hopefully make it easier to understand what is
> > going on.
> >
> > Thanks,
> >   Martin
> >
> 
> Nice work! I'm glad to see this driver get more features.

Agree! It seems this driver can possible to support P2P as well. 

I have reviewed this patchset, and please feel free to add my reviewed-by
    Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
if I have no comment on patches and you don't change them by v2. 

Ping-Ke

Re: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent

[Martin Kaistra]

Am 20.12.23 um 07:28 schrieb Ping-Ke Shih:
> 
> 
>> -----Original Message-----
>> From: Martin Kaistra <martin.kaistra@linutronix.de>
>> Sent: Monday, December 18, 2023 10:37 PM
>> To: linux-wireless@vger.kernel.org
>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
>> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de>
>> Subject: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
>>
>> When the driver is used for concurrent mode, both virtual interfaces can
>> be set to station or AP mode, though only one can be in AP mode at the
>> same time.
>>
>> In order to keep the code simple, use only hw port 0 for AP mode. When
>> an interface is added in AP mode which would be assigned to port 1, use
>> a switch_port function to transparently swap the mapping between virtual
>> interface and hw port.
>>
>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>> ---
>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 84 ++++++++++++++++++-
>>   1 file changed, 82 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 88730791091a7..595f447874f4d 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -6613,6 +6613,84 @@ static int rtl8xxxu_submit_int_urb(struct ieee80211_hw *hw)
>>          return ret;
>>   }
>>
>> +static void rtl8xxxu_switch_ports(struct rtl8xxxu_priv *priv)
>> +{
> 
> [...]
> 
>> +
>> +       vif = priv->vifs[0];
>> +       priv->vifs[0] = priv->vifs[1];
>> +       priv->vifs[1] = vif;
>> +       rtlvif = (struct rtl8xxxu_vif *)priv->vifs[1]->drv_priv;
>> +       rtlvif->port_num = 1;
> 
> nit: Would it be better to swap port_num as well? Currently, port_num of vifs[0]
> will be set to 0 by caller, but not sure if further people could misuse this
> function.

the main reason, I did not include setting port_num for priv->vifs[0], is that 
priv->vifs[0] is a NULL pointer in the current way this function is called from 
rtl8xxxu_add_interface().

do you think it makes sense to add

if (priv->vifs[0])
        rtlvif = (struct rtl8xxxu_vif *)priv->vifs[0]->drv_priv;
        rtlvif->port_num = 0;

just for completeness sake, even though this code path will currently never get 
executed?

> 
> 
>> +}
>> +
>>   static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>>                                    struct ieee80211_vif *vif)
>>   {
>> @@ -6640,8 +6718,10 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>>                  }
>>                  break;
>>          case NL80211_IFTYPE_AP:
>> -               if (port_num == 1)
>> -                       return -EOPNOTSUPP;
>> +               if (port_num == 1) {
>> +                       rtl8xxxu_switch_ports(priv);
>> +                       port_num = 0;
>> +               }
>>
>>                  rtl8xxxu_write8(priv, REG_BEACON_CTRL,
>>                                  BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
>> --
>> 2.39.2
> 

Re: [PATCH 18/20] wifi: rtl8xxxu: add hw crypto support for AP mode

[Martin Kaistra]

Am 19.12.23 um 23:29 schrieb Bitterblue Smith:
> On 18/12/2023 16:36, Martin Kaistra wrote:
>> Add a custom function for allocating entries in the sec cam. This allows
>> us to store multiple keys with the same keyidx.
>>
>> The maximum number of sec cam entries (for 8188f) is 16 according to the
>> vendor driver.
> 
> The other chips seem to support more:
> 
> RTL8723BU, RTL8192EU, RTL8192FU: 64
> RTL8188EU: 32
> RTL8710BU: 32 (even though it supports only 16 macid)
> 
> I'm not sure about the RTL8723AU and RTL8192CU. Those drivers are
> older and different.
> 
> Perhaps this should be a member of struct rtl8xxxu_fileops, like
> max_macid_num?

Sounds good to me, I will implement this for v2.

> 
>>
>> Set the bssid as mac address for group keys instead of just using the
>> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
>> for differentiating them from pairwise keys like in the vendor driver.
>>
>> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
>> broadcast/multicast packets in AP mode.
>>
>> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
>>
>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>> ---
>>   .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  4 ++
>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
>>   2 files changed, 42 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> index 889ef7217f142..a0222d2666000 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
>>   #define DESC_RATE_ID_SHIFT		16
>>   #define DESC_RATE_ID_MASK		0xf
>>   #define TXDESC_NAVUSEHDR		BIT(20)
>> +#define TXDESC_EN_DESC_ID		BIT(21)
>>   #define TXDESC_SEC_RC4			0x00400000
>>   #define TXDESC_SEC_AES			0x00c00000
>>   #define TXDESC_PKT_OFFSET_SHIFT		26
>> @@ -1774,6 +1775,7 @@ struct rtl8xxxu_cfo_tracking {
>>   #define RTL8XXXU_HW_LED_CONTROL	2
>>   #define RTL8XXXU_MAX_MAC_ID_NUM	128
>>   #define RTL8XXXU_BC_MC_MACID	0
>> +#define RTL8XXXU_MAX_SEC_CAM_NUM	16
>>   
>>   struct rtl8xxxu_priv {
>>   	struct ieee80211_hw *hw;
>> @@ -1907,6 +1909,7 @@ struct rtl8xxxu_priv {
>>   	char led_name[32];
>>   	struct led_classdev led_cdev;
>>   	DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
>> +	DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
>>   };
>>   
>>   struct rtl8xxxu_sta_info {
>> @@ -1918,6 +1921,7 @@ struct rtl8xxxu_sta_info {
>>   
>>   struct rtl8xxxu_vif {
>>   	int port_num;
>> +	u8 hw_key_idx;
>>   };
>>   
>>   struct rtl8xxxu_rx_urb {
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index c2a577ebd061e..88730791091a7 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -4558,8 +4558,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
>>   	 * This is a bit of a hack - the lower bits of the cipher
>>   	 * suite selector happens to match the cipher index in the CAM
>>   	 */
>> -	addr = key->keyidx << CAM_CMD_KEY_SHIFT;
>> +	addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>>   	ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
>> +	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>> +		ctrl |= BIT(6);
>>   
>>   	for (j = 5; j >= 0; j--) {
>>   		switch (j) {
>> @@ -5545,13 +5547,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>   	struct rtl8xxxu_tx_urb *tx_urb;
>>   	struct ieee80211_sta *sta = NULL;
>>   	struct ieee80211_vif *vif = tx_info->control.vif;
>> +	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>   	struct device *dev = &priv->udev->dev;
>>   	u32 queue, rts_rate;
>>   	u16 pktlen = skb->len;
>>   	int tx_desc_size = priv->fops->tx_desc_size;
>>   	u8 macid;
>>   	int ret;
>> -	bool ampdu_enable, sgi = false, short_preamble = false;
>> +	bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
>>   
>>   	if (skb_headroom(skb) < tx_desc_size) {
>>   		dev_warn(dev,
>> @@ -5593,10 +5596,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>   		tx_desc->txdw0 =
>>   			TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
>>   	if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
>> -	    is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
>> +	    is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
>>   		tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
>> +		bmc = true;
>> +	}
>> +
>>   
>>   	tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
>> +	macid = rtl8xxxu_get_macid(priv, sta);
>>   
>>   	if (tx_info->control.hw_key) {
>>   		switch (tx_info->control.hw_key->cipher) {
>> @@ -5611,6 +5618,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>   		default:
>>   			break;
>>   		}
>> +		if (bmc && rtlvif->hw_key_idx != 0xff) {
>> +			tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
>> +			macid = rtlvif->hw_key_idx;
>> +		}
> 
> cpu_to_le32(TXDESC_EN_DESC_ID)
> 
> The vendor drivers only do this for data frames. Is it the same here?
> 
>>   	}
>>   
>>   	/* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
>> @@ -5654,7 +5665,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>   	else
>>   		rts_rate = 0;
>>   
>> -	macid = rtl8xxxu_get_macid(priv, sta);
>>   	priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
>>   				ampdu_enable, rts_rate, macid);
>>   
>> @@ -6656,6 +6666,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>>   
>>   	priv->vifs[port_num] = vif;
>>   	rtlvif->port_num = port_num;
>> +	rtlvif->hw_key_idx = 0xff;
>>   
>>   	rtl8xxxu_set_linktype(priv, vif->type, port_num);
>>   	ether_addr_copy(priv->mac_addr, vif->addr);
>> @@ -6832,11 +6843,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
>>   	return 0;
>>   }
>>   
>> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>> +{
>> +	struct rtl8xxxu_priv *priv = hw->priv;
>> +
>> +	return find_first_zero_bit(priv->cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
>> +}
>> +
>>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>   			    struct ieee80211_vif *vif,
>>   			    struct ieee80211_sta *sta,
>>   			    struct ieee80211_key_conf *key)
>>   {
>> +	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>   	struct rtl8xxxu_priv *priv = hw->priv;
>>   	struct device *dev = &priv->udev->dev;
>>   	u8 mac_addr[ETH_ALEN];
>> @@ -6848,9 +6867,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>   	dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
>>   		__func__, cmd, key->cipher, key->keyidx);
>>   
>> -	if (vif->type != NL80211_IFTYPE_STATION)
>> -		return -EOPNOTSUPP;
>> -
>>   	if (key->keyidx > 3)
>>   		return -EOPNOTSUPP;
>>   
>> @@ -6874,7 +6890,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>   		ether_addr_copy(mac_addr, sta->addr);
>>   	} else {
>>   		dev_dbg(dev, "%s: group key\n", __func__);
>> -		eth_broadcast_addr(mac_addr);
>> +		ether_addr_copy(mac_addr, vif->bss_conf.bssid);
>>   	}
>>   
>>   	val16 = rtl8xxxu_read16(priv, REG_CR);
>> @@ -6888,16 +6904,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>   
>>   	switch (cmd) {
>>   	case SET_KEY:
>> -		key->hw_key_idx = key->keyidx;
>> +
>> +		retval = rtl8xxxu_get_free_sec_cam(hw);
>> +		if (retval < 0)
>> +			return -EOPNOTSUPP;
>> +
>> +		key->hw_key_idx = retval;
>> +
>> +		if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>> +			rtlvif->hw_key_idx = key->hw_key_idx;
>> +
>>   		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
>>   		rtl8xxxu_cam_write(priv, key, mac_addr);
>> +		set_bit(key->hw_key_idx, priv->cam_map);
>>   		retval = 0;
>>   		break;
>>   	case DISABLE_KEY:
>>   		rtl8xxxu_write32(priv, REG_CAM_WRITE, 0x00000000);
>>   		val32 = CAM_CMD_POLLING | CAM_CMD_WRITE |
>> -			key->keyidx << CAM_CMD_KEY_SHIFT;
>> +			key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>>   		rtl8xxxu_write32(priv, REG_CAM_CMD, val32);
>> +		rtlvif->hw_key_idx = 0xff;
>> +		clear_bit(key->hw_key_idx, priv->cam_map);
>>   		retval = 0;
>>   		break;
>>   	default:
> 

Re: [PATCH 16/20] wifi: rtl8xxxu: support multiple interfaces in get_macid()

[Martin Kaistra]

Am 20.12.23 um 07:14 schrieb Ping-Ke Shih:
> 
> 
>> -----Original Message-----
>> From: Martin Kaistra <martin.kaistra@linutronix.de>
>> Sent: Monday, December 18, 2023 10:37 PM
>> To: linux-wireless@vger.kernel.org
>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
>> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de>
>> Subject: [PATCH 16/20] wifi: rtl8xxxu: support multiple interfaces in get_macid()
>>
>> As sta_info->macid does not get set in station mode, we can simplify
>> this function by directly returning 0 if sta itself or sta_info is not
>> set.
>>
>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>> ---
>>   drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 3851fc90339e0..ad76cddef81b2 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -4053,10 +4053,13 @@ static inline u8 rtl8xxxu_get_macid(struct rtl8xxxu_priv *priv,
>>   {
>>          struct rtl8xxxu_sta_info *sta_info;
>>
>> -       if (!priv->vif || priv->vif->type == NL80211_IFTYPE_STATION || !sta)
>> +       if (!sta)
>>                  return 0;
>>
>>          sta_info = (struct rtl8xxxu_sta_info *)sta->drv_priv;
>> +       if (!sta_info)
>> +               return 0;
>> +
>>          return sta_info->macid;
> 
> I checked where driver assign macid, and only
> 
> 	if (vif->type == NL80211_IFTYPE_AP) {
> 		sta_info->macid = rtl8xxxu_acquire_macid(priv);
> 
> That means STA mode can be macid == 0 always, right?
> This will be a problem. At least TX rate will be incorrect.

Yes, currently macid for STA mode is always 0. Would it be enough to set macid 
in STA mode to either 0 or 1 depending on port_num?

> 
> Ping-Ke
> 

Re: [PATCH 07/20] wifi: rtl8xxxu: extend check for matching bssid to both interfaces

[Martin Kaistra]

Am 20.12.23 um 06:51 schrieb Ping-Ke Shih:
> 
> 
>> -----Original Message-----
>> From: Martin Kaistra <martin.kaistra@linutronix.de>
>> Sent: Monday, December 18, 2023 10:37 PM
>> To: linux-wireless@vger.kernel.org
>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
>> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de>
>> Subject: [PATCH 07/20] wifi: rtl8xxxu: extend check for matching bssid to both interfaces
>>
>> The driver will support two interfaces soon, which both can be in
>> station mode, so extend the check, whether cfo information should be
>> parsed, to cover both interfaces.
>>
>> For better code readability put the lines with priv->vifs[port_num] in a
>> separate function.
>>
>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>> ---
>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 22 ++++++++++++-------
>>   1 file changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index fd6b6e2eba038..c3039049e9f5b 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -5706,6 +5706,16 @@ static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work)
>>          rtl8xxxu_send_beacon_frame(hw, vif);
>>   }
>>
>> +static inline bool rtl8xxxu_is_packet_match_bssid(struct rtl8xxxu_priv *priv,
>> +                                                 struct ieee80211_hdr *hdr,
>> +                                                 int port_num)
>> +{
>> +       return priv->vifs[port_num] &&
>> +               priv->vifs[port_num]->type == NL80211_IFTYPE_STATION &&
>> +               priv->vifs[port_num]->cfg.assoc &&
>> +               ether_addr_equal(priv->vifs[port_num]->bss_conf.bssid, hdr->addr2);
> 
> nit: coding style: align "priv->vifs", like
> 
> return priv->vifs[...]
>         priv->vifs[port_num]....
> 
> 
>> +}
>> +
>>   void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
>>                                   struct ieee80211_rx_status *rx_status,
>>                                   struct rtl8723au_phy_stats *phy_stats,
>> @@ -5722,12 +5732,10 @@ void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
>>                  rx_status->signal = priv->fops->cck_rssi(priv, phy_stats);
>>          } else {
>>                  bool parse_cfo = priv->fops->set_crystal_cap &&
>> -                                priv->vif &&
>> -                                priv->vif->type == NL80211_IFTYPE_STATION &&
>> -                                priv->vif->cfg.assoc &&
>>                                   !crc_icv_err &&
>>                                   !ieee80211_is_ctl(hdr->frame_control) &&
>> -                                ether_addr_equal(priv->vif->bss_conf.bssid, hdr->addr2);
>> +                                (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) ||
>> +                                 rtl8xxxu_is_packet_match_bssid(priv, hdr, 1));
> 
> I feel that driver can only track single one CFO (carrier frequency offset)
> from AP. Considering STA+STA case with two different APs, it would cause
> ping-pong CFO values between two APs.
> 
> A simple way is just to ignore CFO for STA+STA case. Another way is to
> reference the methods implemented by rtw89 where function name is
> rtw89_phy_multi_sta_cfo_calc(). One method is to record CFO tail for each
> mac_id and use the average as target CFO value to hardware.

This is only a problem, if both interfaces are in STA mode and are actually 
connected, right?
Then, I will add a check and ignore CFO in that case.

> 
> Ping-Ke
> 

Re: [PATCH 07/20] wifi: rtl8xxxu: extend check for matching bssid to both interfaces

[Ping-Ke Shih]

On Wed, 2023-12-20 at 17:43 +0100, Martin Kaistra wrote:
> 
> Am 20.12.23 um 06:51 schrieb Ping-Ke Shih:
> > 
> > I feel that driver can only track single one CFO (carrier frequency offset)
> > from AP. Considering STA+STA case with two different APs, it would cause
> > ping-pong CFO values between two APs.
> > 
> > A simple way is just to ignore CFO for STA+STA case. Another way is to
> > reference the methods implemented by rtw89 where function name is
> > rtw89_phy_multi_sta_cfo_calc(). One method is to record CFO tail for each
> > mac_id and use the average as target CFO value to hardware.
> 
> This is only a problem, if both interfaces are in STA mode and are actually
> connected, right?

Yes. 

Re: [PATCH 16/20] wifi: rtl8xxxu: support multiple interfaces in get_macid()

[Ping-Ke Shih]

On Wed, 2023-12-20 at 17:40 +0100, Martin Kaistra wrote:
> 
> Am 20.12.23 um 07:14 schrieb Ping-Ke Shih:
> > 
> > 
> > I checked where driver assign macid, and only
> > 
> >       if (vif->type == NL80211_IFTYPE_AP) {
> >               sta_info->macid = rtl8xxxu_acquire_macid(priv);
> > 
> > That means STA mode can be macid == 0 always, right?
> > This will be a problem. At least TX rate will be incorrect.
> 
> Yes, currently macid for STA mode is always 0. Would it be enough to set macid
> in STA mode to either 0 or 1 depending on port_num?
> 

I am not very sure if macid 0 plays a special role, but others (macid >= 1)
can be dynamically assigned to each stations. 

I think we can reserve macid 0 and 1 for port 0 and 1 respectively,
and dynamically assign macid 2 or larger to TDLS or AP mode peer, like

macid     port num         STA mode            AP mode
------    ----------      -------------      ---------------------------
  0           0            to/from AP          broadcast
  1           1            to/from AP             X
  2~       0 or 1          to/from TDLS        to/from connected station

Re: [PATCH 14/20] wifi: rtl8xxxu: support multiple interfaces in bss_info_changed()

[Martin Kaistra]

Am 20.12.23 um 07:09 schrieb Ping-Ke Shih:
> 
> 
>> -----Original Message-----
>> From: Martin Kaistra <martin.kaistra@linutronix.de>
>> Sent: Monday, December 18, 2023 10:37 PM
>> To: linux-wireless@vger.kernel.org
>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
>> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de>
>> Subject: [PATCH 14/20] wifi: rtl8xxxu: support multiple interfaces in bss_info_changed()
>>
>> Call set_linktype now with correct port_num. Only react to beacon
>> changes if port_num == 0, as we only support AP mode on this port.
>>
>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>> ---
>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 26 +++++++++++--------
>>   1 file changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 59dd50844f1ae..f929b01615d00 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> 
> [...]
> 
>> @@ -5090,16 +5092,18 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>>                  rtl8xxxu_set_basic_rates(priv, bss_conf->basic_rates);
>>          }
>>
>> -       if (changed & BSS_CHANGED_BEACON_ENABLED) {
>> -               if (bss_conf->enable_beacon)
>> -                       rtl8xxxu_start_tx_beacon(priv);
>> -               else
>> -                       rtl8xxxu_stop_tx_beacon(priv);
>> +       /* beacon only supported for port_num = 0 */
>> +       if (rtlvif->port_num == 0) {
> 
> As your design, AP mode must play on port 0. Could mac80211 notify driver BEACON
> changed on port 1?

Looking at mac80211 code, there is an explicit check for vif.type when changed 
contains BSS_CHANGED_BEACON or BSS_CHANGED_BEACON_ENABLED. As rtl8xxxu driver 
only allows to add NL80211_IFTYPE_AP on port_num = 0, a notification on port 1 
will never happen. I will remove this check for v2.


> 
>> +               if (changed & BSS_CHANGED_BEACON_ENABLED) {
>> +                       if (bss_conf->enable_beacon)
>> +                               rtl8xxxu_start_tx_beacon(priv);
>> +                       else
>> +                               rtl8xxxu_stop_tx_beacon(priv);
>> +               }
>> +               if (changed & BSS_CHANGED_BEACON)
>> +                       schedule_work(&priv->update_beacon_work);
>>          }
>>
>> -       if (changed & BSS_CHANGED_BEACON)
>> -               schedule_work(&priv->update_beacon_work);
>> -
>>   error:
>>          return;
>>   }
> 
> Ping-Ke
> 

Re: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent

[Ping-Ke Shih]

On Wed, 2023-12-20 at 17:34 +0100, Martin Kaistra wrote:
> External mail.
> 
> 
> 
> Am 20.12.23 um 07:28 schrieb Ping-Ke Shih:
> > 
> > > -----Original Message-----
> > > From: Martin Kaistra <martin.kaistra@linutronix.de>
> > > Sent: Monday, December 18, 2023 10:37 PM
> > > To: linux-wireless@vger.kernel.org
> > > Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> > > <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
> > > <bigeasy@linutronix.de>
> > > Subject: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
> > > 
> > > +
> > > +       vif = priv->vifs[0];
> > > +       priv->vifs[0] = priv->vifs[1];
> > > +       priv->vifs[1] = vif;
> > > +       rtlvif = (struct rtl8xxxu_vif *)priv->vifs[1]->drv_priv;
> > > +       rtlvif->port_num = 1;
> > 
> > nit: Would it be better to swap port_num as well? Currently, port_num of vifs[0]
> > will be set to 0 by caller, but not sure if further people could misuse this
> > function.
> 
> the main reason, I did not include setting port_num for priv->vifs[0], is that
> priv->vifs[0] is a NULL pointer in the current way this function is called from
> rtl8xxxu_add_interface().
> 
> do you think it makes sense to add
> 
> if (priv->vifs[0])
>         rtlvif = (struct rtl8xxxu_vif *)priv->vifs[0]->drv_priv;
>         rtlvif->port_num = 0;
> 
> just for completeness sake, even though this code path will currently never get
> executed?
> 

I missed that point. I just did focus on "switch", but actually this is
"move" from port 0 to 1, right? 

As I know, two cases only work on port 0. One is AP mode that you are
doing, and the other is PS in STA mode that isn't implemented by rtl8xxxxu.
For AP mode, current implement moving port 0 to 1 is fine.

In the future, PS in STA mode could need to move port 1 to 0, because 
we may disconnect port 0 first and we want STA on port 1 entering PS.

But, I think we can defer this until we really need it. 

Re: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent

[Martin Kaistra]

Am 21.12.23 um 09:25 schrieb Ping-Ke Shih:
> On Wed, 2023-12-20 at 17:34 +0100, Martin Kaistra wrote:
>> External mail.
>>
>>
>>
>> Am 20.12.23 um 07:28 schrieb Ping-Ke Shih:
>>>
>>>> -----Original Message-----
>>>> From: Martin Kaistra <martin.kaistra@linutronix.de>
>>>> Sent: Monday, December 18, 2023 10:37 PM
>>>> To: linux-wireless@vger.kernel.org
>>>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
>>>> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
>>>> <bigeasy@linutronix.de>
>>>> Subject: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
>>>>
>>>> +
>>>> +       vif = priv->vifs[0];
>>>> +       priv->vifs[0] = priv->vifs[1];
>>>> +       priv->vifs[1] = vif;
>>>> +       rtlvif = (struct rtl8xxxu_vif *)priv->vifs[1]->drv_priv;
>>>> +       rtlvif->port_num = 1;
>>>
>>> nit: Would it be better to swap port_num as well? Currently, port_num of vifs[0]
>>> will be set to 0 by caller, but not sure if further people could misuse this
>>> function.
>>
>> the main reason, I did not include setting port_num for priv->vifs[0], is that
>> priv->vifs[0] is a NULL pointer in the current way this function is called from
>> rtl8xxxu_add_interface().
>>
>> do you think it makes sense to add
>>
>> if (priv->vifs[0])
>>          rtlvif = (struct rtl8xxxu_vif *)priv->vifs[0]->drv_priv;
>>          rtlvif->port_num = 0;
>>
>> just for completeness sake, even though this code path will currently never get
>> executed?
>>
> 
> I missed that point. I just did focus on "switch", but actually this is
> "move" from port 0 to 1, right?

Yes, currently, the function is only used to move the STA mode interface from 0 
to 1 in order to make room for AP on 0.

I will leave this patch as is for v2. When the function is used in the future 
for a different scenario, the possibility of vifs[0] or vifs[1] being NULL needs 
to be thought through anyway and if necessary the setting of port_num = 0 can be 
added then as well.

> 
> As I know, two cases only work on port 0. One is AP mode that you are
> doing, and the other is PS in STA mode that isn't implemented by rtl8xxxxu.
> For AP mode, current implement moving port 0 to 1 is fine.
> 
> In the future, PS in STA mode could need to move port 1 to 0, because
> we may disconnect port 0 first and we want STA on port 1 entering PS.
> 
> But, I think we can defer this until we really need it.
> 
> 

Re: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent

[Ping-Ke Shih]

On Thu, 2023-12-21 at 11:54 +0100, Martin Kaistra wrote:
> 
> Am 21.12.23 um 09:25 schrieb Ping-Ke Shih:
> > On Wed, 2023-12-20 at 17:34 +0100, Martin Kaistra wrote:
> > > 
> > > Am 20.12.23 um 07:28 schrieb Ping-Ke Shih:
> > > > > -----Original Message-----
> > > > > From: Martin Kaistra <martin.kaistra@linutronix.de>
> > > > > Sent: Monday, December 18, 2023 10:37 PM
> > > > > To: linux-wireless@vger.kernel.org
> > > > > Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> > > > > <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej
> > > > > Siewior
> > > > > <bigeasy@linutronix.de>
> > > > > Subject: [PATCH 19/20] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
> > > > > 
> > > > > +
> > > > > +       vif = priv->vifs[0];
> > > > > +       priv->vifs[0] = priv->vifs[1];
> > > > > +       priv->vifs[1] = vif;
> > > > > +       rtlvif = (struct rtl8xxxu_vif *)priv->vifs[1]->drv_priv;
> > > > > +       rtlvif->port_num = 1;
> > > > 
> > > > nit: Would it be better to swap port_num as well? Currently, port_num of vifs[0]
> > > > will be set to 0 by caller, but not sure if further people could misuse this
> > > > function.
> > > 
> > > the main reason, I did not include setting port_num for priv->vifs[0], is that
> > > priv->vifs[0] is a NULL pointer in the current way this function is called from
> > > rtl8xxxu_add_interface().
> > > 
> > > do you think it makes sense to add
> > > 
> > > if (priv->vifs[0])
> > >          rtlvif = (struct rtl8xxxu_vif *)priv->vifs[0]->drv_priv;
> > >          rtlvif->port_num = 0;
> > > 
> > > just for completeness sake, even though this code path will currently never get
> > > executed?
> > > 
> > 
> > I missed that point. I just did focus on "switch", but actually this is
> > "move" from port 0 to 1, right?
> 
> Yes, currently, the function is only used to move the STA mode interface from 0
> to 1 in order to make room for AP on 0.
> 
> I will leave this patch as is for v2. When the function is used in the future
> for a different scenario, the possibility of vifs[0] or vifs[1] being NULL needs
> to be thought through anyway and if necessary the setting of port_num = 0 can be
> added then as well.
> 

Would you like to add a comment like above description to help people to
understand your thinking?

Anyway, this patch looks good to me. Please add my reviewed-by by v2.