Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <35c86c8e-afdb-adb6-f331-af74c99d3a02@quicinc.com>
Date:   Tue, 25 Jul 2023 12:44:09 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH v4 03/11] wifi: ath12k: WMI support to process EHT
 capabilities
Content-Language: en-US
To:     Wen Gong <quic_wgong@quicinc.com>, Kalle Valo <kvalo@kernel.org>
CC:     <ath12k@lists.infradead.org>, <linux-wireless@vger.kernel.org>,
        "Pradeep Kumar Chitrapu" <quic_pradeepc@quicinc.com>
References: <20230602235820.23912-1-quic_alokad@quicinc.com>
 <20230602235820.23912-4-quic_alokad@quicinc.com> <87ttv9aqtn.fsf@kernel.org>
 <dc0ae263-e5f6-b876-842a-7b598caf68fa@quicinc.com>
From:   Aloka Dixit <quic_alokad@quicinc.com>
In-Reply-To: <dc0ae263-e5f6-b876-842a-7b598caf68fa@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 7/25/2023 3:51 AM, Wen Gong wrote:
> On 6/15/2023 7:49 PM, Kalle Valo wrote:
>> Aloka Dixit <quic_alokad@quicinc.com> writes:
>>
> [...]
>> [...]
>>
>>> +struct ath12k_fw_pdev {
>>> +    u32 pdev_id;
>>> +    u32 phy_id;
>>> +    u32 supported_bands;
>>> +};
>> So we have now two very similar structures, ath12k_pdev and
>> ath12k_fw_pdev. It would be good to document above the structs their
>> purpose. Any ideas what I could add?
> Currently ath12k has single_pdev parameter, it impact the interface 
> number of wifi.
> 
> For single_pdev=ture, ab->num_radios is set to 1 and all capbility of 
> 2.4 GHz/
> 5 GHz/6 GHz is saved into ab->pdevs[0] in 
> ath12k_wmi_ext_soc_hal_reg_caps_parse().
> ath12k_mac_register() will call only one time into 
> __ath12k_mac_register(), and
> ieee80211_register_hw() is called only one time, and only one wifi 
> interface in
> the result of "iw dev/ifconfig", the interface support 2.4 GHz/5 GHz/6 
> GHz. Actually
> there hare more than one PHY in firmware, firmware maintain the all PHY 
> data and
> provide a single pdev/soc interface to ath12k, then ath12k do not need 
> to consider
> the PHY info such as channel/BAND<->PHY mapping relationship, it is 
> convenient for
> ath12k to support 2.4 GHz/5 GHz/6 GHz.
> 
> For single_pdev=false, ab->num_radios is more than 1, ab->pdevs[] has 
> muti valid
> element, each element only support one BAND. Firmware do not provide a 
> single
> pdev/soc interface, so ath12k need to maintain the PHY data such as 
> channel/BAND<->
> PHY mapping relationship, and ath12k_mac_register() will call muti-time 
> into
> __ath12k_mac_register(), and ieee80211_register_hw() is called 
> muti-time, and muti
> wifi interface in the result of "iw dev/ifconfig", and each interface 
> only support
> one BAND, for example, wlan0 only support 2.4 GHz, wlan1 only support 5 
> GHz, and
> wlan2 only support 6 GHz.
> 
> For single_pdev=ture, ath12k also need to know some info about the PHYs 
> sush as
> pdev_id/phy_id/supported_bands in this patch, so ab->fw_pdev is used to 
> store the
> PHY data.


The new struct ath12k_fw_pdev is anyway going to have as many elements 
as the num_radio, right? Then there must be a way to reuse the common 
struct ath12k_pdev instead as it done for QCN devices. I understand this 
it will need changes to all the existing code for the single-pdev where 
it might have hard-coded pdev[0] for the original structure. Something 
to think about in a separate patch-set.

For now I will send new version with comments (as far as I understood 
it) because without this patch WCN devices show a crash during boot-up.

Thanks.