Re: [PATCH 2/2] wifi: ath12k: skip unknown direct buffer ring module IDs

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

On 5/5/2026 10:24 AM, Nazar Mokrynskyi wrote:
> The firmware may advertise direct buffer ring capabilities for module
> IDs beyond what the driver currently knows about (WMI_DIRECT_BUF_MAX).
> This happens with newer firmware versions that support additional ring
> types not yet implemented in the driver.

What "newer firmware version" are you using that has this issue?

Can you share the "Skipping unknown direct buf ring module id" logs from your
modified driver?

> 
> The current code treats an unknown module_id as a fatal error, returning
> -EINVAL and tearing down the entire driver initialization. This is
> incorrect: the driver only needs to set up rings for types it uses
> (SPECTRAL=0, CFR=1) and can safely ignore capability advertisements for
> unknown types.
> 
> Change the unknown module_id handling to skip the entry with a debug
> message rather than failing, allowing initialization to proceed.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.6-01243-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Nazar Mokrynskyi <nazar@mokrynskyi.com>
> ---
>  drivers/net/wireless/ath/ath12k/wmi.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index 484fdd3b1..0e776a8d8 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -4814,10 +4814,10 @@ static int ath12k_wmi_dma_ring_caps(struct ath12k_base *ab,
>  	dir_buff_caps = ab->db_caps;
>  	for (i = 0; i < dma_caps_parse->n_dma_ring_caps; i++) {
>  		if (le32_to_cpu(dma_caps[i].module_id) >= WMI_DIRECT_BUF_MAX) {
> -			ath12k_warn(ab, "Invalid module id %d\n",
> -				    le32_to_cpu(dma_caps[i].module_id));
> -			ret = -EINVAL;
> -			goto free_dir_buff;
> +			ath12k_dbg(ab, ATH12K_DBG_WMI,
> +				   "Skipping unknown direct buf ring module id %d\n",
> +				   le32_to_cpu(dma_caps[i].module_id));
> +			continue;

by continuing you don't populate the dir_buff_caps[i] information below,
leaving that record with zeroed data from the initial allocation. Does other
code correctly handle that?

would it be better to have a separate count of the number of dir_buff_caps
records that are actually filled, and then update
dma_caps_parse->n_dma_ring_caps to match once the parsing has completed?

>  		}
>  
>  		dir_buff_caps[i].id = le32_to_cpu(dma_caps[i].module_id);
> @@ -4829,10 +4829,6 @@ static int ath12k_wmi_dma_ring_caps(struct ath12k_base *ab,
>  	}
>  
>  	return 0;
> -
> -free_dir_buff:
> -	ath12k_wmi_free_dbring_caps(ab);
> -	return ret;
>  }
>  
>  static void