| Message ID | 20250130051838.1924079-1-quic_rajkbhag@quicinc.com |
|---|---|
| Headers | show |
| Series | wifi: ath12k: Enable IPQ5424 AHB WiFi device | expand |
On 30/01/2025 06:18, Raj Kumar Bhagat wrote: > Add support for the new ath12k AHB device IPQ5424, as currently, Ath12k > AHB only supports IPQ5332 WiFi devices. > > The IPQ5424 is an IEEE 802.11be 2 GHz WiFi device, supporting 4x4 > configurations. To enable the IPQ5424 device: > - Add the necessary hardware parameters for IPQ5424. > - Modify the boot-up sequence for ath12k AHB to accommodate the > requirements of the IPQ5424 device. > > Depends-On: [PATCH v5 00/13] wifi: ath12k: add Ath12k AHB driver support for IPQ5332 > Link: https://lore.kernel.org/all/20250130043508.1885026-1-quic_rajkbhag@quicinc.com/ > You marked this a RFC, so this is not ready, but nothing explains this here. What sort of comments are you looking for? What's incomplete with this work? Best regards, Krzysztof
On Thu, Jan 30, 2025 at 10:48:38AM +0530, Raj Kumar Bhagat wrote: > From: Sowmiya Sree Elavalagan <quic_ssreeela@quicinc.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@quicinc.com> > Co-developed-by: Saravanakumar Duraisamy <quic_saradura@quicinc.com> > Signed-off-by: Saravanakumar Duraisamy <quic_saradura@quicinc.com> > Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.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
On Thu, Jan 30, 2025 at 10:48:34AM +0530, Raj Kumar Bhagat wrote: > Describe and add the device-tree bindings for the ATH12K AHB wifi device > IPQ5424. > > Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com> > --- > .../net/wireless/qcom,ath12k-ahb.yaml | 119 ++++++++++++++++++ > 1 file changed, 119 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k-ahb.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k-ahb.yaml > index bd953a028dc3..1d24389a0ab3 100644 > --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k-ahb.yaml > +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k-ahb.yaml > @@ -18,6 +18,7 @@ properties: > compatible: > enum: > - qcom,ipq5332-wifi > + - qcom,ipq5424-wifi Patch should end here, another example for something that doesn't have different properties etc is pointless. Thanks, Conor. > > reg: > maxItems: 1 > @@ -317,3 +318,121 @@ examples: > "stop", > "spawn"; > }; > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + wifi1: wifi@c000000 { > + compatible = "qcom,ipq5424-wifi"; > + reg = <0x0c000000 0x1000000>; > + clocks = <&gcc GCC_XO_CLK_SRC>; > + clock-names = "xo"; > + interrupts-extended = <&wcss_smp2p_in 8 IRQ_TYPE_NONE>, > + <&wcss_smp2p_in 9 IRQ_TYPE_NONE>, > + <&wcss_smp2p_in 12 IRQ_TYPE_NONE>, > + <&wcss_smp2p_in 11 IRQ_TYPE_NONE>, > + <&intc GIC_SPI 559 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 560 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 561 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 518 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 519 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 520 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 521 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 522 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 523 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 524 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 525 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 526 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 527 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 528 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 529 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 574 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 578 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 576 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 627 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 540 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 549 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 580 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 537 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 536 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 535 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 534 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 571 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 571 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 567 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 637 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 637 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 632 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 590 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 583 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 582 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 581 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 533 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 532 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 531 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 530 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 626 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 569 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 569 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 565 IRQ_TYPE_EDGE_RISING>, > + <&intc GIC_SPI 502 IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "fatal", > + "ready", > + "spawn", > + "stop-ack", > + "misc-pulse1", > + "misc-latch", > + "sw-exception", > + "ce0", > + "ce1", > + "ce2", > + "ce3", > + "ce4", > + "ce5", > + "ce6", > + "ce7", > + "ce8", > + "ce9", > + "ce10", > + "ce11", > + "host2wbm-desc-feed", > + "host2reo-re-injection", > + "host2reo-command", > + "host2rxdma-monitor-ring1", > + "reo2ost-exception", > + "wbm2host-rx-release", > + "reo2host-status", > + "reo2host-destination-ring4", > + "reo2host-destination-ring3", > + "reo2host-destination-ring2", > + "reo2host-destination-ring1", > + "rxdma2host-monitor-destination-mac3", > + "rxdma2host-monitor-destination-mac2", > + "rxdma2host-monitor-destination-mac1", > + "host2rxdma-host-buf-ring-mac3", > + "host2rxdma-host-buf-ring-mac2", > + "host2rxdma-host-buf-ring-mac1", > + "host2tcl-input-ring4", > + "host2tcl-input-ring3", > + "host2tcl-input-ring2", > + "host2tcl-input-ring1", > + "wbm2host-tx-completions-ring4", > + "wbm2host-tx-completions-ring3", > + "wbm2host-tx-completions-ring2", > + "wbm2host-tx-completions-ring1", > + "host2tx-monitor-ring1", > + "txmon2host-monitor-destination-mac3", > + "txmon2host-monitor-destination-mac2", > + "txmon2host-monitor-destination-mac1", > + "umac-reset"; > + > + memory-region = <&q6_region>, <&m3_dump>, <&q6_caldb>, <&mlo_mem>; > + memory-region-names = "q6-region", "m3-dump", "q6-caldb", "mlo-global-mem"; > + qcom,ath12k-calibration-variant = "RDP466_1"; > + qcom,rproc = <&q6v5_wcss>; > + qcom,smem-states = <&wcss_smp2p_out 8>, > + <&wcss_smp2p_out 9>, > + <&wcss_smp2p_out 10>; > + qcom,smem-state-names = "shutdown", > + "stop", > + "spawn"; > + }; > -- > 2.34.1 >
On 1/30/2025 12:38 AM, Krzysztof Kozlowski wrote: > On Thu, Jan 30, 2025 at 10:48:36AM +0530, Raj Kumar Bhagat wrote: >> From: Saravanakumar Duraisamy <quic_saradura@quicinc.com> >> >> Add register addresses (ath12k_hw_regs) for new ath12k AHB based >> WiFi device IPQ5424. >> >> 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: Saravanakumar Duraisamy <quic_saradura@quicinc.com> >> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com> >> --- >> drivers/net/wireless/ath/ath12k/hal.h | 3 + >> drivers/net/wireless/ath/ath12k/hw.c | 87 ++++++++++++++++++++++++++- >> 2 files changed, 89 insertions(+), 1 deletion(-) >> > > That's not a separate patch. Regs do not make any sense on their own, > so this commit makes no sense on its own. This approach was suggested during internal review so that each addition can be reviewed without distraction from the other pieces. None of this is actually enabled until the final patch, so "trickling in" the pieces seems preferable to me. /jeff