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

[Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>]

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