diff mbox series

[3/3] arm64: dts: rockchip: Add Orange Pi 5

Message ID a1eca379d151c3f91f4cd4e1751ba389096c4f13.1692102057.git.efectn@6tel.net (mailing list archive)
State New, archived
Headers show
Series Add Support for Orange Pi 5 | expand

Commit Message

Muhammed Efe Cetin Aug. 15, 2023, 12:59 p.m. UTC
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

Comments

Krzysztof Kozlowski Aug. 15, 2023, 1:45 p.m. UTC | #1
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
Ondřej Jirman Aug. 15, 2023, 4:39 p.m. UTC | #2
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
>
Muhammed Efe Cetin Aug. 17, 2023, 1:33 p.m. UTC | #3
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
Ondřej Jirman Aug. 17, 2023, 1:57 p.m. UTC | #4
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.
Muhammed Efe Cetin Aug. 17, 2023, 2:57 p.m. UTC | #5
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
Ondřej Jirman Aug. 17, 2023, 3:30 p.m. UTC | #6
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 {
> > 		...
> > 
> >>>
Ondřej Jirman Aug. 17, 2023, 3:32 p.m. UTC | #7
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 {
> > > 		...
> > > 
> > >>>
Muhammed Efe Cetin Aug. 17, 2023, 4:28 p.m. UTC | #8
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
Muhammed Efe Cetin Aug. 17, 2023, 5:23 p.m. UTC | #9
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
Jonas Karlman Aug. 17, 2023, 9:05 p.m. UTC | #10
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>;
> +			};
> +		};
> +	};
> +};
> +

[...]
diff mbox series

Patch

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";
+};