[PATCH v2 0/6] wifi: rtw89: handle EHT rate

[PATCH v2 0/6] wifi: rtw89: handle EHT rate

[Ping-Ke Shih]

We have parsed new formats used by WiFi 7 chips before, and this patchset
is to handle EHT rate. The handlers contain
 - RX rate
   * from RX descriptor, which is also used to find corresponding PPDU
     status packet
 - TX rate
   * RA (Rate adaptive) mask to tell firmware selectable rates
   * RA report tells driver the rate selected by firmware
 - debugfs
   * show TX/RX rate above
 - monitor mode
   * add enumerators of radiotap bandwidth of EHT U-SIG
   * add EHT radiotap if working on monitor mode

v2:
  - correct commit message of patch 5/6 about the source of bandwidth
    definition of EHT U-SIG suggested by Johannes

Ping-Ke Shih (6):
  wifi: rtw89: parse EHT information from RX descriptor and PPDU status
    packet
  wifi: rtw89: Add EHT rate mask as parameters of RA H2C command
  wifi: rtw89: parse TX EHT rate selected by firmware from RA C2H report
  wifi: rtw89: show EHT rate in debugfs
  wifi: radiotap: add bandwidth definition of EHT U-SIG
  wifi: rtw89: add EHT radiotap in monitor mode

 drivers/net/wireless/realtek/rtw89/core.c  | 125 ++++++++++++++++++---
 drivers/net/wireless/realtek/rtw89/core.h  |  11 +-
 drivers/net/wireless/realtek/rtw89/debug.c |  14 +++
 drivers/net/wireless/realtek/rtw89/phy.c   |  70 +++++++++++-
 drivers/net/wireless/realtek/rtw89/phy.h   |   5 +
 include/net/ieee80211_radiotap.h           |   6 +
 6 files changed, 214 insertions(+), 17 deletions(-)

-- 
2.25.1

[PATCH v2 1/6] wifi: rtw89: parse EHT information from RX descriptor and PPDU status packet

[Ping-Ke Shih]

There are two kinds of RX packets -- normal and its PPDU status packet.
Both have RX descriptor containing some information such as rate, GI and
bandwidth, and we use these information to find the relationship between
two kinds of packets. Then, we can get more information like RSSI and EVM
from PPDU status packet.

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/core.c | 59 +++++++++++++++++------
 1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
index cca18d7ea1dd..2742e6646cf1 100644
--- a/drivers/net/wireless/realtek/rtw89/core.c
+++ b/drivers/net/wireless/realtek/rtw89/core.c
@@ -1621,32 +1621,49 @@ static void rtw89_core_rx_process_phy_sts(struct rtw89_dev *rtwdev,
 					  phy_ppdu);
 }
 
-static u8 rtw89_rxdesc_to_nl_he_gi(struct rtw89_dev *rtwdev,
-				   const struct rtw89_rx_desc_info *desc_info,
-				   bool rx_status)
+static u8 rtw89_rxdesc_to_nl_he_eht_gi(struct rtw89_dev *rtwdev,
+				       u8 desc_info_gi,
+				       bool rx_status, bool eht)
 {
-	switch (desc_info->gi_ltf) {
+	switch (desc_info_gi) {
 	case RTW89_GILTF_SGI_4XHE08:
 	case RTW89_GILTF_2XHE08:
 	case RTW89_GILTF_1XHE08:
-		return NL80211_RATE_INFO_HE_GI_0_8;
+		return eht ? NL80211_RATE_INFO_EHT_GI_0_8 :
+			     NL80211_RATE_INFO_HE_GI_0_8;
 	case RTW89_GILTF_2XHE16:
 	case RTW89_GILTF_1XHE16:
-		return NL80211_RATE_INFO_HE_GI_1_6;
+		return eht ? NL80211_RATE_INFO_EHT_GI_1_6 :
+			     NL80211_RATE_INFO_HE_GI_1_6;
 	case RTW89_GILTF_LGI_4XHE32:
-		return NL80211_RATE_INFO_HE_GI_3_2;
+		return eht ? NL80211_RATE_INFO_EHT_GI_3_2 :
+			     NL80211_RATE_INFO_HE_GI_3_2;
 	default:
-		rtw89_warn(rtwdev, "invalid gi_ltf=%d", desc_info->gi_ltf);
-		return rx_status ? NL80211_RATE_INFO_HE_GI_3_2 : U8_MAX;
+		rtw89_warn(rtwdev, "invalid gi_ltf=%d", desc_info_gi);
+		if (rx_status)
+			return eht ? NL80211_RATE_INFO_EHT_GI_3_2 :
+				     NL80211_RATE_INFO_HE_GI_3_2;
+		return U8_MAX;
 	}
 }
 
+static
+bool rtw89_check_rx_statu_gi_match(struct ieee80211_rx_status *status, u8 gi_ltf,
+				   bool eht)
+{
+	if (eht)
+		return status->eht.gi == gi_ltf;
+
+	return status->he_gi == gi_ltf;
+}
+
 static bool rtw89_core_rx_ppdu_match(struct rtw89_dev *rtwdev,
 				     struct rtw89_rx_desc_info *desc_info,
 				     struct ieee80211_rx_status *status)
 {
 	u8 band = desc_info->bb_sel ? RTW89_PHY_1 : RTW89_PHY_0;
 	u8 data_rate_mode, bw, rate_idx = MASKBYTE0, gi_ltf;
+	bool eht = false;
 	u16 data_rate;
 	bool ret;
 
@@ -1657,19 +1674,20 @@ static bool rtw89_core_rx_ppdu_match(struct rtw89_dev *rtwdev,
 		/* rate_idx is still hardware value here */
 	} else if (data_rate_mode == DATA_RATE_MODE_HT) {
 		rate_idx = rtw89_get_data_ht_mcs(rtwdev, data_rate);
-	} else if (data_rate_mode == DATA_RATE_MODE_VHT) {
-		rate_idx = rtw89_get_data_mcs(rtwdev, data_rate);
-	} else if (data_rate_mode == DATA_RATE_MODE_HE) {
+	} else if (data_rate_mode == DATA_RATE_MODE_VHT ||
+		   data_rate_mode == DATA_RATE_MODE_HE ||
+		   data_rate_mode == DATA_RATE_MODE_EHT) {
 		rate_idx = rtw89_get_data_mcs(rtwdev, data_rate);
 	} else {
 		rtw89_warn(rtwdev, "invalid RX rate mode %d\n", data_rate_mode);
 	}
 
+	eht = data_rate_mode == DATA_RATE_MODE_EHT;
 	bw = rtw89_hw_to_rate_info_bw(desc_info->bw);
-	gi_ltf = rtw89_rxdesc_to_nl_he_gi(rtwdev, desc_info, false);
+	gi_ltf = rtw89_rxdesc_to_nl_he_eht_gi(rtwdev, desc_info->gi_ltf, false, eht);
 	ret = rtwdev->ppdu_sts.curr_rx_ppdu_cnt[band] == desc_info->ppdu_cnt &&
 	      status->rate_idx == rate_idx &&
-	      status->he_gi == gi_ltf &&
+	      rtw89_check_rx_statu_gi_match(status, gi_ltf, eht) &&
 	      status->bw == bw;
 
 	return ret;
@@ -2168,6 +2186,8 @@ static void rtw89_core_update_rx_status(struct rtw89_dev *rtwdev,
 		rtw89_chandef_get(rtwdev, RTW89_SUB_ENTITY_0);
 	u16 data_rate;
 	u8 data_rate_mode;
+	bool eht = false;
+	u8 gi;
 
 	/* currently using single PHY */
 	rx_status->freq = chandef->chan->center_freq;
@@ -2215,12 +2235,21 @@ static void rtw89_core_update_rx_status(struct rtw89_dev *rtwdev,
 		rx_status->encoding = RX_ENC_HE;
 		rx_status->rate_idx = rtw89_get_data_mcs(rtwdev, data_rate);
 		rx_status->nss = rtw89_get_data_nss(rtwdev, data_rate) + 1;
+	} else if (data_rate_mode == DATA_RATE_MODE_EHT) {
+		rx_status->encoding = RX_ENC_EHT;
+		rx_status->rate_idx = rtw89_get_data_mcs(rtwdev, data_rate);
+		rx_status->nss = rtw89_get_data_nss(rtwdev, data_rate) + 1;
+		eht = true;
 	} else {
 		rtw89_warn(rtwdev, "invalid RX rate mode %d\n", data_rate_mode);
 	}
 
 	/* he_gi is used to match ppdu, so we always fill it. */
-	rx_status->he_gi = rtw89_rxdesc_to_nl_he_gi(rtwdev, desc_info, true);
+	gi = rtw89_rxdesc_to_nl_he_eht_gi(rtwdev, desc_info->gi_ltf, true, eht);
+	if (eht)
+		rx_status->eht.gi = gi;
+	else
+		rx_status->he_gi = gi;
 	rx_status->flag |= RX_FLAG_MACTIME_START;
 	rx_status->mactime = desc_info->free_run_cnt;
 
-- 
2.25.1

[PATCH v2 2/6] wifi: rtw89: Add EHT rate mask as parameters of RA H2C command

[Ping-Ke Shih]

Set EHT rate mask to RA (rate adaptive) H2C command according to handshake
result. The EHT rate mask format looks like

  44               28               12        4    0
   +----------------+----------------+--------+----+
   |  EHT 2SS rate  |  EHT 1SS rate  |  OFDM  | CCK|
   +----------------+----------------+--------+----+

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/core.h |  1 +
 drivers/net/wireless/realtek/rtw89/phy.c  | 58 ++++++++++++++++++++++-
 drivers/net/wireless/realtek/rtw89/phy.h  |  5 ++
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
index 5bf18110b379..f103ac085d2b 100644
--- a/drivers/net/wireless/realtek/rtw89/core.h
+++ b/drivers/net/wireless/realtek/rtw89/core.h
@@ -2734,6 +2734,7 @@ enum rtw89_ra_mode {
 	RTW89_RA_MODE_HT = BIT(2),
 	RTW89_RA_MODE_VHT = BIT(3),
 	RTW89_RA_MODE_HE = BIT(4),
+	RTW89_RA_MODE_EHT = BIT(5),
 };
 
 enum rtw89_ra_report_mode {
diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c
index d04eaf7c5500..2a3edf775668 100644
--- a/drivers/net/wireless/realtek/rtw89/phy.c
+++ b/drivers/net/wireless/realtek/rtw89/phy.c
@@ -88,6 +88,55 @@ static u64 get_he_ra_mask(struct ieee80211_sta *sta)
 	return get_mcs_ra_mask(mcs_map, 11, 2);
 }
 
+static u64 get_eht_mcs_ra_mask(u8 *max_nss, u8 start_mcs, u8 n_nss)
+{
+	u64 nss_mcs_shift;
+	u64 nss_mcs_val;
+	u64 mask = 0;
+	int i, j;
+	u8 nss;
+
+	for (i = 0; i < n_nss; i++) {
+		nss = u8_get_bits(max_nss[i], IEEE80211_EHT_MCS_NSS_RX);
+		if (!nss)
+			continue;
+
+		nss_mcs_val = GENMASK_ULL(start_mcs + i * 2, 0);
+
+		for (j = 0, nss_mcs_shift = 12; j < nss; j++, nss_mcs_shift += 16)
+			mask |= nss_mcs_val << nss_mcs_shift;
+	}
+
+	return mask;
+}
+
+static u64 get_eht_ra_mask(struct ieee80211_sta *sta)
+{
+	struct ieee80211_sta_eht_cap *eht_cap = &sta->deflink.eht_cap;
+	struct ieee80211_eht_mcs_nss_supp_20mhz_only *mcs_nss_20mhz;
+	struct ieee80211_eht_mcs_nss_supp_bw *mcs_nss;
+
+	switch (sta->deflink.bandwidth) {
+	case IEEE80211_STA_RX_BW_320:
+		mcs_nss = &eht_cap->eht_mcs_nss_supp.bw._320;
+		/* MCS 9, 11, 13 */
+		return get_eht_mcs_ra_mask(mcs_nss->rx_tx_max_nss, 9, 3);
+	case IEEE80211_STA_RX_BW_160:
+		mcs_nss = &eht_cap->eht_mcs_nss_supp.bw._160;
+		/* MCS 9, 11, 13 */
+		return get_eht_mcs_ra_mask(mcs_nss->rx_tx_max_nss, 9, 3);
+	case IEEE80211_STA_RX_BW_80:
+	default:
+		mcs_nss = &eht_cap->eht_mcs_nss_supp.bw._80;
+		/* MCS 9, 11, 13 */
+		return get_eht_mcs_ra_mask(mcs_nss->rx_tx_max_nss, 9, 3);
+	case IEEE80211_STA_RX_BW_20:
+		mcs_nss_20mhz = &eht_cap->eht_mcs_nss_supp.only_20mhz;
+		/* MCS 7, 9, 11, 13 */
+		return get_eht_mcs_ra_mask(mcs_nss_20mhz->rx_tx_max_nss, 7, 4);
+	}
+}
+
 #define RA_FLOOR_TABLE_SIZE	7
 #define RA_FLOOR_UP_GAP		3
 static u64 rtw89_phy_ra_mask_rssi(struct rtw89_dev *rtwdev, u8 rssi,
@@ -194,6 +243,9 @@ rtw89_ra_mask_vht_rates[4] = {RA_MASK_VHT_1SS_RATES, RA_MASK_VHT_2SS_RATES,
 static const u64
 rtw89_ra_mask_he_rates[4] = {RA_MASK_HE_1SS_RATES, RA_MASK_HE_2SS_RATES,
 			     RA_MASK_HE_3SS_RATES, RA_MASK_HE_4SS_RATES};
+static const u64
+rtw89_ra_mask_eht_rates[4] = {RA_MASK_EHT_1SS_RATES, RA_MASK_EHT_2SS_RATES,
+			      RA_MASK_EHT_3SS_RATES, RA_MASK_EHT_4SS_RATES};
 
 static void rtw89_phy_ra_gi_ltf(struct rtw89_dev *rtwdev,
 				struct rtw89_sta *rtwsta,
@@ -255,7 +307,11 @@ static void rtw89_phy_ra_sta_update(struct rtw89_dev *rtwdev,
 
 	memset(ra, 0, sizeof(*ra));
 	/* Set the ra mask from sta's capability */
-	if (sta->deflink.he_cap.has_he) {
+	if (sta->deflink.eht_cap.has_eht) {
+		mode |= RTW89_RA_MODE_EHT;
+		ra_mask |= get_eht_ra_mask(sta);
+		high_rate_masks = rtw89_ra_mask_eht_rates;
+	} else if (sta->deflink.he_cap.has_he) {
 		mode |= RTW89_RA_MODE_HE;
 		csi_mode = RTW89_RA_RPT_MODE_HE;
 		ra_mask |= get_he_ra_mask(sta);
diff --git a/drivers/net/wireless/realtek/rtw89/phy.h b/drivers/net/wireless/realtek/rtw89/phy.h
index 9473798b9dac..02521d984c9b 100644
--- a/drivers/net/wireless/realtek/rtw89/phy.h
+++ b/drivers/net/wireless/realtek/rtw89/phy.h
@@ -46,6 +46,11 @@
 #define RA_MASK_HE_3SS_RATES	GENMASK_ULL(47, 36)
 #define RA_MASK_HE_4SS_RATES	GENMASK_ULL(59, 48)
 #define RA_MASK_HE_RATES	GENMASK_ULL(59, 12)
+#define RA_MASK_EHT_1SS_RATES	GENMASK_ULL(27, 12)
+#define RA_MASK_EHT_2SS_RATES	GENMASK_ULL(43, 28)
+#define RA_MASK_EHT_3SS_RATES	GENMASK_ULL(59, 44)
+#define RA_MASK_EHT_4SS_RATES	GENMASK_ULL(62, 60)
+#define RA_MASK_EHT_RATES	GENMASK_ULL(62, 12)
 
 #define CFO_TRK_ENABLE_TH (2 << 2)
 #define CFO_TRK_STOP_TH_4 (30 << 2)
-- 
2.25.1

[PATCH v2 3/6] wifi: rtw89: parse TX EHT rate selected by firmware from RA C2H report

[Ping-Ke Shih]

RA (rate adaptive) C2H report is to reflect current TX rate firmware is
using. Parse C2H event encoded in EHT mode, and then user space and debugfs
can use the information to know TX rate.

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/core.h |  1 +
 drivers/net/wireless/realtek/rtw89/phy.c  | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
index f103ac085d2b..8c0dfd73031e 100644
--- a/drivers/net/wireless/realtek/rtw89/core.h
+++ b/drivers/net/wireless/realtek/rtw89/core.h
@@ -2742,6 +2742,7 @@ enum rtw89_ra_report_mode {
 	RTW89_RA_RPT_MODE_HT,
 	RTW89_RA_RPT_MODE_VHT,
 	RTW89_RA_RPT_MODE_HE,
+	RTW89_RA_RPT_MODE_EHT,
 };
 
 enum rtw89_dig_noisy_level {
diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c
index 2a3edf775668..8a306a86f1f0 100644
--- a/drivers/net/wireless/realtek/rtw89/phy.c
+++ b/drivers/net/wireless/realtek/rtw89/phy.c
@@ -2399,6 +2399,18 @@ static void rtw89_phy_c2h_ra_rpt_iter(void *data, struct ieee80211_sta *sta)
 			ra_report->txrate.he_gi = NL80211_RATE_INFO_HE_GI_3_2;
 		mcs = ra_report->txrate.mcs;
 		break;
+	case RTW89_RA_RPT_MODE_EHT:
+		ra_report->txrate.flags |= RATE_INFO_FLAGS_EHT_MCS;
+		ra_report->txrate.mcs = u8_get_bits(rate, RTW89_RA_RATE_MASK_MCS_V1);
+		ra_report->txrate.nss = u8_get_bits(rate, RTW89_RA_RATE_MASK_NSS_V1) + 1;
+		if (giltf == RTW89_GILTF_2XHE08 || giltf == RTW89_GILTF_1XHE08)
+			ra_report->txrate.eht_gi = NL80211_RATE_INFO_EHT_GI_0_8;
+		else if (giltf == RTW89_GILTF_2XHE16 || giltf == RTW89_GILTF_1XHE16)
+			ra_report->txrate.eht_gi = NL80211_RATE_INFO_EHT_GI_1_6;
+		else
+			ra_report->txrate.eht_gi = NL80211_RATE_INFO_EHT_GI_3_2;
+		mcs = ra_report->txrate.mcs;
+		break;
 	}
 
 	ra_report->txrate.bw = rtw89_hw_to_rate_info_bw(bw);
-- 
2.25.1

[PATCH v2 4/6] wifi: rtw89: show EHT rate in debugfs

[Ping-Ke Shih]

Since we have TX rate from RA report of C2H event and RX rate from RX
descriptor, show them in debugfs like

  TX rate [1]: EHT 2SS MCS-7 GI:3.2 BW:80	(hw_rate=0x427)
  RX rate [1]: EHT 2SS MCS-7 GI:3.2 BW:80	(hw_rate=0x427)

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/debug.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw89/debug.c b/drivers/net/wireless/realtek/rtw89/debug.c
index 6990d3679bc0..a3f795d240ea 100644
--- a/drivers/net/wireless/realtek/rtw89/debug.c
+++ b/drivers/net/wireless/realtek/rtw89/debug.c
@@ -3467,6 +3467,11 @@ static void rtw89_sta_info_get_iter(void *data, struct ieee80211_sta *sta)
 		[NL80211_RATE_INFO_HE_GI_1_6] = "1.6",
 		[NL80211_RATE_INFO_HE_GI_3_2] = "3.2",
 	};
+	static const char * const eht_gi_str[] = {
+		[NL80211_RATE_INFO_EHT_GI_0_8] = "0.8",
+		[NL80211_RATE_INFO_EHT_GI_1_6] = "1.6",
+		[NL80211_RATE_INFO_EHT_GI_3_2] = "3.2",
+	};
 	struct rtw89_sta *rtwsta = (struct rtw89_sta *)sta->drv_priv;
 	struct rate_info *rate = &rtwsta->ra_report.txrate;
 	struct ieee80211_rx_status *status = &rtwsta->rx_status;
@@ -3492,6 +3497,10 @@ static void rtw89_sta_info_get_iter(void *data, struct ieee80211_sta *sta)
 		seq_printf(m, "HE %dSS MCS-%d GI:%s", rate->nss, rate->mcs,
 			   rate->he_gi <= NL80211_RATE_INFO_HE_GI_3_2 ?
 			   he_gi_str[rate->he_gi] : "N/A");
+	else if (rate->flags & RATE_INFO_FLAGS_EHT_MCS)
+		seq_printf(m, "EHT %dSS MCS-%d GI:%s", rate->nss, rate->mcs,
+			   rate->eht_gi < ARRAY_SIZE(eht_gi_str) ?
+			   eht_gi_str[rate->eht_gi] : "N/A");
 	else
 		seq_printf(m, "Legacy %d", rate->legacy);
 	seq_printf(m, "%s", rtwsta->ra_report.might_fallback_legacy ? " FB_G" : "");
@@ -3520,6 +3529,11 @@ static void rtw89_sta_info_get_iter(void *data, struct ieee80211_sta *sta)
 			   status->he_gi <= NL80211_RATE_INFO_HE_GI_3_2 ?
 			   he_gi_str[rate->he_gi] : "N/A");
 		break;
+	case RX_ENC_EHT:
+		seq_printf(m, "EHT %dSS MCS-%d GI:%s", status->nss, status->rate_idx,
+			   status->eht.gi < ARRAY_SIZE(eht_gi_str) ?
+			   eht_gi_str[status->eht.gi] : "N/A");
+		break;
 	}
 	seq_printf(m, " BW:%u", rtw89_rate_info_bw_to_mhz(status->bw));
 	seq_printf(m, "\t(hw_rate=0x%x)\n", rtwsta->rx_hw_rate);
-- 
2.25.1

[PATCH v2 5/6] wifi: radiotap: add bandwidth definition of EHT U-SIG

[Ping-Ke Shih]

Define EHT U-SIG bandwidth used by radiotap according to Table 36-28
"U-SIG field of an EHT MU PPDU" in 802.11be (D3.0).

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
v2:
  - correct commit message about the source of bandwidth definition
    of EHT U-SIG suggested by Johannes
---
 include/net/ieee80211_radiotap.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/net/ieee80211_radiotap.h b/include/net/ieee80211_radiotap.h
index 2338f8d2a8b3..925bac726a92 100644
--- a/include/net/ieee80211_radiotap.h
+++ b/include/net/ieee80211_radiotap.h
@@ -539,6 +539,12 @@ enum ieee80211_radiotap_eht_usig_common {
 	IEEE80211_RADIOTAP_EHT_USIG_COMMON_VALIDATE_BITS_OK	= 0x00000080,
 	IEEE80211_RADIOTAP_EHT_USIG_COMMON_PHY_VER		= 0x00007000,
 	IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW			= 0x00038000,
+		IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_20MHZ		= 0,
+		IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_40MHZ		= 1,
+		IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_80MHZ		= 2,
+		IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_160MHZ		= 3,
+		IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_320MHZ_1		= 4,
+		IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_320MHZ_2		= 5,
 	IEEE80211_RADIOTAP_EHT_USIG_COMMON_UL_DL		= 0x00040000,
 	IEEE80211_RADIOTAP_EHT_USIG_COMMON_BSS_COLOR		= 0x01f80000,
 	IEEE80211_RADIOTAP_EHT_USIG_COMMON_TXOP			= 0xfe000000,
-- 
2.25.1

Re: [PATCH v2 5/6] wifi: radiotap: add bandwidth definition of EHT U-SIG

[Kalle Valo]

Ping-Ke Shih <pkshih@realtek.com> writes:

> Define EHT U-SIG bandwidth used by radiotap according to Table 36-28
> "U-SIG field of an EHT MU PPDU" in 802.11be (D3.0).
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
> v2:
>   - correct commit message about the source of bandwidth definition
>     of EHT U-SIG suggested by Johannes
> ---
>  include/net/ieee80211_radiotap.h | 6 ++++++
>  1 file changed, 6 insertions(+)

This is nitpicking but in general it's good to have the patches touching
files outside of drivers/net/wireless in the beginning of the patchset
to make them more visible. No need to resend because of this, just a
small tip.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

RE: [PATCH v2 5/6] wifi: radiotap: add bandwidth definition of EHT U-SIG

[Ping-Ke Shih]

> -----Original Message-----
> From: Kalle Valo <kvalo@kernel.org>
> Sent: Tuesday, October 10, 2023 2:20 PM
> To: Ping-Ke Shih <pkshih@realtek.com>
> Cc: johannes@sipsolutions.net; linux-wireless@vger.kernel.org
> Subject: Re: [PATCH v2 5/6] wifi: radiotap: add bandwidth definition of EHT U-SIG
> 
> Ping-Ke Shih <pkshih@realtek.com> writes:
> 
> > Define EHT U-SIG bandwidth used by radiotap according to Table 36-28
> > "U-SIG field of an EHT MU PPDU" in 802.11be (D3.0).
> >
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> > ---
> > v2:
> >   - correct commit message about the source of bandwidth definition
> >     of EHT U-SIG suggested by Johannes
> > ---
> >  include/net/ieee80211_radiotap.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> 
> This is nitpicking but in general it's good to have the patches touching
> files outside of drivers/net/wireless in the beginning of the patchset
> to make them more visible. No need to resend because of this, just a
> small tip.
> 

Got it. I will pay attention to it next time. 

Ping-Ke

[PATCH v2 6/6] wifi: rtw89: add EHT radiotap in monitor mode

[Ping-Ke Shih]

Add IEEE80211_RADIOTAP_EHT and IEEE80211_RADIOTAP_EHT_USIG radiotap to
fill basic EHT NSS, MCS, GI and bandwidth.

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/core.c | 66 +++++++++++++++++++++++
 drivers/net/wireless/realtek/rtw89/core.h |  9 +++-
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
index 2742e6646cf1..8cb1715d049a 100644
--- a/drivers/net/wireless/realtek/rtw89/core.c
+++ b/drivers/net/wireless/realtek/rtw89/core.c
@@ -1907,6 +1907,70 @@ static void rtw89_core_hw_to_sband_rate(struct ieee80211_rx_status *rx_status)
 	rx_status->rate_idx -= 4;
 }
 
+static u8 rx_status_bw_to_radiotap_eht_usig[] = {
+	[RATE_INFO_BW_20] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_20MHZ,
+	[RATE_INFO_BW_5] = U8_MAX,
+	[RATE_INFO_BW_10] = U8_MAX,
+	[RATE_INFO_BW_40] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_40MHZ,
+	[RATE_INFO_BW_80] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_80MHZ,
+	[RATE_INFO_BW_160] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_160MHZ,
+	[RATE_INFO_BW_HE_RU] = U8_MAX,
+	[RATE_INFO_BW_320] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_320MHZ_1,
+	[RATE_INFO_BW_EHT_RU] = U8_MAX,
+};
+
+static void rtw89_core_update_radiotap_eht(struct rtw89_dev *rtwdev,
+					   struct sk_buff *skb,
+					   struct ieee80211_rx_status *rx_status)
+{
+	struct ieee80211_radiotap_eht_usig *usig;
+	struct ieee80211_radiotap_eht *eht;
+	struct ieee80211_radiotap_tlv *tlv;
+	int eht_len = struct_size(eht, user_info, 1);
+	int usig_len = sizeof(*usig);
+	int len;
+	u8 bw;
+
+	len = sizeof(*tlv) + ALIGN(eht_len, 4) +
+	      sizeof(*tlv) + ALIGN(usig_len, 4);
+
+	rx_status->flag |= RX_FLAG_RADIOTAP_TLV_AT_END;
+	skb_reset_mac_header(skb);
+
+	/* EHT */
+	tlv = skb_push(skb, len);
+	memset(tlv, 0, len);
+	tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT);
+	tlv->len = cpu_to_le16(eht_len);
+
+	eht = (struct ieee80211_radiotap_eht *)tlv->data;
+	eht->known = cpu_to_le32(IEEE80211_RADIOTAP_EHT_KNOWN_GI);
+	eht->data[0] =
+		le32_encode_bits(rx_status->eht.gi, IEEE80211_RADIOTAP_EHT_DATA0_GI);
+
+	eht->user_info[0] =
+		cpu_to_le32(IEEE80211_RADIOTAP_EHT_USER_INFO_MCS_KNOWN |
+			    IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_KNOWN_O);
+	eht->user_info[0] |=
+		le32_encode_bits(rx_status->rate_idx, IEEE80211_RADIOTAP_EHT_USER_INFO_MCS) |
+		le32_encode_bits(rx_status->nss, IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_O);
+
+	/* U-SIG */
+	tlv = (void *)tlv + sizeof(*tlv) + ALIGN(eht_len, 4);
+	tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT_USIG);
+	tlv->len = cpu_to_le16(usig_len);
+
+	bw = rx_status->bw < ARRAY_SIZE(rx_status_bw_to_radiotap_eht_usig) ?
+	     rx_status_bw_to_radiotap_eht_usig[rx_status->bw] : U8_MAX;
+	if (bw == U8_MAX)
+		return;
+
+	usig = (struct ieee80211_radiotap_eht_usig *)tlv->data;
+	usig->common =
+		le32_encode_bits(1, IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_KNOWN) |
+		le32_encode_bits(bw, IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW);
+}
+
 static void rtw89_core_update_radiotap(struct rtw89_dev *rtwdev,
 				       struct sk_buff *skb,
 				       struct ieee80211_rx_status *rx_status)
@@ -1925,6 +1989,8 @@ static void rtw89_core_update_radiotap(struct rtw89_dev *rtwdev,
 		rx_status->flag |= RX_FLAG_RADIOTAP_HE;
 		he = skb_push(skb, sizeof(*he));
 		*he = known_he;
+	} else if (rx_status->encoding == RX_ENC_EHT) {
+		rtw89_core_update_radiotap_eht(rtwdev, skb, rx_status);
 	}
 }
 
diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
index 8c0dfd73031e..d5272a82ff8b 100644
--- a/drivers/net/wireless/realtek/rtw89/core.h
+++ b/drivers/net/wireless/realtek/rtw89/core.h
@@ -37,7 +37,14 @@ extern const struct ieee80211_ops rtw89_ops;
 #define RSSI_FACTOR 1
 #define RTW89_RSSI_RAW_TO_DBM(rssi) ((s8)((rssi) >> RSSI_FACTOR) - MAX_RSSI)
 #define RTW89_TX_DIV_RSSI_RAW_TH (2 << RSSI_FACTOR)
-#define RTW89_RADIOTAP_ROOM ALIGN(sizeof(struct ieee80211_radiotap_he), 64)
+#define RTW89_RADIOTAP_ROOM_HE sizeof(struct ieee80211_radiotap_he)
+#define RTW89_RADIOTAP_ROOM_EHT \
+	(sizeof(struct ieee80211_radiotap_tlv) + \
+	 ALIGN(struct_size((struct ieee80211_radiotap_eht *)0, user_info, 1), 4) + \
+	 sizeof(struct ieee80211_radiotap_tlv) + \
+	 ALIGN(sizeof(struct ieee80211_radiotap_eht_usig), 4))
+#define RTW89_RADIOTAP_ROOM \
+	ALIGN(max(RTW89_RADIOTAP_ROOM_HE, RTW89_RADIOTAP_ROOM_EHT), 64)
 
 #define RTW89_HTC_MASK_VARIANT GENMASK(1, 0)
 #define RTW89_HTC_VARIANT_HE 3
-- 
2.25.1

Re: [PATCH v2 6/6] wifi: rtw89: add EHT radiotap in monitor mode

[Kalle Valo]

Ping-Ke Shih <pkshih@realtek.com> writes:

> Add IEEE80211_RADIOTAP_EHT and IEEE80211_RADIOTAP_EHT_USIG radiotap to
> fill basic EHT NSS, MCS, GI and bandwidth.
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
>  drivers/net/wireless/realtek/rtw89/core.c | 66 +++++++++++++++++++++++
>  drivers/net/wireless/realtek/rtw89/core.h |  9 +++-
>  2 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
> index 2742e6646cf1..8cb1715d049a 100644
> --- a/drivers/net/wireless/realtek/rtw89/core.c
> +++ b/drivers/net/wireless/realtek/rtw89/core.c
> @@ -1907,6 +1907,70 @@ static void rtw89_core_hw_to_sband_rate(struct ieee80211_rx_status *rx_status)
>  	rx_status->rate_idx -= 4;
>  }
>  
> +static u8 rx_status_bw_to_radiotap_eht_usig[] = {
> +	[RATE_INFO_BW_20] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_20MHZ,
> +	[RATE_INFO_BW_5] = U8_MAX,
> +	[RATE_INFO_BW_10] = U8_MAX,
> +	[RATE_INFO_BW_40] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_40MHZ,
> +	[RATE_INFO_BW_80] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_80MHZ,
> +	[RATE_INFO_BW_160] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_160MHZ,
> +	[RATE_INFO_BW_HE_RU] = U8_MAX,
> +	[RATE_INFO_BW_320] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_320MHZ_1,
> +	[RATE_INFO_BW_EHT_RU] = U8_MAX,
> +};

Sorry, I noticed this only when I was abot to commit this. Should this
be static const?

> +static void rtw89_core_update_radiotap_eht(struct rtw89_dev *rtwdev,
> +					   struct sk_buff *skb,
> +					   struct ieee80211_rx_status *rx_status)
> +{
> +	struct ieee80211_radiotap_eht_usig *usig;
> +	struct ieee80211_radiotap_eht *eht;
> +	struct ieee80211_radiotap_tlv *tlv;
> +	int eht_len = struct_size(eht, user_info, 1);
> +	int usig_len = sizeof(*usig);
> +	int len;
> +	u8 bw;
> +
> +	len = sizeof(*tlv) + ALIGN(eht_len, 4) +
> +	      sizeof(*tlv) + ALIGN(usig_len, 4);
> +
> +	rx_status->flag |= RX_FLAG_RADIOTAP_TLV_AT_END;
> +	skb_reset_mac_header(skb);
> +
> +	/* EHT */
> +	tlv = skb_push(skb, len);
> +	memset(tlv, 0, len);
> +	tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT);
> +	tlv->len = cpu_to_le16(eht_len);
> +
> +	eht = (struct ieee80211_radiotap_eht *)tlv->data;
> +	eht->known = cpu_to_le32(IEEE80211_RADIOTAP_EHT_KNOWN_GI);
> +	eht->data[0] =
> +		le32_encode_bits(rx_status->eht.gi, IEEE80211_RADIOTAP_EHT_DATA0_GI);
> +
> +	eht->user_info[0] =
> +		cpu_to_le32(IEEE80211_RADIOTAP_EHT_USER_INFO_MCS_KNOWN |
> +			    IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_KNOWN_O);
> +	eht->user_info[0] |=
> +		le32_encode_bits(rx_status->rate_idx, IEEE80211_RADIOTAP_EHT_USER_INFO_MCS) |
> +		le32_encode_bits(rx_status->nss, IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_O);
> +
> +	/* U-SIG */
> +	tlv = (void *)tlv + sizeof(*tlv) + ALIGN(eht_len, 4);
> +	tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT_USIG);
> +	tlv->len = cpu_to_le16(usig_len);
> +
> +	bw = rx_status->bw < ARRAY_SIZE(rx_status_bw_to_radiotap_eht_usig) ?
> +	     rx_status_bw_to_radiotap_eht_usig[rx_status->bw] : U8_MAX;
> +	if (bw == U8_MAX)
> +		return;

This is cosmetics but I feel that 'if' statement is more readable than
':' operator:

if (rx_status->bw >= ARRAY_SIZE(rx_status_bw_to_radiotap_eht_usig)
        return;
        
bw = rx_status_bw_to_radiotap_eht_usig[rx_status->bw];

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

RE: [PATCH v2 6/6] wifi: rtw89: add EHT radiotap in monitor mode

[Ping-Ke Shih]

> -----Original Message-----
> From: Kalle Valo <kvalo@kernel.org>
> Sent: Wednesday, October 11, 2023 5:14 PM
> To: Ping-Ke Shih <pkshih@realtek.com>
> Cc: johannes@sipsolutions.net; linux-wireless@vger.kernel.org
> Subject: Re: [PATCH v2 6/6] wifi: rtw89: add EHT radiotap in monitor mode
> 
> Ping-Ke Shih <pkshih@realtek.com> writes:
> 
> > Add IEEE80211_RADIOTAP_EHT and IEEE80211_RADIOTAP_EHT_USIG radiotap to
> > fill basic EHT NSS, MCS, GI and bandwidth.
> >
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> > ---
> >  drivers/net/wireless/realtek/rtw89/core.c | 66 +++++++++++++++++++++++
> >  drivers/net/wireless/realtek/rtw89/core.h |  9 +++-
> >  2 files changed, 74 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
> > index 2742e6646cf1..8cb1715d049a 100644
> > --- a/drivers/net/wireless/realtek/rtw89/core.c
> > +++ b/drivers/net/wireless/realtek/rtw89/core.c
> > @@ -1907,6 +1907,70 @@ static void rtw89_core_hw_to_sband_rate(struct ieee80211_rx_status *rx_status)
> >       rx_status->rate_idx -= 4;
> >  }
> >
> > +static u8 rx_status_bw_to_radiotap_eht_usig[] = {
> > +     [RATE_INFO_BW_20] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_20MHZ,
> > +     [RATE_INFO_BW_5] = U8_MAX,
> > +     [RATE_INFO_BW_10] = U8_MAX,
> > +     [RATE_INFO_BW_40] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_40MHZ,
> > +     [RATE_INFO_BW_80] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_80MHZ,
> > +     [RATE_INFO_BW_160] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_160MHZ,
> > +     [RATE_INFO_BW_HE_RU] = U8_MAX,
> > +     [RATE_INFO_BW_320] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_320MHZ_1,
> > +     [RATE_INFO_BW_EHT_RU] = U8_MAX,
> > +};
> 
> Sorry, I noticed this only when I was abot to commit this. Should this
> be static const?

Yes, I miss it.

> 
> > +static void rtw89_core_update_radiotap_eht(struct rtw89_dev *rtwdev,
> > +                                        struct sk_buff *skb,
> > +                                        struct ieee80211_rx_status *rx_status)
> > +{
> > +     struct ieee80211_radiotap_eht_usig *usig;
> > +     struct ieee80211_radiotap_eht *eht;
> > +     struct ieee80211_radiotap_tlv *tlv;
> > +     int eht_len = struct_size(eht, user_info, 1);
> > +     int usig_len = sizeof(*usig);
> > +     int len;
> > +     u8 bw;
> > +
> > +     len = sizeof(*tlv) + ALIGN(eht_len, 4) +
> > +           sizeof(*tlv) + ALIGN(usig_len, 4);
> > +
> > +     rx_status->flag |= RX_FLAG_RADIOTAP_TLV_AT_END;
> > +     skb_reset_mac_header(skb);
> > +
> > +     /* EHT */
> > +     tlv = skb_push(skb, len);
> > +     memset(tlv, 0, len);
> > +     tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT);
> > +     tlv->len = cpu_to_le16(eht_len);
> > +
> > +     eht = (struct ieee80211_radiotap_eht *)tlv->data;
> > +     eht->known = cpu_to_le32(IEEE80211_RADIOTAP_EHT_KNOWN_GI);
> > +     eht->data[0] =
> > +             le32_encode_bits(rx_status->eht.gi, IEEE80211_RADIOTAP_EHT_DATA0_GI);
> > +
> > +     eht->user_info[0] =
> > +             cpu_to_le32(IEEE80211_RADIOTAP_EHT_USER_INFO_MCS_KNOWN |
> > +                         IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_KNOWN_O);
> > +     eht->user_info[0] |=
> > +             le32_encode_bits(rx_status->rate_idx, IEEE80211_RADIOTAP_EHT_USER_INFO_MCS) |
> > +             le32_encode_bits(rx_status->nss, IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_O);
> > +
> > +     /* U-SIG */
> > +     tlv = (void *)tlv + sizeof(*tlv) + ALIGN(eht_len, 4);
> > +     tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT_USIG);
> > +     tlv->len = cpu_to_le16(usig_len);
> > +
> > +     bw = rx_status->bw < ARRAY_SIZE(rx_status_bw_to_radiotap_eht_usig) ?
> > +          rx_status_bw_to_radiotap_eht_usig[rx_status->bw] : U8_MAX;
> > +     if (bw == U8_MAX)
> > +             return;
> 
> This is cosmetics but I feel that 'if' statement is more readable than
> ':' operator:
> 
> if (rx_status->bw >= ARRAY_SIZE(rx_status_bw_to_radiotap_eht_usig)
>         return;
> 
> bw = rx_status_bw_to_radiotap_eht_usig[rx_status->bw];
> 

Got it. I will prepare v3 for them.

Thank you.