Re: [RESEND PATCH ath-current 3/3] wifi: ath11k: unregister PM notifier on QMI init failure path

[Miaoqing Pan <miaoqing.pan@oss.qualcomm.com>]

On 8/1/2026 7:06 AM, Jeff Johnson wrote:
> On 7/29/2026 8:09 PM, Miaoqing Pan wrote:
>> ath11k_core_init() registers a PM notifier before the QMI server
>> becomes available. If the QMI server never arrives, the device remove()
>> path can take the early-exit path introduced for QMI initialization
>> failures, skipping ath11k_core_deinit().
> 
> This patch felt wrong to me, so I spent some time with my buddy Claude.
> 
> I asked Claude: in the last patch the commit text says "If the QMI server
> never arrives, the device remove() path can take the early-exit path
> introduced for QMI initialization failures". where is this device remove()
> path code? I'm trying to understand why there needs to be a call to
> ath11k_core_pm_notifier_unregister() in ath11k_ahb_remove() as well as in
> ath11k_core_deinit(). since ath11k_core_pm_notifier_register() is only called
> from one place it is logical that ath11k_core_pm_notifier_unregister() should
> also be called in one place that is symmetrical. the proposed patch breaks the
> init/deinit symmetry.
> 
> Claude said:
> The symmetry argument is well-founded. Here's the actual structure:
> 
> ath11k_core_init() registers the notifier. ath11k_core_deinit() is its
> symmetric counterpart and unregisters it. That is the intended 1:1 pairing.
> 
> The problem is that both ath11k_ahb_remove() and ath11k_pci_remove() have an
> early-exit path for QMI_FAIL that bypasses ath11k_core_deinit() — but still
> need to undo what ath11k_core_init() did. So they replicate the unregister
> call directly. This is an existing pattern: pci.c:1188 already had
> ath11k_core_pm_notifier_unregister() in its QMI_FAIL early-exit before this
> patch series.
> 
> Patch 3 simply extends that same pattern to ahb.c's early-exit path, which was
> missing it.
> 
> The symmetry break isn't introduced by this patch — it was already present for
> PCI. The fix for the symmetry issue would be to restructure so the early-exit
> paths all go through ath11k_core_deinit() (or a new partial-deinit function),
> but that's a larger refactor. This patch's approach matches the existing
> convention in the tree.
> 
> The commit message claim about "the early-exit path introduced for QMI
> initialization failures" refers to the QMI_FAIL branch at ahb.c:1301 — the
> goto qmi_fail block that skips ath11k_core_deinit().
> 
> Thank you, Claude.
> 
> So I guess this patch is OK since it is replicating the PCI pattern, but is
> there any way to have ath11k_core_deinit() called in those early exit paths so
> that we can keep the init/deinit symmetry? It just seems wrong that if
> ath11k_core_init() did not return an error that we would have deinit logic
> that would skip calling ath11k_core_deinit()
> 
The init/deinit symmetry issue is a pre-existing concern and not trivial 
to solve. We can address it separately in a future cleanup/refactoring 
patch.
>>
>> As a result, the PM notifier remains registered after the ath11k base
>> object has been freed. A subsequent suspend or resume event may invoke
>> the stale notifier and trigger a use-after-free.
>>
>> Fix this by explicitly unregistering the PM notifier in the QMI failure
>> cleanup path before releasing ath11k resources.
>>
>> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.2.0.c2-00204-QCAMSLSWPLZ-1
>>
>> Fixes: 32d93b51bc7e ("wifi: ath11k: choose default PM policy for hibernation")
>> Signed-off-by: Miaoqing Pan <miaoqing.pan@oss.qualcomm.com>
>> ---
>>   drivers/net/wireless/ath/ath11k/ahb.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c
>> index 7f5f5c8d7c56..bdb8c99f10fa 100644
>> --- a/drivers/net/wireless/ath/ath11k/ahb.c
>> +++ b/drivers/net/wireless/ath/ath11k/ahb.c
>> @@ -1272,6 +1272,7 @@ static void ath11k_ahb_remove(struct platform_device *pdev)
>>   		ath11k_ahb_power_down(ab, false);
>>   		ath11k_debugfs_soc_destroy(ab);
>>   		ath11k_qmi_deinit_service(ab);
>> +		ath11k_core_pm_notifier_unregister(ab);
>>   		goto qmi_fail;
>>   	}
>>   
>