RE: [PATCH rtw-next v2 5/7] wifi: rtw89: implement C2H TX report handler

Ping-Ke Shih
2025-10-07T03:13:24Z

Fedor Pchelkin <pchelkin@ispras.ru> wrote:
> rtw89 has several ways of handling TX status report events. The first one
> is based on RPP feature which is used by PCIe HCI. The other one depends
> on firmware sending a corresponding C2H message, quite similar to what
> rtw88 has.
>
> Toggle a bit in the TX descriptor and place skb in a queue to wait for a
> message from the firmware. rtw89 has an extra feature providing TX
> reports for multiple retry transmission attempts. When there is a failed
> TX status reported by the firmware, the report is ignored until the limit
> is reached or success status appears. Do all this according to the vendor
> driver for RTL8851BU.
>
> It seems the only way to implement TX status reporting for rtw89 USB.
> This will allow handling TX wait skbs and the ones flagged with
> IEEE80211_TX_CTL_REQ_TX_STATUS correctly.
>
> Found by Linux Verification Center (linuxtesting.org).
>
> Suggested-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>

[...]

> diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
> index fd11b8fb3c89..10c2a39e544b 100644
> --- a/drivers/net/wireless/realtek/rtw89/mac.c
> +++ b/drivers/net/wireless/realtek/rtw89/mac.c
> @@ -5457,6 +5457,20 @@ rtw89_mac_c2h_mcc_status_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32
> rtw89_complete_cond(&rtwdev->mcc.wait, cond, &data);
> }
>
> +static void
> +rtw89_mac_c2h_tx_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 len)
> +{
> + const struct rtw89_c2h_mac_tx_rpt *rpt =
> + (const struct rtw89_c2h_mac_tx_rpt *)c2h->data;
> + u8 sw_define = le32_get_bits(rpt->w2, RTW89_C2H_MAC_TX_RPT_W2_SW_DEFINE);
> + u8 tx_status = le32_get_bits(rpt->w2, RTW89_C2H_MAC_TX_RPT_W2_TX_STATE);
> + u8 data_txcnt = le32_get_bits(rpt->w5, RTW89_C2H_MAC_TX_RPT_W5_DATA_TX_CNT);

Since we'd like reverse X'mas tree order, I'd like separate declarations and
assignments. Like

u8 sw_define, tx_status, data_txcnt;

sw_define = le32_get_bits(rpt->w2, RTW89_C2H_MAC_TX_RPT_W2_SW_DEFINE);
tx_status = le32_get_bits(rpt->w2, RTW89_C2H_MAC_TX_RPT_W2_TX_STATE);
data_txcnt = le32_get_bits(rpt->w5, RTW89_C2H_MAC_TX_RPT_W5_DATA_TX_CNT);

Otherwise, looks good to me.

> +
> + rtw89_debug(rtwdev, RTW89_DBG_TXRX,
> + "C2H TX RPT: sn %d, tx_status %d, data_txcnt %d\n",
> + sw_define, tx_status, data_txcnt);
> +}
> + …