| Message ID | 20250128091012.2574478-3-quic_rajkbhag@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: net: wireless: add wifi node for IPQ5332 | expand |
On 28/01/2025 10:10, Raj Kumar Bhagat wrote: > + /* Default Profile > + * +============+==============+=====================+ > + * | | | | > + * | Region | Start Offset | Size | > + * | | | | > + * +------------+--------------+---------------------+ > + * | | | | > + * | | | | > + * | | | | > + * | WLAN Q6 | 0x4A900000 | 43MB | > + * | | | | > + * | | | | > + * +------------+--------------+---------------------+ > + * | M3 Dump | 0x4D400000 | 1MB | > + * +============+==============+=====================+ > + * | | > + * | | > + * | | > + * | Rest of memory for Linux | > + * | | > + * | | > + * | | > + * +=================================================+ > + */ > + > + reserved-memory { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + q6_region: wcss@4a900000 { > + reg = <0x0 0x4a900000 0x0 0x02b00000>; > + no-map; > + }; > + > + m3_dump: m3-dump@4d400000 { This fails with your wireless patchset. > + reg = <0x0 0x4D400000 0x0 0x100000>; > + no-map; > + }; > + }; > }; > > &blsp1_i2c1 { > @@ -63,3 +104,16 @@ data-pins { > }; > }; > }; > + > +&wifi0 { > + memory-region = <&q6_region>, <&m3_dump>; > + memory-region-names = "q6-region", "m3-dump"; Binding said you have four items. I don't understand why this varies and why this is 2 items instead. Best regards, Krzysztof
On 1/28/2025 2:52 PM, Krzysztof Kozlowski wrote: > On 28/01/2025 10:10, Raj Kumar Bhagat wrote: >> + /* Default Profile >> + * +============+==============+=====================+ >> + * | | | | >> + * | Region | Start Offset | Size | >> + * | | | | >> + * +------------+--------------+---------------------+ >> + * | | | | >> + * | | | | >> + * | | | | >> + * | WLAN Q6 | 0x4A900000 | 43MB | >> + * | | | | >> + * | | | | >> + * +------------+--------------+---------------------+ >> + * | M3 Dump | 0x4D400000 | 1MB | >> + * +============+==============+=====================+ >> + * | | >> + * | | >> + * | | >> + * | Rest of memory for Linux | >> + * | | >> + * | | >> + * | | >> + * +=================================================+ >> + */ >> + >> + reserved-memory { >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + >> + q6_region: wcss@4a900000 { >> + reg = <0x0 0x4a900000 0x0 0x02b00000>; >> + no-map; >> + }; >> + >> + m3_dump: m3-dump@4d400000 { > > This fails with your wireless patchset. > Yes, this will fail with v4 wireless patch. We have v5 (yet to be sent) to read the correct reserved memory. Since, in v4 I was asked to address and send DTS patch separately, The corresponding driver patches are yet to be sent. >> + reg = <0x0 0x4D400000 0x0 0x100000>; >> + no-map; >> + }; >> + }; >> }; >> >> &blsp1_i2c1 { >> @@ -63,3 +104,16 @@ data-pins { >> }; >> }; >> }; >> + >> +&wifi0 { >> + memory-region = <&q6_region>, <&m3_dump>; >> + memory-region-names = "q6-region", "m3-dump"; > > Binding said you have four items. I don't understand why this varies and > why this is 2 items instead. > In total we have 4 items, with minItems as 2. The other 2 items (q6-caldb & mlo-global-mem) functionalities are not yet enabled in ath12k driver. Hence these memory are not reserved yet.
On 28/01/2025 10:41, Raj Kumar Bhagat wrote: >>> + >>> + reserved-memory { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + ranges; >>> + >>> + q6_region: wcss@4a900000 { >>> + reg = <0x0 0x4a900000 0x0 0x02b00000>; >>> + no-map; >>> + }; >>> + >>> + m3_dump: m3-dump@4d400000 { >> >> This fails with your wireless patchset. >> > > Yes, this will fail with v4 wireless patch. > We have v5 (yet to be sent) to read the correct reserved memory. Since, > in v4 I was asked to address and send DTS patch separately, The corresponding > driver patches are yet to be sent. So you send it to prove my point? OK, fine. So this patch confirms your driver changes should be rejected. > >>> + reg = <0x0 0x4D400000 0x0 0x100000>; >>> + no-map; >>> + }; >>> + }; >>> }; >>> >>> &blsp1_i2c1 { >>> @@ -63,3 +104,16 @@ data-pins { >>> }; >>> }; >>> }; >>> + >>> +&wifi0 { >>> + memory-region = <&q6_region>, <&m3_dump>; >>> + memory-region-names = "q6-region", "m3-dump"; >> >> Binding said you have four items. I don't understand why this varies and >> why this is 2 items instead. >> > > In total we have 4 items, with minItems as 2. > The other 2 items (q6-caldb & mlo-global-mem) functionalities are not yet enabled > in ath12k driver. Hence these memory are not reserved yet. That's not a valid reason to skip valid items. DTS is not dependent on driver. Submit proper hardware description, not something carved to match your driver. Best regards, Krzysztof
On 28.01.2025 10:41 AM, Raj Kumar Bhagat wrote: > On 1/28/2025 2:52 PM, Krzysztof Kozlowski wrote: >> On 28/01/2025 10:10, Raj Kumar Bhagat wrote: >>> + /* Default Profile >>> + * +============+==============+=====================+ >>> + * | | | | >>> + * | Region | Start Offset | Size | >>> + * | | | | >>> + * +------------+--------------+---------------------+ >>> + * | | | | >>> + * | | | | >>> + * | | | | >>> + * | WLAN Q6 | 0x4A900000 | 43MB | >>> + * | | | | >>> + * | | | | >>> + * +------------+--------------+---------------------+ >>> + * | M3 Dump | 0x4D400000 | 1MB | >>> + * +============+==============+=====================+ >>> + * | | >>> + * | | >>> + * | | >>> + * | Rest of memory for Linux | >>> + * | | >>> + * | | >>> + * | | >>> + * +=================================================+ >>> + */ >>> + >>> + reserved-memory { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + ranges; >>> + >>> + q6_region: wcss@4a900000 { >>> + reg = <0x0 0x4a900000 0x0 0x02b00000>; >>> + no-map; >>> + }; >>> + >>> + m3_dump: m3-dump@4d400000 { >> >> This fails with your wireless patchset. >> > > Yes, this will fail with v4 wireless patch. > We have v5 (yet to be sent) to read the correct reserved memory. Since, > in v4 I was asked to address and send DTS patch separately, The corresponding > driver patches are yet to be sent. I think the intention was to send them separately, but together time-wise, so that the large set of net changes is somewhat shrunk, but the reviewers can cross-reference both series if/as needed > >>> + reg = <0x0 0x4D400000 0x0 0x100000>; Please use lowercase hex Konrad
On 1/28/2025 3:54 PM, Krzysztof Kozlowski wrote: > On 28/01/2025 10:41, Raj Kumar Bhagat wrote: >>>> + >>>> + reserved-memory { >>>> + #address-cells = <2>; >>>> + #size-cells = <2>; >>>> + ranges; >>>> + >>>> + q6_region: wcss@4a900000 { >>>> + reg = <0x0 0x4a900000 0x0 0x02b00000>; >>>> + no-map; >>>> + }; >>>> + >>>> + m3_dump: m3-dump@4d400000 { >>> >>> This fails with your wireless patchset. >>> >> >> Yes, this will fail with v4 wireless patch. >> We have v5 (yet to be sent) to read the correct reserved memory. Since, >> in v4 I was asked to address and send DTS patch separately, The corresponding >> driver patches are yet to be sent. > > > So you send it to prove my point? OK, fine. So this patch confirms your > driver changes should be rejected. > There seems to be some confusion here. Based on the comments in v4, as quoted below: "Please post the updated DTS in a separate series after fixing all the issues pointed out by the DTS coding style." I understood that you wanted to review only the DT-related patches first before the driver implementation. Hence, this is posted separately before the v5 driver is posted. If I misunderstood, please let me know. We will continue with v5 of the original series 'wifi: ath12k: add Ath12k AHB driver support for IPQ5332'. The v5 will include dt-binding, driver, and reference DTS as well. >> >>>> + reg = <0x0 0x4D400000 0x0 0x100000>; >>>> + no-map; >>>> + }; >>>> + }; >>>> }; >>>> >>>> &blsp1_i2c1 { >>>> @@ -63,3 +104,16 @@ data-pins { >>>> }; >>>> }; >>>> }; >>>> + >>>> +&wifi0 { >>>> + memory-region = <&q6_region>, <&m3_dump>; >>>> + memory-region-names = "q6-region", "m3-dump"; >>> >>> Binding said you have four items. I don't understand why this varies and >>> why this is 2 items instead. >>> >> >> In total we have 4 items, with minItems as 2. >> The other 2 items (q6-caldb & mlo-global-mem) functionalities are not yet enabled >> in ath12k driver. Hence these memory are not reserved yet. > > That's not a valid reason to skip valid items. DTS is not dependent on > driver. Submit proper hardware description, not something carved to > match your driver. > Sure, we can have all four reserved memory sections in the DTS. However, ath12k will not use all four reserved memory sections as of now. Once the functionality corresponding to q6-caldb and mlo-global-mem is enabled, these reserved memory sections will be used. In that case, we can also drop 'minItems' to 2 in the binding?
On 1/28/2025 5:02 PM, Konrad Dybcio wrote: > On 28.01.2025 10:41 AM, Raj Kumar Bhagat wrote: >> On 1/28/2025 2:52 PM, Krzysztof Kozlowski wrote: >>> On 28/01/2025 10:10, Raj Kumar Bhagat wrote: >>>> + /* Default Profile >>>> + * +============+==============+=====================+ >>>> + * | | | | >>>> + * | Region | Start Offset | Size | >>>> + * | | | | >>>> + * +------------+--------------+---------------------+ >>>> + * | | | | >>>> + * | | | | >>>> + * | | | | >>>> + * | WLAN Q6 | 0x4A900000 | 43MB | >>>> + * | | | | >>>> + * | | | | >>>> + * +------------+--------------+---------------------+ >>>> + * | M3 Dump | 0x4D400000 | 1MB | >>>> + * +============+==============+=====================+ >>>> + * | | >>>> + * | | >>>> + * | | >>>> + * | Rest of memory for Linux | >>>> + * | | >>>> + * | | >>>> + * | | >>>> + * +=================================================+ >>>> + */ >>>> + >>>> + reserved-memory { >>>> + #address-cells = <2>; >>>> + #size-cells = <2>; >>>> + ranges; >>>> + >>>> + q6_region: wcss@4a900000 { >>>> + reg = <0x0 0x4a900000 0x0 0x02b00000>; >>>> + no-map; >>>> + }; >>>> + >>>> + m3_dump: m3-dump@4d400000 { >>> >>> This fails with your wireless patchset. >>> >> >> Yes, this will fail with v4 wireless patch. >> We have v5 (yet to be sent) to read the correct reserved memory. Since, >> in v4 I was asked to address and send DTS patch separately, The corresponding >> driver patches are yet to be sent. > > I think the intention was to send them separately, but together time-wise, so > that the large set of net changes is somewhat shrunk, but the reviewers can > cross-reference both series if/as needed > Yes, the intention is similar: to get DT related reviews before posting the next version (v5) of the driver changes. >> >>>> + reg = <0x0 0x4D400000 0x0 0x100000>; > > Please use lowercase hex > Thanks, will do in next version.
diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts index 846413817e9a..07019e224492 100644 --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts @@ -2,7 +2,7 @@ /* * IPQ5332 AP-MI01.2 board device tree source * - * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022-2025 Qualcomm Innovation Center, Inc. All rights reserved. */ /dts-v1/; @@ -12,6 +12,47 @@ / { model = "Qualcomm Technologies, Inc. IPQ5332 MI01.2"; compatible = "qcom,ipq5332-ap-mi01.2", "qcom,ipq5332"; + + /* Default Profile + * +============+==============+=====================+ + * | | | | + * | Region | Start Offset | Size | + * | | | | + * +------------+--------------+---------------------+ + * | | | | + * | | | | + * | | | | + * | WLAN Q6 | 0x4A900000 | 43MB | + * | | | | + * | | | | + * +------------+--------------+---------------------+ + * | M3 Dump | 0x4D400000 | 1MB | + * +============+==============+=====================+ + * | | + * | | + * | | + * | Rest of memory for Linux | + * | | + * | | + * | | + * +=================================================+ + */ + + reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + + q6_region: wcss@4a900000 { + reg = <0x0 0x4a900000 0x0 0x02b00000>; + no-map; + }; + + m3_dump: m3-dump@4d400000 { + reg = <0x0 0x4D400000 0x0 0x100000>; + no-map; + }; + }; }; &blsp1_i2c1 { @@ -63,3 +104,16 @@ data-pins { }; }; }; + +&wifi0 { + memory-region = <&q6_region>, <&m3_dump>; + memory-region-names = "q6-region", "m3-dump"; + 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"; + status = "okay"; +}; diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi index 85e10b20342a..a27a679d4348 100644 --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi @@ -485,6 +485,112 @@ frame@b128000 { }; }; + wifi0: wifi@c000000 { + compatible = "qcom,ipq5332-wifi"; + reg = <0x0c000000 0x1000000>; + clocks = <&gcc GCC_XO_CLK>; + clock-names = "xo"; + interrupts-extended = <&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 422 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 423 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 424 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 425 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 426 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 427 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 428 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 429 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 430 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 431 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 432 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 433 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 491 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 495 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 493 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 544 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 457 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 466 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 497 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 454 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 453 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 452 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 451 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 488 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 488 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 484 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 554 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 554 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 549 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 507 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 500 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 499 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 498 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 450 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 449 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 448 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 447 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 543 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 486 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 486 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 482 IRQ_TYPE_EDGE_RISING>, + <&intc GIC_SPI 419 IRQ_TYPE_EDGE_RISING>; + + interrupt-names = "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"; + + status = "disabled"; + }; + q6v5_wcss: remoteproc@d100000 { compatible = "qcom,ipq5332-wcss-sec-pil"; reg = <0xd100000 0x4040>;
RDP441 is based on IPQ5332. It has inbuilt AHB bus based IPQ5332 WiFi device. Describe and add WiFi node for RDP441. Also, reserve the memory required by IPQ5332 firmware. Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com> --- arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts | 56 ++++++++++- arch/arm64/boot/dts/qcom/ipq5332.dtsi | 106 ++++++++++++++++++++ 2 files changed, 161 insertions(+), 1 deletion(-)