2023-05-22 12:35:57

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 0/5] wifi: rtw89: use struct to access register-based H2C/C2H and RX related info

Originally, access these fields by macros that needs casting and add
an offset, so it could hide some mistakes. Refine these macros by
accessing fields via struct.

Ping-Ke Shih (5):
wifi: rtw89: add chip_ops::query_rxdesc() and rxd_len as helpers to
support newer chips
wifi: rtw89: use struct and le32_get_bits to access RX info
wifi: rtw89: use struct and le32_get_bits() to access received PHY
status IEs
wifi: rtw89: use struct and le32_get_bits() to access RX descriptor
wifi: rtw89: use struct to access register-based H2C/C2H

drivers/net/wireless/realtek/rtw89/core.c | 114 +++++++------
drivers/net/wireless/realtek/rtw89/core.h | 16 +-
drivers/net/wireless/realtek/rtw89/fw.c | 18 +-
drivers/net/wireless/realtek/rtw89/fw.h | 102 ++++++------
drivers/net/wireless/realtek/rtw89/mac.c | 23 +--
drivers/net/wireless/realtek/rtw89/pci.c | 12 +-
drivers/net/wireless/realtek/rtw89/rtw8851b.c | 1 +
drivers/net/wireless/realtek/rtw89/rtw8852a.c | 1 +
drivers/net/wireless/realtek/rtw89/rtw8852b.c | 1 +
drivers/net/wireless/realtek/rtw89/rtw8852c.c | 1 +
drivers/net/wireless/realtek/rtw89/txrx.h | 157 +++++-------------
11 files changed, 210 insertions(+), 236 deletions(-)

--
2.25.1



2023-05-22 12:36:22

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 1/5] wifi: rtw89: add chip_ops::query_rxdesc() and rxd_len as helpers to support newer chips

The next generation chips use different RX descriptor format, so add
a chip_ops to hook suitable handlers. Also, the length of RX descriptor is
different, so add a variable to store the length according to chip and
descriptor content dynamically. Then, the code can be more general.

Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw89/core.c | 4 ++++
drivers/net/wireless/realtek/rtw89/core.h | 14 ++++++++++++++
drivers/net/wireless/realtek/rtw89/pci.c | 12 ++++--------
drivers/net/wireless/realtek/rtw89/rtw8851b.c | 1 +
drivers/net/wireless/realtek/rtw89/rtw8852a.c | 1 +
drivers/net/wireless/realtek/rtw89/rtw8852b.c | 1 +
drivers/net/wireless/realtek/rtw89/rtw8852c.c | 1 +
7 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
index fbcb9b6e6f754..ae547b38dfaa8 100644
--- a/drivers/net/wireless/realtek/rtw89/core.c
+++ b/drivers/net/wireless/realtek/rtw89/core.c
@@ -1819,6 +1819,10 @@ void rtw89_core_query_rxdesc(struct rtw89_dev *rtwdev,
shift_len = desc_info->shift << 1; /* 2-byte unit */
drv_info_len = desc_info->drv_info_size << 3; /* 8-byte unit */
desc_info->offset = data_offset + shift_len + drv_info_len;
+ if (desc_info->long_rxdesc)
+ desc_info->rxd_len = sizeof(struct rtw89_rxdesc_long);
+ else
+ desc_info->rxd_len = sizeof(struct rtw89_rxdesc_short);
desc_info->ready = true;

if (!desc_info->long_rxdesc)
diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
index fe464b7212f55..7a486a7207074 100644
--- a/drivers/net/wireless/realtek/rtw89/core.h
+++ b/drivers/net/wireless/realtek/rtw89/core.h
@@ -776,6 +776,7 @@ struct rtw89_rx_desc_info {
u8 sec_cam_id;
u8 mac_id;
u16 offset;
+ u16 rxd_len;
bool ready;
};

@@ -2824,6 +2825,9 @@ struct rtw89_chip_ops {
s8 pw_ofst, enum rtw89_mac_idx mac_idx);
int (*pwr_on_func)(struct rtw89_dev *rtwdev);
int (*pwr_off_func)(struct rtw89_dev *rtwdev);
+ void (*query_rxdesc)(struct rtw89_dev *rtwdev,
+ struct rtw89_rx_desc_info *desc_info,
+ u8 *data, u32 data_offset);
void (*fill_txdesc)(struct rtw89_dev *rtwdev,
struct rtw89_tx_desc_info *desc_info,
void *txdesc);
@@ -4854,6 +4858,16 @@ static inline void rtw89_ctrl_btg(struct rtw89_dev *rtwdev, bool btg)
chip->ops->ctrl_btg(rtwdev, btg);
}

+static inline
+void rtw89_chip_query_rxdesc(struct rtw89_dev *rtwdev,
+ struct rtw89_rx_desc_info *desc_info,
+ u8 *data, u32 data_offset)
+{
+ const struct rtw89_chip_info *chip = rtwdev->chip;
+
+ chip->ops->query_rxdesc(rtwdev, desc_info, data, data_offset);
+}
+
static inline
void rtw89_chip_fill_txdesc(struct rtw89_dev *rtwdev,
struct rtw89_tx_desc_info *desc_info,
diff --git a/drivers/net/wireless/realtek/rtw89/pci.c b/drivers/net/wireless/realtek/rtw89/pci.c
index 92bfef942d3a9..9402f1a0caea8 100644
--- a/drivers/net/wireless/realtek/rtw89/pci.c
+++ b/drivers/net/wireless/realtek/rtw89/pci.c
@@ -265,7 +265,7 @@ static u32 rtw89_pci_rxbd_deliver_skbs(struct rtw89_dev *rtwdev,
goto err_sync_device;
}

- rtw89_core_query_rxdesc(rtwdev, desc_info, skb->data, rxinfo_size);
+ rtw89_chip_query_rxdesc(rtwdev, desc_info, skb->data, rxinfo_size);

new = rtw89_alloc_skb_for_rx(rtwdev, desc_info->pkt_size);
if (!new)
@@ -274,9 +274,7 @@ static u32 rtw89_pci_rxbd_deliver_skbs(struct rtw89_dev *rtwdev,
rx_ring->diliver_skb = new;

/* first segment has RX desc */
- offset = desc_info->offset;
- offset += desc_info->long_rxdesc ? sizeof(struct rtw89_rxdesc_long) :
- sizeof(struct rtw89_rxdesc_short);
+ offset = desc_info->offset + desc_info->rxd_len;
} else {
offset = sizeof(struct rtw89_pci_rxbd_info);
if (!new) {
@@ -546,12 +544,10 @@ static u32 rtw89_pci_release_tx_skbs(struct rtw89_dev *rtwdev,
return cnt;
}

- rtw89_core_query_rxdesc(rtwdev, &desc_info, skb->data, rxinfo_size);
+ rtw89_chip_query_rxdesc(rtwdev, &desc_info, skb->data, rxinfo_size);

/* first segment has RX desc */
- offset = desc_info.offset;
- offset += desc_info.long_rxdesc ? sizeof(struct rtw89_rxdesc_long) :
- sizeof(struct rtw89_rxdesc_short);
+ offset = desc_info.offset + desc_info.rxd_len;
for (; offset + rpp_size <= rx_info->len; offset += rpp_size) {
rpp = (struct rtw89_pci_rpp_fmt *)(skb->data + offset);
rtw89_pci_release_rpp(rtwdev, rpp);
diff --git a/drivers/net/wireless/realtek/rtw89/rtw8851b.c b/drivers/net/wireless/realtek/rtw89/rtw8851b.c
index 6ecdd3d3f776f..b19579874b2b7 100644
--- a/drivers/net/wireless/realtek/rtw89/rtw8851b.c
+++ b/drivers/net/wireless/realtek/rtw89/rtw8851b.c
@@ -1713,6 +1713,7 @@ static const struct rtw89_chip_ops rtw8851b_chip_ops = {
.rfe_gpio = rtw8851b_rfe_gpio,
.pwr_on_func = rtw8851b_pwr_on_func,
.pwr_off_func = rtw8851b_pwr_off_func,
+ .query_rxdesc = rtw89_core_query_rxdesc,
.fill_txdesc = rtw89_core_fill_txdesc,
.fill_txdesc_fwcmd = rtw89_core_fill_txdesc,
.h2c_dctl_sec_cam = NULL,
diff --git a/drivers/net/wireless/realtek/rtw89/rtw8852a.c b/drivers/net/wireless/realtek/rtw89/rtw8852a.c
index 541ad2f4b0c2b..559835ce86bb6 100644
--- a/drivers/net/wireless/realtek/rtw89/rtw8852a.c
+++ b/drivers/net/wireless/realtek/rtw89/rtw8852a.c
@@ -2050,6 +2050,7 @@ static const struct rtw89_chip_ops rtw8852a_chip_ops = {
.set_txpwr_ul_tb_offset = rtw8852a_set_txpwr_ul_tb_offset,
.pwr_on_func = NULL,
.pwr_off_func = NULL,
+ .query_rxdesc = rtw89_core_query_rxdesc,
.fill_txdesc = rtw89_core_fill_txdesc,
.fill_txdesc_fwcmd = rtw89_core_fill_txdesc,
.cfg_ctrl_path = rtw89_mac_cfg_ctrl_path,
diff --git a/drivers/net/wireless/realtek/rtw89/rtw8852b.c b/drivers/net/wireless/realtek/rtw89/rtw8852b.c
index 4e4023f114ec8..2a143751abc5f 100644
--- a/drivers/net/wireless/realtek/rtw89/rtw8852b.c
+++ b/drivers/net/wireless/realtek/rtw89/rtw8852b.c
@@ -2472,6 +2472,7 @@ static const struct rtw89_chip_ops rtw8852b_chip_ops = {
.set_txpwr_ul_tb_offset = rtw8852b_set_txpwr_ul_tb_offset,
.pwr_on_func = rtw8852b_pwr_on_func,
.pwr_off_func = rtw8852b_pwr_off_func,
+ .query_rxdesc = rtw89_core_query_rxdesc,
.fill_txdesc = rtw89_core_fill_txdesc,
.fill_txdesc_fwcmd = rtw89_core_fill_txdesc,
.cfg_ctrl_path = rtw89_mac_cfg_ctrl_path,
diff --git a/drivers/net/wireless/realtek/rtw89/rtw8852c.c b/drivers/net/wireless/realtek/rtw89/rtw8852c.c
index e4c984c87d6cc..9c7c9812d4f45 100644
--- a/drivers/net/wireless/realtek/rtw89/rtw8852c.c
+++ b/drivers/net/wireless/realtek/rtw89/rtw8852c.c
@@ -2780,6 +2780,7 @@ static const struct rtw89_chip_ops rtw8852c_chip_ops = {
.set_txpwr_ul_tb_offset = rtw8852c_set_txpwr_ul_tb_offset,
.pwr_on_func = rtw8852c_pwr_on_func,
.pwr_off_func = rtw8852c_pwr_off_func,
+ .query_rxdesc = rtw89_core_query_rxdesc,
.fill_txdesc = rtw89_core_fill_txdesc_v1,
.fill_txdesc_fwcmd = rtw89_core_fill_txdesc_fwcmd_v1,
.cfg_ctrl_path = rtw89_mac_cfg_ctrl_path_v1,
--
2.25.1


2023-05-22 12:36:33

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 3/5] wifi: rtw89: use struct and le32_get_bits() to access received PHY status IEs

PHY status IEs generated by BB hardware is to provide more detail
information related to received packets, such as RSSI and bandwidth.

To avoid type casting, change buf type from u8* to void* as well.

This patch doesn't change logic at all.

Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw89/core.c | 53 ++++++++++++++---------
drivers/net/wireless/realtek/rtw89/core.h | 2 +-
drivers/net/wireless/realtek/rtw89/txrx.h | 37 ++++++++--------
3 files changed, 52 insertions(+), 40 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
index c92a83bf2f36f..5a52815fc6077 100644
--- a/drivers/net/wireless/realtek/rtw89/core.c
+++ b/drivers/net/wireless/realtek/rtw89/core.c
@@ -1280,7 +1280,8 @@ static void rtw89_core_rx_process_phy_ppdu_iter(void *data,

#define VAR_LEN 0xff
#define VAR_LEN_UNIT 8
-static u16 rtw89_core_get_phy_status_ie_len(struct rtw89_dev *rtwdev, u8 *addr)
+static u16 rtw89_core_get_phy_status_ie_len(struct rtw89_dev *rtwdev,
+ const struct rtw89_phy_sts_iehdr *iehdr)
{
static const u8 physts_ie_len_tab[32] = {
16, 32, 24, 24, 8, 8, 8, 8, VAR_LEN, 8, VAR_LEN, 176, VAR_LEN,
@@ -1290,19 +1291,20 @@ static u16 rtw89_core_get_phy_status_ie_len(struct rtw89_dev *rtwdev, u8 *addr)
u16 ie_len;
u8 ie;

- ie = RTW89_GET_PHY_STS_IE_TYPE(addr);
+ ie = le32_get_bits(iehdr->w0, RTW89_PHY_STS_IEHDR_TYPE);
if (physts_ie_len_tab[ie] != VAR_LEN)
ie_len = physts_ie_len_tab[ie];
else
- ie_len = RTW89_GET_PHY_STS_IE_LEN(addr) * VAR_LEN_UNIT;
+ ie_len = le32_get_bits(iehdr->w0, RTW89_PHY_STS_IEHDR_LEN) * VAR_LEN_UNIT;

return ie_len;
}

-static void rtw89_core_parse_phy_status_ie01(struct rtw89_dev *rtwdev, u8 *addr,
+static void rtw89_core_parse_phy_status_ie01(struct rtw89_dev *rtwdev,
+ const struct rtw89_phy_sts_iehdr *iehdr,
struct rtw89_rx_phy_ppdu *phy_ppdu)
{
- const struct rtw89_phy_sts_ie0 *ie = (const struct rtw89_phy_sts_ie0 *)addr;
+ const struct rtw89_phy_sts_ie0 *ie = (const struct rtw89_phy_sts_ie0 *)iehdr;
s16 cfo;
u32 t;

@@ -1330,15 +1332,17 @@ static void rtw89_core_parse_phy_status_ie01(struct rtw89_dev *rtwdev, u8 *addr,
rtw89_phy_cfo_parse(rtwdev, cfo, phy_ppdu);
}

-static int rtw89_core_process_phy_status_ie(struct rtw89_dev *rtwdev, u8 *addr,
+static int rtw89_core_process_phy_status_ie(struct rtw89_dev *rtwdev,
+ const struct rtw89_phy_sts_iehdr *iehdr,
struct rtw89_rx_phy_ppdu *phy_ppdu)
{
u8 ie;

- ie = RTW89_GET_PHY_STS_IE_TYPE(addr);
+ ie = le32_get_bits(iehdr->w0, RTW89_PHY_STS_IEHDR_TYPE);
+
switch (ie) {
case RTW89_PHYSTS_IE01_CMN_OFDM:
- rtw89_core_parse_phy_status_ie01(rtwdev, addr, phy_ppdu);
+ rtw89_core_parse_phy_status_ie01(rtwdev, iehdr, phy_ppdu);
break;
default:
break;
@@ -1349,21 +1353,26 @@ static int rtw89_core_process_phy_status_ie(struct rtw89_dev *rtwdev, u8 *addr,

static void rtw89_core_update_phy_ppdu(struct rtw89_rx_phy_ppdu *phy_ppdu)
{
+ const struct rtw89_phy_sts_hdr *hdr = phy_ppdu->buf;
u8 *rssi = phy_ppdu->rssi;
- u8 *buf = phy_ppdu->buf;

- phy_ppdu->ie = RTW89_GET_PHY_STS_IE_MAP(buf);
- phy_ppdu->rssi_avg = RTW89_GET_PHY_STS_RSSI_AVG(buf);
- rssi[RF_PATH_A] = RTW89_GET_PHY_STS_RSSI_A(buf);
- rssi[RF_PATH_B] = RTW89_GET_PHY_STS_RSSI_B(buf);
- rssi[RF_PATH_C] = RTW89_GET_PHY_STS_RSSI_C(buf);
- rssi[RF_PATH_D] = RTW89_GET_PHY_STS_RSSI_D(buf);
+ phy_ppdu->ie = le32_get_bits(hdr->w0, RTW89_PHY_STS_HDR_W0_IE_MAP);
+ phy_ppdu->rssi_avg = le32_get_bits(hdr->w0, RTW89_PHY_STS_HDR_W0_RSSI_AVG);
+ rssi[RF_PATH_A] = le32_get_bits(hdr->w1, RTW89_PHY_STS_HDR_W1_RSSI_A);
+ rssi[RF_PATH_B] = le32_get_bits(hdr->w1, RTW89_PHY_STS_HDR_W1_RSSI_B);
+ rssi[RF_PATH_C] = le32_get_bits(hdr->w1, RTW89_PHY_STS_HDR_W1_RSSI_C);
+ rssi[RF_PATH_D] = le32_get_bits(hdr->w1, RTW89_PHY_STS_HDR_W1_RSSI_D);
}

static int rtw89_core_rx_process_phy_ppdu(struct rtw89_dev *rtwdev,
struct rtw89_rx_phy_ppdu *phy_ppdu)
{
- if (RTW89_GET_PHY_STS_LEN(phy_ppdu->buf) << 3 != phy_ppdu->len) {
+ const struct rtw89_phy_sts_hdr *hdr = phy_ppdu->buf;
+ u32 len_from_header;
+
+ len_from_header = le32_get_bits(hdr->w0, RTW89_PHY_STS_HDR_W0_LEN) << 3;
+
+ if (len_from_header != phy_ppdu->len) {
rtw89_debug(rtwdev, RTW89_DBG_UNEXP, "phy ppdu len mismatch\n");
return -EINVAL;
}
@@ -1376,17 +1385,19 @@ static int rtw89_core_rx_parse_phy_sts(struct rtw89_dev *rtwdev,
struct rtw89_rx_phy_ppdu *phy_ppdu)
{
u16 ie_len;
- u8 *pos, *end;
+ void *pos, *end;

/* mark invalid reports and bypass them */
if (phy_ppdu->ie < RTW89_CCK_PKT)
return -EINVAL;

- pos = (u8 *)phy_ppdu->buf + PHY_STS_HDR_LEN;
- end = (u8 *)phy_ppdu->buf + phy_ppdu->len;
+ pos = phy_ppdu->buf + PHY_STS_HDR_LEN;
+ end = phy_ppdu->buf + phy_ppdu->len;
while (pos < end) {
- ie_len = rtw89_core_get_phy_status_ie_len(rtwdev, pos);
- rtw89_core_process_phy_status_ie(rtwdev, pos, phy_ppdu);
+ const struct rtw89_phy_sts_iehdr *iehdr = pos;
+
+ ie_len = rtw89_core_get_phy_status_ie_len(rtwdev, iehdr);
+ rtw89_core_process_phy_status_ie(rtwdev, iehdr, phy_ppdu);
pos += ie_len;
if (pos > end || ie_len == 0) {
rtw89_debug(rtwdev, RTW89_DBG_TXRX,
diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
index 7a486a7207074..2609881e6892c 100644
--- a/drivers/net/wireless/realtek/rtw89/core.h
+++ b/drivers/net/wireless/realtek/rtw89/core.h
@@ -550,7 +550,7 @@ struct rtw89_rate_desc {
#define RF_PATH_MAX 4
#define RTW89_MAX_PPDU_CNT 8
struct rtw89_rx_phy_ppdu {
- u8 *buf;
+ void *buf;
u32 len;
u8 rssi_avg;
u8 rssi[RF_PATH_MAX];
diff --git a/drivers/net/wireless/realtek/rtw89/txrx.h b/drivers/net/wireless/realtek/rtw89/txrx.h
index d096d16c8d3be..5f20656aa1ad6 100644
--- a/drivers/net/wireless/realtek/rtw89/txrx.h
+++ b/drivers/net/wireless/realtek/rtw89/txrx.h
@@ -277,24 +277,25 @@ struct rtw89_rxinfo {
#define RTW89_RXINFO_W1_SERVICE GENMASK(15, 0)
#define RTW89_RXINFO_W1_PLCP_LEN GENMASK(23, 16)

-#define RTW89_GET_PHY_STS_IE_MAP(sts) \
- le32_get_bits(*((const __le32 *)(sts)), GENMASK(4, 0))
-#define RTW89_GET_PHY_STS_RSSI_A(sts) \
- le32_get_bits(*((const __le32 *)(sts) + 1), GENMASK(7, 0))
-#define RTW89_GET_PHY_STS_RSSI_B(sts) \
- le32_get_bits(*((const __le32 *)(sts) + 1), GENMASK(15, 8))
-#define RTW89_GET_PHY_STS_RSSI_C(sts) \
- le32_get_bits(*((const __le32 *)(sts) + 1), GENMASK(23, 16))
-#define RTW89_GET_PHY_STS_RSSI_D(sts) \
- le32_get_bits(*((const __le32 *)(sts) + 1), GENMASK(31, 24))
-#define RTW89_GET_PHY_STS_LEN(sts) \
- le32_get_bits(*((const __le32 *)sts), GENMASK(15, 8))
-#define RTW89_GET_PHY_STS_RSSI_AVG(sts) \
- le32_get_bits(*((const __le32 *)sts), GENMASK(31, 24))
-#define RTW89_GET_PHY_STS_IE_TYPE(ie) \
- le32_get_bits(*((const __le32 *)ie), GENMASK(4, 0))
-#define RTW89_GET_PHY_STS_IE_LEN(ie) \
- le32_get_bits(*((const __le32 *)ie), GENMASK(11, 5))
+struct rtw89_phy_sts_hdr {
+ __le32 w0;
+ __le32 w1;
+} __packed;
+
+#define RTW89_PHY_STS_HDR_W0_IE_MAP GENMASK(4, 0)
+#define RTW89_PHY_STS_HDR_W0_LEN GENMASK(15, 8)
+#define RTW89_PHY_STS_HDR_W0_RSSI_AVG GENMASK(31, 24)
+#define RTW89_PHY_STS_HDR_W1_RSSI_A GENMASK(7, 0)
+#define RTW89_PHY_STS_HDR_W1_RSSI_B GENMASK(15, 8)
+#define RTW89_PHY_STS_HDR_W1_RSSI_C GENMASK(23, 16)
+#define RTW89_PHY_STS_HDR_W1_RSSI_D GENMASK(31, 24)
+
+struct rtw89_phy_sts_iehdr {
+ __le32 w0;
+};
+
+#define RTW89_PHY_STS_IEHDR_TYPE GENMASK(4, 0)
+#define RTW89_PHY_STS_IEHDR_LEN GENMASK(11, 5)

struct rtw89_phy_sts_ie0 {
__le32 w0;
--
2.25.1


2023-05-22 12:36:34

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 4/5] wifi: rtw89: use struct and le32_get_bits() to access RX descriptor

RX descriptor is to provide basic and important information related to
packets, such as packet size, security, MAC ID and so on. Change to use
struct to access these fields, and not change logic at all.

Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw89/core.c | 50 ++++++++---------
drivers/net/wireless/realtek/rtw89/txrx.h | 65 -----------------------
2 files changed, 25 insertions(+), 90 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
index 5a52815fc6077..101047686fffb 100644
--- a/drivers/net/wireless/realtek/rtw89/core.c
+++ b/drivers/net/wireless/realtek/rtw89/core.c
@@ -1806,27 +1806,27 @@ void rtw89_core_query_rxdesc(struct rtw89_dev *rtwdev,
u8 shift_len, drv_info_len;

rxd_s = (struct rtw89_rxdesc_short *)(data + data_offset);
- desc_info->pkt_size = RTW89_GET_RXWD_PKT_SIZE(rxd_s);
- desc_info->drv_info_size = RTW89_GET_RXWD_DRV_INFO_SIZE(rxd_s);
- desc_info->long_rxdesc = RTW89_GET_RXWD_LONG_RXD(rxd_s);
- desc_info->pkt_type = RTW89_GET_RXWD_RPKT_TYPE(rxd_s);
- desc_info->mac_info_valid = RTW89_GET_RXWD_MAC_INFO_VALID(rxd_s);
+ desc_info->pkt_size = le32_get_bits(rxd_s->dword0, AX_RXD_RPKT_LEN_MASK);
+ desc_info->drv_info_size = le32_get_bits(rxd_s->dword0, AX_RXD_DRV_INFO_SIZE_MASK);
+ desc_info->long_rxdesc = le32_get_bits(rxd_s->dword0, AX_RXD_LONG_RXD);
+ desc_info->pkt_type = le32_get_bits(rxd_s->dword0, AX_RXD_RPKT_TYPE_MASK);
+ desc_info->mac_info_valid = le32_get_bits(rxd_s->dword0, AX_RXD_MAC_INFO_VLD);
if (chip->chip_id == RTL8852C)
- desc_info->bw = RTW89_GET_RXWD_BW_V1(rxd_s);
+ desc_info->bw = le32_get_bits(rxd_s->dword1, AX_RXD_BW_v1_MASK);
else
- desc_info->bw = RTW89_GET_RXWD_BW(rxd_s);
- desc_info->data_rate = RTW89_GET_RXWD_DATA_RATE(rxd_s);
- desc_info->gi_ltf = RTW89_GET_RXWD_GI_LTF(rxd_s);
- desc_info->user_id = RTW89_GET_RXWD_USER_ID(rxd_s);
- desc_info->sr_en = RTW89_GET_RXWD_SR_EN(rxd_s);
- desc_info->ppdu_cnt = RTW89_GET_RXWD_PPDU_CNT(rxd_s);
- desc_info->ppdu_type = RTW89_GET_RXWD_PPDU_TYPE(rxd_s);
- desc_info->free_run_cnt = RTW89_GET_RXWD_FREE_RUN_CNT(rxd_s);
- desc_info->icv_err = RTW89_GET_RXWD_ICV_ERR(rxd_s);
- desc_info->crc32_err = RTW89_GET_RXWD_CRC32_ERR(rxd_s);
- desc_info->hw_dec = RTW89_GET_RXWD_HW_DEC(rxd_s);
- desc_info->sw_dec = RTW89_GET_RXWD_SW_DEC(rxd_s);
- desc_info->addr1_match = RTW89_GET_RXWD_A1_MATCH(rxd_s);
+ desc_info->bw = le32_get_bits(rxd_s->dword1, AX_RXD_BW_MASK);
+ desc_info->data_rate = le32_get_bits(rxd_s->dword1, AX_RXD_RX_DATARATE_MASK);
+ desc_info->gi_ltf = le32_get_bits(rxd_s->dword1, AX_RXD_RX_GI_LTF_MASK);
+ desc_info->user_id = le32_get_bits(rxd_s->dword1, AX_RXD_USER_ID_MASK);
+ desc_info->sr_en = le32_get_bits(rxd_s->dword1, AX_RXD_SR_EN);
+ desc_info->ppdu_cnt = le32_get_bits(rxd_s->dword1, AX_RXD_PPDU_CNT_MASK);
+ desc_info->ppdu_type = le32_get_bits(rxd_s->dword1, AX_RXD_PPDU_TYPE_MASK);
+ desc_info->free_run_cnt = le32_get_bits(rxd_s->dword2, AX_RXD_FREERUN_CNT_MASK);
+ desc_info->icv_err = le32_get_bits(rxd_s->dword3, AX_RXD_ICV_ERR);
+ desc_info->crc32_err = le32_get_bits(rxd_s->dword3, AX_RXD_CRC32_ERR);
+ desc_info->hw_dec = le32_get_bits(rxd_s->dword3, AX_RXD_HW_DEC);
+ desc_info->sw_dec = le32_get_bits(rxd_s->dword3, AX_RXD_SW_DEC);
+ desc_info->addr1_match = le32_get_bits(rxd_s->dword3, AX_RXD_A1_MATCH);

shift_len = desc_info->shift << 1; /* 2-byte unit */
drv_info_len = desc_info->drv_info_size << 3; /* 8-byte unit */
@@ -1841,12 +1841,12 @@ void rtw89_core_query_rxdesc(struct rtw89_dev *rtwdev,
return;

rxd_l = (struct rtw89_rxdesc_long *)(data + data_offset);
- desc_info->frame_type = RTW89_GET_RXWD_TYPE(rxd_l);
- desc_info->addr_cam_valid = RTW89_GET_RXWD_ADDR_CAM_VLD(rxd_l);
- desc_info->addr_cam_id = RTW89_GET_RXWD_ADDR_CAM_ID(rxd_l);
- desc_info->sec_cam_id = RTW89_GET_RXWD_SEC_CAM_ID(rxd_l);
- desc_info->mac_id = RTW89_GET_RXWD_MAC_ID(rxd_l);
- desc_info->rx_pl_id = RTW89_GET_RXWD_RX_PL_ID(rxd_l);
+ desc_info->frame_type = le32_get_bits(rxd_l->dword4, AX_RXD_TYPE_MASK);
+ desc_info->addr_cam_valid = le32_get_bits(rxd_l->dword5, AX_RXD_ADDR_CAM_VLD);
+ desc_info->addr_cam_id = le32_get_bits(rxd_l->dword5, AX_RXD_ADDR_CAM_MASK);
+ desc_info->sec_cam_id = le32_get_bits(rxd_l->dword5, AX_RXD_SEC_CAM_IDX_MASK);
+ desc_info->mac_id = le32_get_bits(rxd_l->dword5, AX_RXD_MAC_ID_MASK);
+ desc_info->rx_pl_id = le32_get_bits(rxd_l->dword5, AX_RXD_RX_PL_ID_MASK);
}
EXPORT_SYMBOL(rtw89_core_query_rxdesc);

diff --git a/drivers/net/wireless/realtek/rtw89/txrx.h b/drivers/net/wireless/realtek/rtw89/txrx.h
index 5f20656aa1ad6..ec96da36eacc6 100644
--- a/drivers/net/wireless/realtek/rtw89/txrx.h
+++ b/drivers/net/wireless/realtek/rtw89/txrx.h
@@ -186,71 +186,6 @@
#define AX_RXD_BIP_KEYID BIT(27)
#define AX_RXD_BIP_ENC BIT(28)

-/* RX DESC helpers */
-/* Short Descriptor */
-#define RTW89_GET_RXWD_LONG_RXD(rxdesc) \
- le32_get_bits((rxdesc)->dword0, BIT(31))
-#define RTW89_GET_RXWD_DRV_INFO_SIZE(rxdesc) \
- le32_get_bits((rxdesc)->dword0, GENMASK(30, 28))
-#define RTW89_GET_RXWD_RPKT_TYPE(rxdesc) \
- le32_get_bits((rxdesc)->dword0, GENMASK(27, 24))
-#define RTW89_GET_RXWD_MAC_INFO_VALID(rxdesc) \
- le32_get_bits((rxdesc)->dword0, BIT(23))
-#define RTW89_GET_RXWD_BB_SEL(rxdesc) \
- le32_get_bits((rxdesc)->dword0, BIT(22))
-#define RTW89_GET_RXWD_HD_IV_LEN(rxdesc) \
- le32_get_bits((rxdesc)->dword0, GENMASK(21, 16))
-#define RTW89_GET_RXWD_SHIFT(rxdesc) \
- le32_get_bits((rxdesc)->dword0, GENMASK(15, 14))
-#define RTW89_GET_RXWD_PKT_SIZE(rxdesc) \
- le32_get_bits((rxdesc)->dword0, GENMASK(13, 0))
-#define RTW89_GET_RXWD_BW(rxdesc) \
- le32_get_bits((rxdesc)->dword1, GENMASK(31, 30))
-#define RTW89_GET_RXWD_BW_V1(rxdesc) \
- le32_get_bits((rxdesc)->dword1, GENMASK(31, 29))
-#define RTW89_GET_RXWD_GI_LTF(rxdesc) \
- le32_get_bits((rxdesc)->dword1, GENMASK(27, 25))
-#define RTW89_GET_RXWD_DATA_RATE(rxdesc) \
- le32_get_bits((rxdesc)->dword1, GENMASK(24, 16))
-#define RTW89_GET_RXWD_USER_ID(rxdesc) \
- le32_get_bits((rxdesc)->dword1, GENMASK(15, 8))
-#define RTW89_GET_RXWD_SR_EN(rxdesc) \
- le32_get_bits((rxdesc)->dword1, BIT(7))
-#define RTW89_GET_RXWD_PPDU_CNT(rxdesc) \
- le32_get_bits((rxdesc)->dword1, GENMASK(6, 4))
-#define RTW89_GET_RXWD_PPDU_TYPE(rxdesc) \
- le32_get_bits((rxdesc)->dword1, GENMASK(3, 0))
-#define RTW89_GET_RXWD_FREE_RUN_CNT(rxdesc) \
- le32_get_bits((rxdesc)->dword2, GENMASK(31, 0))
-#define RTW89_GET_RXWD_ICV_ERR(rxdesc) \
- le32_get_bits((rxdesc)->dword3, BIT(10))
-#define RTW89_GET_RXWD_CRC32_ERR(rxdesc) \
- le32_get_bits((rxdesc)->dword3, BIT(9))
-#define RTW89_GET_RXWD_HW_DEC(rxdesc) \
- le32_get_bits((rxdesc)->dword3, BIT(2))
-#define RTW89_GET_RXWD_SW_DEC(rxdesc) \
- le32_get_bits((rxdesc)->dword3, BIT(1))
-#define RTW89_GET_RXWD_A1_MATCH(rxdesc) \
- le32_get_bits((rxdesc)->dword3, BIT(0))
-
-/* Long Descriptor */
-#define RTW89_GET_RXWD_FRAG(rxdesc) \
- le32_get_bits((rxdesc)->dword4, GENMASK(31, 28))
-#define RTW89_GET_RXWD_SEQ(rxdesc) \
- le32_get_bits((rxdesc)->dword4, GENMASK(27, 16))
-#define RTW89_GET_RXWD_TYPE(rxdesc) \
- le32_get_bits((rxdesc)->dword4, GENMASK(1, 0))
-#define RTW89_GET_RXWD_ADDR_CAM_VLD(rxdesc) \
- le32_get_bits((rxdesc)->dword5, BIT(28))
-#define RTW89_GET_RXWD_RX_PL_ID(rxdesc) \
- le32_get_bits((rxdesc)->dword5, GENMASK(27, 24))
-#define RTW89_GET_RXWD_MAC_ID(rxdesc) \
- le32_get_bits((rxdesc)->dword5, GENMASK(23, 16))
-#define RTW89_GET_RXWD_ADDR_CAM_ID(rxdesc) \
- le32_get_bits((rxdesc)->dword5, GENMASK(15, 8))
-#define RTW89_GET_RXWD_SEC_CAM_ID(rxdesc) \
- le32_get_bits((rxdesc)->dword5, GENMASK(7, 0))
-
struct rtw89_rxinfo_user {
__le32 w0;
};
--
2.25.1


2023-05-22 12:36:35

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 5/5] wifi: rtw89: use struct to access register-based H2C/C2H

The register-based H2C/C2H are used to exchange commands and events with
firmware. The exchange data is limited, but it is relatively simple,
because it can work before HCI initialization. To make these code clean,
use struct to access them. This patch doesn't change logic at all.

Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw89/fw.c | 18 ++--
drivers/net/wireless/realtek/rtw89/fw.h | 102 ++++++++++++-----------
drivers/net/wireless/realtek/rtw89/mac.c | 23 ++---
3 files changed, 77 insertions(+), 66 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
index ad277f22b1973..8f1ef151b8f6e 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.c
+++ b/drivers/net/wireless/realtek/rtw89/fw.c
@@ -2916,12 +2916,13 @@ static int rtw89_fw_write_h2c_reg(struct rtw89_dev *rtwdev,
}

len = DIV_ROUND_UP(info->content_len + RTW89_H2CREG_HDR_LEN,
- sizeof(info->h2creg[0]));
+ sizeof(info->u.h2creg[0]));
+
+ u32p_replace_bits(&info->u.hdr.w0, info->id, RTW89_H2CREG_HDR_FUNC_MASK);
+ u32p_replace_bits(&info->u.hdr.w0, len, RTW89_H2CREG_HDR_LEN_MASK);

- RTW89_SET_H2CREG_HDR_FUNC(&info->h2creg[0], info->id);
- RTW89_SET_H2CREG_HDR_LEN(&info->h2creg[0], len);
for (i = 0; i < RTW89_H2CREG_MAX; i++)
- rtw89_write32(rtwdev, h2c_reg[i], info->h2creg[i]);
+ rtw89_write32(rtwdev, h2c_reg[i], info->u.h2creg[i]);

fw_info->h2c_counter++;
rtw89_write8_mask(rtwdev, chip->h2c_counter_reg.addr,
@@ -2951,13 +2952,14 @@ static int rtw89_fw_read_c2h_reg(struct rtw89_dev *rtwdev,
}

for (i = 0; i < RTW89_C2HREG_MAX; i++)
- info->c2hreg[i] = rtw89_read32(rtwdev, c2h_reg[i]);
+ info->u.c2hreg[i] = rtw89_read32(rtwdev, c2h_reg[i]);

rtw89_write8(rtwdev, chip->c2h_ctrl_reg, 0);

- info->id = RTW89_GET_C2H_HDR_FUNC(*info->c2hreg);
- info->content_len = (RTW89_GET_C2H_HDR_LEN(*info->c2hreg) << 2) -
- RTW89_C2HREG_HDR_LEN;
+ info->id = u32_get_bits(info->u.hdr.w0, RTW89_C2HREG_HDR_FUNC_MASK);
+ info->content_len =
+ (u32_get_bits(info->u.hdr.w0, RTW89_C2HREG_HDR_LEN_MASK) << 2) -
+ RTW89_C2HREG_HDR_LEN;

fw_info->c2h_counter++;
rtw89_write8_mask(rtwdev, chip->c2h_counter_reg.addr,
diff --git a/drivers/net/wireless/realtek/rtw89/fw.h b/drivers/net/wireless/realtek/rtw89/fw.h
index 048283750a2d2..dd71beb152aeb 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.h
+++ b/drivers/net/wireless/realtek/rtw89/fw.h
@@ -18,15 +18,51 @@ enum rtw89_fw_dl_status {
RTW89_FWDL_WCPU_FW_INIT_RDY = 7
};

-#define RTW89_GET_C2H_HDR_FUNC(info) \
- u32_get_bits(info, GENMASK(6, 0))
-#define RTW89_GET_C2H_HDR_LEN(info) \
- u32_get_bits(info, GENMASK(11, 8))
+struct rtw89_c2hreg_hdr {
+ u32 w0;
+};
+
+#define RTW89_C2HREG_HDR_FUNC_MASK GENMASK(6, 0)
+#define RTW89_C2HREG_HDR_ACK BIT(7)
+#define RTW89_C2HREG_HDR_LEN_MASK GENMASK(11, 8)
+#define RTW89_C2HREG_HDR_SEQ_MASK GENMASK(15, 12)
+
+struct rtw89_c2hreg_phycap {
+ u32 w0;
+ u32 w1;
+ u32 w2;
+ u32 w3;
+} __packed;
+
+#define RTW89_C2HREG_PHYCAP_W0_FUNC GENMASK(6, 0)
+#define RTW89_C2HREG_PHYCAP_W0_ACK BIT(7)
+#define RTW89_C2HREG_PHYCAP_W0_LEN GENMASK(11, 8)
+#define RTW89_C2HREG_PHYCAP_W0_SEQ GENMASK(15, 12)
+#define RTW89_C2HREG_PHYCAP_W0_RX_NSS GENMASK(23, 16)
+#define RTW89_C2HREG_PHYCAP_W0_BW GENMASK(31, 24)
+#define RTW89_C2HREG_PHYCAP_W1_TX_NSS GENMASK(7, 0)
+#define RTW89_C2HREG_PHYCAP_W1_PROT GENMASK(15, 8)
+#define RTW89_C2HREG_PHYCAP_W1_NIC GENMASK(23, 16)
+#define RTW89_C2HREG_PHYCAP_W1_WL_FUNC GENMASK(31, 24)
+#define RTW89_C2HREG_PHYCAP_W2_HW_TYPE GENMASK(7, 0)
+#define RTW89_C2HREG_PHYCAP_W3_ANT_TX_NUM GENMASK(15, 8)
+#define RTW89_C2HREG_PHYCAP_W3_ANT_RX_NUM GENMASK(23, 16)
+
+struct rtw89_h2creg_hdr {
+ u32 w0;
+};
+
+#define RTW89_H2CREG_HDR_FUNC_MASK GENMASK(6, 0)
+#define RTW89_H2CREG_HDR_LEN_MASK GENMASK(11, 8)

-#define RTW89_SET_H2CREG_HDR_FUNC(info, val) \
- u32p_replace_bits(info, val, GENMASK(6, 0))
-#define RTW89_SET_H2CREG_HDR_LEN(info, val) \
- u32p_replace_bits(info, val, GENMASK(11, 8))
+struct rtw89_h2creg_sch_tx_en {
+ u32 w0;
+ u32 w1;
+} __packed;
+
+#define RTW89_H2CREG_SCH_TX_EN_W0_EN GENMASK(31, 16)
+#define RTW89_H2CREG_SCH_TX_EN_W1_MASK GENMASK(15, 0)
+#define RTW89_H2CREG_SCH_TX_EN_W1_BAND BIT(16)

#define RTW89_H2CREG_MAX 4
#define RTW89_C2HREG_MAX 4
@@ -36,13 +72,21 @@ enum rtw89_fw_dl_status {
struct rtw89_mac_c2h_info {
u8 id;
u8 content_len;
- u32 c2hreg[RTW89_C2HREG_MAX];
+ union {
+ u32 c2hreg[RTW89_C2HREG_MAX];
+ struct rtw89_c2hreg_hdr hdr;
+ struct rtw89_c2hreg_phycap phycap;
+ } u;
};

struct rtw89_mac_h2c_info {
u8 id;
u8 content_len;
- u32 h2creg[RTW89_H2CREG_MAX];
+ union {
+ u32 h2creg[RTW89_H2CREG_MAX];
+ struct rtw89_h2creg_hdr hdr;
+ struct rtw89_h2creg_sch_tx_en sch_tx_en;
+ } u;
};

enum rtw89_mac_h2c_type {
@@ -63,33 +107,6 @@ enum rtw89_mac_c2h_type {
RTW89_FWCMD_C2HREG_FUNC_NULL = 0xFF
};

-#define RTW89_GET_C2H_PHYCAP_FUNC(info) \
- u32_get_bits(*((const u32 *)(info)), GENMASK(6, 0))
-#define RTW89_GET_C2H_PHYCAP_ACK(info) \
- u32_get_bits(*((const u32 *)(info)), BIT(7))
-#define RTW89_GET_C2H_PHYCAP_LEN(info) \
- u32_get_bits(*((const u32 *)(info)), GENMASK(11, 8))
-#define RTW89_GET_C2H_PHYCAP_SEQ(info) \
- u32_get_bits(*((const u32 *)(info)), GENMASK(15, 12))
-#define RTW89_GET_C2H_PHYCAP_RX_NSS(info) \
- u32_get_bits(*((const u32 *)(info)), GENMASK(23, 16))
-#define RTW89_GET_C2H_PHYCAP_BW(info) \
- u32_get_bits(*((const u32 *)(info)), GENMASK(31, 24))
-#define RTW89_GET_C2H_PHYCAP_TX_NSS(info) \
- u32_get_bits(*((const u32 *)(info) + 1), GENMASK(7, 0))
-#define RTW89_GET_C2H_PHYCAP_PROT(info) \
- u32_get_bits(*((const u32 *)(info) + 1), GENMASK(15, 8))
-#define RTW89_GET_C2H_PHYCAP_NIC(info) \
- u32_get_bits(*((const u32 *)(info) + 1), GENMASK(23, 16))
-#define RTW89_GET_C2H_PHYCAP_WL_FUNC(info) \
- u32_get_bits(*((const u32 *)(info) + 1), GENMASK(31, 24))
-#define RTW89_GET_C2H_PHYCAP_HW_TYPE(info) \
- u32_get_bits(*((const u32 *)(info) + 2), GENMASK(7, 0))
-#define RTW89_GET_C2H_PHYCAP_ANT_TX_NUM(info) \
- u32_get_bits(*((const u32 *)(info) + 3), GENMASK(15, 8))
-#define RTW89_GET_C2H_PHYCAP_ANT_RX_NUM(info) \
- u32_get_bits(*((const u32 *)(info) + 3), GENMASK(23, 16))
-
enum rtw89_fw_c2h_category {
RTW89_C2H_CAT_TEST,
RTW89_C2H_CAT_MAC,
@@ -214,17 +231,6 @@ struct rtw89_fw_macid_pause_grp {
__le32 mask_grp[4];
} __packed;

-struct rtw89_h2creg_sch_tx_en {
- u8 func:7;
- u8 ack:1;
- u8 total_len:4;
- u8 seq_num:4;
- u16 tx_en:16;
- u16 mask:16;
- u8 band:1;
- u16 rsvd:15;
-} __packed;
-
#define RTW89_H2C_MAX_SIZE 2048
#define RTW89_CHANNEL_TIME 45
#define RTW89_CHANNEL_TIME_6G 20
diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
index acba53cf7cc31..92274892e01cc 100644
--- a/drivers/net/wireless/realtek/rtw89/mac.c
+++ b/drivers/net/wireless/realtek/rtw89/mac.c
@@ -2646,6 +2646,7 @@ int rtw89_mac_setup_phycap(struct rtw89_dev *rtwdev)
struct rtw89_hal *hal = &rtwdev->hal;
const struct rtw89_chip_info *chip = rtwdev->chip;
struct rtw89_mac_c2h_info c2h_info = {0};
+ const struct rtw89_c2hreg_phycap *phycap;
u8 tx_nss;
u8 rx_nss;
u8 tx_ant;
@@ -2656,10 +2657,12 @@ int rtw89_mac_setup_phycap(struct rtw89_dev *rtwdev)
if (ret)
return ret;

- tx_nss = RTW89_GET_C2H_PHYCAP_TX_NSS(c2h_info.c2hreg);
- rx_nss = RTW89_GET_C2H_PHYCAP_RX_NSS(c2h_info.c2hreg);
- tx_ant = RTW89_GET_C2H_PHYCAP_ANT_TX_NUM(c2h_info.c2hreg);
- rx_ant = RTW89_GET_C2H_PHYCAP_ANT_RX_NUM(c2h_info.c2hreg);
+ phycap = &c2h_info.u.phycap;
+
+ tx_nss = u32_get_bits(phycap->w1, RTW89_C2HREG_PHYCAP_W1_TX_NSS);
+ rx_nss = u32_get_bits(phycap->w0, RTW89_C2HREG_PHYCAP_W0_RX_NSS);
+ tx_ant = u32_get_bits(phycap->w3, RTW89_C2HREG_PHYCAP_W3_ANT_TX_NUM);
+ rx_ant = u32_get_bits(phycap->w3, RTW89_C2HREG_PHYCAP_W3_ANT_RX_NUM);

hal->tx_nss = tx_nss ? min_t(u8, tx_nss, chip->tx_nss) : chip->tx_nss;
hal->rx_nss = rx_nss ? min_t(u8, rx_nss, chip->rx_nss) : chip->rx_nss;
@@ -2700,14 +2703,14 @@ static int rtw89_hw_sch_tx_en_h2c(struct rtw89_dev *rtwdev, u8 band,
u32 ret;
struct rtw89_mac_c2h_info c2h_info = {0};
struct rtw89_mac_h2c_info h2c_info = {0};
- struct rtw89_h2creg_sch_tx_en *h2creg =
- (struct rtw89_h2creg_sch_tx_en *)h2c_info.h2creg;
+ struct rtw89_h2creg_sch_tx_en *sch_tx_en = &h2c_info.u.sch_tx_en;

h2c_info.id = RTW89_FWCMD_H2CREG_FUNC_SCH_TX_EN;
- h2c_info.content_len = sizeof(*h2creg) - RTW89_H2CREG_HDR_LEN;
- h2creg->tx_en = tx_en_u16;
- h2creg->mask = mask_u16;
- h2creg->band = band;
+ h2c_info.content_len = sizeof(*sch_tx_en) - RTW89_H2CREG_HDR_LEN;
+
+ u32p_replace_bits(&sch_tx_en->w0, tx_en_u16, RTW89_H2CREG_SCH_TX_EN_W0_EN);
+ u32p_replace_bits(&sch_tx_en->w1, mask_u16, RTW89_H2CREG_SCH_TX_EN_W1_MASK);
+ u32p_replace_bits(&sch_tx_en->w1, band, RTW89_H2CREG_SCH_TX_EN_W1_BAND);

ret = rtw89_fw_msg_reg(rtwdev, &h2c_info, &c2h_info);
if (ret)
--
2.25.1


2023-05-22 12:37:03

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 2/5] wifi: rtw89: use struct and le32_get_bits to access RX info

If received packet type is PPDU status, RX info provides information
attached by MAC hardware, and mention how long BB information attached.

This conversion patch doesn't change logic at all.

Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw89/core.c | 7 +--
drivers/net/wireless/realtek/rtw89/txrx.h | 53 +++++++++++------------
2 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
index ae547b38dfaa8..c92a83bf2f36f 100644
--- a/drivers/net/wireless/realtek/rtw89/core.c
+++ b/drivers/net/wireless/realtek/rtw89/core.c
@@ -1213,14 +1213,15 @@ static int rtw89_core_rx_process_mac_ppdu(struct rtw89_dev *rtwdev,
struct sk_buff *skb,
struct rtw89_rx_phy_ppdu *phy_ppdu)
{
+ const struct rtw89_rxinfo *rxinfo = (const struct rtw89_rxinfo *)skb->data;
bool rx_cnt_valid = false;
u8 plcp_size = 0;
u8 usr_num = 0;
u8 *phy_sts;

- rx_cnt_valid = RTW89_GET_RXINFO_RX_CNT_VLD(skb->data);
- plcp_size = RTW89_GET_RXINFO_PLCP_LEN(skb->data) << 3;
- usr_num = RTW89_GET_RXINFO_USR_NUM(skb->data);
+ rx_cnt_valid = le32_get_bits(rxinfo->w0, RTW89_RXINFO_W0_RX_CNT_VLD);
+ plcp_size = le32_get_bits(rxinfo->w1, RTW89_RXINFO_W1_PLCP_LEN) << 3;
+ usr_num = le32_get_bits(rxinfo->w0, RTW89_RXINFO_W0_USR_NUM);
if (usr_num > RTW89_PPDU_MAX_USR) {
rtw89_warn(rtwdev, "Invalid user number in mac info\n");
return -EINVAL;
diff --git a/drivers/net/wireless/realtek/rtw89/txrx.h b/drivers/net/wireless/realtek/rtw89/txrx.h
index d880ecb879ca8..d096d16c8d3be 100644
--- a/drivers/net/wireless/realtek/rtw89/txrx.h
+++ b/drivers/net/wireless/realtek/rtw89/txrx.h
@@ -251,34 +251,31 @@
#define RTW89_GET_RXWD_SEC_CAM_ID(rxdesc) \
le32_get_bits((rxdesc)->dword5, GENMASK(7, 0))

-#define RTW89_GET_RXINFO_USR_NUM(rpt) \
- le32_get_bits(*((const __le32 *)rpt), GENMASK(3, 0))
-#define RTW89_GET_RXINFO_FW_DEFINE(rpt) \
- le32_get_bits(*((const __le32 *)rpt), GENMASK(15, 8))
-#define RTW89_GET_RXINFO_LSIG_LEN(rpt) \
- le32_get_bits(*((const __le32 *)rpt), GENMASK(27, 16))
-#define RTW89_GET_RXINFO_IS_TO_SELF(rpt) \
- le32_get_bits(*((const __le32 *)rpt), BIT(28))
-#define RTW89_GET_RXINFO_RX_CNT_VLD(rpt) \
- le32_get_bits(*((const __le32 *)rpt), BIT(29))
-#define RTW89_GET_RXINFO_LONG_RXD(rpt) \
- le32_get_bits(*((const __le32 *)rpt), GENMASK(31, 30))
-#define RTW89_GET_RXINFO_SERVICE(rpt) \
- le32_get_bits(*((const __le32 *)(rpt) + 1), GENMASK(15, 0))
-#define RTW89_GET_RXINFO_PLCP_LEN(rpt) \
- le32_get_bits(*((const __le32 *)(rpt) + 1), GENMASK(23, 16))
-#define RTW89_GET_RXINFO_MAC_ID_VALID(rpt, usr) \
- le32_get_bits(*((const __le32 *)(rpt) + (usr) + 2), BIT(0))
-#define RTW89_GET_RXINFO_DATA(rpt, usr) \
- le32_get_bits(*((const __le32 *)(rpt) + (usr) + 2), BIT(1))
-#define RTW89_GET_RXINFO_CTRL(rpt, usr) \
- le32_get_bits(*((const __le32 *)(rpt) + (usr) + 2), BIT(2))
-#define RTW89_GET_RXINFO_MGMT(rpt, usr) \
- le32_get_bits(*((const __le32 *)(rpt) + (usr) + 2), BIT(3))
-#define RTW89_GET_RXINFO_BCM(rpt, usr) \
- le32_get_bits(*((const __le32 *)(rpt) + (usr) + 2), BIT(4))
-#define RTW89_GET_RXINFO_MACID(rpt, usr) \
- le32_get_bits(*((const __le32 *)(rpt) + (usr) + 2), GENMASK(15, 8))
+struct rtw89_rxinfo_user {
+ __le32 w0;
+};
+
+#define RTW89_RXINFO_USER_MAC_ID_VALID BIT(0)
+#define RTW89_RXINFO_USER_DATA BIT(1)
+#define RTW89_RXINFO_USER_CTRL BIT(2)
+#define RTW89_RXINFO_USER_MGMT BIT(3)
+#define RTW89_RXINFO_USER_BCM BIT(4)
+#define RTW89_RXINFO_USER_MACID GENMASK(15, 8)
+
+struct rtw89_rxinfo {
+ __le32 w0;
+ __le32 w1;
+ struct rtw89_rxinfo_user user[];
+} __packed;
+
+#define RTW89_RXINFO_W0_USR_NUM GENMASK(3, 0)
+#define RTW89_RXINFO_W0_FW_DEFINE GENMASK(15, 8)
+#define RTW89_RXINFO_W0_LSIG_LEN GENMASK(27, 16)
+#define RTW89_RXINFO_W0_IS_TO_SELF BIT(28)
+#define RTW89_RXINFO_W0_RX_CNT_VLD BIT(29)
+#define RTW89_RXINFO_W0_LONG_RXD GENMASK(31, 30)
+#define RTW89_RXINFO_W1_SERVICE GENMASK(15, 0)
+#define RTW89_RXINFO_W1_PLCP_LEN GENMASK(23, 16)

#define RTW89_GET_PHY_STS_IE_MAP(sts) \
le32_get_bits(*((const __le32 *)(sts)), GENMASK(4, 0))
--
2.25.1


2023-05-25 16:09:31

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 5/5] wifi: rtw89: use struct to access register-based H2C/C2H

Ping-Ke Shih <[email protected]> writes:

> The register-based H2C/C2H are used to exchange commands and events with
> firmware. The exchange data is limited, but it is relatively simple,
> because it can work before HCI initialization. To make these code clean,
> use struct to access them. This patch doesn't change logic at all.
>
> Signed-off-by: Ping-Ke Shih <[email protected]>

[...]

> --- a/drivers/net/wireless/realtek/rtw89/fw.h
> +++ b/drivers/net/wireless/realtek/rtw89/fw.h
> @@ -18,15 +18,51 @@ enum rtw89_fw_dl_status {
> RTW89_FWDL_WCPU_FW_INIT_RDY = 7
> };
>
> -#define RTW89_GET_C2H_HDR_FUNC(info) \
> - u32_get_bits(info, GENMASK(6, 0))
> -#define RTW89_GET_C2H_HDR_LEN(info) \
> - u32_get_bits(info, GENMASK(11, 8))
> +struct rtw89_c2hreg_hdr {
> + u32 w0;
> +};

Why this is u32? Shouldn't it be __le32?

> +#define RTW89_C2HREG_HDR_FUNC_MASK GENMASK(6, 0)
> +#define RTW89_C2HREG_HDR_ACK BIT(7)
> +#define RTW89_C2HREG_HDR_LEN_MASK GENMASK(11, 8)
> +#define RTW89_C2HREG_HDR_SEQ_MASK GENMASK(15, 12)
> +
> +struct rtw89_c2hreg_phycap {
> + u32 w0;
> + u32 w1;
> + u32 w2;
> + u32 w3;
> +} __packed;

Here as well? And I saw more in the patch.

Of course these were already there so isn't a problem introduced by this
patchset, but I started wondering if we are missing some little endian
types?

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

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

2023-05-25 16:13:57

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/5] wifi: rtw89: add chip_ops::query_rxdesc() and rxd_len as helpers to support newer chips

Ping-Ke Shih <[email protected]> wrote:

> The next generation chips use different RX descriptor format, so add
> a chip_ops to hook suitable handlers. Also, the length of RX descriptor is
> different, so add a variable to store the length according to chip and
> descriptor content dynamically. Then, the code can be more general.
>
> Signed-off-by: Ping-Ke Shih <[email protected]>

5 patches applied to wireless-next.git, thanks.

de9f93385d0f wifi: rtw89: add chip_ops::query_rxdesc() and rxd_len as helpers to support newer chips
88bdc3ff956c wifi: rtw89: use struct and le32_get_bits to access RX info
332debb80488 wifi: rtw89: use struct and le32_get_bits() to access received PHY status IEs
c26700d2df01 wifi: rtw89: use struct and le32_get_bits() to access RX descriptor
68012b44dfc7 wifi: rtw89: use struct to access register-based H2C/C2H

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

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


2023-05-26 11:51:10

by Ping-Ke Shih

[permalink] [raw]
Subject: Re: [PATCH 5/5] wifi: rtw89: use struct to access register-based H2C/C2H

On Thu, 2023-05-25 at 19:07 +0300, Kalle Valo wrote:
>
> Ping-Ke Shih <[email protected]> writes:
>
> > The register-based H2C/C2H are used to exchange commands and events with
> > firmware. The exchange data is limited, but it is relatively simple,
> > because it can work before HCI initialization. To make these code clean,
> > use struct to access them. This patch doesn't change logic at all.
> >
> > Signed-off-by: Ping-Ke Shih <[email protected]>
>
> [...]
>
> > --- a/drivers/net/wireless/realtek/rtw89/fw.h
> > +++ b/drivers/net/wireless/realtek/rtw89/fw.h
> > @@ -18,15 +18,51 @@ enum rtw89_fw_dl_status {
> > RTW89_FWDL_WCPU_FW_INIT_RDY = 7
> > };
> >
> > -#define RTW89_GET_C2H_HDR_FUNC(info) \
> > - u32_get_bits(info, GENMASK(6, 0))
> > -#define RTW89_GET_C2H_HDR_LEN(info) \
> > - u32_get_bits(info, GENMASK(11, 8))
> > +struct rtw89_c2hreg_hdr {
> > + u32 w0;
> > +};
>
> Why this is u32? Shouldn't it be __le32?
>
> > +#define RTW89_C2HREG_HDR_FUNC_MASK GENMASK(6, 0)
> > +#define RTW89_C2HREG_HDR_ACK BIT(7)
> > +#define RTW89_C2HREG_HDR_LEN_MASK GENMASK(11, 8)
> > +#define RTW89_C2HREG_HDR_SEQ_MASK GENMASK(15, 12)
> > +
> > +struct rtw89_c2hreg_phycap {
> > + u32 w0;
> > + u32 w1;
> > + u32 w2;
> > + u32 w3;
> > +} __packed;
>
> Here as well? And I saw more in the patch.
>
> Of course these were already there so isn't a problem introduced by this
> patchset, but I started wondering if we are missing some little endian
> types?
>

I had the same question as yours when I did this conversion, but they
are correct because we access these H2C commands/C2H events via registers
which are CPU order.

A bug I found is that use struct bit-field to access these data. This
will cause big-endian machine wrong, and I fix them by this patch as well.

I hope we don't miss something.

Ping-Ke


2023-05-26 11:51:31

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 5/5] wifi: rtw89: use struct to access register-based H2C/C2H

Ping-Ke Shih <[email protected]> writes:

> On Thu, 2023-05-25 at 19:07 +0300, Kalle Valo wrote:
>
>>
>> Ping-Ke Shih <[email protected]> writes:
>>
>> > The register-based H2C/C2H are used to exchange commands and events with
>> > firmware. The exchange data is limited, but it is relatively simple,
>> > because it can work before HCI initialization. To make these code clean,
>> > use struct to access them. This patch doesn't change logic at all.
>> >
>> > Signed-off-by: Ping-Ke Shih <[email protected]>
>>
>> [...]
>>
>> > --- a/drivers/net/wireless/realtek/rtw89/fw.h
>> > +++ b/drivers/net/wireless/realtek/rtw89/fw.h
>> > @@ -18,15 +18,51 @@ enum rtw89_fw_dl_status {
>> > RTW89_FWDL_WCPU_FW_INIT_RDY = 7
>> > };
>> >
>> > -#define RTW89_GET_C2H_HDR_FUNC(info) \
>> > - u32_get_bits(info, GENMASK(6, 0))
>> > -#define RTW89_GET_C2H_HDR_LEN(info) \
>> > - u32_get_bits(info, GENMASK(11, 8))
>> > +struct rtw89_c2hreg_hdr {
>> > + u32 w0;
>> > +};
>>
>> Why this is u32? Shouldn't it be __le32?
>>
>> > +#define RTW89_C2HREG_HDR_FUNC_MASK GENMASK(6, 0)
>> > +#define RTW89_C2HREG_HDR_ACK BIT(7)
>> > +#define RTW89_C2HREG_HDR_LEN_MASK GENMASK(11, 8)
>> > +#define RTW89_C2HREG_HDR_SEQ_MASK GENMASK(15, 12)
>> > +
>> > +struct rtw89_c2hreg_phycap {
>> > + u32 w0;
>> > + u32 w1;
>> > + u32 w2;
>> > + u32 w3;
>> > +} __packed;
>>
>> Here as well? And I saw more in the patch.
>>
>> Of course these were already there so isn't a problem introduced by this
>> patchset, but I started wondering if we are missing some little endian
>> types?
>>
>
> I had the same question as yours when I did this conversion, but they
> are correct because we access these H2C commands/C2H events via registers
> which are CPU order.

Ah, thanks for the explanation.

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

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