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

[Nazar Mokrynskyi <nazar@mokrynskyi.com>]

11.05.26 22:02, Jeff Johnson:
> 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?
ID is the same as in the original warning:

[   10.611560] ath12k_pci 0000:01:00.0: Invalid module id 2
[   10.616467] ath12k_pci 0000:01:00.0: failed to parse tlv -22

The firmware according to system logs (all this info is provided in [PATCH 0/2] email):

[    7.688137] ath12k_pci 0000:01:00.0: fw_version 0x160484db fw_build_timestamp 2025-12-09 20:09 fw_build_id QC_IMAGE_VERSION_STRING=WLAN.WBE.1.6-01243-QCAHKSWPL_SILICONZ-1

This is running on OpenWRT 25.12, which uses 6.12.74 kernel with modules backported from 6.18.7 kernel:
[    0.000000] Linux version 6.12.74 (builder@buildhost) (x86_64-openwrt-linux-musl-gcc (OpenWrt GCC 14.3.0 r32802-f505120278) 14.3.0, GNU ld (GNU Binutils) 2.44) #0 SMP Wed Mar 25 20:09:53 2026
[    6.997293] Loading modules backported from Linux version v6.18.7-0-g5dfbc5357
[    6.999210] Backport generated by backports.git c8a37ce

>> 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?
I'm very far from kernel development, let alone ath12k driver specifically.
The patch is provided to better showcase the problems I had.
If there is a better way to address it, I'd really appreciate if someone more knowledgeable could take over, I just hope this is helpful.

With these changes QCN9274 runs an access point with over 3 weeks uptime as of right now.
That is not necessary the proof of anything, but it does seem to work in practice so far.
>>  		}
>>  
>>  		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