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

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

[Zhen XIN]

On 1/1/1970 8:00 AM, Ping-Ke Shih wrote:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>>>> @@ -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.

On my RTL8723DS, without condition"&& queue <= RTW_TX_QUEUE_VO", there 
are messages in the console:

[ 23.298425] rtw_8723ds mmc2:0001:1: failed to get tx report from firmware

Ever after I doubled the RTW_TX_PROBE_TIMEOUT (500 * 2), there messages 
were still there, and AP mode didn't work:

root@OpenWrt:~# iw dev phy0-ap0 station dump Station 04:ea:56:2f:6f:07 
(on phy0-ap0) inactive time: 480 ms ... authorized: no authenticated: 
yes associated: yes

Seems tx status report didn't reach hostapd.

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

[Bitterblue Smith]

On 08/04/2025 06:29, Zhen XIN wrote:
> 
> On 1/1/1970 8:00 AM, Ping-Ke Shih wrote:
>> Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>>>>> @@ -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.
> 
> On my RTL8723DS, without condition"&& queue <= RTW_TX_QUEUE_VO", there are messages in the console:
> 
> [ 23.298425] rtw_8723ds mmc2:0001:1: failed to get tx report from firmware
> 
> Ever after I doubled the RTW_TX_PROBE_TIMEOUT (500 * 2), there messages were still there, and AP mode didn't work:
> 
> root@OpenWrt:~# iw dev phy0-ap0 station dump Station 04:ea:56:2f:6f:07 (on phy0-ap0) inactive time: 480 ms ... authorized: no authenticated: yes associated: yes
> 
> Seems tx status report didn't reach hostapd.
> 
> 

That's because management frames are going to the high queue instead
of the management queue:

static u8 rtw_sdio_get_tx_qsel(struct rtw_dev *rtwdev, struct sk_buff *skb,
			       u8 queue)
{
	switch (queue) {
	case RTW_TX_QUEUE_BCN:
		return TX_DESC_QSEL_BEACON;
	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;

And the chip is not configured to provide TX reports for the high
queue.

All the chips should be using the management queue for management
frames. What happens if you change it like this?

	case RTW_TX_QUEUE_MGMT:
		return TX_DESC_QSEL_MGMT;

If that doesn't work, try to change the rqpn table as well. Right now
it's like this:

static const struct rtw_rqpn rqpn_table_8723d[] = {
	{RTW_DMA_MAPPING_NORMAL, RTW_DMA_MAPPING_NORMAL,
	 RTW_DMA_MAPPING_LOW, RTW_DMA_MAPPING_LOW,
	 RTW_DMA_MAPPING_EXTRA, RTW_DMA_MAPPING_HIGH},

I think RTW_DMA_MAPPING_EXTRA for dma_map_mg is wrong for SDIO. This
is what the out of tree rtl8723ds driver does:

static const struct rtw_rqpn rqpn_table_8723d[] = {
	{RTW_DMA_MAPPING_HIGH, RTW_DMA_MAPPING_NORMAL,
	 RTW_DMA_MAPPING_LOW, RTW_DMA_MAPPING_LOW,
	 RTW_DMA_MAPPING_HIGH, RTW_DMA_MAPPING_HIGH},

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

[Zhen Xin (NSB)]

On 4/8/2025 8:53 PM, Bitterblue Smith wrote:
> Caution: This is an external email. Please be very careful when clicking links or opening attachments. See http://nok.it/nsb for additional information.
>
> On 08/04/2025 06:29, Zhen XIN wrote:
>> On 1/1/1970 8:00 AM, Ping-Ke Shih wrote:
>>> Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>>>>>> @@ -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.
>> On my RTL8723DS, without condition"&& queue <= RTW_TX_QUEUE_VO", there are messages in the console:
>>
>> [ 23.298425] rtw_8723ds mmc2:0001:1: failed to get tx report from firmware
>>
>> Ever after I doubled the RTW_TX_PROBE_TIMEOUT (500 * 2), there messages were still there, and AP mode didn't work:
>>
>> root@OpenWrt:~# iw dev phy0-ap0 station dump Station 04:ea:56:2f:6f:07 (on phy0-ap0) inactive time: 480 ms ... authorized: no authenticated: yes associated: yes
>>
>> Seems tx status report didn't reach hostapd.
>>
>>
> That's because management frames are going to the high queue instead
> of the management queue:
>
> static u8 rtw_sdio_get_tx_qsel(struct rtw_dev *rtwdev, struct sk_buff *skb,
> 			       u8 queue)
> {
> 	switch (queue) {
> 	case RTW_TX_QUEUE_BCN:
> 		return TX_DESC_QSEL_BEACON;
> 	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;
>
> And the chip is not configured to provide TX reports for the high
> queue.
>
> All the chips should be using the management queue for management
> frames. What happens if you change it like this?
>
> 	case RTW_TX_QUEUE_MGMT:
> 		return TX_DESC_QSEL_MGMT;
>
That works, thank you very much! I will update the patch.