| Message ID | 20250402160310.996141-1-zhen.xin@nokia-sbell.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,-v1] wifi: rtw88: sdio: Tx status for management frames | expand |
Hello, thank you for sending the patch! On Wed, Apr 2, 2025 at 6:03 PM Zhen XIN <zhen.xin@nokia-sbell.com> wrote: > > Rtl8732ds doesn't work in AP-Mode due to the missing tx status for management frames > This patch enables tx status report for all tx skbs Ping-Ke, there's a bit more discussion at [0] Do you have any suggestions regarding the approach used here? Best regards, Martin [0] https://github.com/lwfinger/rtw88/issues/245#issuecomment-2765046681
Hi Martin, I replied original mail, because I think discussion would be clearer. Zhen XIN <zhen.xin@nokia-sbell.com> wrote: > Rtl8732ds doesn't work in AP-Mode due to the missing tx status for management frames > This patch enables tx status report for all tx skbs > > Signed-off-by: Zhen XIN <zhen.xin@nokia-sbell.com> > --- > drivers/net/wireless/realtek/rtw88/sdio.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c > index e024061bdbf7..84f71e13b5ae 100644 > --- a/drivers/net/wireless/realtek/rtw88/sdio.c > +++ b/drivers/net/wireless/realtek/rtw88/sdio.c > @@ -1186,7 +1186,7 @@ static int rtw_sdio_request_irq(struct rtw_dev *rtwdev, > } > > static void rtw_sdio_indicate_tx_status(struct rtw_dev *rtwdev, > - struct sk_buff *skb) > + struct sk_buff *skb, enum rtw_tx_queue_type queue) > { > struct rtw_sdio_tx_data *tx_data = rtw_sdio_get_tx_data(skb); > struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); > @@ -1195,7 +1195,7 @@ static void rtw_sdio_indicate_tx_status(struct rtw_dev *rtwdev, > skb_pull(skb, rtwdev->chip->tx_pkt_desc_sz); > > /* enqueue to wait for tx report */ > - if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) { > + if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS && queue <= RTW_TX_QUEUE_VO) { Is this because you have seen "failed to get tx report"? Have you tried to increasing RTW_TX_PROBE_TIMEOUT? If it still can't get TX report, we might take this workaround with comments to mention why we need it. Or a local variable with proper naming to point out this, like bool queue_has_no_tx_report = queue > RTW_TX_QUEUE_VO; By the way, USB behavior is very like to SDIO, but TX report seems to work well. > rtw_tx_report_enqueue(rtwdev, skb, tx_data->sn); > return; > } > @@ -1227,10 +1227,7 @@ static void rtw_sdio_process_tx_queue(struct rtw_dev *rtwdev, > return; > } > > - if (queue <= RTW_TX_QUEUE_VO) > - rtw_sdio_indicate_tx_status(rtwdev, skb); > - else > - dev_kfree_skb_any(skb); > + rtw_sdio_indicate_tx_status(rtwdev, skb, queue); I think this change is reasonable, since skb via all kinds of queues could set IEEE80211_TX_CTL_REQ_TX_STATUS. > } > > static void rtw_sdio_tx_handler(struct work_struct *work) > -- > 2.25.1
Hi Ping-Ke, On Mon, Apr 7, 2025 at 5:30 AM Ping-Ke Shih <pkshih@realtek.com> wrote: > > Hi Martin, > > I replied original mail, because I think discussion would be clearer. makes sense, thank you! [...] > > @@ -1195,7 +1195,7 @@ static void rtw_sdio_indicate_tx_status(struct rtw_dev *rtwdev, > > skb_pull(skb, rtwdev->chip->tx_pkt_desc_sz); > > > > /* enqueue to wait for tx report */ > > - if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) { > > + if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS && queue <= RTW_TX_QUEUE_VO) { > > Is this because you have seen "failed to get tx report"? > Have you tried to increasing RTW_TX_PROBE_TIMEOUT? > > If it still can't get TX report, we might take this workaround with comments > to mention why we need it. Or a local variable with proper naming to point out > this, like > > bool queue_has_no_tx_report = queue > RTW_TX_QUEUE_VO; > > > By the way, USB behavior is very like to SDIO, but TX report seems to work well. On my RTL8822CS I can confirm your thought: I don't notice any extra "failed to get tx report" messages regardless of whether I have "&& queue <= RTW_TX_QUEUE_VO" or not. Best regards, Martin
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > Hi Ping-Ke, > > On Mon, Apr 7, 2025 at 5:30 AM Ping-Ke Shih <pkshih@realtek.com> wrote: > > > > Hi Martin, > > > > I replied original mail, because I think discussion would be clearer. > makes sense, thank you! > > [...] > > > @@ -1195,7 +1195,7 @@ static void rtw_sdio_indicate_tx_status(struct rtw_dev *rtwdev, > > > skb_pull(skb, rtwdev->chip->tx_pkt_desc_sz); > > > > > > /* enqueue to wait for tx report */ > > > - if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) { > > > + if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS && queue <= RTW_TX_QUEUE_VO) { > > > > Is this because you have seen "failed to get tx report"? > > Have you tried to increasing RTW_TX_PROBE_TIMEOUT? > > > > If it still can't get TX report, we might take this workaround with comments > > to mention why we need it. Or a local variable with proper naming to point out > > this, like > > > > bool queue_has_no_tx_report = queue > RTW_TX_QUEUE_VO; > > > > > > By the way, USB behavior is very like to SDIO, but TX report seems to work well. > On my RTL8822CS I can confirm your thought: > I don't notice any extra "failed to get tx report" messages regardless > of whether I have "&& queue <= RTW_TX_QUEUE_VO" or not. > This workaround might need an chip attribute to enable then. Not sure if people in the GitHub thread have experiments on all supported SDIO WiFi chips.
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c index e024061bdbf7..84f71e13b5ae 100644 --- a/drivers/net/wireless/realtek/rtw88/sdio.c +++ b/drivers/net/wireless/realtek/rtw88/sdio.c @@ -1186,7 +1186,7 @@ static int rtw_sdio_request_irq(struct rtw_dev *rtwdev, } static void rtw_sdio_indicate_tx_status(struct rtw_dev *rtwdev, - struct sk_buff *skb) + struct sk_buff *skb, enum rtw_tx_queue_type queue) { struct rtw_sdio_tx_data *tx_data = rtw_sdio_get_tx_data(skb); struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); @@ -1195,7 +1195,7 @@ static void rtw_sdio_indicate_tx_status(struct rtw_dev *rtwdev, skb_pull(skb, rtwdev->chip->tx_pkt_desc_sz); /* enqueue to wait for tx report */ - if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) { + if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS && queue <= RTW_TX_QUEUE_VO) { rtw_tx_report_enqueue(rtwdev, skb, tx_data->sn); return; } @@ -1227,10 +1227,7 @@ static void rtw_sdio_process_tx_queue(struct rtw_dev *rtwdev, return; } - if (queue <= RTW_TX_QUEUE_VO) - rtw_sdio_indicate_tx_status(rtwdev, skb); - else - dev_kfree_skb_any(skb); + rtw_sdio_indicate_tx_status(rtwdev, skb, queue); } static void rtw_sdio_tx_handler(struct work_struct *work)
Rtl8732ds doesn't work in AP-Mode due to the missing tx status for management frames This patch enables tx status report for all tx skbs Signed-off-by: Zhen XIN <zhen.xin@nokia-sbell.com> --- drivers/net/wireless/realtek/rtw88/sdio.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)