Re: [PATCH rtw-next v2] wifi: rtw88: pci: wake the TX queues when the rings are reset

[Abdurrahman Karadag]

> I'm thinking if here is too early to wake up queues. Should we do this in
> rtw_reconfig_complete()?

It is too early, and rtw_reconfig_complete() is the right place. v3 does
that.

On the reset path rtw_core_stop() has already cleared RTW_FLAG_RUNNING by
the time rtw_pci_reset_trx_ring() runs, so anything mac80211 hands the
driver between that wake and the end of the restart is freed again by
rtw_ops_tx(). reconfig_complete() is called from the wake_up label in
ieee80211_reconfig(), where the comment reads "If this is for hw restart
things are still running". It also makes the fix bus-independent instead
of PCI-only.

I measured it at the new hook before sending, with two builds of current
rtw-next that differ by nothing but this patch, using mac80211's own
tracepoints instead of a register read. Under load the BE queue is
stopped by the ring-full path, then rtw_fw_recovery() is called:

  without the patch
    511.499278  stop_queue  queue:2 reason:0      DRIVER, left dangling
    511.501625  stop_queue  queue:0-3 reason:4    SUSPEND, the restart
    511.957404  drv_reconfig_complete reconfig_type:0
    511.957411  wake_queue  queue:0-3 reason:4    only SUSPEND is woken
    AC_BE stays 0x00000001, 100% loss

  with the patch
    475.930915  drv_reconfig_complete reconfig_type:0
    475.930925  wake_queue  queue:2 reason:0      10 us later
    475.930926  wake_queue  queue:0-3 reason:4
    AC_BE reads 0x00000000, 0% loss

So the wake lands after the restart has completed and before mac80211
wakes the SUSPEND reason itself. No TXPAUSE write and no injection this
time - the ring fills on its own under load, which is also why I am happy
to leave the measurement out of the commit message.

One consequence I should flag, since v2 covered it and v3 does not. A
plain "ip link set wlan0 down/up" also reaches rtw_pci_reset_trx_ring(),
and mac80211 never clears a DRIVER stop on interface down - nothing
outside the stop/wake helpers touches local->q_stop_reasons[]. So with v3
a queue stopped before a down/up stays stopped. Is the restart case the
one you want fixed, or should a down/up release it too?

I also left ring->queue_stopped alone. It stays set until the first
completion after the restart, which clears it, and rtw_pci_tx_write()
does not read it before stopping a queue, so a stale value costs nothing.
I can clear it in rtw_pci_reset_trx_ring() if you would rather the driver
state did not claim a stop that is gone.

> As I see, the commit message can be very simple -- wake up queues for
> ieee80211_restart_hw() case.

Done, and the subject names it now. The measurement paragraph is out -
you are right that the stall belongs elsewhere.