* [PATCH 0/2] Enable PCIe controllers present in Qualcomm ipq5210
@ 2026-05-14 4:13 Varadarajan Narayanan
2026-05-14 4:13 ` [PATCH 1/2] dt-bindings: PCI: qcom,pcie-ipq9574: Document the ipq5210 pcie controller Varadarajan Narayanan
2026-05-14 4:13 ` [PATCH 2/2] arm64: dts: qcom: ipq5210: Enable PCIe support Varadarajan Narayanan
0 siblings, 2 replies; 5+ messages in thread
From: Varadarajan Narayanan @ 2026-05-14 4:13 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-arm-msm, linux-pci, devicetree, linux-kernel,
Varadarajan Narayanan
Update the relevant bindings and DT files to enable support
for the PCIe phy and controller.
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
Varadarajan Narayanan (2):
dt-bindings: PCI: qcom,pcie-ipq9574: Document the ipq5210 pcie controller
arm64: dts: qcom: ipq5210: Enable PCIe support
.../devicetree/bindings/pci/qcom,pcie-ipq9574.yaml | 1 +
arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts | 43 ++++
arch/arm64/boot/dts/qcom/ipq5210.dtsi | 261 ++++++++++++++++++++-
3 files changed, 303 insertions(+), 2 deletions(-)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260514-pci-ipq5210-977672bcad62
prerequisite-change-id: 20260514-pci-phy-38ec9b1c5a90:v1
prerequisite-patch-id: aea0d856b804b879fd5f24ea02f809b4dec84a30
prerequisite-patch-id: 26fb9a62b5ba0fc6275c0b081c7a16348d5a4fd1
prerequisite-change-id: 20260514-icc-ipq5210-0ab03f3a3e83:v1
prerequisite-patch-id: 0b6145b6635b18fe79fbbff5815041b43778c5ed
prerequisite-patch-id: 924c6ff7baf4283ac7991ee94c803a00fc5cece4
prerequisite-patch-id: c2fe1800fe769dccd37f94c19860a07f979e3c4c
Best regards,
--
Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] dt-bindings: PCI: qcom,pcie-ipq9574: Document the ipq5210 pcie controller 2026-05-14 4:13 [PATCH 0/2] Enable PCIe controllers present in Qualcomm ipq5210 Varadarajan Narayanan @ 2026-05-14 4:13 ` Varadarajan Narayanan 2026-05-15 10:55 ` Krzysztof Kozlowski 2026-05-14 4:13 ` [PATCH 2/2] arm64: dts: qcom: ipq5210: Enable PCIe support Varadarajan Narayanan 1 sibling, 1 reply; 5+ messages in thread From: Varadarajan Narayanan @ 2026-05-14 4:13 UTC (permalink / raw) To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio Cc: linux-arm-msm, linux-pci, devicetree, linux-kernel, Varadarajan Narayanan Document the ipq5210 PCIe controller using ipq9574 as fallback compatible. Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com> --- Documentation/devicetree/bindings/pci/qcom,pcie-ipq9574.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ipq9574.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ipq9574.yaml index 4be342cc04e1..a75ff554c283 100644 --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ipq9574.yaml +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ipq9574.yaml @@ -17,6 +17,7 @@ properties: - qcom,pcie-ipq9574 - items: - enum: + - qcom,pcie-ipq5210 - qcom,pcie-ipq5332 - qcom,pcie-ipq5424 - const: qcom,pcie-ipq9574 -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] dt-bindings: PCI: qcom,pcie-ipq9574: Document the ipq5210 pcie controller 2026-05-14 4:13 ` [PATCH 1/2] dt-bindings: PCI: qcom,pcie-ipq9574: Document the ipq5210 pcie controller Varadarajan Narayanan @ 2026-05-15 10:55 ` Krzysztof Kozlowski 0 siblings, 0 replies; 5+ messages in thread From: Krzysztof Kozlowski @ 2026-05-15 10:55 UTC (permalink / raw) To: Varadarajan Narayanan Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio, linux-arm-msm, linux-pci, devicetree, linux-kernel On Thu, May 14, 2026 at 09:43:01AM +0530, Varadarajan Narayanan wrote: > Document the ipq5210 PCIe controller using ipq9574 as fallback compatible. > > Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com> > --- > Documentation/devicetree/bindings/pci/qcom,pcie-ipq9574.yaml | 1 + > 1 file changed, 1 insertion(+) Please have your patches reviewed internally, including quite useful tools. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] arm64: dts: qcom: ipq5210: Enable PCIe support 2026-05-14 4:13 [PATCH 0/2] Enable PCIe controllers present in Qualcomm ipq5210 Varadarajan Narayanan 2026-05-14 4:13 ` [PATCH 1/2] dt-bindings: PCI: qcom,pcie-ipq9574: Document the ipq5210 pcie controller Varadarajan Narayanan @ 2026-05-14 4:13 ` Varadarajan Narayanan 2026-05-14 12:33 ` sashiko-bot 1 sibling, 1 reply; 5+ messages in thread From: Varadarajan Narayanan @ 2026-05-14 4:13 UTC (permalink / raw) To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio Cc: linux-arm-msm, linux-pci, devicetree, linux-kernel, Varadarajan Narayanan Add DT entries to enable the PCIe controllers found in ipq5210. Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com> --- arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts | 43 +++++ arch/arm64/boot/dts/qcom/ipq5210.dtsi | 261 +++++++++++++++++++++++++++- 2 files changed, 302 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts b/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts index 941f866ecfe9..5e599a1cea3f 100644 --- a/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts +++ b/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts @@ -5,6 +5,7 @@ /dts-v1/; +#include <dt-bindings/gpio/gpio.h> #include "ipq5210.dtsi" / { @@ -20,6 +21,32 @@ chosen { }; }; +&pcie0_phy { + status = "okay"; +}; + +&pcie0_rp { + reset-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>; +}; + +&pcie0 { + pinctrl-0 = <&pcie0_default_state>; + status = "okay"; +}; + +&pcie1_phy { + status = "okay"; +}; + +&pcie1_rp { + reset-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>; +}; + +&pcie1 { + pinctrl-0 = <&pcie1_default_state>; + status = "okay"; +}; + &sdhc { max-frequency = <192000000>; bus-width = <4>; @@ -36,6 +63,22 @@ &sleep_clk { }; &tlmm { + pcie0_default_state: pcie0-default-state { + pins = "gpio32"; + function = "gpio"; + drive-strength = <6>; + bias-pull-down; + output-low; + }; + + pcie1_default_state: pcie1-default-state { + pins = "gpio29"; + function = "gpio"; + drive-strength = <6>; + bias-pull-down; + output-low; + }; + qup_uart1_default_state: qup-uart1-default-state { pins = "gpio38", "gpio39"; function = "qup_se1"; diff --git a/arch/arm64/boot/dts/qcom/ipq5210.dtsi b/arch/arm64/boot/dts/qcom/ipq5210.dtsi index 3761eb03ab24..ec1c9a8c08e0 100644 --- a/arch/arm64/boot/dts/qcom/ipq5210.dtsi +++ b/arch/arm64/boot/dts/qcom/ipq5210.dtsi @@ -5,6 +5,7 @@ #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/clock/qcom,ipq5210-gcc.h> +#include <dt-bindings/interconnect/qcom,ipq5210.h> #include <dt-bindings/reset/qcom,ipq5210-gcc.h> / { @@ -13,6 +14,18 @@ / { interrupt-parent = <&intc>; clocks { + pcie30_phy0_pipe_clk: pcie30_phy0_pipe_clk { + compatible = "fixed-clock"; + clock-frequency = <250000000>; + #clock-cells = <0>; + }; + + pcie30_phy1_pipe_clk: pcie30_phy1_pipe_clk { + compatible = "fixed-clock"; + clock-frequency = <250000000>; + #clock-cells = <0>; + }; + sleep_clk: sleep-clk { compatible = "fixed-clock"; #clock-cells = <0>; @@ -130,6 +143,54 @@ soc@0 { dma-ranges = <0 0 0 0 0x10 0>; ranges = <0 0 0 0 0x10 0>; + pcie0_phy: phy@84000 { + compatible = "qcom,ipq5210-qmp-gen3x1-pcie-phy", + "qcom,ipq9574-qmp-gen3x1-pcie-phy"; + reg = <0x0 0x00084000 0x0 0x1000>; + + clocks = <&gcc GCC_PCIE0_AUX_CLK>, + <&gcc GCC_PCIE0_AHB_CLK>, + <&gcc GCC_PCIE0_PIPE_CLK>; + clock-names = "aux", "cfg_ahb", "pipe"; + + assigned-clocks = <&gcc GCC_PCIE0_AUX_CLK>; + assigned-clock-rates = <20000000>; + + resets = <&gcc GCC_PCIE0_PHY_BCR>, + <&gcc GCC_PCIE0PHY_PHY_BCR>; + reset-names = "phy", "common"; + + #clock-cells = <0>; + clock-output-names = "gcc_pcie0_pipe_clk_src"; + + #phy-cells = <0>; + status = "disabled"; + }; + + pcie1_phy: phy@f4000 { + compatible = "qcom,ipq5210-qmp-gen3x2-pcie-phy", + "qcom,ipq9574-qmp-gen3x2-pcie-phy"; + reg = <0x0 0x000f4000 0x0 0x2000>; + + clocks = <&gcc GCC_PCIE1_AUX_CLK>, + <&gcc GCC_PCIE1_AHB_CLK>, + <&gcc GCC_PCIE1_PIPE_CLK>; + clock-names = "aux", "cfg_ahb", "pipe"; + + assigned-clocks = <&gcc GCC_PCIE1_AUX_CLK>, <&gcc GCC_PCIE1_AHB_CLK>; + assigned-clock-rates = <20000000>, <100000000>; + + resets = <&gcc GCC_PCIE1_PHY_BCR>, + <&gcc GCC_PCIE1PHY_PHY_BCR>; + reset-names = "phy", "common"; + + #clock-cells = <0>; + clock-output-names = "gcc_pcie1_pipe_clk_src"; + + #phy-cells = <0>; + status = "disabled"; + }; + tlmm: pinctrl@1000000 { compatible = "qcom,ipq5210-tlmm"; reg = <0x0 0x01000000 0x0 0x300000>; @@ -146,8 +207,8 @@ gcc: clock-controller@1800000 { reg = <0x0 0x01800000 0x0 0x40000>; clocks = <&xo_board>, <&sleep_clk>, - <0>, - <0>, + <&pcie30_phy0_pipe_clk>, + <&pcie30_phy1_pipe_clk>, <0>, <0>; #clock-cells = <1>; @@ -299,6 +360,202 @@ frame@b128000 { status = "disabled"; }; }; + + pcie1: pcie@50000000 { + compatible = "qcom,pcie-ipq5210", "qcom,pcie-ipq9574"; + reg = <0x0 0x50000000 0x0 0xf1c>, + <0x0 0x50000f20 0x0 0xa8>, + <0x0 0x50001000 0x0 0x1000>, + <0x0 0x000f0000 0x0 0x3000>, + <0x0 0x50100000 0x0 0x1000>, + <0x0 0x000f6000 0x0 0x1000>; + reg-names = "dbi", + "elbi", + "atu", + "parf", + "config", + "mhi"; + device_type = "pci"; + linux,pci-domain = <1>; + bus-range = <0x00 0xff>; + num-lanes = <2>; + #address-cells = <3>; + #size-cells = <2>; + + ranges = <0x81000000 0x0 0x50200000 0x0 0x50200000 0x0 0x00100000>, + <0x82000000 0x0 0x50300000 0x0 0x50300000 0x0 0x0fd00000>; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0x7>; + interrupt-map = <0 0 0 1 &intc 0 0 GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 2 &intc 0 0 GIC_SPI 201 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 3 &intc 0 0 GIC_SPI 202 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 4 &intc 0 0 GIC_SPI 203 IRQ_TYPE_LEVEL_HIGH>; + + interrupts = <GIC_SPI 191 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi0", + "msi1", + "msi2", + "msi3", + "msi4", + "msi5", + "msi6", + "msi7"; + + clocks = <&gcc GCC_PCIE1_AXI_M_CLK>, + <&gcc GCC_PCIE1_AXI_S_CLK>, + <&gcc GCC_PCIE1_AXI_S_BRIDGE_CLK>, + <&gcc GCC_PCIE1_RCHNG_CLK>, + <&gcc GCC_PCIE1_AHB_CLK>, + <&gcc GCC_PCIE1_AUX_CLK>; + + clock-names = "axi_m", + "axi_s", + "axi_bridge", + "rchng", + "ahb", + "aux"; + + resets = <&gcc GCC_PCIE1_PIPE_ARES>, + <&gcc GCC_PCIE1_CORE_STICKY_RESET>, + <&gcc GCC_PCIE1_AXI_S_STICKY_RESET>, + <&gcc GCC_PCIE1_AXI_S_ARES>, + <&gcc GCC_PCIE1_AXI_M_STICKY_RESET>, + <&gcc GCC_PCIE1_AXI_M_ARES>, + <&gcc GCC_PCIE1_AUX_ARES>, + <&gcc GCC_PCIE1_AHB_ARES>; + + reset-names = "pipe", + "sticky", + "axi_s_sticky", + "axi_s", + "axi_m_sticky", + "axi_m", + "aux", + "ahb"; + + interconnects = <&gcc MASTER_CNOC_PCIE1 &gcc SLAVE_CNOC_PCIE1>, + <&gcc MASTER_SNOC_PCIE1 &gcc SLAVE_SNOC_PCIE1>; + interconnect-names = "pcie-mem", "cpu-pcie"; + + status = "disabled"; + + pcie1_rp: pcie@0 { + device_type = "pci"; + reg = <0x0 0x0 0x0 0x0 0x0>; + bus-range = <0x01 0xff>; + phys = <&pcie1_phy>; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + }; + }; + + pcie0: pcie@70000000 { + compatible = "qcom,pcie-ipq5210", "qcom,pcie-ipq9574"; + reg = <0x0 0x70000000 0x0 0xf1c>, + <0x0 0x70000f20 0x0 0xa8>, + <0x0 0x70001000 0x0 0x1000>, + <0x0 0x00080000 0x0 0x3000>, + <0x0 0x70100000 0x0 0x1000>, + <0x0 0x00086000 0x0 0x1000>; + reg-names = "dbi", + "elbi", + "atu", + "parf", + "config", + "mhi"; + device_type = "pci"; + linux,pci-domain = <0>; + bus-range = <0x00 0xff>; + num-lanes = <1>; + #address-cells = <3>; + #size-cells = <2>; + + ranges = <0x81000000 0x0 0x70200000 0x0 0x70200000 0x0 0x00100000>, + <0x82000000 0x0 0x70300000 0x0 0x70300000 0x0 0x0fd00000>; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0x7>; + interrupt-map = <0 0 0 1 &intc 0 0 GIC_SPI 185 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 2 &intc 0 0 GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 3 &intc 0 0 GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 4 &intc 0 0 GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>; + + interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 181 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 182 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi0", + "msi1", + "msi2", + "msi3", + "msi4", + "msi5", + "msi6", + "msi7"; + + clocks = <&gcc GCC_PCIE0_AXI_M_CLK>, + <&gcc GCC_PCIE0_AXI_S_CLK>, + <&gcc GCC_PCIE0_AXI_S_BRIDGE_CLK>, + <&gcc GCC_PCIE0_RCHNG_CLK>, + <&gcc GCC_PCIE0_AHB_CLK>, + <&gcc GCC_PCIE0_AUX_CLK>; + + clock-names = "axi_m", + "axi_s", + "axi_bridge", + "rchng", + "ahb", + "aux"; + + resets = <&gcc GCC_PCIE0_PIPE_ARES>, + <&gcc GCC_PCIE0_CORE_STICKY_RESET>, + <&gcc GCC_PCIE0_AXI_S_STICKY_RESET>, + <&gcc GCC_PCIE0_AXI_S_ARES>, + <&gcc GCC_PCIE0_AXI_M_STICKY_RESET>, + <&gcc GCC_PCIE0_AXI_M_ARES>, + <&gcc GCC_PCIE0_AUX_ARES>, + <&gcc GCC_PCIE0_AHB_ARES>; + + reset-names = "pipe", + "sticky", + "axi_s_sticky", + "axi_s", + "axi_m_sticky", + "axi_m", + "aux", + "ahb"; + + interconnects = <&gcc MASTER_CNOC_PCIE0 &gcc SLAVE_CNOC_PCIE0>, + <&gcc MASTER_SNOC_PCIE0 &gcc SLAVE_SNOC_PCIE0>; + interconnect-names = "pcie-mem", "cpu-pcie"; + + status = "disabled"; + + pcie0_rp: pcie@0 { + device_type = "pci"; + reg = <0x0 0x0 0x0 0x0 0x0>; + bus-range = <0x01 0xff>; + phys = <&pcie0_phy>; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + }; + }; }; timer { -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: ipq5210: Enable PCIe support 2026-05-14 4:13 ` [PATCH 2/2] arm64: dts: qcom: ipq5210: Enable PCIe support Varadarajan Narayanan @ 2026-05-14 12:33 ` sashiko-bot 0 siblings, 0 replies; 5+ messages in thread From: sashiko-bot @ 2026-05-14 12:33 UTC (permalink / raw) To: Varadarajan Narayanan; +Cc: krzk+dt, devicetree, linux-pci, conor+dt, robh Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] The 'phys' property is incorrectly placed in the PCIe Root Port child node rather than the main PCIe controller node. - [Medium] Dummy fixed clocks are used for the PCIe pipe clocks instead of the actual PHY nodes. - [Medium] The `ranges` property in `pcie0` and `pcie1` incorrectly sets the child PCI address for I/O space to match the CPU address. - [Low] The `pcie0` and `pcie1` nodes define `pinctrl-0` but omit the `pinctrl-names` property. -- commit 6a61b5c948b9e28a9b86554a5e888347a3c559ab Author: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com> arm64: dts: qcom: ipq5210: Enable PCIe support Add DT entries to enable the PCIe controllers found in ipq5210. > diff --git a/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts b/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts > index 941f866ecfe95..5e599a1cea3f4 100644 > --- a/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts > +++ b/arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts [ ... ] > @@ -20,6 +21,32 @@ chosen { > }; > }; > > +&pcie0_phy { > + status = "okay"; > +}; > + > +&pcie0_rp { > + reset-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>; > +}; > + > +&pcie0 { > + pinctrl-0 = <&pcie0_default_state>; Should this node also include a pinctrl-names property? Omitting pinctrl-names = "default" might trigger validation warnings, even if the pinctrl core falls back to index 0. > + status = "okay"; > +}; > + > +&pcie1_phy { > + status = "okay"; > +}; > + > +&pcie1_rp { > + reset-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>; > +}; > + > +&pcie1 { > + pinctrl-0 = <&pcie1_default_state>; Similarly, should pinctrl-names = "default" be added here? > + status = "okay"; > +}; > + > &sdhc { > max-frequency = <192000000>; > bus-width = <4>; > diff --git a/arch/arm64/boot/dts/qcom/ipq5210.dtsi b/arch/arm64/boot/dts/qcom/ipq5210.dtsi > index 3761eb03ab246..ec1c9a8c08e0a 100644 > --- a/arch/arm64/boot/dts/qcom/ipq5210.dtsi > +++ b/arch/arm64/boot/dts/qcom/ipq5210.dtsi [ ... ] > @@ -13,6 +14,18 @@ / { > interrupt-parent = <&intc>; > > clocks { > + pcie30_phy0_pipe_clk: pcie30_phy0_pipe_clk { > + compatible = "fixed-clock"; > + clock-frequency = <250000000>; > + #clock-cells = <0>; > + }; > + > + pcie30_phy1_pipe_clk: pcie30_phy1_pipe_clk { > + compatible = "fixed-clock"; > + clock-frequency = <250000000>; > + #clock-cells = <0>; > + }; > + Is it intentional to use dummy fixed clocks for the PCIe pipe clocks here? The PCIe PHY nodes below (pcie0_phy, pcie1_phy) are correctly configured as clock providers (#clock-cells = <0> and clock-output-names). Hardcoding these as fixed clocks severs the dependency graph, so the GCC might not establish a parent-child relationship with the PHY. Could this allow the clock to be enabled without the PHY being powered on? [ ... ] > + pcie1: pcie@50000000 { [ ... ] > + #size-cells = <2>; > + > + ranges = <0x81000000 0x0 0x50200000 0x0 0x50200000 0x0 0x00100000>, Does this I/O space mapping correctly set the PCI child address? This maps the CPU address 0x50200000 to PCI I/O address 0x50200000, but standard PCI architectures typically expect the I/O port address space to start at address 0 on the PCI bus side. Should it translate the CPU I/O base address to the PCI I/O address 0? > + <0x82000000 0x0 0x50300000 0x0 0x50300000 0x0 0x0fd00000>; [ ... ] > + pcie1_rp: pcie@0 { > + device_type = "pci"; > + reg = <0x0 0x0 0x0 0x0 0x0>; > + bus-range = <0x01 0xff>; > + phys = <&pcie1_phy>; Should the phys property be placed in the main PCIe controller node instead of the Root Port child node? The Qualcomm PCIe controller driver expects to find the phys property in the main controller node using devm_phy_optional_get(dev, "pciephy"). If it is placed in the child node, devm_phy_optional_get() might return NULL and silently bypass PHY initialization. > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + }; > + }; > + > + pcie0: pcie@70000000 { [ ... ] > + #size-cells = <2>; > + > + ranges = <0x81000000 0x0 0x70200000 0x0 0x70200000 0x0 0x00100000>, Similar to pcie1, should this I/O space mapping translate the CPU I/O base address to the PCI I/O address 0 instead of matching the CPU address? > + <0x82000000 0x0 0x70300000 0x0 0x70300000 0x0 0x0fd00000>; [ ... ] > + pcie0_rp: pcie@0 { > + device_type = "pci"; > + reg = <0x0 0x0 0x0 0x0 0x0>; > + bus-range = <0x01 0xff>; > + phys = <&pcie0_phy>; Also similarly here, should this phys property belong to the parent pcie0 node instead of the pcie0_rp child node? > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + }; > + }; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260514-pci-ipq5210-v1-0-a09436200b35@oss.qualcomm.com?part=2 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-15 10:55 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-14 4:13 [PATCH 0/2] Enable PCIe controllers present in Qualcomm ipq5210 Varadarajan Narayanan 2026-05-14 4:13 ` [PATCH 1/2] dt-bindings: PCI: qcom,pcie-ipq9574: Document the ipq5210 pcie controller Varadarajan Narayanan 2026-05-15 10:55 ` Krzysztof Kozlowski 2026-05-14 4:13 ` [PATCH 2/2] arm64: dts: qcom: ipq5210: Enable PCIe support Varadarajan Narayanan 2026-05-14 12:33 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox