2023-04-21 12:51:56

by Devi Priya

[permalink] [raw]
Subject: [PATCH V3 0/6] Add PCIe support for IPQ9574

This series adds support for enabling the PCIe host devices (PCIe0, PCIe1,
PCIe2, PCIe3) found on IPQ9574 platform.
The PCIe0 & PCIe1 are 1-lane Gen3 host and PCIe2 & PCIe3
are 2-lane Gen3 host.

DTS patch is based on the below series
https://lore.kernel.org/linux-arm-msm/[email protected]/

Changes in V3:
- Dropped the phy driver and binding patches as they have been
posted as a separate series.
- Dropped the pinctrl binding fix patch as it is unrelated to the series
dt-bindings: pinctrl: qcom: Add few missing functions.
- Rebased on linux-next/master.
- Detailed change logs are added to the respective patches.

Changes in V2:
https://lore.kernel.org/linux-arm-msm/[email protected]/
- Reordered the patches and splitted the board DT changes
into a separate patch as suggested
- Detailed change logs are added to the respective patches

[V1]
https://lore.kernel.org/linux-arm-msm/[email protected]/

Devi Priya (6):
dt-bindings: clock: Add PCIe pipe clock definitions
clk: qcom: gcc-ipq9574: Add PCIe pipe clocks
dt-bindings: PCI: qcom: Add IPQ9574
arm64: dts: qcom: ipq9574: Add PCIe PHYs and controller nodes
arm64: dts: qcom: ipq9574: Enable PCIe PHYs and controllers
PCI: qcom: Add support for IPQ9574

.../devicetree/bindings/pci/qcom,pcie.yaml | 40 ++
arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 62 +++
arch/arm64/boot/dts/qcom/ipq9574.dtsi | 375 +++++++++++++++++-
drivers/clk/qcom/gcc-ipq9574.c | 76 ++++
drivers/pci/controller/dwc/pcie-qcom.c | 61 ++-
include/dt-bindings/clock/qcom,ipq9574-gcc.h | 4 +
6 files changed, 595 insertions(+), 23 deletions(-)


base-commit: 44bf136283e567b2b62653be7630e7511da41da2
--
2.17.1


2023-04-21 12:52:06

by Devi Priya

[permalink] [raw]
Subject: [PATCH V3 1/6] dt-bindings: clock: Add PCIe pipe clock definitions

Add PCIe pipe clock definitions for IPQ9574 SoC.

Acked-by: Stephen Boyd <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
Co-developed-by: Anusha Rao <[email protected]>
Signed-off-by: Anusha Rao <[email protected]>
Signed-off-by: Devi Priya <[email protected]>
---
Changes in V3:
- Picked up the Acked-by tags

include/dt-bindings/clock/qcom,ipq9574-gcc.h | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,ipq9574-gcc.h b/include/dt-bindings/clock/qcom,ipq9574-gcc.h
index 5a2961bfe893..2d7b46027ce9 100644
--- a/include/dt-bindings/clock/qcom,ipq9574-gcc.h
+++ b/include/dt-bindings/clock/qcom,ipq9574-gcc.h
@@ -210,4 +210,8 @@
#define GCC_SNOC_PCIE1_1LANE_S_CLK 201
#define GCC_SNOC_PCIE2_2LANE_S_CLK 202
#define GCC_SNOC_PCIE3_2LANE_S_CLK 203
+#define GCC_PCIE0_PIPE_CLK 204
+#define GCC_PCIE1_PIPE_CLK 205
+#define GCC_PCIE2_PIPE_CLK 206
+#define GCC_PCIE3_PIPE_CLK 207
#endif
--
2.17.1

2023-04-21 12:52:29

by Devi Priya

[permalink] [raw]
Subject: [PATCH V3 3/6] dt-bindings: PCI: qcom: Add IPQ9574

Add bindings for PCIe hosts on IPQ9574 platform and allow
msi-parent property.

Signed-off-by: Devi Priya <[email protected]>
---
Changes in V3:
- Rebased on linux-next/master

.../devicetree/bindings/pci/qcom,pcie.yaml | 40 +++++++++++++++++++
1 file changed, 40 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 81971be4e554..a92cecc5fe6f 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -26,6 +26,7 @@ properties:
- qcom,pcie-ipq8064-v2
- qcom,pcie-ipq8074
- qcom,pcie-ipq8074-gen3
+ - qcom,pcie-ipq9574
- qcom,pcie-msm8996
- qcom,pcie-qcs404
- qcom,pcie-sa8540p
@@ -113,6 +114,8 @@ properties:
power-domains:
maxItems: 1

+ msi-parent: true
+
perst-gpios:
description: GPIO controlled connection to PERST# signal
maxItems: 1
@@ -138,6 +141,8 @@ anyOf:
- required:
- msi-map
- msi-map-mask
+ - required:
+ - msi-parent

allOf:
- $ref: /schemas/pci/pci-bus.yaml#
@@ -171,6 +176,7 @@ allOf:
enum:
- qcom,pcie-ipq6018
- qcom,pcie-ipq8074-gen3
+ - qcom,pcie-ipq9574
then:
properties:
reg:
@@ -382,6 +388,39 @@ allOf:
- const: ahb # AHB Reset
- const: axi_m_sticky # AXI Master Sticky reset

+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,pcie-ipq9574
+ then:
+ properties:
+ clocks:
+ minItems: 6
+ maxItems: 6
+ clock-names:
+ items:
+ - const: ahb # AHB clock
+ - const: aux # Auxiliary clock
+ - const: axi_m # AXI Master clock
+ - const: axi_s # AXI Slave clock
+ - const: axi_bridge # AXI bridge clock
+ - const: rchng
+ resets:
+ minItems: 8
+ maxItems: 8
+ reset-names:
+ items:
+ - const: pipe # PIPE reset
+ - const: sticky # Core Sticky reset
+ - const: axi_s_sticky # AXI Slave Sticky reset
+ - const: axi_s # AXI Slave reset
+ - const: axi_m_sticky # AXI Master Sticky reset
+ - const: axi_m # AXI Master reset
+ - const: aux # AUX Reset
+ - const: ahb # AHB Reset
+
- if:
properties:
compatible:
@@ -767,6 +806,7 @@ allOf:
- qcom,pcie-ipq8064v2
- qcom,pcie-ipq8074
- qcom,pcie-ipq8074-gen3
+ - qcom,pcie-ipq9574
- qcom,pcie-qcs404
then:
required:
--
2.17.1

2023-04-21 12:52:39

by Devi Priya

[permalink] [raw]
Subject: [PATCH V3 4/6] arm64: dts: qcom: ipq9574: Add PCIe PHYs and controller nodes

Add PCIe0, PCIe1, PCIe2, PCIe3 (and corresponding PHY) devices
found on IPQ9574 platform. The PCIe0 & PCIe1 are 1-lane Gen3
host whereas PCIe2 & PCIe3 are 2-lane Gen3 host.

Co-developed-by: Anusha Rao <[email protected]>
Signed-off-by: Anusha Rao <[email protected]>
Signed-off-by: Devi Priya <[email protected]>
---
Changes in V3:
- Fixed up the PCI I/O port ranges

arch/arm64/boot/dts/qcom/ipq9574.dtsi | 375 +++++++++++++++++++++++++-
1 file changed, 370 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index e757b57957cf..953a839a1141 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -6,8 +6,8 @@
* Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
*/

-#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/clock/qcom,ipq9574-gcc.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/reset/qcom,ipq9574-gcc.h>

/ {
@@ -116,6 +116,58 @@
#size-cells = <1>;
ranges = <0 0 0 0xffffffff>;

+ pcie0_phy: phy@84000 {
+ compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
+ reg = <0x00084000 0x1000>;
+
+ clocks = <&gcc GCC_PCIE0_AUX_CLK>,
+ <&gcc GCC_PCIE0_AHB_CLK>,
+ <&gcc GCC_ANOC_PCIE0_1LANE_M_CLK>,
+ <&gcc GCC_SNOC_PCIE0_1LANE_S_CLK>,
+ <&gcc GCC_PCIE0_PIPE_CLK>;
+ clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "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";
+
+ };
+
+ pcie2_phy: phy@8c000 {
+ compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
+ reg = <0x0008c000 0x2000>;
+
+ clocks = <&gcc GCC_PCIE2_AUX_CLK>,
+ <&gcc GCC_PCIE2_AHB_CLK>,
+ <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
+ <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>,
+ <&gcc GCC_PCIE2_PIPE_CLK>;
+ clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "pipe";
+
+ assigned-clocks = <&gcc GCC_PCIE2_AUX_CLK>;
+ assigned-clock-rates = <20000000>;
+
+ resets = <&gcc GCC_PCIE2_PHY_BCR>,
+ <&gcc GCC_PCIE2PHY_PHY_BCR>;
+ reset-names = "phy", "common";
+
+ #clock-cells = <0>;
+ clock-output-names = "gcc_pcie2_pipe_clk_src";
+
+ #phy-cells = <0>;
+ status = "disabled";
+
+ };
+
rng: rng@e3000 {
compatible = "qcom,prng-ee";
reg = <0x000e3000 0x1000>;
@@ -123,6 +175,58 @@
clock-names = "core";
};

+ pcie3_phy: phy@f4000 {
+ compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
+ reg = <0x000f4000 0x2000>;
+
+ clocks = <&gcc GCC_PCIE3_AUX_CLK>,
+ <&gcc GCC_PCIE3_AHB_CLK>,
+ <&gcc GCC_ANOC_PCIE3_2LANE_M_CLK>,
+ <&gcc GCC_SNOC_PCIE3_2LANE_S_CLK>,
+ <&gcc GCC_PCIE3_PIPE_CLK>;
+ clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "pipe";
+
+ assigned-clocks = <&gcc GCC_PCIE3_AUX_CLK>;
+ assigned-clock-rates = <20000000>;
+
+ resets = <&gcc GCC_PCIE3_PHY_BCR>,
+ <&gcc GCC_PCIE3PHY_PHY_BCR>;
+ reset-names = "phy", "common";
+
+ #clock-cells = <0>;
+ clock-output-names = "gcc_pcie3_pipe_clk_src";
+
+ #phy-cells = <0>;
+ status = "disabled";
+
+ };
+
+ pcie1_phy: phy@fc000 {
+ compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
+ reg = <0x000fc000 0x1000>;
+
+ clocks = <&gcc GCC_PCIE1_AUX_CLK>,
+ <&gcc GCC_PCIE1_AHB_CLK>,
+ <&gcc GCC_ANOC_PCIE1_1LANE_M_CLK>,
+ <&gcc GCC_SNOC_PCIE1_1LANE_S_CLK>,
+ <&gcc GCC_PCIE1_PIPE_CLK>;
+ clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "pipe";
+
+ assigned-clocks = <&gcc GCC_PCIE1_AUX_CLK>;
+ assigned-clock-rates = <20000000>;
+
+ 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,ipq9574-tlmm";
reg = <0x01000000 0x300000>;
@@ -146,10 +250,10 @@
reg = <0x01800000 0x80000>;
clocks = <&xo_board_clk>,
<&sleep_clk>,
- <0>,
- <0>,
- <0>,
- <0>,
+ <&pcie0_phy>,
+ <&pcie1_phy>,
+ <&pcie2_phy>,
+ <&pcie3_phy>,
<0>;
#clock-cells = <1>;
#reset-cells = <1>;
@@ -478,6 +582,267 @@
status = "disabled";
};
};
+
+ pcie1: pci@10000000 {
+ compatible = "qcom,pcie-ipq9574";
+ reg = <0x10000000 0xf1d>,
+ <0x10000F20 0xa8>,
+ <0x10001000 0x1000>,
+ <0x000F8000 0x4000>,
+ <0x10100000 0x1000>;
+ reg-names = "dbi", "elbi", "atu", "parf", "config";
+ device_type = "pci";
+ linux,pci-domain = <2>;
+ bus-range = <0x00 0xff>;
+ num-lanes = <1>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ ranges = <0x01000000 0x0 0x00000000 0x10200000 0x0 0x100000>, /* I/O */
+ <0x02000000 0x0 0x10300000 0x10300000 0x0 0x7d00000>; /* MEM */
+
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 0x7>;
+ interrupt-map = <0 0 0 1 &intc 0 35 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
+ <0 0 0 2 &intc 0 49 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
+ <0 0 0 3 &intc 0 84 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
+ <0 0 0 4 &intc 0 85 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
+
+ interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "global_irq";
+
+ /* clocks and clock-names are used to enable the clock in CBCR */
+ clocks = <&gcc GCC_PCIE1_AHB_CLK>,
+ <&gcc GCC_PCIE1_AUX_CLK>,
+ <&gcc GCC_PCIE1_AXI_M_CLK>,
+ <&gcc GCC_PCIE1_AXI_S_CLK>,
+ <&gcc GCC_PCIE1_AXI_S_BRIDGE_CLK>,
+ <&gcc GCC_PCIE1_RCHNG_CLK>;
+ clock-names = "ahb",
+ "aux",
+ "axi_m",
+ "axi_s",
+ "axi_bridge",
+ "rchng";
+
+ resets = <&gcc GCC_PCIE1_PIPE_ARES>,
+ <&gcc GCC_PCIE1_CORE_STICKY_ARES>,
+ <&gcc GCC_PCIE1_AXI_S_STICKY_ARES>,
+ <&gcc GCC_PCIE1_AXI_S_ARES>,
+ <&gcc GCC_PCIE1_AXI_M_STICKY_ARES>,
+ <&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";
+
+ phys = <&pcie1_phy>;
+ phy-names = "pciephy";
+ msi-parent = <&v2m0>;
+ status = "disabled";
+ };
+
+ pcie3: pci@18000000 {
+ compatible = "qcom,pcie-ipq9574";
+ reg = <0x18000000 0xf1d>,
+ <0x18000F20 0xa8>,
+ <0x18001000 0x1000>,
+ <0x000F0000 0x4000>,
+ <0x18100000 0x1000>;
+ reg-names = "dbi", "elbi", "atu", "parf", "config";
+ device_type = "pci";
+ linux,pci-domain = <4>;
+ bus-range = <0x00 0xff>;
+ num-lanes = <2>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ ranges = <0x01000000 0x0 0x00000000 0x18200000 0x0 0x100000>, /* I/O */
+ <0x02000000 0x0 0x18300000 0x18300000 0x0 0x7d00000>; /* MEM */
+
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 0x7>;
+ interrupt-map = <0 0 0 1 &intc 0 189 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
+ <0 0 0 2 &intc 0 190 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
+ <0 0 0 3 &intc 0 191 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
+ <0 0 0 4 &intc 0 192 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
+
+ interrupts = <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "global_irq";
+
+ /* clocks and clock-names are used to enable the clock in CBCR */
+ clocks = <&gcc GCC_PCIE3_AHB_CLK>,
+ <&gcc GCC_PCIE3_AUX_CLK>,
+ <&gcc GCC_PCIE3_AXI_M_CLK>,
+ <&gcc GCC_PCIE3_AXI_S_CLK>,
+ <&gcc GCC_PCIE3_AXI_S_BRIDGE_CLK>,
+ <&gcc GCC_PCIE3_RCHNG_CLK>;
+ clock-names = "ahb",
+ "aux",
+ "axi_m",
+ "axi_s",
+ "axi_bridge",
+ "rchng";
+
+ resets = <&gcc GCC_PCIE3_PIPE_ARES>,
+ <&gcc GCC_PCIE3_CORE_STICKY_ARES>,
+ <&gcc GCC_PCIE3_AXI_S_STICKY_ARES>,
+ <&gcc GCC_PCIE3_AXI_S_ARES>,
+ <&gcc GCC_PCIE3_AXI_M_STICKY_ARES>,
+ <&gcc GCC_PCIE3_AXI_M_ARES>,
+ <&gcc GCC_PCIE3_AUX_ARES>,
+ <&gcc GCC_PCIE3_AHB_ARES>;
+ reset-names = "pipe",
+ "sticky",
+ "axi_s_sticky",
+ "axi_s",
+ "axi_m_sticky",
+ "axi_m",
+ "aux",
+ "ahb";
+
+ phys = <&pcie3_phy>;
+ phy-names = "pciephy";
+ msi-parent = <&v2m0>;
+ status = "disabled";
+ };
+
+ pcie2: pci@20000000 {
+ compatible = "qcom,pcie-ipq9574";
+ reg = <0x20000000 0xf1d>,
+ <0x20000F20 0xa8>,
+ <0x20001000 0x1000>,
+ <0x00088000 0x4000>,
+ <0x20100000 0x1000>;
+ reg-names = "dbi", "elbi", "atu", "parf", "config";
+ device_type = "pci";
+ linux,pci-domain = <3>;
+ bus-range = <0x00 0xff>;
+ num-lanes = <2>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ ranges = <0x01000000 0x0 0x00000000 0x20200000 0x0 0x100000>, /* I/O */
+ <0x02000000 0x0 0x20300000 0x20300000 0x0 0x7d00000>; /* MEM */
+
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 0x7>;
+ interrupt-map = <0 0 0 1 &intc 0 164 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
+ <0 0 0 2 &intc 0 165 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
+ <0 0 0 3 &intc 0 186 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
+ <0 0 0 4 &intc 0 187 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
+
+ interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "global_irq";
+
+ /* clocks and clock-names are used to enable the clock in CBCR */
+ clocks = <&gcc GCC_PCIE2_AHB_CLK>,
+ <&gcc GCC_PCIE2_AUX_CLK>,
+ <&gcc GCC_PCIE2_AXI_M_CLK>,
+ <&gcc GCC_PCIE2_AXI_S_CLK>,
+ <&gcc GCC_PCIE2_AXI_S_BRIDGE_CLK>,
+ <&gcc GCC_PCIE2_RCHNG_CLK>;
+ clock-names = "ahb",
+ "aux",
+ "axi_m",
+ "axi_s",
+ "axi_bridge",
+ "rchng";
+
+ resets = <&gcc GCC_PCIE2_PIPE_ARES>,
+ <&gcc GCC_PCIE2_CORE_STICKY_ARES>,
+ <&gcc GCC_PCIE2_AXI_S_STICKY_ARES>,
+ <&gcc GCC_PCIE2_AXI_S_ARES>,
+ <&gcc GCC_PCIE2_AXI_M_STICKY_ARES>,
+ <&gcc GCC_PCIE2_AXI_M_ARES>,
+ <&gcc GCC_PCIE2_AUX_ARES>,
+ <&gcc GCC_PCIE2_AHB_ARES>;
+ reset-names = "pipe",
+ "sticky",
+ "axi_s_sticky",
+ "axi_s",
+ "axi_m_sticky",
+ "axi_m",
+ "aux",
+ "ahb";
+
+ phys = <&pcie2_phy>;
+ phy-names = "pciephy";
+ msi-parent = <&v2m0>;
+ status = "disabled";
+ };
+
+ pcie0: pci@28000000 {
+ compatible = "qcom,pcie-ipq9574";
+ reg = <0x28000000 0xf1d>,
+ <0x28000F20 0xa8>,
+ <0x28001000 0x1000>,
+ <0x00080000 0x4000>,
+ <0x28100000 0x1000>;
+ reg-names = "dbi", "elbi", "atu", "parf", "config";
+ device_type = "pci";
+ linux,pci-domain = <1>;
+ bus-range = <0x00 0xff>;
+ num-lanes = <1>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ ranges = <0x01000000 0x0 0x00000000 0x28200000 0x0 0x100000>, /* I/O */
+ <0x02000000 0x0 0x28300000 0x28300000 0x0 0x7d00000>; /* MEM */
+
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 0x7>;
+ interrupt-map = <0 0 0 1 &intc 0 75 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
+ <0 0 0 2 &intc 0 78 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
+ <0 0 0 3 &intc 0 79 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
+ <0 0 0 4 &intc 0 83 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
+
+ interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "global_irq";
+
+ /* clocks and clock-names are used to enable the clock in CBCR */
+ clocks = <&gcc GCC_PCIE0_AHB_CLK>,
+ <&gcc GCC_PCIE0_AUX_CLK>,
+ <&gcc GCC_PCIE0_AXI_M_CLK>,
+ <&gcc GCC_PCIE0_AXI_S_CLK>,
+ <&gcc GCC_PCIE0_AXI_S_BRIDGE_CLK>,
+ <&gcc GCC_PCIE0_RCHNG_CLK>;
+ clock-names = "ahb",
+ "aux",
+ "axi_m",
+ "axi_s",
+ "axi_bridge",
+ "rchng";
+
+ resets = <&gcc GCC_PCIE0_PIPE_ARES>,
+ <&gcc GCC_PCIE0_CORE_STICKY_ARES>,
+ <&gcc GCC_PCIE0_AXI_S_STICKY_ARES>,
+ <&gcc GCC_PCIE0_AXI_S_ARES>,
+ <&gcc GCC_PCIE0_AXI_M_STICKY_ARES>,
+ <&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";
+
+ phys = <&pcie0_phy>;
+ phy-names = "pciephy";
+ msi-parent = <&v2m0>;
+ status = "disabled";
+ };
+
};

timer {
--
2.17.1

2023-04-21 12:52:43

by Devi Priya

[permalink] [raw]
Subject: [PATCH V3 5/6] arm64: dts: qcom: ipq9574: Enable PCIe PHYs and controllers

Enable the PCIe controller and PHY nodes corresponding to
RDP 433.

Signed-off-by: Devi Priya <[email protected]>
---
Changes in V3:
- No change

arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 62 +++++++++++++++++++++
1 file changed, 62 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
index 7be578017bf7..3ae38cf327ea 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
+++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
@@ -8,6 +8,7 @@

/dts-v1/;

+#include <dt-bindings/gpio/gpio.h>
#include "ipq9574.dtsi"

/ {
@@ -43,6 +44,42 @@
};
};

+&pcie1_phy {
+ status = "okay";
+};
+
+&pcie1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie_1_pin>;
+
+ perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
+ status = "okay";
+};
+
+&pcie2_phy {
+ status = "okay";
+};
+
+&pcie2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie_2_pin>;
+
+ perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
+ status = "okay";
+};
+
+&pcie3_phy {
+ status = "okay";
+};
+
+&pcie3 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie_3_pin>;
+
+ perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
+ status = "okay";
+};
+
&sdhc_1 {
pinctrl-0 = <&sdc_default_state>;
pinctrl-names = "default";
@@ -60,6 +97,31 @@
};

&tlmm {
+
+ pcie_1_pin: pcie-1-state {
+ pins = "gpio26";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-pull-down;
+ output-low;
+ };
+
+ pcie_2_pin: pcie-2-state {
+ pins = "gpio29";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-pull-down;
+ output-low;
+ };
+
+ pcie_3_pin: pcie-3-state {
+ pins = "gpio32";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-pull-up;
+ output-low;
+ };
+
sdc_default_state: sdc-default-state {
clk-pins {
pins = "gpio5";
--
2.17.1

2023-04-21 12:52:43

by Devi Priya

[permalink] [raw]
Subject: [PATCH V3 6/6] PCI: qcom: Add support for IPQ9574

The IPQ9574 platform has 4 Gen3 PCIe controllers: two single-lane
and two dual-lane based on SNPS core 5.70a
The Qcom IP rev is 1.27.0 and Synopsys IP rev is 5.80a
Added a new compatible 'qcom,pcie-ipq9574' and 'ops_1_27_0'
which reuses all the members of 'ops_2_9_0' except for the post_init
as the SLV_ADDR_SPACE_SIZE configuration differs between 2_9_0
and 1_27_0.
Also, modified get_resources of 'ops 2_9_0' to get the clocks
from the device tree and modelled the post init sequence as
a common function to avoid code redundancy.

Co-developed-by: Anusha Rao <[email protected]>
Signed-off-by: Anusha Rao <[email protected]>
Signed-off-by: Devi Priya <[email protected]>
---
Changes in V3:
- Rebased on top of linux-next/master

drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++--------
1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 4ab30892f6ef..3682ecdead1f 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -107,6 +107,7 @@

/* PARF_SLV_ADDR_SPACE_SIZE register value */
#define SLV_ADDR_SPACE_SZ 0x10000000
+#define SLV_ADDR_SPACE_SZ_1_27_0 0x08000000

/* PARF_MHI_CLOCK_RESET_CTRL register fields */
#define AHB_CLK_EN BIT(0)
@@ -202,10 +203,10 @@ struct qcom_pcie_resources_2_7_0 {
struct reset_control *rst;
};

-#define QCOM_PCIE_2_9_0_MAX_CLOCKS 5
struct qcom_pcie_resources_2_9_0 {
- struct clk_bulk_data clks[QCOM_PCIE_2_9_0_MAX_CLOCKS];
+ struct clk_bulk_data *clks;
struct reset_control *rst;
+ int num_clks;
};

union qcom_pcie_resources {
@@ -1050,17 +1051,10 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
struct dw_pcie *pci = pcie->pci;
struct device *dev = pci->dev;
- int ret;

- res->clks[0].id = "iface";
- res->clks[1].id = "axi_m";
- res->clks[2].id = "axi_s";
- res->clks[3].id = "axi_bridge";
- res->clks[4].id = "rchng";
-
- ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
- if (ret < 0)
- return ret;
+ res->num_clks = devm_clk_bulk_get_all(dev, &res->clks);
+ if (res->clks < 0)
+ return res->num_clks;

res->rst = devm_reset_control_array_get_exclusive(dev);
if (IS_ERR(res->rst))
@@ -1073,7 +1067,7 @@ static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
{
struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;

- clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
+ clk_bulk_disable_unprepare(res->num_clks, res->clks);
}

static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
@@ -1102,19 +1096,16 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)

usleep_range(2000, 2500);

- return clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
+ return clk_bulk_prepare_enable(res->num_clks, res->clks);
}

-static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
+static int qcom_pcie_post_init(struct qcom_pcie *pcie)
{
struct dw_pcie *pci = pcie->pci;
u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
u32 val;
int i;

- writel(SLV_ADDR_SPACE_SZ,
- pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
-
val = readl(pcie->parf + PARF_PHY_CTRL);
val &= ~PHY_TEST_PWR_DOWN;
writel(val, pcie->parf + PARF_PHY_CTRL);
@@ -1151,6 +1142,26 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
return 0;
}

+static int qcom_pcie_post_init_1_27_0(struct qcom_pcie *pcie)
+{
+ writel(SLV_ADDR_SPACE_SZ_1_27_0,
+ pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
+
+ qcom_pcie_post_init(pcie);
+
+ return 0;
+}
+
+static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
+{
+ writel(SLV_ADDR_SPACE_SZ,
+ pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
+
+ qcom_pcie_post_init(pcie);
+
+ return 0;
+}
+
static int qcom_pcie_link_up(struct dw_pcie *pci)
{
u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
@@ -1291,6 +1302,15 @@ static const struct qcom_pcie_ops ops_2_9_0 = {
.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
};

+/* Qcom IP rev.: 1.27.0 Synopsys IP rev.: 5.80a */
+static const struct qcom_pcie_ops ops_1_27_0 = {
+ .get_resources = qcom_pcie_get_resources_2_9_0,
+ .init = qcom_pcie_init_2_9_0,
+ .post_init = qcom_pcie_post_init_1_27_0,
+ .deinit = qcom_pcie_deinit_2_9_0,
+ .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
+};
+
static const struct qcom_pcie_cfg cfg_1_0_0 = {
.ops = &ops_1_0_0,
};
@@ -1323,6 +1343,10 @@ static const struct qcom_pcie_cfg cfg_2_9_0 = {
.ops = &ops_2_9_0,
};

+static const struct qcom_pcie_cfg cfg_1_27_0 = {
+ .ops = &ops_1_27_0,
+};
+
static const struct dw_pcie_ops dw_pcie_ops = {
.link_up = qcom_pcie_link_up,
.start_link = qcom_pcie_start_link,
@@ -1607,6 +1631,7 @@ static const struct of_device_id qcom_pcie_match[] = {
{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
{ .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
+ { .compatible = "qcom,pcie-ipq9574", .data = &cfg_1_27_0 },
{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
--
2.17.1

2023-04-21 12:52:55

by Devi Priya

[permalink] [raw]
Subject: [PATCH V3 2/6] clk: qcom: gcc-ipq9574: Add PCIe pipe clocks

Add the PCIe pipe clocks needed for enabling PCIe in IPQ9574.

Acked-by: Stephen Boyd <[email protected]>
Co-developed-by: Anusha Rao <[email protected]>
Signed-off-by: Anusha Rao <[email protected]>
Signed-off-by: Devi Priya <[email protected]>
---
Changes in V3:
- Picked up the Acked-by tags

drivers/clk/qcom/gcc-ipq9574.c | 76 ++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)

diff --git a/drivers/clk/qcom/gcc-ipq9574.c b/drivers/clk/qcom/gcc-ipq9574.c
index ca40cb810a95..a4cf750043af 100644
--- a/drivers/clk/qcom/gcc-ipq9574.c
+++ b/drivers/clk/qcom/gcc-ipq9574.c
@@ -1522,6 +1522,24 @@ static struct clk_regmap_phy_mux pcie0_pipe_clk_src = {
},
};

+static struct clk_branch gcc_pcie0_pipe_clk = {
+ .halt_reg = 0x28044,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x28044,
+ .enable_mask = BIT(0),
+ .hw.init = &(const struct clk_init_data) {
+ .name = "gcc_pcie0_pipe_clk",
+ .parent_hws = (const struct clk_hw *[]) {
+ &pcie0_pipe_clk_src.clkr.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static struct clk_regmap_phy_mux pcie1_pipe_clk_src = {
.reg = 0x29064,
.clkr = {
@@ -1536,6 +1554,24 @@ static struct clk_regmap_phy_mux pcie1_pipe_clk_src = {
},
};

+static struct clk_branch gcc_pcie1_pipe_clk = {
+ .halt_reg = 0x29044,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x29044,
+ .enable_mask = BIT(0),
+ .hw.init = &(const struct clk_init_data) {
+ .name = "gcc_pcie1_pipe_clk",
+ .parent_hws = (const struct clk_hw *[]) {
+ &pcie1_pipe_clk_src.clkr.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static struct clk_regmap_phy_mux pcie2_pipe_clk_src = {
.reg = 0x2a064,
.clkr = {
@@ -1550,6 +1586,24 @@ static struct clk_regmap_phy_mux pcie2_pipe_clk_src = {
},
};

+static struct clk_branch gcc_pcie2_pipe_clk = {
+ .halt_reg = 0x2a044,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x2a044,
+ .enable_mask = BIT(0),
+ .hw.init = &(const struct clk_init_data) {
+ .name = "gcc_pcie2_pipe_clk",
+ .parent_hws = (const struct clk_hw *[]) {
+ &pcie2_pipe_clk_src.clkr.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static struct clk_regmap_phy_mux pcie3_pipe_clk_src = {
.reg = 0x2b064,
.clkr = {
@@ -1564,6 +1618,24 @@ static struct clk_regmap_phy_mux pcie3_pipe_clk_src = {
},
};

+static struct clk_branch gcc_pcie3_pipe_clk = {
+ .halt_reg = 0x2b044,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x2b044,
+ .enable_mask = BIT(0),
+ .hw.init = &(const struct clk_init_data) {
+ .name = "gcc_pcie3_pipe_clk",
+ .parent_hws = (const struct clk_hw *[]) {
+ &pcie3_pipe_clk_src.clkr.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static const struct freq_tbl ftbl_pcie_rchng_clk_src[] = {
F(24000000, P_XO, 1, 0, 0),
F(100000000, P_GPLL0, 8, 0, 0),
@@ -3878,9 +3950,13 @@ static struct clk_regmap *gcc_ipq9574_clks[] = {
[GCC_PCIE3_AXI_S_BRIDGE_CLK] = &gcc_pcie3_axi_s_bridge_clk.clkr,
[GCC_PCIE3_AXI_S_CLK] = &gcc_pcie3_axi_s_clk.clkr,
[PCIE0_PIPE_CLK_SRC] = &pcie0_pipe_clk_src.clkr,
+ [GCC_PCIE0_PIPE_CLK] = &gcc_pcie0_pipe_clk.clkr,
[PCIE1_PIPE_CLK_SRC] = &pcie1_pipe_clk_src.clkr,
+ [GCC_PCIE1_PIPE_CLK] = &gcc_pcie1_pipe_clk.clkr,
[PCIE2_PIPE_CLK_SRC] = &pcie2_pipe_clk_src.clkr,
+ [GCC_PCIE2_PIPE_CLK] = &gcc_pcie2_pipe_clk.clkr,
[PCIE3_PIPE_CLK_SRC] = &pcie3_pipe_clk_src.clkr,
+ [GCC_PCIE3_PIPE_CLK] = &gcc_pcie3_pipe_clk.clkr,
[PCIE_AUX_CLK_SRC] = &pcie_aux_clk_src.clkr,
[GCC_PCIE0_AUX_CLK] = &gcc_pcie0_aux_clk.clkr,
[GCC_PCIE1_AUX_CLK] = &gcc_pcie1_aux_clk.clkr,
--
2.17.1

2023-04-22 00:06:57

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH V3 6/6] PCI: qcom: Add support for IPQ9574

On Fri, 21 Apr 2023 at 15:51, Devi Priya <[email protected]> wrote:
>
> The IPQ9574 platform has 4 Gen3 PCIe controllers: two single-lane
> and two dual-lane based on SNPS core 5.70a
> The Qcom IP rev is 1.27.0 and Synopsys IP rev is 5.80a
> Added a new compatible 'qcom,pcie-ipq9574' and 'ops_1_27_0'
> which reuses all the members of 'ops_2_9_0' except for the post_init
> as the SLV_ADDR_SPACE_SIZE configuration differs between 2_9_0
> and 1_27_0.
> Also, modified get_resources of 'ops 2_9_0' to get the clocks
> from the device tree and modelled the post init sequence as
> a common function to avoid code redundancy.
>
> Co-developed-by: Anusha Rao <[email protected]>
> Signed-off-by: Anusha Rao <[email protected]>
> Signed-off-by: Devi Priya <[email protected]>
> ---
> Changes in V3:
> - Rebased on top of linux-next/master
>
> drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++--------
> 1 file changed, 43 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 4ab30892f6ef..3682ecdead1f 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -107,6 +107,7 @@
>
> /* PARF_SLV_ADDR_SPACE_SIZE register value */
> #define SLV_ADDR_SPACE_SZ 0x10000000
> +#define SLV_ADDR_SPACE_SZ_1_27_0 0x08000000
>
> /* PARF_MHI_CLOCK_RESET_CTRL register fields */
> #define AHB_CLK_EN BIT(0)
> @@ -202,10 +203,10 @@ struct qcom_pcie_resources_2_7_0 {
> struct reset_control *rst;
> };
>
> -#define QCOM_PCIE_2_9_0_MAX_CLOCKS 5
> struct qcom_pcie_resources_2_9_0 {
> - struct clk_bulk_data clks[QCOM_PCIE_2_9_0_MAX_CLOCKS];
> + struct clk_bulk_data *clks;
> struct reset_control *rst;
> + int num_clks;
> };
>
> union qcom_pcie_resources {
> @@ -1050,17 +1051,10 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
> struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> struct dw_pcie *pci = pcie->pci;
> struct device *dev = pci->dev;
> - int ret;
>
> - res->clks[0].id = "iface";
> - res->clks[1].id = "axi_m";
> - res->clks[2].id = "axi_s";
> - res->clks[3].id = "axi_bridge";
> - res->clks[4].id = "rchng";
> -
> - ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
> - if (ret < 0)
> - return ret;
> + res->num_clks = devm_clk_bulk_get_all(dev, &res->clks);
> + if (res->clks < 0)
> + return res->num_clks;
>
> res->rst = devm_reset_control_array_get_exclusive(dev);
> if (IS_ERR(res->rst))
> @@ -1073,7 +1067,7 @@ static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
> {
> struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
>
> - clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
> + clk_bulk_disable_unprepare(res->num_clks, res->clks);
> }
>
> static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
> @@ -1102,19 +1096,16 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
>
> usleep_range(2000, 2500);
>
> - return clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
> + return clk_bulk_prepare_enable(res->num_clks, res->clks);
> }
>
> -static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> +static int qcom_pcie_post_init(struct qcom_pcie *pcie)
> {
> struct dw_pcie *pci = pcie->pci;
> u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> u32 val;
> int i;
>
> - writel(SLV_ADDR_SPACE_SZ,
> - pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> -
> val = readl(pcie->parf + PARF_PHY_CTRL);
> val &= ~PHY_TEST_PWR_DOWN;
> writel(val, pcie->parf + PARF_PHY_CTRL);
> @@ -1151,6 +1142,26 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> return 0;
> }
>
> +static int qcom_pcie_post_init_1_27_0(struct qcom_pcie *pcie)
> +{
> + writel(SLV_ADDR_SPACE_SZ_1_27_0,
> + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> +
> + qcom_pcie_post_init(pcie);
> +
> + return 0;
> +}
> +
> +static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> +{
> + writel(SLV_ADDR_SPACE_SZ,
> + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> +
> + qcom_pcie_post_init(pcie);
> +
> + return 0;
> +}

I'm not sure about moving the SLV_ADDR_SPACE_SIZE initialization from
init() to post_init(). Probably a better solution might be to have two
init() callbacks and to call the common function from both of them.

> +
> static int qcom_pcie_link_up(struct dw_pcie *pci)
> {
> u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> @@ -1291,6 +1302,15 @@ static const struct qcom_pcie_ops ops_2_9_0 = {
> .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> };
>
> +/* Qcom IP rev.: 1.27.0 Synopsys IP rev.: 5.80a */
> +static const struct qcom_pcie_ops ops_1_27_0 = {
> + .get_resources = qcom_pcie_get_resources_2_9_0,
> + .init = qcom_pcie_init_2_9_0,
> + .post_init = qcom_pcie_post_init_1_27_0,
> + .deinit = qcom_pcie_deinit_2_9_0,
> + .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> +};
> +
> static const struct qcom_pcie_cfg cfg_1_0_0 = {
> .ops = &ops_1_0_0,
> };
> @@ -1323,6 +1343,10 @@ static const struct qcom_pcie_cfg cfg_2_9_0 = {
> .ops = &ops_2_9_0,
> };
>
> +static const struct qcom_pcie_cfg cfg_1_27_0 = {
> + .ops = &ops_1_27_0,
> +};
> +
> static const struct dw_pcie_ops dw_pcie_ops = {
> .link_up = qcom_pcie_link_up,
> .start_link = qcom_pcie_start_link,
> @@ -1607,6 +1631,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> { .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
> + { .compatible = "qcom,pcie-ipq9574", .data = &cfg_1_27_0 },
> { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> { .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
> --
> 2.17.1
>


--
With best wishes
Dmitry

2023-04-22 00:17:01

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH V3 5/6] arm64: dts: qcom: ipq9574: Enable PCIe PHYs and controllers

On Fri, 21 Apr 2023 at 15:51, Devi Priya <[email protected]> wrote:
>
> Enable the PCIe controller and PHY nodes corresponding to
> RDP 433.
>
> Signed-off-by: Devi Priya <[email protected]>
> ---
> Changes in V3:
> - No change
>
> arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 62 +++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> index 7be578017bf7..3ae38cf327ea 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> @@ -8,6 +8,7 @@
>
> /dts-v1/;
>
> +#include <dt-bindings/gpio/gpio.h>
> #include "ipq9574.dtsi"
>
> / {
> @@ -43,6 +44,42 @@
> };
> };
>
> +&pcie1_phy {
> + status = "okay";
> +};
> +
> +&pcie1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie_1_pin>;
> +
> + perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;

Usually qcom PCIe hosts also define wake-gpios.

> + status = "okay";
> +};
> +
> +&pcie2_phy {
> + status = "okay";
> +};
> +
> +&pcie2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie_2_pin>;
> +
> + perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
> + status = "okay";
> +};
> +
> +&pcie3_phy {
> + status = "okay";
> +};
> +
> +&pcie3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie_3_pin>;
> +
> + perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
> + status = "okay";
> +};
> +
> &sdhc_1 {
> pinctrl-0 = <&sdc_default_state>;
> pinctrl-names = "default";
> @@ -60,6 +97,31 @@
> };
>
> &tlmm {
> +
> + pcie_1_pin: pcie-1-state {
> + pins = "gpio26";
> + function = "gpio";
> + drive-strength = <8>;
> + bias-pull-down;
> + output-low;

No clkreq and no wake gpios?

> + };
> +
> + pcie_2_pin: pcie-2-state {
> + pins = "gpio29";
> + function = "gpio";
> + drive-strength = <8>;
> + bias-pull-down;
> + output-low;
> + };
> +
> + pcie_3_pin: pcie-3-state {
> + pins = "gpio32";
> + function = "gpio";
> + drive-strength = <8>;
> + bias-pull-up;
> + output-low;
> + };
> +
> sdc_default_state: sdc-default-state {
> clk-pins {
> pins = "gpio5";
> --
> 2.17.1
>


--
With best wishes
Dmitry

2023-04-22 00:23:01

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH V3 4/6] arm64: dts: qcom: ipq9574: Add PCIe PHYs and controller nodes

On Fri, 21 Apr 2023 at 15:50, Devi Priya <[email protected]> wrote:
>
> Add PCIe0, PCIe1, PCIe2, PCIe3 (and corresponding PHY) devices
> found on IPQ9574 platform. The PCIe0 & PCIe1 are 1-lane Gen3
> host whereas PCIe2 & PCIe3 are 2-lane Gen3 host.
>
> Co-developed-by: Anusha Rao <[email protected]>
> Signed-off-by: Anusha Rao <[email protected]>
> Signed-off-by: Devi Priya <[email protected]>
> ---
> Changes in V3:
> - Fixed up the PCI I/O port ranges
>
> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 375 +++++++++++++++++++++++++-
> 1 file changed, 370 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index e757b57957cf..953a839a1141 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> @@ -6,8 +6,8 @@
> * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> -#include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/clock/qcom,ipq9574-gcc.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/reset/qcom,ipq9574-gcc.h>
>
> / {
> @@ -116,6 +116,58 @@
> #size-cells = <1>;
> ranges = <0 0 0 0xffffffff>;
>
> + pcie0_phy: phy@84000 {
> + compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
> + reg = <0x00084000 0x1000>;
> +
> + clocks = <&gcc GCC_PCIE0_AUX_CLK>,
> + <&gcc GCC_PCIE0_AHB_CLK>,
> + <&gcc GCC_ANOC_PCIE0_1LANE_M_CLK>,
> + <&gcc GCC_SNOC_PCIE0_1LANE_S_CLK>,
> + <&gcc GCC_PCIE0_PIPE_CLK>;
> + clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "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";
> +
> + };
> +
> + pcie2_phy: phy@8c000 {
> + compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> + reg = <0x0008c000 0x2000>;
> +
> + clocks = <&gcc GCC_PCIE2_AUX_CLK>,
> + <&gcc GCC_PCIE2_AHB_CLK>,
> + <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
> + <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>,
> + <&gcc GCC_PCIE2_PIPE_CLK>;
> + clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "pipe";
> +
> + assigned-clocks = <&gcc GCC_PCIE2_AUX_CLK>;
> + assigned-clock-rates = <20000000>;
> +
> + resets = <&gcc GCC_PCIE2_PHY_BCR>,
> + <&gcc GCC_PCIE2PHY_PHY_BCR>;
> + reset-names = "phy", "common";
> +
> + #clock-cells = <0>;
> + clock-output-names = "gcc_pcie2_pipe_clk_src";
> +
> + #phy-cells = <0>;
> + status = "disabled";
> +
> + };
> +
> rng: rng@e3000 {
> compatible = "qcom,prng-ee";
> reg = <0x000e3000 0x1000>;
> @@ -123,6 +175,58 @@
> clock-names = "core";
> };
>
> + pcie3_phy: phy@f4000 {
> + compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> + reg = <0x000f4000 0x2000>;
> +
> + clocks = <&gcc GCC_PCIE3_AUX_CLK>,
> + <&gcc GCC_PCIE3_AHB_CLK>,
> + <&gcc GCC_ANOC_PCIE3_2LANE_M_CLK>,
> + <&gcc GCC_SNOC_PCIE3_2LANE_S_CLK>,
> + <&gcc GCC_PCIE3_PIPE_CLK>;
> + clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "pipe";
> +
> + assigned-clocks = <&gcc GCC_PCIE3_AUX_CLK>;
> + assigned-clock-rates = <20000000>;
> +
> + resets = <&gcc GCC_PCIE3_PHY_BCR>,
> + <&gcc GCC_PCIE3PHY_PHY_BCR>;
> + reset-names = "phy", "common";
> +
> + #clock-cells = <0>;
> + clock-output-names = "gcc_pcie3_pipe_clk_src";
> +
> + #phy-cells = <0>;
> + status = "disabled";
> +
> + };
> +
> + pcie1_phy: phy@fc000 {
> + compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
> + reg = <0x000fc000 0x1000>;
> +
> + clocks = <&gcc GCC_PCIE1_AUX_CLK>,
> + <&gcc GCC_PCIE1_AHB_CLK>,
> + <&gcc GCC_ANOC_PCIE1_1LANE_M_CLK>,
> + <&gcc GCC_SNOC_PCIE1_1LANE_S_CLK>,
> + <&gcc GCC_PCIE1_PIPE_CLK>;
> + clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "pipe";
> +
> + assigned-clocks = <&gcc GCC_PCIE1_AUX_CLK>;
> + assigned-clock-rates = <20000000>;
> +
> + 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,ipq9574-tlmm";
> reg = <0x01000000 0x300000>;
> @@ -146,10 +250,10 @@
> reg = <0x01800000 0x80000>;
> clocks = <&xo_board_clk>,
> <&sleep_clk>,
> - <0>,
> - <0>,
> - <0>,
> - <0>,
> + <&pcie0_phy>,
> + <&pcie1_phy>,
> + <&pcie2_phy>,
> + <&pcie3_phy>,
> <0>;
> #clock-cells = <1>;
> #reset-cells = <1>;
> @@ -478,6 +582,267 @@
> status = "disabled";
> };
> };
> +
> + pcie1: pci@10000000 {
> + compatible = "qcom,pcie-ipq9574";
> + reg = <0x10000000 0xf1d>,
> + <0x10000F20 0xa8>,
> + <0x10001000 0x1000>,
> + <0x000F8000 0x4000>,
> + <0x10100000 0x1000>;
> + reg-names = "dbi", "elbi", "atu", "parf", "config";
> + device_type = "pci";
> + linux,pci-domain = <2>;
> + bus-range = <0x00 0xff>;
> + num-lanes = <1>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> +
> + ranges = <0x01000000 0x0 0x00000000 0x10200000 0x0 0x100000>, /* I/O */
> + <0x02000000 0x0 0x10300000 0x10300000 0x0 0x7d00000>; /* MEM */
> +
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 0x7>;
> + interrupt-map = <0 0 0 1 &intc 0 35 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> + <0 0 0 2 &intc 0 49 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> + <0 0 0 3 &intc 0 84 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
> + <0 0 0 4 &intc 0 85 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
> +

No iommu-map?

> + interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "global_irq";
> +
> + /* clocks and clock-names are used to enable the clock in CBCR */
> + clocks = <&gcc GCC_PCIE1_AHB_CLK>,
> + <&gcc GCC_PCIE1_AUX_CLK>,
> + <&gcc GCC_PCIE1_AXI_M_CLK>,
> + <&gcc GCC_PCIE1_AXI_S_CLK>,
> + <&gcc GCC_PCIE1_AXI_S_BRIDGE_CLK>,
> + <&gcc GCC_PCIE1_RCHNG_CLK>;
> + clock-names = "ahb",
> + "aux",
> + "axi_m",
> + "axi_s",
> + "axi_bridge",
> + "rchng";
> +
> + resets = <&gcc GCC_PCIE1_PIPE_ARES>,
> + <&gcc GCC_PCIE1_CORE_STICKY_ARES>,
> + <&gcc GCC_PCIE1_AXI_S_STICKY_ARES>,
> + <&gcc GCC_PCIE1_AXI_S_ARES>,
> + <&gcc GCC_PCIE1_AXI_M_STICKY_ARES>,
> + <&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";
> +
> + phys = <&pcie1_phy>;
> + phy-names = "pciephy";
> + msi-parent = <&v2m0>;
> + status = "disabled";
> + };
> +
> + pcie3: pci@18000000 {
> + compatible = "qcom,pcie-ipq9574";
> + reg = <0x18000000 0xf1d>,
> + <0x18000F20 0xa8>,
> + <0x18001000 0x1000>,
> + <0x000F0000 0x4000>,
> + <0x18100000 0x1000>;
> + reg-names = "dbi", "elbi", "atu", "parf", "config";
> + device_type = "pci";
> + linux,pci-domain = <4>;
> + bus-range = <0x00 0xff>;
> + num-lanes = <2>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> +
> + ranges = <0x01000000 0x0 0x00000000 0x18200000 0x0 0x100000>, /* I/O */
> + <0x02000000 0x0 0x18300000 0x18300000 0x0 0x7d00000>; /* MEM */
> +
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 0x7>;
> + interrupt-map = <0 0 0 1 &intc 0 189 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> + <0 0 0 2 &intc 0 190 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> + <0 0 0 3 &intc 0 191 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
> + <0 0 0 4 &intc 0 192 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
> +
> + interrupts = <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "global_irq";
> +
> + /* clocks and clock-names are used to enable the clock in CBCR */
> + clocks = <&gcc GCC_PCIE3_AHB_CLK>,
> + <&gcc GCC_PCIE3_AUX_CLK>,
> + <&gcc GCC_PCIE3_AXI_M_CLK>,
> + <&gcc GCC_PCIE3_AXI_S_CLK>,
> + <&gcc GCC_PCIE3_AXI_S_BRIDGE_CLK>,
> + <&gcc GCC_PCIE3_RCHNG_CLK>;
> + clock-names = "ahb",
> + "aux",
> + "axi_m",
> + "axi_s",
> + "axi_bridge",
> + "rchng";
> +
> + resets = <&gcc GCC_PCIE3_PIPE_ARES>,
> + <&gcc GCC_PCIE3_CORE_STICKY_ARES>,
> + <&gcc GCC_PCIE3_AXI_S_STICKY_ARES>,
> + <&gcc GCC_PCIE3_AXI_S_ARES>,
> + <&gcc GCC_PCIE3_AXI_M_STICKY_ARES>,
> + <&gcc GCC_PCIE3_AXI_M_ARES>,
> + <&gcc GCC_PCIE3_AUX_ARES>,
> + <&gcc GCC_PCIE3_AHB_ARES>;
> + reset-names = "pipe",
> + "sticky",
> + "axi_s_sticky",
> + "axi_s",
> + "axi_m_sticky",
> + "axi_m",
> + "aux",
> + "ahb";
> +
> + phys = <&pcie3_phy>;
> + phy-names = "pciephy";
> + msi-parent = <&v2m0>;
> + status = "disabled";
> + };
> +
> + pcie2: pci@20000000 {
> + compatible = "qcom,pcie-ipq9574";
> + reg = <0x20000000 0xf1d>,
> + <0x20000F20 0xa8>,
> + <0x20001000 0x1000>,
> + <0x00088000 0x4000>,
> + <0x20100000 0x1000>;
> + reg-names = "dbi", "elbi", "atu", "parf", "config";
> + device_type = "pci";
> + linux,pci-domain = <3>;
> + bus-range = <0x00 0xff>;
> + num-lanes = <2>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> +
> + ranges = <0x01000000 0x0 0x00000000 0x20200000 0x0 0x100000>, /* I/O */
> + <0x02000000 0x0 0x20300000 0x20300000 0x0 0x7d00000>; /* MEM */
> +
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 0x7>;
> + interrupt-map = <0 0 0 1 &intc 0 164 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> + <0 0 0 2 &intc 0 165 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> + <0 0 0 3 &intc 0 186 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
> + <0 0 0 4 &intc 0 187 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
> +
> + interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "global_irq";
> +
> + /* clocks and clock-names are used to enable the clock in CBCR */
> + clocks = <&gcc GCC_PCIE2_AHB_CLK>,
> + <&gcc GCC_PCIE2_AUX_CLK>,
> + <&gcc GCC_PCIE2_AXI_M_CLK>,
> + <&gcc GCC_PCIE2_AXI_S_CLK>,
> + <&gcc GCC_PCIE2_AXI_S_BRIDGE_CLK>,
> + <&gcc GCC_PCIE2_RCHNG_CLK>;
> + clock-names = "ahb",
> + "aux",
> + "axi_m",
> + "axi_s",
> + "axi_bridge",
> + "rchng";
> +
> + resets = <&gcc GCC_PCIE2_PIPE_ARES>,
> + <&gcc GCC_PCIE2_CORE_STICKY_ARES>,
> + <&gcc GCC_PCIE2_AXI_S_STICKY_ARES>,
> + <&gcc GCC_PCIE2_AXI_S_ARES>,
> + <&gcc GCC_PCIE2_AXI_M_STICKY_ARES>,
> + <&gcc GCC_PCIE2_AXI_M_ARES>,
> + <&gcc GCC_PCIE2_AUX_ARES>,
> + <&gcc GCC_PCIE2_AHB_ARES>;
> + reset-names = "pipe",
> + "sticky",
> + "axi_s_sticky",
> + "axi_s",
> + "axi_m_sticky",
> + "axi_m",
> + "aux",
> + "ahb";
> +
> + phys = <&pcie2_phy>;
> + phy-names = "pciephy";
> + msi-parent = <&v2m0>;
> + status = "disabled";
> + };
> +
> + pcie0: pci@28000000 {
> + compatible = "qcom,pcie-ipq9574";
> + reg = <0x28000000 0xf1d>,
> + <0x28000F20 0xa8>,
> + <0x28001000 0x1000>,
> + <0x00080000 0x4000>,
> + <0x28100000 0x1000>;
> + reg-names = "dbi", "elbi", "atu", "parf", "config";
> + device_type = "pci";
> + linux,pci-domain = <1>;
> + bus-range = <0x00 0xff>;
> + num-lanes = <1>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> +
> + ranges = <0x01000000 0x0 0x00000000 0x28200000 0x0 0x100000>, /* I/O */
> + <0x02000000 0x0 0x28300000 0x28300000 0x0 0x7d00000>; /* MEM */
> +
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 0x7>;
> + interrupt-map = <0 0 0 1 &intc 0 75 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> + <0 0 0 2 &intc 0 78 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> + <0 0 0 3 &intc 0 79 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
> + <0 0 0 4 &intc 0 83 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
> +
> + interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "global_irq";
> +
> + /* clocks and clock-names are used to enable the clock in CBCR */
> + clocks = <&gcc GCC_PCIE0_AHB_CLK>,
> + <&gcc GCC_PCIE0_AUX_CLK>,
> + <&gcc GCC_PCIE0_AXI_M_CLK>,
> + <&gcc GCC_PCIE0_AXI_S_CLK>,
> + <&gcc GCC_PCIE0_AXI_S_BRIDGE_CLK>,
> + <&gcc GCC_PCIE0_RCHNG_CLK>;
> + clock-names = "ahb",
> + "aux",
> + "axi_m",
> + "axi_s",
> + "axi_bridge",
> + "rchng";
> +
> + resets = <&gcc GCC_PCIE0_PIPE_ARES>,
> + <&gcc GCC_PCIE0_CORE_STICKY_ARES>,
> + <&gcc GCC_PCIE0_AXI_S_STICKY_ARES>,
> + <&gcc GCC_PCIE0_AXI_S_ARES>,
> + <&gcc GCC_PCIE0_AXI_M_STICKY_ARES>,
> + <&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";
> +
> + phys = <&pcie0_phy>;
> + phy-names = "pciephy";
> + msi-parent = <&v2m0>;
> + status = "disabled";
> + };
> +
> };
>
> timer {
> --
> 2.17.1
>


--
With best wishes
Dmitry

2023-04-22 01:10:18

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH V3 2/6] clk: qcom: gcc-ipq9574: Add PCIe pipe clocks

On Fri, 21 Apr 2023 at 15:50, Devi Priya <[email protected]> wrote:
>
> Add the PCIe pipe clocks needed for enabling PCIe in IPQ9574.
>
> Acked-by: Stephen Boyd <[email protected]>
> Co-developed-by: Anusha Rao <[email protected]>
> Signed-off-by: Anusha Rao <[email protected]>
> Signed-off-by: Devi Priya <[email protected]>
> ---
> Changes in V3:
> - Picked up the Acked-by tags
>
> drivers/clk/qcom/gcc-ipq9574.c | 76 ++++++++++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)


Reviewed-by: Dmitry Baryshkov <[email protected]>

--
With best wishes
Dmitry

2023-04-25 17:39:05

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] dt-bindings: PCI: qcom: Add IPQ9574

On Fri, Apr 21, 2023 at 06:19:35PM +0530, Devi Priya wrote:
> Add bindings for PCIe hosts on IPQ9574 platform and allow
> msi-parent property.

Go see the long discussion about why msi-parent with msi-map is wrong.
If something changed, explain that here.

>
> Signed-off-by: Devi Priya <[email protected]>
> ---
> Changes in V3:
> - Rebased on linux-next/master
>
> .../devicetree/bindings/pci/qcom,pcie.yaml | 40 +++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 81971be4e554..a92cecc5fe6f 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -26,6 +26,7 @@ properties:
> - qcom,pcie-ipq8064-v2
> - qcom,pcie-ipq8074
> - qcom,pcie-ipq8074-gen3
> + - qcom,pcie-ipq9574
> - qcom,pcie-msm8996
> - qcom,pcie-qcs404
> - qcom,pcie-sa8540p
> @@ -113,6 +114,8 @@ properties:
> power-domains:
> maxItems: 1
>
> + msi-parent: true
> +
> perst-gpios:
> description: GPIO controlled connection to PERST# signal
> maxItems: 1
> @@ -138,6 +141,8 @@ anyOf:
> - required:
> - msi-map
> - msi-map-mask
> + - required:
> + - msi-parent
>
> allOf:
> - $ref: /schemas/pci/pci-bus.yaml#
> @@ -171,6 +176,7 @@ allOf:
> enum:
> - qcom,pcie-ipq6018
> - qcom,pcie-ipq8074-gen3
> + - qcom,pcie-ipq9574
> then:
> properties:
> reg:
> @@ -382,6 +388,39 @@ allOf:
> - const: ahb # AHB Reset
> - const: axi_m_sticky # AXI Master Sticky reset
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,pcie-ipq9574
> + then:
> + properties:
> + clocks:
> + minItems: 6
> + maxItems: 6
> + clock-names:
> + items:
> + - const: ahb # AHB clock
> + - const: aux # Auxiliary clock
> + - const: axi_m # AXI Master clock
> + - const: axi_s # AXI Slave clock
> + - const: axi_bridge # AXI bridge clock
> + - const: rchng
> + resets:
> + minItems: 8
> + maxItems: 8
> + reset-names:
> + items:
> + - const: pipe # PIPE reset
> + - const: sticky # Core Sticky reset
> + - const: axi_s_sticky # AXI Slave Sticky reset
> + - const: axi_s # AXI Slave reset
> + - const: axi_m_sticky # AXI Master Sticky reset
> + - const: axi_m # AXI Master reset
> + - const: aux # AUX Reset
> + - const: ahb # AHB Reset
> +
> - if:
> properties:
> compatible:
> @@ -767,6 +806,7 @@ allOf:
> - qcom,pcie-ipq8064v2
> - qcom,pcie-ipq8074
> - qcom,pcie-ipq8074-gen3
> + - qcom,pcie-ipq9574
> - qcom,pcie-qcs404
> then:
> required:
> --
> 2.17.1
>

2023-05-02 06:06:02

by Devi Priya

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] dt-bindings: PCI: qcom: Add IPQ9574



On 4/25/2023 11:03 PM, Rob Herring wrote:
> On Fri, Apr 21, 2023 at 06:19:35PM +0530, Devi Priya wrote:
>> Add bindings for PCIe hosts on IPQ9574 platform and allow
>> msi-parent property.
>
> Go see the long discussion about why msi-parent with msi-map is wrong.
> If something changed, explain that here.
Sure, okay

Thanks,
Devi Priya
>
>>
>> Signed-off-by: Devi Priya <[email protected]>
>> ---
>> Changes in V3:
>> - Rebased on linux-next/master
>>
>> .../devicetree/bindings/pci/qcom,pcie.yaml | 40 +++++++++++++++++++
>> 1 file changed, 40 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> index 81971be4e554..a92cecc5fe6f 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> @@ -26,6 +26,7 @@ properties:
>> - qcom,pcie-ipq8064-v2
>> - qcom,pcie-ipq8074
>> - qcom,pcie-ipq8074-gen3
>> + - qcom,pcie-ipq9574
>> - qcom,pcie-msm8996
>> - qcom,pcie-qcs404
>> - qcom,pcie-sa8540p
>> @@ -113,6 +114,8 @@ properties:
>> power-domains:
>> maxItems: 1
>>
>> + msi-parent: true
>> +
>> perst-gpios:
>> description: GPIO controlled connection to PERST# signal
>> maxItems: 1
>> @@ -138,6 +141,8 @@ anyOf:
>> - required:
>> - msi-map
>> - msi-map-mask
>> + - required:
>> + - msi-parent
>>
>> allOf:
>> - $ref: /schemas/pci/pci-bus.yaml#
>> @@ -171,6 +176,7 @@ allOf:
>> enum:
>> - qcom,pcie-ipq6018
>> - qcom,pcie-ipq8074-gen3
>> + - qcom,pcie-ipq9574
>> then:
>> properties:
>> reg:
>> @@ -382,6 +388,39 @@ allOf:
>> - const: ahb # AHB Reset
>> - const: axi_m_sticky # AXI Master Sticky reset
>>
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,pcie-ipq9574
>> + then:
>> + properties:
>> + clocks:
>> + minItems: 6
>> + maxItems: 6
>> + clock-names:
>> + items:
>> + - const: ahb # AHB clock
>> + - const: aux # Auxiliary clock
>> + - const: axi_m # AXI Master clock
>> + - const: axi_s # AXI Slave clock
>> + - const: axi_bridge # AXI bridge clock
>> + - const: rchng
>> + resets:
>> + minItems: 8
>> + maxItems: 8
>> + reset-names:
>> + items:
>> + - const: pipe # PIPE reset
>> + - const: sticky # Core Sticky reset
>> + - const: axi_s_sticky # AXI Slave Sticky reset
>> + - const: axi_s # AXI Slave reset
>> + - const: axi_m_sticky # AXI Master Sticky reset
>> + - const: axi_m # AXI Master reset
>> + - const: aux # AUX Reset
>> + - const: ahb # AHB Reset
>> +
>> - if:
>> properties:
>> compatible:
>> @@ -767,6 +806,7 @@ allOf:
>> - qcom,pcie-ipq8064v2
>> - qcom,pcie-ipq8074
>> - qcom,pcie-ipq8074-gen3
>> + - qcom,pcie-ipq9574
>> - qcom,pcie-qcs404
>> then:
>> required:
>> --
>> 2.17.1
>>

2023-05-02 06:39:37

by Devi Priya

[permalink] [raw]
Subject: Re: [PATCH V3 6/6] PCI: qcom: Add support for IPQ9574



On 4/22/2023 5:35 AM, Dmitry Baryshkov wrote:
> On Fri, 21 Apr 2023 at 15:51, Devi Priya <[email protected]> wrote:
>>
>> The IPQ9574 platform has 4 Gen3 PCIe controllers: two single-lane
>> and two dual-lane based on SNPS core 5.70a
>> The Qcom IP rev is 1.27.0 and Synopsys IP rev is 5.80a
>> Added a new compatible 'qcom,pcie-ipq9574' and 'ops_1_27_0'
>> which reuses all the members of 'ops_2_9_0' except for the post_init
>> as the SLV_ADDR_SPACE_SIZE configuration differs between 2_9_0
>> and 1_27_0.
>> Also, modified get_resources of 'ops 2_9_0' to get the clocks
>> from the device tree and modelled the post init sequence as
>> a common function to avoid code redundancy.
>>
>> Co-developed-by: Anusha Rao <[email protected]>
>> Signed-off-by: Anusha Rao <[email protected]>
>> Signed-off-by: Devi Priya <[email protected]>
>> ---
>> Changes in V3:
>> - Rebased on top of linux-next/master
>>
>> drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++--------
>> 1 file changed, 43 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 4ab30892f6ef..3682ecdead1f 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -107,6 +107,7 @@
>>
>> /* PARF_SLV_ADDR_SPACE_SIZE register value */
>> #define SLV_ADDR_SPACE_SZ 0x10000000
>> +#define SLV_ADDR_SPACE_SZ_1_27_0 0x08000000
>>
>> /* PARF_MHI_CLOCK_RESET_CTRL register fields */
>> #define AHB_CLK_EN BIT(0)
>> @@ -202,10 +203,10 @@ struct qcom_pcie_resources_2_7_0 {
>> struct reset_control *rst;
>> };
>>
>> -#define QCOM_PCIE_2_9_0_MAX_CLOCKS 5
>> struct qcom_pcie_resources_2_9_0 {
>> - struct clk_bulk_data clks[QCOM_PCIE_2_9_0_MAX_CLOCKS];
>> + struct clk_bulk_data *clks;
>> struct reset_control *rst;
>> + int num_clks;
>> };
>>
>> union qcom_pcie_resources {
>> @@ -1050,17 +1051,10 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
>> struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
>> struct dw_pcie *pci = pcie->pci;
>> struct device *dev = pci->dev;
>> - int ret;
>>
>> - res->clks[0].id = "iface";
>> - res->clks[1].id = "axi_m";
>> - res->clks[2].id = "axi_s";
>> - res->clks[3].id = "axi_bridge";
>> - res->clks[4].id = "rchng";
>> -
>> - ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
>> - if (ret < 0)
>> - return ret;
>> + res->num_clks = devm_clk_bulk_get_all(dev, &res->clks);
>> + if (res->clks < 0)
>> + return res->num_clks;
>>
>> res->rst = devm_reset_control_array_get_exclusive(dev);
>> if (IS_ERR(res->rst))
>> @@ -1073,7 +1067,7 @@ static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
>> {
>> struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
>>
>> - clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
>> + clk_bulk_disable_unprepare(res->num_clks, res->clks);
>> }
>>
>> static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
>> @@ -1102,19 +1096,16 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
>>
>> usleep_range(2000, 2500);
>>
>> - return clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
>> + return clk_bulk_prepare_enable(res->num_clks, res->clks);
>> }
>>
>> -static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>> +static int qcom_pcie_post_init(struct qcom_pcie *pcie)
>> {
>> struct dw_pcie *pci = pcie->pci;
>> u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>> u32 val;
>> int i;
>>
>> - writel(SLV_ADDR_SPACE_SZ,
>> - pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
>> -
>> val = readl(pcie->parf + PARF_PHY_CTRL);
>> val &= ~PHY_TEST_PWR_DOWN;
>> writel(val, pcie->parf + PARF_PHY_CTRL);
>> @@ -1151,6 +1142,26 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>> return 0;
>> }
>>
>> +static int qcom_pcie_post_init_1_27_0(struct qcom_pcie *pcie)
>> +{
>> + writel(SLV_ADDR_SPACE_SZ_1_27_0,
>> + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
>> +
>> + qcom_pcie_post_init(pcie);
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>> +{
>> + writel(SLV_ADDR_SPACE_SZ,
>> + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
>> +
>> + qcom_pcie_post_init(pcie);
>> +
>> + return 0;
>> +}
>
> I'm not sure about moving the SLV_ADDR_SPACE_SIZE initialization from
> init() to post_init(). Probably a better solution might be to have two
> init() callbacks and to call the common function from both of them.
>
Hi Dmitry, Originally, the SLV_ADDR_SPACE_SIZE initialization was done
part of post_init() callback only and we haven't moved it from init() to
post_init().We have just added two post_init() callbacks to
handle the SLV_ADDR_SPACE_SIZE initialization accordingly for 1_27_0 and
2_9_0.

Thanks,
Devi Priya
>> +
>> static int qcom_pcie_link_up(struct dw_pcie *pci)
>> {
>> u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>> @@ -1291,6 +1302,15 @@ static const struct qcom_pcie_ops ops_2_9_0 = {
>> .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
>> };
>>
>> +/* Qcom IP rev.: 1.27.0 Synopsys IP rev.: 5.80a */
>> +static const struct qcom_pcie_ops ops_1_27_0 = {
>> + .get_resources = qcom_pcie_get_resources_2_9_0,
>> + .init = qcom_pcie_init_2_9_0,
>> + .post_init = qcom_pcie_post_init_1_27_0,
>> + .deinit = qcom_pcie_deinit_2_9_0,
>> + .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
>> +};
>> +
>> static const struct qcom_pcie_cfg cfg_1_0_0 = {
>> .ops = &ops_1_0_0,
>> };
>> @@ -1323,6 +1343,10 @@ static const struct qcom_pcie_cfg cfg_2_9_0 = {
>> .ops = &ops_2_9_0,
>> };
>>
>> +static const struct qcom_pcie_cfg cfg_1_27_0 = {
>> + .ops = &ops_1_27_0,
>> +};
>> +
>> static const struct dw_pcie_ops dw_pcie_ops = {
>> .link_up = qcom_pcie_link_up,
>> .start_link = qcom_pcie_start_link,
>> @@ -1607,6 +1631,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>> { .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
>> { .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
>> { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
>> + { .compatible = "qcom,pcie-ipq9574", .data = &cfg_1_27_0 },
>> { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
>> { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
>> { .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
>> --
>> 2.17.1
>>
>
>

2023-05-02 08:37:29

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH V3 6/6] PCI: qcom: Add support for IPQ9574

On Tue, 2 May 2023 at 09:36, Devi Priya <[email protected]> wrote:
>
>
>
> On 4/22/2023 5:35 AM, Dmitry Baryshkov wrote:
> > On Fri, 21 Apr 2023 at 15:51, Devi Priya <[email protected]> wrote:
> >>
> >> The IPQ9574 platform has 4 Gen3 PCIe controllers: two single-lane
> >> and two dual-lane based on SNPS core 5.70a
> >> The Qcom IP rev is 1.27.0 and Synopsys IP rev is 5.80a
> >> Added a new compatible 'qcom,pcie-ipq9574' and 'ops_1_27_0'
> >> which reuses all the members of 'ops_2_9_0' except for the post_init
> >> as the SLV_ADDR_SPACE_SIZE configuration differs between 2_9_0
> >> and 1_27_0.
> >> Also, modified get_resources of 'ops 2_9_0' to get the clocks
> >> from the device tree and modelled the post init sequence as
> >> a common function to avoid code redundancy.
> >>
> >> Co-developed-by: Anusha Rao <[email protected]>
> >> Signed-off-by: Anusha Rao <[email protected]>
> >> Signed-off-by: Devi Priya <[email protected]>
> >> ---
> >> Changes in V3:
> >> - Rebased on top of linux-next/master
> >>
> >> drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++--------
> >> 1 file changed, 43 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> >> index 4ab30892f6ef..3682ecdead1f 100644
> >> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> >> @@ -107,6 +107,7 @@
> >>
> >> /* PARF_SLV_ADDR_SPACE_SIZE register value */
> >> #define SLV_ADDR_SPACE_SZ 0x10000000
> >> +#define SLV_ADDR_SPACE_SZ_1_27_0 0x08000000
> >>
> >> /* PARF_MHI_CLOCK_RESET_CTRL register fields */
> >> #define AHB_CLK_EN BIT(0)
> >> @@ -202,10 +203,10 @@ struct qcom_pcie_resources_2_7_0 {
> >> struct reset_control *rst;
> >> };
> >>
> >> -#define QCOM_PCIE_2_9_0_MAX_CLOCKS 5
> >> struct qcom_pcie_resources_2_9_0 {
> >> - struct clk_bulk_data clks[QCOM_PCIE_2_9_0_MAX_CLOCKS];
> >> + struct clk_bulk_data *clks;
> >> struct reset_control *rst;
> >> + int num_clks;
> >> };
> >>
> >> union qcom_pcie_resources {
> >> @@ -1050,17 +1051,10 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
> >> struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> >> struct dw_pcie *pci = pcie->pci;
> >> struct device *dev = pci->dev;
> >> - int ret;
> >>
> >> - res->clks[0].id = "iface";
> >> - res->clks[1].id = "axi_m";
> >> - res->clks[2].id = "axi_s";
> >> - res->clks[3].id = "axi_bridge";
> >> - res->clks[4].id = "rchng";
> >> -
> >> - ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
> >> - if (ret < 0)
> >> - return ret;
> >> + res->num_clks = devm_clk_bulk_get_all(dev, &res->clks);
> >> + if (res->clks < 0)
> >> + return res->num_clks;
> >>
> >> res->rst = devm_reset_control_array_get_exclusive(dev);
> >> if (IS_ERR(res->rst))
> >> @@ -1073,7 +1067,7 @@ static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
> >> {
> >> struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> >>
> >> - clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
> >> + clk_bulk_disable_unprepare(res->num_clks, res->clks);
> >> }
> >>
> >> static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
> >> @@ -1102,19 +1096,16 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
> >>
> >> usleep_range(2000, 2500);
> >>
> >> - return clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
> >> + return clk_bulk_prepare_enable(res->num_clks, res->clks);
> >> }
> >>
> >> -static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> >> +static int qcom_pcie_post_init(struct qcom_pcie *pcie)
> >> {
> >> struct dw_pcie *pci = pcie->pci;
> >> u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> >> u32 val;
> >> int i;
> >>
> >> - writel(SLV_ADDR_SPACE_SZ,
> >> - pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> >> -
> >> val = readl(pcie->parf + PARF_PHY_CTRL);
> >> val &= ~PHY_TEST_PWR_DOWN;
> >> writel(val, pcie->parf + PARF_PHY_CTRL);
> >> @@ -1151,6 +1142,26 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> >> return 0;
> >> }
> >>
> >> +static int qcom_pcie_post_init_1_27_0(struct qcom_pcie *pcie)
> >> +{
> >> + writel(SLV_ADDR_SPACE_SZ_1_27_0,
> >> + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> >> +
> >> + qcom_pcie_post_init(pcie);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> >> +{
> >> + writel(SLV_ADDR_SPACE_SZ,
> >> + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> >> +
> >> + qcom_pcie_post_init(pcie);
> >> +
> >> + return 0;
> >> +}
> >
> > I'm not sure about moving the SLV_ADDR_SPACE_SIZE initialization from
> > init() to post_init(). Probably a better solution might be to have two
> > init() callbacks and to call the common function from both of them.
> >
> Hi Dmitry, Originally, the SLV_ADDR_SPACE_SIZE initialization was done
> part of post_init() callback only and we haven't moved it from init() to
> post_init().We have just added two post_init() callbacks to
> handle the SLV_ADDR_SPACE_SIZE initialization accordingly for 1_27_0 and
> 2_9_0.

Ack, I see then.

Reviewed-by: Dmitry Baryshkov <[email protected]>

--
With best wishes
Dmitry

2023-05-08 11:05:49

by Devi Priya

[permalink] [raw]
Subject: Re: [PATCH V3 4/6] arm64: dts: qcom: ipq9574: Add PCIe PHYs and controller nodes



On 4/22/2023 5:49 AM, Dmitry Baryshkov wrote:
> On Fri, 21 Apr 2023 at 15:50, Devi Priya <[email protected]> wrote:
>>
>> Add PCIe0, PCIe1, PCIe2, PCIe3 (and corresponding PHY) devices
>> found on IPQ9574 platform. The PCIe0 & PCIe1 are 1-lane Gen3
>> host whereas PCIe2 & PCIe3 are 2-lane Gen3 host.
>>
>> Co-developed-by: Anusha Rao <[email protected]>
>> Signed-off-by: Anusha Rao <[email protected]>
>> Signed-off-by: Devi Priya <[email protected]>
>> ---
>> Changes in V3:
>> - Fixed up the PCI I/O port ranges
>>
>> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 375 +++++++++++++++++++++++++-
>> 1 file changed, 370 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> index e757b57957cf..953a839a1141 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> @@ -6,8 +6,8 @@
>> * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
>> */
>>
>> -#include <dt-bindings/interrupt-controller/arm-gic.h>
>> #include <dt-bindings/clock/qcom,ipq9574-gcc.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> #include <dt-bindings/reset/qcom,ipq9574-gcc.h>
>>
>> / {
>> @@ -116,6 +116,58 @@
>> #size-cells = <1>;
>> ranges = <0 0 0 0xffffffff>;
>>
>> + pcie0_phy: phy@84000 {
>> + compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>> + reg = <0x00084000 0x1000>;
>> +
>> + clocks = <&gcc GCC_PCIE0_AUX_CLK>,
>> + <&gcc GCC_PCIE0_AHB_CLK>,
>> + <&gcc GCC_ANOC_PCIE0_1LANE_M_CLK>,
>> + <&gcc GCC_SNOC_PCIE0_1LANE_S_CLK>,
>> + <&gcc GCC_PCIE0_PIPE_CLK>;
>> + clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "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";
>> +
>> + };
>> +
>> + pcie2_phy: phy@8c000 {
>> + compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>> + reg = <0x0008c000 0x2000>;
>> +
>> + clocks = <&gcc GCC_PCIE2_AUX_CLK>,
>> + <&gcc GCC_PCIE2_AHB_CLK>,
>> + <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
>> + <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>,
>> + <&gcc GCC_PCIE2_PIPE_CLK>;
>> + clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "pipe";
>> +
>> + assigned-clocks = <&gcc GCC_PCIE2_AUX_CLK>;
>> + assigned-clock-rates = <20000000>;
>> +
>> + resets = <&gcc GCC_PCIE2_PHY_BCR>,
>> + <&gcc GCC_PCIE2PHY_PHY_BCR>;
>> + reset-names = "phy", "common";
>> +
>> + #clock-cells = <0>;
>> + clock-output-names = "gcc_pcie2_pipe_clk_src";
>> +
>> + #phy-cells = <0>;
>> + status = "disabled";
>> +
>> + };
>> +
>> rng: rng@e3000 {
>> compatible = "qcom,prng-ee";
>> reg = <0x000e3000 0x1000>;
>> @@ -123,6 +175,58 @@
>> clock-names = "core";
>> };
>>
>> + pcie3_phy: phy@f4000 {
>> + compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>> + reg = <0x000f4000 0x2000>;
>> +
>> + clocks = <&gcc GCC_PCIE3_AUX_CLK>,
>> + <&gcc GCC_PCIE3_AHB_CLK>,
>> + <&gcc GCC_ANOC_PCIE3_2LANE_M_CLK>,
>> + <&gcc GCC_SNOC_PCIE3_2LANE_S_CLK>,
>> + <&gcc GCC_PCIE3_PIPE_CLK>;
>> + clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "pipe";
>> +
>> + assigned-clocks = <&gcc GCC_PCIE3_AUX_CLK>;
>> + assigned-clock-rates = <20000000>;
>> +
>> + resets = <&gcc GCC_PCIE3_PHY_BCR>,
>> + <&gcc GCC_PCIE3PHY_PHY_BCR>;
>> + reset-names = "phy", "common";
>> +
>> + #clock-cells = <0>;
>> + clock-output-names = "gcc_pcie3_pipe_clk_src";
>> +
>> + #phy-cells = <0>;
>> + status = "disabled";
>> +
>> + };
>> +
>> + pcie1_phy: phy@fc000 {
>> + compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>> + reg = <0x000fc000 0x1000>;
>> +
>> + clocks = <&gcc GCC_PCIE1_AUX_CLK>,
>> + <&gcc GCC_PCIE1_AHB_CLK>,
>> + <&gcc GCC_ANOC_PCIE1_1LANE_M_CLK>,
>> + <&gcc GCC_SNOC_PCIE1_1LANE_S_CLK>,
>> + <&gcc GCC_PCIE1_PIPE_CLK>;
>> + clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane", "pipe";
>> +
>> + assigned-clocks = <&gcc GCC_PCIE1_AUX_CLK>;
>> + assigned-clock-rates = <20000000>;
>> +
>> + 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,ipq9574-tlmm";
>> reg = <0x01000000 0x300000>;
>> @@ -146,10 +250,10 @@
>> reg = <0x01800000 0x80000>;
>> clocks = <&xo_board_clk>,
>> <&sleep_clk>,
>> - <0>,
>> - <0>,
>> - <0>,
>> - <0>,
>> + <&pcie0_phy>,
>> + <&pcie1_phy>,
>> + <&pcie2_phy>,
>> + <&pcie3_phy>,
>> <0>;
>> #clock-cells = <1>;
>> #reset-cells = <1>;
>> @@ -478,6 +582,267 @@
>> status = "disabled";
>> };
>> };
>> +
>> + pcie1: pci@10000000 {
>> + compatible = "qcom,pcie-ipq9574";
>> + reg = <0x10000000 0xf1d>,
>> + <0x10000F20 0xa8>,
>> + <0x10001000 0x1000>,
>> + <0x000F8000 0x4000>,
>> + <0x10100000 0x1000>;
>> + reg-names = "dbi", "elbi", "atu", "parf", "config";
>> + device_type = "pci";
>> + linux,pci-domain = <2>;
>> + bus-range = <0x00 0xff>;
>> + num-lanes = <1>;
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> +
>> + ranges = <0x01000000 0x0 0x00000000 0x10200000 0x0 0x100000>, /* I/O */
>> + <0x02000000 0x0 0x10300000 0x10300000 0x0 0x7d00000>; /* MEM */
>> +
>> + #interrupt-cells = <1>;
>> + interrupt-map-mask = <0 0 0 0x7>;
>> + interrupt-map = <0 0 0 1 &intc 0 35 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>> + <0 0 0 2 &intc 0 49 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>> + <0 0 0 3 &intc 0 84 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
>> + <0 0 0 4 &intc 0 85 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>> +
>
> No iommu-map?
We do not enable the IOMMU stage1 translation for PCIe and the registers
have secure access only from TrustZone (It enables only stage2 for
Access control)

Thanks,
Devi Priya
>
>> + interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "global_irq";
>> +
>> + /* clocks and clock-names are used to enable the clock in CBCR */
>> + clocks = <&gcc GCC_PCIE1_AHB_CLK>,
>> + <&gcc GCC_PCIE1_AUX_CLK>,
>> + <&gcc GCC_PCIE1_AXI_M_CLK>,
>> + <&gcc GCC_PCIE1_AXI_S_CLK>,
>> + <&gcc GCC_PCIE1_AXI_S_BRIDGE_CLK>,
>> + <&gcc GCC_PCIE1_RCHNG_CLK>;
>> + clock-names = "ahb",
>> + "aux",
>> + "axi_m",
>> + "axi_s",
>> + "axi_bridge",
>> + "rchng";
>> +
>> + resets = <&gcc GCC_PCIE1_PIPE_ARES>,
>> + <&gcc GCC_PCIE1_CORE_STICKY_ARES>,
>> + <&gcc GCC_PCIE1_AXI_S_STICKY_ARES>,
>> + <&gcc GCC_PCIE1_AXI_S_ARES>,
>> + <&gcc GCC_PCIE1_AXI_M_STICKY_ARES>,
>> + <&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";
>> +
>> + phys = <&pcie1_phy>;
>> + phy-names = "pciephy";
>> + msi-parent = <&v2m0>;
>> + status = "disabled";
>> + };
>> +
>> + pcie3: pci@18000000 {
>> + compatible = "qcom,pcie-ipq9574";
>> + reg = <0x18000000 0xf1d>,
>> + <0x18000F20 0xa8>,
>> + <0x18001000 0x1000>,
>> + <0x000F0000 0x4000>,
>> + <0x18100000 0x1000>;
>> + reg-names = "dbi", "elbi", "atu", "parf", "config";
>> + device_type = "pci";
>> + linux,pci-domain = <4>;
>> + bus-range = <0x00 0xff>;
>> + num-lanes = <2>;
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> +
>> + ranges = <0x01000000 0x0 0x00000000 0x18200000 0x0 0x100000>, /* I/O */
>> + <0x02000000 0x0 0x18300000 0x18300000 0x0 0x7d00000>; /* MEM */
>> +
>> + #interrupt-cells = <1>;
>> + interrupt-map-mask = <0 0 0 0x7>;
>> + interrupt-map = <0 0 0 1 &intc 0 189 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>> + <0 0 0 2 &intc 0 190 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>> + <0 0 0 3 &intc 0 191 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
>> + <0 0 0 4 &intc 0 192 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>> +
>> + interrupts = <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "global_irq";
>> +
>> + /* clocks and clock-names are used to enable the clock in CBCR */
>> + clocks = <&gcc GCC_PCIE3_AHB_CLK>,
>> + <&gcc GCC_PCIE3_AUX_CLK>,
>> + <&gcc GCC_PCIE3_AXI_M_CLK>,
>> + <&gcc GCC_PCIE3_AXI_S_CLK>,
>> + <&gcc GCC_PCIE3_AXI_S_BRIDGE_CLK>,
>> + <&gcc GCC_PCIE3_RCHNG_CLK>;
>> + clock-names = "ahb",
>> + "aux",
>> + "axi_m",
>> + "axi_s",
>> + "axi_bridge",
>> + "rchng";
>> +
>> + resets = <&gcc GCC_PCIE3_PIPE_ARES>,
>> + <&gcc GCC_PCIE3_CORE_STICKY_ARES>,
>> + <&gcc GCC_PCIE3_AXI_S_STICKY_ARES>,
>> + <&gcc GCC_PCIE3_AXI_S_ARES>,
>> + <&gcc GCC_PCIE3_AXI_M_STICKY_ARES>,
>> + <&gcc GCC_PCIE3_AXI_M_ARES>,
>> + <&gcc GCC_PCIE3_AUX_ARES>,
>> + <&gcc GCC_PCIE3_AHB_ARES>;
>> + reset-names = "pipe",
>> + "sticky",
>> + "axi_s_sticky",
>> + "axi_s",
>> + "axi_m_sticky",
>> + "axi_m",
>> + "aux",
>> + "ahb";
>> +
>> + phys = <&pcie3_phy>;
>> + phy-names = "pciephy";
>> + msi-parent = <&v2m0>;
>> + status = "disabled";
>> + };
>> +
>> + pcie2: pci@20000000 {
>> + compatible = "qcom,pcie-ipq9574";
>> + reg = <0x20000000 0xf1d>,
>> + <0x20000F20 0xa8>,
>> + <0x20001000 0x1000>,
>> + <0x00088000 0x4000>,
>> + <0x20100000 0x1000>;
>> + reg-names = "dbi", "elbi", "atu", "parf", "config";
>> + device_type = "pci";
>> + linux,pci-domain = <3>;
>> + bus-range = <0x00 0xff>;
>> + num-lanes = <2>;
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> +
>> + ranges = <0x01000000 0x0 0x00000000 0x20200000 0x0 0x100000>, /* I/O */
>> + <0x02000000 0x0 0x20300000 0x20300000 0x0 0x7d00000>; /* MEM */
>> +
>> + #interrupt-cells = <1>;
>> + interrupt-map-mask = <0 0 0 0x7>;
>> + interrupt-map = <0 0 0 1 &intc 0 164 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>> + <0 0 0 2 &intc 0 165 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>> + <0 0 0 3 &intc 0 186 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
>> + <0 0 0 4 &intc 0 187 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>> +
>> + interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "global_irq";
>> +
>> + /* clocks and clock-names are used to enable the clock in CBCR */
>> + clocks = <&gcc GCC_PCIE2_AHB_CLK>,
>> + <&gcc GCC_PCIE2_AUX_CLK>,
>> + <&gcc GCC_PCIE2_AXI_M_CLK>,
>> + <&gcc GCC_PCIE2_AXI_S_CLK>,
>> + <&gcc GCC_PCIE2_AXI_S_BRIDGE_CLK>,
>> + <&gcc GCC_PCIE2_RCHNG_CLK>;
>> + clock-names = "ahb",
>> + "aux",
>> + "axi_m",
>> + "axi_s",
>> + "axi_bridge",
>> + "rchng";
>> +
>> + resets = <&gcc GCC_PCIE2_PIPE_ARES>,
>> + <&gcc GCC_PCIE2_CORE_STICKY_ARES>,
>> + <&gcc GCC_PCIE2_AXI_S_STICKY_ARES>,
>> + <&gcc GCC_PCIE2_AXI_S_ARES>,
>> + <&gcc GCC_PCIE2_AXI_M_STICKY_ARES>,
>> + <&gcc GCC_PCIE2_AXI_M_ARES>,
>> + <&gcc GCC_PCIE2_AUX_ARES>,
>> + <&gcc GCC_PCIE2_AHB_ARES>;
>> + reset-names = "pipe",
>> + "sticky",
>> + "axi_s_sticky",
>> + "axi_s",
>> + "axi_m_sticky",
>> + "axi_m",
>> + "aux",
>> + "ahb";
>> +
>> + phys = <&pcie2_phy>;
>> + phy-names = "pciephy";
>> + msi-parent = <&v2m0>;
>> + status = "disabled";
>> + };
>> +
>> + pcie0: pci@28000000 {
>> + compatible = "qcom,pcie-ipq9574";
>> + reg = <0x28000000 0xf1d>,
>> + <0x28000F20 0xa8>,
>> + <0x28001000 0x1000>,
>> + <0x00080000 0x4000>,
>> + <0x28100000 0x1000>;
>> + reg-names = "dbi", "elbi", "atu", "parf", "config";
>> + device_type = "pci";
>> + linux,pci-domain = <1>;
>> + bus-range = <0x00 0xff>;
>> + num-lanes = <1>;
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> +
>> + ranges = <0x01000000 0x0 0x00000000 0x28200000 0x0 0x100000>, /* I/O */
>> + <0x02000000 0x0 0x28300000 0x28300000 0x0 0x7d00000>; /* MEM */
>> +
>> + #interrupt-cells = <1>;
>> + interrupt-map-mask = <0 0 0 0x7>;
>> + interrupt-map = <0 0 0 1 &intc 0 75 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>> + <0 0 0 2 &intc 0 78 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>> + <0 0 0 3 &intc 0 79 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
>> + <0 0 0 4 &intc 0 83 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>> +
>> + interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "global_irq";
>> +
>> + /* clocks and clock-names are used to enable the clock in CBCR */
>> + clocks = <&gcc GCC_PCIE0_AHB_CLK>,
>> + <&gcc GCC_PCIE0_AUX_CLK>,
>> + <&gcc GCC_PCIE0_AXI_M_CLK>,
>> + <&gcc GCC_PCIE0_AXI_S_CLK>,
>> + <&gcc GCC_PCIE0_AXI_S_BRIDGE_CLK>,
>> + <&gcc GCC_PCIE0_RCHNG_CLK>;
>> + clock-names = "ahb",
>> + "aux",
>> + "axi_m",
>> + "axi_s",
>> + "axi_bridge",
>> + "rchng";
>> +
>> + resets = <&gcc GCC_PCIE0_PIPE_ARES>,
>> + <&gcc GCC_PCIE0_CORE_STICKY_ARES>,
>> + <&gcc GCC_PCIE0_AXI_S_STICKY_ARES>,
>> + <&gcc GCC_PCIE0_AXI_S_ARES>,
>> + <&gcc GCC_PCIE0_AXI_M_STICKY_ARES>,
>> + <&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";
>> +
>> + phys = <&pcie0_phy>;
>> + phy-names = "pciephy";
>> + msi-parent = <&v2m0>;
>> + status = "disabled";
>> + };
>> +
>> };
>>
>> timer {
>> --
>> 2.17.1
>>
>
>

2023-05-08 11:07:07

by Devi Priya

[permalink] [raw]
Subject: Re: [PATCH V3 5/6] arm64: dts: qcom: ipq9574: Enable PCIe PHYs and controllers



On 4/22/2023 5:43 AM, Dmitry Baryshkov wrote:
> On Fri, 21 Apr 2023 at 15:51, Devi Priya <[email protected]> wrote:
>>
>> Enable the PCIe controller and PHY nodes corresponding to
>> RDP 433.
>>
>> Signed-off-by: Devi Priya <[email protected]>
>> ---
>> Changes in V3:
>> - No change
>>
>> arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 62 +++++++++++++++++++++
>> 1 file changed, 62 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>> index 7be578017bf7..3ae38cf327ea 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>> @@ -8,6 +8,7 @@
>>
>> /dts-v1/;
>>
>> +#include <dt-bindings/gpio/gpio.h>
>> #include "ipq9574.dtsi"
>>
>> / {
>> @@ -43,6 +44,42 @@
>> };
>> };
>>
>> +&pcie1_phy {
>> + status = "okay";
>> +};
>> +
>> +&pcie1 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pcie_1_pin>;
>> +
>> + perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
>
> Usually qcom PCIe hosts also define wake-gpios.
In IPQ9574, we do not have hot plug support and host always starts the
enumeration for the device. Hence no wake pin is required.
>
>> + status = "okay";
>> +};
>> +
>> +&pcie2_phy {
>> + status = "okay";
>> +};
>> +
>> +&pcie2 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pcie_2_pin>;
>> +
>> + perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
>> + status = "okay";
>> +};
>> +
>> +&pcie3_phy {
>> + status = "okay";
>> +};
>> +
>> +&pcie3 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pcie_3_pin>;
>> +
>> + perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
>> + status = "okay";
>> +};
>> +
>> &sdhc_1 {
>> pinctrl-0 = <&sdc_default_state>;
>> pinctrl-names = "default";
>> @@ -60,6 +97,31 @@
>> };
>>
>> &tlmm {
>> +
>> + pcie_1_pin: pcie-1-state {
>> + pins = "gpio26";
>> + function = "gpio";
>> + drive-strength = <8>;
>> + bias-pull-down;
>> + output-low;
>
> No clkreq and no wake gpios?
We do not use any PCIe low power states and link is always in L0.

Thanks,
Devi Priya
>
>> + };
>> +
>> + pcie_2_pin: pcie-2-state {
>> + pins = "gpio29";
>> + function = "gpio";
>> + drive-strength = <8>;
>> + bias-pull-down;
>> + output-low;
>> + };
>> +
>> + pcie_3_pin: pcie-3-state {
>> + pins = "gpio32";
>> + function = "gpio";
>> + drive-strength = <8>;
>> + bias-pull-up;
>> + output-low;
>> + };
>> +
>> sdc_default_state: sdc-default-state {
>> clk-pins {
>> pins = "gpio5";
>> --
>> 2.17.1
>>
>
>

2023-05-08 11:59:12

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH V3 5/6] arm64: dts: qcom: ipq9574: Enable PCIe PHYs and controllers

On 08/05/2023 13:55, Devi Priya wrote:
>
>
> On 4/22/2023 5:43 AM, Dmitry Baryshkov wrote:
>> On Fri, 21 Apr 2023 at 15:51, Devi Priya <[email protected]>
>> wrote:
>>>
>>> Enable the PCIe controller and PHY nodes corresponding to
>>> RDP 433.
>>>
>>> Signed-off-by: Devi Priya <[email protected]>
>>> ---
>>>   Changes in V3:
>>>          - No change
>>>
>>>   arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 62 +++++++++++++++++++++
>>>   1 file changed, 62 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>>> b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>>> index 7be578017bf7..3ae38cf327ea 100644
>>> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>>> @@ -8,6 +8,7 @@
>>>
>>>   /dts-v1/;
>>>
>>> +#include <dt-bindings/gpio/gpio.h>
>>>   #include "ipq9574.dtsi"
>>>
>>>   / {
>>> @@ -43,6 +44,42 @@
>>>          };
>>>   };
>>>
>>> +&pcie1_phy {
>>> +       status = "okay";
>>> +};
>>> +
>>> +&pcie1 {
>>> +       pinctrl-names = "default";
>>> +       pinctrl-0 = <&pcie_1_pin>;
>>> +
>>> +       perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
>>
>> Usually qcom PCIe hosts also define wake-gpios.
> In IPQ9574, we do not have hot plug support and host always starts the
> enumeration for the device. Hence no wake pin is required.

None of the qcom PCIe hosts support hotplug, if I remember correctly.
This is not a reason not to describe the hardware.

>>
>>> +       status = "okay";
>>> +};
>>> +
>>> +&pcie2_phy {
>>> +       status = "okay";
>>> +};
>>> +
>>> +&pcie2 {
>>> +       pinctrl-names = "default";
>>> +       pinctrl-0 = <&pcie_2_pin>;
>>> +
>>> +       perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
>>> +       status = "okay";
>>> +};
>>> +
>>> +&pcie3_phy {
>>> +       status = "okay";
>>> +};
>>> +
>>> +&pcie3 {
>>> +       pinctrl-names = "default";
>>> +       pinctrl-0 = <&pcie_3_pin>;
>>> +
>>> +       perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
>>> +       status = "okay";
>>> +};
>>> +
>>>   &sdhc_1 {
>>>          pinctrl-0 = <&sdc_default_state>;
>>>          pinctrl-names = "default";
>>> @@ -60,6 +97,31 @@
>>>   };
>>>
>>>   &tlmm {
>>> +
>>> +       pcie_1_pin: pcie-1-state {
>>> +               pins = "gpio26";
>>> +               function = "gpio";
>>> +               drive-strength = <8>;
>>> +               bias-pull-down;
>>> +               output-low;
>>
>> No clkreq and no wake gpios?
> We do not use any PCIe low power states and link is always in L0.

Again. We = software. Please describe the hardware here.

>
> Thanks,
> Devi Priya
>>
>>> +       };
>>> +
>>> +       pcie_2_pin: pcie-2-state {
>>> +               pins = "gpio29";
>>> +               function = "gpio";
>>> +               drive-strength = <8>;
>>> +               bias-pull-down;
>>> +               output-low;
>>> +       };
>>> +
>>> +       pcie_3_pin: pcie-3-state {
>>> +               pins = "gpio32";
>>> +               function = "gpio";
>>> +               drive-strength = <8>;
>>> +               bias-pull-up;
>>> +               output-low;
>>> +       };
>>> +
>>>          sdc_default_state: sdc-default-state {
>>>                  clk-pins {
>>>                          pins = "gpio5";
>>> --
>>> 2.17.1
>>>
>>
>>

--
With best wishes
Dmitry

2023-05-08 12:06:16

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH V3 4/6] arm64: dts: qcom: ipq9574: Add PCIe PHYs and controller nodes

On 08/05/2023 13:53, Devi Priya wrote:
>
>
> On 4/22/2023 5:49 AM, Dmitry Baryshkov wrote:
>> On Fri, 21 Apr 2023 at 15:50, Devi Priya <[email protected]>
>> wrote:
>>>
>>> Add PCIe0, PCIe1, PCIe2, PCIe3 (and corresponding PHY) devices
>>> found on IPQ9574 platform. The PCIe0 & PCIe1 are 1-lane Gen3
>>> host whereas PCIe2 & PCIe3 are 2-lane Gen3 host.
>>>
>>> Co-developed-by: Anusha Rao <[email protected]>
>>> Signed-off-by: Anusha Rao <[email protected]>
>>> Signed-off-by: Devi Priya <[email protected]>
>>> ---
>>>   Changes in V3:
>>>          - Fixed up the PCI I/O port ranges
>>>
>>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi | 375 +++++++++++++++++++++++++-
>>>   1 file changed, 370 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>> b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>> index e757b57957cf..953a839a1141 100644
>>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>> @@ -6,8 +6,8 @@
>>>    * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights
>>> reserved.
>>>    */
>>>
>>> -#include <dt-bindings/interrupt-controller/arm-gic.h>
>>>   #include <dt-bindings/clock/qcom,ipq9574-gcc.h>
>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>>   #include <dt-bindings/reset/qcom,ipq9574-gcc.h>
>>>
>>>   / {
>>> @@ -116,6 +116,58 @@
>>>                  #size-cells = <1>;
>>>                  ranges = <0 0 0 0xffffffff>;
>>>
>>> +               pcie0_phy: phy@84000 {
>>> +                       compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>>> +                       reg = <0x00084000 0x1000>;
>>> +
>>> +                       clocks = <&gcc GCC_PCIE0_AUX_CLK>,
>>> +                                <&gcc GCC_PCIE0_AHB_CLK>,
>>> +                                <&gcc GCC_ANOC_PCIE0_1LANE_M_CLK>,
>>> +                                <&gcc GCC_SNOC_PCIE0_1LANE_S_CLK>,
>>> +                                <&gcc GCC_PCIE0_PIPE_CLK>;
>>> +                       clock-names = "aux", "cfg_ahb", "anoc_lane",
>>> "snoc_lane", "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";
>>> +
>>> +               };
>>> +
>>> +               pcie2_phy: phy@8c000 {
>>> +                       compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>>> +                       reg = <0x0008c000 0x2000>;
>>> +
>>> +                       clocks = <&gcc GCC_PCIE2_AUX_CLK>,
>>> +                                <&gcc GCC_PCIE2_AHB_CLK>,
>>> +                                <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
>>> +                                <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>,
>>> +                                <&gcc GCC_PCIE2_PIPE_CLK>;
>>> +                       clock-names = "aux", "cfg_ahb", "anoc_lane",
>>> "snoc_lane", "pipe";
>>> +
>>> +                       assigned-clocks = <&gcc GCC_PCIE2_AUX_CLK>;
>>> +                       assigned-clock-rates = <20000000>;
>>> +
>>> +                       resets = <&gcc GCC_PCIE2_PHY_BCR>,
>>> +                                <&gcc GCC_PCIE2PHY_PHY_BCR>;
>>> +                       reset-names = "phy", "common";
>>> +
>>> +                       #clock-cells = <0>;
>>> +                       clock-output-names = "gcc_pcie2_pipe_clk_src";
>>> +
>>> +                       #phy-cells = <0>;
>>> +                       status = "disabled";
>>> +
>>> +               };
>>> +
>>>                  rng: rng@e3000 {
>>>                          compatible = "qcom,prng-ee";
>>>                          reg = <0x000e3000 0x1000>;
>>> @@ -123,6 +175,58 @@
>>>                          clock-names = "core";
>>>                  };
>>>
>>> +               pcie3_phy: phy@f4000 {
>>> +                       compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>>> +                       reg = <0x000f4000 0x2000>;
>>> +
>>> +                       clocks = <&gcc GCC_PCIE3_AUX_CLK>,
>>> +                                <&gcc GCC_PCIE3_AHB_CLK>,
>>> +                                <&gcc GCC_ANOC_PCIE3_2LANE_M_CLK>,
>>> +                                <&gcc GCC_SNOC_PCIE3_2LANE_S_CLK>,
>>> +                                <&gcc GCC_PCIE3_PIPE_CLK>;
>>> +                       clock-names = "aux", "cfg_ahb", "anoc_lane",
>>> "snoc_lane", "pipe";
>>> +
>>> +                       assigned-clocks = <&gcc GCC_PCIE3_AUX_CLK>;
>>> +                       assigned-clock-rates = <20000000>;
>>> +
>>> +                       resets = <&gcc GCC_PCIE3_PHY_BCR>,
>>> +                                <&gcc GCC_PCIE3PHY_PHY_BCR>;
>>> +                       reset-names = "phy", "common";
>>> +
>>> +                       #clock-cells = <0>;
>>> +                       clock-output-names = "gcc_pcie3_pipe_clk_src";
>>> +
>>> +                       #phy-cells = <0>;
>>> +                       status = "disabled";
>>> +
>>> +               };
>>> +
>>> +               pcie1_phy: phy@fc000 {
>>> +                       compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>>> +                       reg = <0x000fc000 0x1000>;
>>> +
>>> +                       clocks = <&gcc GCC_PCIE1_AUX_CLK>,
>>> +                                <&gcc GCC_PCIE1_AHB_CLK>,
>>> +                                <&gcc GCC_ANOC_PCIE1_1LANE_M_CLK>,
>>> +                                <&gcc GCC_SNOC_PCIE1_1LANE_S_CLK>,
>>> +                                <&gcc GCC_PCIE1_PIPE_CLK>;
>>> +                       clock-names = "aux", "cfg_ahb", "anoc_lane",
>>> "snoc_lane", "pipe";
>>> +
>>> +                       assigned-clocks = <&gcc GCC_PCIE1_AUX_CLK>;
>>> +                       assigned-clock-rates = <20000000>;
>>> +
>>> +                       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,ipq9574-tlmm";
>>>                          reg = <0x01000000 0x300000>;
>>> @@ -146,10 +250,10 @@
>>>                          reg = <0x01800000 0x80000>;
>>>                          clocks = <&xo_board_clk>,
>>>                                   <&sleep_clk>,
>>> -                                <0>,
>>> -                                <0>,
>>> -                                <0>,
>>> -                                <0>,
>>> +                                <&pcie0_phy>,
>>> +                                <&pcie1_phy>,
>>> +                                <&pcie2_phy>,
>>> +                                <&pcie3_phy>,
>>>                                   <0>;
>>>                          #clock-cells = <1>;
>>>                          #reset-cells = <1>;
>>> @@ -478,6 +582,267 @@
>>>                                  status = "disabled";
>>>                          };
>>>                  };
>>> +
>>> +               pcie1: pci@10000000 {
>>> +                       compatible = "qcom,pcie-ipq9574";
>>> +                       reg =  <0x10000000 0xf1d>,
>>> +                              <0x10000F20 0xa8>,
>>> +                              <0x10001000 0x1000>,
>>> +                              <0x000F8000 0x4000>,
>>> +                              <0x10100000 0x1000>;
>>> +                       reg-names = "dbi", "elbi", "atu", "parf",
>>> "config";
>>> +                       device_type = "pci";
>>> +                       linux,pci-domain = <2>;
>>> +                       bus-range = <0x00 0xff>;
>>> +                       num-lanes = <1>;
>>> +                       #address-cells = <3>;
>>> +                       #size-cells = <2>;
>>> +
>>> +                       ranges = <0x01000000 0x0 0x00000000
>>> 0x10200000 0x0 0x100000>,  /* I/O */
>>> +                                <0x02000000 0x0 0x10300000
>>> 0x10300000 0x0 0x7d00000>; /* MEM */
>>> +
>>> +                       #interrupt-cells = <1>;
>>> +                       interrupt-map-mask = <0 0 0 0x7>;
>>> +                       interrupt-map = <0 0 0 1 &intc 0 35
>>> IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>>> +                                       <0 0 0 2 &intc 0 49
>>> IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>>> +                                       <0 0 0 3 &intc 0 84
>>> IRQ_TYPE_LEVEL_HIGH>, /* int_c */
>>> +                                       <0 0 0 4 &intc 0 85
>>> IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>>> +
>>
>> No iommu-map?
> We do not enable the IOMMU stage1 translation for PCIe and the registers
> have secure access only from TrustZone (It enables only stage2 for
> Access control)

So, no SMMU protection for PCIe transactions? This sounds like a step
backwards.

--
With best wishes
Dmitry

2023-05-08 12:30:36

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH V3 6/6] PCI: qcom: Add support for IPQ9574

On Fri, Apr 21, 2023 at 06:19:38PM +0530, Devi Priya wrote:
> The IPQ9574 platform has 4 Gen3 PCIe controllers: two single-lane
> and two dual-lane based on SNPS core 5.70a
> The Qcom IP rev is 1.27.0 and Synopsys IP rev is 5.80a
> Added a new compatible 'qcom,pcie-ipq9574' and 'ops_1_27_0'
> which reuses all the members of 'ops_2_9_0' except for the post_init
> as the SLV_ADDR_SPACE_SIZE configuration differs between 2_9_0
> and 1_27_0.
> Also, modified get_resources of 'ops 2_9_0' to get the clocks
> from the device tree and modelled the post init sequence as
> a common function to avoid code redundancy.
>
> Co-developed-by: Anusha Rao <[email protected]>
> Signed-off-by: Anusha Rao <[email protected]>
> Signed-off-by: Devi Priya <[email protected]>

One comment below. With that fixed,

Reviewed-by: Manivannan Sadhasivam <[email protected]>

- Mani

> ---
> Changes in V3:
> - Rebased on top of linux-next/master
>
> drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++--------
> 1 file changed, 43 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 4ab30892f6ef..3682ecdead1f 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -107,6 +107,7 @@
>
> /* PARF_SLV_ADDR_SPACE_SIZE register value */
> #define SLV_ADDR_SPACE_SZ 0x10000000
> +#define SLV_ADDR_SPACE_SZ_1_27_0 0x08000000
>
> /* PARF_MHI_CLOCK_RESET_CTRL register fields */
> #define AHB_CLK_EN BIT(0)
> @@ -202,10 +203,10 @@ struct qcom_pcie_resources_2_7_0 {
> struct reset_control *rst;
> };
>
> -#define QCOM_PCIE_2_9_0_MAX_CLOCKS 5
> struct qcom_pcie_resources_2_9_0 {
> - struct clk_bulk_data clks[QCOM_PCIE_2_9_0_MAX_CLOCKS];
> + struct clk_bulk_data *clks;
> struct reset_control *rst;
> + int num_clks;
> };
>
> union qcom_pcie_resources {
> @@ -1050,17 +1051,10 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
> struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> struct dw_pcie *pci = pcie->pci;
> struct device *dev = pci->dev;
> - int ret;
>
> - res->clks[0].id = "iface";
> - res->clks[1].id = "axi_m";
> - res->clks[2].id = "axi_s";
> - res->clks[3].id = "axi_bridge";
> - res->clks[4].id = "rchng";
> -
> - ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
> - if (ret < 0)
> - return ret;
> + res->num_clks = devm_clk_bulk_get_all(dev, &res->clks);
> + if (res->clks < 0)
> + return res->num_clks;

Why not return proper error no?

>
> res->rst = devm_reset_control_array_get_exclusive(dev);
> if (IS_ERR(res->rst))
> @@ -1073,7 +1067,7 @@ static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
> {
> struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
>
> - clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
> + clk_bulk_disable_unprepare(res->num_clks, res->clks);
> }
>
> static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
> @@ -1102,19 +1096,16 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
>
> usleep_range(2000, 2500);
>
> - return clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
> + return clk_bulk_prepare_enable(res->num_clks, res->clks);
> }
>
> -static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> +static int qcom_pcie_post_init(struct qcom_pcie *pcie)
> {
> struct dw_pcie *pci = pcie->pci;
> u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> u32 val;
> int i;
>
> - writel(SLV_ADDR_SPACE_SZ,
> - pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> -
> val = readl(pcie->parf + PARF_PHY_CTRL);
> val &= ~PHY_TEST_PWR_DOWN;
> writel(val, pcie->parf + PARF_PHY_CTRL);
> @@ -1151,6 +1142,26 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> return 0;
> }
>
> +static int qcom_pcie_post_init_1_27_0(struct qcom_pcie *pcie)
> +{
> + writel(SLV_ADDR_SPACE_SZ_1_27_0,
> + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> +
> + qcom_pcie_post_init(pcie);
> +
> + return 0;
> +}
> +
> +static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> +{
> + writel(SLV_ADDR_SPACE_SZ,
> + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> +
> + qcom_pcie_post_init(pcie);
> +
> + return 0;
> +}
> +
> static int qcom_pcie_link_up(struct dw_pcie *pci)
> {
> u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> @@ -1291,6 +1302,15 @@ static const struct qcom_pcie_ops ops_2_9_0 = {
> .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> };
>
> +/* Qcom IP rev.: 1.27.0 Synopsys IP rev.: 5.80a */
> +static const struct qcom_pcie_ops ops_1_27_0 = {
> + .get_resources = qcom_pcie_get_resources_2_9_0,
> + .init = qcom_pcie_init_2_9_0,
> + .post_init = qcom_pcie_post_init_1_27_0,
> + .deinit = qcom_pcie_deinit_2_9_0,
> + .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> +};
> +
> static const struct qcom_pcie_cfg cfg_1_0_0 = {
> .ops = &ops_1_0_0,
> };
> @@ -1323,6 +1343,10 @@ static const struct qcom_pcie_cfg cfg_2_9_0 = {
> .ops = &ops_2_9_0,
> };
>
> +static const struct qcom_pcie_cfg cfg_1_27_0 = {
> + .ops = &ops_1_27_0,
> +};
> +
> static const struct dw_pcie_ops dw_pcie_ops = {
> .link_up = qcom_pcie_link_up,
> .start_link = qcom_pcie_start_link,
> @@ -1607,6 +1631,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> { .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
> + { .compatible = "qcom,pcie-ipq9574", .data = &cfg_1_27_0 },
> { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> { .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
> --
> 2.17.1
>

--
மணிவண்ணன் சதாசிவம்

2023-05-08 13:03:21

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH V3 6/6] PCI: qcom: Add support for IPQ9574

On Mon, 8 May 2023 at 15:21, Manivannan Sadhasivam <[email protected]> wrote:
>
> On Fri, Apr 21, 2023 at 06:19:38PM +0530, Devi Priya wrote:
> > The IPQ9574 platform has 4 Gen3 PCIe controllers: two single-lane
> > and two dual-lane based on SNPS core 5.70a
> > The Qcom IP rev is 1.27.0 and Synopsys IP rev is 5.80a
> > Added a new compatible 'qcom,pcie-ipq9574' and 'ops_1_27_0'
> > which reuses all the members of 'ops_2_9_0' except for the post_init
> > as the SLV_ADDR_SPACE_SIZE configuration differs between 2_9_0
> > and 1_27_0.
> > Also, modified get_resources of 'ops 2_9_0' to get the clocks
> > from the device tree and modelled the post init sequence as
> > a common function to avoid code redundancy.
> >
> > Co-developed-by: Anusha Rao <[email protected]>
> > Signed-off-by: Anusha Rao <[email protected]>
> > Signed-off-by: Devi Priya <[email protected]>
>
> One comment below. With that fixed,
>
> Reviewed-by: Manivannan Sadhasivam <[email protected]>
>
> - Mani
>
> > ---
> > Changes in V3:
> > - Rebased on top of linux-next/master
> >
> > drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++--------
> > 1 file changed, 43 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 4ab30892f6ef..3682ecdead1f 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -107,6 +107,7 @@
> >
> > /* PARF_SLV_ADDR_SPACE_SIZE register value */
> > #define SLV_ADDR_SPACE_SZ 0x10000000
> > +#define SLV_ADDR_SPACE_SZ_1_27_0 0x08000000
> >
> > /* PARF_MHI_CLOCK_RESET_CTRL register fields */
> > #define AHB_CLK_EN BIT(0)
> > @@ -202,10 +203,10 @@ struct qcom_pcie_resources_2_7_0 {
> > struct reset_control *rst;
> > };
> >
> > -#define QCOM_PCIE_2_9_0_MAX_CLOCKS 5
> > struct qcom_pcie_resources_2_9_0 {
> > - struct clk_bulk_data clks[QCOM_PCIE_2_9_0_MAX_CLOCKS];
> > + struct clk_bulk_data *clks;
> > struct reset_control *rst;
> > + int num_clks;
> > };
> >
> > union qcom_pcie_resources {
> > @@ -1050,17 +1051,10 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
> > struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> > struct dw_pcie *pci = pcie->pci;
> > struct device *dev = pci->dev;
> > - int ret;
> >
> > - res->clks[0].id = "iface";
> > - res->clks[1].id = "axi_m";
> > - res->clks[2].id = "axi_s";
> > - res->clks[3].id = "axi_bridge";
> > - res->clks[4].id = "rchng";
> > -
> > - ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
> > - if (ret < 0)
> > - return ret;
> > + res->num_clks = devm_clk_bulk_get_all(dev, &res->clks);
> > + if (res->clks < 0)
> > + return res->num_clks;
>
> Why not return proper error no?

Instead the question should be, why not the proper condition: it tells
`if (res->clks < 0)', while it should be `if (res->num_clks < 0)'.

>
> >
> > res->rst = devm_reset_control_array_get_exclusive(dev);
> > if (IS_ERR(res->rst))
> > @@ -1073,7 +1067,7 @@ static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
> > {
> > struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> >
> > - clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
> > + clk_bulk_disable_unprepare(res->num_clks, res->clks);
> > }
> >
> > static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
> > @@ -1102,19 +1096,16 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
> >
> > usleep_range(2000, 2500);
> >
> > - return clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
> > + return clk_bulk_prepare_enable(res->num_clks, res->clks);
> > }
> >
> > -static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> > +static int qcom_pcie_post_init(struct qcom_pcie *pcie)
> > {
> > struct dw_pcie *pci = pcie->pci;
> > u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > u32 val;
> > int i;
> >
> > - writel(SLV_ADDR_SPACE_SZ,
> > - pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> > -
> > val = readl(pcie->parf + PARF_PHY_CTRL);
> > val &= ~PHY_TEST_PWR_DOWN;
> > writel(val, pcie->parf + PARF_PHY_CTRL);
> > @@ -1151,6 +1142,26 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> > return 0;
> > }
> >
> > +static int qcom_pcie_post_init_1_27_0(struct qcom_pcie *pcie)
> > +{
> > + writel(SLV_ADDR_SPACE_SZ_1_27_0,
> > + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> > +
> > + qcom_pcie_post_init(pcie);
> > +
> > + return 0;
> > +}
> > +
> > +static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> > +{
> > + writel(SLV_ADDR_SPACE_SZ,
> > + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> > +
> > + qcom_pcie_post_init(pcie);
> > +
> > + return 0;
> > +}
> > +
> > static int qcom_pcie_link_up(struct dw_pcie *pci)
> > {
> > u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > @@ -1291,6 +1302,15 @@ static const struct qcom_pcie_ops ops_2_9_0 = {
> > .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> > };
> >
> > +/* Qcom IP rev.: 1.27.0 Synopsys IP rev.: 5.80a */
> > +static const struct qcom_pcie_ops ops_1_27_0 = {
> > + .get_resources = qcom_pcie_get_resources_2_9_0,
> > + .init = qcom_pcie_init_2_9_0,
> > + .post_init = qcom_pcie_post_init_1_27_0,
> > + .deinit = qcom_pcie_deinit_2_9_0,
> > + .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> > +};
> > +
> > static const struct qcom_pcie_cfg cfg_1_0_0 = {
> > .ops = &ops_1_0_0,
> > };
> > @@ -1323,6 +1343,10 @@ static const struct qcom_pcie_cfg cfg_2_9_0 = {
> > .ops = &ops_2_9_0,
> > };
> >
> > +static const struct qcom_pcie_cfg cfg_1_27_0 = {
> > + .ops = &ops_1_27_0,
> > +};
> > +
> > static const struct dw_pcie_ops dw_pcie_ops = {
> > .link_up = qcom_pcie_link_up,
> > .start_link = qcom_pcie_start_link,
> > @@ -1607,6 +1631,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > { .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> > { .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> > { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
> > + { .compatible = "qcom,pcie-ipq9574", .data = &cfg_1_27_0 },
> > { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> > { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> > { .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
> > --
> > 2.17.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்



--
With best wishes
Dmitry

2023-05-08 16:05:20

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH V3 6/6] PCI: qcom: Add support for IPQ9574

On Mon, May 08, 2023 at 03:46:53PM +0300, Dmitry Baryshkov wrote:
> On Mon, 8 May 2023 at 15:21, Manivannan Sadhasivam <[email protected]> wrote:
> >
> > On Fri, Apr 21, 2023 at 06:19:38PM +0530, Devi Priya wrote:
> > > The IPQ9574 platform has 4 Gen3 PCIe controllers: two single-lane
> > > and two dual-lane based on SNPS core 5.70a
> > > The Qcom IP rev is 1.27.0 and Synopsys IP rev is 5.80a
> > > Added a new compatible 'qcom,pcie-ipq9574' and 'ops_1_27_0'
> > > which reuses all the members of 'ops_2_9_0' except for the post_init
> > > as the SLV_ADDR_SPACE_SIZE configuration differs between 2_9_0
> > > and 1_27_0.
> > > Also, modified get_resources of 'ops 2_9_0' to get the clocks
> > > from the device tree and modelled the post init sequence as
> > > a common function to avoid code redundancy.
> > >
> > > Co-developed-by: Anusha Rao <[email protected]>
> > > Signed-off-by: Anusha Rao <[email protected]>
> > > Signed-off-by: Devi Priya <[email protected]>
> >
> > One comment below. With that fixed,
> >
> > Reviewed-by: Manivannan Sadhasivam <[email protected]>
> >
> > - Mani
> >
> > > ---
> > > Changes in V3:
> > > - Rebased on top of linux-next/master
> > >
> > > drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++--------
> > > 1 file changed, 43 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index 4ab30892f6ef..3682ecdead1f 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -107,6 +107,7 @@
> > >
> > > /* PARF_SLV_ADDR_SPACE_SIZE register value */
> > > #define SLV_ADDR_SPACE_SZ 0x10000000
> > > +#define SLV_ADDR_SPACE_SZ_1_27_0 0x08000000
> > >
> > > /* PARF_MHI_CLOCK_RESET_CTRL register fields */
> > > #define AHB_CLK_EN BIT(0)
> > > @@ -202,10 +203,10 @@ struct qcom_pcie_resources_2_7_0 {
> > > struct reset_control *rst;
> > > };
> > >
> > > -#define QCOM_PCIE_2_9_0_MAX_CLOCKS 5
> > > struct qcom_pcie_resources_2_9_0 {
> > > - struct clk_bulk_data clks[QCOM_PCIE_2_9_0_MAX_CLOCKS];
> > > + struct clk_bulk_data *clks;
> > > struct reset_control *rst;
> > > + int num_clks;
> > > };
> > >
> > > union qcom_pcie_resources {
> > > @@ -1050,17 +1051,10 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
> > > struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> > > struct dw_pcie *pci = pcie->pci;
> > > struct device *dev = pci->dev;
> > > - int ret;
> > >
> > > - res->clks[0].id = "iface";
> > > - res->clks[1].id = "axi_m";
> > > - res->clks[2].id = "axi_s";
> > > - res->clks[3].id = "axi_bridge";
> > > - res->clks[4].id = "rchng";
> > > -
> > > - ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
> > > - if (ret < 0)
> > > - return ret;
> > > + res->num_clks = devm_clk_bulk_get_all(dev, &res->clks);
> > > + if (res->clks < 0)
> > > + return res->num_clks;
> >
> > Why not return proper error no?
>
> Instead the question should be, why not the proper condition: it tells
> `if (res->clks < 0)', while it should be `if (res->num_clks < 0)'.
>

Heh. I completely overlooked that part. Yes, the if condition itself should be
fixed.

- Mani

> >
> > >
> > > res->rst = devm_reset_control_array_get_exclusive(dev);
> > > if (IS_ERR(res->rst))
> > > @@ -1073,7 +1067,7 @@ static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
> > > {
> > > struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> > >
> > > - clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
> > > + clk_bulk_disable_unprepare(res->num_clks, res->clks);
> > > }
> > >
> > > static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
> > > @@ -1102,19 +1096,16 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
> > >
> > > usleep_range(2000, 2500);
> > >
> > > - return clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
> > > + return clk_bulk_prepare_enable(res->num_clks, res->clks);
> > > }
> > >
> > > -static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> > > +static int qcom_pcie_post_init(struct qcom_pcie *pcie)
> > > {
> > > struct dw_pcie *pci = pcie->pci;
> > > u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > > u32 val;
> > > int i;
> > >
> > > - writel(SLV_ADDR_SPACE_SZ,
> > > - pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> > > -
> > > val = readl(pcie->parf + PARF_PHY_CTRL);
> > > val &= ~PHY_TEST_PWR_DOWN;
> > > writel(val, pcie->parf + PARF_PHY_CTRL);
> > > @@ -1151,6 +1142,26 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> > > return 0;
> > > }
> > >
> > > +static int qcom_pcie_post_init_1_27_0(struct qcom_pcie *pcie)
> > > +{
> > > + writel(SLV_ADDR_SPACE_SZ_1_27_0,
> > > + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> > > +
> > > + qcom_pcie_post_init(pcie);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
> > > +{
> > > + writel(SLV_ADDR_SPACE_SZ,
> > > + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
> > > +
> > > + qcom_pcie_post_init(pcie);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static int qcom_pcie_link_up(struct dw_pcie *pci)
> > > {
> > > u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > > @@ -1291,6 +1302,15 @@ static const struct qcom_pcie_ops ops_2_9_0 = {
> > > .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> > > };
> > >
> > > +/* Qcom IP rev.: 1.27.0 Synopsys IP rev.: 5.80a */
> > > +static const struct qcom_pcie_ops ops_1_27_0 = {
> > > + .get_resources = qcom_pcie_get_resources_2_9_0,
> > > + .init = qcom_pcie_init_2_9_0,
> > > + .post_init = qcom_pcie_post_init_1_27_0,
> > > + .deinit = qcom_pcie_deinit_2_9_0,
> > > + .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> > > +};
> > > +
> > > static const struct qcom_pcie_cfg cfg_1_0_0 = {
> > > .ops = &ops_1_0_0,
> > > };
> > > @@ -1323,6 +1343,10 @@ static const struct qcom_pcie_cfg cfg_2_9_0 = {
> > > .ops = &ops_2_9_0,
> > > };
> > >
> > > +static const struct qcom_pcie_cfg cfg_1_27_0 = {
> > > + .ops = &ops_1_27_0,
> > > +};
> > > +
> > > static const struct dw_pcie_ops dw_pcie_ops = {
> > > .link_up = qcom_pcie_link_up,
> > > .start_link = qcom_pcie_start_link,
> > > @@ -1607,6 +1631,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> > > { .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> > > { .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> > > { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
> > > + { .compatible = "qcom,pcie-ipq9574", .data = &cfg_1_27_0 },
> > > { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> > > { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> > > { .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
> > > --
> > > 2.17.1
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
>
>
>
> --
> With best wishes
> Dmitry

--
மணிவண்ணன் சதாசிவம்

2023-05-09 09:24:12

by Devi Priya

[permalink] [raw]
Subject: Re: [PATCH V3 6/6] PCI: qcom: Add support for IPQ9574



On 5/8/2023 9:07 PM, Manivannan Sadhasivam wrote:
> On Mon, May 08, 2023 at 03:46:53PM +0300, Dmitry Baryshkov wrote:
>> On Mon, 8 May 2023 at 15:21, Manivannan Sadhasivam <[email protected]> wrote:
>>>
>>> On Fri, Apr 21, 2023 at 06:19:38PM +0530, Devi Priya wrote:
>>>> The IPQ9574 platform has 4 Gen3 PCIe controllers: two single-lane
>>>> and two dual-lane based on SNPS core 5.70a
>>>> The Qcom IP rev is 1.27.0 and Synopsys IP rev is 5.80a
>>>> Added a new compatible 'qcom,pcie-ipq9574' and 'ops_1_27_0'
>>>> which reuses all the members of 'ops_2_9_0' except for the post_init
>>>> as the SLV_ADDR_SPACE_SIZE configuration differs between 2_9_0
>>>> and 1_27_0.
>>>> Also, modified get_resources of 'ops 2_9_0' to get the clocks
>>>> from the device tree and modelled the post init sequence as
>>>> a common function to avoid code redundancy.
>>>>
>>>> Co-developed-by: Anusha Rao <[email protected]>
>>>> Signed-off-by: Anusha Rao <[email protected]>
>>>> Signed-off-by: Devi Priya <[email protected]>
>>>
>>> One comment below. With that fixed,
>>>
>>> Reviewed-by: Manivannan Sadhasivam <[email protected]>
>>>
>>> - Mani
>>>
>>>> ---
>>>> Changes in V3:
>>>> - Rebased on top of linux-next/master
>>>>
>>>> drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++--------
>>>> 1 file changed, 43 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>>> index 4ab30892f6ef..3682ecdead1f 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>>> @@ -107,6 +107,7 @@
>>>>
>>>> /* PARF_SLV_ADDR_SPACE_SIZE register value */
>>>> #define SLV_ADDR_SPACE_SZ 0x10000000
>>>> +#define SLV_ADDR_SPACE_SZ_1_27_0 0x08000000
>>>>
>>>> /* PARF_MHI_CLOCK_RESET_CTRL register fields */
>>>> #define AHB_CLK_EN BIT(0)
>>>> @@ -202,10 +203,10 @@ struct qcom_pcie_resources_2_7_0 {
>>>> struct reset_control *rst;
>>>> };
>>>>
>>>> -#define QCOM_PCIE_2_9_0_MAX_CLOCKS 5
>>>> struct qcom_pcie_resources_2_9_0 {
>>>> - struct clk_bulk_data clks[QCOM_PCIE_2_9_0_MAX_CLOCKS];
>>>> + struct clk_bulk_data *clks;
>>>> struct reset_control *rst;
>>>> + int num_clks;
>>>> };
>>>>
>>>> union qcom_pcie_resources {
>>>> @@ -1050,17 +1051,10 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
>>>> struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
>>>> struct dw_pcie *pci = pcie->pci;
>>>> struct device *dev = pci->dev;
>>>> - int ret;
>>>>
>>>> - res->clks[0].id = "iface";
>>>> - res->clks[1].id = "axi_m";
>>>> - res->clks[2].id = "axi_s";
>>>> - res->clks[3].id = "axi_bridge";
>>>> - res->clks[4].id = "rchng";
>>>> -
>>>> - ret = devm_clk_bulk_get(dev, ARRAY_SIZE(res->clks), res->clks);
>>>> - if (ret < 0)
>>>> - return ret;
>>>> + res->num_clks = devm_clk_bulk_get_all(dev, &res->clks);
>>>> + if (res->clks < 0)
>>>> + return res->num_clks;
>>>
>>> Why not return proper error no?
>>
>> Instead the question should be, why not the proper condition: it tells
>> `if (res->clks < 0)', while it should be `if (res->num_clks < 0)'.
>>
>
> Heh. I completely overlooked that part. Yes, the if condition itself should be
> fixed.
>
> - Mani

Sure, will fix it up!

Regards,
Devi Priya
>
>>>
>>>>
>>>> res->rst = devm_reset_control_array_get_exclusive(dev);
>>>> if (IS_ERR(res->rst))
>>>> @@ -1073,7 +1067,7 @@ static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
>>>> {
>>>> struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
>>>>
>>>> - clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
>>>> + clk_bulk_disable_unprepare(res->num_clks, res->clks);
>>>> }
>>>>
>>>> static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
>>>> @@ -1102,19 +1096,16 @@ static int qcom_pcie_init_2_9_0(struct qcom_pcie *pcie)
>>>>
>>>> usleep_range(2000, 2500);
>>>>
>>>> - return clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
>>>> + return clk_bulk_prepare_enable(res->num_clks, res->clks);
>>>> }
>>>>
>>>> -static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>>>> +static int qcom_pcie_post_init(struct qcom_pcie *pcie)
>>>> {
>>>> struct dw_pcie *pci = pcie->pci;
>>>> u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>>>> u32 val;
>>>> int i;
>>>>
>>>> - writel(SLV_ADDR_SPACE_SZ,
>>>> - pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
>>>> -
>>>> val = readl(pcie->parf + PARF_PHY_CTRL);
>>>> val &= ~PHY_TEST_PWR_DOWN;
>>>> writel(val, pcie->parf + PARF_PHY_CTRL);
>>>> @@ -1151,6 +1142,26 @@ static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>>>> return 0;
>>>> }
>>>>
>>>> +static int qcom_pcie_post_init_1_27_0(struct qcom_pcie *pcie)
>>>> +{
>>>> + writel(SLV_ADDR_SPACE_SZ_1_27_0,
>>>> + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
>>>> +
>>>> + qcom_pcie_post_init(pcie);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int qcom_pcie_post_init_2_9_0(struct qcom_pcie *pcie)
>>>> +{
>>>> + writel(SLV_ADDR_SPACE_SZ,
>>>> + pcie->parf + PARF_SLV_ADDR_SPACE_SIZE);
>>>> +
>>>> + qcom_pcie_post_init(pcie);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static int qcom_pcie_link_up(struct dw_pcie *pci)
>>>> {
>>>> u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>>>> @@ -1291,6 +1302,15 @@ static const struct qcom_pcie_ops ops_2_9_0 = {
>>>> .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
>>>> };
>>>>
>>>> +/* Qcom IP rev.: 1.27.0 Synopsys IP rev.: 5.80a */
>>>> +static const struct qcom_pcie_ops ops_1_27_0 = {
>>>> + .get_resources = qcom_pcie_get_resources_2_9_0,
>>>> + .init = qcom_pcie_init_2_9_0,
>>>> + .post_init = qcom_pcie_post_init_1_27_0,
>>>> + .deinit = qcom_pcie_deinit_2_9_0,
>>>> + .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
>>>> +};
>>>> +
>>>> static const struct qcom_pcie_cfg cfg_1_0_0 = {
>>>> .ops = &ops_1_0_0,
>>>> };
>>>> @@ -1323,6 +1343,10 @@ static const struct qcom_pcie_cfg cfg_2_9_0 = {
>>>> .ops = &ops_2_9_0,
>>>> };
>>>>
>>>> +static const struct qcom_pcie_cfg cfg_1_27_0 = {
>>>> + .ops = &ops_1_27_0,
>>>> +};
>>>> +
>>>> static const struct dw_pcie_ops dw_pcie_ops = {
>>>> .link_up = qcom_pcie_link_up,
>>>> .start_link = qcom_pcie_start_link,
>>>> @@ -1607,6 +1631,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>>>> { .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
>>>> { .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
>>>> { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
>>>> + { .compatible = "qcom,pcie-ipq9574", .data = &cfg_1_27_0 },
>>>> { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
>>>> { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
>>>> { .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
>>>> --
>>>> 2.17.1
>>>>
>>>
>>> --
>>> மணிவண்ணன் சதாசிவம்
>>
>>
>>
>> --
>> With best wishes
>> Dmitry
>

2023-05-15 09:46:57

by Devi Priya

[permalink] [raw]
Subject: Re: [PATCH V3 4/6] arm64: dts: qcom: ipq9574: Add PCIe PHYs and controller nodes



On 5/8/2023 5:10 PM, Dmitry Baryshkov wrote:
> On 08/05/2023 13:53, Devi Priya wrote:
>>
>>
>> On 4/22/2023 5:49 AM, Dmitry Baryshkov wrote:
>>> On Fri, 21 Apr 2023 at 15:50, Devi Priya <[email protected]>
>>> wrote:
>>>>
>>>> Add PCIe0, PCIe1, PCIe2, PCIe3 (and corresponding PHY) devices
>>>> found on IPQ9574 platform. The PCIe0 & PCIe1 are 1-lane Gen3
>>>> host whereas PCIe2 & PCIe3 are 2-lane Gen3 host.
>>>>
>>>> Co-developed-by: Anusha Rao <[email protected]>
>>>> Signed-off-by: Anusha Rao <[email protected]>
>>>> Signed-off-by: Devi Priya <[email protected]>
>>>> ---
>>>>   Changes in V3:
>>>>          - Fixed up the PCI I/O port ranges
>>>>
>>>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi | 375
>>>> +++++++++++++++++++++++++-
>>>>   1 file changed, 370 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>>> b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>>> index e757b57957cf..953a839a1141 100644
>>>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>>> @@ -6,8 +6,8 @@
>>>>    * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights
>>>> reserved.
>>>>    */
>>>>
>>>> -#include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>   #include <dt-bindings/clock/qcom,ipq9574-gcc.h>
>>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>   #include <dt-bindings/reset/qcom,ipq9574-gcc.h>
>>>>
>>>>   / {
>>>> @@ -116,6 +116,58 @@
>>>>                  #size-cells = <1>;
>>>>                  ranges = <0 0 0 0xffffffff>;
>>>>
>>>> +               pcie0_phy: phy@84000 {
>>>> +                       compatible =
>>>> "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>>>> +                       reg = <0x00084000 0x1000>;
>>>> +
>>>> +                       clocks = <&gcc GCC_PCIE0_AUX_CLK>,
>>>> +                                <&gcc GCC_PCIE0_AHB_CLK>,
>>>> +                                <&gcc GCC_ANOC_PCIE0_1LANE_M_CLK>,
>>>> +                                <&gcc GCC_SNOC_PCIE0_1LANE_S_CLK>,
>>>> +                                <&gcc GCC_PCIE0_PIPE_CLK>;
>>>> +                       clock-names = "aux", "cfg_ahb", "anoc_lane",
>>>> "snoc_lane", "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";
>>>> +
>>>> +               };
>>>> +
>>>> +               pcie2_phy: phy@8c000 {
>>>> +                       compatible =
>>>> "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>>>> +                       reg = <0x0008c000 0x2000>;
>>>> +
>>>> +                       clocks = <&gcc GCC_PCIE2_AUX_CLK>,
>>>> +                                <&gcc GCC_PCIE2_AHB_CLK>,
>>>> +                                <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
>>>> +                                <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>,
>>>> +                                <&gcc GCC_PCIE2_PIPE_CLK>;
>>>> +                       clock-names = "aux", "cfg_ahb", "anoc_lane",
>>>> "snoc_lane", "pipe";
>>>> +
>>>> +                       assigned-clocks = <&gcc GCC_PCIE2_AUX_CLK>;
>>>> +                       assigned-clock-rates = <20000000>;
>>>> +
>>>> +                       resets = <&gcc GCC_PCIE2_PHY_BCR>,
>>>> +                                <&gcc GCC_PCIE2PHY_PHY_BCR>;
>>>> +                       reset-names = "phy", "common";
>>>> +
>>>> +                       #clock-cells = <0>;
>>>> +                       clock-output-names = "gcc_pcie2_pipe_clk_src";
>>>> +
>>>> +                       #phy-cells = <0>;
>>>> +                       status = "disabled";
>>>> +
>>>> +               };
>>>> +
>>>>                  rng: rng@e3000 {
>>>>                          compatible = "qcom,prng-ee";
>>>>                          reg = <0x000e3000 0x1000>;
>>>> @@ -123,6 +175,58 @@
>>>>                          clock-names = "core";
>>>>                  };
>>>>
>>>> +               pcie3_phy: phy@f4000 {
>>>> +                       compatible =
>>>> "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>>>> +                       reg = <0x000f4000 0x2000>;
>>>> +
>>>> +                       clocks = <&gcc GCC_PCIE3_AUX_CLK>,
>>>> +                                <&gcc GCC_PCIE3_AHB_CLK>,
>>>> +                                <&gcc GCC_ANOC_PCIE3_2LANE_M_CLK>,
>>>> +                                <&gcc GCC_SNOC_PCIE3_2LANE_S_CLK>,
>>>> +                                <&gcc GCC_PCIE3_PIPE_CLK>;
>>>> +                       clock-names = "aux", "cfg_ahb", "anoc_lane",
>>>> "snoc_lane", "pipe";
>>>> +
>>>> +                       assigned-clocks = <&gcc GCC_PCIE3_AUX_CLK>;
>>>> +                       assigned-clock-rates = <20000000>;
>>>> +
>>>> +                       resets = <&gcc GCC_PCIE3_PHY_BCR>,
>>>> +                                <&gcc GCC_PCIE3PHY_PHY_BCR>;
>>>> +                       reset-names = "phy", "common";
>>>> +
>>>> +                       #clock-cells = <0>;
>>>> +                       clock-output-names = "gcc_pcie3_pipe_clk_src";
>>>> +
>>>> +                       #phy-cells = <0>;
>>>> +                       status = "disabled";
>>>> +
>>>> +               };
>>>> +
>>>> +               pcie1_phy: phy@fc000 {
>>>> +                       compatible =
>>>> "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>>>> +                       reg = <0x000fc000 0x1000>;
>>>> +
>>>> +                       clocks = <&gcc GCC_PCIE1_AUX_CLK>,
>>>> +                                <&gcc GCC_PCIE1_AHB_CLK>,
>>>> +                                <&gcc GCC_ANOC_PCIE1_1LANE_M_CLK>,
>>>> +                                <&gcc GCC_SNOC_PCIE1_1LANE_S_CLK>,
>>>> +                                <&gcc GCC_PCIE1_PIPE_CLK>;
>>>> +                       clock-names = "aux", "cfg_ahb", "anoc_lane",
>>>> "snoc_lane", "pipe";
>>>> +
>>>> +                       assigned-clocks = <&gcc GCC_PCIE1_AUX_CLK>;
>>>> +                       assigned-clock-rates = <20000000>;
>>>> +
>>>> +                       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,ipq9574-tlmm";
>>>>                          reg = <0x01000000 0x300000>;
>>>> @@ -146,10 +250,10 @@
>>>>                          reg = <0x01800000 0x80000>;
>>>>                          clocks = <&xo_board_clk>,
>>>>                                   <&sleep_clk>,
>>>> -                                <0>,
>>>> -                                <0>,
>>>> -                                <0>,
>>>> -                                <0>,
>>>> +                                <&pcie0_phy>,
>>>> +                                <&pcie1_phy>,
>>>> +                                <&pcie2_phy>,
>>>> +                                <&pcie3_phy>,
>>>>                                   <0>;
>>>>                          #clock-cells = <1>;
>>>>                          #reset-cells = <1>;
>>>> @@ -478,6 +582,267 @@
>>>>                                  status = "disabled";
>>>>                          };
>>>>                  };
>>>> +
>>>> +               pcie1: pci@10000000 {
>>>> +                       compatible = "qcom,pcie-ipq9574";
>>>> +                       reg =  <0x10000000 0xf1d>,
>>>> +                              <0x10000F20 0xa8>,
>>>> +                              <0x10001000 0x1000>,
>>>> +                              <0x000F8000 0x4000>,
>>>> +                              <0x10100000 0x1000>;
>>>> +                       reg-names = "dbi", "elbi", "atu", "parf",
>>>> "config";
>>>> +                       device_type = "pci";
>>>> +                       linux,pci-domain = <2>;
>>>> +                       bus-range = <0x00 0xff>;
>>>> +                       num-lanes = <1>;
>>>> +                       #address-cells = <3>;
>>>> +                       #size-cells = <2>;
>>>> +
>>>> +                       ranges = <0x01000000 0x0 0x00000000
>>>> 0x10200000 0x0 0x100000>,  /* I/O */
>>>> +                                <0x02000000 0x0 0x10300000
>>>> 0x10300000 0x0 0x7d00000>; /* MEM */
>>>> +
>>>> +                       #interrupt-cells = <1>;
>>>> +                       interrupt-map-mask = <0 0 0 0x7>;
>>>> +                       interrupt-map = <0 0 0 1 &intc 0 35
>>>> IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>>>> +                                       <0 0 0 2 &intc 0 49
>>>> IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>>>> +                                       <0 0 0 3 &intc 0 84
>>>> IRQ_TYPE_LEVEL_HIGH>, /* int_c */
>>>> +                                       <0 0 0 4 &intc 0 85
>>>> IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>>>> +
>>>
>>> No iommu-map?
>> We do not enable the IOMMU stage1 translation for PCIe and the registers
>> have secure access only from TrustZone (It enables only stage2 for
>> Access control)
>
> So, no SMMU protection for PCIe transactions? This sounds like a step
> backwards.
Yes, we are not using stage1 translations.

Thanks,
Devi Priya
>

2023-05-15 09:47:18

by Devi Priya

[permalink] [raw]
Subject: Re: [PATCH V3 5/6] arm64: dts: qcom: ipq9574: Enable PCIe PHYs and controllers



On 5/8/2023 5:09 PM, Dmitry Baryshkov wrote:
> On 08/05/2023 13:55, Devi Priya wrote:
>>
>>
>> On 4/22/2023 5:43 AM, Dmitry Baryshkov wrote:
>>> On Fri, 21 Apr 2023 at 15:51, Devi Priya <[email protected]>
>>> wrote:
>>>>
>>>> Enable the PCIe controller and PHY nodes corresponding to
>>>> RDP 433.
>>>>
>>>> Signed-off-by: Devi Priya <[email protected]>
>>>> ---
>>>>   Changes in V3:
>>>>          - No change
>>>>
>>>>   arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 62
>>>> +++++++++++++++++++++
>>>>   1 file changed, 62 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>>>> b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>>>> index 7be578017bf7..3ae38cf327ea 100644
>>>> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>>>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>>>> @@ -8,6 +8,7 @@
>>>>
>>>>   /dts-v1/;
>>>>
>>>> +#include <dt-bindings/gpio/gpio.h>
>>>>   #include "ipq9574.dtsi"
>>>>
>>>>   / {
>>>> @@ -43,6 +44,42 @@
>>>>          };
>>>>   };
>>>>
>>>> +&pcie1_phy {
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&pcie1 {
>>>> +       pinctrl-names = "default";
>>>> +       pinctrl-0 = <&pcie_1_pin>;
>>>> +
>>>> +       perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
>>>
>>> Usually qcom PCIe hosts also define wake-gpios.
>> In IPQ9574, we do not have hot plug support and host always starts the
>> enumeration for the device. Hence no wake pin is required.
>
> None of the qcom PCIe hosts support hotplug, if I remember correctly.
> This is not a reason not to describe the hardware.

Okay, will add the pin definitions for wake and clkreq in the next spin.
>
>>>
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&pcie2_phy {
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&pcie2 {
>>>> +       pinctrl-names = "default";
>>>> +       pinctrl-0 = <&pcie_2_pin>;
>>>> +
>>>> +       perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&pcie3_phy {
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>> +&pcie3 {
>>>> +       pinctrl-names = "default";
>>>> +       pinctrl-0 = <&pcie_3_pin>;
>>>> +
>>>> +       perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
>>>> +       status = "okay";
>>>> +};
>>>> +
>>>>   &sdhc_1 {
>>>>          pinctrl-0 = <&sdc_default_state>;
>>>>          pinctrl-names = "default";
>>>> @@ -60,6 +97,31 @@
>>>>   };
>>>>
>>>>   &tlmm {
>>>> +
>>>> +       pcie_1_pin: pcie-1-state {
>>>> +               pins = "gpio26";
>>>> +               function = "gpio";
>>>> +               drive-strength = <8>;
>>>> +               bias-pull-down;
>>>> +               output-low;
>>>
>>> No clkreq and no wake gpios?
>> We do not use any PCIe low power states and link is always in L0.
>
> Again. We = software. Please describe the hardware here.
Got it.
>
>>
>> Thanks,
>> Devi Priya
>>>
>>>> +       };
>>>> +
>>>> +       pcie_2_pin: pcie-2-state {
>>>> +               pins = "gpio29";
>>>> +               function = "gpio";
>>>> +               drive-strength = <8>;
>>>> +               bias-pull-down;
>>>> +               output-low;
>>>> +       };
>>>> +
>>>> +       pcie_3_pin: pcie-3-state {
>>>> +               pins = "gpio32";
>>>> +               function = "gpio";
>>>> +               drive-strength = <8>;
>>>> +               bias-pull-up;
>>>> +               output-low;
>>>> +       };
>>>> +
>>>>          sdc_default_state: sdc-default-state {
>>>>                  clk-pins {
>>>>                          pins = "gpio5";
>>>> --
>>>> 2.17.1
>>>>
>>>
>>>
>
Thanks,
Devi Priya

2023-05-15 10:04:07

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH V3 4/6] arm64: dts: qcom: ipq9574: Add PCIe PHYs and controller nodes

On Mon, 15 May 2023 at 12:36, Devi Priya <[email protected]> wrote:
>
>
>
> On 5/8/2023 5:10 PM, Dmitry Baryshkov wrote:
> > On 08/05/2023 13:53, Devi Priya wrote:
> >>
> >>
> >> On 4/22/2023 5:49 AM, Dmitry Baryshkov wrote:
> >>> On Fri, 21 Apr 2023 at 15:50, Devi Priya <[email protected]>
> >>> wrote:
> >>>>
> >>>> Add PCIe0, PCIe1, PCIe2, PCIe3 (and corresponding PHY) devices
> >>>> found on IPQ9574 platform. The PCIe0 & PCIe1 are 1-lane Gen3
> >>>> host whereas PCIe2 & PCIe3 are 2-lane Gen3 host.
> >>>>
> >>>> Co-developed-by: Anusha Rao <[email protected]>
> >>>> Signed-off-by: Anusha Rao <[email protected]>
> >>>> Signed-off-by: Devi Priya <[email protected]>
> >>>> ---
> >>>> Changes in V3:
> >>>> - Fixed up the PCI I/O port ranges
> >>>>
> >>>> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 375
> >>>> +++++++++++++++++++++++++-
> >>>> 1 file changed, 370 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> >>>> b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> >>>> index e757b57957cf..953a839a1141 100644
> >>>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> >>>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> >>>> @@ -6,8 +6,8 @@
> >>>> * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights
> >>>> reserved.
> >>>> */
> >>>>
> >>>> -#include <dt-bindings/interrupt-controller/arm-gic.h>
> >>>> #include <dt-bindings/clock/qcom,ipq9574-gcc.h>
> >>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> >>>> #include <dt-bindings/reset/qcom,ipq9574-gcc.h>
> >>>>
> >>>> / {
> >>>> @@ -116,6 +116,58 @@
> >>>> #size-cells = <1>;
> >>>> ranges = <0 0 0 0xffffffff>;
> >>>>
> >>>> + pcie0_phy: phy@84000 {
> >>>> + compatible =
> >>>> "qcom,ipq9574-qmp-gen3x1-pcie-phy";
> >>>> + reg = <0x00084000 0x1000>;
> >>>> +
> >>>> + clocks = <&gcc GCC_PCIE0_AUX_CLK>,
> >>>> + <&gcc GCC_PCIE0_AHB_CLK>,
> >>>> + <&gcc GCC_ANOC_PCIE0_1LANE_M_CLK>,
> >>>> + <&gcc GCC_SNOC_PCIE0_1LANE_S_CLK>,
> >>>> + <&gcc GCC_PCIE0_PIPE_CLK>;
> >>>> + clock-names = "aux", "cfg_ahb", "anoc_lane",
> >>>> "snoc_lane", "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";
> >>>> +
> >>>> + };
> >>>> +
> >>>> + pcie2_phy: phy@8c000 {
> >>>> + compatible =
> >>>> "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> >>>> + reg = <0x0008c000 0x2000>;
> >>>> +
> >>>> + clocks = <&gcc GCC_PCIE2_AUX_CLK>,
> >>>> + <&gcc GCC_PCIE2_AHB_CLK>,
> >>>> + <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
> >>>> + <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>,
> >>>> + <&gcc GCC_PCIE2_PIPE_CLK>;
> >>>> + clock-names = "aux", "cfg_ahb", "anoc_lane",
> >>>> "snoc_lane", "pipe";
> >>>> +
> >>>> + assigned-clocks = <&gcc GCC_PCIE2_AUX_CLK>;
> >>>> + assigned-clock-rates = <20000000>;
> >>>> +
> >>>> + resets = <&gcc GCC_PCIE2_PHY_BCR>,
> >>>> + <&gcc GCC_PCIE2PHY_PHY_BCR>;
> >>>> + reset-names = "phy", "common";
> >>>> +
> >>>> + #clock-cells = <0>;
> >>>> + clock-output-names = "gcc_pcie2_pipe_clk_src";
> >>>> +
> >>>> + #phy-cells = <0>;
> >>>> + status = "disabled";
> >>>> +
> >>>> + };
> >>>> +
> >>>> rng: rng@e3000 {
> >>>> compatible = "qcom,prng-ee";
> >>>> reg = <0x000e3000 0x1000>;
> >>>> @@ -123,6 +175,58 @@
> >>>> clock-names = "core";
> >>>> };
> >>>>
> >>>> + pcie3_phy: phy@f4000 {
> >>>> + compatible =
> >>>> "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> >>>> + reg = <0x000f4000 0x2000>;
> >>>> +
> >>>> + clocks = <&gcc GCC_PCIE3_AUX_CLK>,
> >>>> + <&gcc GCC_PCIE3_AHB_CLK>,
> >>>> + <&gcc GCC_ANOC_PCIE3_2LANE_M_CLK>,
> >>>> + <&gcc GCC_SNOC_PCIE3_2LANE_S_CLK>,
> >>>> + <&gcc GCC_PCIE3_PIPE_CLK>;
> >>>> + clock-names = "aux", "cfg_ahb", "anoc_lane",
> >>>> "snoc_lane", "pipe";
> >>>> +
> >>>> + assigned-clocks = <&gcc GCC_PCIE3_AUX_CLK>;
> >>>> + assigned-clock-rates = <20000000>;
> >>>> +
> >>>> + resets = <&gcc GCC_PCIE3_PHY_BCR>,
> >>>> + <&gcc GCC_PCIE3PHY_PHY_BCR>;
> >>>> + reset-names = "phy", "common";
> >>>> +
> >>>> + #clock-cells = <0>;
> >>>> + clock-output-names = "gcc_pcie3_pipe_clk_src";
> >>>> +
> >>>> + #phy-cells = <0>;
> >>>> + status = "disabled";
> >>>> +
> >>>> + };
> >>>> +
> >>>> + pcie1_phy: phy@fc000 {
> >>>> + compatible =
> >>>> "qcom,ipq9574-qmp-gen3x1-pcie-phy";
> >>>> + reg = <0x000fc000 0x1000>;
> >>>> +
> >>>> + clocks = <&gcc GCC_PCIE1_AUX_CLK>,
> >>>> + <&gcc GCC_PCIE1_AHB_CLK>,
> >>>> + <&gcc GCC_ANOC_PCIE1_1LANE_M_CLK>,
> >>>> + <&gcc GCC_SNOC_PCIE1_1LANE_S_CLK>,
> >>>> + <&gcc GCC_PCIE1_PIPE_CLK>;
> >>>> + clock-names = "aux", "cfg_ahb", "anoc_lane",
> >>>> "snoc_lane", "pipe";
> >>>> +
> >>>> + assigned-clocks = <&gcc GCC_PCIE1_AUX_CLK>;
> >>>> + assigned-clock-rates = <20000000>;
> >>>> +
> >>>> + 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,ipq9574-tlmm";
> >>>> reg = <0x01000000 0x300000>;
> >>>> @@ -146,10 +250,10 @@
> >>>> reg = <0x01800000 0x80000>;
> >>>> clocks = <&xo_board_clk>,
> >>>> <&sleep_clk>,
> >>>> - <0>,
> >>>> - <0>,
> >>>> - <0>,
> >>>> - <0>,
> >>>> + <&pcie0_phy>,
> >>>> + <&pcie1_phy>,
> >>>> + <&pcie2_phy>,
> >>>> + <&pcie3_phy>,
> >>>> <0>;
> >>>> #clock-cells = <1>;
> >>>> #reset-cells = <1>;
> >>>> @@ -478,6 +582,267 @@
> >>>> status = "disabled";
> >>>> };
> >>>> };
> >>>> +
> >>>> + pcie1: pci@10000000 {
> >>>> + compatible = "qcom,pcie-ipq9574";
> >>>> + reg = <0x10000000 0xf1d>,
> >>>> + <0x10000F20 0xa8>,
> >>>> + <0x10001000 0x1000>,
> >>>> + <0x000F8000 0x4000>,
> >>>> + <0x10100000 0x1000>;
> >>>> + reg-names = "dbi", "elbi", "atu", "parf",
> >>>> "config";
> >>>> + device_type = "pci";
> >>>> + linux,pci-domain = <2>;
> >>>> + bus-range = <0x00 0xff>;
> >>>> + num-lanes = <1>;
> >>>> + #address-cells = <3>;
> >>>> + #size-cells = <2>;
> >>>> +
> >>>> + ranges = <0x01000000 0x0 0x00000000
> >>>> 0x10200000 0x0 0x100000>, /* I/O */
> >>>> + <0x02000000 0x0 0x10300000
> >>>> 0x10300000 0x0 0x7d00000>; /* MEM */
> >>>> +
> >>>> + #interrupt-cells = <1>;
> >>>> + interrupt-map-mask = <0 0 0 0x7>;
> >>>> + interrupt-map = <0 0 0 1 &intc 0 35
> >>>> IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> >>>> + <0 0 0 2 &intc 0 49
> >>>> IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> >>>> + <0 0 0 3 &intc 0 84
> >>>> IRQ_TYPE_LEVEL_HIGH>, /* int_c */
> >>>> + <0 0 0 4 &intc 0 85
> >>>> IRQ_TYPE_LEVEL_HIGH>; /* int_d */
> >>>> +
> >>>
> >>> No iommu-map?
> >> We do not enable the IOMMU stage1 translation for PCIe and the registers
> >> have secure access only from TrustZone (It enables only stage2 for
> >> Access control)
> >
> > So, no SMMU protection for PCIe transactions? This sounds like a step
> > backwards.
> Yes, we are not using stage1 translations.

We = software or we = hardware? If there is a hardware interface to
SMMU, please describe it here.

--
With best wishes
Dmitry

2023-05-15 13:29:31

by Devi Priya

[permalink] [raw]
Subject: Re: [PATCH V3 4/6] arm64: dts: qcom: ipq9574: Add PCIe PHYs and controller nodes



On 5/15/2023 3:21 PM, Dmitry Baryshkov wrote:
> On Mon, 15 May 2023 at 12:36, Devi Priya <[email protected]> wrote:
>>
>>
>>
>> On 5/8/2023 5:10 PM, Dmitry Baryshkov wrote:
>>> On 08/05/2023 13:53, Devi Priya wrote:
>>>>
>>>>
>>>> On 4/22/2023 5:49 AM, Dmitry Baryshkov wrote:
>>>>> On Fri, 21 Apr 2023 at 15:50, Devi Priya <[email protected]>
>>>>> wrote:
>>>>>>
>>>>>> Add PCIe0, PCIe1, PCIe2, PCIe3 (and corresponding PHY) devices
>>>>>> found on IPQ9574 platform. The PCIe0 & PCIe1 are 1-lane Gen3
>>>>>> host whereas PCIe2 & PCIe3 are 2-lane Gen3 host.
>>>>>>
>>>>>> Co-developed-by: Anusha Rao <[email protected]>
>>>>>> Signed-off-by: Anusha Rao <[email protected]>
>>>>>> Signed-off-by: Devi Priya <[email protected]>
>>>>>> ---
>>>>>> Changes in V3:
>>>>>> - Fixed up the PCI I/O port ranges
>>>>>>
>>>>>> arch/arm64/boot/dts/qcom/ipq9574.dtsi | 375
>>>>>> +++++++++++++++++++++++++-
>>>>>> 1 file changed, 370 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>>>>> b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>>>>> index e757b57957cf..953a839a1141 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>>>>>> @@ -6,8 +6,8 @@
>>>>>> * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights
>>>>>> reserved.
>>>>>> */
>>>>>>
>>>>>> -#include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>>> #include <dt-bindings/clock/qcom,ipq9574-gcc.h>
>>>>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>>> #include <dt-bindings/reset/qcom,ipq9574-gcc.h>
>>>>>>
>>>>>> / {
>>>>>> @@ -116,6 +116,58 @@
>>>>>> #size-cells = <1>;
>>>>>> ranges = <0 0 0 0xffffffff>;
>>>>>>
>>>>>> + pcie0_phy: phy@84000 {
>>>>>> + compatible =
>>>>>> "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>>>>>> + reg = <0x00084000 0x1000>;
>>>>>> +
>>>>>> + clocks = <&gcc GCC_PCIE0_AUX_CLK>,
>>>>>> + <&gcc GCC_PCIE0_AHB_CLK>,
>>>>>> + <&gcc GCC_ANOC_PCIE0_1LANE_M_CLK>,
>>>>>> + <&gcc GCC_SNOC_PCIE0_1LANE_S_CLK>,
>>>>>> + <&gcc GCC_PCIE0_PIPE_CLK>;
>>>>>> + clock-names = "aux", "cfg_ahb", "anoc_lane",
>>>>>> "snoc_lane", "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";
>>>>>> +
>>>>>> + };
>>>>>> +
>>>>>> + pcie2_phy: phy@8c000 {
>>>>>> + compatible =
>>>>>> "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>>>>>> + reg = <0x0008c000 0x2000>;
>>>>>> +
>>>>>> + clocks = <&gcc GCC_PCIE2_AUX_CLK>,
>>>>>> + <&gcc GCC_PCIE2_AHB_CLK>,
>>>>>> + <&gcc GCC_ANOC_PCIE2_2LANE_M_CLK>,
>>>>>> + <&gcc GCC_SNOC_PCIE2_2LANE_S_CLK>,
>>>>>> + <&gcc GCC_PCIE2_PIPE_CLK>;
>>>>>> + clock-names = "aux", "cfg_ahb", "anoc_lane",
>>>>>> "snoc_lane", "pipe";
>>>>>> +
>>>>>> + assigned-clocks = <&gcc GCC_PCIE2_AUX_CLK>;
>>>>>> + assigned-clock-rates = <20000000>;
>>>>>> +
>>>>>> + resets = <&gcc GCC_PCIE2_PHY_BCR>,
>>>>>> + <&gcc GCC_PCIE2PHY_PHY_BCR>;
>>>>>> + reset-names = "phy", "common";
>>>>>> +
>>>>>> + #clock-cells = <0>;
>>>>>> + clock-output-names = "gcc_pcie2_pipe_clk_src";
>>>>>> +
>>>>>> + #phy-cells = <0>;
>>>>>> + status = "disabled";
>>>>>> +
>>>>>> + };
>>>>>> +
>>>>>> rng: rng@e3000 {
>>>>>> compatible = "qcom,prng-ee";
>>>>>> reg = <0x000e3000 0x1000>;
>>>>>> @@ -123,6 +175,58 @@
>>>>>> clock-names = "core";
>>>>>> };
>>>>>>
>>>>>> + pcie3_phy: phy@f4000 {
>>>>>> + compatible =
>>>>>> "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>>>>>> + reg = <0x000f4000 0x2000>;
>>>>>> +
>>>>>> + clocks = <&gcc GCC_PCIE3_AUX_CLK>,
>>>>>> + <&gcc GCC_PCIE3_AHB_CLK>,
>>>>>> + <&gcc GCC_ANOC_PCIE3_2LANE_M_CLK>,
>>>>>> + <&gcc GCC_SNOC_PCIE3_2LANE_S_CLK>,
>>>>>> + <&gcc GCC_PCIE3_PIPE_CLK>;
>>>>>> + clock-names = "aux", "cfg_ahb", "anoc_lane",
>>>>>> "snoc_lane", "pipe";
>>>>>> +
>>>>>> + assigned-clocks = <&gcc GCC_PCIE3_AUX_CLK>;
>>>>>> + assigned-clock-rates = <20000000>;
>>>>>> +
>>>>>> + resets = <&gcc GCC_PCIE3_PHY_BCR>,
>>>>>> + <&gcc GCC_PCIE3PHY_PHY_BCR>;
>>>>>> + reset-names = "phy", "common";
>>>>>> +
>>>>>> + #clock-cells = <0>;
>>>>>> + clock-output-names = "gcc_pcie3_pipe_clk_src";
>>>>>> +
>>>>>> + #phy-cells = <0>;
>>>>>> + status = "disabled";
>>>>>> +
>>>>>> + };
>>>>>> +
>>>>>> + pcie1_phy: phy@fc000 {
>>>>>> + compatible =
>>>>>> "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>>>>>> + reg = <0x000fc000 0x1000>;
>>>>>> +
>>>>>> + clocks = <&gcc GCC_PCIE1_AUX_CLK>,
>>>>>> + <&gcc GCC_PCIE1_AHB_CLK>,
>>>>>> + <&gcc GCC_ANOC_PCIE1_1LANE_M_CLK>,
>>>>>> + <&gcc GCC_SNOC_PCIE1_1LANE_S_CLK>,
>>>>>> + <&gcc GCC_PCIE1_PIPE_CLK>;
>>>>>> + clock-names = "aux", "cfg_ahb", "anoc_lane",
>>>>>> "snoc_lane", "pipe";
>>>>>> +
>>>>>> + assigned-clocks = <&gcc GCC_PCIE1_AUX_CLK>;
>>>>>> + assigned-clock-rates = <20000000>;
>>>>>> +
>>>>>> + 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,ipq9574-tlmm";
>>>>>> reg = <0x01000000 0x300000>;
>>>>>> @@ -146,10 +250,10 @@
>>>>>> reg = <0x01800000 0x80000>;
>>>>>> clocks = <&xo_board_clk>,
>>>>>> <&sleep_clk>,
>>>>>> - <0>,
>>>>>> - <0>,
>>>>>> - <0>,
>>>>>> - <0>,
>>>>>> + <&pcie0_phy>,
>>>>>> + <&pcie1_phy>,
>>>>>> + <&pcie2_phy>,
>>>>>> + <&pcie3_phy>,
>>>>>> <0>;
>>>>>> #clock-cells = <1>;
>>>>>> #reset-cells = <1>;
>>>>>> @@ -478,6 +582,267 @@
>>>>>> status = "disabled";
>>>>>> };
>>>>>> };
>>>>>> +
>>>>>> + pcie1: pci@10000000 {
>>>>>> + compatible = "qcom,pcie-ipq9574";
>>>>>> + reg = <0x10000000 0xf1d>,
>>>>>> + <0x10000F20 0xa8>,
>>>>>> + <0x10001000 0x1000>,
>>>>>> + <0x000F8000 0x4000>,
>>>>>> + <0x10100000 0x1000>;
>>>>>> + reg-names = "dbi", "elbi", "atu", "parf",
>>>>>> "config";
>>>>>> + device_type = "pci";
>>>>>> + linux,pci-domain = <2>;
>>>>>> + bus-range = <0x00 0xff>;
>>>>>> + num-lanes = <1>;
>>>>>> + #address-cells = <3>;
>>>>>> + #size-cells = <2>;
>>>>>> +
>>>>>> + ranges = <0x01000000 0x0 0x00000000
>>>>>> 0x10200000 0x0 0x100000>, /* I/O */
>>>>>> + <0x02000000 0x0 0x10300000
>>>>>> 0x10300000 0x0 0x7d00000>; /* MEM */
>>>>>> +
>>>>>> + #interrupt-cells = <1>;
>>>>>> + interrupt-map-mask = <0 0 0 0x7>;
>>>>>> + interrupt-map = <0 0 0 1 &intc 0 35
>>>>>> IRQ_TYPE_LEVEL_HIGH>, /* int_a */
>>>>>> + <0 0 0 2 &intc 0 49
>>>>>> IRQ_TYPE_LEVEL_HIGH>, /* int_b */
>>>>>> + <0 0 0 3 &intc 0 84
>>>>>> IRQ_TYPE_LEVEL_HIGH>, /* int_c */
>>>>>> + <0 0 0 4 &intc 0 85
>>>>>> IRQ_TYPE_LEVEL_HIGH>; /* int_d */
>>>>>> +
>>>>>
>>>>> No iommu-map?
>>>> We do not enable the IOMMU stage1 translation for PCIe and the registers
>>>> have secure access only from TrustZone (It enables only stage2 for
>>>> Access control)
>>>
>>> So, no SMMU protection for PCIe transactions? This sounds like a step
>>> backwards.
>> Yes, we are not using stage1 translations.
>
> We = software or we = hardware? If there is a hardware interface to
> SMMU, please describe it here.
>
Trustzone software protects all non-secure access to any SMMU register.
Hence it is not possible to enable stage 1 translation from HLOS.
HLOS touching any SMMU register would result in a 'secure access violation'

Thanks,
Devi Priya