Re: [PATCH v5 07/13] wifi: ath12k: add support for fixed QMI firmware memory

[Krzysztof Kozlowski <krzk@kernel.org>]

On 04/02/2025 10:06, Raj Kumar Bhagat wrote:
> On 2/3/2025 3:42 PM, Krzysztof Kozlowski wrote:
>> On 03/02/2025 10:44, Raj Kumar Bhagat wrote:
>>> On 1/30/2025 1:16 PM, Krzysztof Kozlowski wrote:
>>>> On 30/01/2025 05:35, Raj Kumar Bhagat wrote:
>>>>> @@ -2646,6 +2663,136 @@ static int ath12k_qmi_alloc_target_mem_chunk(struct ath12k_base *ab)
>>>>>  	return ret;
>>>>>  }
>>>>>  
>>>>> +static int ath12k_qmi_assign_target_mem_chunk(struct ath12k_base *ab)
>>>>> +{
>>>>> +	struct device_node *mem_node;
>>>>> +	struct resource res, m3_res;
>>>>> +	u32 bdf_start_addr;
>>>>> +	int i, idx, ret;
>>>>> +
>>>>> +	for (i = 0, idx = 0; i < ab->qmi.mem_seg_count; i++) {
>>>>> +		switch (ab->qmi.target_mem[i].type) {
>>>>> +		case HOST_DDR_REGION_TYPE:
>>>>> +			mem_node = ath12k_core_get_reserved_mem_by_name(ab, "q6-region");
>>>>
>>>>
>>>> Why cannot you use existing API for reserved memory -
>>>> of_reserved_mem_lookup()?
>>>>
>>>
>>> The of_reserved_mem_lookup() requires reserved memory node to read the memory and
>>> return in the structure "struct reserved_mem".
>>>
>>> The of_reserved_mem_lookup() would be used after we get the reserved memory node
>>> using the API - ath12k_core_get_reserved_mem_by_name(ab, "q6-region");
>>>
>>> In next version we would use of_reserved_mem_lookup(), Something like below:
>>>     mem_node = ath12k_core_get_reserved_mem_by_name(ab, "q6-region");
>>
>> Then why do you need ath12k_core_get_reserved_mem_by_name() in the first
>> place? Just use of_reserved_mem_lookup() directly. Why do you need to
>> parse phandle before of_reserved_mem_lookup()?
>>
> 
> Sorry, I'm having difficulty understanding this.
> We have the WiFi node at ab->dev->of_node, but we don't have a node for the reserved-memory
> 'q6-region'. The of_reserved_mem_lookup() function requires the device node for 'q6-region'.
> 
> Could you please suggest how we can use of_reserved_mem_lookup() without obtaining the
> 'q6-region' node first.


Hm, it seems you are not using it for this device, so indeed you need to
parse phandle. You can still code it simpler -
of_property_match_string() is not necessary and
of_address_to_resource()+of_node_put()  could be in the
ath12k_core_get_reserved_mem()

Best regards,
Krzysztof