Re: [PATCH] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params

On 12/10/25 12:46 AM, Baochen Qiang wrote:
> 
> 
> On 12/10/2025 10:40 AM, Alexandru Gagniuc wrote:
>> ".max_tx_ring" is an upper bounds to indexing ".tcl2wbm_rbm_map". It
>> is initialized in, core.c, a different file than the array init. This
>> spaghetti-like relation is fragile and not obvious. Accidentally
>> setting ".max_tx_ring" too high leads to a hard to track out-of-
>> bounds access and memory corruption.
>>
>> Clarify this dependency by moving ".max_tx_ring" adjacent to the array
>> ".tcl2wbm_rbm_map". Use ARRAY_SIZE() instead of #defines to initialize
>> the length field. Remove DP_TCL_NUM_RING_MAX_QCA6390, as it is no
>> longer required.
>>
>> The intent is to make the code easier to understand rather than fix
>> an existing bug.
>>
> 
> Even the code chane works, I am not sure whether we should do this. Because, logically
> max_tx_ring represents hardware capability which is static. However the change actually
> implies max_tx_ring varies on tcl2wbm_rbm_map definition.

I see what you mean, although tcl2wbm_rbm_map is const. More details below.


> If we are going to add something to avoid the potential out-of-bound access or to improve
> code readability, how about something like
> 
> 	ASSERT(hw_params.max_tx_ring <= ARRAR_SIZE(tcl2wbm_rbm_map))
A static assert might be a good solution. I don't know how to do that.
By the time we have hw_params.max_tx_ring and tcl2wbm_rbm_map, the
latter is a pointer, so we can't use ARRAY_SIZE(). We could try to do
it dynamically, but I feel that's spaghetti code:

	if (tcl2wbm_rbm_map == &ath11k_hw_hal_params_ipq8074)
		ASSERT(hw_params.max_tx_ring <= ARRAY_SIZE(ath11k_hw_hal_params_ipq8074));
	else if (...)
		...

Alternatively, I can take the suggestion from your other email, and
keep the "max_tx_ring", or "num_tx_rings" name. Because it is part of
the hw_params struct (via .hal_params), it still describes the
hardware. While the value is derived from a constant array, instead
of being hardcoded, it remains an immutable quantity, consistent with
a static hardware descriptor, would you agree?

Alex