Re: [PATCH 2/2] wifi: ath12k: implement custom wake_tx_queue with flow control

On Fri, Jul 10, 2026 at 9:25 PM Jose Ignacio Tornos Martinez
<[email protected]> wrote:
>
> Under heavy traffic, ath12k can hang and experiences -ENOMEM errors
> ("failed to transmit frame -12") when the hardware TCL ring fills up.
> This issue is more commonly observed in VMs with PCIe passthrough but
> also occurs on bare metal systems.
>
> Implement a custom wake_tx_queue operation that:
>
> 1. Checks hardware ring space before dequeuing packets from mac80211
> 2. Uses per-packet locking to serialize ring access and prevent races
> 3. Syncs with hardware state to get accurate free slot count
> 4. Returns early during firmware crash in the same way as other tx paths
>
> This approach follows the pattern used in the iwlwifi driver, adapted
> for ath12k's hardware ring architecture.
>
> This prevents hangs, eliminates -ENOMEM errors, and improves throughput
> by optimizing resource usage and preventing unnecessary packet drops.
>
> Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
> ---
>  drivers/net/wireless/ath/ath12k/dp.c       |  1 +
>  drivers/net/wireless/ath/ath12k/dp.h       |  2 +
>  drivers/net/wireless/ath/ath12k/hal.c      |  1 +
>  drivers/net/wireless/ath/ath12k/wifi7/hw.c | 50 +++++++++++++++++++++-
>  4 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/dp.c b/drivers/net/wireless/ath/ath12k/dp.c
> index af5f11fc1d84..3d46cfbf0a1c 100644
> --- a/drivers/net/wireless/ath/ath12k/dp.c
> +++ b/drivers/net/wireless/ath/ath12k/dp.c
> @@ -1539,6 +1539,7 @@ static int ath12k_dp_setup(struct ath12k_base *ab)
>         }
>
>         for (i = 0; i < ab->hw_params->max_tx_ring; i++) {
> +               spin_lock_init(&dp->tx_ring[i].wake_tx_lock);
>                 dp->tx_ring[i].tcl_data_ring_id = i;
>
>                 dp->tx_ring[i].tx_status_head = 0;
> diff --git a/drivers/net/wireless/ath/ath12k/dp.h b/drivers/net/wireless/ath/ath12k/dp.h
> index f8cfc7bb29dd..68d2020be9b8 100644
> --- a/drivers/net/wireless/ath/ath12k/dp.h
> +++ b/drivers/net/wireless/ath/ath12k/dp.h
> @@ -58,6 +58,8 @@ struct dp_tx_ring {
>         u8 tcl_data_ring_id;
>         struct dp_srng tcl_data_ring;
>         struct dp_srng tcl_comp_ring;
> +       /* Serializes wake_tx_queue operations for this ring */
> +       spinlock_t wake_tx_lock;
>         struct hal_wbm_completion_ring_tx *tx_status;
>         int tx_status_head;
>         int tx_status_tail;
> diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c
> index a164563fff28..c1c656e4550b 100644
> --- a/drivers/net/wireless/ath/ath12k/hal.c
> +++ b/drivers/net/wireless/ath/ath12k/hal.c
> @@ -390,6 +390,7 @@ int ath12k_hal_srng_src_num_free(struct ath12k_base *ab, struct hal_srng *srng,
>         else
>                 return ((srng->ring_size - hp + tp) / srng->entry_size) - 1;
>  }
> +EXPORT_SYMBOL(ath12k_hal_srng_src_num_free);
>
>  void *ath12k_hal_srng_src_next_peek(struct ath12k_base *ab,
>                                     struct hal_srng *srng)
> diff --git a/drivers/net/wireless/ath/ath12k/wifi7/hw.c b/drivers/net/wireless/ath/ath12k/wifi7/hw.c
> index d9fdd2fc8298..e3a6f9cdee24 100644
> --- a/drivers/net/wireless/ath/ath12k/wifi7/hw.c
> +++ b/drivers/net/wireless/ath/ath12k/wifi7/hw.c
> @@ -1100,9 +1100,57 @@ static void ath12k_wifi7_mac_op_tx(struct ieee80211_hw *hw,
>         }
>  }
>
> +static void ath12k_wifi7_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
> +                                             struct ieee80211_txq *txq)
> +{
> +       struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
> +       struct ieee80211_tx_control control = {
> +               .sta = txq->sta,
> +       };
> +       struct ath12k *ar = ah->radio;

This assignment is wrong and all traffic is incorrectly steered to
radio[0] as ah->radio is a flexible array, not a pointer

> +       struct dp_tx_ring *tx_ring;
> +       struct hal_srng *tcl_ring;
> +       struct ath12k_dp *dp;
> +       struct sk_buff *skb;
> +       int num_free;
> +
> +       if (!ar)
> +               return;
> +
> +       if (unlikely(test_bit(ATH12K_FLAG_CRASH_FLUSH, &ar->ab->dev_flags)))
> +               return;
> +
> +       dp = ar->ab->dp;
> +       tx_ring = &dp->tx_ring[txq->ac % dp->hw_params->max_tx_ring];
> +       tcl_ring = &dp->hal->srng_list[tx_ring->tcl_data_ring.ring_id];

tx_ring/tcl_ring selection should be corrected.
> +
> +       while (1) {
> +               spin_lock_bh(&tx_ring->wake_tx_lock);

Do we need this spin_lock_bh?

> +
> +               spin_lock(&tcl_ring->lock);
> +               num_free = ath12k_hal_srng_src_num_free(ar->ab, tcl_ring, true);
> +               spin_unlock(&tcl_ring->lock);
> +
Do we need this check and spin_lock here? already ath12k_wifi7_dp_tx()
has this lock and fetches the next entry. Can we check those return
value here and break the loop?

> +               if (num_free == 0) {
> +                       spin_unlock_bh(&tx_ring->wake_tx_lock);
> +                       break;
> +               }
> +
> +               skb = ieee80211_tx_dequeue(hw, txq);
> +               if (!skb) {
> +                       spin_unlock_bh(&tx_ring->wake_tx_lock);
> +                       break;
> +               }
> +
> +               ath12k_wifi7_mac_op_tx(hw, &control, skb);
> +
> +               spin_unlock_bh(&tx_ring->wake_tx_lock);
> +       }
> +}
> +
>  static const struct ieee80211_ops ath12k_ops_wifi7 = {
>         .tx                             = ath12k_wifi7_mac_op_tx,
> -       .wake_tx_queue                  = ieee80211_handle_wake_tx_queue,
> +       .wake_tx_queue                  = ath12k_wifi7_mac_op_wake_tx_queue,
>         .start                          = ath12k_mac_op_start,
>         .stop                           = ath12k_mac_op_stop,
>         .reconfig_complete              = ath12k_mac_op_reconfig_complete,
> --
> 2.54.0
>
>


--
- Tamizh.