Skip to contentSkip to conversation

Turing RK1 SoM DT updates

PATCH Applied Sam Edwards 3 participants 11 messages Sep 12, 2024 – Sep 30, 2024

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover Turing RK1 SoM DT updates

Hello list,

It's been a little under a year since support for the Turing RK1 RK3588-based SoM board, was introduced upstream. Since then, the driver developers have had great successes in further RK3588 enablement, achieving such enhancements as:
- Stable thermal ADC support
- USBDP PHY support
- Panthor: a driver for the Mali-G610 GPU found in RK3588

Accordingly I am pleased to present this DT patchset, which leverages those driver advancements to enable the corresponding features in the Turing RK1:
- Automatic fan speed control (patch 3/5)
- USB support (patch 4/5)
- GPU support (patch 5/5)

It also includes a bugfix (patch 2/5) for a previously-discussed [1] PCIe boot hang problem, for which it was decided [2] that a fix required first splitting apart the pinctrl defs for the low-speed signals (patch 1/5).

These patches are ordered by importance. Please apply this series partially if deemed necessary.

Thank you for your time,
Sam

[1]: https://lore.kernel.org/lkml/[email protected]/
[2]: https://lore.kernel.org/lkml/6311244.MhkbZ0Pkbq@diego/

Sam Edwards (5):
arm64: dts: rockchip: Split up RK3588's PCIe pinctrls
arm64: dts: rockchip: Fix Turing RK1 PCIe3 hang
arm64: dts: rockchip: Enable automatic fan control on Turing RK1
arm64: dts: rockchip: Enable all 3 USBs on Turing RK1
arm64: dts: rockchip: Enable GPU on Turing RK1

 .../dts/rockchip/rk3588-base-pinctrl.dtsi     | 271 ++++++++++++++----
 .../boot/dts/rockchip/rk3588-turing-rk1.dtsi  | 137 ++++++++-
 .../boot/dts/rockchip/rk3588s-rock-5a.dts     |   6 +-
 3 files changed, 363 insertions(+), 51 deletions(-)
Signature · 1 line
2.44.2
#1 lore raw

PATCH 1/5 arm64: dts: rockchip: Split up RK3588's PCIe pinctrls

These pinctrls manage the low-speed PCIe signals:
- CLKREQ#: An output on the RK3588 (both RC or EP modes), used to request that external clock-generation circuitry provide a clock.
- PERST#: An input on the RK3588 in EP mode, used to detect a reset signal from the RC. In RC mode, the hardware does not use this signal:
Linux itself generates it by putting the pin in GPIO mode.
- WAKE#: In EP mode, this is an output; in RC mode, this is an input.

Each of these signals serves a distinct purpose, and more importantly, PERST# should not be muxed when the RK3588 is in the RC role. Bundling them together in pinctrl groups prevents proper use: indeed, almost none of the current board-specific .dts files make any use of them. (Exception: Rock 5A recently had a patch land that misuses _pins; this
patch corrects that.)

However, on some RK3588 boards, the PCIe 3 controller will indefinitely stall the boot if CLKREQ# is not muxed (details in the next patch). This patch unbundles the signals to allow them to be used.

---
 .../dts/rockchip/rk3588-base-pinctrl.dtsi     | 271 ++++++++++++++----
 .../boot/dts/rockchip/rk3588s-rock-5a.dts     |   6 +-
 2 files changed, 228 insertions(+), 49 deletions(-)
arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi +223 -48
index d1368418502a..7f874c77410c 100644
@@ -1612,23 +1612,43 @@ npu_pins: npu-pins {
 
 	pcie20x1 {
 		/omit-if-no-ref/
-		pcie20x1m0_pins: pcie20x1m0-pins {
+		pcie20x1m0_clkreqn: pcie20x1m0-clkreqn {
 			rockchip,pins =
 				/* pcie20x1_2_clkreqn_m0 */
-				<3 RK_PC7 4 &pcfg_pull_none>,
+				<3 RK_PC7 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie20x1m0_perstn: pcie20x1m0-perstn {
+			rockchip,pins =
 				/* pcie20x1_2_perstn_m0 */
-				<3 RK_PD1 4 &pcfg_pull_none>,
+				<3 RK_PD1 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie20x1m0_waken: pcie20x1m0-waken {
+			rockchip,pins =
 				/* pcie20x1_2_waken_m0 */
 				<3 RK_PD0 4 &pcfg_pull_none>;
 		};
 
 		/omit-if-no-ref/
-		pcie20x1m1_pins: pcie20x1m1-pins {
+		pcie20x1m1_clkreqn: pcie20x1m1-clkreqn {
 			rockchip,pins =
 				/* pcie20x1_2_clkreqn_m1 */
-				<4 RK_PB7 4 &pcfg_pull_none>,
+				<4 RK_PB7 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie20x1m1_perstn: pcie20x1m1-perstn {
+			rockchip,pins =
 				/* pcie20x1_2_perstn_m1 */
-				<4 RK_PC1 4 &pcfg_pull_none>,
+				<4 RK_PC1 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie20x1m1_waken: pcie20x1m1-waken {
+			rockchip,pins =
 				/* pcie20x1_2_waken_m1 */
 				<4 RK_PC0 4 &pcfg_pull_none>;
 		};
@@ -1654,52 +1674,127 @@ pcie30phy_pins: pcie30phy-pins {
 
 	pcie30x1 {
 		/omit-if-no-ref/
-		pcie30x1m0_pins: pcie30x1m0-pins {
+		pcie30x1m0_0_clkreqn: pcie30x1m0-0-clkreqn {
 			rockchip,pins =
 				/* pcie30x1_0_clkreqn_m0 */
-				<0 RK_PC0 12 &pcfg_pull_none>,
+				<0 RK_PC0 12 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m0_0_perstn: pcie30x1m0-0-perstn {
+			rockchip,pins =
 				/* pcie30x1_0_perstn_m0 */
-				<0 RK_PC5 12 &pcfg_pull_none>,
+				<0 RK_PC5 12 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m0_0_waken: pcie30x1m0-0-waken {
+			rockchip,pins =
 				/* pcie30x1_0_waken_m0 */
-				<0 RK_PC4 12 &pcfg_pull_none>,
+				<0 RK_PC4 12 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m0_1_clkreqn: pcie30x1m0-1-clkreqn {
+			rockchip,pins =
 				/* pcie30x1_1_clkreqn_m0 */
-				<0 RK_PB5 12 &pcfg_pull_none>,
+				<0 RK_PB5 12 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m0_1_perstn: pcie30x1m0-1-perstn {
+			rockchip,pins =
 				/* pcie30x1_1_perstn_m0 */
-				<0 RK_PB7 12 &pcfg_pull_none>,
+				<0 RK_PB7 12 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m0_1_waken: pcie30x1m0-1-waken {
+			rockchip,pins =
 				/* pcie30x1_1_waken_m0 */
 				<0 RK_PB6 12 &pcfg_pull_none>;
 		};
 
 		/omit-if-no-ref/
-		pcie30x1m1_pins: pcie30x1m1-pins {
+		pcie30x1m1_0_clkreqn: pcie30x1m1-0-clkreqn {
 			rockchip,pins =
 				/* pcie30x1_0_clkreqn_m1 */
-				<4 RK_PA3 4 &pcfg_pull_none>,
+				<4 RK_PA3 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m1_0_perstn: pcie30x1m1-0-perstn {
+			rockchip,pins =
 				/* pcie30x1_0_perstn_m1 */
-				<4 RK_PA5 4 &pcfg_pull_none>,
+				<4 RK_PA5 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m1_0_waken: pcie30x1m1-0-waken {
+			rockchip,pins =
 				/* pcie30x1_0_waken_m1 */
-				<4 RK_PA4 4 &pcfg_pull_none>,
+				<4 RK_PA4 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m1_1_clkreqn: pcie30x1m1-1-clkreqn {
+			rockchip,pins =
 				/* pcie30x1_1_clkreqn_m1 */
-				<4 RK_PA0 4 &pcfg_pull_none>,
+				<4 RK_PA0 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m1_1_perstn: pcie30x1m1-1-perstn {
+			rockchip,pins =
 				/* pcie30x1_1_perstn_m1 */
-				<4 RK_PA2 4 &pcfg_pull_none>,
+				<4 RK_PA2 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m1_1_waken: pcie30x1m1-1-waken {
+			rockchip,pins =
 				/* pcie30x1_1_waken_m1 */
 				<4 RK_PA1 4 &pcfg_pull_none>;
 		};
 
 		/omit-if-no-ref/
-		pcie30x1m2_pins: pcie30x1m2-pins {
+		pcie30x1m2_0_clkreqn: pcie30x1m2-0-clkreqn {
 			rockchip,pins =
 				/* pcie30x1_0_clkreqn_m2 */
-				<1 RK_PB5 4 &pcfg_pull_none>,
+				<1 RK_PB5 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m2_0_perstn: pcie30x1m2-0-perstn {
+			rockchip,pins =
 				/* pcie30x1_0_perstn_m2 */
-				<1 RK_PB4 4 &pcfg_pull_none>,
+				<1 RK_PB4 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m2_0_waken: pcie30x1m2-0-waken {
+			rockchip,pins =
 				/* pcie30x1_0_waken_m2 */
-				<1 RK_PB3 4 &pcfg_pull_none>,
+				<1 RK_PB3 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m2_1_clkreqn: pcie30x1m2-1-clkreqn {
+			rockchip,pins =
 				/* pcie30x1_1_clkreqn_m2 */
-				<1 RK_PA0 4 &pcfg_pull_none>,
+				<1 RK_PA0 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m2_1_perstn: pcie30x1m2-1-perstn {
+			rockchip,pins =
 				/* pcie30x1_1_perstn_m2 */
-				<1 RK_PA7 4 &pcfg_pull_none>,
+				<1 RK_PA7 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x1m2_1_waken: pcie30x1m2-1-waken {
+			rockchip,pins =
 				/* pcie30x1_1_waken_m2 */
 				<1 RK_PA1 4 &pcfg_pull_none>;
 		};
@@ -1721,45 +1816,85 @@ pcie30x1_1_button_rstn: pcie30x1-1-button-rstn {
 
 	pcie30x2 {
 		/omit-if-no-ref/
-		pcie30x2m0_pins: pcie30x2m0-pins {
+		pcie30x2m0_clkreqn: pcie30x2m0-clkreqn {
 			rockchip,pins =
 				/* pcie30x2_clkreqn_m0 */
-				<0 RK_PD1 12 &pcfg_pull_none>,
+				<0 RK_PD1 12 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x2m0_perstn: pcie30x2m0-perstn {
+			rockchip,pins =
 				/* pcie30x2_perstn_m0 */
-				<0 RK_PD4 12 &pcfg_pull_none>,
+				<0 RK_PD4 12 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x2m0_waken: pcie30x2m0-waken {
+			rockchip,pins =
 				/* pcie30x2_waken_m0 */
 				<0 RK_PD2 12 &pcfg_pull_none>;
 		};
 
 		/omit-if-no-ref/
-		pcie30x2m1_pins: pcie30x2m1-pins {
+		pcie30x2m1_clkreqn: pcie30x2m1-clkreqn {
 			rockchip,pins =
 				/* pcie30x2_clkreqn_m1 */
-				<4 RK_PA6 4 &pcfg_pull_none>,
+				<4 RK_PA6 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x2m1_perstn: pcie30x2m1-perstn {
+			rockchip,pins =
 				/* pcie30x2_perstn_m1 */
-				<4 RK_PB0 4 &pcfg_pull_none>,
+				<4 RK_PB0 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x2m1_waken: pcie30x2m1-waken {
+			rockchip,pins =
 				/* pcie30x2_waken_m1 */
 				<4 RK_PA7 4 &pcfg_pull_none>;
 		};
 
 		/omit-if-no-ref/
-		pcie30x2m2_pins: pcie30x2m2-pins {
+		pcie30x2m2_clkreqn: pcie30x2m2-clkreqn {
 			rockchip,pins =
 				/* pcie30x2_clkreqn_m2 */
-				<3 RK_PD2 4 &pcfg_pull_none>,
+				<3 RK_PD2 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x2m2_perstn: pcie30x2m2-perstn {
+			rockchip,pins =
 				/* pcie30x2_perstn_m2 */
-				<3 RK_PD4 4 &pcfg_pull_none>,
+				<3 RK_PD4 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x2m2_waken: pcie30x2m2-waken {
+			rockchip,pins =
 				/* pcie30x2_waken_m2 */
 				<3 RK_PD3 4 &pcfg_pull_none>;
 		};
 
 		/omit-if-no-ref/
-		pcie30x2m3_pins: pcie30x2m3-pins {
+		pcie30x2m3_clkreqn: pcie30x2m3-clkreqn {
 			rockchip,pins =
 				/* pcie30x2_clkreqn_m3 */
-				<1 RK_PD7 4 &pcfg_pull_none>,
+				<1 RK_PD7 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x2m3_perstn: pcie30x2m3-perstn {
+			rockchip,pins =
 				/* pcie30x2_perstn_m3 */
-				<1 RK_PB7 4 &pcfg_pull_none>,
+				<1 RK_PB7 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x2m3_waken: pcie30x2m3-waken {
+			rockchip,pins =
 				/* pcie30x2_waken_m3 */
 				<1 RK_PB6 4 &pcfg_pull_none>;
 		};
@@ -1774,45 +1909,85 @@ pcie30x2_button_rstn: pcie30x2-button-rstn {
 
 	pcie30x4 {
 		/omit-if-no-ref/
-		pcie30x4m0_pins: pcie30x4m0-pins {
+		pcie30x4m0_clkreqn: pcie30x4m0-clkreqn {
 			rockchip,pins =
 				/* pcie30x4_clkreqn_m0 */
-				<0 RK_PC6 12 &pcfg_pull_none>,
+				<0 RK_PC6 12 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x4m0_perstn: pcie30x4m0-perstn {
+			rockchip,pins =
 				/* pcie30x4_perstn_m0 */
-				<0 RK_PD0 12 &pcfg_pull_none>,
+				<0 RK_PD0 12 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x4m0_waken: pcie30x4m0-waken {
+			rockchip,pins =
 				/* pcie30x4_waken_m0 */
 				<0 RK_PC7 12 &pcfg_pull_none>;
 		};
 
 		/omit-if-no-ref/
-		pcie30x4m1_pins: pcie30x4m1-pins {
+		pcie30x4m1_clkreqn: pcie30x4m1-clkreqn {
 			rockchip,pins =
 				/* pcie30x4_clkreqn_m1 */
-				<4 RK_PB4 4 &pcfg_pull_none>,
+				<4 RK_PB4 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x4m1_perstn: pcie30x4m1-perstn {
+			rockchip,pins =
 				/* pcie30x4_perstn_m1 */
-				<4 RK_PB6 4 &pcfg_pull_none>,
+				<4 RK_PB6 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x4m1_waken: pcie30x4m1-waken {
+			rockchip,pins =
 				/* pcie30x4_waken_m1 */
 				<4 RK_PB5 4 &pcfg_pull_none>;
 		};
 
 		/omit-if-no-ref/
-		pcie30x4m2_pins: pcie30x4m2-pins {
+		pcie30x4m2_clkreqn: pcie30x4m2-clkreqn {
 			rockchip,pins =
 				/* pcie30x4_clkreqn_m2 */
-				<3 RK_PC4 4 &pcfg_pull_none>,
+				<3 RK_PC4 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x4m2_perstn: pcie30x4m2-perstn {
+			rockchip,pins =
 				/* pcie30x4_perstn_m2 */
-				<3 RK_PC6 4 &pcfg_pull_none>,
+				<3 RK_PC6 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x4m2_waken: pcie30x4m2-waken {
+			rockchip,pins =
 				/* pcie30x4_waken_m2 */
 				<3 RK_PC5 4 &pcfg_pull_none>;
 		};
 
 		/omit-if-no-ref/
-		pcie30x4m3_pins: pcie30x4m3-pins {
+		pcie30x4m3_clkreqn: pcie30x4m3-clkreqn {
 			rockchip,pins =
 				/* pcie30x4_clkreqn_m3 */
-				<1 RK_PB0 4 &pcfg_pull_none>,
+				<1 RK_PB0 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x4m3_perstn: pcie30x4m3-perstn {
+			rockchip,pins =
 				/* pcie30x4_perstn_m3 */
-				<1 RK_PB2 4 &pcfg_pull_none>,
+				<1 RK_PB2 4 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		pcie30x4m3_waken: pcie30x4m3-waken {
+			rockchip,pins =
 				/* pcie30x4_waken_m3 */
 				<1 RK_PB1 4 &pcfg_pull_none>;
 		};
arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts +5 -1
index 294b99dd50da..87fce8d9a964 100644
@@ -310,7 +310,7 @@ rgmii_phy1: ethernet-phy@1 {
 };
 
 &pcie2x1l2 {
-	pinctrl-0 = <&pcie20x1m0_pins>;
+	pinctrl-0 = <&pcie2_reset>, <&pcie20x1m0_clkreqn>, <&pcie20x1m0_waken>;
 	pinctrl-names = "default";
 	reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>;
 	vpcie3v3-supply = <&vcc3v3_wf>;
@@ -328,6 +328,10 @@ pcie {
 		pow_en: pow-en {
 			rockchip,pins = <0 RK_PC5 RK_FUNC_GPIO &pcfg_pull_none>;
 		};
+
+		pcie2_reset: pcie2-reset {
+			rockchip,pins = <3 RK_PD1 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
 	};
 
 	power {
Signature · 1 line
2.44.2
#2 lore raw

PATCH 2/5 arm64: dts: rockchip: Fix Turing RK1 PCIe3 hang

In reply to Sam Edwards

The PCIe 3 PHY in the RK3588 requires a running external reference clock for both external bus transfers and some internal PIPE operations. Without this clock, the PCIe3 controller fails to initialize and ignores DBI transactions indefinitely, which stalls the Linux boot process.

On most RK3588 boards, this is evidently not an issue. But on some "SoM" designs (Turing RK1, Mixtile Core 3588E, ArmSoM AIM7, to name a few), this clock is only provided when the CLKREQ# signal is asserted.

The PCIe 3 PHY generates the CLKREQ# signal when it knows it needs the reference clock for proper operation. Unfortunately, the current DT for Turing RK1 does not mux out these low-speed signals, resulting in broken boots and potentially other issues.

This patch, following the previous one that split up the PCIe pinctrls, resolves this problem for Turing RK1 by explicitly muxing all of the signals needed for PCIe 2 and 3 support.

Cc Jonathan Bennett <[email protected]>
Fixes 2806a69f3f ("arm64: dts: rockchip: Add Turing RK1 SoM support")
---
 arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +2 -2
index dbaa94ca69f4..9bcb5acdea54 100644
@@ -211,7 +211,7 @@ rgmii_phy: ethernet-phy@1 {
 &pcie2x1l1 {
 	linux,pci-domain = <1>;
 	pinctrl-names = "default";
-	pinctrl-0 = <&pcie2_reset>;
+	pinctrl-0 = <&pcie2_reset>, <&pcie30x1m1_0_clkreqn>, <&pcie30x1m1_0_waken>;
 	reset-gpios = <&gpio4 RK_PA2 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
@@ -223,7 +223,7 @@ &pcie30phy {
 &pcie3x4 {
 	linux,pci-domain = <0>;
 	pinctrl-names = "default";
-	pinctrl-0 = <&pcie3_reset>;
+	pinctrl-0 = <&pcie3_reset>, <&pcie30x4m1_clkreqn>, <&pcie30x4m1_waken>;
 	reset-gpios = <&gpio4 RK_PB6 GPIO_ACTIVE_HIGH>;
 	vpcie3v3-supply = <&vcc3v3_pcie30>;
 	status = "okay";
Signature · 1 line
2.44.2
#3 lore raw

PATCH 3/5 arm64: dts: rockchip: Enable automatic fan control on Turing RK1

In reply to Sam Edwards

This patch adds thermal trip points and cooling maps to the Turing RK1 in order to enable automatic control of the external PWM fan. The fan is not active below 45C, as the heatsink alone can generally keep the chip in this temperature region at idle load. This cooling profile errs on the side of quietness, since the RK1 is commonly deployed in a Turing Pi 2 clusterboard alongside three others, with additional cooling provided at the chassis level.

---
 .../boot/dts/rockchip/rk3588-turing-rk1.dtsi  | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +53 -0
index 9bcb5acdea54..f6a12fe12d45 100644
@@ -208,6 +208,59 @@ rgmii_phy: ethernet-phy@1 {
 	};
 };
 
+&package_thermal {
+	trips {
+		package_active1: trip-active1 {
+			temperature = <45000>;
+			hysteresis = <5000>;
+			type = "active";
+		};
+		package_active2: trip-active2 {
+			temperature = <50000>;
+			hysteresis = <5000>;
+			type = "active";
+		};
+		package_active3: trip-active3 {
+			temperature = <60000>;
+			hysteresis = <5000>;
+			type = "active";
+		};
+		package_active4: trip-active4 {
+			temperature = <70000>;
+			hysteresis = <5000>;
+			type = "active";
+		};
+		package_active5: trip-active5 {
+			temperature = <80000>;
+			hysteresis = <5000>;
+			type = "active";
+		};
+	};
+
+	cooling-maps {
+		map1 {
+			trip = <&package_active1>;
+			cooling-device = <&fan 1 1>;
+		};
+		map2 {
+			trip = <&package_active2>;
+			cooling-device = <&fan 2 2>;
+		};
+		map3 {
+			trip = <&package_active3>;
+			cooling-device = <&fan 3 3>;
+		};
+		map4 {
+			trip = <&package_active4>;
+			cooling-device = <&fan 4 4>;
+		};
+		map5 {
+			trip = <&package_active5>;
+			cooling-device = <&fan 5 5>;
+		};
+	};
+};
+
 &pcie2x1l1 {
 	linux,pci-domain = <1>;
 	pinctrl-names = "default";
Signature · 1 line
2.44.2
#4 lore raw

PATCH 4/5 arm64: dts: rockchip: Enable all 3 USBs on Turing RK1

In reply to Sam Edwards

The Turing RK1 contains 3 different USBs:
- USB0: USB 2.0, OTG
- USB1: USB 3.0, host
- USB2: USB 2.0, host

This patch activates the necessary DT nodes to enable all 3 buses.

Future work will be needed on USB0: it is not USB 3.0, but Linux creates an unused USB 3.0 root hub and the controller also requires that USBDP0 be powered up. However, it is possible to remove this dependency. By either patching the xHCI driver to ignore the enumerated USB 3.0 port or setting usb3otg0_host_num_u3_port=0 in the GRF to stop the controller from enumerating a USB 3.0 port in the first place, neither Linux nor the controller will expect USB 3.0 capability, and USBDP0 can then safely be removed from the phys property.

---
 .../boot/dts/rockchip/rk3588-turing-rk1.dtsi  | 64 +++++++++++++++++++
 1 file changed, 64 insertions(+)
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +64 -0
index f6a12fe12d45..6036c4fe6727 100644
@@ -666,3 +666,67 @@ &uart9 {
 	pinctrl-0 = <&uart9m0_xfer>;
 	status = "okay";
 };
+
+/* USB 0: USB 2.0 only, OTG-capable */
+&u2phy0 {
+	status = "okay";
+};
+
+&u2phy0_otg {
+	status = "okay";
+};
+
+&usbdp_phy0 {
+	/*
+	 * TODO: On the RK1, USBDP0 drives the DisplayPort pins, and is not
+	 * involved in this USB2-only bus.  However, if the USB3 port is
+	 * enabled in the xHCI below, the controller will expect this PHY to be
+	 * powered up and holding RX_STATUS idle, or else it will generate an
+	 * endless stream of CSC events whenever a device is plugged in.  Until
+	 * there is a way to communicate to usb_host0_xhci that it doesn't have
+	 * a USB3 port, we have no choice but to power up USBDP0.
+	 */
+	status = "okay";
+};
+
+&usb_host0_xhci {
+	extcon = <&u2phy0>;
+	maximum-speed = "high-speed";
+	status = "okay";
+};
+
+/* USB 1: USB 3.0, host only */
+&u2phy1 {
+	status = "okay";
+};
+
+&u2phy1_otg {
+	status = "okay";
+};
+
+&usbdp_phy1 {
+	status = "okay";
+};
+
+&usb_host1_xhci {
+	extcon = <&u2phy1>;
+	dr_mode = "host";
+	status = "okay";
+};
+
+/* USB 2: USB 2.0, host only */
+&u2phy2 {
+	status = "okay";
+};
+
+&u2phy2_host {
+	status = "okay";
+};
+
+&usb_host0_ehci {
+	status = "okay";
+};
+
+&usb_host0_ohci {
+	status = "okay";
+};
Signature · 1 line
2.44.2
#5 lore raw

Re: [PATCH 4/5] arm64: dts: rockchip: Enable all 3 USBs on Turing RK1

Hi Sam,

Quoted 39 lines · Sam Edwards

The Turing RK1 contains 3 different USBs:
- USB0: USB 2.0, OTG
- USB1: USB 3.0, host
- USB2: USB 2.0, host

This patch activates the necessary DT nodes to enable all 3 buses.

Future work will be needed on USB0: it is not USB 3.0, but Linux creates an unused USB 3.0 root hub and the controller also requires that USBDP0 be powered up. However, it is possible to remove this dependency. By either patching the xHCI driver to ignore the enumerated USB 3.0 port or setting usb3otg0_host_num_u3_port=0 in the GRF to stop the controller from enumerating a USB 3.0 port in the first place, neither Linux nor the controller will expect USB 3.0 capability, and USBDP0 can then safely be removed from the phys property.

---
 .../boot/dts/rockchip/rk3588-turing-rk1.dtsi  | 64 +++++++++++++++++++
 1 file changed, 64 insertions(+)
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +10 -0
index f6a12fe12d45..6036c4fe6727 100644
@@ -666,3 +666,67 @@ &uart9 {
 	pinctrl-0 = <&uart9m0_xfer>;
 	status = "okay";
 };
+
+/* USB 0: USB 2.0 only, OTG-capable */
+&u2phy0 {
+	status = "okay";
+};
+
+&u2phy0_otg {
+	status = "okay";
+};
+

arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi &uart9 { · line 688

Quoted 10 lines
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +10 -0
+&usbdp_phy0 {
+	/*
+	 * TODO: On the RK1, USBDP0 drives the DisplayPort pins, and is not
+	 * involved in this USB2-only bus.  However, if the USB3 port is
+	 * enabled in the xHCI below, the controller will expect this PHY to be
+	 * powered up and holding RX_STATUS idle, or else it will generate an
+	 * endless stream of CSC events whenever a device is plugged in.  Until
+	 * there is a way to communicate to usb_host0_xhci that it doesn't have
+	 * a USB3 port, we have no choice but to power up USBDP0.
+	 */

Sounds like this may be missing

	rockchip,dp-lane-mux = <0 1 2 3>;

or

	rockchip,dp-lane-mux = <3 2 1 0>;

if all lanes are used for DP and none are used for USB.

It should help describe the hw and also help the driver set mode to UDPHY_MODE_DP and that should disable the u3 port, or there may be an issue to fix in the phy driver.

arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi &uart9 { · line 694

Quoted 6 lines
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +6 -0
+	status = "okay";
+};
+
+&usb_host0_xhci {
+	extcon = <&u2phy0>;
+	maximum-speed = "high-speed";

If this only use the USB2 phy, this should probably also override the default phys and phy-names props with:

	phys = <&u2phy0_otg>;
	phy-names = "usb2-phy";

Regards,
Jonas

Quoted 38 lines
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +38 -0
+	status = "okay";
+};
+
+/* USB 1: USB 3.0, host only */
+&u2phy1 {
+	status = "okay";
+};
+
+&u2phy1_otg {
+	status = "okay";
+};
+
+&usbdp_phy1 {
+	status = "okay";
+};
+
+&usb_host1_xhci {
+	extcon = <&u2phy1>;
+	dr_mode = "host";
+	status = "okay";
+};
+
+/* USB 2: USB 2.0, host only */
+&u2phy2 {
+	status = "okay";
+};
+
+&u2phy2_host {
+	status = "okay";
+};
+
+&usb_host0_ehci {
+	status = "okay";
+};
+
+&usb_host0_ohci {
+	status = "okay";
+};
#6 lore raw

Re: [PATCH 4/5] arm64: dts: rockchip: Enable all 3 USBs on Turing RK1

Quoted 9 lines · Jonas Karlman

Hi Sam,

Sounds like this may be missing

        rockchip,dp-lane-mux = <0 1 2 3>;

or

Quoted 7 lines
        rockchip,dp-lane-mux = <3 2 1 0>;

if all lanes are used for DP and none are used for USB.

It should help describe the hw and also help the driver set mode to UDPHY_MODE_DP and that should disable the u3 port, or there may be an issue to fix in the phy driver.

Thanks for your insights Jonas!

I haven't yet gotten to DP enablement so I don't know the correct DP layout. Ultimately I do want the USBDP0 node to have the necessary properties for DP, but alas that's a patch for another day.

Nonetheless, I briefly tried it and I don't think UDPHY_MODE_DP affects the PHY's "backend" (ctrl<->phy iface) at all, only the availability of frontend lanes to the USB-specific backend: So port u3 is still enabled, there's just no way to reach it electrically.

With that in mind, would you recommend that I add a placeholder dp-lane-mux of 0 1 2 3 for now, just to keep the PHY from attempting to speak USB to a DP device? I don't foresee any harm in leaving it as-is but you may know something that I don't.

Quoted 11 lines

arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi &uart9 { · line 694

Quoted 7 lines
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +6 -0
+     status = "okay";
+};
+
+&usb_host0_xhci {
+     extcon = <&u2phy0>;
+     maximum-speed = "high-speed";

If this only use the USB2 phy, this should probably also override the default phys and phy-names props with:

Quoted 2 lines
        phys = <&u2phy0_otg>;
        phy-names = "usb2-phy";

I agree completely: if the controller doesn't need the USB3 PHY, then it should not (implicitly) be specified in the DT. Being able to add these overrides is a big goal of mine as well. :)

Sadly, phys is what initializes USBDP's USB-side backend, so without it the RX_STATUS line goes floating, and because the controller still expects a port there, it misbehaves:

[   30.981076] usb usb2-port1: connect-debounce failed

I can tell the controller that there is no u3 port by doing this in U-Boot:
=> mw.l 0xfd5ac01c 0xf0000000 # usb3otg0_host_num_u3_port=0
=> boot
...and that makes single-PHY operation work perfectly! But unless Linux itself effects that change, this patch can't rely on that GRF being set correctly.

Do you have a recommendation on how I might go about disabling this port? I sent a patchset last year [1] that had the DWC3/xHCI driver ignore enumerated u3 ports when the maximum-speed was set to high-speed, but the consensus seems to be that this shouldn't be addressed at the xHCI driver level, so somehow zeroing the necessary GRF bits sounds like the way to go here. What do you think?

Kind regards,
Sam

[1]: https://lore.kernel.org/all/[email protected]/

#7 lore raw

Re: [PATCH 4/5] arm64: dts: rockchip: Enable all 3 USBs on Turing RK1

Quoted 11 lines · Sam Edwards
Quoted 10 lines · Jonas Karlman

Hi Sam,

Sounds like this may be missing

        rockchip,dp-lane-mux = <0 1 2 3>;

or

        rockchip,dp-lane-mux = <3 2 1 0>;

Small correction, the second 4-lane mode would be described as:

	rockchip,dp-lane-mux = <2 3 0 1>;
Quoted 18 lines
Quoted 6 lines

if all lanes are used for DP and none are used for USB.

It should help describe the hw and also help the driver set mode to UDPHY_MODE_DP and that should disable the u3 port, or there may be an issue to fix in the phy driver.

Thanks for your insights Jonas!

I haven't yet gotten to DP enablement so I don't know the correct DP layout. Ultimately I do want the USBDP0 node to have the necessary properties for DP, but alas that's a patch for another day.

Nonetheless, I briefly tried it and I don't think UDPHY_MODE_DP affects the PHY's "backend" (ctrl<->phy iface) at all, only the availability of frontend lanes to the USB-specific backend: So port u3 is still enabled, there's just no way to reach it electrically.

Quoted 4 lines

With that in mind, would you recommend that I add a placeholder dp-lane-mux of 0 1 2 3 for now, just to keep the PHY from attempting to speak USB to a DP device? I don't foresee any harm in leaving it as-is but you may know something that I don't.

The rk_udphy_u3_port_disable() call in usbdp phy driver should help set the usb3otg0_host_num_u3_port=0 you mentioned:

.usb3otg0_cfg = RK_UDPHY_GEN_GRF_REG(0x001c, 15, 0, 0x1100, 0x0188), .usb3otg1_cfg = RK_UDPHY_GEN_GRF_REG(0x0034, 15, 0, 0x1100, 0x0188),

Here the disable/enable values is little bit inverted in macro, i.e. enable=0x0188 is the value set when u3_port_disable(disable=true) is called.

Guessing that because the phy is not referenced its init() ops never gets called and u3 never gets disabled unless a usb3-phy is referenced.

Quoted 31 lines
Quoted 12 lines

arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi &uart9 { · line 694

Quoted 8 lines
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +6 -0
+     status = "okay";
+};
+
+&usb_host0_xhci {
+     extcon = <&u2phy0>;
+     maximum-speed = "high-speed";

If this only use the USB2 phy, this should probably also override the default phys and phy-names props with:

Quoted 2 lines
        phys = <&u2phy0_otg>;
        phy-names = "usb2-phy";

I agree completely: if the controller doesn't need the USB3 PHY, then it should not (implicitly) be specified in the DT. Being able to add these overrides is a big goal of mine as well. :)

Sadly, phys is what initializes USBDP's USB-side backend, so without it the RX_STATUS line goes floating, and because the controller still expects a port there, it misbehaves:

[   30.981076] usb usb2-port1: connect-debounce failed

I can tell the controller that there is no u3 port by doing this in U-Boot:
=> mw.l 0xfd5ac01c 0xf0000000 # usb3otg0_host_num_u3_port=0
=> boot
...and that makes single-PHY operation work perfectly! But unless Linux itself effects that change, this patch can't rely on that GRF being set correctly.

Quoted 6 lines

Do you have a recommendation on how I might go about disabling this port? I sent a patchset last year [1] that had the DWC3/xHCI driver ignore enumerated u3 ports when the maximum-speed was set to high-speed, but the consensus seems to be that this shouldn't be addressed at the xHCI driver level, so somehow zeroing the necessary GRF bits sounds like the way to go here. What do you think?

Not sure if it would help but could be that part of init() ops should be moved to probe(). Would still require the phy driver to probe before usb controller for that to help/work.

Adding a rockchip,dp-lane-mux prop and keeping the phys prop as-is is probably easiest way for now.

One option for future could possible be to have grf driver disable u3 and modify usbdp phy driver to enable u3 instead of disable u3, not sure this will fully work, init of the usbdp phy seem very sensitive and possible a one-time op. Trying to "usb start" in U-Boot will only work one time, using "usb reset" or a "usb stop/start" cycle will cause usbdp phy init to fail and a full board reset is needed to get port working again.

Regards,
Jonas

Quoted 5 lines

Kind regards,
Sam

[1]: https://lore.kernel.org/all/[email protected]/

#8 lore raw

Re: [PATCH 4/5] arm64: dts: rockchip: Enable all 3 USBs on Turing RK1

Quoted 44 lines · Jonas Karlman
Quoted 11 lines · Sam Edwards
Quoted 10 lines · Jonas Karlman

Hi Sam,

Sounds like this may be missing

        rockchip,dp-lane-mux = <0 1 2 3>;

or

        rockchip,dp-lane-mux = <3 2 1 0>;

Small correction, the second 4-lane mode would be described as:

        rockchip,dp-lane-mux = <2 3 0 1>;
Quoted 18 lines
Quoted 6 lines

if all lanes are used for DP and none are used for USB.

It should help describe the hw and also help the driver set mode to UDPHY_MODE_DP and that should disable the u3 port, or there may be an issue to fix in the phy driver.

Thanks for your insights Jonas!

I haven't yet gotten to DP enablement so I don't know the correct DP layout. Ultimately I do want the USBDP0 node to have the necessary properties for DP, but alas that's a patch for another day.

Nonetheless, I briefly tried it and I don't think UDPHY_MODE_DP affects the PHY's "backend" (ctrl<->phy iface) at all, only the availability of frontend lanes to the USB-specific backend: So port u3 is still enabled, there's just no way to reach it electrically.

Quoted 4 lines

With that in mind, would you recommend that I add a placeholder dp-lane-mux of 0 1 2 3 for now, just to keep the PHY from attempting to speak USB to a DP device? I don't foresee any harm in leaving it as-is but you may know something that I don't.

The rk_udphy_u3_port_disable() call in usbdp phy driver should help set the usb3otg0_host_num_u3_port=0 you mentioned:

Quoted 6 lines

.usb3otg0_cfg = RK_UDPHY_GEN_GRF_REG(0x001c, 15, 0, 0x1100, 0x0188), .usb3otg1_cfg = RK_UDPHY_GEN_GRF_REG(0x0034, 15, 0, 0x1100, 0x0188),

Here the disable/enable values is little bit inverted in macro, i.e. enable=0x0188 is the value set when u3_port_disable(disable=true) is called.

Aha, I didn't notice that the PHY driver had this, thanks for pointing it out! Yes, it's good that it's also switching the clock source and disabling PHY status signals so I should definitely be relying on this code for now.

Quoted 42 lines

Guessing that because the phy is not referenced its init() ops never gets called and u3 never gets disabled unless a usb3-phy is referenced.

Quoted 31 lines
Quoted 12 lines

arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi &uart9 { · line 694

Quoted 8 lines
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +6 -0
+     status = "okay";
+};
+
+&usb_host0_xhci {
+     extcon = <&u2phy0>;
+     maximum-speed = "high-speed";

If this only use the USB2 phy, this should probably also override the default phys and phy-names props with:

Quoted 2 lines
        phys = <&u2phy0_otg>;
        phy-names = "usb2-phy";

I agree completely: if the controller doesn't need the USB3 PHY, then it should not (implicitly) be specified in the DT. Being able to add these overrides is a big goal of mine as well. :)

Sadly, phys is what initializes USBDP's USB-side backend, so without it the RX_STATUS line goes floating, and because the controller still expects a port there, it misbehaves:

[   30.981076] usb usb2-port1: connect-debounce failed

I can tell the controller that there is no u3 port by doing this in U-Boot:
=> mw.l 0xfd5ac01c 0xf0000000 # usb3otg0_host_num_u3_port=0
=> boot
...and that makes single-PHY operation work perfectly! But unless Linux itself effects that change, this patch can't rely on that GRF being set correctly.

Quoted 6 lines

Do you have a recommendation on how I might go about disabling this port? I sent a patchset last year [1] that had the DWC3/xHCI driver ignore enumerated u3 ports when the maximum-speed was set to high-speed, but the consensus seems to be that this shouldn't be addressed at the xHCI driver level, so somehow zeroing the necessary GRF bits sounds like the way to go here. What do you think?

Quoted 6 lines

Not sure if it would help but could be that part of init() ops should be moved to probe(). Would still require the phy driver to probe before usb controller for that to help/work.

Adding a rockchip,dp-lane-mux prop and keeping the phys prop as-is is probably easiest way for now.

Continuing my comments above: I agree fully. My v2 will add a placeholder dp-lane-mux.

Maintainers: I will be sending a v2 of this patch separately in the future; please consider this patch withdrawn from the series.

Quoted 7 lines

One option for future could possible be to have grf driver disable u3 and modify usbdp phy driver to enable u3 instead of disable u3, not sure this will fully work, init of the usbdp phy seem very sensitive and possible a one-time op. Trying to "usb start" in U-Boot will only work one time, using "usb reset" or a "usb stop/start" cycle will cause usbdp phy init to fail and a full board reset is needed to get port working again.

Arguably, it doesn't make sense for the USBDP driver to be affecting these GRFs at all, because *technically* they're configuration signals fed into the DWC3: therefore whatever driver binds to "rockchip,rk3588-dwc3" ought to be setting them according to the PHYs it discovers in the DT. I suspect that this code was only put in the PHY driver because that's a more convenient place to put Rockchip-specific management code for the time being.

Of course, this is all a discussion to be had in a different thread. For now, suffice to say, I agree with your thoughts about the USB3OTGn GRF management situation needing improvement and am interested in lending a hand wherever I can. :)

Thank you for your insights,
Sam

Quoted 10 lines

Regards,
Jonas

Quoted 5 lines

Kind regards,
Sam

[1]: https://lore.kernel.org/all/[email protected]/

#9 lore raw

PATCH 5/5 arm64: dts: rockchip: Enable GPU on Turing RK1

In reply to Sam Edwards

Enable the Mali GPU in the Turing RK1.

This patch also sets the external GPU voltage regulator in the RK806-1 to "always-on" because it is necessary for this regulator to be active when enabling the GPU power domain or the kernel will fail with:

rockchip-pm-domain fd8d8000.power-management:power-controller: \
failed to set domain 'gpu', val=0

rockchip-pm-domain fd8d8000.power-management:power-controller: \
    failed to get ack on domain 'gpu', val=0x1bffff

...followed by a panic when it attempts to access unavailable QoS registers.

Since there is currently no domain-supply or similar to express this dependency, the only way to ensure that the regulator is never off when the GPU power domain is brought up is to ensure that the regulator is never off.

---

Hi list,

This particular patch will probably need to be revisited once something like [1] lands. I'm completely unable to get the GPU up and running without some kind of solution to the power dependency issue, but it's possible that this is because I'm just particularly unlucky in the timing department.

Cheers,
Sam

[1]: https://lore.kernel.org/lkml/[email protected]/T/

---
 .../boot/dts/rockchip/rk3588-turing-rk1.dtsi     | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi +16 -0
index 6036c4fe6727..dedfb9ede4a3 100644
@@ -116,6 +116,11 @@ &gmac1_rgmii_clk
 	status = "okay";
 };
 
+&gpu {
+	mali-supply = <&vdd_gpu_s0>;
+	status = "okay";
+};
+
 &i2c0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c0m2_xfer>;
@@ -386,6 +391,17 @@ rk806_dvs3_null: dvs3-null-pins {
 
 		regulators {
 			vdd_gpu_s0: vdd_gpu_mem_s0: dcdc-reg1 {
+				/*
+				 * RK3588's GPU power domain cannot be enabled
+				 * without this regulator active, but it
+				 * doesn't have to be on when the GPU PD is
+				 * disabled.  Because the PD binding does not
+				 * currently allow us to express this
+				 * relationship, we have no choice but to do
+				 * this instead:
+				 */
+				regulator-always-on;
+
 				regulator-boot-on;
 				regulator-min-microvolt = <550000>;
 				regulator-max-microvolt = <950000>;
Signature · 1 line
2.44.2
maintainer Applied
#10 lore raw

Re: (subset) [PATCH 0/5] Turing RK1 SoM DT updates

In reply to Sam Edwards
Quoted 8 lines · Sam Edwards

It's been a little under a year since support for the Turing RK1 RK3588-based SoM board, was introduced upstream. Since then, the driver developers have had great successes in further RK3588 enablement, achieving such enhancements as:
- Stable thermal ADC support
- USBDP PHY support
- Panthor: a driver for the Mali-G610 GPU found in RK3588

[...]

Applied, thanks!

[1/5] arm64: dts: rockchip: Split up RK3588's PCIe pinctrls

      commit: 4294e32111781b3de4d73b944cbd1bc1662a9a7a
[2/5] arm64: dts: rockchip: Fix Turing RK1 PCIe3 hang
      commit: ed1b30c33bb97abea9de4f749a890f5c88183d71
[3/5] arm64: dts: rockchip: Enable automatic fan control on Turing RK1
      commit: 604c164317ac93feae4b06bceac5bdb0be3bf96f
[5/5] arm64: dts: rockchip: Enable GPU on Turing RK1
      commit: f136ce5d07cbd91c8eadc407ddf1ad00dc1c3cec

Best regards,

Signature · 1 line
Heiko Stuebner <[email protected]>