Re: [PATCH] wifi: ath11k: initialize CFR locks before capability checks

[Jeff Johnson <jeff.johnson@oss.qualcomm.com>]

On 10/8/2026 11:03 PM, Runyu Xiao wrote:
> ath11k_cfr_init() returns before initializing cfr->lock when CFR
> capability is absent.  However, station removal still calls
> ath11k_cfr_decrement_peer_count(), which unconditionally takes this
> lock.  This leaves a production path using an uninitialized spinlock.

If this is really happening...

> 
> Initialize cfr->lock for every radio before the capability check.  Keep
> the CFR ring and lookup-table lock initialization conditional, since
> those objects are only used after CFR capability setup.

...then this is a poor solution.

And my review agent agreed:

- Initializing the locks unconditionally at the top of ath11k_cfr_init()
before the early-exit check would work mechanically, but it's wrong
semantically — callers should not need to reason about which subset of struct
fields is safe to use at any given time.

It also ruled out:
- Guarding the call site in mac.c with a capability check would duplicate the
firmware/hw-support logic outside cfr.c, and there could be other callers in
the future.
- Moving the locks to ath11k_ar_init() would be over-engineering; cfr->lock
only protects CFR state.

And concluded:

The fix: guard ath11k_cfr_decrement_peer_count() with cfr->enabled

In cfr.c, add an early return at the top of ath11k_cfr_decrement_peer_count():

void ath11k_cfr_decrement_peer_count(struct ath11k *ar,
                                     struct ath11k_sta *arsta)
{
      struct ath11k_cfr *cfr = &ar->cfr;

      if (!cfr->enabled)
              return;

      spin_lock_bh(&cfr->lock);

      if (arsta->cfr_capture.cfr_enable)
              cfr->cfr_enabled_peer_cnt--;

      spin_unlock_bh(&cfr->lock);
}

Why this is the right approach:

- cfr->enabled is set to true only after spin_lock_init(&cfr->lock) succeeds
and the ring allocation succeeds (cfr.c:997). It stays false in all early-exit
paths: the firmware/hardware capability check at line 959, the
ath11k_dbring_get_cap() failure continue at line 970, and the ring alloc
failure at line 991.
- The check is already used analogously at line 937 in the relay flush path,
so it's an established pattern in this file.
- This avoids touching ath11k_cfr_init() or the call site in mac.c, keeping
the fix minimal and localized to the function that has the precondition.

/jeff