2024-04-17 14:16:57

by Heiko Stuebner

[permalink] [raw]
Subject: [PATCH] arm64: dts: rockchip: add PCIe3 support on rk3588-jaguar

From: Heiko Stuebner <[email protected]>

The Jaguar SBC provides a M.2 slot connected to the pcie3 controller.
In contrast to a number of other boards the pcie-refclk is gpio-controlled,
so the necessary clock is added to the list of pcie3 clocks.

Signed-off-by: Heiko Stuebner <[email protected]>
---
.../arm64/boot/dts/rockchip/rk3588-jaguar.dts | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
index 5002105dc78e..908fbabd8b00 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
@@ -72,6 +72,25 @@ led-1 {
};
};

+ /*
+ * 100MHz reference clock for PCIe peripherals from PI6C557-05BLE
+ * clock generator.
+ * The clock output is gated via the OE pin on the clock generator.
+ * This is modeled as a fixed-clock plus a gpio-gate-clock.
+ */
+ pcie_refclk_gen: pcie-refclk-gen-clock {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <1000000000>;
+ };
+
+ pcie_refclk: pcie-refclk-clock {
+ compatible = "gpio-gate-clock";
+ clocks = <&pcie_refclk_gen>;
+ #clock-cells = <0>;
+ enable-gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>; /* PCIE30X4_CLKREQN_M0 */
+ };
+
pps {
compatible = "pps-gpio";
gpios = <&gpio0 RK_PD5 GPIO_ACTIVE_HIGH>;
@@ -466,6 +485,40 @@ &pcie2x1l0 {
status = "okay";
};

+&pcie30phy {
+ status = "okay";
+};
+
+&pcie30x4m0_pins {
+ /*
+ * pcie30x4_clkreqn_m0 is used by the refclk generator
+ * pcie30x4_perstn_m0 is used as via the reset-gpio
+ */
+ rockchip,pins =
+ /* pcie30x4_waken_m0 */
+ <0 RK_PC7 12 &pcfg_pull_none>;
+};
+
+&pcie3x4 {
+ /*
+ * The board has a gpio-controlled "pcie_refclk" generator,
+ * so add it to the list of clocks.
+ */
+ clocks = <&cru ACLK_PCIE_4L_MSTR>, <&cru ACLK_PCIE_4L_SLV>,
+ <&cru ACLK_PCIE_4L_DBI>, <&cru PCLK_PCIE_4L>,
+ <&cru CLK_PCIE_AUX0>, <&cru CLK_PCIE4L_PIPE>,
+ <&pcie_refclk>;
+ clock-names = "aclk_mst", "aclk_slv",
+ "aclk_dbi", "pclk",
+ "aux", "pipe",
+ "ref";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie30x4m0_pins>;
+ reset-gpios = <&gpio0 RK_PD0 GPIO_ACTIVE_HIGH>; /* PCIE30X4_PERSTN_M0 */
+ vpcie3v3-supply = <&vcc3v3_mdot2>;
+ status = "okay";
+};
+
&pinctrl {
emmc {
emmc_reset: emmc-reset {
--
2.39.2



2024-04-18 08:07:55

by Quentin Schulz

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: add PCIe3 support on rk3588-jaguar

Hi Heiko,

On 4/17/24 16:16, Heiko Stuebner wrote:
> From: Heiko Stuebner <[email protected]>
>
> The Jaguar SBC provides a M.2 slot connected to the pcie3 controller.
> In contrast to a number of other boards the pcie-refclk is gpio-controlled,
> so the necessary clock is added to the list of pcie3 clocks.
>
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> .../arm64/boot/dts/rockchip/rk3588-jaguar.dts | 53 +++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> index 5002105dc78e..908fbabd8b00 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> @@ -72,6 +72,25 @@ led-1 {
> };
> };
>
> + /*
> + * 100MHz reference clock for PCIe peripherals from PI6C557-05BLE
> + * clock generator.
> + * The clock output is gated via the OE pin on the clock generator.
> + * This is modeled as a fixed-clock plus a gpio-gate-clock.
> + */
> + pcie_refclk_gen: pcie-refclk-gen-clock {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1000000000>;
> + };
> +
> + pcie_refclk: pcie-refclk-clock {
> + compatible = "gpio-gate-clock";
> + clocks = <&pcie_refclk_gen>;
> + #clock-cells = <0>;
> + enable-gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>; /* PCIE30X4_CLKREQN_M0 */

I think we usually want to have the pinctrl for GPIOs as well to not
assume the pins are muxed in that function by default or by the bootloader?

> + };
> +
> pps {
> compatible = "pps-gpio";
> gpios = <&gpio0 RK_PD5 GPIO_ACTIVE_HIGH>;
> @@ -466,6 +485,40 @@ &pcie2x1l0 {
> status = "okay";
> };
>
> +&pcie30phy {
> + status = "okay";
> +};
> +
> +&pcie30x4m0_pins {

I'm wondering if it really makes sense to reuse this node if we're
planning to change the only property it has to mean something different?

> + /*
> + * pcie30x4_clkreqn_m0 is used by the refclk generator
> + * pcie30x4_perstn_m0 is used as via the reset-gpio
> + */
> + rockchip,pins =
> + /* pcie30x4_waken_m0 */
> + <0 RK_PC7 12 &pcfg_pull_none>;
> +};
> +
> +&pcie3x4 {
> + /*
> + * The board has a gpio-controlled "pcie_refclk" generator,
> + * so add it to the list of clocks.
> + */
> + clocks = <&cru ACLK_PCIE_4L_MSTR>, <&cru ACLK_PCIE_4L_SLV>,
> + <&cru ACLK_PCIE_4L_DBI>, <&cru PCLK_PCIE_4L>,
> + <&cru CLK_PCIE_AUX0>, <&cru CLK_PCIE4L_PIPE>,
> + <&pcie_refclk>;
> + clock-names = "aclk_mst", "aclk_slv",
> + "aclk_dbi", "pclk",
> + "aux", "pipe",
> + "ref";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie30x4m0_pins>;
> + reset-gpios = <&gpio0 RK_PD0 GPIO_ACTIVE_HIGH>; /* PCIE30X4_PERSTN_M0 */

Ditto, I assume we want to have a pinmux for that GPIO as well?

Cheers,
Quentin

2024-04-18 08:13:08

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: add PCIe3 support on rk3588-jaguar

Am Donnerstag, 18. April 2024, 10:07:37 CEST schrieb Quentin Schulz:
> On 4/17/24 16:16, Heiko Stuebner wrote:
> > From: Heiko Stuebner <[email protected]>
> >
> > The Jaguar SBC provides a M.2 slot connected to the pcie3 controller.
> > In contrast to a number of other boards the pcie-refclk is gpio-controlled,
> > so the necessary clock is added to the list of pcie3 clocks.
> >
> > Signed-off-by: Heiko Stuebner <[email protected]>
> > ---
> > .../arm64/boot/dts/rockchip/rk3588-jaguar.dts | 53 +++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> > index 5002105dc78e..908fbabd8b00 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
> > @@ -72,6 +72,25 @@ led-1 {
> > };
> > };
> >
> > + /*
> > + * 100MHz reference clock for PCIe peripherals from PI6C557-05BLE
> > + * clock generator.
> > + * The clock output is gated via the OE pin on the clock generator.
> > + * This is modeled as a fixed-clock plus a gpio-gate-clock.
> > + */
> > + pcie_refclk_gen: pcie-refclk-gen-clock {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <1000000000>;
> > + };
> > +
> > + pcie_refclk: pcie-refclk-clock {
> > + compatible = "gpio-gate-clock";
> > + clocks = <&pcie_refclk_gen>;
> > + #clock-cells = <0>;
> > + enable-gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>; /* PCIE30X4_CLKREQN_M0 */
>
> I think we usually want to have the pinctrl for GPIOs as well to not
> assume the pins are muxed in that function by default or by the bootloader?
>
> > + };
> > +
> > pps {
> > compatible = "pps-gpio";
> > gpios = <&gpio0 RK_PD5 GPIO_ACTIVE_HIGH>;
> > @@ -466,6 +485,40 @@ &pcie2x1l0 {
> > status = "okay";
> > };
> >
> > +&pcie30phy {
> > + status = "okay";
> > +};
> > +
> > +&pcie30x4m0_pins {
>
> I'm wondering if it really makes sense to reuse this node if we're
> planning to change the only property it has to mean something different?

ok, will create a specific node for the new pinctrl
As an added pro, this also moves it to the pinctrl area in the dts

> > + /*
> > + * pcie30x4_clkreqn_m0 is used by the refclk generator
> > + * pcie30x4_perstn_m0 is used as via the reset-gpio
> > + */
> > + rockchip,pins =
> > + /* pcie30x4_waken_m0 */
> > + <0 RK_PC7 12 &pcfg_pull_none>;
> > +};
> > +
> > +&pcie3x4 {
> > + /*
> > + * The board has a gpio-controlled "pcie_refclk" generator,
> > + * so add it to the list of clocks.
> > + */
> > + clocks = <&cru ACLK_PCIE_4L_MSTR>, <&cru ACLK_PCIE_4L_SLV>,
> > + <&cru ACLK_PCIE_4L_DBI>, <&cru PCLK_PCIE_4L>,
> > + <&cru CLK_PCIE_AUX0>, <&cru CLK_PCIE4L_PIPE>,
> > + <&pcie_refclk>;
> > + clock-names = "aclk_mst", "aclk_slv",
> > + "aclk_dbi", "pclk",
> > + "aux", "pipe",
> > + "ref";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pcie30x4m0_pins>;
> > + reset-gpios = <&gpio0 RK_PD0 GPIO_ACTIVE_HIGH>; /* PCIE30X4_PERSTN_M0 */
>
> Ditto, I assume we want to have a pinmux for that GPIO as well?

Correct. While the Rockchip pinctrl driver _does_ the muxing when a gpio
is requested, it wouldn't touch pinconf settings u-boot or whoever might
have set differently before.

Heiko



2024-04-18 08:28:14

by Dragan Simic

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: add PCIe3 support on rk3588-jaguar

Hello Heiko,

On 2024-04-18 10:12, Heiko Stübner wrote:
> Am Donnerstag, 18. April 2024, 10:07:37 CEST schrieb Quentin Schulz:
>> On 4/17/24 16:16, Heiko Stuebner wrote:
>> > From: Heiko Stuebner <[email protected]>
>> >
>> > The Jaguar SBC provides a M.2 slot connected to the pcie3 controller.

Just a minor nitpick... s/a M.2/an M.2/

>> > In contrast to a number of other boards the pcie-refclk is gpio-controlled,
>> > so the necessary clock is added to the list of pcie3 clocks.