RE: [RFC -v1] wifi: rtw88: sdio: Tx status for management frames

[Ping-Ke Shih <pkshih@realtek.com>]

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