Re: [PATCH v2 09/11] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation

[luka . gejak]

From: Luka Gejak <luka.gejak@xxxxxxxxx>

On 27/07/2026 09:21, Ping-Ke Shih wrote:
>> +/* 8723BS SDIO TX FIFO back-pressure watermarks: stop the mac80211 queue once
> 
> comment style.

Fixed, both of them.

>> -       queue_work(rtwsdio->txwq, &rtwsdio->tx_handler_data->work);
>> +       mod_delayed_work(rtwsdio->txwq,
>> +                        &rtwsdio->tx_handler_data->work, 0);
> 
> queue_delayed_work()?

Changed inside the TX handler, where the work is not pending and
queue_delayed_work() is the right call.

I kept mod_delayed_work() in rtw_sdio_tx_kick_off() on purpose, because
there it can race with a pending retry. If a page shortage has already
armed the work with RTW_SDIO_TX_RETRY_DELAY, queue_delayed_work() would
see it pending and do nothing, so a newly queued frame would sit for up
to a millisecond for no reason. mod_delayed_work() re-arms it to fire
immediately. I have added a comment saying so.

>>         if (ret) {
>>                 skb_queue_head(&rtwsdio->tx_queue[queue], skb);
> 
> This case is also `processed = true`?
[...]
> Can you cleanup the handlers of return value and processed?
> The logic isn't clear to me.

You are right that it was not clear, and the requeue case was the
reason: the frame had been dequeued but not sent, so neither value
described it well. The out-parameter is gone. rtw_sdio_process_tx_queue()
now returns:

   1  a frame was written
   0  the queue was empty
  <0  the write failed and the frame is back at the head of the queue

which the handler reads as

	ret = rtw_sdio_process_tx_queue(rtwdev, queue);
	if (ret == 0)
		break;
	if (ret < 0) {
		if (rtl8723bs && ret == -EBUSY) {
			queue_delayed_work(... RTW_SDIO_TX_RETRY_DELAY);
			return;
		}
		break;
	}

with the management frame restart after it. Behaviour is unchanged; a
non-EBUSY error still moves on to the next queue.

Best regards,
Luka Gejak