diff mbox series

[v2,20/21] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent

Message ID 20231221164353.603258-21-martin.kaistra@linutronix.de
State Superseded
Headers show
Series wifi: rtl8xxxu: Add concurrent mode for 8188f | expand

Commit Message

Martin Kaistra Dec. 21, 2023, 4:43 p.m. UTC 
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>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 92 ++++++++++++++++++-
 1 file changed, 90 insertions(+), 2 deletions(-)

Comments

Ping-Ke Shih Dec. 22, 2023, 1:54 a.m. UTC | #1 
> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Friday, December 22, 2023 12:44 AM
> 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 v2 20/21] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
> 

[...]

> +
> +       /*
> +        * priv->vifs[0] is NULL here, based on how this function is currently
> +        * called from rtl8xxxu_add_interface().
> +        * When this function will be used in the future for a different
> +        * scenario, please check whether vifs[0] or vifs[1] can be NULL and if
> +        * necessary add code to set port_num = 1.
> +        */

Did you run scripts/checkpatch.pl to this patch? Initial line of comment block
for networking code should not empty, so it should be below: 

+       /* priv->vifs[0] is NULL here, based on how this function is currently
+        * called from rtl8xxxu_add_interface().
+        * When this function will be used in the future for a different
+        * scenario, please check whether vifs[0] or vifs[1] can be NULL and if
+        * necessary add code to set port_num = 1.
+        */
Martin Kaistra Dec. 22, 2023, 7:49 a.m. UTC | #2 
Am 22.12.23 um 02:54 schrieb Ping-Ke Shih:
> 
> 
>> -----Original Message-----
>> From: Martin Kaistra <martin.kaistra@linutronix.de>
>> Sent: Friday, December 22, 2023 12:44 AM
>> 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 v2 20/21] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
>>
> 
> [...]
> 
>> +
>> +       /*
>> +        * priv->vifs[0] is NULL here, based on how this function is currently
>> +        * called from rtl8xxxu_add_interface().
>> +        * When this function will be used in the future for a different
>> +        * scenario, please check whether vifs[0] or vifs[1] can be NULL and if
>> +        * necessary add code to set port_num = 1.
>> +        */
> 
> Did you run scripts/checkpatch.pl to this patch? Initial line of comment block
> for networking code should not empty, so it should be below:
> 
> +       /* priv->vifs[0] is NULL here, based on how this function is currently
> +        * called from rtl8xxxu_add_interface().
> +        * When this function will be used in the future for a different
> +        * scenario, please check whether vifs[0] or vifs[1] can be NULL and if
> +        * necessary add code to set port_num = 1.
> +        */
> 

I did run checkpatch.pl and chose to ignore this warning because all other 
multiline comments in the rtl8xxxu driver also have this initial empty line.

Do you still want me to change it?
Ping-Ke Shih Dec. 22, 2023, 7:54 a.m. UTC | #3 
> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Friday, December 22, 2023 3:49 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Bitterblue Smith
> <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Subject: Re: [PATCH v2 20/21] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
> 
> Am 22.12.23 um 02:54 schrieb Ping-Ke Shih:
> >
> >
> >> -----Original Message-----
> >> From: Martin Kaistra <martin.kaistra@linutronix.de>
> >> Sent: Friday, December 22, 2023 12:44 AM
> >> 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 v2 20/21] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
> >>
> >
> > [...]
> >
> >> +
> >> +       /*
> >> +        * priv->vifs[0] is NULL here, based on how this function is currently
> >> +        * called from rtl8xxxu_add_interface().
> >> +        * When this function will be used in the future for a different
> >> +        * scenario, please check whether vifs[0] or vifs[1] can be NULL and if
> >> +        * necessary add code to set port_num = 1.
> >> +        */
> >
> > Did you run scripts/checkpatch.pl to this patch? Initial line of comment block
> > for networking code should not empty, so it should be below:
> >
> > +       /* priv->vifs[0] is NULL here, based on how this function is currently
> > +        * called from rtl8xxxu_add_interface().
> > +        * When this function will be used in the future for a different
> > +        * scenario, please check whether vifs[0] or vifs[1] can be NULL and if
> > +        * necessary add code to set port_num = 1.
> > +        */
> >
> 
> I did run checkpatch.pl and chose to ignore this warning because all other
> multiline comments in the rtl8xxxu driver also have this initial empty line.
> 
> Do you still want me to change it?

Personally, I would fix all checkpatch.pl warnings. If this change doesn't
bother you too much, I suggest to follow the rule.
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 7aafae9fe76b8..46b22d07c1ca8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6624,6 +6624,92 @@  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;
+
+	/*
+	 * priv->vifs[0] is NULL here, based on how this function is currently
+	 * called from rtl8xxxu_add_interface().
+	 * When this function will be used in the future for a different
+	 * scenario, please check whether vifs[0] or vifs[1] can be NULL and if
+	 * necessary add code to set port_num = 1.
+	 */
+	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)
 {
@@ -6651,8 +6737,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);