[PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[Aaradhana Sahu]

Currently, NAPI instances and their associated netdevs allocated for AHB
external IRQ groups are not released when external IRQ configuration
fails. They are also not freed when arch_init() fails after
ath12k_ahb_config_irq() succeeds during probe, or when the driver is
unloaded.

Fix this by adding a common cleanup helper to release the NAPI instances
and associated netdevs. Call it from the external IRQ configuration error
path, probe error path, and driver cleanup path.

Tested-on: IPQ5332 hw1.0 AHB WLAN.WBE.1.6-01275-QCAHKSWPL_SILICONZ-1

Fixes: 6cee30f0da75 ("wifi: ath12k: add AHB driver support for IPQ5332")
Fixes: 614c23e24ee8 ("wifi: ath12k: Support arch-specific DP device allocation")
Signed-off-by: Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>
---
v2:
  -Remove unnecessary irq_grp->napi_ndev NULL check.
---
 drivers/net/wireless/ath/ath12k/ahb.c | 41 ++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/ahb.c b/drivers/net/wireless/ath/ath12k/ahb.c
index 85360365aa4b..9b77632bd1bf 100644
--- a/drivers/net/wireless/ath/ath12k/ahb.c
+++ b/drivers/net/wireless/ath/ath12k/ahb.c
@@ -191,6 +191,26 @@ static void __ath12k_ahb_ext_irq_disable(struct ath12k_base *ab)
 	}
 }
 
+static void ath12k_ahb_free_napi_for_group(struct ath12k_ext_irq_grp *irq_grp)
+{
+	netif_napi_del(&irq_grp->napi);
+	free_netdev(irq_grp->napi_ndev);
+	irq_grp->napi_ndev = NULL;
+}
+
+static void ath12k_ahb_free_napi(struct ath12k_base *ab, int num_ext_irq_grp)
+{
+	int i;
+
+	for (i = 0; i < num_ext_irq_grp; i++)
+		ath12k_ahb_free_napi_for_group(&ab->ext_irq_grp[i]);
+}
+
+static void ath12k_ahb_free_ext_napi(struct ath12k_base *ab)
+{
+	ath12k_ahb_free_napi(ab, ATH12K_EXT_IRQ_GRP_NUM_MAX);
+}
+
 static void ath12k_ahb_ext_grp_enable(struct ath12k_ext_irq_grp *irq_grp)
 {
 	int i;
@@ -622,8 +642,10 @@ static int ath12k_ahb_config_ext_irq(struct ath12k_base *ab)
 		irq_grp->grp_id = i;
 
 		irq_grp->napi_ndev = alloc_netdev_dummy(0);
-		if (!irq_grp->napi_ndev)
-			return -ENOMEM;
+		if (!irq_grp->napi_ndev) {
+			ret = -ENOMEM;
+			goto free_napi_ndev;
+		}
 
 		netif_napi_add(irq_grp->napi_ndev, &irq_grp->napi,
 			       ath12k_ahb_ext_grp_napi_poll);
@@ -675,12 +697,19 @@ static int ath12k_ahb_config_ext_irq(struct ath12k_base *ab)
 					       ath12k_ahb_ext_interrupt_handler,
 					       IRQF_TRIGGER_RISING,
 					       irq_name[irq_idx], irq_grp);
-			if (ret)
+			if (ret) {
 				ath12k_warn(ab, "failed request_irq for %d\n", irq);
+				ath12k_ahb_free_napi_for_group(irq_grp);
+				goto free_napi_ndev;
+			}
 		}
 	}
 
 	return 0;
+
+free_napi_ndev:
+	ath12k_ahb_free_napi(ab, i);
+	return ret;
 }
 
 static int ath12k_ahb_config_irq(struct ath12k_base *ab)
@@ -1353,7 +1382,7 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
 	ret = ab_ahb->device_family_ops->arch_init(ab);
 	if (ret) {
 		ath12k_err(ab, "AHB arch_init failed %d\n", ret);
-		goto err_rproc_deconfigure;
+		goto err_free_irq;
 	}
 
 	ret = ath12k_core_init(ab);
@@ -1367,6 +1396,9 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
 err_deinit_arch:
 	ab_ahb->device_family_ops->arch_deinit(ab);
 
+err_free_irq:
+	ath12k_ahb_free_ext_napi(ab);
+
 err_rproc_deconfigure:
 	mutex_lock(&ath12k_rproc_info_lock);
 	ath12k_ahb_deconfigure_rproc(ab);
@@ -1409,6 +1441,7 @@ static void ath12k_ahb_free_resources(struct ath12k_base *ab)
 	struct platform_device *pdev = ab->pdev;
 	struct ath12k_ahb *ab_ahb = ath12k_ab_to_ahb(ab);
 
+	ath12k_ahb_free_ext_napi(ab);
 	ath12k_hal_srng_deinit(ab);
 	ath12k_ce_free_pipes(ab);
 	ath12k_ahb_resource_deinit(ab);

base-commit: b64812cb6cec118f6c59a4ec2bd4848db17f6bb2
-- 
2.34.1

Re: [PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[Rameshkumar Sundaram]

On 10/1/2026 10:24 AM, Aaradhana Sahu wrote:
> Currently, NAPI instances and their associated netdevs allocated for AHB
> external IRQ groups are not released when external IRQ configuration
> fails. They are also not freed when arch_init() fails after
> ath12k_ahb_config_irq() succeeds during probe, or when the driver is
> unloaded.
> 
> Fix this by adding a common cleanup helper to release the NAPI instances
> and associated netdevs. Call it from the external IRQ configuration error
> path, probe error path, and driver cleanup path.
> 
> Tested-on: IPQ5332 hw1.0 AHB WLAN.WBE.1.6-01275-QCAHKSWPL_SILICONZ-1
> 
> Fixes: 6cee30f0da75 ("wifi: ath12k: add AHB driver support for IPQ5332")
> Fixes: 614c23e24ee8 ("wifi: ath12k: Support arch-specific DP device allocation")
> Signed-off-by: Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>
> ---
> v2:
>    -Remove unnecessary irq_grp->napi_ndev NULL check.
> ---
>   drivers/net/wireless/ath/ath12k/ahb.c | 41 ++++++++++++++++++++++++---
>   1 file changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/ahb.c b/drivers/net/wireless/ath/ath12k/ahb.c
> index 85360365aa4b..9b77632bd1bf 100644
> --- a/drivers/net/wireless/ath/ath12k/ahb.c
> +++ b/drivers/net/wireless/ath/ath12k/ahb.c
> @@ -191,6 +191,26 @@ static void __ath12k_ahb_ext_irq_disable(struct ath12k_base *ab)
>   	}
>   }
>   
> +static void ath12k_ahb_free_napi_for_group(struct ath12k_ext_irq_grp *irq_grp)
> +{
> +	netif_napi_del(&irq_grp->napi);
> +	free_netdev(irq_grp->napi_ndev);
> +	irq_grp->napi_ndev = NULL;
> +}
> +
> +static void ath12k_ahb_free_napi(struct ath12k_base *ab, int num_ext_irq_grp)
> +{
> +	int i;
> +
> +	for (i = 0; i < num_ext_irq_grp; i++)
> +		ath12k_ahb_free_napi_for_group(&ab->ext_irq_grp[i]);
> +}
> +
> +static void ath12k_ahb_free_ext_napi(struct ath12k_base *ab)
> +{
> +	ath12k_ahb_free_napi(ab, ATH12K_EXT_IRQ_GRP_NUM_MAX);
> +}
> +
>   static void ath12k_ahb_ext_grp_enable(struct ath12k_ext_irq_grp *irq_grp)
>   {
>   	int i;
> @@ -622,8 +642,10 @@ static int ath12k_ahb_config_ext_irq(struct ath12k_base *ab)
>   		irq_grp->grp_id = i;
>   
>   		irq_grp->napi_ndev = alloc_netdev_dummy(0);
> -		if (!irq_grp->napi_ndev)
> -			return -ENOMEM;
> +		if (!irq_grp->napi_ndev) {
> +			ret = -ENOMEM;
> +			goto free_napi_ndev;
> +		}
>   
>   		netif_napi_add(irq_grp->napi_ndev, &irq_grp->napi,
>   			       ath12k_ahb_ext_grp_napi_poll);
> @@ -675,12 +697,19 @@ static int ath12k_ahb_config_ext_irq(struct ath12k_base *ab)


platform_get_irq_byname() can return a negative error code. Should we 
handle that explicitly before calling devm_request_irq()?

Something like:

  irq = platform_get_irq_byname(ab->pdev, irq_name[irq_idx]);
   if (irq < 0) {
	ath12k_warn(ab, "failed to retrieve irq number for %s\n", 
irq_name[irq_idx]);
         ret = irq;
         ath12k_ahb_free_napi_for_group(irq_grp);
         goto free_napi_ndev;
   }


>   					       ath12k_ahb_ext_interrupt_handler,
>   					       IRQF_TRIGGER_RISING,
>   					       irq_name[irq_idx], irq_grp);
> -			if (ret)
> +			if (ret) {
>   				ath12k_warn(ab, "failed request_irq for %d\n", irq);
> +				ath12k_ahb_free_napi_for_group(irq_grp);
> +				goto free_napi_ndev;
> +			}
>   		}
>   	}
>   
>   	return 0;
> +
> +free_napi_ndev:
> +	ath12k_ahb_free_napi(ab, i);
> +	return ret;
>   }
>   
>   static int ath12k_ahb_config_irq(struct ath12k_base *ab)
> @@ -1353,7 +1382,7 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
>   	ret = ab_ahb->device_family_ops->arch_init(ab);
>   	if (ret) {
>   		ath12k_err(ab, "AHB arch_init failed %d\n", ret);
> -		goto err_rproc_deconfigure;
> +		goto err_free_irq;
>   	}
>   
>   	ret = ath12k_core_init(ab);
> @@ -1367,6 +1396,9 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
>   err_deinit_arch:
>   	ab_ahb->device_family_ops->arch_deinit(ab);
>   
> +err_free_irq:
> +	ath12k_ahb_free_ext_napi(ab);
> +
>   err_rproc_deconfigure:
>   	mutex_lock(&ath12k_rproc_info_lock);
>   	ath12k_ahb_deconfigure_rproc(ab);
> @@ -1409,6 +1441,7 @@ static void ath12k_ahb_free_resources(struct ath12k_base *ab)
>   	struct platform_device *pdev = ab->pdev;
>   	struct ath12k_ahb *ab_ahb = ath12k_ab_to_ahb(ab);
>   
> +	ath12k_ahb_free_ext_napi(ab);
>   	ath12k_hal_srng_deinit(ab);
>   	ath12k_ce_free_pipes(ab);
>   	ath12k_ahb_resource_deinit(ab);
> 
> base-commit: b64812cb6cec118f6c59a4ec2bd4848db17f6bb2


-- 
Ramesh

Re: [PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[Aaradhana Sahu]

On 10/8/2026 10:46 PM, Rameshkumar Sundaram wrote:
> On 10/1/2026 10:24 AM, Aaradhana Sahu wrote:
>> Currently, NAPI instances and their associated netdevs allocated for AHB
>> external IRQ groups are not released when external IRQ configuration
>> fails. They are also not freed when arch_init() fails after
>> ath12k_ahb_config_irq() succeeds during probe, or when the driver is
>> unloaded.
>>
>> Fix this by adding a common cleanup helper to release the NAPI instances
>> and associated netdevs. Call it from the external IRQ configuration error
>> path, probe error path, and driver cleanup path.
>>
>> Tested-on: IPQ5332 hw1.0 AHB WLAN.WBE.1.6-01275-QCAHKSWPL_SILICONZ-1
>>
>> Fixes: 6cee30f0da75 ("wifi: ath12k: add AHB driver support for IPQ5332")
>> Fixes: 614c23e24ee8 ("wifi: ath12k: Support arch-specific DP device allocation")
>> Signed-off-by: Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>
>> ---
>> v2:
>>    -Remove unnecessary irq_grp->napi_ndev NULL check.
>> ---
>>   drivers/net/wireless/ath/ath12k/ahb.c | 41 ++++++++++++++++++++++++---
>>   1 file changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/ahb.c b/drivers/net/wireless/ath/ath12k/ahb.c
>> index 85360365aa4b..9b77632bd1bf 100644
>> --- a/drivers/net/wireless/ath/ath12k/ahb.c
>> +++ b/drivers/net/wireless/ath/ath12k/ahb.c
>> @@ -191,6 +191,26 @@ static void __ath12k_ahb_ext_irq_disable(struct ath12k_base *ab)
>>       }
>>   }
>>   +static void ath12k_ahb_free_napi_for_group(struct ath12k_ext_irq_grp *irq_grp)
>> +{
>> +    netif_napi_del(&irq_grp->napi);
>> +    free_netdev(irq_grp->napi_ndev);
>> +    irq_grp->napi_ndev = NULL;
>> +}
>> +
>> +static void ath12k_ahb_free_napi(struct ath12k_base *ab, int num_ext_irq_grp)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < num_ext_irq_grp; i++)
>> +        ath12k_ahb_free_napi_for_group(&ab->ext_irq_grp[i]);
>> +}
>> +
>> +static void ath12k_ahb_free_ext_napi(struct ath12k_base *ab)
>> +{
>> +    ath12k_ahb_free_napi(ab, ATH12K_EXT_IRQ_GRP_NUM_MAX);
>> +}
>> +
>>   static void ath12k_ahb_ext_grp_enable(struct ath12k_ext_irq_grp *irq_grp)
>>   {
>>       int i;
>> @@ -622,8 +642,10 @@ static int ath12k_ahb_config_ext_irq(struct ath12k_base *ab)
>>           irq_grp->grp_id = i;
>>             irq_grp->napi_ndev = alloc_netdev_dummy(0);
>> -        if (!irq_grp->napi_ndev)
>> -            return -ENOMEM;
>> +        if (!irq_grp->napi_ndev) {
>> +            ret = -ENOMEM;
>> +            goto free_napi_ndev;
>> +        }
>>             netif_napi_add(irq_grp->napi_ndev, &irq_grp->napi,
>>                      ath12k_ahb_ext_grp_napi_poll);
>> @@ -675,12 +697,19 @@ static int ath12k_ahb_config_ext_irq(struct ath12k_base *ab)
> 
> 
> platform_get_irq_byname() can return a negative error code. Should we handle that explicitly before calling devm_request_irq()?
> 
> Something like:
> 
>  irq = platform_get_irq_byname(ab->pdev, irq_name[irq_idx]);
>   if (irq < 0) {
>     ath12k_warn(ab, "failed to retrieve irq number for %s\n", irq_name[irq_idx]);
>         ret = irq;
>         ath12k_ahb_free_napi_for_group(irq_grp);
>         goto free_napi_ndev;
>   }
> 

You're right, thanks for pointing this out. This patch focuses on fixing the NAPI resource leaks.
Since handling the IRQ retrieval error is an existing issue, would it be okay to address it in a
separate patch?

> 
>>                              ath12k_ahb_ext_interrupt_handler,
>>                              IRQF_TRIGGER_RISING,
>>                              irq_name[irq_idx], irq_grp);
>> -            if (ret)
>> +            if (ret) {
>>                   ath12k_warn(ab, "failed request_irq for %d\n", irq);
>> +                ath12k_ahb_free_napi_for_group(irq_grp);
>> +                goto free_napi_ndev;
>> +            }
>>           }
>>       }
>>         return 0;
>> +
>> +free_napi_ndev:
>> +    ath12k_ahb_free_napi(ab, i);
>> +    return ret;
>>   }
>>     static int ath12k_ahb_config_irq(struct ath12k_base *ab)
>> @@ -1353,7 +1382,7 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
>>       ret = ab_ahb->device_family_ops->arch_init(ab);
>>       if (ret) {
>>           ath12k_err(ab, "AHB arch_init failed %d\n", ret);
>> -        goto err_rproc_deconfigure;
>> +        goto err_free_irq;
>>       }
>>         ret = ath12k_core_init(ab);
>> @@ -1367,6 +1396,9 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
>>   err_deinit_arch:
>>       ab_ahb->device_family_ops->arch_deinit(ab);
>>   +err_free_irq:
>> +    ath12k_ahb_free_ext_napi(ab);
>> +
>>   err_rproc_deconfigure:
>>       mutex_lock(&ath12k_rproc_info_lock);
>>       ath12k_ahb_deconfigure_rproc(ab);
>> @@ -1409,6 +1441,7 @@ static void ath12k_ahb_free_resources(struct ath12k_base *ab)
>>       struct platform_device *pdev = ab->pdev;
>>       struct ath12k_ahb *ab_ahb = ath12k_ab_to_ahb(ab);
>>   +    ath12k_ahb_free_ext_napi(ab);
>>       ath12k_hal_srng_deinit(ab);
>>       ath12k_ce_free_pipes(ab);
>>       ath12k_ahb_resource_deinit(ab);
>>
>> base-commit: b64812cb6cec118f6c59a4ec2bd4848db17f6bb2
> 
> 

Re: [PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[Jeff Johnson]

On 9/30/2026 9:54 PM, Aaradhana Sahu wrote:
>  static int ath12k_ahb_config_irq(struct ath12k_base *ab)
> @@ -1353,7 +1382,7 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
>  	ret = ab_ahb->device_family_ops->arch_init(ab);
>  	if (ret) {
>  		ath12k_err(ab, "AHB arch_init failed %d\n", ret);
> -		goto err_rproc_deconfigure;
> +		goto err_free_irq;
>  	}
>  
>  	ret = ath12k_core_init(ab);
> @@ -1367,6 +1396,9 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
>  err_deinit_arch:
>  	ab_ahb->device_family_ops->arch_deinit(ab);
>  
> +err_free_irq:

my review agent notes:
**Misleading label name `err_free_irq`**

The label name implies IRQ teardown, but the IRQs registered via
`devm_request_irq()` are automatically freed on device detach — no explicit
free is needed here. This label only frees NAPI/netdev resources. Suggest
renaming to `err_free_ext_napi` or `err_free_napi` to match the actual
operation and avoid confusing future readers.

No need to post a new version just for this -- I'll make this change in my
'pending' branch if the reviewers are otherwise ok with your patch

> +	ath12k_ahb_free_ext_napi(ab);
> +
>  err_rproc_deconfigure:
>  	mutex_lock(&ath12k_rproc_info_lock);
>  	ath12k_ahb_deconfigure_rproc(ab);

Re: [PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[Jeff Johnson]

On 10/8/2026 8:38 PM, Aaradhana Sahu wrote:
> On 10/8/2026 10:46 PM, Rameshkumar Sundaram wrote:
>> On 10/1/2026 10:24 AM, Aaradhana Sahu wrote:

>>> @@ -622,8 +642,10 @@ static int ath12k_ahb_config_ext_irq(struct ath12k_base *ab)
>>>           irq_grp->grp_id = i;
>>>             irq_grp->napi_ndev = alloc_netdev_dummy(0);
>>> -        if (!irq_grp->napi_ndev)
>>> -            return -ENOMEM;
>>> +        if (!irq_grp->napi_ndev) {
>>> +            ret = -ENOMEM;
>>> +            goto free_napi_ndev;
>>> +        }
>>>             netif_napi_add(irq_grp->napi_ndev, &irq_grp->napi,
>>>                      ath12k_ahb_ext_grp_napi_poll);
>>> @@ -675,12 +697,19 @@ static int ath12k_ahb_config_ext_irq(struct ath12k_base *ab)
>>
>>
>> platform_get_irq_byname() can return a negative error code. Should we handle that explicitly before calling devm_request_irq()?
>>
>> Something like:
>>
>>  irq = platform_get_irq_byname(ab->pdev, irq_name[irq_idx]);
>>   if (irq < 0) {
>>     ath12k_warn(ab, "failed to retrieve irq number for %s\n", irq_name[irq_idx]);
>>         ret = irq;
>>         ath12k_ahb_free_napi_for_group(irq_grp);
>>         goto free_napi_ndev;
>>   }
>>
> 
> You're right, thanks for pointing this out. This patch focuses on fixing the NAPI resource leaks.
> Since handling the IRQ retrieval error is an existing issue, would it be okay to address it in a
> separate patch?

Yes, please handle as a separate patch.

My review agent notes:

There are three platform_get_irq_byname() call sites in ahb.c:

  ┌──────┬────────────────────────────────┬─────────────────────────────────┐
  │ Line │            Location            │      Checks return value?       │
  ├──────┼────────────────────────────────┼─────────────────────────────────┤
  │ 692  │ ext IRQ loop                   │ No                              │
  │      │ (ath12k_ahb_config_ext_irq)    │                                 │
  ├──────┼────────────────────────────────┼─────────────────────────────────┤
  │ 730  │ CE IRQ loop                    │ No                              │
  │      │ (ath12k_ahb_config_irq)        │                                 │
  ├──────┼────────────────────────────────┼─────────────────────────────────┤
  │      │                                │ Yes — if                        │
  │ 819  │ userpd IRQ                     │ (ab_ahb->userpd_irq_num[i] < 0) │
  │      │                                │  return ...                     │
  └──────┴────────────────────────────────┴─────────────────────────────────┘

So the driver is already inconsistent: userpd checks, CE and ext don't.

That said, the ext and CE sites aren't silently broken — devm_request_irq()
with a negative IRQ number hits irq_to_desc(), gets NULL, and returns -EINVAL,
which the existing if (ret) check catches. The practical impact is a
confusing error message ("failed request_irq") when the real failure was
platform_get_irq_byname() itself.

And further suggest it should be a separate patch. The reasons:

1. This bug is pre-existing at both line 692 and line 730 — it wasn't
introduced by the NAPI leak patch. Adding the check to only line 692 (as the
reviewer suggests) leaves line 730 unfixed and makes the code more
inconsistent than before.
2. A correct fix needs to cover both sites together, with a consistent error
message and error-path handling.
3. This patch already has a well-defined scope (NAPI cleanup). Adding
unrelated error-handling hygiene muddies the Fixes: attribution and makes it
harder to backport.
> 
>>
>>>                              ath12k_ahb_ext_interrupt_handler,
>>>                              IRQF_TRIGGER_RISING,
>>>                              irq_name[irq_idx], irq_grp);
>>> -            if (ret)
>>> +            if (ret) {
>>>                   ath12k_warn(ab, "failed request_irq for %d\n", irq);
>>> +                ath12k_ahb_free_napi_for_group(irq_grp);
>>> +                goto free_napi_ndev;
>>> +            }
>>>           }
>>>       }
>>>         return 0;
>>> +
>>> +free_napi_ndev:
>>> +    ath12k_ahb_free_napi(ab, i);
>>> +    return ret;
>>>   }
>>>     static int ath12k_ahb_config_irq(struct ath12k_base *ab)

Re: [PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[Baochen Qiang]

On 10/1/2026 12:54 PM, Aaradhana Sahu wrote:
> Currently, NAPI instances and their associated netdevs allocated for AHB
> external IRQ groups are not released when external IRQ configuration
> fails. They are also not freed when arch_init() fails after
> ath12k_ahb_config_irq() succeeds during probe, or when the driver is
> unloaded.
> 
> Fix this by adding a common cleanup helper to release the NAPI instances
> and associated netdevs. Call it from the external IRQ configuration error
> path, probe error path, and driver cleanup path.
> 
> Tested-on: IPQ5332 hw1.0 AHB WLAN.WBE.1.6-01275-QCAHKSWPL_SILICONZ-1
> 
> Fixes: 6cee30f0da75 ("wifi: ath12k: add AHB driver support for IPQ5332")
> Fixes: 614c23e24ee8 ("wifi: ath12k: Support arch-specific DP device allocation")
> Signed-off-by: Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>

Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>

Re: [PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[Rameshkumar Sundaram]

On 10/1/2026 10:24 AM, Aaradhana Sahu wrote:
> Currently, NAPI instances and their associated netdevs allocated for AHB
> external IRQ groups are not released when external IRQ configuration
> fails. They are also not freed when arch_init() fails after
> ath12k_ahb_config_irq() succeeds during probe, or when the driver is
> unloaded.
> 
> Fix this by adding a common cleanup helper to release the NAPI instances
> and associated netdevs. Call it from the external IRQ configuration error
> path, probe error path, and driver cleanup path.
> 
> Tested-on: IPQ5332 hw1.0 AHB WLAN.WBE.1.6-01275-QCAHKSWPL_SILICONZ-1
> 
> Fixes: 6cee30f0da75 ("wifi: ath12k: add AHB driver support for IPQ5332")
> Fixes: 614c23e24ee8 ("wifi: ath12k: Support arch-specific DP device allocation")
> Signed-off-by: Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>
Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>

Re: [PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[Aaradhana Sahu]

On 10/10/2026 2:11 AM, Jeff Johnson wrote:
> On 10/8/2026 8:38 PM, Aaradhana Sahu wrote:
>> On 10/8/2026 10:46 PM, Rameshkumar Sundaram wrote:
>>> On 10/1/2026 10:24 AM, Aaradhana Sahu wrote:
> 
>>>> @@ -622,8 +642,10 @@ static int ath12k_ahb_config_ext_irq(struct ath12k_base *ab)
>>>>           irq_grp->grp_id = i;
>>>>             irq_grp->napi_ndev = alloc_netdev_dummy(0);
>>>> -        if (!irq_grp->napi_ndev)
>>>> -            return -ENOMEM;
>>>> +        if (!irq_grp->napi_ndev) {
>>>> +            ret = -ENOMEM;
>>>> +            goto free_napi_ndev;
>>>> +        }
>>>>             netif_napi_add(irq_grp->napi_ndev, &irq_grp->napi,
>>>>                      ath12k_ahb_ext_grp_napi_poll);
>>>> @@ -675,12 +697,19 @@ static int ath12k_ahb_config_ext_irq(struct ath12k_base *ab)
>>>
>>>
>>> platform_get_irq_byname() can return a negative error code. Should we handle that explicitly before calling devm_request_irq()?
>>>
>>> Something like:
>>>
>>>  irq = platform_get_irq_byname(ab->pdev, irq_name[irq_idx]);
>>>   if (irq < 0) {
>>>     ath12k_warn(ab, "failed to retrieve irq number for %s\n", irq_name[irq_idx]);
>>>         ret = irq;
>>>         ath12k_ahb_free_napi_for_group(irq_grp);
>>>         goto free_napi_ndev;
>>>   }
>>>
>>
>> You're right, thanks for pointing this out. This patch focuses on fixing the NAPI resource leaks.
>> Since handling the IRQ retrieval error is an existing issue, would it be okay to address it in a
>> separate patch?
> 
> Yes, please handle as a separate patch.
> 
> My review agent notes:
> 
> There are three platform_get_irq_byname() call sites in ahb.c:
> 
>   ┌──────┬────────────────────────────────┬─────────────────────────────────┐
>   │ Line │            Location            │      Checks return value?       │
>   ├──────┼────────────────────────────────┼─────────────────────────────────┤
>   │ 692  │ ext IRQ loop                   │ No                              │
>   │      │ (ath12k_ahb_config_ext_irq)    │                                 │
>   ├──────┼────────────────────────────────┼─────────────────────────────────┤
>   │ 730  │ CE IRQ loop                    │ No                              │
>   │      │ (ath12k_ahb_config_irq)        │                                 │
>   ├──────┼────────────────────────────────┼─────────────────────────────────┤
>   │      │                                │ Yes — if                        │
>   │ 819  │ userpd IRQ                     │ (ab_ahb->userpd_irq_num[i] < 0) │
>   │      │                                │  return ...                     │
>   └──────┴────────────────────────────────┴─────────────────────────────────┘
> 
> So the driver is already inconsistent: userpd checks, CE and ext don't.
> 
> That said, the ext and CE sites aren't silently broken — devm_request_irq()
> with a negative IRQ number hits irq_to_desc(), gets NULL, and returns -EINVAL,
> which the existing if (ret) check catches. The practical impact is a
> confusing error message ("failed request_irq") when the real failure was
> platform_get_irq_byname() itself.
> 
> And further suggest it should be a separate patch. The reasons:
> 
> 1. This bug is pre-existing at both line 692 and line 730 — it wasn't
> introduced by the NAPI leak patch. Adding the check to only line 692 (as the
> reviewer suggests) leaves line 730 unfixed and makes the code more
> inconsistent than before.
> 2. A correct fix needs to cover both sites together, with a consistent error
> message and error-path handling.
> 3. This patch already has a well-defined scope (NAPI cleanup). Adding
> unrelated error-handling hygiene muddies the Fixes: attribution and makes it
> harder to backport.

Thanks for the clarification. I'll address the IRQ retrieval error handling in a
separate patch.
>>
>>>
>>>>                              ath12k_ahb_ext_interrupt_handler,
>>>>                              IRQF_TRIGGER_RISING,
>>>>                              irq_name[irq_idx], irq_grp);
>>>> -            if (ret)
>>>> +            if (ret) {
>>>>                   ath12k_warn(ab, "failed request_irq for %d\n", irq);
>>>> +                ath12k_ahb_free_napi_for_group(irq_grp);
>>>> +                goto free_napi_ndev;
>>>> +            }
>>>>           }
>>>>       }
>>>>         return 0;
>>>> +
>>>> +free_napi_ndev:
>>>> +    ath12k_ahb_free_napi(ab, i);
>>>> +    return ret;
>>>>   }
>>>>     static int ath12k_ahb_config_irq(struct ath12k_base *ab)

Re: [PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[Jeff Johnson]

On 10/9/2026 1:17 PM, Jeff Johnson wrote:
> On 9/30/2026 9:54 PM, Aaradhana Sahu wrote:
>>  static int ath12k_ahb_config_irq(struct ath12k_base *ab)
>> @@ -1353,7 +1382,7 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
>>  	ret = ab_ahb->device_family_ops->arch_init(ab);
>>  	if (ret) {
>>  		ath12k_err(ab, "AHB arch_init failed %d\n", ret);
>> -		goto err_rproc_deconfigure;
>> +		goto err_free_irq;
>>  	}
>>  
>>  	ret = ath12k_core_init(ab);
>> @@ -1367,6 +1396,9 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
>>  err_deinit_arch:
>>  	ab_ahb->device_family_ops->arch_deinit(ab);
>>  
>> +err_free_irq:
> 
> my review agent notes:
> **Misleading label name `err_free_irq`**
> 
> The label name implies IRQ teardown, but the IRQs registered via
> `devm_request_irq()` are automatically freed on device detach — no explicit
> free is needed here. This label only frees NAPI/netdev resources. Suggest
> renaming to `err_free_ext_napi` or `err_free_napi` to match the actual
> operation and avoid confusing future readers.
> 
> No need to post a new version just for this -- I'll make this change in my
> 'pending' branch if the reviewers are otherwise ok with your patch
> 
>> +	ath12k_ahb_free_ext_napi(ab);
>> +
>>  err_rproc_deconfigure:
>>  	mutex_lock(&ath12k_rproc_info_lock);
>>  	ath12k_ahb_deconfigure_rproc(ab);

Pending version with the label modified:

https://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git/commit/?h=pending&id=ca18680289858564a3bddf3df67aa758b5f16d8a

Re: [PATCH ath-next v2] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths

[Jeff Johnson]

On Thu, 01 Oct 2026 10:24:23 +0530, Aaradhana Sahu wrote:
> Currently, NAPI instances and their associated netdevs allocated for AHB
> external IRQ groups are not released when external IRQ configuration
> fails. They are also not freed when arch_init() fails after
> ath12k_ahb_config_irq() succeeds during probe, or when the driver is
> unloaded.
> 
> Fix this by adding a common cleanup helper to release the NAPI instances
> and associated netdevs. Call it from the external IRQ configuration error
> path, probe error path, and driver cleanup path.
> 
> [...]

Applied, thanks!

[1/1] wifi: ath12k: Fix NAPI resource leaks in AHB external IRQ paths
      commit: ebc938ba1d5ff33652013b99c7b9a74f05690336

Best regards,
-- 
Jeff Johnson <jeff.johnson@oss.qualcomm.com>