| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <cedf59c5-dd2a-4d1e-960e-d76e45d9c215@quicinc.com>
Date: Wed, 26 Feb 2025 22:53:13 +0530
From: Raj Kumar Bhagat <quic_rajkbhag@...cinc.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
CC: <ath12k@...ts.infradead.org>, <linux-wireless@...r.kernel.org>,
Kalle Valo
<kvalo@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski
<krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Jeff Johnson
<jjohnson@...nel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
Sowmiya Sree Elavalagan
<quic_ssreeela@...cinc.com>,
Saravanakumar Duraisamy
<quic_saradura@...cinc.com>
Subject: Re: [RFC PATCH 5/5] wifi: ath12k: Enable IPQ5424 WiFi device support
On 1/30/2025 2:09 PM, Krzysztof Kozlowski wrote:
> On Thu, Jan 30, 2025 at 10:48:38AM +0530, Raj Kumar Bhagat wrote:
>> From: Sowmiya Sree Elavalagan <quic_ssreeela@...cinc.com>
>>
>> Currently, ath12k AHB (in IPQ5332) uses SCM calls to authenticate the
>> firmware image to bring up userpd. From IPQ5424 onwards, Q6 firmware can
>> directly communicate with the Trusted Management Engine - Lite (TME-L),
>> eliminating the need for SCM calls for userpd bring-up.
>>
>> Hence, to enable IPQ5424 device support, use qcom_mdt_load_no_init() and
>> skip the SCM call as Q6 will directly authenticate the userpd firmware.
>>
>> Tested-on: IPQ5424 hw1.0 AHB WLAN.WBE.1.5-01053-QCAHKSWPL_SILICONZ-1
>> Tested-on: IPQ5332 hw1.0 AHB WLAN.WBE.1.3.1-00130-QCAHKSWPL_SILICONZ-1
>>
>> Signed-off-by: Sowmiya Sree Elavalagan <quic_ssreeela@...cinc.com>
>> Co-developed-by: Saravanakumar Duraisamy <quic_saradura@...cinc.com>
>> Signed-off-by: Saravanakumar Duraisamy <quic_saradura@...cinc.com>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@...cinc.com>
>> ---
>> drivers/net/wireless/ath/ath12k/ahb.c | 80 +++++++++++++++++----------
>> drivers/net/wireless/ath/ath12k/ahb.h | 9 +++
>> 2 files changed, 61 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/ahb.c b/drivers/net/wireless/ath/ath12k/ahb.c
>> index d502b59a78d8..75767915dec3 100644
>> --- a/drivers/net/wireless/ath/ath12k/ahb.c
>> +++ b/drivers/net/wireless/ath/ath12k/ahb.c
>> @@ -21,6 +21,9 @@ static const struct of_device_id ath12k_ahb_of_match[] = {
>> { .compatible = "qcom,ipq5332-wifi",
>> .data = (void *)ATH12K_HW_IPQ5332_HW10,
>> },
>> + { .compatible = "qcom,ipq5424-wifi",
>> + .data = (void *)ATH12K_HW_IPQ5424_HW10,
>> + },
>> { }
>> };
>>
>> @@ -398,8 +401,8 @@ static int ath12k_ahb_power_up(struct ath12k_base *ab)
>> ATH12K_AHB_UPD_SWID;
>>
>> /* Load FW image to a reserved memory location */
>> - ret = qcom_mdt_load(dev, fw, fw_name, pasid, mem_region, mem_phys, mem_size,
>> - &mem_phys);
>> + ret = ab_ahb->ahb_ops->mdt_load(dev, fw, fw_name, pasid, mem_region, mem_phys,
>> + mem_size, &mem_phys);
>> if (ret) {
>> ath12k_err(ab, "Failed to load MDT segments: %d\n", ret);
>> goto err_fw;
>> @@ -430,11 +433,13 @@ static int ath12k_ahb_power_up(struct ath12k_base *ab)
>> goto err_fw2;
>> }
>>
>> - /* Authenticate FW image using peripheral ID */
>> - ret = qcom_scm_pas_auth_and_reset(pasid);
>> - if (ret) {
>> - ath12k_err(ab, "failed to boot the remote processor %d\n", ret);
>> - goto err_fw2;
>> + if (ab_ahb->scm_auth_enabled) {
>> + /* Authenticate FW image using peripheral ID */
>> + ret = qcom_scm_pas_auth_and_reset(pasid);
>> + if (ret) {
>> + ath12k_err(ab, "failed to boot the remote processor %d\n", ret);
>> + goto err_fw2;
>> + }
>> }
>>
>> /* Instruct Q6 to spawn userPD thread */
>> @@ -491,13 +496,15 @@ static void ath12k_ahb_power_down(struct ath12k_base *ab, bool is_suspend)
>>
>> qcom_smem_state_update_bits(ab_ahb->stop_state, BIT(ab_ahb->stop_bit), 0);
>>
>> - pasid = (u32_encode_bits(ab_ahb->userpd_id, ATH12K_USERPD_ID_MASK)) |
>> - ATH12K_AHB_UPD_SWID;
>> - /* Release the firmware */
>> - ret = qcom_scm_pas_shutdown(pasid);
>> - if (ret)
>> - ath12k_err(ab, "scm pas shutdown failed for userPD%d: %d\n",
>> - ab_ahb->userpd_id, ret);
>> + if (ab_ahb->scm_auth_enabled) {
>> + pasid = (u32_encode_bits(ab_ahb->userpd_id, ATH12K_USERPD_ID_MASK)) |
>> + ATH12K_AHB_UPD_SWID;
>> + /* Release the firmware */
>> + ret = qcom_scm_pas_shutdown(pasid);
>> + if (ret)
>> + ath12k_err(ab, "scm pas shutdown failed for userPD%d\n",
>> + ab_ahb->userpd_id);
>> + }
>> }
>>
>> static void ath12k_ahb_init_qmi_ce_config(struct ath12k_base *ab)
>> @@ -707,6 +714,14 @@ static int ath12k_ahb_map_service_to_pipe(struct ath12k_base *ab, u16 service_id
>> return 0;
>> }
>>
>> +static const struct ath12k_ahb_ops ahb_ops_ipq5332 = {
>> + .mdt_load = qcom_mdt_load,
>> +};
>> +
>> +static const struct ath12k_ahb_ops ahb_ops_ipq5424 = {
>> + .mdt_load = qcom_mdt_load_no_init,
>> +};
>> +
>> static const struct ath12k_hif_ops ath12k_ahb_hif_ops_ipq5332 = {
>> .start = ath12k_ahb_start,
>> .stop = ath12k_ahb_stop,
>> @@ -1041,19 +1056,9 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
>> struct device_node *mem_node;
>> struct ath12k_ahb *ab_ahb;
>> enum ath12k_hw_rev hw_rev;
>> - u32 addr, userpd_id;
>> + u32 addr;
>> int ret;
>>
>> - hw_rev = ath12k_ahb_get_hw_rev(pdev);
>> - switch (hw_rev) {
>> - case ATH12K_HW_IPQ5332_HW10:
>> - hif_ops = &ath12k_ahb_hif_ops_ipq5332;
>> - userpd_id = ATH12K_IPQ5332_USERPD_ID;
>> - break;
>> - default:
>> - return -EOPNOTSUPP;
>> - }
>
> You just added this code in previous patchset, why are you moving it?
>
In the v6 of dependent series ([PATCH v6 00/13] wifi: ath12k: add Ath12k AHB driver
support for IPQ5332), we have moved this part of the code in the required position.
In the next version the above hunk will not be required.
>> -
>> ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>> if (ret) {
>> dev_err(&pdev->dev, "Failed to set 32-bit coherent dma\n");
>> @@ -1067,13 +1072,32 @@ static int ath12k_ahb_probe(struct platform_device *pdev)
>> return -ENOMEM;
>> }
>>
>> + ab_ahb = ath12k_ab_to_ahb(ab);
>> + ab_ahb->ab = ab;
>> +
>> + hw_rev = ath12k_ahb_get_hw_rev(pdev);
>> + switch (hw_rev) {
>> + case ATH12K_HW_IPQ5332_HW10:
>> + hif_ops = &ath12k_ahb_hif_ops_ipq5332;
>> + ab_ahb->userpd_id = ATH12K_IPQ5332_USERPD_ID;
>> + ab_ahb->scm_auth_enabled = true;
>> + ab_ahb->ahb_ops = &ahb_ops_ipq5332;
>> + break;
>> + case ATH12K_HW_IPQ5424_HW10:
>> + hif_ops = &ath12k_ahb_hif_ops_ipq5332;
>> + ab_ahb->userpd_id = ATH12K_IPQ5332_USERPD_ID;
>> + ab_ahb->scm_auth_enabled = false;
>> + ab_ahb->ahb_ops = &ahb_ops_ipq5424;
>
> Why you cannot store just proper driver data structure in match data?
> This entire switch is redundant.
>
Thanks for this optimization suggestion. Will implement this in the next version.
Powered by blists - more mailing lists