Re: [PATCH 12/12] ath11k: Change QCN9074 firmware to operate in mode-2

From: Anilkumar Kolli <akolli@codeaurora.org>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: Jouni Malinen <jouni@codeaurora.org>,
	ath11k@lists.infradead.org, linux-wireless@vger.kernel.org,
	Seevalamuthu Mariappan <seevalam@codeaurora.org>
Subject: Re: [PATCH 12/12] ath11k: Change QCN9074 firmware to operate in mode-2
Date: Wed, 08 Dec 2021 18:09:49 +0530	[thread overview]
Message-ID: <2918b38bca0d8d1fac8f1edabe5ecce0@codeaurora.org> (raw)
In-Reply-To: <871r3lkcqc.fsf@codeaurora.org>

On 2021-11-12 13:49, Kalle Valo wrote:
> Jouni Malinen <jouni@codeaurora.org> writes:
> 
>> From: Seevalamuthu Mariappan <seevalam@codeaurora.org>
>> 
>> In mode-2 QCN9074 firmware uses 15 MB of host memory and firmware
>> request 1 MB size segments in QMI, whereas in mode-0 firmware uses 45 
>> MB
>> of host memory and each segment is of 2 MB size. In mode-2 firmware
>> operates with reduced number of vdevs and peers.
>> 
>> Tested-on: QCN9074 hw1.0 PCI 
>> WLAN.HK.2.4.0.1-01838-QCAHKSWPL_SILICONZ-1
>> 
>> Signed-off-by: Seevalamuthu Mariappan <seevalam@codeaurora.org>
>> Co-developed-by: Anilkumar Kolli <akolli@codeaurora.org>
>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
>> ---
>>  drivers/net/wireless/ath/ath11k/core.c | 23 ++++++++++++++++++++++-
>>  drivers/net/wireless/ath/ath11k/core.h |  6 ++++++
>>  drivers/net/wireless/ath/ath11k/hw.c   | 14 +++++++-------
>>  drivers/net/wireless/ath/ath11k/hw.h   | 20 +++++++++++---------
>>  drivers/net/wireless/ath/ath11k/mac.c  | 10 +++++-----
>>  drivers/net/wireless/ath/ath11k/qmi.c  |  2 +-
>>  6 files changed, 52 insertions(+), 23 deletions(-)
>> 
>> diff --git a/drivers/net/wireless/ath/ath11k/core.c 
>> b/drivers/net/wireless/ath/ath11k/core.c
>> index d2ab3b134632..9f2c9795767e 100644
>> --- a/drivers/net/wireless/ath/ath11k/core.c
>> +++ b/drivers/net/wireless/ath/ath11k/core.c
>> @@ -77,6 +77,7 @@ static const struct ath11k_hw_params 
>> ath11k_hw_params[] = {
>>  		.supports_shadow_regs = false,
>>  		.idle_ps = false,
>>  		.cold_boot_calib = true,
>> +		.fw_mem_mode = 0,
>>  		.supports_suspend = false,
>>  		.hal_desc_sz = sizeof(struct hal_rx_desc_ipq8074),
>>  		.fix_l1ss = true,
> 
> [...]
> 
>> +static const struct ath11k_num_vdevs_peers ath11k_vdevs_peers[] = {
>> +	{
>> +		.num_vdevs = (16 + 1),
>> +		.num_peers = 512,
>> +	},
>> +	{
>> +		.num_vdevs = (8 + 1),
>> +		.num_peers = 128,
>> +	},
>> +	{
>> +		.num_vdevs = 8,
>> +		.num_peers = 128,
>> +	},
>> +};
> 
> I am worried about this array. It implies that _all_ hardware support
> these modes but is that really the case? I would guess that these are
> very much hardware and firmware specific values.
> 
> So because of that I would feel clearer to have num_vdevs and num_peers
> in ath11k_hw_params, to make it clear that the values are hardware
> specific. And then have fw_mem_mode, num_vdevs and num_peers in their
> own struct within struct ath11k_hw_params, just like spectral has
> grouped valued together.
> 

Sorry for the delayed response. I will make num_vdevs and num_peers part 
of hw_params.
and post next version.

Thanks
Anil

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k