Re: [PATCH] wifi: ath11k: fix qmi memory allocation logic for CALDB region
Baochen Qiang <[email protected]>
| Newsgroups | org.infradead.lists.ath11k,org.kernel.vger.linux-kernel,org.kernel.vger.linux-wireless,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
On 12/25/2025 5:47 AM, Alex G. wrote: > On Monday, December 8, 2025 4:23:46 AM CST Baochen Qiang wrote: >> On 12/7/2025 1:58 AM, Alexandru Gagniuc wrote: >>> Memory region assignment in ath11k_qmi_assign_target_mem_chunk() >>> >>> assumes that: >>> 1. firmware will make a HOST_DDR_REGION_TYPE request, and >>> 2. this request is processed before CALDB_MEM_REGION_TYPE >>> >>> In this case CALDB_MEM_REGION_TYPE, can safely be assigned immediately >>> after the host region. >>> >>> However, if the HOST_DDR_REGION_TYPE request is not made, or the >>> reserved-memory node is not present, then res.start and res.end are 0, >>> and host_ddr_sz remains uninitialized. The physical address should >>> fall back to ATH11K_QMI_CALDB_ADDRESS. That doesn't happen: >>> >>> resource_size(&res) returns 1 for an empty resource, and thus the if >>> clause never takes the fallback path. ab->qmi.target_mem[idx].paddr >>> is assigned the uninitialized value of host_ddr_sz + 0 (res.start). >>> >>> Use "if (res.end > res.start)" for the predicate, which correctly >>> falls back to ATH11K_QMI_CALDB_ADDRESS. > > I am ready to submit the IPQ9574 support. This patch is a dependency. Should I > include this change in the series that adds IPQ9574? Better not, first reason is for track purpose. And the second, since the patch is fixing a broken commit, IMO being standalone makes it easy for stable team to do backport work. > >> In addition, does it make sense to do of_reserved_mem_region_to_resource() >> before the loop, which may give CALDB_MEM_REGION_TYPE a chance even >> HOST_DDR_REGION_TYPE request is not made? > > I'm sorry that I initially missed this question. I don't think we should move > &res initialization outside the loop. We also need host_ddr_sz to be > initialized by a HOST_DDR_REGION_TYPE (1) request. On IPQ9574, the firmware > doesn't make that request, so host_ddr_sz remains uninitialized. Since &res > and host_ddr_sz are used together, I think it's better to initialize them, > together. fine then > > > Without patch: > > ath11k c000000.wifi: qmi firmware request memory request > ath11k c000000.wifi: qmi mem seg type 4 size 409600 > ath11k c000000.wifi: qmi mem seg type 2 size 262144 > ath11k c000000.wifi: qmi mem seg type 3 size 1048576 > ... > ath11k c000000.wifi: failed to assign qmi target memory: -5 > > > > With patch: > > ath11k c000000.wifi: qmi firmware request memory request > ath11k c000000.wifi: qmi mem seg type 4 size 409600 > ath11k c000000.wifi: qmi mem seg type 2 size 262144 > ath11k c000000.wifi: qmi mem seg type 3 size 1048576 > ath11k c000000.wifi: qmi ignore invalid mem req type 3 > ath11k c000000.wifi: qmi req mem_seg[0] 0x000000004ba00000 409600 4 > ath11k c000000.wifi: qmi req mem_seg[1] 0x000000004b700000 262144 2 > > > Tested on : WLAN.HK.2.9.0.1-01890-QCAHKSWPL_SILICONZ-1 > > Alex > > > >