| 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: <20250130-offbeat-sparkling-nyala-042b72@krzk-bin>
Date: Thu, 30 Jan 2025 09:39:22 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Raj Kumar Bhagat <quic_rajkbhag@...cinc.com>
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 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?
> -
> 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.
Best regards,
Krzysztof
Powered by blists - more mailing lists