Fedor Pchelkin <pchelkin@xxxxxxxxx> 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@xxxxxxxxx> 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); } > > >
