diff mbox series

[RFC,06/14] wifi: rtl8xxxu: Allow creating interface in AP mode

Message ID 20230322171905.492855-7-martin.kaistra@linutronix.de
State Superseded
Headers show
Series wifi: rtl8xxxu: Add AP mode support for 8188f | expand

Commit Message

Martin Kaistra March 22, 2023, 5:18 p.m. UTC 
Use the sequence from the vendor driver for setting up the beacon
related registers.
Also set the MAC address register.

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

Comments

Ping-Ke Shih March 27, 2023, 1:39 a.m. UTC | #1 
> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Thursday, March 23, 2023 1:19 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: [RFC PATCH 06/14] wifi: rtl8xxxu: Allow creating interface in AP mode
> 
> Use the sequence from the vendor driver for setting up the beacon
> related registers.
> Also set the MAC address register.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> ---
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 31 ++++++++++++++++---
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h |  2 ++
>  2 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index b233c66a7a5a8..b20ff8bc40870 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6408,18 +6408,39 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>         int ret;
>         u8 val8;
> 
> +       if (!priv->vif)
> +               priv->vif = vif;
> +       else
> +               return -EOPNOTSUPP;
> +
>         switch (vif->type) {
>         case NL80211_IFTYPE_STATION:
> -               if (!priv->vif)
> -                       priv->vif = vif;
> -               else
> -                       return -EOPNOTSUPP;
>                 rtl8xxxu_stop_tx_beacon(priv);
> 
>                 val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
>                 val8 |= BEACON_ATIM | BEACON_FUNCTION_ENABLE |
>                         BEACON_DISABLE_TSF_UPDATE;
>                 rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
> +               ret = 0;
> +               break;
> +       case NL80211_IFTYPE_AP:
> +               rtl8xxxu_write8(priv, REG_BEACON_CTRL,
> +                               BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
> +               rtl8xxxu_write8(priv, REG_ATIMWND, 0x0c); /* 12ms */
> +               rtl8xxxu_write16(priv, REG_TSFTR_SYN_OFFSET, 0x7fff); /* ~32ms */
> +               rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, DUAL_TSF_RESET_TSF0);
> +
> +               /* enable BCN0 function */
> +               rtl8xxxu_write8(priv, REG_BEACON_CTRL,
> +                               BEACON_DISABLE_TSF_UPDATE |
> +                               BEACON_FUNCTION_ENABLE | BEACON_CTRL_MBSSID |
> +                               BEACON_CTRL_TX_BEACON_RPT);
> +
> +               /* select BCN on port 0 */
> +               val8 = rtl8xxxu_read8(priv, REG_CCK_CHECK);
> +               val8 &= ~BIT_BCN_PORT_SEL;
> +               rtl8xxxu_write8(priv, REG_CCK_CHECK, val8);
> +
>                 ret = 0;
>                 break;
>         default:
> @@ -6427,6 +6448,8 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>         }
> 
>         rtl8xxxu_set_linktype(priv, vif->type);
> +       ether_addr_copy(priv->mac_addr, vif->addr);
> +       rtl8xxxu_set_mac(priv);

rtl8xxxu_set_mac() is already called by rtl8xxxu_init_device(). Is there
anything unexpected? 

While I reviewed first patch, I would like to suggest to call calibration:
  fops->phy_lc_calibrate(priv);
  fops->phy_iq_calibrate(priv);
I traced rtl8xxxu and saw they present in rtl8xxxu_init_device() and rtl8xxxu
doesn't implement idle power saving to turn off power of wifi card. Then, I
think the calibration will be fine if rtl8xxxu only does it once.

So, I would like to know if something I miss.

> 
>         return ret;
>  }
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> index 4dffbab494c3b..83e7f8fd82c0a 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> @@ -572,6 +572,8 @@
>  #define REG_ARFR1                      0x0448
>  #define REG_ARFR2                      0x044c
>  #define REG_ARFR3                      0x0450
> +#define REG_CCK_CHECK                  0x0454
> +#define BIT_BCN_PORT_SEL               BIT(5)
>  #define REG_AMPDU_MAX_TIME_8723B       0x0456
>  #define REG_AGGLEN_LMT                 0x0458
>  #define REG_AMPDU_MIN_SPACE            0x045c
> --
> 2.30.2
Martin Kaistra March 27, 2023, 1:15 p.m. UTC | #2 
Am 27.03.23 um 03:39 schrieb Ping-Ke Shih:
> 
> 
>> -----Original Message-----
>> From: Martin Kaistra <martin.kaistra@linutronix.de>
>> Sent: Thursday, March 23, 2023 1:19 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: [RFC PATCH 06/14] wifi: rtl8xxxu: Allow creating interface in AP mode
>>
>> Use the sequence from the vendor driver for setting up the beacon
>> related registers.
>> Also set the MAC address register.
>>
>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>> ---
>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 31 ++++++++++++++++---
>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h |  2 ++
>>   2 files changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index b233c66a7a5a8..b20ff8bc40870 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -6408,18 +6408,39 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>>          int ret;
>>          u8 val8;
>>
>> +       if (!priv->vif)
>> +               priv->vif = vif;
>> +       else
>> +               return -EOPNOTSUPP;
>> +
>>          switch (vif->type) {
>>          case NL80211_IFTYPE_STATION:
>> -               if (!priv->vif)
>> -                       priv->vif = vif;
>> -               else
>> -                       return -EOPNOTSUPP;
>>                  rtl8xxxu_stop_tx_beacon(priv);
>>
>>                  val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
>>                  val8 |= BEACON_ATIM | BEACON_FUNCTION_ENABLE |
>>                          BEACON_DISABLE_TSF_UPDATE;
>>                  rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>> +               ret = 0;
>> +               break;
>> +       case NL80211_IFTYPE_AP:
>> +               rtl8xxxu_write8(priv, REG_BEACON_CTRL,
>> +                               BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
>> +               rtl8xxxu_write8(priv, REG_ATIMWND, 0x0c); /* 12ms */
>> +               rtl8xxxu_write16(priv, REG_TSFTR_SYN_OFFSET, 0x7fff); /* ~32ms */
>> +               rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, DUAL_TSF_RESET_TSF0);
>> +
>> +               /* enable BCN0 function */
>> +               rtl8xxxu_write8(priv, REG_BEACON_CTRL,
>> +                               BEACON_DISABLE_TSF_UPDATE |
>> +                               BEACON_FUNCTION_ENABLE | BEACON_CTRL_MBSSID |
>> +                               BEACON_CTRL_TX_BEACON_RPT);
>> +
>> +               /* select BCN on port 0 */
>> +               val8 = rtl8xxxu_read8(priv, REG_CCK_CHECK);
>> +               val8 &= ~BIT_BCN_PORT_SEL;
>> +               rtl8xxxu_write8(priv, REG_CCK_CHECK, val8);
>> +
>>                  ret = 0;
>>                  break;
>>          default:
>> @@ -6427,6 +6448,8 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>>          }
>>
>>          rtl8xxxu_set_linktype(priv, vif->type);
>> +       ether_addr_copy(priv->mac_addr, vif->addr);
>> +       rtl8xxxu_set_mac(priv);
> 
> rtl8xxxu_set_mac() is already called by rtl8xxxu_init_device(). Is there
> anything unexpected?

If the hostapd config has the option "bssid" set, this will be set as 
the MAC address for the interface in AP mode. If the MAC address 
registers are not updated, then I don't see any ack frames being send.

The same could probably happen if the MAC address is set via
ip link set dev <wlan_name> address <MAC address>

Maybe the call to rtl8xxxu_set_mac() in rtl8xxxu_init_device() can be 
removed, if it is also called here.

> 
> While I reviewed first patch, I would like to suggest to call calibration:
>    fops->phy_lc_calibrate(priv);
>    fops->phy_iq_calibrate(priv);
> I traced rtl8xxxu and saw they present in rtl8xxxu_init_device() and rtl8xxxu
> doesn't implement idle power saving to turn off power of wifi card. Then, I
> think the calibration will be fine if rtl8xxxu only does it once.
> 
> So, I would like to know if something I miss.
>
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 b233c66a7a5a8..b20ff8bc40870 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6408,18 +6408,39 @@  static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 	int ret;
 	u8 val8;
 
+	if (!priv->vif)
+		priv->vif = vif;
+	else
+		return -EOPNOTSUPP;
+
 	switch (vif->type) {
 	case NL80211_IFTYPE_STATION:
-		if (!priv->vif)
-			priv->vif = vif;
-		else
-			return -EOPNOTSUPP;
 		rtl8xxxu_stop_tx_beacon(priv);
 
 		val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
 		val8 |= BEACON_ATIM | BEACON_FUNCTION_ENABLE |
 			BEACON_DISABLE_TSF_UPDATE;
 		rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
+		ret = 0;
+		break;
+	case NL80211_IFTYPE_AP:
+		rtl8xxxu_write8(priv, REG_BEACON_CTRL,
+				BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
+		rtl8xxxu_write8(priv, REG_ATIMWND, 0x0c); /* 12ms */
+		rtl8xxxu_write16(priv, REG_TSFTR_SYN_OFFSET, 0x7fff); /* ~32ms */
+		rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, DUAL_TSF_RESET_TSF0);
+
+		/* enable BCN0 function */
+		rtl8xxxu_write8(priv, REG_BEACON_CTRL,
+				BEACON_DISABLE_TSF_UPDATE |
+				BEACON_FUNCTION_ENABLE | BEACON_CTRL_MBSSID |
+				BEACON_CTRL_TX_BEACON_RPT);
+
+		/* select BCN on port 0 */
+		val8 = rtl8xxxu_read8(priv, REG_CCK_CHECK);
+		val8 &= ~BIT_BCN_PORT_SEL;
+		rtl8xxxu_write8(priv, REG_CCK_CHECK, val8);
+
 		ret = 0;
 		break;
 	default:
@@ -6427,6 +6448,8 @@  static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 	}
 
 	rtl8xxxu_set_linktype(priv, vif->type);
+	ether_addr_copy(priv->mac_addr, vif->addr);
+	rtl8xxxu_set_mac(priv);
 
 	return ret;
 }
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
index 4dffbab494c3b..83e7f8fd82c0a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
@@ -572,6 +572,8 @@ 
 #define REG_ARFR1			0x0448
 #define REG_ARFR2			0x044c
 #define REG_ARFR3			0x0450
+#define REG_CCK_CHECK			0x0454
+#define BIT_BCN_PORT_SEL		BIT(5)
 #define REG_AMPDU_MAX_TIME_8723B	0x0456
 #define REG_AGGLEN_LMT			0x0458
 #define REG_AMPDU_MIN_SPACE		0x045c