diff mbox series

[7/7] arm64: dts: qcom: ipq9574: Add PCIe PHYs and controller nodes

Message ID 20230214164135.17039-8-quic_devipriy@quicinc.com
State New
Headers show
Series [1/7] dt-bindings: PCI: qcom: Add IPQ9574 specific compatible | expand

Commit Message

Devi Priya Feb. 14, 2023, 4:41 p.m. UTC 
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 <quic_anusha@quicinc.com>
Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts |  28 ++
 arch/arm64/boot/dts/qcom/ipq9574.dtsi        | 477 ++++++++++++++++++-
 2 files changed, 499 insertions(+), 6 deletions(-)

Comments

Sricharan R Feb. 17, 2023, 8:35 a.m. UTC | #1 
Hi Devi,

On 2/14/2023 10:11 PM, Devi Priya 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 <quic_anusha@quicinc.com>
> Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> ---
>   arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts |  28 ++
>   arch/arm64/boot/dts/qcom/ipq9574.dtsi        | 477 ++++++++++++++++++-
>   2 files changed, 499 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
> index 2c8430197ec0..21b53f34ce84 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
> @@ -8,6 +8,7 @@
>   
>   /dts-v1/;
>   
> +#include <dt-bindings/gpio/gpio.h>
>   #include "ipq9574.dtsi"
>   
>   / {
> @@ -29,6 +30,33 @@
>   	status = "okay";
>   };
>   
> +&pcie1_phy {
> +	status = "okay";
> +};
> +
> +&pcie1_x1 {
> +	perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
> +&pcie2_phy {
> +	status = "okay";
> +};
> +
> +&pcie2_x2 {
> +	perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
> +&pcie3_phy {
> +	status = "okay";
> +};
> +
> +&pcie3_x2 {
> +	perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
>   &sdhc_1 {
>   	pinctrl-0 = <&sdc_default_state>;
>   	pinctrl-names = "default";
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 062f80798ebb..a32dbdeb5bed 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>
>   
>   / {
> @@ -22,11 +22,41 @@
>   			#clock-cells = <0>;
>   		};
>   
> +		pcie30_phy0_pipe_clk: pcie30_phy0_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pcie30_phy1_pipe_clk: pcie30_phy1_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pcie30_phy2_pipe_clk: pcie30_phy2_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pcie30_phy3_pipe_clk: pcie30_phy3_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
>   		sleep_clk: sleep-clk {
>   			compatible = "fixed-clock";
>   			#clock-cells = <0>;
>   		};
>   
> +		usb3phy_0_cc_pipe_clk: usb3phy_0_cc_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <125000000>;
> +			#clock-cells = <0>;
> +		};
> +

   Why is the usb clock added here ?

>   		xo_board_clk: xo-board-clk {
>   			compatible = "fixed-clock";
>   			#clock-cells = <0>;
> @@ -121,6 +151,155 @@
>   		#size-cells = <1>;
>   		ranges = <0 0 0 0xffffffff>;
>   
> +		pcie0_phy: phy@84000 {
> +			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
> +			reg = <0x00084000 0x1bc>; /* Serdes PLL */
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +			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>;
> +			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
> +
> +			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";
> +
> +			status = "disabled";
> +
> +			pcie0_lane: phy@84200 {
> +				reg = <0x00084200 0x16c>, /* Serdes Tx */
> +				      <0x00084400 0x200>, /* Serdes Rx */
> +				      <0x00084800 0x1f0>, /* PCS: Lane0, COM, PCIE */
> +				      <0x00084c00 0xf4>;  /* pcs_misc */
> +				#phy-cells = <0>;
> +
> +				clocks = <&gcc GCC_PCIE0_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "gcc_pcie0_pipe_clk_src";
> +				#clock-cells = <0>;
> +			};
> +		};
> +
> +		pcie2_phy: phy@8c000 {

   Can the phy/pcie nodes labelled in order ?
   Currently it 0/2/3/1 ?

> +			compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> +			reg = <0x0008c000 0x1bc>; /* Serdes PLL */
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			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>;
> +			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
> +
> +			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";
> +
> +			status = "disabled";
> +
> +			pcie2_lanes: phy@8c200 {
> +				reg = <0x0008c200 0x16c>, /* Serdes Tx0 */
> +				      <0x0008c400 0x200>, /* Serdes Rx0 */
> +				      <0x0008d000 0x1f0>, /* PCS: Lane0, COM, PCIE */
> +				      <0x0008c600 0x16c>, /* Serdes Tx1 */
> +				      <0x0008c800 0x200>, /* Serdes Rx1 */
> +				      <0x0008d400 0x0f8>; /* pcs_misc */
> +
> +				#phy-cells = <0>;
> +
> +				clocks = <&gcc GCC_PCIE2_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "gcc_pcie2_pipe_clk_src";
> +				#clock-cells = <0>;
> +			};
> +		};
> +
> +		pcie3_phy: phy@f4000 {
> +			compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> +			reg = <0x000f4000 0x1bc>; /* Serdes PLL */
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			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>;
> +			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
> +
> +			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";
> +
> +			status = "disabled";
> +
> +			pcie3_lanes: phy@f4200 {
> +				reg = <0x000f4200 0x16c>, /* Serdes Tx0 */
> +				      <0x000f4400 0x200>, /* Serdes Rx0 */
> +				      <0x000f5000 0x1f0>, /* PCS: Lane0, COM, PCIE */
> +				      <0x000f4600 0x16c>, /* Serdes Tx1 */
> +				      <0x000f4800 0x200>, /* Serdes Rx1 */
> +				      <0x000f5400 0x0f8>; /* pcs_misc */
> +
> +				#phy-cells = <0>;
> +
> +				clocks = <&gcc GCC_PCIE3_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "gcc_pcie3_pipe_clk_src";
> +				#clock-cells = <0>;
> +			};
> +		};
> +
> +		pcie1_phy: phy@fc000 {
> +			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
> +			reg = <0x000fc000 0x1bc>; /* Serdes PLL */
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			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>;
> +			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
> +
> +			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";
> +
> +			status = "disabled";
> +
> +			pcie1_lane: phy@fc200 {
> +				reg = <0x000fc200 0x16c>, /* Serdes Tx */
> +				      <0x000fc400 0x200>, /* Serdes Rx */
> +				      <0x000fc800 0x1f0>, /* PCS: Lane0, COM, PCIE */
> +				      <0x000fcc00 0xf4>;  /* pcs_misc */
> +				#phy-cells = <0>;
> +
> +				clocks = <&gcc GCC_PCIE1_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "gcc_pcie1_pipe_clk_src";
> +				#clock-cells = <0>;
> +			};
> +		};
> +
>   		tlmm: pinctrl@1000000 {
>   			compatible = "qcom,ipq9574-tlmm";
>   			reg = <0x01000000 0x300000>;
> @@ -145,11 +324,11 @@
>   			clocks = <&xo_board_clk>,
>   				 <&sleep_clk>,
>   				 <&bias_pll_ubi_nc_clk>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <0>;
> +				 <&pcie30_phy0_pipe_clk>,
> +				 <&pcie30_phy1_pipe_clk>,
> +				 <&pcie30_phy2_pipe_clk>,
> +				 <&pcie30_phy3_pipe_clk>,
> +				 <&usb3phy_0_cc_pipe_clk>;

    Same , why usb3 clk is added here ?

Regards,
  Sricharan
Devi Priya Feb. 20, 2023, 1:47 p.m. UTC | #2 
On 2/17/2023 2:05 PM, Sricharan Ramabadhran wrote:
> 
> Hi Devi,
> 
> On 2/14/2023 10:11 PM, Devi Priya 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 <quic_anusha@quicinc.com>
>> Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
>> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts |  28 ++
>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi        | 477 ++++++++++++++++++-
>>   2 files changed, 499 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts 
>> b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
>> index 2c8430197ec0..21b53f34ce84 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
>> @@ -8,6 +8,7 @@
>>   /dts-v1/;
>> +#include <dt-bindings/gpio/gpio.h>
>>   #include "ipq9574.dtsi"
>>   / {
>> @@ -29,6 +30,33 @@
>>       status = "okay";
>>   };
>> +&pcie1_phy {
>> +    status = "okay";
>> +};
>> +
>> +&pcie1_x1 {
>> +    perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
>> +    status = "okay";
>> +};
>> +
>> +&pcie2_phy {
>> +    status = "okay";
>> +};
>> +
>> +&pcie2_x2 {
>> +    perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
>> +    status = "okay";
>> +};
>> +
>> +&pcie3_phy {
>> +    status = "okay";
>> +};
>> +
>> +&pcie3_x2 {
>> +    perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
>> +    status = "okay";
>> +};
>> +
>>   &sdhc_1 {
>>       pinctrl-0 = <&sdc_default_state>;
>>       pinctrl-names = "default";
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi 
>> b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> index 062f80798ebb..a32dbdeb5bed 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>
>>   / {
>> @@ -22,11 +22,41 @@
>>               #clock-cells = <0>;
>>           };
>> +        pcie30_phy0_pipe_clk: pcie30_phy0_pipe_clk {
>> +            compatible = "fixed-clock";
>> +            clock-frequency = <250000000>;
>> +            #clock-cells = <0>;
>> +        };
>> +
>> +        pcie30_phy1_pipe_clk: pcie30_phy1_pipe_clk {
>> +            compatible = "fixed-clock";
>> +            clock-frequency = <250000000>;
>> +            #clock-cells = <0>;
>> +        };
>> +
>> +        pcie30_phy2_pipe_clk: pcie30_phy2_pipe_clk {
>> +            compatible = "fixed-clock";
>> +            clock-frequency = <250000000>;
>> +            #clock-cells = <0>;
>> +        };
>> +
>> +        pcie30_phy3_pipe_clk: pcie30_phy3_pipe_clk {
>> +            compatible = "fixed-clock";
>> +            clock-frequency = <250000000>;
>> +            #clock-cells = <0>;
>> +        };
>> +
>>           sleep_clk: sleep-clk {
>>               compatible = "fixed-clock";
>>               #clock-cells = <0>;
>>           };
>> +        usb3phy_0_cc_pipe_clk: usb3phy_0_cc_pipe_clk {
>> +            compatible = "fixed-clock";
>> +            clock-frequency = <125000000>;
>> +            #clock-cells = <0>;
>> +        };
>> +
> 
>    Why is the usb clock added here ?
As we have brought in the QMP PHY support, added the usb pipe clock too.
Will drop it as it is unrelated to the series.
> 
>>           xo_board_clk: xo-board-clk {
>>               compatible = "fixed-clock";
>>               #clock-cells = <0>;
>> @@ -121,6 +151,155 @@
>>           #size-cells = <1>;
>>           ranges = <0 0 0 0xffffffff>;
>> +        pcie0_phy: phy@84000 {
>> +            compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>> +            reg = <0x00084000 0x1bc>; /* Serdes PLL */
>> +            #address-cells = <1>;
>> +            #size-cells = <1>;
>> +            ranges;
>> +            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>;
>> +            clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
>> +
>> +            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";
>> +
>> +            status = "disabled";
>> +
>> +            pcie0_lane: phy@84200 {
>> +                reg = <0x00084200 0x16c>, /* Serdes Tx */
>> +                      <0x00084400 0x200>, /* Serdes Rx */
>> +                      <0x00084800 0x1f0>, /* PCS: Lane0, COM, PCIE */
>> +                      <0x00084c00 0xf4>;  /* pcs_misc */
>> +                #phy-cells = <0>;
>> +
>> +                clocks = <&gcc GCC_PCIE0_PIPE_CLK>;
>> +                clock-names = "pipe0";
>> +                clock-output-names = "gcc_pcie0_pipe_clk_src";
>> +                #clock-cells = <0>;
>> +            };
>> +        };
>> +
>> +        pcie2_phy: phy@8c000 {
> 
>    Can the phy/pcie nodes labelled in order ?
>    Currently it 0/2/3/1 ?
Sure, okay
> 
>> +            compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>> +            reg = <0x0008c000 0x1bc>; /* Serdes PLL */
>> +            #address-cells = <1>;
>> +            #size-cells = <1>;
>> +            ranges;
>> +
>> +            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>;
>> +            clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
>> +
>> +            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";
>> +
>> +            status = "disabled";
>> +
>> +            pcie2_lanes: phy@8c200 {
>> +                reg = <0x0008c200 0x16c>, /* Serdes Tx0 */
>> +                      <0x0008c400 0x200>, /* Serdes Rx0 */
>> +                      <0x0008d000 0x1f0>, /* PCS: Lane0, COM, PCIE */
>> +                      <0x0008c600 0x16c>, /* Serdes Tx1 */
>> +                      <0x0008c800 0x200>, /* Serdes Rx1 */
>> +                      <0x0008d400 0x0f8>; /* pcs_misc */
>> +
>> +                #phy-cells = <0>;
>> +
>> +                clocks = <&gcc GCC_PCIE2_PIPE_CLK>;
>> +                clock-names = "pipe0";
>> +                clock-output-names = "gcc_pcie2_pipe_clk_src";
>> +                #clock-cells = <0>;
>> +            };
>> +        };
>> +
>> +        pcie3_phy: phy@f4000 {
>> +            compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>> +            reg = <0x000f4000 0x1bc>; /* Serdes PLL */
>> +            #address-cells = <1>;
>> +            #size-cells = <1>;
>> +            ranges;
>> +
>> +            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>;
>> +            clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
>> +
>> +            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";
>> +
>> +            status = "disabled";
>> +
>> +            pcie3_lanes: phy@f4200 {
>> +                reg = <0x000f4200 0x16c>, /* Serdes Tx0 */
>> +                      <0x000f4400 0x200>, /* Serdes Rx0 */
>> +                      <0x000f5000 0x1f0>, /* PCS: Lane0, COM, PCIE */
>> +                      <0x000f4600 0x16c>, /* Serdes Tx1 */
>> +                      <0x000f4800 0x200>, /* Serdes Rx1 */
>> +                      <0x000f5400 0x0f8>; /* pcs_misc */
>> +
>> +                #phy-cells = <0>;
>> +
>> +                clocks = <&gcc GCC_PCIE3_PIPE_CLK>;
>> +                clock-names = "pipe0";
>> +                clock-output-names = "gcc_pcie3_pipe_clk_src";
>> +                #clock-cells = <0>;
>> +            };
>> +        };
>> +
>> +        pcie1_phy: phy@fc000 {
>> +            compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>> +            reg = <0x000fc000 0x1bc>; /* Serdes PLL */
>> +            #address-cells = <1>;
>> +            #size-cells = <1>;
>> +            ranges;
>> +
>> +            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>;
>> +            clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
>> +
>> +            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";
>> +
>> +            status = "disabled";
>> +
>> +            pcie1_lane: phy@fc200 {
>> +                reg = <0x000fc200 0x16c>, /* Serdes Tx */
>> +                      <0x000fc400 0x200>, /* Serdes Rx */
>> +                      <0x000fc800 0x1f0>, /* PCS: Lane0, COM, PCIE */
>> +                      <0x000fcc00 0xf4>;  /* pcs_misc */
>> +                #phy-cells = <0>;
>> +
>> +                clocks = <&gcc GCC_PCIE1_PIPE_CLK>;
>> +                clock-names = "pipe0";
>> +                clock-output-names = "gcc_pcie1_pipe_clk_src";
>> +                #clock-cells = <0>;
>> +            };
>> +        };
>> +
>>           tlmm: pinctrl@1000000 {
>>               compatible = "qcom,ipq9574-tlmm";
>>               reg = <0x01000000 0x300000>;
>> @@ -145,11 +324,11 @@
>>               clocks = <&xo_board_clk>,
>>                    <&sleep_clk>,
>>                    <&bias_pll_ubi_nc_clk>,
>> -                 <0>,
>> -                 <0>,
>> -                 <0>,
>> -                 <0>,
>> -                 <0>;
>> +                 <&pcie30_phy0_pipe_clk>,
>> +                 <&pcie30_phy1_pipe_clk>,
>> +                 <&pcie30_phy2_pipe_clk>,
>> +                 <&pcie30_phy3_pipe_clk>,
>> +                 <&usb3phy_0_cc_pipe_clk>;
> 
>     Same , why usb3 clk is added here ?
Sure, will drop it
> 
> Regards,
>   Sricharan
> 
Best Regards,
Devi Priya
Kathiravan Thirumoorthy Feb. 24, 2023, 6:57 a.m. UTC | #3 
On 2/14/2023 10:11 PM, Devi Priya 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 <quic_anusha@quicinc.com>
> Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> ---
>   arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts |  28 ++
>   arch/arm64/boot/dts/qcom/ipq9574.dtsi        | 477 ++++++++++++++++++-
>   2 files changed, 499 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
> index 2c8430197ec0..21b53f34ce84 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
> @@ -8,6 +8,7 @@
>   
>   /dts-v1/;
>   
> +#include <dt-bindings/gpio/gpio.h>
>   #include "ipq9574.dtsi"
>   
>   / {
> @@ -29,6 +30,33 @@
>   	status = "okay";
>   };
>   
> +&pcie1_phy {
> +	status = "okay";
> +};
> +
> +&pcie1_x1 {
> +	perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
> +&pcie2_phy {
> +	status = "okay";
> +};
> +
> +&pcie2_x2 {
> +	perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
> +&pcie3_phy {
> +	status = "okay";
> +};
> +
> +&pcie3_x2 {
> +	perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
>   &sdhc_1 {
>   	pinctrl-0 = <&sdc_default_state>;
>   	pinctrl-names = "default";
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 062f80798ebb..a32dbdeb5bed 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>
>   
>   / {
> @@ -22,11 +22,41 @@
>   			#clock-cells = <0>;
>   		};
>   
> +		pcie30_phy0_pipe_clk: pcie30_phy0_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pcie30_phy1_pipe_clk: pcie30_phy1_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pcie30_phy2_pipe_clk: pcie30_phy2_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pcie30_phy3_pipe_clk: pcie30_phy3_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
>   		sleep_clk: sleep-clk {
>   			compatible = "fixed-clock";
>   			#clock-cells = <0>;
>   		};
>   
> +		usb3phy_0_cc_pipe_clk: usb3phy_0_cc_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <125000000>;
> +			#clock-cells = <0>;
> +		};
> +
>   		xo_board_clk: xo-board-clk {
>   			compatible = "fixed-clock";
>   			#clock-cells = <0>;
> @@ -121,6 +151,155 @@
>   		#size-cells = <1>;
>   		ranges = <0 0 0 0xffffffff>;
>   
> +		pcie0_phy: phy@84000 {
> +			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
> +			reg = <0x00084000 0x1bc>; /* Serdes PLL */
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +			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>;
> +			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
> +
> +			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";
> +
> +			status = "disabled";
> +
> +			pcie0_lane: phy@84200 {
> +				reg = <0x00084200 0x16c>, /* Serdes Tx */
> +				      <0x00084400 0x200>, /* Serdes Rx */
> +				      <0x00084800 0x1f0>, /* PCS: Lane0, COM, PCIE */
> +				      <0x00084c00 0xf4>;  /* pcs_misc */
> +				#phy-cells = <0>;
> +
> +				clocks = <&gcc GCC_PCIE0_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "gcc_pcie0_pipe_clk_src";
> +				#clock-cells = <0>;
> +			};
> +		};
> +
> +		pcie2_phy: phy@8c000 {
> +			compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> +			reg = <0x0008c000 0x1bc>; /* Serdes PLL */
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			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>;
> +			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
> +
> +			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";
> +
> +			status = "disabled";
> +
> +			pcie2_lanes: phy@8c200 {
> +				reg = <0x0008c200 0x16c>, /* Serdes Tx0 */
> +				      <0x0008c400 0x200>, /* Serdes Rx0 */
> +				      <0x0008d000 0x1f0>, /* PCS: Lane0, COM, PCIE */
> +				      <0x0008c600 0x16c>, /* Serdes Tx1 */
> +				      <0x0008c800 0x200>, /* Serdes Rx1 */
> +				      <0x0008d400 0x0f8>; /* pcs_misc */
> +
> +				#phy-cells = <0>;
> +
> +				clocks = <&gcc GCC_PCIE2_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "gcc_pcie2_pipe_clk_src";
> +				#clock-cells = <0>;
> +			};
> +		};
> +
> +		pcie3_phy: phy@f4000 {
> +			compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
> +			reg = <0x000f4000 0x1bc>; /* Serdes PLL */
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			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>;
> +			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
> +
> +			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";
> +
> +			status = "disabled";
> +
> +			pcie3_lanes: phy@f4200 {
> +				reg = <0x000f4200 0x16c>, /* Serdes Tx0 */
> +				      <0x000f4400 0x200>, /* Serdes Rx0 */
> +				      <0x000f5000 0x1f0>, /* PCS: Lane0, COM, PCIE */
> +				      <0x000f4600 0x16c>, /* Serdes Tx1 */
> +				      <0x000f4800 0x200>, /* Serdes Rx1 */
> +				      <0x000f5400 0x0f8>; /* pcs_misc */
> +
> +				#phy-cells = <0>;
> +
> +				clocks = <&gcc GCC_PCIE3_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "gcc_pcie3_pipe_clk_src";
> +				#clock-cells = <0>;
> +			};
> +		};
> +
> +		pcie1_phy: phy@fc000 {
> +			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
> +			reg = <0x000fc000 0x1bc>; /* Serdes PLL */
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			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>;
> +			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
> +
> +			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";
> +
> +			status = "disabled";
> +
> +			pcie1_lane: phy@fc200 {
> +				reg = <0x000fc200 0x16c>, /* Serdes Tx */
> +				      <0x000fc400 0x200>, /* Serdes Rx */
> +				      <0x000fc800 0x1f0>, /* PCS: Lane0, COM, PCIE */
> +				      <0x000fcc00 0xf4>;  /* pcs_misc */
> +				#phy-cells = <0>;
> +
> +				clocks = <&gcc GCC_PCIE1_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "gcc_pcie1_pipe_clk_src";
> +				#clock-cells = <0>;
> +			};
> +		};
> +
>   		tlmm: pinctrl@1000000 {
>   			compatible = "qcom,ipq9574-tlmm";
>   			reg = <0x01000000 0x300000>;
> @@ -145,11 +324,11 @@
>   			clocks = <&xo_board_clk>,
>   				 <&sleep_clk>,
>   				 <&bias_pll_ubi_nc_clk>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <0>;
> +				 <&pcie30_phy0_pipe_clk>,
> +				 <&pcie30_phy1_pipe_clk>,
> +				 <&pcie30_phy2_pipe_clk>,
> +				 <&pcie30_phy3_pipe_clk>,
> +				 <&usb3phy_0_cc_pipe_clk>;


pipe clock source is PHY. So should we add the pcie_phy phandle here and 
use it like how it is done in other targets, ex: sm8550.dtsi?


>   			#clock-cells = <1>;
>   			#reset-cells = <1>;
>   			#power-domain-cells = <1>;
> @@ -282,6 +461,292 @@
>   				status = "disabled";
>   			};
>   		};
> +
> +		pcie1_x1: pci@10000000 {
> +			compatible = "qcom,pcie-ipq9574";
> +			reg =  <0x10000000 0xf1d>,
> +			       <0x10000F20 0xa8>,
> +			       <0x10001000 0x1000>,
> +			       <0x000F8000 0x4000>,
> +			       <0x10100000 0x1000>,
> +			       <0x00618108 0x4>;
> +			reg-names = "dbi", "elbi", "atu", "parf", "config", "aggr_noc";
> +			device_type = "pci";
> +			linux,pci-domain = <2>;
> +			bus-range = <0x00 0xff>;
> +			num-lanes = <1>;
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +
> +			ranges = <0x81000000 0 0x10200000 0x10200000
> +				  0 0x00100000   /* downstream I/O */
> +				  0x82000000 0 0x10300000 0x10300000
> +				  0 0x07d00000>; /* non-prefetchable memory */
> +
> +			#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";


Controller driver doesn't support the "global_irq" yet. Please correct 
me If I am wrong.


Thanks, Kathiravan T.
Manivannan Sadhasivam Feb. 24, 2023, 8:59 a.m. UTC | #4 
On Tue, Feb 14, 2023 at 10:11:35PM +0530, Devi Priya 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.
> 

Please split the board devicetree changes into a separate patch.

> Co-developed-by: Anusha Rao <quic_anusha@quicinc.com>
> Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts |  28 ++
>  arch/arm64/boot/dts/qcom/ipq9574.dtsi        | 477 ++++++++++++++++++-
>  2 files changed, 499 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
> index 2c8430197ec0..21b53f34ce84 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
> @@ -8,6 +8,7 @@
>  
>  /dts-v1/;
>  
> +#include <dt-bindings/gpio/gpio.h>
>  #include "ipq9574.dtsi"
>  
>  / {
> @@ -29,6 +30,33 @@
>  	status = "okay";
>  };
>  
> +&pcie1_phy {
> +	status = "okay";

No PHY power supply needed? Same comment for rest of the PHY nodes.

> +};
> +
> +&pcie1_x1 {

No need to add a suffix to node label indicating the lane config.

> +	perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;

What about "wake" pin? Don't you need pinctrl definitions for these GPIOs?
Same comment for rest of the PCIe nodes.

> +	status = "okay";
> +};
> +
> +&pcie2_phy {
> +	status = "okay";
> +};
> +
> +&pcie2_x2 {
> +	perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
> +&pcie3_phy {
> +	status = "okay";
> +};
> +
> +&pcie3_x2 {
> +	perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
>  &sdhc_1 {
>  	pinctrl-0 = <&sdc_default_state>;
>  	pinctrl-names = "default";
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 062f80798ebb..a32dbdeb5bed 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>
>  
>  / {
> @@ -22,11 +22,41 @@
>  			#clock-cells = <0>;
>  		};
>  
> +		pcie30_phy0_pipe_clk: pcie30_phy0_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pcie30_phy1_pipe_clk: pcie30_phy1_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pcie30_phy2_pipe_clk: pcie30_phy2_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		pcie30_phy3_pipe_clk: pcie30_phy3_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};

Why PIPE clocks are modeled as fixed clocks unlike other SoCs?

> +
>  		sleep_clk: sleep-clk {
>  			compatible = "fixed-clock";
>  			#clock-cells = <0>;
>  		};
>  
> +		usb3phy_0_cc_pipe_clk: usb3phy_0_cc_pipe_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <125000000>;
> +			#clock-cells = <0>;
> +		};

Spurious?

> +
>  		xo_board_clk: xo-board-clk {
>  			compatible = "fixed-clock";
>  			#clock-cells = <0>;
> @@ -121,6 +151,155 @@
>  		#size-cells = <1>;
>  		ranges = <0 0 0 0xffffffff>;
>  
> +		pcie0_phy: phy@84000 {
> +			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
> +			reg = <0x00084000 0x1bc>; /* Serdes PLL */
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +			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>;
> +			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";

Care to explain what these anoc_lane and snoc_lane clocks are?

> +
> +			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";
> +
> +			status = "disabled";
> +
> +			pcie0_lane: phy@84200 {
> +				reg = <0x00084200 0x16c>, /* Serdes Tx */
> +				      <0x00084400 0x200>, /* Serdes Rx */
> +				      <0x00084800 0x1f0>, /* PCS: Lane0, COM, PCIE */
> +				      <0x00084c00 0xf4>;  /* pcs_misc */
> +				#phy-cells = <0>;
> +
> +				clocks = <&gcc GCC_PCIE0_PIPE_CLK>;
> +				clock-names = "pipe0";
> +				clock-output-names = "gcc_pcie0_pipe_clk_src";
> +				#clock-cells = <0>;
> +			};
> +		};
> +

[...]

> +		pcie1_x1: pci@10000000 {
> +			compatible = "qcom,pcie-ipq9574";
> +			reg =  <0x10000000 0xf1d>,
> +			       <0x10000F20 0xa8>,
> +			       <0x10001000 0x1000>,
> +			       <0x000F8000 0x4000>,
> +			       <0x10100000 0x1000>,
> +			       <0x00618108 0x4>;
> +			reg-names = "dbi", "elbi", "atu", "parf", "config", "aggr_noc";

As I asked in the binding patch, why "aggr_noc" region is required?

> +			device_type = "pci";
> +			linux,pci-domain = <2>;
> +			bus-range = <0x00 0xff>;
> +			num-lanes = <1>;
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +
> +			ranges = <0x81000000 0 0x10200000 0x10200000
> +				  0 0x00100000   /* downstream I/O */
> +				  0x82000000 0 0x10300000 0x10300000
> +				  0 0x07d00000>; /* non-prefetchable memory */

Don't split the ranges and encode them in a single line.

Also, the I'm not sure why you have set the relocatable flag (n) for both
ranges i.e., in 0x81000000 and 0x82000000.

> +
> +			#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 */
> +

Again, wrap the interrupts in a single line.

> +			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "global_irq";
> +

Linux doesn't support global_irq yet. But since devicetree is supposed to
describe the hardware, you can keep it.

Above comment applies to rest of the PCIe nodes.

> +			/* 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_lane>;
> +			phy-names = "pciephy";
> +			msi-parent = <&v2m0>;
> +			status = "disabled";
> +		};
> +

[...]

> +		pcie2_x2: 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>;

Space after =

Thanks,
Mani
Devi Priya March 3, 2023, 12:09 p.m. UTC | #5 
On 2/24/2023 12:27 PM, Kathiravan T wrote:
> 
> On 2/14/2023 10:11 PM, Devi Priya 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 <quic_anusha@quicinc.com>
>> Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
>> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts |  28 ++
>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi        | 477 ++++++++++++++++++-
>>   2 files changed, 499 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts 
>> b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
>> index 2c8430197ec0..21b53f34ce84 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
>> @@ -8,6 +8,7 @@
>>   /dts-v1/;
>> +#include <dt-bindings/gpio/gpio.h>
>>   #include "ipq9574.dtsi"
>>   / {
>> @@ -29,6 +30,33 @@
>>       status = "okay";
>>   };
>> +&pcie1_phy {
>> +    status = "okay";
>> +};
>> +
>> +&pcie1_x1 {
>> +    perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
>> +    status = "okay";
>> +};
>> +
>> +&pcie2_phy {
>> +    status = "okay";
>> +};
>> +
>> +&pcie2_x2 {
>> +    perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
>> +    status = "okay";
>> +};
>> +
>> +&pcie3_phy {
>> +    status = "okay";
>> +};
>> +
>> +&pcie3_x2 {
>> +    perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
>> +    status = "okay";
>> +};
>> +
>>   &sdhc_1 {
>>       pinctrl-0 = <&sdc_default_state>;
>>       pinctrl-names = "default";
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi 
>> b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> index 062f80798ebb..a32dbdeb5bed 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>
>>   / {
>> @@ -22,11 +22,41 @@
>>               #clock-cells = <0>;
>>           };
>> +        pcie30_phy0_pipe_clk: pcie30_phy0_pipe_clk {
>> +            compatible = "fixed-clock";
>> +            clock-frequency = <250000000>;
>> +            #clock-cells = <0>;
>> +        };
>> +
>> +        pcie30_phy1_pipe_clk: pcie30_phy1_pipe_clk {
>> +            compatible = "fixed-clock";
>> +            clock-frequency = <250000000>;
>> +            #clock-cells = <0>;
>> +        };
>> +
>> +        pcie30_phy2_pipe_clk: pcie30_phy2_pipe_clk {
>> +            compatible = "fixed-clock";
>> +            clock-frequency = <250000000>;
>> +            #clock-cells = <0>;
>> +        };
>> +
>> +        pcie30_phy3_pipe_clk: pcie30_phy3_pipe_clk {
>> +            compatible = "fixed-clock";
>> +            clock-frequency = <250000000>;
>> +            #clock-cells = <0>;
>> +        };
>> +
>>           sleep_clk: sleep-clk {
>>               compatible = "fixed-clock";
>>               #clock-cells = <0>;
>>           };
>> +        usb3phy_0_cc_pipe_clk: usb3phy_0_cc_pipe_clk {
>> +            compatible = "fixed-clock";
>> +            clock-frequency = <125000000>;
>> +            #clock-cells = <0>;
>> +        };
>> +
>>           xo_board_clk: xo-board-clk {
>>               compatible = "fixed-clock";
>>               #clock-cells = <0>;
>> @@ -121,6 +151,155 @@
>>           #size-cells = <1>;
>>           ranges = <0 0 0 0xffffffff>;
>> +        pcie0_phy: phy@84000 {
>> +            compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>> +            reg = <0x00084000 0x1bc>; /* Serdes PLL */
>> +            #address-cells = <1>;
>> +            #size-cells = <1>;
>> +            ranges;
>> +            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>;
>> +            clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
>> +
>> +            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";
>> +
>> +            status = "disabled";
>> +
>> +            pcie0_lane: phy@84200 {
>> +                reg = <0x00084200 0x16c>, /* Serdes Tx */
>> +                      <0x00084400 0x200>, /* Serdes Rx */
>> +                      <0x00084800 0x1f0>, /* PCS: Lane0, COM, PCIE */
>> +                      <0x00084c00 0xf4>;  /* pcs_misc */
>> +                #phy-cells = <0>;
>> +
>> +                clocks = <&gcc GCC_PCIE0_PIPE_CLK>;
>> +                clock-names = "pipe0";
>> +                clock-output-names = "gcc_pcie0_pipe_clk_src";
>> +                #clock-cells = <0>;
>> +            };
>> +        };
>> +
>> +        pcie2_phy: phy@8c000 {
>> +            compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>> +            reg = <0x0008c000 0x1bc>; /* Serdes PLL */
>> +            #address-cells = <1>;
>> +            #size-cells = <1>;
>> +            ranges;
>> +
>> +            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>;
>> +            clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
>> +
>> +            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";
>> +
>> +            status = "disabled";
>> +
>> +            pcie2_lanes: phy@8c200 {
>> +                reg = <0x0008c200 0x16c>, /* Serdes Tx0 */
>> +                      <0x0008c400 0x200>, /* Serdes Rx0 */
>> +                      <0x0008d000 0x1f0>, /* PCS: Lane0, COM, PCIE */
>> +                      <0x0008c600 0x16c>, /* Serdes Tx1 */
>> +                      <0x0008c800 0x200>, /* Serdes Rx1 */
>> +                      <0x0008d400 0x0f8>; /* pcs_misc */
>> +
>> +                #phy-cells = <0>;
>> +
>> +                clocks = <&gcc GCC_PCIE2_PIPE_CLK>;
>> +                clock-names = "pipe0";
>> +                clock-output-names = "gcc_pcie2_pipe_clk_src";
>> +                #clock-cells = <0>;
>> +            };
>> +        };
>> +
>> +        pcie3_phy: phy@f4000 {
>> +            compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
>> +            reg = <0x000f4000 0x1bc>; /* Serdes PLL */
>> +            #address-cells = <1>;
>> +            #size-cells = <1>;
>> +            ranges;
>> +
>> +            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>;
>> +            clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
>> +
>> +            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";
>> +
>> +            status = "disabled";
>> +
>> +            pcie3_lanes: phy@f4200 {
>> +                reg = <0x000f4200 0x16c>, /* Serdes Tx0 */
>> +                      <0x000f4400 0x200>, /* Serdes Rx0 */
>> +                      <0x000f5000 0x1f0>, /* PCS: Lane0, COM, PCIE */
>> +                      <0x000f4600 0x16c>, /* Serdes Tx1 */
>> +                      <0x000f4800 0x200>, /* Serdes Rx1 */
>> +                      <0x000f5400 0x0f8>; /* pcs_misc */
>> +
>> +                #phy-cells = <0>;
>> +
>> +                clocks = <&gcc GCC_PCIE3_PIPE_CLK>;
>> +                clock-names = "pipe0";
>> +                clock-output-names = "gcc_pcie3_pipe_clk_src";
>> +                #clock-cells = <0>;
>> +            };
>> +        };
>> +
>> +        pcie1_phy: phy@fc000 {
>> +            compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>> +            reg = <0x000fc000 0x1bc>; /* Serdes PLL */
>> +            #address-cells = <1>;
>> +            #size-cells = <1>;
>> +            ranges;
>> +
>> +            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>;
>> +            clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
>> +
>> +            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";
>> +
>> +            status = "disabled";
>> +
>> +            pcie1_lane: phy@fc200 {
>> +                reg = <0x000fc200 0x16c>, /* Serdes Tx */
>> +                      <0x000fc400 0x200>, /* Serdes Rx */
>> +                      <0x000fc800 0x1f0>, /* PCS: Lane0, COM, PCIE */
>> +                      <0x000fcc00 0xf4>;  /* pcs_misc */
>> +                #phy-cells = <0>;
>> +
>> +                clocks = <&gcc GCC_PCIE1_PIPE_CLK>;
>> +                clock-names = "pipe0";
>> +                clock-output-names = "gcc_pcie1_pipe_clk_src";
>> +                #clock-cells = <0>;
>> +            };
>> +        };
>> +
>>           tlmm: pinctrl@1000000 {
>>               compatible = "qcom,ipq9574-tlmm";
>>               reg = <0x01000000 0x300000>;
>> @@ -145,11 +324,11 @@
>>               clocks = <&xo_board_clk>,
>>                    <&sleep_clk>,
>>                    <&bias_pll_ubi_nc_clk>,
>> -                 <0>,
>> -                 <0>,
>> -                 <0>,
>> -                 <0>,
>> -                 <0>;
>> +                 <&pcie30_phy0_pipe_clk>,
>> +                 <&pcie30_phy1_pipe_clk>,
>> +                 <&pcie30_phy2_pipe_clk>,
>> +                 <&pcie30_phy3_pipe_clk>,
>> +                 <&usb3phy_0_cc_pipe_clk>;
> 
> 
> pipe clock source is PHY. So should we add the pcie_phy phandle here and 
> use it like how it is done in other targets, ex: sm8550.dtsi?
Yes sure kathir. Will update this in the next series
> 
> 
>>               #clock-cells = <1>;
>>               #reset-cells = <1>;
>>               #power-domain-cells = <1>;
>> @@ -282,6 +461,292 @@
>>                   status = "disabled";
>>               };
>>           };
>> +
>> +        pcie1_x1: pci@10000000 {
>> +            compatible = "qcom,pcie-ipq9574";
>> +            reg =  <0x10000000 0xf1d>,
>> +                   <0x10000F20 0xa8>,
>> +                   <0x10001000 0x1000>,
>> +                   <0x000F8000 0x4000>,
>> +                   <0x10100000 0x1000>,
>> +                   <0x00618108 0x4>;
>> +            reg-names = "dbi", "elbi", "atu", "parf", "config", 
>> "aggr_noc";
>> +            device_type = "pci";
>> +            linux,pci-domain = <2>;
>> +            bus-range = <0x00 0xff>;
>> +            num-lanes = <1>;
>> +            #address-cells = <3>;
>> +            #size-cells = <2>;
>> +
>> +            ranges = <0x81000000 0 0x10200000 0x10200000
>> +                  0 0x00100000   /* downstream I/O */
>> +                  0x82000000 0 0x10300000 0x10300000
>> +                  0 0x07d00000>; /* non-prefetchable memory */
>> +
>> +            #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";
> 
> 
> Controller driver doesn't support the "global_irq" yet. Please correct 
> me If I am wrong.
> 
> 
> Thanks, Kathiravan T.
> 
Regards,
Devi Priya
Devi Priya March 7, 2023, 2:42 p.m. UTC | #6 
On 2/24/2023 2:29 PM, Manivannan Sadhasivam wrote:
> On Tue, Feb 14, 2023 at 10:11:35PM +0530, Devi Priya 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.
>>
> 
> Please split the board devicetree changes into a separate patch.
Sure, okay
> 
>> Co-developed-by: Anusha Rao <quic_anusha@quicinc.com>
>> Signed-off-by: Anusha Rao <quic_anusha@quicinc.com>
>> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts |  28 ++
>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi        | 477 ++++++++++++++++++-
>>   2 files changed, 499 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
>> index 2c8430197ec0..21b53f34ce84 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
>> @@ -8,6 +8,7 @@
>>   
>>   /dts-v1/;
>>   
>> +#include <dt-bindings/gpio/gpio.h>
>>   #include "ipq9574.dtsi"
>>   
>>   / {
>> @@ -29,6 +30,33 @@
>>   	status = "okay";
>>   };
>>   
>> +&pcie1_phy {
>> +	status = "okay";
> 
> No PHY power supply needed? Same comment for rest of the PHY nodes.
The PHY power supplies (VDDA_0P9 and VDDA_1P8) would be turned 'on'
by default and so no supply is added here

> 
>> +};
>> +
>> +&pcie1_x1 {
> 
> No need to add a suffix to node label indicating the lane config.
Okay
> 
>> +	perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
> 
> What about "wake" pin? Don't you need pinctrl definitions for these GPIOs?
> Same comment for rest of the PCIe nodes.
In IPQ9574, Wake pin isn't required as the slave devices are not
hot-pluggable & they get enumerated during the bootup. Will add the
pinctrl definition for the Perst gpio in V2
> 
>> +	status = "okay";
>> +};
>> +
>> +&pcie2_phy {
>> +	status = "okay";
>> +};
>> +
>> +&pcie2_x2 {
>> +	perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
>> +	status = "okay";
>> +};
>> +
>> +&pcie3_phy {
>> +	status = "okay";
>> +};
>> +
>> +&pcie3_x2 {
>> +	perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
>> +	status = "okay";
>> +};
>> +
>>   &sdhc_1 {
>>   	pinctrl-0 = <&sdc_default_state>;
>>   	pinctrl-names = "default";
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> index 062f80798ebb..a32dbdeb5bed 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>
>>   
>>   / {
>> @@ -22,11 +22,41 @@
>>   			#clock-cells = <0>;
>>   		};
>>   
>> +		pcie30_phy0_pipe_clk: pcie30_phy0_pipe_clk {
>> +			compatible = "fixed-clock";
>> +			clock-frequency = <250000000>;
>> +			#clock-cells = <0>;
>> +		};
>> +
>> +		pcie30_phy1_pipe_clk: pcie30_phy1_pipe_clk {
>> +			compatible = "fixed-clock";
>> +			clock-frequency = <250000000>;
>> +			#clock-cells = <0>;
>> +		};
>> +
>> +		pcie30_phy2_pipe_clk: pcie30_phy2_pipe_clk {
>> +			compatible = "fixed-clock";
>> +			clock-frequency = <250000000>;
>> +			#clock-cells = <0>;
>> +		};
>> +
>> +		pcie30_phy3_pipe_clk: pcie30_phy3_pipe_clk {
>> +			compatible = "fixed-clock";
>> +			clock-frequency = <250000000>;
>> +			#clock-cells = <0>;
>> +		};
> 
> Why PIPE clocks are modeled as fixed clocks unlike other SoCs?
Sure, will add the clocks to corresponding PHY node and use the phandle
similar to other targets
> 
>> +
>>   		sleep_clk: sleep-clk {
>>   			compatible = "fixed-clock";
>>   			#clock-cells = <0>;
>>   		};
>>   
>> +		usb3phy_0_cc_pipe_clk: usb3phy_0_cc_pipe_clk {
>> +			compatible = "fixed-clock";
>> +			clock-frequency = <125000000>;
>> +			#clock-cells = <0>;
>> +		};
> 
> Spurious?
Will drop it
> 
>> +
>>   		xo_board_clk: xo-board-clk {
>>   			compatible = "fixed-clock";
>>   			#clock-cells = <0>;
>> @@ -121,6 +151,155 @@
>>   		#size-cells = <1>;
>>   		ranges = <0 0 0 0xffffffff>;
>>   
>> +		pcie0_phy: phy@84000 {
>> +			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
>> +			reg = <0x00084000 0x1bc>; /* Serdes PLL */
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			ranges;
>> +			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>;
>> +			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
> 
> Care to explain what these anoc_lane and snoc_lane clocks are?
snoc & anoc lane clocks are used in the SNOC/ANOC Network Interface Unit
(NIU) which connects to the corresponding PCIE master/slave interface
> 
>> +
>> +			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";
>> +
>> +			status = "disabled";
>> +
>> +			pcie0_lane: phy@84200 {
>> +				reg = <0x00084200 0x16c>, /* Serdes Tx */
>> +				      <0x00084400 0x200>, /* Serdes Rx */
>> +				      <0x00084800 0x1f0>, /* PCS: Lane0, COM, PCIE */
>> +				      <0x00084c00 0xf4>;  /* pcs_misc */
>> +				#phy-cells = <0>;
>> +
>> +				clocks = <&gcc GCC_PCIE0_PIPE_CLK>;
>> +				clock-names = "pipe0";
>> +				clock-output-names = "gcc_pcie0_pipe_clk_src";
>> +				#clock-cells = <0>;
>> +			};
>> +		};
>> +
> 
> [...]
> 
>> +		pcie1_x1: pci@10000000 {
>> +			compatible = "qcom,pcie-ipq9574";
>> +			reg =  <0x10000000 0xf1d>,
>> +			       <0x10000F20 0xa8>,
>> +			       <0x10001000 0x1000>,
>> +			       <0x000F8000 0x4000>,
>> +			       <0x10100000 0x1000>,
>> +			       <0x00618108 0x4>;
>> +			reg-names = "dbi", "elbi", "atu", "parf", "config", "aggr_noc";
> 
> As I asked in the binding patch, why "aggr_noc" region is required?
The ANOC runs at a fixed frequency of 342MHz.
For the connected PCIe slave devices that run at lesser frequency,
the aggr_noc's rate adapter register is updated to configure
the packet transmission rate to ensure no wait cycles are inserted.
Can we use the 'syscon' property here to set the rate adapter?
> 
>> +			device_type = "pci";
>> +			linux,pci-domain = <2>;
>> +			bus-range = <0x00 0xff>;
>> +			num-lanes = <1>;
>> +			#address-cells = <3>;
>> +			#size-cells = <2>;
>> +
>> +			ranges = <0x81000000 0 0x10200000 0x10200000
>> +				  0 0x00100000   /* downstream I/O */
>> +				  0x82000000 0 0x10300000 0x10300000
>> +				  0 0x07d00000>; /* non-prefetchable memory */
> 
> Don't split the ranges and encode them in a single line.
Okay
> 
> Also, the I'm not sure why you have set the relocatable flag (n) for both
> ranges i.e., in 0x81000000 and 0x82000000.
Will check and add comment in V2
> 
>> +
>> +			#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 */
>> +
> 
> Again, wrap the interrupts in a single line.
Sure, okay
> 
>> +			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-names = "global_irq";
>> +
> 
> Linux doesn't support global_irq yet. But since devicetree is supposed to
> describe the hardware, you can keep it.
Okay
> 
> Above comment applies to rest of the PCIe nodes.
> 
>> +			/* 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_lane>;
>> +			phy-names = "pciephy";
>> +			msi-parent = <&v2m0>;
>> +			status = "disabled";
>> +		};
>> +
> 
> [...]
> 
>> +		pcie2_x2: 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>;
> 
> Space after =
Sure, okay
> 
> Thanks,
> Mani
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
index 2c8430197ec0..21b53f34ce84 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
+++ b/arch/arm64/boot/dts/qcom/ipq9574-al02-c7.dts
@@ -8,6 +8,7 @@ 
 
 /dts-v1/;
 
+#include <dt-bindings/gpio/gpio.h>
 #include "ipq9574.dtsi"
 
 / {
@@ -29,6 +30,33 @@ 
 	status = "okay";
 };
 
+&pcie1_phy {
+	status = "okay";
+};
+
+&pcie1_x1 {
+	perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};
+
+&pcie2_phy {
+	status = "okay";
+};
+
+&pcie2_x2 {
+	perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};
+
+&pcie3_phy {
+	status = "okay";
+};
+
+&pcie3_x2 {
+	perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};
+
 &sdhc_1 {
 	pinctrl-0 = <&sdc_default_state>;
 	pinctrl-names = "default";
diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 062f80798ebb..a32dbdeb5bed 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>
 
 / {
@@ -22,11 +22,41 @@ 
 			#clock-cells = <0>;
 		};
 
+		pcie30_phy0_pipe_clk: pcie30_phy0_pipe_clk {
+			compatible = "fixed-clock";
+			clock-frequency = <250000000>;
+			#clock-cells = <0>;
+		};
+
+		pcie30_phy1_pipe_clk: pcie30_phy1_pipe_clk {
+			compatible = "fixed-clock";
+			clock-frequency = <250000000>;
+			#clock-cells = <0>;
+		};
+
+		pcie30_phy2_pipe_clk: pcie30_phy2_pipe_clk {
+			compatible = "fixed-clock";
+			clock-frequency = <250000000>;
+			#clock-cells = <0>;
+		};
+
+		pcie30_phy3_pipe_clk: pcie30_phy3_pipe_clk {
+			compatible = "fixed-clock";
+			clock-frequency = <250000000>;
+			#clock-cells = <0>;
+		};
+
 		sleep_clk: sleep-clk {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
 		};
 
+		usb3phy_0_cc_pipe_clk: usb3phy_0_cc_pipe_clk {
+			compatible = "fixed-clock";
+			clock-frequency = <125000000>;
+			#clock-cells = <0>;
+		};
+
 		xo_board_clk: xo-board-clk {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
@@ -121,6 +151,155 @@ 
 		#size-cells = <1>;
 		ranges = <0 0 0 0xffffffff>;
 
+		pcie0_phy: phy@84000 {
+			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
+			reg = <0x00084000 0x1bc>; /* Serdes PLL */
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			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>;
+			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
+
+			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";
+
+			status = "disabled";
+
+			pcie0_lane: phy@84200 {
+				reg = <0x00084200 0x16c>, /* Serdes Tx */
+				      <0x00084400 0x200>, /* Serdes Rx */
+				      <0x00084800 0x1f0>, /* PCS: Lane0, COM, PCIE */
+				      <0x00084c00 0xf4>;  /* pcs_misc */
+				#phy-cells = <0>;
+
+				clocks = <&gcc GCC_PCIE0_PIPE_CLK>;
+				clock-names = "pipe0";
+				clock-output-names = "gcc_pcie0_pipe_clk_src";
+				#clock-cells = <0>;
+			};
+		};
+
+		pcie2_phy: phy@8c000 {
+			compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
+			reg = <0x0008c000 0x1bc>; /* Serdes PLL */
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			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>;
+			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
+
+			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";
+
+			status = "disabled";
+
+			pcie2_lanes: phy@8c200 {
+				reg = <0x0008c200 0x16c>, /* Serdes Tx0 */
+				      <0x0008c400 0x200>, /* Serdes Rx0 */
+				      <0x0008d000 0x1f0>, /* PCS: Lane0, COM, PCIE */
+				      <0x0008c600 0x16c>, /* Serdes Tx1 */
+				      <0x0008c800 0x200>, /* Serdes Rx1 */
+				      <0x0008d400 0x0f8>; /* pcs_misc */
+
+				#phy-cells = <0>;
+
+				clocks = <&gcc GCC_PCIE2_PIPE_CLK>;
+				clock-names = "pipe0";
+				clock-output-names = "gcc_pcie2_pipe_clk_src";
+				#clock-cells = <0>;
+			};
+		};
+
+		pcie3_phy: phy@f4000 {
+			compatible = "qcom,ipq9574-qmp-gen3x2-pcie-phy";
+			reg = <0x000f4000 0x1bc>; /* Serdes PLL */
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			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>;
+			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
+
+			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";
+
+			status = "disabled";
+
+			pcie3_lanes: phy@f4200 {
+				reg = <0x000f4200 0x16c>, /* Serdes Tx0 */
+				      <0x000f4400 0x200>, /* Serdes Rx0 */
+				      <0x000f5000 0x1f0>, /* PCS: Lane0, COM, PCIE */
+				      <0x000f4600 0x16c>, /* Serdes Tx1 */
+				      <0x000f4800 0x200>, /* Serdes Rx1 */
+				      <0x000f5400 0x0f8>; /* pcs_misc */
+
+				#phy-cells = <0>;
+
+				clocks = <&gcc GCC_PCIE3_PIPE_CLK>;
+				clock-names = "pipe0";
+				clock-output-names = "gcc_pcie3_pipe_clk_src";
+				#clock-cells = <0>;
+			};
+		};
+
+		pcie1_phy: phy@fc000 {
+			compatible = "qcom,ipq9574-qmp-gen3x1-pcie-phy";
+			reg = <0x000fc000 0x1bc>; /* Serdes PLL */
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			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>;
+			clock-names = "aux", "cfg_ahb", "anoc_lane", "snoc_lane";
+
+			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";
+
+			status = "disabled";
+
+			pcie1_lane: phy@fc200 {
+				reg = <0x000fc200 0x16c>, /* Serdes Tx */
+				      <0x000fc400 0x200>, /* Serdes Rx */
+				      <0x000fc800 0x1f0>, /* PCS: Lane0, COM, PCIE */
+				      <0x000fcc00 0xf4>;  /* pcs_misc */
+				#phy-cells = <0>;
+
+				clocks = <&gcc GCC_PCIE1_PIPE_CLK>;
+				clock-names = "pipe0";
+				clock-output-names = "gcc_pcie1_pipe_clk_src";
+				#clock-cells = <0>;
+			};
+		};
+
 		tlmm: pinctrl@1000000 {
 			compatible = "qcom,ipq9574-tlmm";
 			reg = <0x01000000 0x300000>;
@@ -145,11 +324,11 @@ 
 			clocks = <&xo_board_clk>,
 				 <&sleep_clk>,
 				 <&bias_pll_ubi_nc_clk>,
-				 <0>,
-				 <0>,
-				 <0>,
-				 <0>,
-				 <0>;
+				 <&pcie30_phy0_pipe_clk>,
+				 <&pcie30_phy1_pipe_clk>,
+				 <&pcie30_phy2_pipe_clk>,
+				 <&pcie30_phy3_pipe_clk>,
+				 <&usb3phy_0_cc_pipe_clk>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 			#power-domain-cells = <1>;
@@ -282,6 +461,292 @@ 
 				status = "disabled";
 			};
 		};
+
+		pcie1_x1: pci@10000000 {
+			compatible = "qcom,pcie-ipq9574";
+			reg =  <0x10000000 0xf1d>,
+			       <0x10000F20 0xa8>,
+			       <0x10001000 0x1000>,
+			       <0x000F8000 0x4000>,
+			       <0x10100000 0x1000>,
+			       <0x00618108 0x4>;
+			reg-names = "dbi", "elbi", "atu", "parf", "config", "aggr_noc";
+			device_type = "pci";
+			linux,pci-domain = <2>;
+			bus-range = <0x00 0xff>;
+			num-lanes = <1>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			ranges = <0x81000000 0 0x10200000 0x10200000
+				  0 0x00100000   /* downstream I/O */
+				  0x82000000 0 0x10300000 0x10300000
+				  0 0x07d00000>; /* non-prefetchable memory */
+
+			#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_lane>;
+			phy-names = "pciephy";
+			msi-parent = <&v2m0>;
+			status = "disabled";
+		};
+
+		pcie3_x2: 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 = <0x81000000 0 0x18200000 0x18200000
+				  0 0x00100000   /* downstream I/O */
+				  0x82000000 0 0x18300000 0x18300000
+				  0 0x07d00000>; /* non-prefetchable memory */
+
+			#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_lanes>;
+			phy-names = "pciephy";
+			msi-parent = <&v2m0>;
+			status = "disabled";
+		};
+
+		pcie2_x2: 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 = <0x81000000 0 0x20200000 0x20200000
+				  0 0x00100000   /* downstream I/O */
+				  0x82000000 0 0x20300000 0x20300000
+				  0 0x07d00000>; /* non-prefetchable memory */
+
+			#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_lanes>;
+			phy-names = "pciephy";
+			msi-parent = <&v2m0>;
+			status = "disabled";
+		};
+
+		pcie0_x1: pci@28000000 {
+			compatible = "qcom,pcie-ipq9574";
+			reg =  <0x28000000 0xf1d>,
+			       <0x28000F20 0xa8>,
+			       <0x28001000 0x1000>,
+			       <0x00080000 0x2000>,
+			       <0x28100000 0x1000>,
+			       <0x00618088 0x4>;
+			reg-names = "dbi", "elbi", "atu", "parf", "config", "aggr_noc";
+			device_type = "pci";
+			linux,pci-domain = <1>;
+			bus-range = <0x00 0xff>;
+			num-lanes = <1>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+
+			ranges = <0x81000000 0 0x28200000 0x28200000
+				  0 0x00100000   /* downstream I/O */
+				  0x82000000 0 0x28300000 0x28300000
+				  0 0x07d00000>; /* non-prefetchable memory */
+
+			#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_lane>;
+			phy-names = "pciephy";
+			msi-parent = <&v2m0>;
+			status = "disabled";
+		};
 	};
 
 	timer {