Fedor Pchelkin pchelkin@ispras.ru wrote:
Thanks! I agree with all aforementioned comments but wonder about this one:
On Thu, 28. Aug 08:07, Zong-Zhe Yang wrote:
Fedor Pchelkin pchelkin@ispras.ru wrote:
--- a/drivers/net/wireless/realtek/rtw89/pci.c +++ b/drivers/net/wireless/realtek/rtw89/pci.c @@ -464,10 +464,7 @@ static void rtw89_pci_tx_status(struct rtw89_dev *rtwdev, struct rtw89_tx_skb_data *skb_data = RTW89_TX_SKB_CB(skb); struct ieee80211_tx_info *info;
rtw89_core_tx_wait_complete(rtwdev, skb_data, tx_status == RTW89_TX_DONE);
info = IEEE80211_SKB_CB(skb);
ieee80211_tx_info_clear_status(info);
if (info->flags & IEEE80211_TX_CTL_NO_ACK) info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED;
@@ -494,6 +491,10 @@ static void rtw89_pci_tx_status(struct rtw89_dev *rtwdev, } }
- if (rtw89_core_tx_wait_complete(rtwdev, skb_data, tx_status == RTW89_TX_DONE))
return;- ieee80211_tx_info_clear_status(info);
Don't change order of these calls. (it's wrong for normal pkt because their tx_info are cleared after filled)
ieee80211_tx_info_clear_status() clears only TX status part of the ieee80211_tx_info. It doesn't touch 'flags' field - the only one filled here by rtw89_pci_tx_status(). It shouldn't be wrong for normal packets.
I double checked it again and think you are right. I misread tx_info->flags against tx_info->status.flags. Sorry.
The reason for changing the order of those calls is to have a chance to update tx_ring statistics before fast return from rtw89_pci_tx_status() in case of tx_wait packets.
But, ergh, I can't find those stats reported anywhere in the driver so it looks like just not a real issue currently and I'd rather not change the order, okay.
These statistics are used when debugging normal packets from stack. For driver packets (with tx wait), I think top callers, e.g. rtw89_core_send_nullfunc, will warns when tx failed. So, don't care these statistics.
ieee80211_tx_status_ni(rtwdev->hw, skb); }