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@xxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxx>
>> Co-developed-by: Saravanakumar Duraisamy <quic_saradura@xxxxxxxxxxx>
>> Signed-off-by: Saravanakumar Duraisamy <quic_saradura@xxxxxxxxxxx>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@xxxxxxxxxxx>
>> ---
>> 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.
