devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add Support for Orange Pi 5
@ 2023-08-15 12:58 Muhammed Efe Cetin
  2023-08-15 12:58 ` [PATCH 1/3] dt-bindings: arm: rockchip: Add Orange Pi 5 board Muhammed Efe Cetin
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Muhammed Efe Cetin @ 2023-08-15 12:58 UTC (permalink / raw)
  To: linux-rockchip
  Cc: devicetree, linux-arm-kernel, linux-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, heiko, sebastian.reichel,
	Muhammed Efe Cetin

Hi,

These series add initial support for Orange Pi 5 and SFC node for RK3588S.

Muhammed Efe Cetin (3):
  dt-bindings: arm: rockchip: Add Orange Pi 5 board
  arm64: dts: rockchip: Add sfc node to rk3588s
  arm64: dts: rockchip: Add Orange Pi 5

 .../devicetree/bindings/arm/rockchip.yaml     |   5 +
 .../boot/dts/rockchip/rk3588s-orangepi-5.dts  | 873 ++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi     |  13 +
 3 files changed, 891 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts

-- 
2.41.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] dt-bindings: arm: rockchip: Add Orange Pi 5 board
  2023-08-15 12:58 [PATCH 0/3] Add Support for Orange Pi 5 Muhammed Efe Cetin
@ 2023-08-15 12:58 ` Muhammed Efe Cetin
  2023-08-15 13:46   ` Krzysztof Kozlowski
  2023-08-15 12:59 ` [PATCH 2/3] arm64: dts: rockchip: Add sfc node to rk3588s Muhammed Efe Cetin
  2023-08-15 12:59 ` [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5 Muhammed Efe Cetin
  2 siblings, 1 reply; 16+ messages in thread
From: Muhammed Efe Cetin @ 2023-08-15 12:58 UTC (permalink / raw)
  To: linux-rockchip
  Cc: devicetree, linux-arm-kernel, linux-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, heiko, sebastian.reichel,
	Muhammed Efe Cetin

Add Orange Pi 5 SBC from Xunlong.

Signed-off-by: Muhammed Efe Cetin <efectn@6tel.net>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
index ca5389862887..b9649e27bc82 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -877,6 +877,11 @@ properties:
               - xunlong,orangepi-r1-plus-lts
           - const: rockchip,rk3328
 
+      - description: Xunlong Orange Pi 5
+        items:
+          - const: xunlong,orangepi-5
+          - const: rockchip,rk3588s
+
       - description: Zkmagic A95X Z2
         items:
           - const: zkmagic,a95x-z2
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] arm64: dts: rockchip: Add sfc node to rk3588s
  2023-08-15 12:58 [PATCH 0/3] Add Support for Orange Pi 5 Muhammed Efe Cetin
  2023-08-15 12:58 ` [PATCH 1/3] dt-bindings: arm: rockchip: Add Orange Pi 5 board Muhammed Efe Cetin
@ 2023-08-15 12:59 ` Muhammed Efe Cetin
  2023-08-17 21:04   ` Jonas Karlman
  2023-08-15 12:59 ` [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5 Muhammed Efe Cetin
  2 siblings, 1 reply; 16+ messages in thread
From: Muhammed Efe Cetin @ 2023-08-15 12:59 UTC (permalink / raw)
  To: linux-rockchip
  Cc: devicetree, linux-arm-kernel, linux-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, heiko, sebastian.reichel,
	Muhammed Efe Cetin

Add sfc node to rk3588s.dtsi from downstream kernel.

Signed-off-by: Muhammed Efe Cetin <efectn@6tel.net>
---
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 5544f66c6ff4..a38a0158fce0 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -1424,6 +1424,19 @@ sata-port@0 {
 		};
 	};
 
+	sfc: spi@fe2b0000 {
+		compatible = "rockchip,sfc";
+		reg = <0x0 0xfe2b0000 0x0 0x4000>;
+		interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
+		clock-names = "clk_sfc", "hclk_sfc";
+		assigned-clocks = <&cru SCLK_SFC>;
+		assigned-clock-rates = <100000000>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
 	sdmmc: mmc@fe2c0000 {
 		compatible = "rockchip,rk3588-dw-mshc", "rockchip,rk3288-dw-mshc";
 		reg = <0x0 0xfe2c0000 0x0 0x4000>;
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-15 12:58 [PATCH 0/3] Add Support for Orange Pi 5 Muhammed Efe Cetin
  2023-08-15 12:58 ` [PATCH 1/3] dt-bindings: arm: rockchip: Add Orange Pi 5 board Muhammed Efe Cetin
  2023-08-15 12:59 ` [PATCH 2/3] arm64: dts: rockchip: Add sfc node to rk3588s Muhammed Efe Cetin
@ 2023-08-15 12:59 ` Muhammed Efe Cetin
  2023-08-15 13:45   ` Krzysztof Kozlowski
                     ` (2 more replies)
  2 siblings, 3 replies; 16+ messages in thread
From: Muhammed Efe Cetin @ 2023-08-15 12:59 UTC (permalink / raw)
  To: linux-rockchip
  Cc: devicetree, linux-arm-kernel, linux-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, heiko, sebastian.reichel,
	Muhammed Efe Cetin

Add initial support for OPi5 that includes support for USB2, PCIe2, Sata,
Sdmmc, SPI Flash, PMIC.

Signed-off-by: Muhammed Efe Cetin <efectn@6tel.net>
---
 .../boot/dts/rockchip/rk3588s-orangepi-5.dts  | 873 ++++++++++++++++++
 1 file changed, 873 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
new file mode 100644
index 000000000000..85071084a207
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
@@ -0,0 +1,873 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/dts-v1/;
+
+#include <dt-bindings/pinctrl/rockchip.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "rk3588s.dtsi"
+
+/ {
+	model = "Xunlong Orange Pi 5";
+	compatible = "xunlong,orangepi-5", "rockchip,rk3588s";
+
+	aliases {
+		mmc0 = &sdmmc;
+		serial2 = &uart2;
+	};
+
+	chosen {
+		stdout-path = "serial2:1500000n8";
+	};
+
+	leds {
+		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 =<&leds_gpio>;
+
+		led@1 {
+			gpios = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;
+			label = "status_led";
+			linux,default-trigger = "heartbeat";
+			linux,default-trigger-delay-ms = <0>;
+		};
+	};
+
+	adc-keys {
+		compatible = "adc-keys";
+		io-channels = <&saradc 1>;
+		io-channel-names = "buttons";
+		keyup-threshold-microvolt = <1800000>;
+		poll-interval = <100>;
+
+		vol-up-key {
+			label = "volume up";
+			linux,code = <KEY_VOLUMEUP>;
+			press-threshold-microvolt = <17000>;
+		};
+
+		vol-down-key {
+			label = "volume down";
+			linux,code = <KEY_VOLUMEDOWN>;
+			press-threshold-microvolt = <417000>;
+		};
+
+		menu-key {
+			label = "menu";
+			linux,code = <KEY_MENU>;
+			press-threshold-microvolt = <890000>;
+		};
+
+		back-key {
+			label = "back";
+			linux,code = <KEY_BACK>;
+			press-threshold-microvolt = <1235000>;
+		};
+	};
+
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		brightness-levels = <  0  20  20  21  21  22  22  23
+					  23  24  24  25  25  26  26  27
+					  27  28  28  29  29  30  30  31
+					  31  32  32  33  33  34  34  35
+					  35  36  36  37  37  38  38  39
+					  40  41  42  43  44  45  46  47
+					  48  49  50  51  52  53  54  55
+					  56  57  58  59  60  61  62  63
+					  64  65  66  67  68  69  70  71
+					  72  73  74  75  76  77  78  79
+					  80  81  82  83  84  85  86  87
+					  88  89  90  91  92  93  94  95
+					  96  97  98  99 100 101 102 103
+					  104 105 106 107 108 109 110 111
+					  112 113 114 115 116 117 118 119
+					  120 121 122 123 124 125 126 127
+					  128 129 130 131 132 133 134 135
+					  136 137 138 139 140 141 142 143
+					  144 145 146 147 148 149 150 151
+					  152 153 154 155 156 157 158 159
+					  160 161 162 163 164 165 166 167
+					  168 169 170 171 172 173 174 175
+					  176 177 178 179 180 181 182 183
+					  184 185 186 187 188 189 190 191
+					  192 193 194 195 196 197 198 199
+					  200 201 202 203 204 205 206 207
+					  208 209 210 211 212 213 214 215
+					  216 217 218 219 220 221 222 223
+					  224 225 226 227 228 229 230 231
+					  232 233 234 235 236 237 238 239
+					  240 241 242 243 244 245 246 247
+					  248 249 250 251 252 253 254 255>;
+		default-brightness-level = <200>;
+		pwms = <&pwm2 0 25000 0>;
+	};
+
+	backlight_1: backlight_1 {
+		compatible = "pwm-backlight";
+		brightness-levels = <  0  20  20  21  21  22  22  23
+					  23  24  24  25  25  26  26  27
+					  27  28  28  29  29  30  30  31
+					  31  32  32  33  33  34  34  35
+					  35  36  36  37  37  38  38  39
+					  40  41  42  43  44  45  46  47
+					  48  49  50  51  52  53  54  55
+					  56  57  58  59  60  61  62  63
+					  64  65  66  67  68  69  70  71
+					  72  73  74  75  76  77  78  79
+					  80  81  82  83  84  85  86  87
+					  88  89  90  91  92  93  94  95
+					  96  97  98  99 100 101 102 103
+					  104 105 106 107 108 109 110 111
+					  112 113 114 115 116 117 118 119
+					  120 121 122 123 124 125 126 127
+					  128 129 130 131 132 133 134 135
+					  136 137 138 139 140 141 142 143
+					  144 145 146 147 148 149 150 151
+					  152 153 154 155 156 157 158 159
+					  160 161 162 163 164 165 166 167
+					  168 169 170 171 172 173 174 175
+					  176 177 178 179 180 181 182 183
+					  184 185 186 187 188 189 190 191
+					  192 193 194 195 196 197 198 199
+					  200 201 202 203 204 205 206 207
+					  208 209 210 211 212 213 214 215
+					  216 217 218 219 220 221 222 223
+					  224 225 226 227 228 229 230 231
+					  232 233 234 235 236 237 238 239
+					  240 241 242 243 244 245 246 247
+					  248 249 250 251 252 253 254 255>;
+		default-brightness-level = <200>;
+		pwms = <&pwm6 0 25000 0>;
+	};
+
+	vcc12v_dcin: vcc12v-dcin-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc12v_dcin";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+	};
+
+	vcc5v0_sys: vcc5v0-sys-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc5v0_sys";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc12v_dcin>;
+	};
+
+	vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_1v1_nldo_s3";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <1100000>;
+		regulator-max-microvolt = <1100000>;
+		vin-supply = <&vcc5v0_sys>;
+	};
+
+	vcc_3v3_sd_s0: vcc-3v3-sd-s0-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_3v3_sd_s0";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio4 RK_PB5 GPIO_ACTIVE_LOW>;
+		enable-active-low;
+		vin-supply = <&vcc_3v3_s3>;
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-state-mem {
+			regulator-on-in-suspend;
+		};
+	};
+
+	vcc5v0_usbdcin: vcc5v0-usbdcin-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc5v0_usbdcin";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc12v_dcin>;
+	};
+
+	vcc5v0_usb: vcc5v0-usb-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc5v0_usb";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc5v0_usbdcin>;
+	};
+
+	vbus5v0_typec: vbus5v0-typec-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vbus5v0_typec";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		gpio = <&gpio3 RK_PC0 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&vcc5v0_usb>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&typec5v_pwren>;
+	};
+
+	combophy_avdd0v85: combophy-avdd0v85-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "combophy_avdd0v85";
+		regulator-boot-on;
+		regulator-always-on;
+		regulator-min-microvolt = <850000>;
+		regulator-max-microvolt = <850000>;
+		vin-supply = <&vdd_0v85_s0>;
+	};
+
+	combophy_avdd1v8: combophy-avdd1v8-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "combophy_avdd1v8";
+		regulator-boot-on;
+		regulator-always-on;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		vin-supply = <&avcc_1v8_s0>;
+	};
+
+	vcc3v3_pcie2x1l2: vcc3v3-pcie2x1l2-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc3v3_pcie2x1l2";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		enable-active-high;
+		regulator-boot-on;
+		regulator-always-on;
+		gpios = <&gpio0 RK_PC5 GPIO_ACTIVE_HIGH>;
+		startup-delay-us = <50000>;
+		vin-supply = <&vcc5v0_sys>;
+	};
+};
+
+&combphy0_ps {
+	status = "okay";
+};
+
+&combphy2_psu {
+	status = "okay";
+};
+
+&cpu_b0 {
+	cpu-supply = <&vdd_cpu_big0_s0>;
+	mem-supply = <&vdd_cpu_big0_mem_s0>;
+};
+
+&cpu_b1 {
+	cpu-supply = <&vdd_cpu_big0_s0>;
+	mem-supply = <&vdd_cpu_big0_mem_s0>;
+};
+
+&cpu_b2 {
+	cpu-supply = <&vdd_cpu_big1_s0>;
+	mem-supply = <&vdd_cpu_big1_mem_s0>;
+};
+
+&cpu_b3 {
+	cpu-supply = <&vdd_cpu_big1_s0>;
+	mem-supply = <&vdd_cpu_big1_mem_s0>;
+};
+
+&cpu_l0 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+	mem-supply = <&vdd_cpu_lit_mem_s0>;
+};
+
+&cpu_l1 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+	mem-supply = <&vdd_cpu_lit_mem_s0>;
+};
+
+&cpu_l2 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+	mem-supply = <&vdd_cpu_lit_mem_s0>;
+};
+
+&cpu_l3 {
+	cpu-supply = <&vdd_cpu_lit_s0>;
+	mem-supply = <&vdd_cpu_lit_mem_s0>;
+};
+
+&gmac1 {
+	clock_in_out = "output";
+	phy-handle = <&rgmii_phy1>;
+	phy-mode = "rgmii-rxid";
+	pinctrl-0 = <&gmac1_miim
+					&gmac1_tx_bus2
+					&gmac1_rx_bus2
+					&gmac1_rgmii_clk
+					&gmac1_rgmii_bus>;
+	pinctrl-names = "default";
+	snps,reset-gpio = <&gpio3 RK_PB2 GPIO_ACTIVE_LOW>;
+	snps,reset-active-low;
+	snps,reset-delays-us = <0 20000 100000>;
+	tx_delay = <0x42>;
+	status = "okay";
+};
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0m2_xfer>;
+	status = "okay";
+
+	vdd_cpu_big0_s0: vdd_cpu_big0_mem_s0: regulator@42 {
+		compatible = "rockchip,rk8602";
+		reg = <0x42>;
+		fcs,suspend-voltage-selector = <1>;
+		regulator-name = "vdd_cpu_big0_s0";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <550000>;
+		regulator-max-microvolt = <1050000>;
+		regulator-ramp-delay = <2300>;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+
+	vdd_cpu_big1_s0: vdd_cpu_big1_mem_s0: regulator@43 {
+		compatible = "rockchip,rk8603", "rockchip,rk8602";
+		reg = <0x43>;
+		fcs,suspend-voltage-selector = <1>;
+		regulator-name = "vdd_cpu_big1_s0";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <550000>;
+		regulator-max-microvolt = <1050000>;
+		regulator-ramp-delay = <2300>;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+};
+
+&i2c2 {
+	status = "okay";
+
+	vdd_npu_s0: vdd_npu_mem_s0: regulator@42 {
+		compatible = "rockchip,rk8602";
+		reg = <0x42>;
+		fcs,suspend-voltage-selector = <1>;
+		regulator-name = "vdd_npu_s0";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <550000>;
+		regulator-max-microvolt = <950000>;
+		regulator-ramp-delay = <2300>;
+		vin-supply = <&vcc5v0_sys>;
+
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+};
+
+&i2c6 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c6m3_xfer>;
+	status = "okay";
+
+	hym8563: rtc@51 {
+		compatible = "haoyu,hym8563";
+		reg = <0x51>;
+		#clock-cells = <0>;
+		clock-output-names = "hym8563";
+		pinctrl-names = "default";
+		pinctrl-0 = <&hym8563_int>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PB0 IRQ_TYPE_LEVEL_LOW>;
+		wakeup-source;
+	};
+};
+
+&mdio1 {
+	rgmii_phy1: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0x1>;
+	};
+};
+
+&pcie2x1l2 {
+	reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>;
+	vpcie3v3-supply = <&vcc3v3_pcie2x1l2>;
+	rockchip,skip-scan-in-resume;
+	status = "okay";
+};
+
+&pinctrl {
+	gpio-func {
+		leds_gpio: leds-gpio {
+			rockchip,pins = <0 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	hym8563 {
+		hym8563_int: hym8563-int {
+			rockchip,pins = <0 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	sata {
+		sata_reset:sata-reset{
+			rockchip,pins = <3 RK_PD1 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+};
+
+&pwm2 {
+	pinctrl-names = "active";
+	pinctrl-0 = <&pwm2m0_pins>;
+	status = "okay";
+};
+
+&pwm6 {
+	pinctrl-names = "active";
+	pinctrl-0 = <&pwm6m0_pins>;
+	status = "okay";
+};
+
+&saradc {
+	vref-supply = <&avcc_1v8_s0>;
+	status = "okay";
+};
+
+&sata0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sata_reset>;
+	status = "disabled";
+};
+
+&sdmmc {
+	max-frequency = <150000000>;
+	no-sdio;
+	no-mmc;
+	bus-width = <4>;
+	cap-mmc-highspeed;
+	cap-sd-highspeed;
+	disable-wp;
+	sd-uhs-sdr104;
+	vmmc-supply = <&vcc_3v3_sd_s0>;
+	vqmmc-supply = <&vccio_sd_s0>;
+	status = "okay";
+};
+
+&sfc {
+	pinctrl-0 = <&fspim0_pins>;
+	pinctrl-names = "default";
+	max-freq = <100000000>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+	status = "okay";
+
+	spi_flash: spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0x0>;
+		spi-max-frequency = <100000000>;
+		spi-tx-bus-width = <1>;
+		spi-rx-bus-width = <4>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "okay";
+
+		partitions {
+			compatible = "fixed-partitions";
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			loader@0 {
+				label = "loader";
+				reg = <0x0 0x1000000>;
+			};
+		};
+	};
+};
+
+&spi2 {
+	status = "okay";
+	assigned-clocks = <&cru CLK_SPI2>;
+	assigned-clock-rates = <200000000>;
+	num-cs = <1>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi2m2_cs0 &spi2m2_pins>;
+
+	pmic@0 {
+		compatible = "rockchip,rk806";
+		reg = <0x0>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
+				<&rk806_dvs2_null>, <&rk806_dvs3_null>;
+		spi-max-frequency = <1000000>;
+
+		vcc1-supply = <&vcc5v0_sys>;
+		vcc2-supply = <&vcc5v0_sys>;
+		vcc3-supply = <&vcc5v0_sys>;
+		vcc4-supply = <&vcc5v0_sys>;
+		vcc5-supply = <&vcc5v0_sys>;
+		vcc6-supply = <&vcc5v0_sys>;
+		vcc7-supply = <&vcc5v0_sys>;
+		vcc8-supply = <&vcc5v0_sys>;
+		vcc9-supply = <&vcc5v0_sys>;
+		vcc10-supply = <&vcc5v0_sys>;
+		vcc11-supply = <&vcc_2v0_pldo_s3>;
+		vcc12-supply = <&vcc5v0_sys>;
+		vcc13-supply = <&vcc_1v1_nldo_s3>;
+		vcc14-supply = <&vcc_1v1_nldo_s3>;
+		vcca-supply = <&vcc5v0_sys>;
+
+		gpio-controller;
+		#gpio-cells = <2>;
+
+		rk806_dvs1_null: dvs1-null-pins {
+			pins = "gpio_pwrctrl2";
+			function = "pin_fun0";
+		};
+
+		rk806_dvs2_null: dvs2-null-pins {
+			pins = "gpio_pwrctrl2";
+			function = "pin_fun0";
+		};
+
+		rk806_dvs3_null: dvs3-null-pins {
+			pins = "gpio_pwrctrl3";
+			function = "pin_fun0";
+		};
+
+		regulators {
+			vdd_gpu_s0: vdd_gpu_mem_s0: dcdc-reg1 {
+				regulator-name = "vdd_gpu_s0";
+				regulator-boot-on;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <950000>;
+				regulator-ramp-delay = <12500>;
+				regulator-enable-ramp-delay = <400>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_cpu_lit_s0: vdd_cpu_lit_mem_s0: dcdc-reg2 {
+				regulator-name = "vdd_cpu_lit_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <950000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_log_s0: dcdc-reg3 {
+				regulator-name = "vdd_log_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <675000>;
+				regulator-max-microvolt = <750000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <750000>;
+				};
+			};
+
+			vdd_vdenc_s0: vdd_vdenc_mem_s0: dcdc-reg4 {
+				regulator-name = "vdd_vdenc_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <550000>;
+				regulator-max-microvolt = <950000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_ddr_s0: dcdc-reg5 {
+				regulator-name = "vdd_ddr_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <675000>;
+				regulator-max-microvolt = <900000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <850000>;
+				};
+			};
+
+			vdd2_ddr_s3: dcdc-reg6 {
+				regulator-name = "vdd2_ddr_s3";
+				regulator-always-on;
+				regulator-boot-on;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vcc_2v0_pldo_s3: dcdc-reg7 {
+				regulator-name = "vdd_2v0_pldo_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <2000000>;
+				regulator-max-microvolt = <2000000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <2000000>;
+				};
+			};
+
+			vcc_3v3_s3: dcdc-reg8 {
+				regulator-name = "vcc_3v3_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3300000>;
+				};
+			};
+
+			vddq_ddr_s0: dcdc-reg9 {
+				regulator-name = "vddq_ddr_s0";
+				regulator-always-on;
+				regulator-boot-on;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8_s3: dcdc-reg10 {
+				regulator-name = "vcc_1v8_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			avcc_1v8_s0: pldo-reg1 {
+				regulator-name = "avcc_1v8_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8_s0: pldo-reg2 {
+				regulator-name = "vcc_1v8_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			avdd_1v2_s0: pldo-reg3 {
+				regulator-name = "avdd_1v2_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_3v3_s0: pldo-reg4 {
+				regulator-name = "vcc_3v3_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vccio_sd_s0: pldo-reg5 {
+				regulator-name = "vccio_sd_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-ramp-delay = <12500>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			pldo6_s3: pldo-reg6 {
+				regulator-name = "pldo6_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vdd_0v75_s3: nldo-reg1 {
+				regulator-name = "vdd_0v75_s3";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <750000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <750000>;
+				};
+			};
+
+			vdd_ddr_pll_s0: nldo-reg2 {
+				regulator-name = "vdd_ddr_pll_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <850000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+					regulator-suspend-microvolt = <850000>;
+				};
+			};
+
+			avdd_0v75_s0: nldo-reg3 {
+				regulator-name = "avdd_0v75_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <750000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_0v85_s0: nldo-reg4 {
+				regulator-name = "vdd_0v85_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <850000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_0v75_s0: nldo-reg5 {
+				regulator-name = "vdd_0v75_s0";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <750000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+		};
+	};
+};
+
+&tsadc {
+	status = "okay";
+};
+
+&uart2 {
+	pinctrl-0 = <&uart2m0_xfer>;
+	status = "okay";
+};
+
+&u2phy0 {
+	status = "okay";
+};
+
+&u2phy0_otg {
+	rockchip,typec-vbus-det;
+	status = "okay";
+};
+
+&u2phy2 {
+	status = "okay";
+};
+
+&u2phy2_host {
+	status = "okay";
+};
+
+&u2phy3 {
+	status = "okay";
+};
+
+&u2phy3_host {
+	status = "okay";
+};
+
+&usb_host0_ehci {
+	status = "okay";
+};
+
+&usb_host0_ohci {
+	status = "okay";
+};
+
+&usb_host1_ehci {
+	status = "okay";
+};
+
+&usb_host1_ohci {
+	status = "okay";
+};
+
+&wdt {
+	status = "okay";
+};
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-15 12:59 ` [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5 Muhammed Efe Cetin
@ 2023-08-15 13:45   ` Krzysztof Kozlowski
  2023-08-17 17:23     ` Muhammed Efe Cetin
  2023-08-15 16:39   ` Ondřej Jirman
  2023-08-17 21:05   ` Jonas Karlman
  2 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-15 13:45 UTC (permalink / raw)
  To: Muhammed Efe Cetin, linux-rockchip
  Cc: devicetree, linux-arm-kernel, linux-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, heiko, sebastian.reichel

On 15/08/2023 14:59, Muhammed Efe Cetin wrote:
> Add initial support for OPi5 that includes support for USB2, PCIe2, Sata,
> Sdmmc, SPI Flash, PMIC.
> 
> Signed-off-by: Muhammed Efe Cetin <efectn@6tel.net>
> ---
>  .../boot/dts/rockchip/rk3588s-orangepi-5.dts  | 873 ++++++++++++++++++

Without Makefile this won't be build, so this was not ever tested.

>  1 file changed, 873 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> new file mode 100644
> index 000000000000..85071084a207
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> @@ -0,0 +1,873 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include "rk3588s.dtsi"
> +
> +/ {
> +	model = "Xunlong Orange Pi 5";
> +	compatible = "xunlong,orangepi-5", "rockchip,rk3588s";
> +
> +	aliases {
> +		mmc0 = &sdmmc;
> +		serial2 = &uart2;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial2:1500000n8";
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 =<&leds_gpio>;
> +
> +		led@1 {

Unit address is not correct here, it is not a bus. This should be
reported as warning, so you did not check for warnings.

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

> +			gpios = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;
> +			label = "status_led";
> +			linux,default-trigger = "heartbeat";
> +			linux,default-trigger-delay-ms = <0>;
> +		};
> +	};
> +
> +	adc-keys {
> +		compatible = "adc-keys";
> +		io-channels = <&saradc 1>;
> +		io-channel-names = "buttons";
> +		keyup-threshold-microvolt = <1800000>;
> +		poll-interval = <100>;
> +
> +		vol-up-key {
> +			label = "volume up";
> +			linux,code = <KEY_VOLUMEUP>;
> +			press-threshold-microvolt = <17000>;
> +		};
> +
> +		vol-down-key {
> +			label = "volume down";
> +			linux,code = <KEY_VOLUMEDOWN>;
> +			press-threshold-microvolt = <417000>;
> +		};
> +
> +		menu-key {
> +			label = "menu";
> +			linux,code = <KEY_MENU>;
> +			press-threshold-microvolt = <890000>;
> +		};
> +
> +		back-key {
> +			label = "back";
> +			linux,code = <KEY_BACK>;
> +			press-threshold-microvolt = <1235000>;
> +		};
> +	};
> +
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		brightness-levels = <  0  20  20  21  21  22  22  23
> +					  23  24  24  25  25  26  26  27
> +					  27  28  28  29  29  30  30  31
> +					  31  32  32  33  33  34  34  35
> +					  35  36  36  37  37  38  38  39
> +					  40  41  42  43  44  45  46  47
> +					  48  49  50  51  52  53  54  55
> +					  56  57  58  59  60  61  62  63
> +					  64  65  66  67  68  69  70  71
> +					  72  73  74  75  76  77  78  79
> +					  80  81  82  83  84  85  86  87
> +					  88  89  90  91  92  93  94  95
> +					  96  97  98  99 100 101 102 103
> +					  104 105 106 107 108 109 110 111
> +					  112 113 114 115 116 117 118 119
> +					  120 121 122 123 124 125 126 127
> +					  128 129 130 131 132 133 134 135
> +					  136 137 138 139 140 141 142 143
> +					  144 145 146 147 148 149 150 151
> +					  152 153 154 155 156 157 158 159
> +					  160 161 162 163 164 165 166 167
> +					  168 169 170 171 172 173 174 175
> +					  176 177 178 179 180 181 182 183
> +					  184 185 186 187 188 189 190 191
> +					  192 193 194 195 196 197 198 199
> +					  200 201 202 203 204 205 206 207
> +					  208 209 210 211 212 213 214 215
> +					  216 217 218 219 220 221 222 223
> +					  224 225 226 227 228 229 230 231
> +					  232 233 234 235 236 237 238 239
> +					  240 241 242 243 244 245 246 247
> +					  248 249 250 251 252 253 254 255>;
> +		default-brightness-level = <200>;
> +		pwms = <&pwm2 0 25000 0>;
> +	};
> +
> +	backlight_1: backlight_1 {

No underscores in node names, use -

> +		compatible = "pwm-backlight";
> +		brightness-levels = <  0  20  20  21  21  22  22  23
> +					  23  24  24  25  25  26  26  27
> +					  27  28  28  29  29  30  30  31

...

> +
> +&combphy0_ps {
> +	status = "okay";
> +};
> +
> +&combphy2_psu {
> +	status = "okay";
> +};
> +
> +&cpu_b0 {
> +	cpu-supply = <&vdd_cpu_big0_s0>;
> +	mem-supply = <&vdd_cpu_big0_mem_s0>;
> +};
> +
> +&cpu_b1 {
> +	cpu-supply = <&vdd_cpu_big0_s0>;
> +	mem-supply = <&vdd_cpu_big0_mem_s0>;
> +};
> +
> +&cpu_b2 {
> +	cpu-supply = <&vdd_cpu_big1_s0>;
> +	mem-supply = <&vdd_cpu_big1_mem_s0>;
> +};
> +
> +&cpu_b3 {
> +	cpu-supply = <&vdd_cpu_big1_s0>;
> +	mem-supply = <&vdd_cpu_big1_mem_s0>;
> +};
> +
> +&cpu_l0 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
> +};
> +
> +&cpu_l1 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
> +};
> +
> +&cpu_l2 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
> +};
> +
> +&cpu_l3 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
> +};
> +
> +&gmac1 {
> +	clock_in_out = "output";
> +	phy-handle = <&rgmii_phy1>;
> +	phy-mode = "rgmii-rxid";
> +	pinctrl-0 = <&gmac1_miim
> +					&gmac1_tx_bus2
> +					&gmac1_rx_bus2
> +					&gmac1_rgmii_clk
> +					&gmac1_rgmii_bus>;

Messed alignment.

> +	pinctrl-names = "default";
> +	snps,reset-gpio = <&gpio3 RK_PB2 GPIO_ACTIVE_LOW>;
> +	snps,reset-active-low;
> +	snps,reset-delays-us = <0 20000 100000>;
> +	tx_delay = <0x42>;
> +	status = "okay";
> +};
> +

...

> +
> +&sfc {
> +	pinctrl-0 = <&fspim0_pins>;
> +	pinctrl-names = "default";
> +	max-freq = <100000000>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +
> +	spi_flash: spi-flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0x0>;
> +		spi-max-frequency = <100000000>;
> +		spi-tx-bus-width = <1>;
> +		spi-rx-bus-width = <4>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		status = "okay";

okay is by default, was it disabled anywhere?

> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			loader@0 {
> +				label = "loader";
> +				reg = <0x0 0x1000000>;
> +			};
> +		};
> +	};
> +};
> +


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] dt-bindings: arm: rockchip: Add Orange Pi 5 board
  2023-08-15 12:58 ` [PATCH 1/3] dt-bindings: arm: rockchip: Add Orange Pi 5 board Muhammed Efe Cetin
@ 2023-08-15 13:46   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-15 13:46 UTC (permalink / raw)
  To: Muhammed Efe Cetin, linux-rockchip
  Cc: devicetree, linux-arm-kernel, linux-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, heiko, sebastian.reichel

On 15/08/2023 14:58, Muhammed Efe Cetin wrote:
> Add Orange Pi 5 SBC from Xunlong.
> 
> Signed-off-by: Muhammed Efe Cetin <efectn@6tel.net>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-15 12:59 ` [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5 Muhammed Efe Cetin
  2023-08-15 13:45   ` Krzysztof Kozlowski
@ 2023-08-15 16:39   ` Ondřej Jirman
  2023-08-17 13:33     ` Muhammed Efe Cetin
  2023-08-17 21:05   ` Jonas Karlman
  2 siblings, 1 reply; 16+ messages in thread
From: Ondřej Jirman @ 2023-08-15 16:39 UTC (permalink / raw)
  To: Muhammed Efe Cetin
  Cc: linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
	robh+dt, krzysztof.kozlowski+dt, conor+dt, heiko,
	sebastian.reichel

Hello Muhammed,

Looks like you trusted Xunlong's DT a bit too much. See a few issues
below.

On Tue, Aug 15, 2023 at 03:59:01PM +0300, Muhammed Efe Cetin wrote:
> ing-List: linux-kernel@vger.kernel.org
> 
> Add initial support for OPi5 that includes support for USB2, PCIe2, Sata,
> Sdmmc, SPI Flash, PMIC.
> 
> Signed-off-by: Muhammed Efe Cetin <efectn@6tel.net>
> ---
>  .../boot/dts/rockchip/rk3588s-orangepi-5.dts  | 873 ++++++++++++++++++
>  1 file changed, 873 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> new file mode 100644
> index 000000000000..85071084a207
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> @@ -0,0 +1,873 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include "rk3588s.dtsi"
> +
> +/ {
> +	model = "Xunlong Orange Pi 5";
> +	compatible = "xunlong,orangepi-5", "rockchip,rk3588s";
> +
> +	aliases {
> +		mmc0 = &sdmmc;
> +
> +		serial2 = &uart2;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial2:1500000n8";
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 =<&leds_gpio>;
> +
> +		led@1 {
> +			gpios = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;
> +			label = "status_led";
> +			linux,default-trigger = "heartbeat";
> +			linux,default-trigger-delay-ms = <0>;

This is a PWM LED hooked to PWM0_M2. You can use a PWM LED drvier.

> +		};
> +	};
> +
> +	adc-keys {
> +		compatible = "adc-keys";
> +		io-channels = <&saradc 1>;
> +		io-channel-names = "buttons";
> +		keyup-threshold-microvolt = <1800000>;
> +		poll-interval = <100>;
> +
> +		vol-up-key {
> +			label = "volume up";
> +			linux,code = <KEY_VOLUMEUP>;
> +			press-threshold-microvolt = <17000>;
> +		};
> +
> +		vol-down-key {
> +			label = "volume down";
> +			linux,code = <KEY_VOLUMEDOWN>;
> +			press-threshold-microvolt = <417000>;
> +		};
> +
> +		menu-key {
> +			label = "menu";
> +			linux,code = <KEY_MENU>;
> +			press-threshold-microvolt = <890000>;
> +		};
> +
> +		back-key {
> +			label = "back";
> +			linux,code = <KEY_BACK>;
> +			press-threshold-microvolt = <1235000>;

None of these 4 buttons are in the scehmatic. There's one recovery button hooked
to saradc 0, instead.

> +		};
> +	};
> +
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		brightness-levels = <  0  20  20  21  21  22  22  23
> +					  23  24  24  25  25  26  26  27
> +					  27  28  28  29  29  30  30  31
> +					  31  32  32  33  33  34  34  35
> +					  35  36  36  37  37  38  38  39
> +					  40  41  42  43  44  45  46  47
> +					  48  49  50  51  52  53  54  55
> +					  56  57  58  59  60  61  62  63
> +					  64  65  66  67  68  69  70  71
> +					  72  73  74  75  76  77  78  79
> +					  80  81  82  83  84  85  86  87
> +					  88  89  90  91  92  93  94  95
> +					  96  97  98  99 100 101 102 103
> +					  104 105 106 107 108 109 110 111
> +					  112 113 114 115 116 117 118 119
> +					  120 121 122 123 124 125 126 127
> +					  128 129 130 131 132 133 134 135
> +					  136 137 138 139 140 141 142 143
> +					  144 145 146 147 148 149 150 151
> +					  152 153 154 155 156 157 158 159
> +					  160 161 162 163 164 165 166 167
> +					  168 169 170 171 172 173 174 175
> +					  176 177 178 179 180 181 182 183
> +					  184 185 186 187 188 189 190 191
> +					  192 193 194 195 196 197 198 199
> +					  200 201 202 203 204 205 206 207
> +					  208 209 210 211 212 213 214 215
> +					  216 217 218 219 220 221 222 223
> +					  224 225 226 227 228 229 230 231
> +					  232 233 234 235 236 237 238 239
> +					  240 241 242 243 244 245 246 247
> +					  248 249 250 251 252 253 254 255>;

Linar table will not give you "linear" looking brightness steps. Use some
algorithm to generate a proper curve of PWM duty cycle -> brightness table.

Either way, the board has no backlight. This should be in overlay if someone
wants to connect some particular display to the board.

> +		default-brightness-level = <200>;
> +		pwms = <&pwm2 0 25000 0>;
> +	};
> +
> +	backlight_1: backlight_1 {
> +		compatible = "pwm-backlight";
> +		brightness-levels = <  0  20  20  21  21  22  22  23
> +					  23  24  24  25  25  26  26  27
> +					  27  28  28  29  29  30  30  31
> +					  31  32  32  33  33  34  34  35
> +					  35  36  36  37  37  38  38  39
> +					  40  41  42  43  44  45  46  47
> +					  48  49  50  51  52  53  54  55
> +					  56  57  58  59  60  61  62  63
> +					  64  65  66  67  68  69  70  71
> +					  72  73  74  75  76  77  78  79
> +					  80  81  82  83  84  85  86  87
> +					  88  89  90  91  92  93  94  95
> +					  96  97  98  99 100 101 102 103
> +					  104 105 106 107 108 109 110 111
> +					  112 113 114 115 116 117 118 119
> +					  120 121 122 123 124 125 126 127
> +					  128 129 130 131 132 133 134 135
> +					  136 137 138 139 140 141 142 143
> +					  144 145 146 147 148 149 150 151
> +					  152 153 154 155 156 157 158 159
> +					  160 161 162 163 164 165 166 167
> +					  168 169 170 171 172 173 174 175
> +					  176 177 178 179 180 181 182 183
> +					  184 185 186 187 188 189 190 191
> +					  192 193 194 195 196 197 198 199
> +					  200 201 202 203 204 205 206 207
> +					  208 209 210 211 212 213 214 215
> +					  216 217 218 219 220 221 222 223
> +					  224 225 226 227 228 229 230 231
> +					  232 233 234 235 236 237 238 239
> +					  240 241 242 243 244 245 246 247
> +					  248 249 250 251 252 253 254 255>;
> +		default-brightness-level = <200>;
> +		pwms = <&pwm6 0 25000 0>;
> +	};

Ditto.

> +	vcc12v_dcin: vcc12v-dcin-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc12v_dcin";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +	};

The board has no 12V input. Please avoid low-effort copy pasting from vendor's
device tree. Check everything with the schematic to verify DT is actually
describing the HW correctly, if you really want to start from the vendor's DT
when porting the board to mainline Linux.

> +	vcc5v0_sys: vcc5v0-sys-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc5v0_sys";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		vin-supply = <&vcc12v_dcin>;
> +	};

Not supplied from 12V power supply.

> +	vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc_1v1_nldo_s3";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <1100000>;
> +		regulator-max-microvolt = <1100000>;
> +		vin-supply = <&vcc5v0_sys>;
> +	};

There's no such regulator on the board.

> +	vcc_3v3_sd_s0: vcc-3v3-sd-s0-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc_3v3_sd_s0";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpios = <&gpio4 RK_PB5 GPIO_ACTIVE_LOW>;
> +		enable-active-low;
> +		vin-supply = <&vcc_3v3_s3>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-state-mem {
> +			regulator-on-in-suspend;
> +		};

Fixed regulators don't implement regulator-state-*. Also why are you making
this regulator always on? It will be impossible for mmc subsystem to power
cycle the SD card. (without knowing, which is bad, because after powercycle
it will think that uSD I/O is in 3.3V, while it may still be in 1.8V mode)

> +	};
>
> +	vcc5v0_usbdcin: vcc5v0-usbdcin-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc5v0_usbdcin";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		vin-supply = <&vcc12v_dcin>;
> +	};

No such regulator on the board.

> +	vcc5v0_usb: vcc5v0-usb-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc5v0_usb";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		vin-supply = <&vcc5v0_usbdcin>;
> +	};

No such regulator on the board. Host ports' VBUS is either hardwired
to VCC_5V0 or goes throug a a protection circuit and becomes VCC5V0_USB_HOST.

> +	vbus5v0_typec: vbus5v0-typec-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vbus5v0_typec";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		enable-active-high;
> +		gpio = <&gpio3 RK_PC0 GPIO_ACTIVE_HIGH>;
> +		vin-supply = <&vcc5v0_usb>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&typec5v_pwren>;
> +	};

No such power rail on the board. This should be VBUS_TYPEC power rail.
Please name regulators/power rails as they are named in the schematic.
Otherwise it's impossible to cross-check DT against schematic.

> +	combophy_avdd0v85: combophy-avdd0v85-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "combophy_avdd0v85";
> +		regulator-boot-on;
> +		regulator-always-on;
> +		regulator-min-microvolt = <850000>;
> +		regulator-max-microvolt = <850000>;
> +		vin-supply = <&vdd_0v85_s0>;
> +	};

No such regulator on the board.

> +	combophy_avdd1v8: combophy-avdd1v8-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "combophy_avdd1v8";
> +		regulator-boot-on;
> +		regulator-always-on;
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		vin-supply = <&avcc_1v8_s0>;
> +	};

Again, no such regulator on the board.

> +
> +	vcc3v3_pcie2x1l2: vcc3v3-pcie2x1l2-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc3v3_pcie2x1l2";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		enable-active-high;
> +		regulator-boot-on;
> +		regulator-always-on;
> +		gpios = <&gpio0 RK_PC5 GPIO_ACTIVE_HIGH>;
> +		startup-delay-us = <50000>;

Startup delay for always-on regulator is kinda pointless. Also why is this
always on? It should not be.

> +		vin-supply = <&vcc5v0_sys>;
> +	};

No such regulator on the board. You might have meant VCC3V3_PCIE30,
or not?

> +};
> +
> +&combphy0_ps {
> +	status = "okay";
> +};
> +
> +&combphy2_psu {
> +	status = "okay";
> +};
> +
> +&cpu_b0 {
> +	cpu-supply = <&vdd_cpu_big0_s0>;
> +	mem-supply = <&vdd_cpu_big0_mem_s0>;
> +};
> +
> +&cpu_b1 {
> +	cpu-supply = <&vdd_cpu_big0_s0>;
> +	mem-supply = <&vdd_cpu_big0_mem_s0>;
> +};
> +
> +&cpu_b2 {
> +	cpu-supply = <&vdd_cpu_big1_s0>;
> +	mem-supply = <&vdd_cpu_big1_mem_s0>;
> +};
> +
> +&cpu_b3 {
> +	cpu-supply = <&vdd_cpu_big1_s0>;
> +	mem-supply = <&vdd_cpu_big1_mem_s0>;
> +};
> +
> +&cpu_l0 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
> +};
> +
> +&cpu_l1 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
> +};
> +
> +&cpu_l2 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
> +};
> +
> +&cpu_l3 {
> +	cpu-supply = <&vdd_cpu_lit_s0>;
> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
> +};
> +
> +&gmac1 {
> +	clock_in_out = "output";
> +	phy-handle = <&rgmii_phy1>;
> +	phy-mode = "rgmii-rxid";
> +	pinctrl-0 = <&gmac1_miim
> +					&gmac1_tx_bus2
> +					&gmac1_rx_bus2
> +					&gmac1_rgmii_clk
> +					&gmac1_rgmii_bus>;
> +	pinctrl-names = "default";
> +	snps,reset-gpio = <&gpio3 RK_PB2 GPIO_ACTIVE_LOW>;
> +	snps,reset-active-low;
> +	snps,reset-delays-us = <0 20000 100000>;
> +	tx_delay = <0x42>;
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0m2_xfer>;
> +	status = "okay";
> +
> +	vdd_cpu_big0_s0: vdd_cpu_big0_mem_s0: regulator@42 {
> +		compatible = "rockchip,rk8602";
> +		reg = <0x42>;
> +		fcs,suspend-voltage-selector = <1>;
> +		regulator-name = "vdd_cpu_big0_s0";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <550000>;
> +		regulator-max-microvolt = <1050000>;
> +		regulator-ramp-delay = <2300>;
> +		vin-supply = <&vcc5v0_sys>;
> +
> +		regulator-state-mem {
> +			regulator-off-in-suspend;
> +		};
> +	};
> +
> +	vdd_cpu_big1_s0: vdd_cpu_big1_mem_s0: regulator@43 {
> +		compatible = "rockchip,rk8603", "rockchip,rk8602";
> +		reg = <0x43>;
> +		fcs,suspend-voltage-selector = <1>;
> +		regulator-name = "vdd_cpu_big1_s0";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <550000>;
> +		regulator-max-microvolt = <1050000>;
> +		regulator-ramp-delay = <2300>;
> +		vin-supply = <&vcc5v0_sys>;
> +
> +		regulator-state-mem {
> +			regulator-off-in-suspend;
> +		};
> +	};
> +};
> +
> +&i2c2 {
> +	status = "okay";
> +
> +	vdd_npu_s0: vdd_npu_mem_s0: regulator@42 {
> +		compatible = "rockchip,rk8602";
> +		reg = <0x42>;
> +		fcs,suspend-voltage-selector = <1>;
> +		regulator-name = "vdd_npu_s0";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <550000>;
> +		regulator-max-microvolt = <950000>;
> +		regulator-ramp-delay = <2300>;
> +		vin-supply = <&vcc5v0_sys>;
> +
> +		regulator-state-mem {
> +			regulator-off-in-suspend;
> +		};
> +	};
> +};
> +
> +&i2c6 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c6m3_xfer>;
> +	status = "okay";
> +
> +	hym8563: rtc@51 {
> +		compatible = "haoyu,hym8563";
> +		reg = <0x51>;
> +		#clock-cells = <0>;
> +		clock-output-names = "hym8563";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&hym8563_int>;
> +		interrupt-parent = <&gpio0>;
> +		interrupts = <RK_PB0 IRQ_TYPE_LEVEL_LOW>;
> +		wakeup-source;
> +	};
> +};
> +
> +&mdio1 {
> +	rgmii_phy1: ethernet-phy@1 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0x1>;
> +	};
> +};
> +
> +&pcie2x1l2 {
> +	reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>;
> +	vpcie3v3-supply = <&vcc3v3_pcie2x1l2>;
> +	rockchip,skip-scan-in-resume;

There's no such property.

> +	status = "okay";
> +};
> +
> +&pinctrl {
> +	gpio-func {
> +		leds_gpio: leds-gpio {
> +			rockchip,pins = <0 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> +
> +	hym8563 {
> +		hym8563_int: hym8563-int {
> +			rockchip,pins = <0 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> +
> +	sata {
> +		sata_reset:sata-reset{
> +			rockchip,pins = <3 RK_PD1 RK_FUNC_GPIO &pcfg_pull_up>;

Not what you think: https://megous.com/dl/tmp/f135580b68cb5dfe.png

> +		};
> +	};
> +};
> +
> +&pwm2 {
> +	pinctrl-names = "active";
> +	pinctrl-0 = <&pwm2m0_pins>;
> +	status = "okay";
> +};
> +
> +&pwm6 {
> +	pinctrl-names = "active";
> +	pinctrl-0 = <&pwm6m0_pins>;
> +	status = "okay";
> +};
> +
> +&saradc {
> +	vref-supply = <&avcc_1v8_s0>;
> +	status = "okay";
> +};
> +
> +&sata0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&sata_reset>;
> +	status = "disabled";
> +};

Where do you see a SATA port? http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/details/Orange-Pi-5.html

Or a reset signal in the schematic?

> +&sdmmc {
> +	max-frequency = <150000000>;
> +	no-sdio;
> +	no-mmc;
> +	bus-width = <4>;
> +	cap-mmc-highspeed;
> +	cap-sd-highspeed;
> +	disable-wp;
> +	sd-uhs-sdr104;
> +	vmmc-supply = <&vcc_3v3_sd_s0>;
> +	vqmmc-supply = <&vccio_sd_s0>;
> +	status = "okay";
> +};
> +
> +&sfc {
> +	pinctrl-0 = <&fspim0_pins>;
> +	pinctrl-names = "default";
> +	max-freq = <100000000>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +
> +	spi_flash: spi-flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0x0>;
> +		spi-max-frequency = <100000000>;
> +		spi-tx-bus-width = <1>;
> +		spi-rx-bus-width = <4>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		status = "okay";
> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			loader@0 {
> +				label = "loader";
> +				reg = <0x0 0x1000000>;
> +			};
> +		};

Partitions are not really needed.

> +	};
> +};
> +
> +&spi2 {
> +	status = "okay";
> +	assigned-clocks = <&cru CLK_SPI2>;
> +	assigned-clock-rates = <200000000>;
> +	num-cs = <1>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi2m2_cs0 &spi2m2_pins>;
> +
> +	pmic@0 {
> +		compatible = "rockchip,rk806";
> +		reg = <0x0>;
> +		interrupt-parent = <&gpio0>;
> +		interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
> +				<&rk806_dvs2_null>, <&rk806_dvs3_null>;
> +		spi-max-frequency = <1000000>;
> +
> +		vcc1-supply = <&vcc5v0_sys>;
> +		vcc2-supply = <&vcc5v0_sys>;
> +		vcc3-supply = <&vcc5v0_sys>;
> +		vcc4-supply = <&vcc5v0_sys>;
> +		vcc5-supply = <&vcc5v0_sys>;
> +		vcc6-supply = <&vcc5v0_sys>;
> +		vcc7-supply = <&vcc5v0_sys>;
> +		vcc8-supply = <&vcc5v0_sys>;
> +		vcc9-supply = <&vcc5v0_sys>;
> +		vcc10-supply = <&vcc5v0_sys>;
> +		vcc11-supply = <&vcc_2v0_pldo_s3>;
> +		vcc12-supply = <&vcc5v0_sys>;
> +		vcc13-supply = <&vcc_1v1_nldo_s3>;
> +		vcc14-supply = <&vcc_1v1_nldo_s3>;
> +		vcca-supply = <&vcc5v0_sys>;
> +
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +
> +		rk806_dvs1_null: dvs1-null-pins {
> +			pins = "gpio_pwrctrl2";
> +			function = "pin_fun0";
> +		};
> +
> +		rk806_dvs2_null: dvs2-null-pins {
> +			pins = "gpio_pwrctrl2";
> +			function = "pin_fun0";
> +		};
> +
> +		rk806_dvs3_null: dvs3-null-pins {
> +			pins = "gpio_pwrctrl3";
> +			function = "pin_fun0";
> +		};
> +
> +		regulators {
> +			vdd_gpu_s0: vdd_gpu_mem_s0: dcdc-reg1 {
> +				regulator-name = "vdd_gpu_s0";
> +				regulator-boot-on;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <950000>;
> +				regulator-ramp-delay = <12500>;
> +				regulator-enable-ramp-delay = <400>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_cpu_lit_s0: vdd_cpu_lit_mem_s0: dcdc-reg2 {
> +				regulator-name = "vdd_cpu_lit_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <950000>;
> +				regulator-ramp-delay = <12500>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_log_s0: dcdc-reg3 {
> +				regulator-name = "vdd_log_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <675000>;
> +				regulator-max-microvolt = <750000>;
> +				regulator-ramp-delay = <12500>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +					regulator-suspend-microvolt = <750000>;
> +				};
> +			};
> +
> +			vdd_vdenc_s0: vdd_vdenc_mem_s0: dcdc-reg4 {
> +				regulator-name = "vdd_vdenc_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <550000>;
> +				regulator-max-microvolt = <950000>;
> +				regulator-ramp-delay = <12500>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_ddr_s0: dcdc-reg5 {
> +				regulator-name = "vdd_ddr_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <675000>;
> +				regulator-max-microvolt = <900000>;
> +				regulator-ramp-delay = <12500>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +					regulator-suspend-microvolt = <850000>;
> +				};
> +			};
> +
> +			vdd2_ddr_s3: dcdc-reg6 {
> +				regulator-name = "vdd2_ddr_s3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +				};
> +			};
> +
> +			vcc_2v0_pldo_s3: dcdc-reg7 {
> +				regulator-name = "vdd_2v0_pldo_s3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <2000000>;
> +				regulator-max-microvolt = <2000000>;
> +				regulator-ramp-delay = <12500>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <2000000>;
> +				};
> +			};
> +
> +			vcc_3v3_s3: dcdc-reg8 {
> +				regulator-name = "vcc_3v3_s3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <3300000>;
> +				};
> +			};
> +
> +			vddq_ddr_s0: dcdc-reg9 {
> +				regulator-name = "vddq_ddr_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_1v8_s3: dcdc-reg10 {
> +				regulator-name = "vcc_1v8_s3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <1800000>;
> +				};
> +			};
> +
> +			avcc_1v8_s0: pldo-reg1 {
> +				regulator-name = "avcc_1v8_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_1v8_s0: pldo-reg2 {
> +				regulator-name = "vcc_1v8_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +					regulator-suspend-microvolt = <1800000>;
> +				};
> +			};
> +
> +			avdd_1v2_s0: pldo-reg3 {
> +				regulator-name = "avdd_1v2_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_3v3_s0: pldo-reg4 {
> +				regulator-name = "vcc_3v3_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-ramp-delay = <12500>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vccio_sd_s0: pldo-reg5 {
> +				regulator-name = "vccio_sd_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-ramp-delay = <12500>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			pldo6_s3: pldo-reg6 {
> +				regulator-name = "pldo6_s3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <1800000>;
> +				};
> +			};
> +
> +			vdd_0v75_s3: nldo-reg1 {
> +				regulator-name = "vdd_0v75_s3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <750000>;
> +				regulator-max-microvolt = <750000>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <750000>;
> +				};
> +			};
> +
> +			vdd_ddr_pll_s0: nldo-reg2 {
> +				regulator-name = "vdd_ddr_pll_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <850000>;
> +				regulator-max-microvolt = <850000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +					regulator-suspend-microvolt = <850000>;
> +				};
> +			};
> +
> +			avdd_0v75_s0: nldo-reg3 {
> +				regulator-name = "avdd_0v75_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <750000>;
> +				regulator-max-microvolt = <750000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_0v85_s0: nldo-reg4 {
> +				regulator-name = "vdd_0v85_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <850000>;
> +				regulator-max-microvolt = <850000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_0v75_s0: nldo-reg5 {
> +				regulator-name = "vdd_0v75_s0";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <750000>;
> +				regulator-max-microvolt = <750000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};

Have you checked all of the above regulators against the schematic?

> +		};
> +	};
> +};
> +
> +&tsadc {
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	pinctrl-0 = <&uart2m0_xfer>;
> +	status = "okay";
> +};
> +
> +&u2phy0 {
> +	status = "okay";
> +};
> +
> +&u2phy0_otg {
> +	rockchip,typec-vbus-det;

No such property.

That's all from me, but it's not an exhaustive list of issues. You should
go through the schematic and verify that every single node in DT actually
matches what's in there.

kind regards,
	o.

> +	status = "okay";
> +};
> +
> +&u2phy2 {
> +	status = "okay";
> +};
> +
> +&u2phy2_host {
> +	status = "okay";
> +};
> +
> +&u2phy3 {
> +	status = "okay";
> +};
> +
> +&u2phy3_host {
> +	status = "okay";
> +};
> +
> +&usb_host0_ehci {
> +	status = "okay";
> +};
> +
> +&usb_host0_ohci {
> +	status = "okay";
> +};
> +
> +&usb_host1_ehci {
> +	status = "okay";
> +};
> +
> +&usb_host1_ohci {
> +	status = "okay";
> +};
>
> +&wdt {
> +	status = "okay";
> +};
> -- 
> 2.41.0
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Re: [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-15 16:39   ` Ondřej Jirman
@ 2023-08-17 13:33     ` Muhammed Efe Cetin
  2023-08-17 13:57       ` Ondřej Jirman
  0 siblings, 1 reply; 16+ messages in thread
From: Muhammed Efe Cetin @ 2023-08-17 13:33 UTC (permalink / raw)
  To: megi
  Cc: conor+dt, devicetree, efectn, heiko, krzysztof.kozlowski+dt,
	linux-arm-kernel, linux-kernel, linux-rockchip, robh+dt,
	sebastian.reichel

Hi, Ondřej

Thanks for reviews, i'll fix them soon.

On 15.08.2023 19:39, Ondřej Jirman wrote:
> Hello Muhammed,
> 
> Looks like you trusted Xunlong's DT a bit too much. See a few issues
> below.
> 
> On Tue, Aug 15, 2023 at 03:59:01PM +0300, Muhammed Efe Cetin wrote:
>> ing-List: linux-kernel@vger.kernel.org
>>
>> Add initial support for OPi5 that includes support for USB2, PCIe2, Sata,
>> Sdmmc, SPI Flash, PMIC.
>>
>> Signed-off-by: Muhammed Efe Cetin <efectn@6tel.net>
>> ---
>>   .../boot/dts/rockchip/rk3588s-orangepi-5.dts  | 873 ++++++++++++++++++
>>   1 file changed, 873 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
>> new file mode 100644
>> index 000000000000..85071084a207
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
>> @@ -0,0 +1,873 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +
>> +/dts-v1/;
>> +
>> +#include <dt-bindings/pinctrl/rockchip.h>
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +#include "rk3588s.dtsi"
>> +
>> +/ {
>> +	model = "Xunlong Orange Pi 5";
>> +	compatible = "xunlong,orangepi-5", "rockchip,rk3588s";
>> +
>> +	aliases {
>> +		mmc0 = &sdmmc;
>> +
>> +		serial2 = &uart2;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial2:1500000n8";
>> +	};
>> +
>> +	leds {
>> +		compatible = "gpio-leds";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 =<&leds_gpio>;
>> +
>> +		led@1 {
>> +			gpios = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;
>> +			label = "status_led";
>> +			linux,default-trigger = "heartbeat";
>> +			linux,default-trigger-delay-ms = <0>;
> 
> This is a PWM LED hooked to PWM0_M2. You can use a PWM LED drvier.

Yes the mosfet is connected to PWM line. I'll try to use pwm-leds here.

> 
>> +		};
>> +	};
>> +
>> +	adc-keys {
>> +		compatible = "adc-keys";
>> +		io-channels = <&saradc 1>;
>> +		io-channel-names = "buttons";
>> +		keyup-threshold-microvolt = <1800000>;
>> +		poll-interval = <100>;
>> +
>> +		vol-up-key {
>> +			label = "volume up";
>> +			linux,code = <KEY_VOLUMEUP>;
>> +			press-threshold-microvolt = <17000>;
>> +		};
>> +
>> +		vol-down-key {
>> +			label = "volume down";
>> +			linux,code = <KEY_VOLUMEDOWN>;
>> +			press-threshold-microvolt = <417000>;
>> +		};
>> +
>> +		menu-key {
>> +			label = "menu";
>> +			linux,code = <KEY_MENU>;
>> +			press-threshold-microvolt = <890000>;
>> +		};
>> +
>> +		back-key {
>> +			label = "back";
>> +			linux,code = <KEY_BACK>;
>> +			press-threshold-microvolt = <1235000>;
> 
> None of these 4 buttons are in the scehmatic. There's one recovery button hooked
> to saradc 0, instead.

Will remove these too. I won't add recovery button because of it's mostly used for rockusb.

> 
>> +		};
>> +	};
>> +
>> +	backlight: backlight {
>> +		compatible = "pwm-backlight";
>> +		brightness-levels = <  0  20  20  21  21  22  22  23
>> +					  23  24  24  25  25  26  26  27
>> +					  27  28  28  29  29  30  30  31
>> +					  31  32  32  33  33  34  34  35
>> +					  35  36  36  37  37  38  38  39
>> +					  40  41  42  43  44  45  46  47
>> +					  48  49  50  51  52  53  54  55
>> +					  56  57  58  59  60  61  62  63
>> +					  64  65  66  67  68  69  70  71
>> +					  72  73  74  75  76  77  78  79
>> +					  80  81  82  83  84  85  86  87
>> +					  88  89  90  91  92  93  94  95
>> +					  96  97  98  99 100 101 102 103
>> +					  104 105 106 107 108 109 110 111
>> +					  112 113 114 115 116 117 118 119
>> +					  120 121 122 123 124 125 126 127
>> +					  128 129 130 131 132 133 134 135
>> +					  136 137 138 139 140 141 142 143
>> +					  144 145 146 147 148 149 150 151
>> +					  152 153 154 155 156 157 158 159
>> +					  160 161 162 163 164 165 166 167
>> +					  168 169 170 171 172 173 174 175
>> +					  176 177 178 179 180 181 182 183
>> +					  184 185 186 187 188 189 190 191
>> +					  192 193 194 195 196 197 198 199
>> +					  200 201 202 203 204 205 206 207
>> +					  208 209 210 211 212 213 214 215
>> +					  216 217 218 219 220 221 222 223
>> +					  224 225 226 227 228 229 230 231
>> +					  232 233 234 235 236 237 238 239
>> +					  240 241 242 243 244 245 246 247
>> +					  248 249 250 251 252 253 254 255>;
> 
> Linar table will not give you "linear" looking brightness steps. Use some
> algorithm to generate a proper curve of PWM duty cycle -> brightness table.
> 
> Either way, the board has no backlight. This should be in overlay if someone
> wants to connect some particular display to the board.

You are right. It's better idea to use them in overlay instead of making them a part of devicetree.

> 
>> +		default-brightness-level = <200>;
>> +		pwms = <&pwm2 0 25000 0>;
>> +	};
>> +
>> +	backlight_1: backlight_1 {
>> +		compatible = "pwm-backlight";
>> +		brightness-levels = <  0  20  20  21  21  22  22  23
>> +					  23  24  24  25  25  26  26  27
>> +					  27  28  28  29  29  30  30  31
>> +					  31  32  32  33  33  34  34  35
>> +					  35  36  36  37  37  38  38  39
>> +					  40  41  42  43  44  45  46  47
>> +					  48  49  50  51  52  53  54  55
>> +					  56  57  58  59  60  61  62  63
>> +					  64  65  66  67  68  69  70  71
>> +					  72  73  74  75  76  77  78  79
>> +					  80  81  82  83  84  85  86  87
>> +					  88  89  90  91  92  93  94  95
>> +					  96  97  98  99 100 101 102 103
>> +					  104 105 106 107 108 109 110 111
>> +					  112 113 114 115 116 117 118 119
>> +					  120 121 122 123 124 125 126 127
>> +					  128 129 130 131 132 133 134 135
>> +					  136 137 138 139 140 141 142 143
>> +					  144 145 146 147 148 149 150 151
>> +					  152 153 154 155 156 157 158 159
>> +					  160 161 162 163 164 165 166 167
>> +					  168 169 170 171 172 173 174 175
>> +					  176 177 178 179 180 181 182 183
>> +					  184 185 186 187 188 189 190 191
>> +					  192 193 194 195 196 197 198 199
>> +					  200 201 202 203 204 205 206 207
>> +					  208 209 210 211 212 213 214 215
>> +					  216 217 218 219 220 221 222 223
>> +					  224 225 226 227 228 229 230 231
>> +					  232 233 234 235 236 237 238 239
>> +					  240 241 242 243 244 245 246 247
>> +					  248 249 250 251 252 253 254 255>;
>> +		default-brightness-level = <200>;
>> +		pwms = <&pwm6 0 25000 0>;
>> +	};
> 
> Ditto.
> 
>> +	vcc12v_dcin: vcc12v-dcin-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vcc12v_dcin";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +		regulator-min-microvolt = <12000000>;
>> +		regulator-max-microvolt = <12000000>;
>> +	};
> 
> The board has no 12V input. Please avoid low-effort copy pasting from vendor's
> device tree. Check everything with the schematic to verify DT is actually
> describing the HW correctly, if you really want to start from the vendor's DT
> when porting the board to mainline Linux.

Thanks i'll remove it.

> 
>> +	vcc5v0_sys: vcc5v0-sys-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vcc5v0_sys";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		vin-supply = <&vcc12v_dcin>;
>> +	};
> 
> Not supplied from 12V power supply.
> 
>> +	vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vcc_1v1_nldo_s3";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +		regulator-min-microvolt = <1100000>;
>> +		regulator-max-microvolt = <1100000>;
>> +		vin-supply = <&vcc5v0_sys>;
>> +	};
> 
> There's no such regulator on the board.

It's connected to PMIC https://i.imgur.com/sVJdC5K.png

> 
>> +	vcc_3v3_sd_s0: vcc-3v3-sd-s0-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vcc_3v3_sd_s0";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpios = <&gpio4 RK_PB5 GPIO_ACTIVE_LOW>;
>> +		enable-active-low;
>> +		vin-supply = <&vcc_3v3_s3>;
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +		regulator-state-mem {
>> +			regulator-on-in-suspend;
>> +		};
> 
> Fixed regulators don't implement regulator-state-*. Also why are you making
> this regulator always on? It will be impossible for mmc subsystem to power
> cycle the SD card. (without knowing, which is bad, because after powercycle
> it will think that uSD I/O is in 3.3V, while it may still be in 1.8V mode)

I'll remove regulator-always-on and regulator-state-mem.

> 
>> +	};
>>
>> +	vcc5v0_usbdcin: vcc5v0-usbdcin-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vcc5v0_usbdcin";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		vin-supply = <&vcc12v_dcin>;
>> +	};
> 
> No such regulator on the board.

Will remove it.

> 
>> +	vcc5v0_usb: vcc5v0-usb-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vcc5v0_usb";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		vin-supply = <&vcc5v0_usbdcin>;
>> +	};
> 
> No such regulator on the board. Host ports' VBUS is either hardwired
> to VCC_5V0 or goes throug a a protection circuit and becomes VCC5V0_USB_HOST.

Will remove it.

> 
>> +	vbus5v0_typec: vbus5v0-typec-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vbus5v0_typec";
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		enable-active-high;
>> +		gpio = <&gpio3 RK_PC0 GPIO_ACTIVE_HIGH>;
>> +		vin-supply = <&vcc5v0_usb>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&typec5v_pwren>;
>> +	};
> 
> No such power rail on the board. This should be VBUS_TYPEC power rail.
> Please name regulators/power rails as they are named in the schematic.
> Otherwise it's impossible to cross-check DT against schematic.

Perhaps i can add it as a comment line. Otherwise the current name seems OK to me but still open to suggestions.

> 
>> +	combophy_avdd0v85: combophy-avdd0v85-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "combophy_avdd0v85";
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +		regulator-min-microvolt = <850000>;
>> +		regulator-max-microvolt = <850000>;
>> +		vin-supply = <&vdd_0v85_s0>;
>> +	};
> 
> No such regulator on the board.

Will remove it.

> 
>> +	combophy_avdd1v8: combophy-avdd1v8-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "combophy_avdd1v8";
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		vin-supply = <&avcc_1v8_s0>;
>> +	};
> 
> Again, no such regulator on the board.

Will remove it.

> 
>> +
>> +	vcc3v3_pcie2x1l2: vcc3v3-pcie2x1l2-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vcc3v3_pcie2x1l2";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		enable-active-high;
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +		gpios = <&gpio0 RK_PC5 GPIO_ACTIVE_HIGH>;
>> +		startup-delay-us = <50000>;
> 
> Startup delay for always-on regulator is kinda pointless. Also why is this
> always on? It should not be.

Agree, i'll remove regulator-always-on property.

> 
>> +		vin-supply = <&vcc5v0_sys>;
>> +	};
> 
> No such regulator on the board. You might have meant VCC3V3_PCIE30,
> or not?

Yes. However their naming in schematics is pretty nonsense. The board doesn't have PCIe3, i think current naming is more appropriate.

> 
>> +};
>> +
>> +&combphy0_ps {
>> +	status = "okay";
>> +};
>> +
>> +&combphy2_psu {
>> +	status = "okay";
>> +};
>> +
>> +&cpu_b0 {
>> +	cpu-supply = <&vdd_cpu_big0_s0>;
>> +	mem-supply = <&vdd_cpu_big0_mem_s0>;
>> +};
>> +
>> +&cpu_b1 {
>> +	cpu-supply = <&vdd_cpu_big0_s0>;
>> +	mem-supply = <&vdd_cpu_big0_mem_s0>;
>> +};
>> +
>> +&cpu_b2 {
>> +	cpu-supply = <&vdd_cpu_big1_s0>;
>> +	mem-supply = <&vdd_cpu_big1_mem_s0>;
>> +};
>> +
>> +&cpu_b3 {
>> +	cpu-supply = <&vdd_cpu_big1_s0>;
>> +	mem-supply = <&vdd_cpu_big1_mem_s0>;
>> +};
>> +
>> +&cpu_l0 {
>> +	cpu-supply = <&vdd_cpu_lit_s0>;
>> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
>> +};
>> +
>> +&cpu_l1 {
>> +	cpu-supply = <&vdd_cpu_lit_s0>;
>> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
>> +};
>> +
>> +&cpu_l2 {
>> +	cpu-supply = <&vdd_cpu_lit_s0>;
>> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
>> +};
>> +
>> +&cpu_l3 {
>> +	cpu-supply = <&vdd_cpu_lit_s0>;
>> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
>> +};
>> +
>> +&gmac1 {
>> +	clock_in_out = "output";
>> +	phy-handle = <&rgmii_phy1>;
>> +	phy-mode = "rgmii-rxid";
>> +	pinctrl-0 = <&gmac1_miim
>> +					&gmac1_tx_bus2
>> +					&gmac1_rx_bus2
>> +					&gmac1_rgmii_clk
>> +					&gmac1_rgmii_bus>;
>> +	pinctrl-names = "default";
>> +	snps,reset-gpio = <&gpio3 RK_PB2 GPIO_ACTIVE_LOW>;
>> +	snps,reset-active-low;
>> +	snps,reset-delays-us = <0 20000 100000>;
>> +	tx_delay = <0x42>;
>> +	status = "okay";
>> +};
>> +
>> +&i2c0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c0m2_xfer>;
>> +	status = "okay";
>> +
>> +	vdd_cpu_big0_s0: vdd_cpu_big0_mem_s0: regulator@42 {
>> +		compatible = "rockchip,rk8602";
>> +		reg = <0x42>;
>> +		fcs,suspend-voltage-selector = <1>;
>> +		regulator-name = "vdd_cpu_big0_s0";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +		regulator-min-microvolt = <550000>;
>> +		regulator-max-microvolt = <1050000>;
>> +		regulator-ramp-delay = <2300>;
>> +		vin-supply = <&vcc5v0_sys>;
>> +
>> +		regulator-state-mem {
>> +			regulator-off-in-suspend;
>> +		};
>> +	};
>> +
>> +	vdd_cpu_big1_s0: vdd_cpu_big1_mem_s0: regulator@43 {
>> +		compatible = "rockchip,rk8603", "rockchip,rk8602";
>> +		reg = <0x43>;
>> +		fcs,suspend-voltage-selector = <1>;
>> +		regulator-name = "vdd_cpu_big1_s0";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +		regulator-min-microvolt = <550000>;
>> +		regulator-max-microvolt = <1050000>;
>> +		regulator-ramp-delay = <2300>;
>> +		vin-supply = <&vcc5v0_sys>;
>> +
>> +		regulator-state-mem {
>> +			regulator-off-in-suspend;
>> +		};
>> +	};
>> +};
>> +
>> +&i2c2 {
>> +	status = "okay";
>> +
>> +	vdd_npu_s0: vdd_npu_mem_s0: regulator@42 {
>> +		compatible = "rockchip,rk8602";
>> +		reg = <0x42>;
>> +		fcs,suspend-voltage-selector = <1>;
>> +		regulator-name = "vdd_npu_s0";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +		regulator-min-microvolt = <550000>;
>> +		regulator-max-microvolt = <950000>;
>> +		regulator-ramp-delay = <2300>;
>> +		vin-supply = <&vcc5v0_sys>;
>> +
>> +		regulator-state-mem {
>> +			regulator-off-in-suspend;
>> +		};
>> +	};
>> +};
>> +
>> +&i2c6 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c6m3_xfer>;
>> +	status = "okay";
>> +
>> +	hym8563: rtc@51 {
>> +		compatible = "haoyu,hym8563";
>> +		reg = <0x51>;
>> +		#clock-cells = <0>;
>> +		clock-output-names = "hym8563";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&hym8563_int>;
>> +		interrupt-parent = <&gpio0>;
>> +		interrupts = <RK_PB0 IRQ_TYPE_LEVEL_LOW>;
>> +		wakeup-source;
>> +	};
>> +};
>> +
>> +&mdio1 {
>> +	rgmii_phy1: ethernet-phy@1 {
>> +		compatible = "ethernet-phy-ieee802.3-c22";
>> +		reg = <0x1>;
>> +	};
>> +};
>> +
>> +&pcie2x1l2 {
>> +	reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>;
>> +	vpcie3v3-supply = <&vcc3v3_pcie2x1l2>;
>> +	rockchip,skip-scan-in-resume;
> 
> There's no such property.

Will remove it.

> 
>> +	status = "okay";
>> +};
>> +
>> +&pinctrl {
>> +	gpio-func {
>> +		leds_gpio: leds-gpio {
>> +			rockchip,pins = <0 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>;
>> +		};
>> +	};
>> +
>> +	hym8563 {
>> +		hym8563_int: hym8563-int {
>> +			rockchip,pins = <0 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
>> +		};
>> +	};
>> +
>> +	sata {
>> +		sata_reset:sata-reset{
>> +			rockchip,pins = <3 RK_PD1 RK_FUNC_GPIO &pcfg_pull_up>;
> 
> Not what you think: https://megous.com/dl/tmp/f135580b68cb5dfe.png
> 
>> +		};
>> +	};
>> +};
>> +
>> +&pwm2 {
>> +	pinctrl-names = "active";
>> +	pinctrl-0 = <&pwm2m0_pins>;
>> +	status = "okay";
>> +};
>> +
>> +&pwm6 {
>> +	pinctrl-names = "active";
>> +	pinctrl-0 = <&pwm6m0_pins>;
>> +	status = "okay";
>> +};
>> +
>> +&saradc {
>> +	vref-supply = <&avcc_1v8_s0>;
>> +	status = "okay";
>> +};
>> +
>> +&sata0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&sata_reset>;
>> +	status = "disabled";
>> +};
> 
> Where do you see a SATA port? http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/details/Orange-Pi-5.html
> 
> Or a reset signal in the schematic?

It's needed to use sata node once you want to use mSata SSD. I don't know if it works without sata_reset pinctrl. Don't have mSata SSD to try.

> 
>> +&sdmmc {
>> +	max-frequency = <150000000>;
>> +	no-sdio;
>> +	no-mmc;
>> +	bus-width = <4>;
>> +	cap-mmc-highspeed;
>> +	cap-sd-highspeed;
>> +	disable-wp;
>> +	sd-uhs-sdr104;
>> +	vmmc-supply = <&vcc_3v3_sd_s0>;
>> +	vqmmc-supply = <&vccio_sd_s0>;
>> +	status = "okay";
>> +};
>> +
>> +&sfc {
>> +	pinctrl-0 = <&fspim0_pins>;
>> +	pinctrl-names = "default";
>> +	max-freq = <100000000>;
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	status = "okay";
>> +
>> +	spi_flash: spi-flash@0 {
>> +		compatible = "jedec,spi-nor";
>> +		reg = <0x0>;
>> +		spi-max-frequency = <100000000>;
>> +		spi-tx-bus-width = <1>;
>> +		spi-rx-bus-width = <4>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		status = "okay";
>> +
>> +		partitions {
>> +			compatible = "fixed-partitions";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +
>> +			loader@0 {
>> +				label = "loader";
>> +				reg = <0x0 0x1000000>;
>> +			};
>> +		};
> 
> Partitions are not really needed.

Will remove it.

> 
>> +	};
>> +};
>> +
>> +&spi2 {
>> +	status = "okay";
>> +	assigned-clocks = <&cru CLK_SPI2>;
>> +	assigned-clock-rates = <200000000>;
>> +	num-cs = <1>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&spi2m2_cs0 &spi2m2_pins>;
>> +
>> +	pmic@0 {
>> +		compatible = "rockchip,rk806";
>> +		reg = <0x0>;
>> +		interrupt-parent = <&gpio0>;
>> +		interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
>> +				<&rk806_dvs2_null>, <&rk806_dvs3_null>;
>> +		spi-max-frequency = <1000000>;
>> +
>> +		vcc1-supply = <&vcc5v0_sys>;
>> +		vcc2-supply = <&vcc5v0_sys>;
>> +		vcc3-supply = <&vcc5v0_sys>;
>> +		vcc4-supply = <&vcc5v0_sys>;
>> +		vcc5-supply = <&vcc5v0_sys>;
>> +		vcc6-supply = <&vcc5v0_sys>;
>> +		vcc7-supply = <&vcc5v0_sys>;
>> +		vcc8-supply = <&vcc5v0_sys>;
>> +		vcc9-supply = <&vcc5v0_sys>;
>> +		vcc10-supply = <&vcc5v0_sys>;
>> +		vcc11-supply = <&vcc_2v0_pldo_s3>;
>> +		vcc12-supply = <&vcc5v0_sys>;
>> +		vcc13-supply = <&vcc_1v1_nldo_s3>;
>> +		vcc14-supply = <&vcc_1v1_nldo_s3>;
>> +		vcca-supply = <&vcc5v0_sys>;
>> +
>> +		gpio-controller;
>> +		#gpio-cells = <2>;
>> +
>> +		rk806_dvs1_null: dvs1-null-pins {
>> +			pins = "gpio_pwrctrl2";
>> +			function = "pin_fun0";
>> +		};
>> +
>> +		rk806_dvs2_null: dvs2-null-pins {
>> +			pins = "gpio_pwrctrl2";
>> +			function = "pin_fun0";
>> +		};
>> +
>> +		rk806_dvs3_null: dvs3-null-pins {
>> +			pins = "gpio_pwrctrl3";
>> +			function = "pin_fun0";
>> +		};
>> +
>> +		regulators {
>> +			vdd_gpu_s0: vdd_gpu_mem_s0: dcdc-reg1 {
>> +				regulator-name = "vdd_gpu_s0";
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <550000>;
>> +				regulator-max-microvolt = <950000>;
>> +				regulator-ramp-delay = <12500>;
>> +				regulator-enable-ramp-delay = <400>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
>> +
>> +			vdd_cpu_lit_s0: vdd_cpu_lit_mem_s0: dcdc-reg2 {
>> +				regulator-name = "vdd_cpu_lit_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <550000>;
>> +				regulator-max-microvolt = <950000>;
>> +				regulator-ramp-delay = <12500>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
>> +
>> +			vdd_log_s0: dcdc-reg3 {
>> +				regulator-name = "vdd_log_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <675000>;
>> +				regulator-max-microvolt = <750000>;
>> +				regulator-ramp-delay = <12500>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +					regulator-suspend-microvolt = <750000>;
>> +				};
>> +			};
>> +
>> +			vdd_vdenc_s0: vdd_vdenc_mem_s0: dcdc-reg4 {
>> +				regulator-name = "vdd_vdenc_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <550000>;
>> +				regulator-max-microvolt = <950000>;
>> +				regulator-ramp-delay = <12500>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
>> +
>> +			vdd_ddr_s0: dcdc-reg5 {
>> +				regulator-name = "vdd_ddr_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <675000>;
>> +				regulator-max-microvolt = <900000>;
>> +				regulator-ramp-delay = <12500>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +					regulator-suspend-microvolt = <850000>;
>> +				};
>> +			};
>> +
>> +			vdd2_ddr_s3: dcdc-reg6 {
>> +				regulator-name = "vdd2_ddr_s3";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +
>> +				regulator-state-mem {
>> +					regulator-on-in-suspend;
>> +				};
>> +			};
>> +
>> +			vcc_2v0_pldo_s3: dcdc-reg7 {
>> +				regulator-name = "vdd_2v0_pldo_s3";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <2000000>;
>> +				regulator-max-microvolt = <2000000>;
>> +				regulator-ramp-delay = <12500>;
>> +
>> +				regulator-state-mem {
>> +					regulator-on-in-suspend;
>> +					regulator-suspend-microvolt = <2000000>;
>> +				};
>> +			};
>> +
>> +			vcc_3v3_s3: dcdc-reg8 {
>> +				regulator-name = "vcc_3v3_s3";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <3300000>;
>> +				regulator-max-microvolt = <3300000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-on-in-suspend;
>> +					regulator-suspend-microvolt = <3300000>;
>> +				};
>> +			};
>> +
>> +			vddq_ddr_s0: dcdc-reg9 {
>> +				regulator-name = "vddq_ddr_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
>> +
>> +			vcc_1v8_s3: dcdc-reg10 {
>> +				regulator-name = "vcc_1v8_s3";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <1800000>;
>> +				regulator-max-microvolt = <1800000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-on-in-suspend;
>> +					regulator-suspend-microvolt = <1800000>;
>> +				};
>> +			};
>> +
>> +			avcc_1v8_s0: pldo-reg1 {
>> +				regulator-name = "avcc_1v8_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <1800000>;
>> +				regulator-max-microvolt = <1800000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
>> +
>> +			vcc_1v8_s0: pldo-reg2 {
>> +				regulator-name = "vcc_1v8_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <1800000>;
>> +				regulator-max-microvolt = <1800000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +					regulator-suspend-microvolt = <1800000>;
>> +				};
>> +			};
>> +
>> +			avdd_1v2_s0: pldo-reg3 {
>> +				regulator-name = "avdd_1v2_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <1200000>;
>> +				regulator-max-microvolt = <1200000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
>> +
>> +			vcc_3v3_s0: pldo-reg4 {
>> +				regulator-name = "vcc_3v3_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <3300000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				regulator-ramp-delay = <12500>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
>> +
>> +			vccio_sd_s0: pldo-reg5 {
>> +				regulator-name = "vccio_sd_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <1800000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				regulator-ramp-delay = <12500>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
>> +
>> +			pldo6_s3: pldo-reg6 {
>> +				regulator-name = "pldo6_s3";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <1800000>;
>> +				regulator-max-microvolt = <1800000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-on-in-suspend;
>> +					regulator-suspend-microvolt = <1800000>;
>> +				};
>> +			};
>> +
>> +			vdd_0v75_s3: nldo-reg1 {
>> +				regulator-name = "vdd_0v75_s3";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <750000>;
>> +				regulator-max-microvolt = <750000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-on-in-suspend;
>> +					regulator-suspend-microvolt = <750000>;
>> +				};
>> +			};
>> +
>> +			vdd_ddr_pll_s0: nldo-reg2 {
>> +				regulator-name = "vdd_ddr_pll_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <850000>;
>> +				regulator-max-microvolt = <850000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +					regulator-suspend-microvolt = <850000>;
>> +				};
>> +			};
>> +
>> +			avdd_0v75_s0: nldo-reg3 {
>> +				regulator-name = "avdd_0v75_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <750000>;
>> +				regulator-max-microvolt = <750000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
>> +
>> +			vdd_0v85_s0: nldo-reg4 {
>> +				regulator-name = "vdd_0v85_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <850000>;
>> +				regulator-max-microvolt = <850000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
>> +
>> +			vdd_0v75_s0: nldo-reg5 {
>> +				regulator-name = "vdd_0v75_s0";
>> +				regulator-always-on;
>> +				regulator-boot-on;
>> +				regulator-min-microvolt = <750000>;
>> +				regulator-max-microvolt = <750000>;
>> +
>> +				regulator-state-mem {
>> +					regulator-off-in-suspend;
>> +				};
>> +			};
> 
> Have you checked all of the above regulators against the schematic?

Just checked bucks roughly. I think they are OK.

> 
>> +		};
>> +	};
>> +};
>> +
>> +&tsadc {
>> +	status = "okay";
>> +};
>> +
>> +&uart2 {
>> +	pinctrl-0 = <&uart2m0_xfer>;
>> +	status = "okay";
>> +};
>> +
>> +&u2phy0 {
>> +	status = "okay";
>> +};
>> +
>> +&u2phy0_otg {
>> +	rockchip,typec-vbus-det;
> 
> No such property.

Will remove it.

> 
> That's all from me, but it's not an exhaustive list of issues. You should
> go through the schematic and verify that every single node in DT actually
> matches what's in there.
> 
> kind regards,
> 	o.
> 
>> +	status = "okay";
>> +};
>> +
>> +&u2phy2 {
>> +	status = "okay";
>> +};
>> +
>> +&u2phy2_host {
>> +	status = "okay";
>> +};
>> +
>> +&u2phy3 {
>> +	status = "okay";
>> +};
>> +
>> +&u2phy3_host {
>> +	status = "okay";
>> +};
>> +
>> +&usb_host0_ehci {
>> +	status = "okay";
>> +};
>> +
>> +&usb_host0_ohci {
>> +	status = "okay";
>> +};
>> +
>> +&usb_host1_ehci {
>> +	status = "okay";
>> +};
>> +
>> +&usb_host1_ohci {
>> +	status = "okay";
>> +};
>>
>> +&wdt {
>> +	status = "okay";
>> +};
>> -- 
>> 2.41.0
>>

Regards,
Efe

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-17 13:33     ` Muhammed Efe Cetin
@ 2023-08-17 13:57       ` Ondřej Jirman
  2023-08-17 14:57         ` Muhammed Efe Cetin
  0 siblings, 1 reply; 16+ messages in thread
From: Ondřej Jirman @ 2023-08-17 13:57 UTC (permalink / raw)
  To: Muhammed Efe Cetin
  Cc: conor+dt, devicetree, heiko, krzysztof.kozlowski+dt,
	linux-arm-kernel, linux-kernel, linux-rockchip, robh+dt,
	sebastian.reichel

Hi Muhammed,

On Thu, Aug 17, 2023 at 04:33:55PM +0300, Muhammed Efe Cetin wrote:
> 
> Hi, Ondřej
> 
> Thanks for reviews, i'll fix them soon.
> 
> On 15.08.2023 19:39, Ondřej Jirman wrote:
> > Hello Muhammed,
> > 
> >> [...]
> >> +
> >> +	adc-keys {
> >> +		compatible = "adc-keys";
> >> +		io-channels = <&saradc 1>;
> >> +		io-channel-names = "buttons";
> >> +		keyup-threshold-microvolt = <1800000>;
> >> +		poll-interval = <100>;
> >> +
> >> +		vol-up-key {
> >> +			label = "volume up";
> >> +			linux,code = <KEY_VOLUMEUP>;
> >> +			press-threshold-microvolt = <17000>;
> >> +		};
> >> +
> >> +		vol-down-key {
> >> +			label = "volume down";
> >> +			linux,code = <KEY_VOLUMEDOWN>;
> >> +			press-threshold-microvolt = <417000>;
> >> +		};
> >> +
> >> +		menu-key {
> >> +			label = "menu";
> >> +			linux,code = <KEY_MENU>;
> >> +			press-threshold-microvolt = <890000>;
> >> +		};
> >> +
> >> +		back-key {
> >> +			label = "back";
> >> +			linux,code = <KEY_BACK>;
> >> +			press-threshold-microvolt = <1235000>;
> > 
> > None of these 4 buttons are in the scehmatic. There's one recovery button hooked
> > to saradc 0, instead.
> 
> Will remove these too. I won't add recovery button because of it's mostly used for rockusb.

Recovery button is useful from userspace, too. Please keep it. You already have
the DT nodes, please just reduce them to one node for the recovery key.

> >> [...]
> > 
> >> +	vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "vcc_1v1_nldo_s3";
> >> +		regulator-always-on;
> >> +		regulator-boot-on;
> >> +		regulator-min-microvolt = <1100000>;
> >> +		regulator-max-microvolt = <1100000>;
> >> +		vin-supply = <&vcc5v0_sys>;
> >> +	};
> > 
> > There's no such regulator on the board.
> 
> It's connected to PMIC https://i.imgur.com/sVJdC5K.png

It's not a separate fixed regulator. It's a PMIC output from buck6 https://megous.com/dl/tmp/8630fa17407c75b9.png

So this is VDD2_DDR_S3. If you want to keep the alias, just add extra alias to
dcdc-reg6 like this:

		...
	vcc_1v1_nldo_s3: vdd2_ddr_s3: dcdc-reg6 {
		...

> > 
> >> +	vbus5v0_typec: vbus5v0-typec-regulator {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "vbus5v0_typec";
> >> +		regulator-min-microvolt = <5000000>;
> >> +		regulator-max-microvolt = <5000000>;
> >> +		enable-active-high;
> >> +		gpio = <&gpio3 RK_PC0 GPIO_ACTIVE_HIGH>;
> >> +		vin-supply = <&vcc5v0_usb>;
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&typec5v_pwren>;
> >> +	};
> > 
> > No such power rail on the board. This should be VBUS_TYPEC power rail.
> > Please name regulators/power rails as they are named in the schematic.
> > Otherwise it's impossible to cross-check DT against schematic.
> 
> Perhaps i can add it as a comment line. Otherwise the current name seems OK to
> me but still open to suggestions.

Please just use the name that's in the schematic, even if it's subotpimal. Your
comment will not be seen in debugfs/sysfs and other places, where it's also
useful for debugging.

> > 
> > No such regulator on the board. You might have meant VCC3V3_PCIE30,
> > or not?
> 
> Yes. However their naming in schematics is pretty nonsense. The board doesn't
> have PCIe3, i think current naming is more appropriate.

Same as above. It's more important to be able to match DT/runtime debug outputs
and schematic than having a subjectively more meaningful name which is not used
anywhere in the documentation.

> >> +	sata {
> >> +		sata_reset:sata-reset{
> >> +			rockchip,pins = <3 RK_PD1 RK_FUNC_GPIO &pcfg_pull_up>;
> > 
> > Not what you think: https://megous.com/dl/tmp/f135580b68cb5dfe.png
> > 
> >> +		};
> >> +	};
> >> +};
> >> +
> >> +&pwm2 {
> >> +	pinctrl-names = "active";
> >> +	pinctrl-0 = <&pwm2m0_pins>;
> >> +	status = "okay";
> >> +};
> >> +
> >> +&pwm6 {
> >> +	pinctrl-names = "active";
> >> +	pinctrl-0 = <&pwm6m0_pins>;
> >> +	status = "okay";
> >> +};
> >> +
> >> +&saradc {
> >> +	vref-supply = <&avcc_1v8_s0>;
> >> +	status = "okay";
> >> +};
> >> +
> >> +&sata0 {
> >> +	pinctrl-names = "default";
> >> +	pinctrl-0 = <&sata_reset>;
> >> +	status = "disabled";
> >> +};
> > 
> > Where do you see a SATA port? http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/details/Orange-Pi-5.html
> > 
> > Or a reset signal in the schematic?
> 
> It's needed to use sata node once you want to use mSata SSD. I don't know if
> it works without sata_reset pinctrl. Don't have mSata SSD to try.

Ok, that makes sense. It would be better to add this once you can test it, though.
And if there's no way to switch between SATA and PCIe at runtime, it's probably
just another thing for an overlay which would enable/disable/add all the appropriate
DT nodes.

thank you,
	o.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-17 13:57       ` Ondřej Jirman
@ 2023-08-17 14:57         ` Muhammed Efe Cetin
  2023-08-17 15:30           ` Ondřej Jirman
  0 siblings, 1 reply; 16+ messages in thread
From: Muhammed Efe Cetin @ 2023-08-17 14:57 UTC (permalink / raw)
  To: megi
  Cc: conor+dt, devicetree, efectn, heiko, krzysztof.kozlowski+dt,
	linux-arm-kernel, linux-kernel, linux-rockchip, robh+dt,
	sebastian.reichel

Hi, Ondřej

On 17.08.2023 16:57, Ondřej Jirman wrote:
> Hi Muhammed,
> 
> On Thu, Aug 17, 2023 at 04:33:55PM +0300, Muhammed Efe Cetin wrote:
>>
>> Hi, Ondřej
>>
>> Thanks for reviews, i'll fix them soon.
>>
>> On 15.08.2023 19:39, Ondřej Jirman wrote:
>>> Hello Muhammed,
>>>
>>>> [...]
>>>> +
>>>> +	adc-keys {
>>>> +		compatible = "adc-keys";
>>>> +		io-channels = <&saradc 1>;
>>>> +		io-channel-names = "buttons";
>>>> +		keyup-threshold-microvolt = <1800000>;
>>>> +		poll-interval = <100>;
>>>> +
>>>> +		vol-up-key {
>>>> +			label = "volume up";
>>>> +			linux,code = <KEY_VOLUMEUP>;
>>>> +			press-threshold-microvolt = <17000>;
>>>> +		};
>>>> +
>>>> +		vol-down-key {
>>>> +			label = "volume down";
>>>> +			linux,code = <KEY_VOLUMEDOWN>;
>>>> +			press-threshold-microvolt = <417000>;
>>>> +		};
>>>> +
>>>> +		menu-key {
>>>> +			label = "menu";
>>>> +			linux,code = <KEY_MENU>;
>>>> +			press-threshold-microvolt = <890000>;
>>>> +		};
>>>> +
>>>> +		back-key {
>>>> +			label = "back";
>>>> +			linux,code = <KEY_BACK>;
>>>> +			press-threshold-microvolt = <1235000>;
>>>
>>> None of these 4 buttons are in the scehmatic. There's one recovery button hooked
>>> to saradc 0, instead.
>>
>> Will remove these too. I won't add recovery button because of it's mostly used for rockusb.
> 
> Recovery button is useful from userspace, too. Please keep it. You already have
> the DT nodes, please just reduce them to one node for the recovery key.

Ok i will add it as KEY_VENDOR button.

> 
>>>> [...]
>>>
>>>> +	vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
>>>> +		compatible = "regulator-fixed";
>>>> +		regulator-name = "vcc_1v1_nldo_s3";
>>>> +		regulator-always-on;
>>>> +		regulator-boot-on;
>>>> +		regulator-min-microvolt = <1100000>;
>>>> +		regulator-max-microvolt = <1100000>;
>>>> +		vin-supply = <&vcc5v0_sys>;
>>>> +	};
>>>
>>> There's no such regulator on the board.
>>
>> It's connected to PMIC https://i.imgur.com/sVJdC5K.png
> 
> It's not a separate fixed regulator. It's a PMIC output from buck6 https://megous.com/dl/tmp/8630fa17407c75b9.png
> 

I think it should be fixed regulator. It's used as vcc13-supply and vcc14-supply regulator on PMIC and it's same as other rk3588 boards.

> So this is VDD2_DDR_S3. If you want to keep the alias, just add extra alias to
> dcdc-reg6 like this:
> 
> 		...
> 	vcc_1v1_nldo_s3: vdd2_ddr_s3: dcdc-reg6 {
> 		...
> 
>>>
>>>> +	vbus5v0_typec: vbus5v0-typec-regulator {
>>>> +		compatible = "regulator-fixed";
>>>> +		regulator-name = "vbus5v0_typec";
>>>> +		regulator-min-microvolt = <5000000>;
>>>> +		regulator-max-microvolt = <5000000>;
>>>> +		enable-active-high;
>>>> +		gpio = <&gpio3 RK_PC0 GPIO_ACTIVE_HIGH>;
>>>> +		vin-supply = <&vcc5v0_usb>;
>>>> +		pinctrl-names = "default";
>>>> +		pinctrl-0 = <&typec5v_pwren>;
>>>> +	};
>>>
>>> No such power rail on the board. This should be VBUS_TYPEC power rail.
>>> Please name regulators/power rails as they are named in the schematic.
>>> Otherwise it's impossible to cross-check DT against schematic.
>>
>> Perhaps i can add it as a comment line. Otherwise the current name seems OK to
>> me but still open to suggestions.
> 
> Please just use the name that's in the schematic, even if it's subotpimal. Your
> comment will not be seen in debugfs/sysfs and other places, where it's also
> useful for debugging.

Ok i will make this one "vbus_typec"

> 
>>>
>>> No such regulator on the board. You might have meant VCC3V3_PCIE30,
>>> or not?
>>
>> Yes. However their naming in schematics is pretty nonsense. The board doesn't
>> have PCIe3, i think current naming is more appropriate.
> 
> Same as above. It's more important to be able to match DT/runtime debug outputs
> and schematic than having a subjectively more meaningful name which is not used
> anywhere in the documentation.

I agree with you but VCC3V3_PCIE30 is really confusing regulator name. I guess Xunlong made a mistake about it. I think we can name it as "vcc3v3_pcie20"

> 
>>>> +	sata {
>>>> +		sata_reset:sata-reset{
>>>> +			rockchip,pins = <3 RK_PD1 RK_FUNC_GPIO &pcfg_pull_up>;
>>>
>>> Not what you think: https://megous.com/dl/tmp/f135580b68cb5dfe.png
>>>
>>>> +		};
>>>> +	};
>>>> +};
>>>> +
>>>> +&pwm2 {
>>>> +	pinctrl-names = "active";
>>>> +	pinctrl-0 = <&pwm2m0_pins>;
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>> +&pwm6 {
>>>> +	pinctrl-names = "active";
>>>> +	pinctrl-0 = <&pwm6m0_pins>;
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>> +&saradc {
>>>> +	vref-supply = <&avcc_1v8_s0>;
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>> +&sata0 {
>>>> +	pinctrl-names = "default";
>>>> +	pinctrl-0 = <&sata_reset>;
>>>> +	status = "disabled";
>>>> +};
>>>
>>> Where do you see a SATA port? http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/details/Orange-Pi-5.html
>>>
>>> Or a reset signal in the schematic?
>>
>> It's needed to use sata node once you want to use mSata SSD. I don't know if
>> it works without sata_reset pinctrl. Don't have mSata SSD to try.
> 
> Ok, that makes sense. It would be better to add this once you can test it, though.
> And if there's no way to switch between SATA and PCIe at runtime, it's probably
> just another thing for an overlay which would enable/disable/add all the appropriate
> DT nodes.

Yeah you are right.

> 
> thank you,
> 	o.

Regards,
Efe

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-17 14:57         ` Muhammed Efe Cetin
@ 2023-08-17 15:30           ` Ondřej Jirman
  2023-08-17 15:32             ` Ondřej Jirman
  0 siblings, 1 reply; 16+ messages in thread
From: Ondřej Jirman @ 2023-08-17 15:30 UTC (permalink / raw)
  To: Muhammed Efe Cetin
  Cc: conor+dt, devicetree, heiko, krzysztof.kozlowski+dt,
	linux-arm-kernel, linux-kernel, linux-rockchip, robh+dt,
	sebastian.reichel

On Thu, Aug 17, 2023 at 05:57:55PM +0300, Muhammed Efe Cetin wrote:
> 
> Hi, Ondřej
> 
> On 17.08.2023 16:57, Ondřej Jirman wrote:
> > Hi Muhammed,
> > 
> >>>> [...]
> >>>
> >>>> +	vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
> >>>> +		compatible = "regulator-fixed";
> >>>> +		regulator-name = "vcc_1v1_nldo_s3";
> >>>> +		regulator-always-on;
> >>>> +		regulator-boot-on;
> >>>> +		regulator-min-microvolt = <1100000>;
> >>>> +		regulator-max-microvolt = <1100000>;
> >>>> +		vin-supply = <&vcc5v0_sys>;
> >>>> +	};
> >>>
> >>> There's no such regulator on the board.
> >>
> >> It's connected to PMIC https://i.imgur.com/sVJdC5K.png
> > 
> > It's not a separate fixed regulator. It's a PMIC output from buck6 https://megous.com/dl/tmp/8630fa17407c75b9.png
> > 
> 
> I think it should be fixed regulator. It's used as vcc13-supply and
> vcc14-supply regulator on PMIC and it's same as other rk3588 boards.

Yes, BUCK6 output is input to some LDOs. If you make this a regulator-fixed,
BUCK6 will not get enabled when those LDOs are enabled, and the LDOs will not
work because they'll lack input power.

Your regulator-fixed does nothing to enable BUCK6 which is where vcc_1v1_nldo_s3
power rail is connected.

It only works for you now, because dcdc-reg6 is marked as regulator-always-on,
so it's already enabled when you need those dependent LDOs.

regards,
	o.

> > So this is VDD2_DDR_S3. If you want to keep the alias, just add extra alias to
> > dcdc-reg6 like this:
> > 
> > 		...
> > 	vcc_1v1_nldo_s3: vdd2_ddr_s3: dcdc-reg6 {
> > 		...
> > 
> >>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-17 15:30           ` Ondřej Jirman
@ 2023-08-17 15:32             ` Ondřej Jirman
  2023-08-17 16:28               ` Muhammed Efe Cetin
  0 siblings, 1 reply; 16+ messages in thread
From: Ondřej Jirman @ 2023-08-17 15:32 UTC (permalink / raw)
  To: Muhammed Efe Cetin, conor+dt, devicetree, heiko,
	krzysztof.kozlowski+dt, linux-arm-kernel, linux-kernel,
	linux-rockchip, robh+dt, sebastian.reichel

On Thu, Aug 17, 2023 at 05:30:04PM +0200, megi xff wrote:
> 
> On Thu, Aug 17, 2023 at 05:57:55PM +0300, Muhammed Efe Cetin wrote:
> > 
> > Hi, Ondřej
> > 
> > On 17.08.2023 16:57, Ondřej Jirman wrote:
> > > Hi Muhammed,
> > > 
> > >>>> [...]
> > >>>
> > >>>> +	vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
> > >>>> +		compatible = "regulator-fixed";
> > >>>> +		regulator-name = "vcc_1v1_nldo_s3";
> > >>>> +		regulator-always-on;
> > >>>> +		regulator-boot-on;
> > >>>> +		regulator-min-microvolt = <1100000>;
> > >>>> +		regulator-max-microvolt = <1100000>;
> > >>>> +		vin-supply = <&vcc5v0_sys>;
> > >>>> +	};
> > >>>
> > >>> There's no such regulator on the board.
> > >>
> > >> It's connected to PMIC https://i.imgur.com/sVJdC5K.png
> > > 
> > > It's not a separate fixed regulator. It's a PMIC output from buck6 https://megous.com/dl/tmp/8630fa17407c75b9.png
> > > 
> > 
> > I think it should be fixed regulator. It's used as vcc13-supply and
> > vcc14-supply regulator on PMIC and it's same as other rk3588 boards.
> 
> Yes, BUCK6 output is input to some LDOs. If you make this a regulator-fixed,
> BUCK6 will not get enabled when those LDOs are enabled, and the LDOs will not
> work because they'll lack input power.
> 
> Your regulator-fixed does nothing to enable BUCK6 which is where vcc_1v1_nldo_s3
> power rail is connected.
> 
> It only works for you now, because dcdc-reg6 is marked as regulator-always-on,
> so it's already enabled when you need those dependent LDOs.

And if other boards have this same HW setup and user separate DT node with
regulator-fixed for this, they're broken, too.

regards,
	o.

> regards,
> 	o.
> 
> > > So this is VDD2_DDR_S3. If you want to keep the alias, just add extra alias to
> > > dcdc-reg6 like this:
> > > 
> > > 		...
> > > 	vcc_1v1_nldo_s3: vdd2_ddr_s3: dcdc-reg6 {
> > > 		...
> > > 
> > >>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-17 15:32             ` Ondřej Jirman
@ 2023-08-17 16:28               ` Muhammed Efe Cetin
  0 siblings, 0 replies; 16+ messages in thread
From: Muhammed Efe Cetin @ 2023-08-17 16:28 UTC (permalink / raw)
  To: megi
  Cc: conor+dt, devicetree, efectn, heiko, krzysztof.kozlowski+dt,
	linux-arm-kernel, linux-kernel, linux-rockchip, robh+dt,
	sebastian.reichel

Hi, Ondřej

On 17.08.2023 18:32, Ondřej Jirman wrote:
> On Thu, Aug 17, 2023 at 05:30:04PM +0200, megi xff wrote:
>>
>> On Thu, Aug 17, 2023 at 05:57:55PM +0300, Muhammed Efe Cetin wrote:
>>>
>>> Hi, Ondřej
>>>
>>> On 17.08.2023 16:57, Ondřej Jirman wrote:
>>>> Hi Muhammed,
>>>>
>>>>>>> [...]
>>>>>>
>>>>>>> +	vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
>>>>>>> +		compatible = "regulator-fixed";
>>>>>>> +		regulator-name = "vcc_1v1_nldo_s3";
>>>>>>> +		regulator-always-on;
>>>>>>> +		regulator-boot-on;
>>>>>>> +		regulator-min-microvolt = <1100000>;
>>>>>>> +		regulator-max-microvolt = <1100000>;
>>>>>>> +		vin-supply = <&vcc5v0_sys>;
>>>>>>> +	};
>>>>>>
>>>>>> There's no such regulator on the board.
>>>>>
>>>>> It's connected to PMIC https://i.imgur.com/sVJdC5K.png
>>>>
>>>> It's not a separate fixed regulator. It's a PMIC output from buck6 https://megous.com/dl/tmp/8630fa17407c75b9.png
>>>>
>>>
>>> I think it should be fixed regulator. It's used as vcc13-supply and
>>> vcc14-supply regulator on PMIC and it's same as other rk3588 boards.
>>
>> Yes, BUCK6 output is input to some LDOs. If you make this a regulator-fixed,
>> BUCK6 will not get enabled when those LDOs are enabled, and the LDOs will not
>> work because they'll lack input power.
>>
>> Your regulator-fixed does nothing to enable BUCK6 which is where vcc_1v1_nldo_s3
>> power rail is connected.
>>
>> It only works for you now, because dcdc-reg6 is marked as regulator-always-on,
>> so it's already enabled when you need those dependent LDOs.
> 
> And if other boards have this same HW setup and user separate DT node with
> regulator-fixed for this, they're broken, too.

As i've seen on upstream and Rockchip SDK; boards have dual RK806, have vcc_1v1_nldo_s3 node inside of pmic (rk3588-evb1-v10) and boards have single RK806, have separated vcc_1v1_nldo_s3 node. I don't know why they preferred this way.

> 
> regards,
> 	o.
> 
>> regards,
>> 	o.
>>
>>>> So this is VDD2_DDR_S3. If you want to keep the alias, just add extra alias to
>>>> dcdc-reg6 like this:
>>>>
>>>> 		...
>>>> 	vcc_1v1_nldo_s3: vdd2_ddr_s3: dcdc-reg6 {
>>>> 		...
>>>>
>>>>>>

Regards,
Efe

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-15 13:45   ` Krzysztof Kozlowski
@ 2023-08-17 17:23     ` Muhammed Efe Cetin
  0 siblings, 0 replies; 16+ messages in thread
From: Muhammed Efe Cetin @ 2023-08-17 17:23 UTC (permalink / raw)
  To: krzysztof.kozlowski
  Cc: conor+dt, devicetree, efectn, heiko, krzysztof.kozlowski+dt,
	linux-arm-kernel, linux-kernel, linux-rockchip, robh+dt,
	sebastian.reichel

Hi Krzysztof,

Thanks for review, i'll fix them and send v2 soon.

On 15.08.2023 16:45, Krzysztof Kozlowski wrote:
> On 15/08/2023 14:59, Muhammed Efe Cetin wrote:
>> Add initial support for OPi5 that includes support for USB2, PCIe2, Sata,
>> Sdmmc, SPI Flash, PMIC.
>>
>> Signed-off-by: Muhammed Efe Cetin <efectn@6tel.net>
>> ---
>>   .../boot/dts/rockchip/rk3588s-orangepi-5.dts  | 873 ++++++++++++++++++
> 
> Without Makefile this won't be build, so this was not ever tested.
> 
>>   1 file changed, 873 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
>> new file mode 100644
>> index 000000000000..85071084a207
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
>> @@ -0,0 +1,873 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +
>> +/dts-v1/;
>> +
>> +#include <dt-bindings/pinctrl/rockchip.h>
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +#include "rk3588s.dtsi"
>> +
>> +/ {
>> +	model = "Xunlong Orange Pi 5";
>> +	compatible = "xunlong,orangepi-5", "rockchip,rk3588s";
>> +
>> +	aliases {
>> +		mmc0 = &sdmmc;
>> +		serial2 = &uart2;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial2:1500000n8";
>> +	};
>> +
>> +	leds {
>> +		compatible = "gpio-leds";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 =<&leds_gpio>;
>> +
>> +		led@1 {
> 
> Unit address is not correct here, it is not a bus. This should be
> reported as warning, so you did not check for warnings.
> 
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check W=1` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).
> 
>> +			gpios = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;
>> +			label = "status_led";
>> +			linux,default-trigger = "heartbeat";
>> +			linux,default-trigger-delay-ms = <0>;
>> +		};
>> +	};
>> +
>> +	adc-keys {
>> +		compatible = "adc-keys";
>> +		io-channels = <&saradc 1>;
>> +		io-channel-names = "buttons";
>> +		keyup-threshold-microvolt = <1800000>;
>> +		poll-interval = <100>;
>> +
>> +		vol-up-key {
>> +			label = "volume up";
>> +			linux,code = <KEY_VOLUMEUP>;
>> +			press-threshold-microvolt = <17000>;
>> +		};
>> +
>> +		vol-down-key {
>> +			label = "volume down";
>> +			linux,code = <KEY_VOLUMEDOWN>;
>> +			press-threshold-microvolt = <417000>;
>> +		};
>> +
>> +		menu-key {
>> +			label = "menu";
>> +			linux,code = <KEY_MENU>;
>> +			press-threshold-microvolt = <890000>;
>> +		};
>> +
>> +		back-key {
>> +			label = "back";
>> +			linux,code = <KEY_BACK>;
>> +			press-threshold-microvolt = <1235000>;
>> +		};
>> +	};
>> +
>> +	backlight: backlight {
>> +		compatible = "pwm-backlight";
>> +		brightness-levels = <  0  20  20  21  21  22  22  23
>> +					  23  24  24  25  25  26  26  27
>> +					  27  28  28  29  29  30  30  31
>> +					  31  32  32  33  33  34  34  35
>> +					  35  36  36  37  37  38  38  39
>> +					  40  41  42  43  44  45  46  47
>> +					  48  49  50  51  52  53  54  55
>> +					  56  57  58  59  60  61  62  63
>> +					  64  65  66  67  68  69  70  71
>> +					  72  73  74  75  76  77  78  79
>> +					  80  81  82  83  84  85  86  87
>> +					  88  89  90  91  92  93  94  95
>> +					  96  97  98  99 100 101 102 103
>> +					  104 105 106 107 108 109 110 111
>> +					  112 113 114 115 116 117 118 119
>> +					  120 121 122 123 124 125 126 127
>> +					  128 129 130 131 132 133 134 135
>> +					  136 137 138 139 140 141 142 143
>> +					  144 145 146 147 148 149 150 151
>> +					  152 153 154 155 156 157 158 159
>> +					  160 161 162 163 164 165 166 167
>> +					  168 169 170 171 172 173 174 175
>> +					  176 177 178 179 180 181 182 183
>> +					  184 185 186 187 188 189 190 191
>> +					  192 193 194 195 196 197 198 199
>> +					  200 201 202 203 204 205 206 207
>> +					  208 209 210 211 212 213 214 215
>> +					  216 217 218 219 220 221 222 223
>> +					  224 225 226 227 228 229 230 231
>> +					  232 233 234 235 236 237 238 239
>> +					  240 241 242 243 244 245 246 247
>> +					  248 249 250 251 252 253 254 255>;
>> +		default-brightness-level = <200>;
>> +		pwms = <&pwm2 0 25000 0>;
>> +	};
>> +
>> +	backlight_1: backlight_1 {
> 
> No underscores in node names, use -
> 
>> +		compatible = "pwm-backlight";
>> +		brightness-levels = <  0  20  20  21  21  22  22  23
>> +					  23  24  24  25  25  26  26  27
>> +					  27  28  28  29  29  30  30  31
> 
> ...
> 
>> +
>> +&combphy0_ps {
>> +	status = "okay";
>> +};
>> +
>> +&combphy2_psu {
>> +	status = "okay";
>> +};
>> +
>> +&cpu_b0 {
>> +	cpu-supply = <&vdd_cpu_big0_s0>;
>> +	mem-supply = <&vdd_cpu_big0_mem_s0>;
>> +};
>> +
>> +&cpu_b1 {
>> +	cpu-supply = <&vdd_cpu_big0_s0>;
>> +	mem-supply = <&vdd_cpu_big0_mem_s0>;
>> +};
>> +
>> +&cpu_b2 {
>> +	cpu-supply = <&vdd_cpu_big1_s0>;
>> +	mem-supply = <&vdd_cpu_big1_mem_s0>;
>> +};
>> +
>> +&cpu_b3 {
>> +	cpu-supply = <&vdd_cpu_big1_s0>;
>> +	mem-supply = <&vdd_cpu_big1_mem_s0>;
>> +};
>> +
>> +&cpu_l0 {
>> +	cpu-supply = <&vdd_cpu_lit_s0>;
>> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
>> +};
>> +
>> +&cpu_l1 {
>> +	cpu-supply = <&vdd_cpu_lit_s0>;
>> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
>> +};
>> +
>> +&cpu_l2 {
>> +	cpu-supply = <&vdd_cpu_lit_s0>;
>> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
>> +};
>> +
>> +&cpu_l3 {
>> +	cpu-supply = <&vdd_cpu_lit_s0>;
>> +	mem-supply = <&vdd_cpu_lit_mem_s0>;
>> +};
>> +
>> +&gmac1 {
>> +	clock_in_out = "output";
>> +	phy-handle = <&rgmii_phy1>;
>> +	phy-mode = "rgmii-rxid";
>> +	pinctrl-0 = <&gmac1_miim
>> +					&gmac1_tx_bus2
>> +					&gmac1_rx_bus2
>> +					&gmac1_rgmii_clk
>> +					&gmac1_rgmii_bus>;
> 
> Messed alignment.
> 
>> +	pinctrl-names = "default";
>> +	snps,reset-gpio = <&gpio3 RK_PB2 GPIO_ACTIVE_LOW>;
>> +	snps,reset-active-low;
>> +	snps,reset-delays-us = <0 20000 100000>;
>> +	tx_delay = <0x42>;
>> +	status = "okay";
>> +};
>> +
> 
> ...
> 
>> +
>> +&sfc {
>> +	pinctrl-0 = <&fspim0_pins>;
>> +	pinctrl-names = "default";
>> +	max-freq = <100000000>;
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	status = "okay";
>> +
>> +	spi_flash: spi-flash@0 {
>> +		compatible = "jedec,spi-nor";
>> +		reg = <0x0>;
>> +		spi-max-frequency = <100000000>;
>> +		spi-tx-bus-width = <1>;
>> +		spi-rx-bus-width = <4>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		status = "okay";
> 
> okay is by default, was it disabled anywhere?
> 
>> +
>> +		partitions {
>> +			compatible = "fixed-partitions";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +
>> +			loader@0 {
>> +				label = "loader";
>> +				reg = <0x0 0x1000000>;
>> +			};
>> +		};
>> +	};
>> +};
>> +
> 
> 
> Best regards,
> Krzysztof
> 

Regards,
Efe


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] arm64: dts: rockchip: Add sfc node to rk3588s
  2023-08-15 12:59 ` [PATCH 2/3] arm64: dts: rockchip: Add sfc node to rk3588s Muhammed Efe Cetin
@ 2023-08-17 21:04   ` Jonas Karlman
  0 siblings, 0 replies; 16+ messages in thread
From: Jonas Karlman @ 2023-08-17 21:04 UTC (permalink / raw)
  To: Muhammed Efe Cetin, linux-rockchip
  Cc: devicetree, linux-arm-kernel, linux-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, heiko, sebastian.reichel

On 2023-08-15 14:59, Muhammed Efe Cetin wrote:
> Add sfc node to rk3588s.dtsi from downstream kernel.
> 
> Signed-off-by: Muhammed Efe Cetin <efectn@6tel.net>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> index 5544f66c6ff4..a38a0158fce0 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -1424,6 +1424,19 @@ sata-port@0 {
>  		};
>  	};
>  
> +	sfc: spi@fe2b0000 {
> +		compatible = "rockchip,sfc";
> +		reg = <0x0 0xfe2b0000 0x0 0x4000>;
> +		interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>;

gic use 4 interrupt-cells, should be:

  interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH 0>;

> +		clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
> +		clock-names = "clk_sfc", "hclk_sfc";
> +		assigned-clocks = <&cru SCLK_SFC>;
> +		assigned-clock-rates = <100000000>;

You should not need to assign clock rate here, do it in your board dts
if you really must.

Regards,
Jonas

> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		status = "disabled";
> +	};
> +
>  	sdmmc: mmc@fe2c0000 {
>  		compatible = "rockchip,rk3588-dw-mshc", "rockchip,rk3288-dw-mshc";
>  		reg = <0x0 0xfe2c0000 0x0 0x4000>;


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5
  2023-08-15 12:59 ` [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5 Muhammed Efe Cetin
  2023-08-15 13:45   ` Krzysztof Kozlowski
  2023-08-15 16:39   ` Ondřej Jirman
@ 2023-08-17 21:05   ` Jonas Karlman
  2 siblings, 0 replies; 16+ messages in thread
From: Jonas Karlman @ 2023-08-17 21:05 UTC (permalink / raw)
  To: Muhammed Efe Cetin, linux-rockchip
  Cc: devicetree, linux-arm-kernel, linux-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, heiko, sebastian.reichel

On 2023-08-15 14:59, Muhammed Efe Cetin wrote:
> Add initial support for OPi5 that includes support for USB2, PCIe2, Sata,
> Sdmmc, SPI Flash, PMIC.
> 
> Signed-off-by: Muhammed Efe Cetin <efectn@6tel.net>
> ---
>  .../boot/dts/rockchip/rk3588s-orangepi-5.dts  | 873 ++++++++++++++++++
>  1 file changed, 873 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> new file mode 100644
> index 000000000000..85071084a207
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dts
> @@ -0,0 +1,873 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +

[...]

> +
> +&sfc {
> +	pinctrl-0 = <&fspim0_pins>;
> +	pinctrl-names = "default";
> +	max-freq = <100000000>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +
> +	spi_flash: spi-flash@0 {

Node should be named flash@0 to help SPI flash boot with U-Boot.

Regards,
Jonas

> +		compatible = "jedec,spi-nor";
> +		reg = <0x0>;
> +		spi-max-frequency = <100000000>;
> +		spi-tx-bus-width = <1>;
> +		spi-rx-bus-width = <4>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		status = "okay";
> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			loader@0 {
> +				label = "loader";
> +				reg = <0x0 0x1000000>;
> +			};
> +		};
> +	};
> +};
> +

[...]


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-08-17 21:06 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-15 12:58 [PATCH 0/3] Add Support for Orange Pi 5 Muhammed Efe Cetin
2023-08-15 12:58 ` [PATCH 1/3] dt-bindings: arm: rockchip: Add Orange Pi 5 board Muhammed Efe Cetin
2023-08-15 13:46   ` Krzysztof Kozlowski
2023-08-15 12:59 ` [PATCH 2/3] arm64: dts: rockchip: Add sfc node to rk3588s Muhammed Efe Cetin
2023-08-17 21:04   ` Jonas Karlman
2023-08-15 12:59 ` [PATCH 3/3] arm64: dts: rockchip: Add Orange Pi 5 Muhammed Efe Cetin
2023-08-15 13:45   ` Krzysztof Kozlowski
2023-08-17 17:23     ` Muhammed Efe Cetin
2023-08-15 16:39   ` Ondřej Jirman
2023-08-17 13:33     ` Muhammed Efe Cetin
2023-08-17 13:57       ` Ondřej Jirman
2023-08-17 14:57         ` Muhammed Efe Cetin
2023-08-17 15:30           ` Ondřej Jirman
2023-08-17 15:32             ` Ondřej Jirman
2023-08-17 16:28               ` Muhammed Efe Cetin
2023-08-17 21:05   ` Jonas Karlman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).