2023-06-16 04:16:46

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 0/6] wifi: rtw88: correct AP and PS mode behaviors

In AP mode, HI queue is used to transmit broadcast/multicast right after
issuing beacon, and we need set MORE_DATA in TX description to make
hardware transmits all packets in one go. Also, have special deal with
this queue when doing scan and flush.

After stopping AP from SCC (AP + STA), only one STA is working, so PS mode
is expected to enable. However, firmware checks the MAC port used by AP
mode before, and then can't enter PS mode. We add the last patch to set
"default port" to the port used by STA. Then, firmware will check correct
port to enter PS mode in expectation.

Po-Hao Huang (6):
wifi: rtw88: use struct instead of macros to set TX desc
wifi: rtw88: Fix AP mode incorrect DTIM behavior
wifi: rtw88: Skip high queue in hci_flush
wifi: rtw88: Stop high queue during scan
wifi: rtw88: refine register based H2C command
wifi: rtw88: fix not entering PS mode after AP stops

drivers/net/wireless/realtek/rtw88/fw.c | 68 ++++++++++
drivers/net/wireless/realtek/rtw88/fw.h | 13 ++
drivers/net/wireless/realtek/rtw88/mac80211.c | 3 +
drivers/net/wireless/realtek/rtw88/main.c | 15 ++-
drivers/net/wireless/realtek/rtw88/main.h | 1 +
drivers/net/wireless/realtek/rtw88/pci.c | 5 +-
drivers/net/wireless/realtek/rtw88/reg.h | 2 +
drivers/net/wireless/realtek/rtw88/rtw8723d.c | 5 +-
drivers/net/wireless/realtek/rtw88/tx.c | 84 +++++++-----
drivers/net/wireless/realtek/rtw88/tx.h | 122 +++++++-----------
drivers/net/wireless/realtek/rtw88/usb.c | 15 ++-
11 files changed, 216 insertions(+), 117 deletions(-)

--
2.25.1



2023-06-16 04:16:46

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 2/6] wifi: rtw88: Fix AP mode incorrect DTIM behavior

From: Po-Hao Huang <[email protected]>

Broadcast and multicast packets in high queue should be transmitted
all at once during DTIM. But without proper settings, hardware fails
to recognize that there are multiple packets and fetches only one.
Fix this by signaling hardware with more data bit set when there are
packets in the high queue.

Signed-off-by: Po-Hao Huang <[email protected]>
Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw88/mac80211.c | 2 ++
drivers/net/wireless/realtek/rtw88/reg.h | 1 +
drivers/net/wireless/realtek/rtw88/tx.c | 7 ++++++-
drivers/net/wireless/realtek/rtw88/tx.h | 1 +
drivers/net/wireless/realtek/rtw88/usb.c | 3 +++
5 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 09bcc2345bb05..0dc83c2f420da 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -449,6 +449,7 @@ static int rtw_ops_start_ap(struct ieee80211_hw *hw,
const struct rtw_chip_info *chip = rtwdev->chip;

mutex_lock(&rtwdev->mutex);
+ rtw_write32_set(rtwdev, REG_TCR, BIT_TCR_UPDATE_HGQMD);
rtwdev->ap_active = true;
rtw_store_op_chan(rtwdev, true);
chip->ops->phy_calibration(rtwdev);
@@ -464,6 +465,7 @@ static void rtw_ops_stop_ap(struct ieee80211_hw *hw,
struct rtw_dev *rtwdev = hw->priv;

mutex_lock(&rtwdev->mutex);
+ rtw_write32_clr(rtwdev, REG_TCR, BIT_TCR_UPDATE_HGQMD);
rtwdev->ap_active = false;
if (!rtw_core_check_sta_active(rtwdev))
rtw_clear_op_chan(rtwdev);
diff --git a/drivers/net/wireless/realtek/rtw88/reg.h b/drivers/net/wireless/realtek/rtw88/reg.h
index 2a2ae2081f347..60de9de1cc7a8 100644
--- a/drivers/net/wireless/realtek/rtw88/reg.h
+++ b/drivers/net/wireless/realtek/rtw88/reg.h
@@ -410,6 +410,7 @@
#define REG_TCR 0x0604
#define BIT_PWRMGT_HWDATA_EN BIT(7)
#define BIT_TCR_UPDATE_TIMIE BIT(5)
+#define BIT_TCR_UPDATE_HGQMD BIT(4)
#define REG_RCR 0x0608
#define BIT_APP_FCS BIT(31)
#define BIT_APP_MIC BIT(30)
diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
index edf351102f6a9..702e1fbc2ca62 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -35,6 +35,10 @@ void rtw_tx_stats(struct rtw_dev *rtwdev, struct ieee80211_vif *vif,
void rtw_tx_fill_tx_desc(struct rtw_tx_pkt_info *pkt_info, struct sk_buff *skb)
{
struct rtw_tx_desc *tx_desc = (struct rtw_tx_desc *)skb->data;
+ bool more_data = false;
+
+ if (pkt_info->qsel == TX_DESC_QSEL_HIGH)
+ more_data = true;

tx_desc->w0 = le32_encode_bits(pkt_info->tx_pkt_size, RTW_TX_DESC_W0_TXPKTSIZE) |
le32_encode_bits(pkt_info->offset, RTW_TX_DESC_W0_OFFSET) |
@@ -45,7 +49,8 @@ void rtw_tx_fill_tx_desc(struct rtw_tx_pkt_info *pkt_info, struct sk_buff *skb)
tx_desc->w1 = le32_encode_bits(pkt_info->qsel, RTW_TX_DESC_W1_QSEL) |
le32_encode_bits(pkt_info->rate_id, RTW_TX_DESC_W1_RATE_ID) |
le32_encode_bits(pkt_info->sec_type, RTW_TX_DESC_W1_SEC_TYPE) |
- le32_encode_bits(pkt_info->pkt_offset, RTW_TX_DESC_W1_PKT_OFFSET);
+ le32_encode_bits(pkt_info->pkt_offset, RTW_TX_DESC_W1_PKT_OFFSET) |
+ le32_encode_bits(more_data, RTW_TX_DESC_W1_MORE_DATA);

tx_desc->w2 = le32_encode_bits(pkt_info->ampdu_en, RTW_TX_DESC_W2_AGG_EN) |
le32_encode_bits(pkt_info->report, RTW_TX_DESC_W2_SPE_RPT) |
diff --git a/drivers/net/wireless/realtek/rtw88/tx.h b/drivers/net/wireless/realtek/rtw88/tx.h
index c7ad41d3379e5..0dc0885ea3168 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.h
+++ b/drivers/net/wireless/realtek/rtw88/tx.h
@@ -31,6 +31,7 @@ struct rtw_tx_desc {
#define RTW_TX_DESC_W1_RATE_ID GENMASK(20, 16)
#define RTW_TX_DESC_W1_SEC_TYPE GENMASK(23, 22)
#define RTW_TX_DESC_W1_PKT_OFFSET GENMASK(28, 24)
+#define RTW_TX_DESC_W1_MORE_DATA BIT(29)
#define RTW_TX_DESC_W2_AGG_EN BIT(12)
#define RTW_TX_DESC_W2_SPE_RPT BIT(19)
#define RTW_TX_DESC_W2_AMPDU_DEN GENMASK(22, 20)
diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index dbfd5576cc120..1ab8950620ccf 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -469,6 +469,9 @@ static u8 rtw_usb_tx_queue_mapping_to_qsel(struct sk_buff *skb)

if (unlikely(ieee80211_is_mgmt(fc) || ieee80211_is_ctl(fc)))
qsel = TX_DESC_QSEL_MGMT;
+ else if (is_broadcast_ether_addr(hdr->addr1) ||
+ is_multicast_ether_addr(hdr->addr1))
+ qsel = TX_DESC_QSEL_HIGH;
else if (skb_get_queue_mapping(skb) <= IEEE80211_AC_BK)
qsel = skb->priority;
else
--
2.25.1


2023-06-16 04:16:46

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 1/6] wifi: rtw88: use struct instead of macros to set TX desc

From: Po-Hao Huang <[email protected]>

Remove macros that set TX descriptors. Use struct and
le32_encode_bits() with mask definitions.

Signed-off-by: Po-Hao Huang <[email protected]>
Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw88/rtw8723d.c | 5 +-
drivers/net/wireless/realtek/rtw88/tx.c | 79 +++++++-----
drivers/net/wireless/realtek/rtw88/tx.h | 121 +++++++-----------
drivers/net/wireless/realtek/rtw88/usb.c | 12 +-
4 files changed, 104 insertions(+), 113 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/rtw8723d.c b/drivers/net/wireless/realtek/rtw88/rtw8723d.c
index cadf66f4e8545..f385a41a32565 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8723d.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8723d.c
@@ -1970,15 +1970,16 @@ static void rtw8723d_fill_txdesc_checksum(struct rtw_dev *rtwdev,
size_t words = 32 / 2; /* calculate the first 32 bytes (16 words) */
__le16 chksum = 0;
__le16 *data = (__le16 *)(txdesc);
+ struct rtw_tx_desc *tx_desc = (struct rtw_tx_desc *)txdesc;

- SET_TX_DESC_TXDESC_CHECKSUM(txdesc, 0x0000);
+ le32_replace_bits(tx_desc->w7, 0, RTW_TX_DESC_W7_TXDESC_CHECKSUM);

while (words--)
chksum ^= *data++;

chksum = ~chksum;

- SET_TX_DESC_TXDESC_CHECKSUM(txdesc, __le16_to_cpu(chksum));
+ le32_replace_bits(tx_desc->w7, __le16_to_cpu(chksum), RTW_TX_DESC_W7_TXDESC_CHECKSUM);
}

static struct rtw_chip_ops rtw8723d_ops = {
diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
index bb5c7492c98b0..edf351102f6a9 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -34,43 +34,52 @@ void rtw_tx_stats(struct rtw_dev *rtwdev, struct ieee80211_vif *vif,

void rtw_tx_fill_tx_desc(struct rtw_tx_pkt_info *pkt_info, struct sk_buff *skb)
{
- __le32 *txdesc = (__le32 *)skb->data;
-
- SET_TX_DESC_TXPKTSIZE(txdesc, pkt_info->tx_pkt_size);
- SET_TX_DESC_OFFSET(txdesc, pkt_info->offset);
- SET_TX_DESC_PKT_OFFSET(txdesc, pkt_info->pkt_offset);
- SET_TX_DESC_QSEL(txdesc, pkt_info->qsel);
- SET_TX_DESC_BMC(txdesc, pkt_info->bmc);
- SET_TX_DESC_RATE_ID(txdesc, pkt_info->rate_id);
- SET_TX_DESC_DATARATE(txdesc, pkt_info->rate);
- SET_TX_DESC_DISDATAFB(txdesc, pkt_info->dis_rate_fallback);
- SET_TX_DESC_USE_RATE(txdesc, pkt_info->use_rate);
- SET_TX_DESC_SEC_TYPE(txdesc, pkt_info->sec_type);
- SET_TX_DESC_DATA_BW(txdesc, pkt_info->bw);
- SET_TX_DESC_SW_SEQ(txdesc, pkt_info->seq);
- SET_TX_DESC_MAX_AGG_NUM(txdesc, pkt_info->ampdu_factor);
- SET_TX_DESC_AMPDU_DENSITY(txdesc, pkt_info->ampdu_density);
- SET_TX_DESC_DATA_STBC(txdesc, pkt_info->stbc);
- SET_TX_DESC_DATA_LDPC(txdesc, pkt_info->ldpc);
- SET_TX_DESC_AGG_EN(txdesc, pkt_info->ampdu_en);
- SET_TX_DESC_LS(txdesc, pkt_info->ls);
- SET_TX_DESC_DATA_SHORT(txdesc, pkt_info->short_gi);
- SET_TX_DESC_SPE_RPT(txdesc, pkt_info->report);
- SET_TX_DESC_SW_DEFINE(txdesc, pkt_info->sn);
- SET_TX_DESC_USE_RTS(txdesc, pkt_info->rts);
+ struct rtw_tx_desc *tx_desc = (struct rtw_tx_desc *)skb->data;
+
+ tx_desc->w0 = le32_encode_bits(pkt_info->tx_pkt_size, RTW_TX_DESC_W0_TXPKTSIZE) |
+ le32_encode_bits(pkt_info->offset, RTW_TX_DESC_W0_OFFSET) |
+ le32_encode_bits(pkt_info->bmc, RTW_TX_DESC_W0_BMC) |
+ le32_encode_bits(pkt_info->ls, RTW_TX_DESC_W0_LS) |
+ le32_encode_bits(pkt_info->dis_qselseq, RTW_TX_DESC_W0_DISQSELSEQ);
+
+ tx_desc->w1 = le32_encode_bits(pkt_info->qsel, RTW_TX_DESC_W1_QSEL) |
+ le32_encode_bits(pkt_info->rate_id, RTW_TX_DESC_W1_RATE_ID) |
+ le32_encode_bits(pkt_info->sec_type, RTW_TX_DESC_W1_SEC_TYPE) |
+ le32_encode_bits(pkt_info->pkt_offset, RTW_TX_DESC_W1_PKT_OFFSET);
+
+ tx_desc->w2 = le32_encode_bits(pkt_info->ampdu_en, RTW_TX_DESC_W2_AGG_EN) |
+ le32_encode_bits(pkt_info->report, RTW_TX_DESC_W2_SPE_RPT) |
+ le32_encode_bits(pkt_info->ampdu_density, RTW_TX_DESC_W2_AMPDU_DEN) |
+ le32_encode_bits(pkt_info->bt_null, RTW_TX_DESC_W2_BT_NULL);
+
+ tx_desc->w3 = le32_encode_bits(pkt_info->hw_ssn_sel, RTW_TX_DESC_W3_HW_SSN_SEL) |
+ le32_encode_bits(pkt_info->use_rate, RTW_TX_DESC_W3_USE_RATE) |
+ le32_encode_bits(pkt_info->dis_rate_fallback, RTW_TX_DESC_W3_DISDATAFB) |
+ le32_encode_bits(pkt_info->rts, RTW_TX_DESC_W3_USE_RTS) |
+ le32_encode_bits(pkt_info->nav_use_hdr, RTW_TX_DESC_W3_NAVUSEHDR) |
+ le32_encode_bits(pkt_info->ampdu_factor, RTW_TX_DESC_W3_MAX_AGG_NUM);
+
+ tx_desc->w4 = le32_encode_bits(pkt_info->rate, RTW_TX_DESC_W4_DATARATE);
+
+ tx_desc->w5 = le32_encode_bits(pkt_info->short_gi, RTW_TX_DESC_W5_DATA_SHORT) |
+ le32_encode_bits(pkt_info->bw, RTW_TX_DESC_W5_DATA_BW) |
+ le32_encode_bits(pkt_info->ldpc, RTW_TX_DESC_W5_DATA_LDPC) |
+ le32_encode_bits(pkt_info->stbc, RTW_TX_DESC_W5_DATA_STBC);
+
+ tx_desc->w6 = le32_encode_bits(pkt_info->sn, RTW_TX_DESC_W6_SW_DEFINE);
+
+ tx_desc->w8 = le32_encode_bits(pkt_info->en_hwseq, RTW_TX_DESC_W8_EN_HWSEQ);
+
+ tx_desc->w9 = le32_encode_bits(pkt_info->seq, RTW_TX_DESC_W9_SW_SEQ);
+
if (pkt_info->rts) {
- SET_TX_DESC_RTSRATE(txdesc, DESC_RATE24M);
- SET_TX_DESC_DATA_RTS_SHORT(txdesc, 1);
- }
- SET_TX_DESC_DISQSELSEQ(txdesc, pkt_info->dis_qselseq);
- SET_TX_DESC_EN_HWSEQ(txdesc, pkt_info->en_hwseq);
- SET_TX_DESC_HW_SSN_SEL(txdesc, pkt_info->hw_ssn_sel);
- SET_TX_DESC_NAVUSEHDR(txdesc, pkt_info->nav_use_hdr);
- SET_TX_DESC_BT_NULL(txdesc, pkt_info->bt_null);
- if (pkt_info->tim_offset) {
- SET_TX_DESC_TIM_EN(txdesc, 1);
- SET_TX_DESC_TIM_OFFSET(txdesc, pkt_info->tim_offset);
+ tx_desc->w4 |= le32_encode_bits(DESC_RATE24M, RTW_TX_DESC_W4_RTSRATE);
+ tx_desc->w5 |= le32_encode_bits(1, RTW_TX_DESC_W5_DATA_RTS_SHORT);
}
+
+ if (pkt_info->tim_offset)
+ tx_desc->w9 |= le32_encode_bits(1, RTW_TX_DESC_W9_TIM_EN) |
+ le32_encode_bits(pkt_info->tim_offset, RTW_TX_DESC_W9_TIM_OFFSET);
}
EXPORT_SYMBOL(rtw_tx_fill_tx_desc);

diff --git a/drivers/net/wireless/realtek/rtw88/tx.h b/drivers/net/wireless/realtek/rtw88/tx.h
index 197d5868c8ad9..c7ad41d3379e5 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.h
+++ b/drivers/net/wireless/realtek/rtw88/tx.h
@@ -9,76 +9,52 @@

#define RTW_TX_PROBE_TIMEOUT msecs_to_jiffies(500)

-#define SET_TX_DESC_TXPKTSIZE(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x00, value, GENMASK(15, 0))
-#define SET_TX_DESC_OFFSET(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x00, value, GENMASK(23, 16))
-#define SET_TX_DESC_PKT_OFFSET(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x01, value, GENMASK(28, 24))
-#define SET_TX_DESC_QSEL(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x01, value, GENMASK(12, 8))
-#define SET_TX_DESC_BMC(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x00, value, BIT(24))
-#define SET_TX_DESC_RATE_ID(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x01, value, GENMASK(20, 16))
-#define SET_TX_DESC_DATARATE(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x04, value, GENMASK(6, 0))
-#define SET_TX_DESC_DISDATAFB(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x03, value, BIT(10))
-#define SET_TX_DESC_USE_RATE(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x03, value, BIT(8))
-#define SET_TX_DESC_SEC_TYPE(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x01, value, GENMASK(23, 22))
-#define SET_TX_DESC_DATA_BW(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x05, value, GENMASK(6, 5))
-#define SET_TX_DESC_SW_SEQ(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x09, value, GENMASK(23, 12))
-#define SET_TX_DESC_TIM_EN(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x09, value, BIT(7))
-#define SET_TX_DESC_TIM_OFFSET(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x09, value, GENMASK(6, 0))
-#define SET_TX_DESC_MAX_AGG_NUM(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x03, value, GENMASK(21, 17))
-#define SET_TX_DESC_USE_RTS(tx_desc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x03, value, BIT(12))
-#define SET_TX_DESC_RTSRATE(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x04, value, GENMASK(28, 24))
-#define SET_TX_DESC_DATA_RTS_SHORT(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x05, value, BIT(12))
-#define SET_TX_DESC_AMPDU_DENSITY(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x02, value, GENMASK(22, 20))
-#define SET_TX_DESC_DATA_STBC(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x05, value, GENMASK(9, 8))
-#define SET_TX_DESC_DATA_LDPC(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x05, value, BIT(7))
-#define SET_TX_DESC_AGG_EN(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x02, value, BIT(12))
-#define SET_TX_DESC_LS(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x00, value, BIT(26))
-#define SET_TX_DESC_DATA_SHORT(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x05, value, BIT(4))
-#define SET_TX_DESC_SPE_RPT(tx_desc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x02, value, BIT(19))
-#define SET_TX_DESC_SW_DEFINE(tx_desc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x06, value, GENMASK(11, 0))
-#define SET_TX_DESC_DISQSELSEQ(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x00, value, BIT(31))
-#define SET_TX_DESC_EN_HWSEQ(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x08, value, BIT(15))
-#define SET_TX_DESC_HW_SSN_SEL(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x03, value, GENMASK(7, 6))
-#define SET_TX_DESC_NAVUSEHDR(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x03, value, BIT(15))
-#define SET_TX_DESC_BT_NULL(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x02, value, BIT(23))
-#define SET_TX_DESC_TXDESC_CHECKSUM(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x07, value, GENMASK(15, 0))
-#define SET_TX_DESC_DMA_TXAGG_NUM(txdesc, value) \
- le32p_replace_bits((__le32 *)(txdesc) + 0x07, value, GENMASK(31, 24))
-#define GET_TX_DESC_PKT_OFFSET(txdesc) \
- le32_get_bits(*((__le32 *)(txdesc) + 0x01), GENMASK(28, 24))
-#define GET_TX_DESC_QSEL(txdesc) \
- le32_get_bits(*((__le32 *)(txdesc) + 0x01), GENMASK(12, 8))
+struct rtw_tx_desc {
+ __le32 w0;
+ __le32 w1;
+ __le32 w2;
+ __le32 w3;
+ __le32 w4;
+ __le32 w5;
+ __le32 w6;
+ __le32 w7;
+ __le32 w8;
+ __le32 w9;
+} __packed;
+
+#define RTW_TX_DESC_W0_TXPKTSIZE GENMASK(15, 0)
+#define RTW_TX_DESC_W0_OFFSET GENMASK(23, 16)
+#define RTW_TX_DESC_W0_BMC BIT(24)
+#define RTW_TX_DESC_W0_LS BIT(26)
+#define RTW_TX_DESC_W0_DISQSELSEQ BIT(31)
+#define RTW_TX_DESC_W1_QSEL GENMASK(12, 8)
+#define RTW_TX_DESC_W1_RATE_ID GENMASK(20, 16)
+#define RTW_TX_DESC_W1_SEC_TYPE GENMASK(23, 22)
+#define RTW_TX_DESC_W1_PKT_OFFSET GENMASK(28, 24)
+#define RTW_TX_DESC_W2_AGG_EN BIT(12)
+#define RTW_TX_DESC_W2_SPE_RPT BIT(19)
+#define RTW_TX_DESC_W2_AMPDU_DEN GENMASK(22, 20)
+#define RTW_TX_DESC_W2_BT_NULL BIT(23)
+#define RTW_TX_DESC_W3_HW_SSN_SEL GENMASK(7, 6)
+#define RTW_TX_DESC_W3_USE_RATE BIT(8)
+#define RTW_TX_DESC_W3_DISDATAFB BIT(10)
+#define RTW_TX_DESC_W3_USE_RTS BIT(12)
+#define RTW_TX_DESC_W3_NAVUSEHDR BIT(15)
+#define RTW_TX_DESC_W3_MAX_AGG_NUM GENMASK(21, 17)
+#define RTW_TX_DESC_W4_DATARATE GENMASK(6, 0)
+#define RTW_TX_DESC_W4_RTSRATE GENMASK(28, 24)
+#define RTW_TX_DESC_W5_DATA_SHORT BIT(4)
+#define RTW_TX_DESC_W5_DATA_BW GENMASK(6, 5)
+#define RTW_TX_DESC_W5_DATA_LDPC BIT(7)
+#define RTW_TX_DESC_W5_DATA_STBC GENMASK(9, 8)
+#define RTW_TX_DESC_W5_DATA_RTS_SHORT BIT(12)
+#define RTW_TX_DESC_W6_SW_DEFINE GENMASK(11, 0)
+#define RTW_TX_DESC_W7_TXDESC_CHECKSUM GENMASK(15, 0)
+#define RTW_TX_DESC_W7_DMA_TXAGG_NUM GENMASK(31, 24)
+#define RTW_TX_DESC_W8_EN_HWSEQ BIT(15)
+#define RTW_TX_DESC_W9_SW_SEQ GENMASK(23, 12)
+#define RTW_TX_DESC_W9_TIM_EN BIT(7)
+#define RTW_TX_DESC_W9_TIM_OFFSET GENMASK(6, 0)

enum rtw_tx_desc_queue_select {
TX_DESC_QSEL_TID0 = 0,
@@ -139,13 +115,14 @@ void fill_txdesc_checksum_common(u8 *txdesc, size_t words)
{
__le16 chksum = 0;
__le16 *data = (__le16 *)(txdesc);
+ struct rtw_tx_desc *tx_desc = (struct rtw_tx_desc *)txdesc;

- SET_TX_DESC_TXDESC_CHECKSUM(txdesc, 0x0000);
+ le32_replace_bits(tx_desc->w7, 0, RTW_TX_DESC_W7_TXDESC_CHECKSUM);

while (words--)
chksum ^= *data++;

- SET_TX_DESC_TXDESC_CHECKSUM(txdesc, __le16_to_cpu(chksum));
+ le32_replace_bits(tx_desc->w7, __le16_to_cpu(chksum), RTW_TX_DESC_W7_TXDESC_CHECKSUM);
}

static inline void rtw_tx_fill_txdesc_checksum(struct rtw_dev *rtwdev,
diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 976eafa739a2d..dbfd5576cc120 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -24,11 +24,12 @@ struct rtw_usb_txcb {
static void rtw_usb_fill_tx_checksum(struct rtw_usb *rtwusb,
struct sk_buff *skb, int agg_num)
{
+ struct rtw_tx_desc *tx_desc = (struct rtw_tx_desc *)skb->data;
struct rtw_dev *rtwdev = rtwusb->rtwdev;
struct rtw_tx_pkt_info pkt_info;

- SET_TX_DESC_DMA_TXAGG_NUM(skb->data, agg_num);
- pkt_info.pkt_offset = GET_TX_DESC_PKT_OFFSET(skb->data);
+ le32_replace_bits(tx_desc->w7, agg_num, RTW_TX_DESC_W7_TXDESC_CHECKSUM);
+ pkt_info.pkt_offset = le32_get_bits(tx_desc->w1, RTW_TX_DESC_W1_PKT_OFFSET);
rtw_tx_fill_txdesc_checksum(rtwdev, &pkt_info, skb->data);
}

@@ -306,11 +307,13 @@ static int rtw_usb_write_port(struct rtw_dev *rtwdev, u8 qsel, struct sk_buff *s
static bool rtw_usb_tx_agg_skb(struct rtw_usb *rtwusb, struct sk_buff_head *list)
{
struct rtw_dev *rtwdev = rtwusb->rtwdev;
+ struct rtw_tx_desc *tx_desc;
struct rtw_usb_txcb *txcb;
struct sk_buff *skb_head;
struct sk_buff *skb_iter;
int agg_num = 0;
unsigned int align_next = 0;
+ u8 qsel;

if (skb_queue_empty(list))
return false;
@@ -363,9 +366,10 @@ static bool rtw_usb_tx_agg_skb(struct rtw_usb *rtwusb, struct sk_buff_head *list

queue:
skb_queue_tail(&txcb->tx_ack_queue, skb_head);
+ tx_desc = (struct rtw_tx_desc *)skb_head->data;
+ qsel = le32_get_bits(tx_desc->w1, RTW_TX_DESC_W1_QSEL);

- rtw_usb_write_port(rtwdev, GET_TX_DESC_QSEL(skb_head->data), skb_head,
- rtw_usb_write_port_tx_complete, txcb);
+ rtw_usb_write_port(rtwdev, qsel, skb_head, rtw_usb_write_port_tx_complete, txcb);

return true;
}
--
2.25.1


2023-06-16 04:16:48

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 3/6] wifi: rtw88: Skip high queue in hci_flush

From: Po-Hao Huang <[email protected]>

The flush period may not always intersect with DTIM and when that
happens, an error log "timed out to flush pci TX ring[6]" is shown.
Bypass this since hardware will do proper transmission on the next
DTIM period for broadcast/multicast packets in high queue.

Signed-off-by: Po-Hao Huang <[email protected]>
Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw88/pci.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 672ddde808160..44a8fff34cddf 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -738,8 +738,9 @@ static void __rtw_pci_flush_queues(struct rtw_dev *rtwdev, u32 pci_queues,
u8 q;

for (q = 0; q < RTK_MAX_TX_QUEUE_NUM; q++) {
- /* It may be not necessary to flush BCN and H2C tx queues. */
- if (q == RTW_TX_QUEUE_BCN || q == RTW_TX_QUEUE_H2C)
+ /* Unnecessary to flush BCN, H2C and HI tx queues. */
+ if (q == RTW_TX_QUEUE_BCN || q == RTW_TX_QUEUE_H2C ||
+ q == RTW_TX_QUEUE_HI0)
continue;

if (pci_queues & BIT(q))
--
2.25.1


2023-06-16 04:16:50

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 5/6] wifi: rtw88: refine register based H2C command

From: Po-Hao Huang <[email protected]>

Since register based H2C commands don't need endian conversion.
Introduce a new API that don't do conversion and send it directly.
New caller are expected to encode with cpu order and gradually
replace the old ones.

Signed-off-by: Po-Hao Huang <[email protected]>
Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw88/fw.c | 51 +++++++++++++++++++++++++
drivers/net/wireless/realtek/rtw88/fw.h | 5 +++
2 files changed, 56 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 2a8ccc8a7f60d..5e329bb95bb89 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -308,6 +308,57 @@ void rtw_fw_c2h_cmd_isr(struct rtw_dev *rtwdev)
}
EXPORT_SYMBOL(rtw_fw_c2h_cmd_isr);

+static void rtw_fw_send_h2c_command_register(struct rtw_dev *rtwdev,
+ struct rtw_h2c_register *h2c)
+{
+ u32 box_reg, box_ex_reg;
+ u8 box_state, box;
+ int ret;
+
+ rtw_dbg(rtwdev, RTW_DBG_FW, "send H2C content %08x %08x\n", h2c->w0,
+ h2c->w1);
+
+ lockdep_assert_held(&rtwdev->mutex);
+
+ box = rtwdev->h2c.last_box_num;
+ switch (box) {
+ case 0:
+ box_reg = REG_HMEBOX0;
+ box_ex_reg = REG_HMEBOX0_EX;
+ break;
+ case 1:
+ box_reg = REG_HMEBOX1;
+ box_ex_reg = REG_HMEBOX1_EX;
+ break;
+ case 2:
+ box_reg = REG_HMEBOX2;
+ box_ex_reg = REG_HMEBOX2_EX;
+ break;
+ case 3:
+ box_reg = REG_HMEBOX3;
+ box_ex_reg = REG_HMEBOX3_EX;
+ break;
+ default:
+ WARN(1, "invalid h2c mail box number\n");
+ return;
+ }
+
+ ret = read_poll_timeout_atomic(rtw_read8, box_state,
+ !((box_state >> box) & 0x1), 100, 3000,
+ false, rtwdev, REG_HMETFR);
+
+ if (ret) {
+ rtw_err(rtwdev, "failed to send h2c command\n");
+ return;
+ }
+
+ rtw_write32(rtwdev, box_ex_reg, h2c->w1);
+ rtw_write32(rtwdev, box_reg, h2c->w0);
+
+ if (++rtwdev->h2c.last_box_num >= 4)
+ rtwdev->h2c.last_box_num = 0;
+}
+
static void rtw_fw_send_h2c_command(struct rtw_dev *rtwdev,
u8 *h2c)
{
diff --git a/drivers/net/wireless/realtek/rtw88/fw.h b/drivers/net/wireless/realtek/rtw88/fw.h
index 397cbc3f6af6e..11a77d86cd144 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.h
+++ b/drivers/net/wireless/realtek/rtw88/fw.h
@@ -81,6 +81,11 @@ struct rtw_c2h_adaptivity {
u8 option;
} __packed;

+struct rtw_h2c_register {
+ u32 w0;
+ u32 w1;
+} __packed;
+
struct rtw_h2c_cmd {
__le32 msg;
__le32 msg_ext;
--
2.25.1


2023-06-16 04:16:51

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 4/6] wifi: rtw88: Stop high queue during scan

From: Po-Hao Huang <[email protected]>

When traversing channel list, TX in high queue should be disabled
along with beacon function, so packets won't be sent to incorrect
channels.

Signed-off-by: Po-Hao Huang <[email protected]>
Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw88/main.c | 7 +++++--
drivers/net/wireless/realtek/rtw88/reg.h | 1 +
2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 9447a3aae3b5e..d55b041a6bb93 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -2403,10 +2403,13 @@ void rtw_core_enable_beacon(struct rtw_dev *rtwdev, bool enable)
if (!rtwdev->ap_active)
return;

- if (enable)
+ if (enable) {
rtw_write32_set(rtwdev, REG_BCN_CTRL, BIT_EN_BCN_FUNCTION);
- else
+ rtw_write32_clr(rtwdev, REG_TXPAUSE, BIT_HIGH_QUEUE);
+ } else {
rtw_write32_clr(rtwdev, REG_BCN_CTRL, BIT_EN_BCN_FUNCTION);
+ rtw_write32_set(rtwdev, REG_TXPAUSE, BIT_HIGH_QUEUE);
+ }
}

MODULE_AUTHOR("Realtek Corporation");
diff --git a/drivers/net/wireless/realtek/rtw88/reg.h b/drivers/net/wireless/realtek/rtw88/reg.h
index 60de9de1cc7a8..7c6c11d50ff30 100644
--- a/drivers/net/wireless/realtek/rtw88/reg.h
+++ b/drivers/net/wireless/realtek/rtw88/reg.h
@@ -378,6 +378,7 @@
#define BIT_SIFS_BK_EN BIT(12)
#define REG_TXPAUSE 0x0522
#define BIT_AC_QUEUE GENMASK(7, 0)
+#define BIT_HIGH_QUEUE BIT(5)
#define REG_RD_CTRL 0x0524
#define BIT_EDCCA_MSK_CNTDOWN_EN BIT(11)
#define BIT_DIS_TXOP_CFE BIT(10)
--
2.25.1


2023-06-16 04:16:52

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 6/6] wifi: rtw88: fix not entering PS mode after AP stops

From: Po-Hao Huang <[email protected]>

Without this patch, firmware only track beacons for port 0 and since
we will always start AP on port 0, this results in misbehavior of
power saving mode on other ports after AP stops.

The "default port" H2C command is used to notify which port should
firmware track. Update the correct settings to firmware so power
saving mode can work properly.

Signed-off-by: Po-Hao Huang <[email protected]>
Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw88/fw.c | 17 +++++++++++++++++
drivers/net/wireless/realtek/rtw88/fw.h | 8 ++++++++
drivers/net/wireless/realtek/rtw88/mac80211.c | 1 +
drivers/net/wireless/realtek/rtw88/main.c | 8 ++++++++
drivers/net/wireless/realtek/rtw88/main.h | 1 +
5 files changed, 35 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 5e329bb95bb89..567bbedd8ee09 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -519,6 +519,23 @@ void rtw_fw_query_bt_info(struct rtw_dev *rtwdev)
rtw_fw_send_h2c_command(rtwdev, h2c_pkt);
}

+void rtw_fw_default_port(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif)
+{
+ struct rtw_h2c_register h2c = {};
+
+ if (rtwvif->net_type != RTW_NET_MGD_LINKED)
+ return;
+
+ /* Leave LPS before default port H2C so FW timer is correct */
+ rtw_leave_lps(rtwdev);
+
+ h2c.w0 = u32_encode_bits(H2C_CMD_DEFAULT_PORT, RTW_H2C_W0_CMDID) |
+ u32_encode_bits(rtwvif->port, RTW_H2C_DEFAULT_PORT_W0_PORTID) |
+ u32_encode_bits(rtwvif->mac_id, RTW_H2C_DEFAULT_PORT_W0_MACID);
+
+ rtw_fw_send_h2c_command_register(rtwdev, &h2c);
+}
+
void rtw_fw_wl_ch_info(struct rtw_dev *rtwdev, u8 link, u8 ch, u8 bw)
{
u8 h2c_pkt[H2C_PKT_SIZE] = {0};
diff --git a/drivers/net/wireless/realtek/rtw88/fw.h b/drivers/net/wireless/realtek/rtw88/fw.h
index 11a77d86cd144..43ccdf9965ac4 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.h
+++ b/drivers/net/wireless/realtek/rtw88/fw.h
@@ -86,6 +86,12 @@ struct rtw_h2c_register {
u32 w1;
} __packed;

+#define RTW_H2C_W0_CMDID GENMASK(7, 0)
+
+/* H2C_CMD_DEFAULT_PORT command */
+#define RTW_H2C_DEFAULT_PORT_W0_PORTID GENMASK(15, 8)
+#define RTW_H2C_DEFAULT_PORT_W0_MACID GENMASK(23, 16)
+
struct rtw_h2c_cmd {
__le32 msg;
__le32 msg_ext;
@@ -535,6 +541,7 @@ static inline void rtw_h2c_pkt_set_header(u8 *h2c_pkt, u8 sub_id)
#define H2C_CMD_MEDIA_STATUS_RPT 0x01
#define H2C_CMD_SET_PWR_MODE 0x20
#define H2C_CMD_LPS_PG_INFO 0x2b
+#define H2C_CMD_DEFAULT_PORT 0x2c
#define H2C_CMD_RA_INFO 0x40
#define H2C_CMD_RSSI_MONITOR 0x42
#define H2C_CMD_BCN_FILTER_OFFLOAD_P0 0x56
@@ -806,6 +813,7 @@ void rtw_fw_c2h_cmd_rx_irqsafe(struct rtw_dev *rtwdev, u32 pkt_offset,
void rtw_fw_c2h_cmd_handle(struct rtw_dev *rtwdev, struct sk_buff *skb);
void rtw_fw_send_general_info(struct rtw_dev *rtwdev);
void rtw_fw_send_phydm_info(struct rtw_dev *rtwdev);
+void rtw_fw_default_port(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif);

void rtw_fw_do_iqk(struct rtw_dev *rtwdev, struct rtw_iqk_para *para);
void rtw_fw_inform_rfk_status(struct rtw_dev *rtwdev, bool start);
diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 0dc83c2f420da..c5a07285fa541 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -378,6 +378,7 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,

rtw_fw_download_rsvd_page(rtwdev);
rtw_send_rsvd_page_h2c(rtwdev);
+ rtw_fw_default_port(rtwdev, rtwvif);
rtw_coex_media_status_notify(rtwdev, vif->cfg.assoc);
if (rtw_bf_support)
rtw_bf_assoc(rtwdev, vif, conf);
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index d55b041a6bb93..c853e2f2d448f 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -334,12 +334,15 @@ int rtw_sta_add(struct rtw_dev *rtwdev, struct ieee80211_sta *sta,
struct ieee80211_vif *vif)
{
struct rtw_sta_info *si = (struct rtw_sta_info *)sta->drv_priv;
+ struct rtw_vif *rtwvif = (struct rtw_vif *)vif->drv_priv;
int i;

si->mac_id = rtw_acquire_macid(rtwdev);
if (si->mac_id >= RTW_MAX_MAC_ID_NUM)
return -ENOSPC;

+ if (vif->type == NL80211_IFTYPE_STATION && vif->cfg.assoc == 0)
+ rtwvif->mac_id = si->mac_id;
si->rtwdev = rtwdev;
si->sta = sta;
si->vif = vif;
@@ -2340,6 +2343,9 @@ static void rtw_port_switch_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
rtw_dbg(rtwdev, RTW_DBG_STATE, "AP port switch from %d -> %d\n",
rtwvif_ap->port, rtwvif_target->port);

+ /* Leave LPS so the value swapped are not in PS mode */
+ rtw_leave_lps(rtwdev);
+
reg1 = &rtwvif_ap->conf->net_type;
reg2 = &rtwvif_target->conf->net_type;
rtw_swap_reg_mask(rtwdev, reg1, reg2);
@@ -2358,6 +2364,8 @@ static void rtw_port_switch_iter(void *data, u8 *mac, struct ieee80211_vif *vif)

swap(rtwvif_target->port, rtwvif_ap->port);
swap(rtwvif_target->conf, rtwvif_ap->conf);
+
+ rtw_fw_default_port(rtwdev, rtwvif_target);
}

void rtw_core_port_switch(struct rtw_dev *rtwdev, struct ieee80211_vif *vif)
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 9e841f6991a9a..f9dd2ab941c8f 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -803,6 +803,7 @@ struct rtw_bf_info {
struct rtw_vif {
enum rtw_net_type net_type;
u16 aid;
+ u8 mac_id; /* for STA mode only */
u8 mac_addr[ETH_ALEN];
u8 bssid[ETH_ALEN];
u8 port;
--
2.25.1


2023-06-16 11:15:58

by Bitterblue Smith

[permalink] [raw]
Subject: Re: [PATCH 1/6] wifi: rtw88: use struct instead of macros to set TX desc

On 16/06/2023 06:44, Ping-Ke Shih wrote:
> From: Po-Hao Huang <[email protected]>
>
> Remove macros that set TX descriptors. Use struct and
> le32_encode_bits() with mask definitions.
>
> Signed-off-by: Po-Hao Huang <[email protected]>
> Signed-off-by: Ping-Ke Shih <[email protected]>
> ---
> drivers/net/wireless/realtek/rtw88/rtw8723d.c | 5 +-
> drivers/net/wireless/realtek/rtw88/tx.c | 79 +++++++-----
> drivers/net/wireless/realtek/rtw88/tx.h | 121 +++++++-----------
> drivers/net/wireless/realtek/rtw88/usb.c | 12 +-
> 4 files changed, 104 insertions(+), 113 deletions(-)
>

[...]

> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index 976eafa739a2d..dbfd5576cc120 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -24,11 +24,12 @@ struct rtw_usb_txcb {
> static void rtw_usb_fill_tx_checksum(struct rtw_usb *rtwusb,
> struct sk_buff *skb, int agg_num)
> {
> + struct rtw_tx_desc *tx_desc = (struct rtw_tx_desc *)skb->data;
> struct rtw_dev *rtwdev = rtwusb->rtwdev;
> struct rtw_tx_pkt_info pkt_info;
>
> - SET_TX_DESC_DMA_TXAGG_NUM(skb->data, agg_num);
> - pkt_info.pkt_offset = GET_TX_DESC_PKT_OFFSET(skb->data);
> + le32_replace_bits(tx_desc->w7, agg_num, RTW_TX_DESC_W7_TXDESC_CHECKSUM);

This looks like the wrong mask. Should it be RTW_TX_DESC_W7_DMA_TXAGG_NUM ?

> + pkt_info.pkt_offset = le32_get_bits(tx_desc->w1, RTW_TX_DESC_W1_PKT_OFFSET);
> rtw_tx_fill_txdesc_checksum(rtwdev, &pkt_info, skb->data);
> }
>

2023-06-16 13:14:54

by Ping-Ke Shih

[permalink] [raw]
Subject: Re: [PATCH 1/6] wifi: rtw88: use struct instead of macros to set TX desc

On Fri, 2023-06-16 at 14:10 +0300, Bitterblue Smith wrote:
>
> On 16/06/2023 06:44, Ping-Ke Shih wrote:
> > From: Po-Hao Huang <[email protected]>
> >
> > Remove macros that set TX descriptors. Use struct and
> > le32_encode_bits() with mask definitions.
> >
> > Signed-off-by: Po-Hao Huang <[email protected]>
> > Signed-off-by: Ping-Ke Shih <[email protected]>
> > ---
> > drivers/net/wireless/realtek/rtw88/rtw8723d.c | 5 +-
> > drivers/net/wireless/realtek/rtw88/tx.c | 79 +++++++-----
> > drivers/net/wireless/realtek/rtw88/tx.h | 121 +++++++-----------
> > drivers/net/wireless/realtek/rtw88/usb.c | 12 +-
> > 4 files changed, 104 insertions(+), 113 deletions(-)
> >
>
> [...]
>
> > diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> > index 976eafa739a2d..dbfd5576cc120 100644
> > --- a/drivers/net/wireless/realtek/rtw88/usb.c
> > +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> > @@ -24,11 +24,12 @@ struct rtw_usb_txcb {
> > static void rtw_usb_fill_tx_checksum(struct rtw_usb *rtwusb,
> > struct sk_buff *skb, int agg_num)
> > {
> > + struct rtw_tx_desc *tx_desc = (struct rtw_tx_desc *)skb->data;
> > struct rtw_dev *rtwdev = rtwusb->rtwdev;
> > struct rtw_tx_pkt_info pkt_info;
> >
> > - SET_TX_DESC_DMA_TXAGG_NUM(skb->data, agg_num);
> > - pkt_info.pkt_offset = GET_TX_DESC_PKT_OFFSET(skb->data);
> > + le32_replace_bits(tx_desc->w7, agg_num, RTW_TX_DESC_W7_TXDESC_CHECKSUM);
>
> This looks like the wrong mask. Should it be RTW_TX_DESC_W7_DMA_TXAGG_NUM ?

That is wrong indeed. Also, should use le32p_replace_bits() instead.
We have sent v2 to correct them.
Thanks for pointing out this.

>
> > + pkt_info.pkt_offset = le32_get_bits(tx_desc->w1, RTW_TX_DESC_W1_PKT_OFFSET);
> > rtw_tx_fill_txdesc_checksum(rtwdev, &pkt_info, skb->data);
> > }
> >
>
> ------Please consider the environment before printing this e-mail.