Re: [PATCH ath-next v4] wifi: ath12k: avoid dynamic alloc when parsing wmi tb

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

On 3/19/2026 7:35 AM, Nicolas Escande wrote:
> On Thu Mar 19, 2026 at 12:08 PM CET, Rameshkumar Sundaram wrote:
>>
>> Since CONFIG_ATH12K is tristate, a built-in boot can continue past a 
>> failed ath12k_init() and still run ath12k_wifi7_init().
>>
> I genuinely thought the kernel prevented this. I was wrong.
> 
>> Please ensure that later initialization path is guarded against 
>> allocation failure.
>>
> I can add a flag like so to be able to check from ath12k_wifi7_init() if the
> init finished ok. Something in the lines of
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
> index 6c034071cc6d..742fb33f41ff 100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -34,6 +34,9 @@ module_param_named(ftm_mode, ath12k_ftm_mode, bool, 0444);
>  MODULE_PARM_DESC(ftm_mode, "Boots up in factory test mode");
>  EXPORT_SYMBOL(ath12k_ftm_mode);
>  
> +bool ath12k_init_ok = false;
> +EXPORT_SYMBOL(ath12k_init_ok);
> +
>  /* protected with ath12k_hw_group_mutex */
>  static struct list_head ath12k_hw_group_list = LIST_HEAD_INIT(ath12k_hw_group_list);
>  
> @@ -2323,7 +2326,14 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
>  
>  static int ath12k_init(void)
>  {
> -	return ath12k_wmi_alloc();
> +	int ret;
> +
> +	ret = ath12k_wmi_alloc();
> +	if (ret)
> +		return -ENOMEM;
> +
> +	ath12k_init_ok = true;
> +	return 0;
>  }
>  
>  static void ath12k_exit(void)
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 59c193b24764..f35571b1a541 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -101,6 +101,8 @@ enum ath12k_crypt_mode {
>  	ATH12K_CRYPT_MODE_SW,
>  };
>  
> +extern bool ath12k_init_ok;
> +
>  static inline enum wme_ac ath12k_tid_to_ac(u32 tid)
>  {
>  	return (((tid == 0) || (tid == 3)) ? WME_AC_BE :
> diff --git a/drivers/net/wireless/ath/ath12k/wifi7/core.c b/drivers/net/wireless/ath/ath12k/wifi7/core.c
> index a02c57acf137..542ec10fabf1 100644
> --- a/drivers/net/wireless/ath/ath12k/wifi7/core.c
> +++ b/drivers/net/wireless/ath/ath12k/wifi7/core.c
> @@ -38,6 +38,9 @@ void ath12k_wifi7_arch_deinit(struct ath12k_base *ab)
>  
>  static int ath12k_wifi7_init(void)
>  {
> +	if (!ath12k_init_ok)
> +		return -ENOTSUPP;
> +
>  	ahb_err = ath12k_wifi7_ahb_init();
>  	if (ahb_err)
>  		pr_warn("Failed to initialize ath12k Wi-Fi 7 AHB device: %d\n",
> 
> 
> I don't like it much but it is easy enough.
> But I don't know if there is a more idiomatic way of doing things

I'd prefer to expose a function rather than a global variable.
In other words keep the flag static, and expose a function that returns the
value of the flag, i.e.:

bool ath12k_core_initialized(void)
{
	return ath12k_init_ok;
}
EXPORT_SYMBOL(ath12k_core_initialized);

> 
>> Or may be have this allocated on first device probe and free it on last 
>> device deinit ?
> 
> That seems even more involved. It would be easier to go back to the previous
> version and simply, alloc it once per ath12k_base
> 
> What do you guys think ?
> 

Going back to that may be the better solution. It isn't nice that this current
solution may allocate memory when the driver isn't actually used. But I'll let
others on the team weigh in as well.

/jeff