| Message ID | 20250409034910.1637422-1-zhen.xin@nokia-sbell.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Ping-Ke Shih |
| Headers | show |
| Series | [RFC,-v2] wifi: rtw88: sdio: Tx status for management frames | expand |
Hi Martin, Zhen XIN <zhen.xin@nokia-sbell.com> wrote: > Rtw88-sdio is missing the tx status report for management frames > > Fix this by mapping mgmt frames to queue TX_DESC_QSEL_MGMT > > Signed-off-by: Zhen XIN <zhen.xin@nokia-sbell.com> > --- > v2: have the right queue for mgmt frames as pointed out by Bitterblue Smith > --- > drivers/net/wireless/realtek/rtw88/sdio.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c > index e024061bdbf7..4311eb7cffef 100644 > --- a/drivers/net/wireless/realtek/rtw88/sdio.c > +++ b/drivers/net/wireless/realtek/rtw88/sdio.c > @@ -718,10 +718,7 @@ static u8 rtw_sdio_get_tx_qsel(struct rtw_dev *rtwdev, struct sk_buff *skb, > case RTW_TX_QUEUE_H2C: > return TX_DESC_QSEL_H2C; > case RTW_TX_QUEUE_MGMT: > - if (rtw_chip_wcpu_11n(rtwdev)) > - return TX_DESC_QSEL_HIGH; > - else > - return TX_DESC_QSEL_MGMT; > + return TX_DESC_QSEL_MGMT; Do you remember why you did the special deal with 11n chips? And this RFC looks good to me. (except to commit message, but this is RFC) > case RTW_TX_QUEUE_HI0: > return TX_DESC_QSEL_HIGH; > default: > @@ -1227,10 +1224,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); > } > > static void rtw_sdio_tx_handler(struct work_struct *work) > -- > 2.25.1
Hi Ping-Ke, On Thu, Apr 10, 2025 at 6:30 AM Ping-Ke Shih <pkshih@realtek.com> wrote: [...] > > @@ -718,10 +718,7 @@ static u8 rtw_sdio_get_tx_qsel(struct rtw_dev *rtwdev, struct sk_buff *skb, > > case RTW_TX_QUEUE_H2C: > > return TX_DESC_QSEL_H2C; > > case RTW_TX_QUEUE_MGMT: > > - if (rtw_chip_wcpu_11n(rtwdev)) > > - return TX_DESC_QSEL_HIGH; > > - else > > - return TX_DESC_QSEL_MGMT; > > + return TX_DESC_QSEL_MGMT; > > Do you remember why you did the special deal with 11n chips? > And this RFC looks good to me. (except to commit message, but this is RFC) I don't remember - and Jernej said the same thing. However, since we got the first 802.11n hardware for testing long after this part was written my suggestion is: let's roll this into a proper patch, Cc Fiona Klute <fiona.klute@gmx.de> (author of RTL8723CS support) on the resulting patch(es) and then apply the patches (assuming nobody observes any problems). To make this a non-RFC patch the following steps are needed (in my opinion): - split the change into two patches (one which unconditionally calls rtw_sdio_indicate_tx_status()) - another one for the TX_DESC_QSEL_MGMT mapping - each of the patches should include their own description - I checked the history and it seems that both problems were introduced with the original commit, meaning both patches should get the following line (above the Signed-off-by): Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets") - (plus anything Ping-Ke has to add :-) ) Best regards, Martin
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > Hi Ping-Ke, > > On Thu, Apr 10, 2025 at 6:30 AM Ping-Ke Shih <pkshih@realtek.com> wrote: > [...] > > > @@ -718,10 +718,7 @@ static u8 rtw_sdio_get_tx_qsel(struct rtw_dev *rtwdev, struct sk_buff *skb, > > > case RTW_TX_QUEUE_H2C: > > > return TX_DESC_QSEL_H2C; > > > case RTW_TX_QUEUE_MGMT: > > > - if (rtw_chip_wcpu_11n(rtwdev)) > > > - return TX_DESC_QSEL_HIGH; > > > - else > > > - return TX_DESC_QSEL_MGMT; > > > + return TX_DESC_QSEL_MGMT; > > > > Do you remember why you did the special deal with 11n chips? > > And this RFC looks good to me. (except to commit message, but this is RFC) > I don't remember - and Jernej said the same thing. > However, since we got the first 802.11n hardware for testing long > after this part was written my suggestion is: let's roll this into a > proper patch, Cc Fiona Klute <fiona.klute@gmx.de> (author of RTL8723CS > support) on the resulting patch(es) and then apply the patches > (assuming nobody observes any problems). > > To make this a non-RFC patch the following steps are needed (in my opinion): > - split the change into two patches (one which unconditionally calls > rtw_sdio_indicate_tx_status()) > - another one for the TX_DESC_QSEL_MGMT mapping > - each of the patches should include their own description > - I checked the history and it seems that both problems were > introduced with the original commit, meaning both patches should get > the following line (above the Signed-off-by): Fixes: 65371a3f14e7 > ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets") > - (plus anything Ping-Ke has to add :-) ) That's super clear. No other opinion from me. :-)
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c index e024061bdbf7..4311eb7cffef 100644 --- a/drivers/net/wireless/realtek/rtw88/sdio.c +++ b/drivers/net/wireless/realtek/rtw88/sdio.c @@ -718,10 +718,7 @@ static u8 rtw_sdio_get_tx_qsel(struct rtw_dev *rtwdev, struct sk_buff *skb, case RTW_TX_QUEUE_H2C: return TX_DESC_QSEL_H2C; case RTW_TX_QUEUE_MGMT: - if (rtw_chip_wcpu_11n(rtwdev)) - return TX_DESC_QSEL_HIGH; - else - return TX_DESC_QSEL_MGMT; + return TX_DESC_QSEL_MGMT; case RTW_TX_QUEUE_HI0: return TX_DESC_QSEL_HIGH; default: @@ -1227,10 +1224,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); } static void rtw_sdio_tx_handler(struct work_struct *work)
Rtw88-sdio is missing the tx status report for management frames Fix this by mapping mgmt frames to queue TX_DESC_QSEL_MGMT Signed-off-by: Zhen XIN <zhen.xin@nokia-sbell.com> --- v2: have the right queue for mgmt frames as pointed out by Bitterblue Smith --- drivers/net/wireless/realtek/rtw88/sdio.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)