| Message ID | 20200116225617.6318-1-manu@freebsd.org |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [1/2] dt-bindings: Add doc for Pine64 Pinebook Pro | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success |
Hi, overall looks good, but some small things below Am Donnerstag, 16. Januar 2020, 23:56:17 CET schrieb Emmanuel Vadot: > From: Peter Robinson <pbrobinson@gmail.com> > > Pinebook Pro is a RK3399 based laptop produced by Pine64. > > Add a basic DTS for it. > > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > Signed-off-by: Emmanuel Vadot <manu@freebsd.org> > --- > arch/arm64/boot/dts/rockchip/Makefile | 1 + > .../boot/dts/rockchip/rk3399-pinebook-pro.dts | 626 ++++++++++++++++++ > 2 files changed, 627 insertions(+) > create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile > index 48fb631d5451..9099fb7e2073 100644 > --- a/arch/arm64/boot/dts/rockchip/Makefile > +++ b/arch/arm64/boot/dts/rockchip/Makefile > @@ -28,6 +28,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopc-t4.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-m4.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-neo4.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-orangepi.dtb > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-pinebook-pro.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-puma-haikou.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-roc-pc.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-roc-pc-mezzanine.dtb > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts > new file mode 100644 > index 000000000000..d2e3d7b35cc6 > --- /dev/null > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts > @@ -0,0 +1,626 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (c) 2017 Fuzhou Rockchip Electronics Co., Ltd. You might want to clarify the copyright ... 2020 and Pine64-something? > + */ > + > +/dts-v1/; > +#include <dt-bindings/input/linux-event-codes.h> > +#include <dt-bindings/pwm/pwm.h> > +#include <dt-bindings/pinctrl/rockchip.h> > +#include "rk3399.dtsi" > +#include "rk3399-opp.dtsi" > + > +/ { > + model = "Pine64 Pinebook Pro"; > + compatible = "pine64,pinebook-pro", "rockchip,rk3399"; > + > + chosen { > + stdout-path = "serial2:115200n8"; > + }; > + > + aliases { > + spi0 = &spi1; why is this needed ... I'd think spi can just enumerate itself? > + }; > + > + backlight: backlight { > + compatible = "pwm-backlight"; > + enable-gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_HIGH>; > + pwms = <&pwm0 0 740740 0>; > + }; > + > + gpio-keys { > + compatible = "gpio-keys"; > + autorepeat; > + pinctrl-names = "default"; > + pinctrl-0 = <&pwrbtn>; > + > + power { > + debounce-interval = <100>; > + gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>; > + label = "GPIO Key Power"; > + linux,code = <KEY_POWER>; > + wakeup-source; > + }; > + }; > + > + leds { > + status = "okay"; new board-specific nodes don't need a status property > + compatible = "gpio-leds"; > + > + work-led { > + label = "work"; > + gpios = <&gpio0 RK_PB3 GPIO_ACTIVE_HIGH>; > + }; > + > + standby-led { > + label = "standby"; > + gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>; > + }; > + }; > + > + vcc1v8_s3: vcca1v8_s3: vcc1v8-s3 { > + compatible = "regulator-fixed"; > + regulator-name = "vcc1v8_s3"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + vin-supply = <&vcc_1v8>; > + }; > + > + dc_12v: dc-12v { > + compatible = "regulator-fixed"; > + regulator-name = "dc_12v"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <12000000>; > + regulator-max-microvolt = <12000000>; > + }; > + > + vcc3v3_sys: vcc3v3-sys { > + compatible = "regulator-fixed"; > + regulator-name = "vcc3v3_sys"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + vin-supply = <&vcc_sys>; > + }; > + > + vcc5v0_host: vcc5v0-host-regulator { > + compatible = "regulator-fixed"; > + gpio = <&gpio4 RK_PD2 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&host_vbus_drv>; > + regulator-name = "vcc5v0_host"; > + }; > + > + vcc5v0_usb3_host: vcc5v0-usb3-host-regulator { > + compatible = "regulator-fixed"; > + enable-active-high; > + gpio = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&host_usb3_drv>; > + regulator-name = "vcc5v0_usb3_host"; > + regulator-always-on; > + }; > + > + vcc3v3_s0: vcc3v3-s0-regulator { > + compatible = "regulator-fixed"; > + enable-active-high; > + gpio = <&gpio1 RK_PC6 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&lcdvcc_en>; > + regulator-name = "vcc3v3_s0"; > + regulator-always-on; > + }; > + > + vcc_sys: vcc-sys { > + compatible = "regulator-fixed"; > + regulator-name = "vcc_sys"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + vin-supply = <&dc_12v>; > + }; > + > + vdd_log: vdd-log { > + compatible = "pwm-regulator"; > + pwms = <&pwm2 0 25000 1>; > + pwm-supply = <&vcc_sys>; > + regulator-name = "vdd_log"; > + regulator-always-on; > + regulator-boot-on; > + regulator-init-microvolt = <950000>; There is no init-microvolt property in mainline, I'd think bootloader should already setup the pwm to a suitable setting? Not > + regulator-min-microvolt = <800000>; > + regulator-max-microvolt = <1400000>; > + }; > +}; > + > +&cpu_l0 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&cpu_l1 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&cpu_l2 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&cpu_l3 { > + cpu-supply = <&vdd_cpu_l>; > +}; what happened to cpu_b0 and cpu_b1 supplies? Should probably reference the cpu_b regulator below? > + > +&emmc_phy { > + status = "okay"; > +}; > + > +&i2c0 { > + clock-frequency = <400000>; > + i2c-scl-rising-time-ns = <168>; > + i2c-scl-falling-time-ns = <4>; > + status = "okay"; [...] > + vdd_cpu_b: regulator@40 { > + compatible = "silergy,syr827"; > + reg = <0x40>; > + fcs,suspend-voltage-selector = <1>; > + pinctrl-names = "default"; > + pinctrl-0 = <&vsel1_gpio>; > + vsel-gpios = <&gpio1 RK_PC1 GPIO_ACTIVE_HIGH>; This is not yet specified in the syr82x (fan5355) bindings and also unknown to the driver > + regulator-compatible = "fan53555-reg"; > + regulator-name = "vdd_cpu_b"; > + regulator-min-microvolt = <712500>; > + regulator-max-microvolt = <1500000>; > + regulator-ramp-delay = <1000>; > + regulator-always-on; > + regulator-boot-on; > + vin-supply = <&vcc_sys>; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vdd_gpu: regulator@41 { > + compatible = "silergy,syr828"; > + reg = <0x41>; > + fcs,suspend-voltage-selector = <1>; > + pinctrl-names = "default"; > + pinctrl-0 = <&vsel2_gpio>; > + vsel-gpios = <&gpio1 RK_PB6 GPIO_ACTIVE_HIGH>; same > + regulator-compatible = "fan53555-reg"; > + regulator-name = "vdd_gpu"; > + regulator-min-microvolt = <712500>; > + regulator-max-microvolt = <1500000>; > + regulator-ramp-delay = <1000>; > + regulator-always-on; > + regulator-boot-on; > + vin-supply = <&vcc_sys>; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > +}; [...] > +&saradc { > + status = "okay"; needs a vref-supply > +}; > + > +&sdmmc { > + bus-width = <4>; > + status = "okay"; > + max-frequency = <20000000>; vmmc / vqmmc supplies? Especially if one wants to achieve higher speeds on uhs cards. Heiko
On Sun, 19 Jan 2020 21:17:39 +0100 Heiko Stübner <heiko@sntech.de> wrote: > Hi, > > overall looks good, but some small things below > > Am Donnerstag, 16. Januar 2020, 23:56:17 CET schrieb Emmanuel Vadot: > > From: Peter Robinson <pbrobinson@gmail.com> > > > > Pinebook Pro is a RK3399 based laptop produced by Pine64. > > > > Add a basic DTS for it. > > > > Signed-off-by: Peter Robinson <pbrobinson@gmail.com> > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org> > > --- > > arch/arm64/boot/dts/rockchip/Makefile | 1 + > > .../boot/dts/rockchip/rk3399-pinebook-pro.dts | 626 ++++++++++++++++++ > > 2 files changed, 627 insertions(+) > > create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts > > > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile > > index 48fb631d5451..9099fb7e2073 100644 > > --- a/arch/arm64/boot/dts/rockchip/Makefile > > +++ b/arch/arm64/boot/dts/rockchip/Makefile > > @@ -28,6 +28,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopc-t4.dtb > > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-m4.dtb > > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-neo4.dtb > > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-orangepi.dtb > > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-pinebook-pro.dtb > > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-puma-haikou.dtb > > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-roc-pc.dtb > > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-roc-pc-mezzanine.dtb > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts > > new file mode 100644 > > index 000000000000..d2e3d7b35cc6 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts > > @@ -0,0 +1,626 @@ > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > +/* > > + * Copyright (c) 2017 Fuzhou Rockchip Electronics Co., Ltd. > > You might want to clarify the copyright ... 2020 and Pine64-something? This is a DTS that Peter Robinson first wrote, I've just kept the copyright. > > + */ > > + > > +/dts-v1/; > > +#include <dt-bindings/input/linux-event-codes.h> > > +#include <dt-bindings/pwm/pwm.h> > > +#include <dt-bindings/pinctrl/rockchip.h> > > +#include "rk3399.dtsi" > > +#include "rk3399-opp.dtsi" > > + > > +/ { > > + model = "Pine64 Pinebook Pro"; > > + compatible = "pine64,pinebook-pro", "rockchip,rk3399"; > > + > > + chosen { > > + stdout-path = "serial2:115200n8"; > > + }; > > + > > + aliases { > > + spi0 = &spi1; > > why is this needed ... I'd think spi can just enumerate itself? This was taken from u-boot and yes I think it's not needed for both Linux and FreeBSD. > > > + }; > > + > > + backlight: backlight { > > + compatible = "pwm-backlight"; > > + enable-gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_HIGH>; > > + pwms = <&pwm0 0 740740 0>; > > + }; > > + > > + gpio-keys { > > + compatible = "gpio-keys"; > > + autorepeat; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pwrbtn>; > > + > > + power { > > + debounce-interval = <100>; > > + gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>; > > + label = "GPIO Key Power"; > > + linux,code = <KEY_POWER>; > > + wakeup-source; > > + }; > > + }; > > + > > + leds { > > + status = "okay"; > > new board-specific nodes don't need a status property Ok. > > + compatible = "gpio-leds"; > > + > > + work-led { > > + label = "work"; > > + gpios = <&gpio0 RK_PB3 GPIO_ACTIVE_HIGH>; > > + }; > > + > > + standby-led { > > + label = "standby"; > > + gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>; > > + }; > > + }; > > + > > + vcc1v8_s3: vcca1v8_s3: vcc1v8-s3 { > > + compatible = "regulator-fixed"; > > + regulator-name = "vcc1v8_s3"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + vin-supply = <&vcc_1v8>; > > + }; > > + > > + dc_12v: dc-12v { > > + compatible = "regulator-fixed"; > > + regulator-name = "dc_12v"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <12000000>; > > + regulator-max-microvolt = <12000000>; > > + }; > > + > > + vcc3v3_sys: vcc3v3-sys { > > + compatible = "regulator-fixed"; > > + regulator-name = "vcc3v3_sys"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + vin-supply = <&vcc_sys>; > > + }; > > + > > + vcc5v0_host: vcc5v0-host-regulator { > > + compatible = "regulator-fixed"; > > + gpio = <&gpio4 RK_PD2 GPIO_ACTIVE_HIGH>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&host_vbus_drv>; > > + regulator-name = "vcc5v0_host"; > > + }; > > + > > + vcc5v0_usb3_host: vcc5v0-usb3-host-regulator { > > + compatible = "regulator-fixed"; > > + enable-active-high; > > + gpio = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&host_usb3_drv>; > > + regulator-name = "vcc5v0_usb3_host"; > > + regulator-always-on; > > + }; > > + > > + vcc3v3_s0: vcc3v3-s0-regulator { > > + compatible = "regulator-fixed"; > > + enable-active-high; > > + gpio = <&gpio1 RK_PC6 GPIO_ACTIVE_HIGH>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&lcdvcc_en>; > > + regulator-name = "vcc3v3_s0"; > > + regulator-always-on; > > + }; > > + > > + vcc_sys: vcc-sys { > > + compatible = "regulator-fixed"; > > + regulator-name = "vcc_sys"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <5000000>; > > + regulator-max-microvolt = <5000000>; > > + vin-supply = <&dc_12v>; > > + }; > > + > > + vdd_log: vdd-log { > > + compatible = "pwm-regulator"; > > + pwms = <&pwm2 0 25000 1>; > > + pwm-supply = <&vcc_sys>; > > + regulator-name = "vdd_log"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-init-microvolt = <950000>; > > There is no init-microvolt property in mainline, I'd think bootloader should > already setup the pwm to a suitable setting? > > Not I'll fix that. > > + regulator-min-microvolt = <800000>; > > + regulator-max-microvolt = <1400000>; > > + }; > > +}; > > + > > +&cpu_l0 { > > + cpu-supply = <&vdd_cpu_l>; > > +}; > > + > > +&cpu_l1 { > > + cpu-supply = <&vdd_cpu_l>; > > +}; > > + > > +&cpu_l2 { > > + cpu-supply = <&vdd_cpu_l>; > > +}; > > + > > +&cpu_l3 { > > + cpu-supply = <&vdd_cpu_l>; > > +}; > > what happened to cpu_b0 and cpu_b1 supplies? > Should probably reference the cpu_b regulator below? Yes, sorry. > > + > > +&emmc_phy { > > + status = "okay"; > > +}; > > + > > +&i2c0 { > > + clock-frequency = <400000>; > > + i2c-scl-rising-time-ns = <168>; > > + i2c-scl-falling-time-ns = <4>; > > + status = "okay"; > > [...] > > > + vdd_cpu_b: regulator@40 { > > + compatible = "silergy,syr827"; > > + reg = <0x40>; > > + fcs,suspend-voltage-selector = <1>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&vsel1_gpio>; > > + vsel-gpios = <&gpio1 RK_PC1 GPIO_ACTIVE_HIGH>; > > This is not yet specified in the syr82x (fan5355) bindings and > also unknown to the driver I'll have another look at the bindings, thanks. > > + regulator-compatible = "fan53555-reg"; > > + regulator-name = "vdd_cpu_b"; > > + regulator-min-microvolt = <712500>; > > + regulator-max-microvolt = <1500000>; > > + regulator-ramp-delay = <1000>; > > + regulator-always-on; > > + regulator-boot-on; > > + vin-supply = <&vcc_sys>; > > + > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + > > + vdd_gpu: regulator@41 { > > + compatible = "silergy,syr828"; > > + reg = <0x41>; > > + fcs,suspend-voltage-selector = <1>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&vsel2_gpio>; > > + vsel-gpios = <&gpio1 RK_PB6 GPIO_ACTIVE_HIGH>; > > same > > > + regulator-compatible = "fan53555-reg"; > > + regulator-name = "vdd_gpu"; > > + regulator-min-microvolt = <712500>; > > + regulator-max-microvolt = <1500000>; > > + regulator-ramp-delay = <1000>; > > + regulator-always-on; > > + regulator-boot-on; > > + vin-supply = <&vcc_sys>; > > + > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + > > +}; > > [...] > > > +&saradc { > > + status = "okay"; > > needs a vref-supply > > > +}; > > + > > +&sdmmc { > > + bus-width = <4>; > > + status = "okay"; > > + max-frequency = <20000000>; > > vmmc / vqmmc supplies? > Especially if one wants to achieve higher speeds on uhs cards. Yep, will do that. > Heiko > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Emmanuel, Resend from linux-rockchip@lists.infradead.org See below. > From: Peter Robinson <pbrobinson at gmail.com> > > Pinebook Pro is a RK3399 based laptop produced by Pine64. > > Add a basic DTS for it. > > Signed-off-by: Peter Robinson <pbrobinson at gmail.com> > Signed-off-by: Emmanuel Vadot <manu at freebsd.org> > --- > arch/arm64/boot/dts/rockchip/Makefile | 1 + > .../boot/dts/rockchip/rk3399-pinebook-pro.dts | 626 ++++++++++++++++++ > 2 files changed, 627 insertions(+) > create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile > index 48fb631d5451..9099fb7e2073 100644 > --- a/arch/arm64/boot/dts/rockchip/Makefile > +++ b/arch/arm64/boot/dts/rockchip/Makefile > @@ -28,6 +28,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopc-t4.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-m4.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-neo4.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-orangepi.dtb > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-pinebook-pro.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-puma-haikou.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-roc-pc.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-roc-pc-mezzanine.dtb > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts > new file mode 100644 > index 000000000000..d2e3d7b35cc6 > --- /dev/null > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts > @@ -0,0 +1,626 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (c) 2017 Fuzhou Rockchip Electronics Co., Ltd. > + */ > + > +/dts-v1/; > +#include <dt-bindings/input/linux-event-codes.h> > +#include <dt-bindings/pwm/pwm.h> > +#include <dt-bindings/pinctrl/rockchip.h> > +#include "rk3399.dtsi" > +#include "rk3399-opp.dtsi" > + > +/ { > + model = "Pine64 Pinebook Pro"; > + compatible = "pine64,pinebook-pro", "rockchip,rk3399"; > + > + chosen { > + stdout-path = "serial2:115200n8"; > + }; > + > + aliases { > + spi0 = &spi1; Why do we need this? Same comment as Heiko. > + }; > + > + backlight: backlight { > + compatible = "pwm-backlight"; > + enable-gpios = <&gpio1 RK_PA0 GPIO_ACTIVE_HIGH>; > + pwms = <&pwm0 0 740740 0>; > + }; > + > + gpio-keys { > + compatible = "gpio-keys"; > + autorepeat; > + pinctrl-names = "default"; > + pinctrl-0 = <&pwrbtn>; > + > + power { > + debounce-interval = <100>; > + gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>; > + label = "GPIO Key Power"; > + linux,code = <KEY_POWER>; > + wakeup-source; > + }; > + }; > + > + leds { > + status = "okay"; status below Same comment as Heiko. > + compatible = "gpio-leds"; > + > + work-led { > + label = "work"; > + gpios = <&gpio0 RK_PB3 GPIO_ACTIVE_HIGH>; > + }; > + > + standby-led { > + label = "standby"; > + gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>; > + }; > + }; > + > + vcc1v8_s3: vcca1v8_s3: vcc1v8-s3 { > + compatible = "regulator-fixed"; > + regulator-name = "vcc1v8_s3"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + vin-supply = <&vcc_1v8>; > + }; > + > + dc_12v: dc-12v { > + compatible = "regulator-fixed"; > + regulator-name = "dc_12v"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <12000000>; > + regulator-max-microvolt = <12000000>; > + }; > + > + vcc3v3_sys: vcc3v3-sys { > + compatible = "regulator-fixed"; > + regulator-name = "vcc3v3_sys"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + vin-supply = <&vcc_sys>; > + }; > + > + vcc5v0_host: vcc5v0-host-regulator { > + compatible = "regulator-fixed"; > + gpio = <&gpio4 RK_PD2 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&host_vbus_drv>; > + regulator-name = "vcc5v0_host"; > + }; > + > + vcc5v0_usb3_host: vcc5v0-usb3-host-regulator { > + compatible = "regulator-fixed"; > + enable-active-high; > + gpio = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&host_usb3_drv>; > + regulator-name = "vcc5v0_usb3_host"; > + regulator-always-on; > + }; > + > + vcc3v3_s0: vcc3v3-s0-regulator { > + compatible = "regulator-fixed"; > + enable-active-high; > + gpio = <&gpio1 RK_PC6 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&lcdvcc_en>; > + regulator-name = "vcc3v3_s0"; > + regulator-always-on; > + }; > + > + vcc_sys: vcc-sys { > + compatible = "regulator-fixed"; > + regulator-name = "vcc_sys"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + vin-supply = <&dc_12v>; > + }; > + > + vdd_log: vdd-log { > + compatible = "pwm-regulator"; > + pwms = <&pwm2 0 25000 1>; > + pwm-supply = <&vcc_sys>; > + regulator-name = "vdd_log"; > + regulator-always-on; > + regulator-boot-on; > + regulator-init-microvolt = <950000>; > + regulator-min-microvolt = <800000>; > + regulator-max-microvolt = <1400000>; > + }; > +}; > + > +&cpu_l0 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&cpu_l1 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&cpu_l2 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&cpu_l3 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&emmc_phy { > + status = "okay"; > +}; > + > +&i2c0 { > + clock-frequency = <400000>; > + i2c-scl-rising-time-ns = <168>; > + i2c-scl-falling-time-ns = <4>; > + status = "okay"; > + > + rk808: pmic at 1b { > + compatible = "rockchip,rk808"; > + reg = <0x1b>; > + interrupt-parent = <&gpio1>; > + interrupts = <21 IRQ_TYPE_LEVEL_LOW>; > + #clock-cells = <1>; > + clock-output-names = "xin32k", "rk808-clkout2"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pmic_int_l>; > + rockchip,system-power-controller; > + wakeup-source; > + > + vcc1-supply = <&vcc_sys>; > + vcc2-supply = <&vcc_sys>; > + vcc3-supply = <&vcc_sys>; > + vcc4-supply = <&vcc_sys>; > + vcc6-supply = <&vcc_sys>; > + vcc7-supply = <&vcc_sys>; > + vcc8-supply = <&vcc3v3_sys>; > + vcc9-supply = <&vcc_sys>; > + vcc10-supply = <&vcc_sys>; > + vcc11-supply = <&vcc_sys>; > + vcc12-supply = <&vcc3v3_sys>; > + vddio-supply = <&vcc_1v8>; > + > + regulators { > + vdd_center: DCDC_REG1 { > + regulator-name = "vdd_center"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <750000>; > + regulator-max-microvolt = <1350000>; > + regulator-ramp-delay = <6001>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vdd_cpu_l: DCDC_REG2 { > + regulator-name = "vdd_cpu_l"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <750000>; > + regulator-max-microvolt = <1350000>; > + regulator-ramp-delay = <6001>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc_ddr: DCDC_REG3 { > + regulator-name = "vcc_ddr"; > + regulator-always-on; > + regulator-boot-on; > + regulator-state-mem { > + regulator-on-in-suspend; > + }; > + }; > + > + vcc_1v8: DCDC_REG4 { > + regulator-name = "vcc_1v8"; > + 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>; > + }; > + }; > + > + vcc1v8_dvp: LDO_REG1 { > + regulator-name = "vcc1v8_dvp"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc3v0_touch: LDO_REG2 { > + regulator-name = "vcc3v0_touch"; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3000000>; > + regulator-state-mem { > + regulator-on-in-suspend; > + regulator-suspend-microvolt = <3000000>; > + }; > + }; > + > + vcc1v8_pmu: LDO_REG3 { > + regulator-name = "vcc1v8_pmu"; > + 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>; > + }; > + }; > + > + vcc_sdio: LDO_REG4 { > + regulator-name = "vcc_sdio"; > + 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>; > + }; > + }; > + > + vcca3v0_codec: LDO_REG5 { > + regulator-name = "vcca3v0_codec"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3000000>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc_1v5: LDO_REG6 { > + regulator-name = "vcc_1v5"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1500000>; > + regulator-max-microvolt = <1500000>; > + regulator-state-mem { > + regulator-on-in-suspend; > + regulator-suspend-microvolt = <1500000>; > + }; > + }; > + > + vcca1v8_codec: LDO_REG7 { > + regulator-name = "vcca1v8_codec"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc_3v0: LDO_REG8 { > + regulator-name = "vcc_3v0"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3000000>; > + regulator-state-mem { > + regulator-on-in-suspend; > + regulator-suspend-microvolt = <3000000>; > + }; > + }; > + > + vcc3v3_s3: SWITCH_REG1 { > + regulator-name = "vcc3v3_s3"; > + regulator-always-on; > + regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + switch_reg2: SWITCH_REG2 { > + regulator-name = "SWITCH_REG2"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + }; > + }; > + > + vdd_cpu_b: regulator at 40 { > + compatible = "silergy,syr827"; > + reg = <0x40>; > + fcs,suspend-voltage-selector = <1>; > + pinctrl-names = "default"; > + pinctrl-0 = <&vsel1_gpio>; > + vsel-gpios = <&gpio1 RK_PC1 GPIO_ACTIVE_HIGH>; > + regulator-compatible = "fan53555-reg"; > + regulator-name = "vdd_cpu_b"; > + regulator-min-microvolt = <712500>; > + regulator-max-microvolt = <1500000>; > + regulator-ramp-delay = <1000>; > + regulator-always-on; > + regulator-boot-on; > + vin-supply = <&vcc_sys>; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vdd_gpu: regulator at 41 { > + compatible = "silergy,syr828"; > + reg = <0x41>; > + fcs,suspend-voltage-selector = <1>; > + pinctrl-names = "default"; > + pinctrl-0 = <&vsel2_gpio>; > + vsel-gpios = <&gpio1 RK_PB6 GPIO_ACTIVE_HIGH>; > + regulator-compatible = "fan53555-reg"; > + regulator-name = "vdd_gpu"; > + regulator-min-microvolt = <712500>; > + regulator-max-microvolt = <1500000>; > + regulator-ramp-delay = <1000>; > + regulator-always-on; > + regulator-boot-on; > + vin-supply = <&vcc_sys>; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > +}; > + > +&i2c4 { > + i2c-scl-rising-time-ns = <600>; > + i2c-scl-falling-time-ns = <20>; > + status = "okay"; > + > + fusb0: typec-portc at 22 { > + compatible = "fcs,fusb302"; > + reg = <0x22>; > + interrupt-parent = <&gpio1>; > + interrupts = <RK_PA2 IRQ_TYPE_LEVEL_LOW>; > + pinctrl-names = "default"; > + pinctrl-0 = <&fusb0_int>; > + vbus-supply = <&vcc5v0_usb3_host>; > + status = "okay"; Already okay I think? > + }; > +}; > + > +&io_domains { > + status = "okay"; status below > + > + bt656-supply = <&vcc1v8_dvp>; > + audio-supply = <&vcca1v8_codec>; > + sdmmc-supply = <&vcc_sdio>; > + gpio1830-supply = <&vcc_3v0>; > +}; > + > +&pmu_io_domains { Sort nodes alphabetically. > + pmu1830-supply = <&vcc_3v0>; > + status = "okay"; > +}; > + > +&pinctrl { > + buttons { > + pwrbtn: pwrbtn { > + rockchip,pins = <0 RK_PA5 RK_FUNC_GPIO &pcfg_pull_up>; > + }; > + }; > + > + fusb302x { > + fusb0_int: fusb0-int { > + rockchip,pins = <1 RK_PA2 RK_FUNC_GPIO &pcfg_pull_up>; > + }; > + }; > + > + pmic { > + pmic_int_l: pmic-int-l { > + rockchip,pins = <3 RK_PB2 RK_FUNC_GPIO &pcfg_pull_up>; > + }; > + > + vsel1_gpio: vsel1-gpio { > + rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>; > + }; > + > + vsel2_gpio: vsel2-gpio { > + rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>; > + }; > + }; > + > + sdio-pwrseq { > + wifi_enable_h: wifi-enable-h { > + rockchip,pins = <0 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>; > + }; > + }; > + > + usb-typec { > + vcc5v0_typec_en: vcc5v0_typec_en { > + rockchip,pins = <1 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>; > + }; > + }; > + > + usb2 { > + vcc5v0_host_en: vcc5v0-host-en { > + rockchip,pins = <4 RK_PC5 RK_FUNC_GPIO &pcfg_pull_none>; > + }; > + > + host_vbus_drv: host-vbus-drv { > + rockchip,pins = <4 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>; > + }; > + > + host_usb3_drv: host-usb3-drv { > + rockchip,pins = <1 RK_PB5 RK_FUNC_GPIO &pcfg_pull_none>; > + }; > + > + /* Shared between LCD and usb */ > + lcdvcc_en: lcdvcc-en { > + rockchip,pins = <1 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>; > + }; > + }; > +}; > + > +&pwm0 { > + status = "okay"; > +}; > + > +&pwm1 { > + status = "okay"; > +}; > + > +&pwm2 { > + status = "okay"; > +}; > + > +&saradc { > + status = "okay"; > +}; > + > +&sdmmc { > + bus-width = <4>; > + status = "okay"; status below > + max-frequency = <20000000>; > +}; > + > +&sdhci { Sort nodes alphabetically. > + bus-width = <8>; > + max-frequency = <25000000>; > + mmc-hs400-1_8v; > + mmc-hs400-enhanced-strobe; > + non-removable; > + status = "okay"; > +}; > + > +&tcphy0 { > + status = "okay"; > +}; > + > +&tcphy1 { > + status = "okay"; > +}; > + > +&tsadc { > + /* tshut mode 0:CRU 1:GPIO */ > + rockchip,hw-tshut-mode = <1>; > + /* tshut polarity 0:LOW 1:HIGH */ > + rockchip,hw-tshut-polarity = <1>; > + status = "okay"; > +}; > + > +&u2phy0 { > + status = "okay"; > + > + u2phy0_otg: otg-port { > + status = "okay"; > + }; > + > + u2phy0_host: host-port { > + phy-supply = <&vcc5v0_host>; > + status = "okay"; > + }; > +}; > + > +&u2phy1 { > + status = "okay"; > + > + u2phy1_otg: otg-port { > + status = "okay"; > + }; > + > + u2phy1_host: host-port { > + phy-supply = <&vcc5v0_host>; > + status = "okay"; > + }; > +}; > + > +&uart2 { > + status = "okay"; > +}; > + > +&usb_host0_ehci { > + status = "okay"; > +}; > + > +&usb_host0_ohci { > + status = "okay"; > +}; > + > +&usb_host1_ehci { > + phy-supply = <&vcc3v3_s0>; > + status = "okay"; > +}; > + > +&usb_host1_ohci { > + phy-supply = <&vcc3v3_s0>; > + status = "okay"; > +}; > + > +&usbdrd3_0 { > + status = "okay"; > +}; > + > +&usbdrd_dwc3_0 { > + status = "okay"; status below > + dr_mode = "otg" > +}; > + > +&usbdrd3_1 { > + status = "okay"; > +}; > + > +&usbdrd_dwc3_1 { > + status = "okay"; > + dr_mode = "host"; status below > +}; > + > +&spi1 { Sort nodes alphabetically. > + status = "okay"; > + > + spiflash: spi-flash at 0 { Can't find spi-flash in the devicetree documentation. flash@0 ? > + #address-cells = <0x1>; > + #size-cells = <1>; #address-cells and #size-cells for what use here? > + compatible = "jedec,spi-nor"; > + reg = <0x0>; reg = <0>; > + spi-max-frequency = <10000000>; > + status = "okay"; > + > + partitions { > + compatible = "fixed-partitions"; Partitions normally shouldn't go in a dts file. It's up to the user. > + #address-cells = <1>; > + #size-cells = <1>; > + > + loader at 8000 { > + label = "loader"; > + reg = <0x0 0x3F8000>; > + }; > + > + env at 3f8000 { > + label = "env"; > + reg = <0x3F8000 0x8000>; > + }; > + > + vendor at 7c0000 { > + label = "vendor"; > + reg = <0x7C0000 0x40000>; > + }; > + }; > + }; > +}; > -- > 2.24.1
Hi, I do strongly oppose merging this DTS. It is missing several device components (for example the eDP panel and the lid switch) and the power supply setup does not reflect the implementation in the device at all. Also some control/irq GPIOs are incorrect. See comments below. The schematic of the Pinebook Pro can be found here: http://files.pine64.org/doc/PinebookPro/pinebookpro_v2.1_mainboard_schematic.pdf I'm currently planning on submitting a DTS with correct power supply and GPIO setup as well as more supported hardware components late February. Unfortunately I won't get around to doing it any sooner. Current state of the DTS can be found here: https://gitlab.manjaro.org/tsys/linux-pinebook-pro/blob/v5.5-rc7/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts > From: Peter Robinson <pbrobinson@gmail.com> > > + gpio-keys { > + compatible = "gpio-keys"; > + autorepeat; > + pinctrl-names = "default"; > + pinctrl-0 = <&pwrbtn>; > + > + power { > + debounce-interval = <100>; > + gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>; > + label = "GPIO Key Power"; > + linux,code = <KEY_POWER>; > + wakeup-source; > + }; Missing lid switch > + }; > + > + leds { > + status = "okay"; > + compatible = "gpio-leds"; > + > + work-led { > + label = "work"; > + gpios = <&gpio0 RK_PB3 GPIO_ACTIVE_HIGH>; > + }; > + > + standby-led { > + label = "standby"; > + gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>; > + }; > + }; > + > + vcc1v8_s3: vcca1v8_s3: vcc1v8-s3 { vcc1v8_s3 and vcca1v8_s3 are not the same. They are powered from different vin supplies. > + compatible = "regulator-fixed"; > + regulator-name = "vcc1v8_s3"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + vin-supply = <&vcc_1v8>; > + }; > + > + dc_12v: dc-12v { > + compatible = "regulator-fixed"; > + regulator-name = "dc_12v"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <12000000>; > + regulator-max-microvolt = <12000000>; > + }; There is no primary 12 V supply on the Pinebook Pro > + > + vcc3v3_sys: vcc3v3-sys { > + compatible = "regulator-fixed"; > + regulator-name = "vcc3v3_sys"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + vin-supply = <&vcc_sys>; > + }; > + > + vcc5v0_host: vcc5v0-host-regulator { > + compatible = "regulator-fixed"; > + gpio = <&gpio4 RK_PD2 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&host_vbus_drv>; > + regulator-name = "vcc5v0_host"; > + }; > + > + vcc5v0_usb3_host: vcc5v0-usb3-host-regulator { > + compatible = "regulator-fixed"; > + enable-active-high; > + gpio = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&host_usb3_drv>; > + regulator-name = "vcc5v0_usb3_host"; > + regulator-always-on; > + }; > + > + vcc3v3_s0: vcc3v3-s0-regulator { > + compatible = "regulator-fixed"; > + enable-active-high; > + gpio = <&gpio1 RK_PC6 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&lcdvcc_en>; > + regulator-name = "vcc3v3_s0"; > + regulator-always-on; > + }; > + > + vcc_sys: vcc-sys { > + compatible = "regulator-fixed"; > + regulator-name = "vcc_sys"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + vin-supply = <&dc_12v>; > + }; > +The main system voltage is not 5 V and it is not powered by a 12 V rail > + vdd_log: vdd-log { > + compatible = "pwm-regulator"; > + pwms = <&pwm2 0 25000 1>; > + pwm-supply = <&vcc_sys>; > + regulator-name = "vdd_log"; > + regulator-always-on; > + regulator-boot-on; > + regulator-init-microvolt = <950000>; > + regulator-min-microvolt = <800000>; > + regulator-max-microvolt = <1400000>; > + }; > +}; > + Regulators for backlight, 0.9 V rail, USB-C power out and SD card power are missing Also most regulator names do not match the names of the power rails in the schematic > +&cpu_l0 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&cpu_l1 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&cpu_l2 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&cpu_l3 { > + cpu-supply = <&vdd_cpu_l>; > +}; > + > +&emmc_phy { > + status = "okay"; > +}; > + > +&i2c0 { > + clock-frequency = <400000>; > + i2c-scl-rising-time-ns = <168>; > + i2c-scl-falling-time-ns = <4>; > + status = "okay"; > + > + rk808: pmic@1b { > + compatible = "rockchip,rk808"; > + reg = <0x1b>; > + interrupt-parent = <&gpio1>; > + interrupts = <21 IRQ_TYPE_LEVEL_LOW>; Interrupt GPIO is not correct, should be 10. This will stop the RTC from working correctly. > + #clock-cells = <1>; > + clock-output-names = "xin32k", "rk808-clkout2"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pmic_int_l>; > + rockchip,system-power-controller; > + wakeup-source; > + > + vcc1-supply = <&vcc_sys>; > + vcc2-supply = <&vcc_sys>; > + vcc3-supply = <&vcc_sys>; > + vcc4-supply = <&vcc_sys>; > + vcc6-supply = <&vcc_sys>; > + vcc7-supply = <&vcc_sys>; > + vcc8-supply = <&vcc3v3_sys>; > + vcc9-supply = <&vcc_sys>; > + vcc10-supply = <&vcc_sys>; > + vcc11-supply = <&vcc_sys>; > + vcc12-supply = <&vcc3v3_sys>; > + vddio-supply = <&vcc_1v8>; vddio-supply is not vcc_1v8 but vcc_3v0 > + > + regulators { > + vdd_center: DCDC_REG1 { > + regulator-name = "vdd_center"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <750000>; > + regulator-max-microvolt = <1350000>; > + regulator-ramp-delay = <6001>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vdd_cpu_l: DCDC_REG2 { > + regulator-name = "vdd_cpu_l"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <750000>; > + regulator-max-microvolt = <1350000>; > + regulator-ramp-delay = <6001>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc_ddr: DCDC_REG3 { > + regulator-name = "vcc_ddr"; > + regulator-always-on; > + regulator-boot-on; > + regulator-state-mem { > + regulator-on-in-suspend; > + }; > + }; > + > + vcc_1v8: DCDC_REG4 { > + regulator-name = "vcc_1v8"; > + 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>; > + }; > + }; > + > + vcc1v8_dvp: LDO_REG1 { > + regulator-name = "vcc1v8_dvp"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc3v0_touch: LDO_REG2 { > + regulator-name = "vcc3v0_touch"; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3000000>; > + regulator-state-mem { > + regulator-on-in-suspend; > + regulator-suspend-microvolt = <3000000>; > + }; > + }; > + > + vcc1v8_pmu: LDO_REG3 { > + regulator-name = "vcc1v8_pmu"; > + 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>; > + }; > + }; > + > + vcc_sdio: LDO_REG4 { > + regulator-name = "vcc_sdio"; > + 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>; > + }; > + }; > + > + vcca3v0_codec: LDO_REG5 { > + regulator-name = "vcca3v0_codec"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3000000>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc_1v5: LDO_REG6 { > + regulator-name = "vcc_1v5"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1500000>; > + regulator-max-microvolt = <1500000>; > + regulator-state-mem { > + regulator-on-in-suspend; > + regulator-suspend-microvolt = <1500000>; > + }; > + }; > + > + vcca1v8_codec: LDO_REG7 { > + regulator-name = "vcca1v8_codec"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vcc_3v0: LDO_REG8 { > + regulator-name = "vcc_3v0"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3000000>; > + regulator-state-mem { > + regulator-on-in-suspend; > + regulator-suspend-microvolt = <3000000>; > + }; > + }; > + > + vcc3v3_s3: SWITCH_REG1 { > + regulator-name = "vcc3v3_s3"; > + regulator-always-on; > + regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + switch_reg2: SWITCH_REG2 { > + regulator-name = "SWITCH_REG2"; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + }; > + }; > + > + vdd_cpu_b: regulator@40 { > + compatible = "silergy,syr827"; > + reg = <0x40>; > + fcs,suspend-voltage-selector = <1>; > + pinctrl-names = "default"; > + pinctrl-0 = <&vsel1_gpio>; > + vsel-gpios = <&gpio1 RK_PC1 GPIO_ACTIVE_HIGH>; > + regulator-compatible = "fan53555-reg"; > + regulator-name = "vdd_cpu_b"; > + regulator-min-microvolt = <712500>; > + regulator-max-microvolt = <1500000>; > + regulator-ramp-delay = <1000>; > + regulator-always-on; > + regulator-boot-on; > + vin-supply = <&vcc_sys>; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > + vdd_gpu: regulator@41 { > + compatible = "silergy,syr828"; > + reg = <0x41>; > + fcs,suspend-voltage-selector = <1>; > + pinctrl-names = "default"; > + pinctrl-0 = <&vsel2_gpio>; > + vsel-gpios = <&gpio1 RK_PB6 GPIO_ACTIVE_HIGH>; > + regulator-compatible = "fan53555-reg"; > + regulator-name = "vdd_gpu"; > + regulator-min-microvolt = <712500>; > + regulator-max-microvolt = <1500000>; > + regulator-ramp-delay = <1000>; > + regulator-always-on; > + regulator-boot-on; > + vin-supply = <&vcc_sys>; > + > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > + }; > + > +}; Additionally there a quite a few components missing. This includes the aforementioned lid switch and eDP panel but there are also a battery gauge, two chargers, a es8316 audio codec, two speaker amplifiers and a BT/WiFi module. Best regards, Tobias
Hi, Am Dienstag, 21. Januar 2020, 15:13:06 CET schrieb Tobias Schramm: > I do strongly oppose merging this DTS. It is missing several device > components (for example the eDP panel and the lid switch) and the power > supply setup does not reflect the implementation in the device at all. > Also some control/irq GPIOs are incorrect. See comments below. > > The schematic of the Pinebook Pro can be found here: > http://files.pine64.org/doc/PinebookPro/pinebookpro_v2.1_mainboard_schematic.pdf > > I'm currently planning on submitting a DTS with correct power supply and > GPIO setup as well as more supported hardware components late February. > Unfortunately I won't get around to doing it any sooner. > > Current state of the DTS can be found here: > https://gitlab.manjaro.org/tsys/linux-pinebook-pro/blob/v5.5-rc7/arch/arm64/boot/dts/rockchip/rk3399-pinebook-pro.dts I really like power trees that match the actual schematics, so I'm all for it ;-) . I don't have a Pinebook Pro myself, so won't be comparing the power-tree to the schematics myself though. As for timing, it's too late for 5.6 anyway, so you have time until around 5.6-rc5 gets released so around 8 weeks. The other parts you can coordinate between you ;-) Heiko > > From: Peter Robinson <pbrobinson@gmail.com> > > > > > + gpio-keys { > > + compatible = "gpio-keys"; > > + autorepeat; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pwrbtn>; > > + > > + power { > > + debounce-interval = <100>; > > + gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>; > > + label = "GPIO Key Power"; > > + linux,code = <KEY_POWER>; > > + wakeup-source; > > + }; > > Missing lid switch > > + }; > > + > > + leds { > > + status = "okay"; > > + compatible = "gpio-leds"; > > + > > + work-led { > > + label = "work"; > > + gpios = <&gpio0 RK_PB3 GPIO_ACTIVE_HIGH>; > > + }; > > + > > + standby-led { > > + label = "standby"; > > + gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>; > > + }; > > + }; > > + > > + vcc1v8_s3: vcca1v8_s3: vcc1v8-s3 { > vcc1v8_s3 and vcca1v8_s3 are not the same. They are powered from > different vin supplies. > > > + compatible = "regulator-fixed"; > > + regulator-name = "vcc1v8_s3"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + vin-supply = <&vcc_1v8>; > > + }; > > + > > + dc_12v: dc-12v { > > + compatible = "regulator-fixed"; > > + regulator-name = "dc_12v"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <12000000>; > > + regulator-max-microvolt = <12000000>; > > + }; > There is no primary 12 V supply on the Pinebook Pro > > > + > > + vcc3v3_sys: vcc3v3-sys { > > + compatible = "regulator-fixed"; > > + regulator-name = "vcc3v3_sys"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + vin-supply = <&vcc_sys>; > > + }; > > + > > + vcc5v0_host: vcc5v0-host-regulator { > > + compatible = "regulator-fixed"; > > + gpio = <&gpio4 RK_PD2 GPIO_ACTIVE_HIGH>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&host_vbus_drv>; > > + regulator-name = "vcc5v0_host"; > > + }; > > + > > + vcc5v0_usb3_host: vcc5v0-usb3-host-regulator { > > + compatible = "regulator-fixed"; > > + enable-active-high; > > + gpio = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&host_usb3_drv>; > > + regulator-name = "vcc5v0_usb3_host"; > > + regulator-always-on; > > + }; > > + > > + vcc3v3_s0: vcc3v3-s0-regulator { > > + compatible = "regulator-fixed"; > > + enable-active-high; > > + gpio = <&gpio1 RK_PC6 GPIO_ACTIVE_HIGH>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&lcdvcc_en>; > > + regulator-name = "vcc3v3_s0"; > > + regulator-always-on; > > + }; > > + > > + vcc_sys: vcc-sys { > > + compatible = "regulator-fixed"; > > + regulator-name = "vcc_sys"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <5000000>; > > + regulator-max-microvolt = <5000000>; > > + vin-supply = <&dc_12v>; > > + }; > > +The main system voltage is not 5 V and it is not powered by a 12 V rail > > > + vdd_log: vdd-log { > > + compatible = "pwm-regulator"; > > + pwms = <&pwm2 0 25000 1>; > > + pwm-supply = <&vcc_sys>; > > + regulator-name = "vdd_log"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-init-microvolt = <950000>; > > + regulator-min-microvolt = <800000>; > > + regulator-max-microvolt = <1400000>; > > + }; > > +}; > > + > > Regulators for backlight, 0.9 V rail, USB-C power out and SD card power > are missing > > Also most regulator names do not match the names of the power rails in > the schematic > > > +&cpu_l0 { > > + cpu-supply = <&vdd_cpu_l>; > > +}; > > + > > +&cpu_l1 { > > + cpu-supply = <&vdd_cpu_l>; > > +}; > > + > > +&cpu_l2 { > > + cpu-supply = <&vdd_cpu_l>; > > +}; > > + > > +&cpu_l3 { > > + cpu-supply = <&vdd_cpu_l>; > > +}; > > + > > +&emmc_phy { > > + status = "okay"; > > +}; > > + > > +&i2c0 { > > + clock-frequency = <400000>; > > + i2c-scl-rising-time-ns = <168>; > > + i2c-scl-falling-time-ns = <4>; > > + status = "okay"; > > + > > + rk808: pmic@1b { > > + compatible = "rockchip,rk808"; > > + reg = <0x1b>; > > + interrupt-parent = <&gpio1>; > > + interrupts = <21 IRQ_TYPE_LEVEL_LOW>; > Interrupt GPIO is not correct, should be 10. This will stop the RTC from > working correctly. > > > + #clock-cells = <1>; > > + clock-output-names = "xin32k", "rk808-clkout2"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pmic_int_l>; > > + rockchip,system-power-controller; > > + wakeup-source; > > + > > + vcc1-supply = <&vcc_sys>; > > + vcc2-supply = <&vcc_sys>; > > + vcc3-supply = <&vcc_sys>; > > + vcc4-supply = <&vcc_sys>; > > + vcc6-supply = <&vcc_sys>; > > + vcc7-supply = <&vcc_sys>; > > + vcc8-supply = <&vcc3v3_sys>; > > + vcc9-supply = <&vcc_sys>; > > + vcc10-supply = <&vcc_sys>; > > + vcc11-supply = <&vcc_sys>; > > + vcc12-supply = <&vcc3v3_sys>; > > + vddio-supply = <&vcc_1v8>; > vddio-supply is not vcc_1v8 but vcc_3v0 > > > + > > + regulators { > > + vdd_center: DCDC_REG1 { > > + regulator-name = "vdd_center"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <750000>; > > + regulator-max-microvolt = <1350000>; > > + regulator-ramp-delay = <6001>; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + > > + vdd_cpu_l: DCDC_REG2 { > > + regulator-name = "vdd_cpu_l"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <750000>; > > + regulator-max-microvolt = <1350000>; > > + regulator-ramp-delay = <6001>; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + > > + vcc_ddr: DCDC_REG3 { > > + regulator-name = "vcc_ddr"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + }; > > + }; > > + > > + vcc_1v8: DCDC_REG4 { > > + regulator-name = "vcc_1v8"; > > + 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>; > > + }; > > + }; > > + > > + vcc1v8_dvp: LDO_REG1 { > > + regulator-name = "vcc1v8_dvp"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + > > + vcc3v0_touch: LDO_REG2 { > > + regulator-name = "vcc3v0_touch"; > > + regulator-min-microvolt = <3000000>; > > + regulator-max-microvolt = <3000000>; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + regulator-suspend-microvolt = <3000000>; > > + }; > > + }; > > + > > + vcc1v8_pmu: LDO_REG3 { > > + regulator-name = "vcc1v8_pmu"; > > + 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>; > > + }; > > + }; > > + > > + vcc_sdio: LDO_REG4 { > > + regulator-name = "vcc_sdio"; > > + 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>; > > + }; > > + }; > > + > > + vcca3v0_codec: LDO_REG5 { > > + regulator-name = "vcca3v0_codec"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <3000000>; > > + regulator-max-microvolt = <3000000>; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + > > + vcc_1v5: LDO_REG6 { > > + regulator-name = "vcc_1v5"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <1500000>; > > + regulator-max-microvolt = <1500000>; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + regulator-suspend-microvolt = <1500000>; > > + }; > > + }; > > + > > + vcca1v8_codec: LDO_REG7 { > > + regulator-name = "vcca1v8_codec"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + > > + vcc_3v0: LDO_REG8 { > > + regulator-name = "vcc_3v0"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <3000000>; > > + regulator-max-microvolt = <3000000>; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + regulator-suspend-microvolt = <3000000>; > > + }; > > + }; > > + > > + vcc3v3_s3: SWITCH_REG1 { > > + regulator-name = "vcc3v3_s3"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + > > + switch_reg2: SWITCH_REG2 { > > + regulator-name = "SWITCH_REG2"; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + }; > > + }; > > + > > + vdd_cpu_b: regulator@40 { > > + compatible = "silergy,syr827"; > > + reg = <0x40>; > > + fcs,suspend-voltage-selector = <1>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&vsel1_gpio>; > > + vsel-gpios = <&gpio1 RK_PC1 GPIO_ACTIVE_HIGH>; > > + regulator-compatible = "fan53555-reg"; > > + regulator-name = "vdd_cpu_b"; > > + regulator-min-microvolt = <712500>; > > + regulator-max-microvolt = <1500000>; > > + regulator-ramp-delay = <1000>; > > + regulator-always-on; > > + regulator-boot-on; > > + vin-supply = <&vcc_sys>; > > + > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + > > + vdd_gpu: regulator@41 { > > + compatible = "silergy,syr828"; > > + reg = <0x41>; > > + fcs,suspend-voltage-selector = <1>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&vsel2_gpio>; > > + vsel-gpios = <&gpio1 RK_PB6 GPIO_ACTIVE_HIGH>; > > + regulator-compatible = "fan53555-reg"; > > + regulator-name = "vdd_gpu"; > > + regulator-min-microvolt = <712500>; > > + regulator-max-microvolt = <1500000>; > > + regulator-ramp-delay = <1000>; > > + regulator-always-on; > > + regulator-boot-on; > > + vin-supply = <&vcc_sys>; > > + > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > + }; > > + > > +}; > > Additionally there a quite a few components missing. This includes the > aforementioned lid switch and eDP panel but there are also a battery > gauge, two chargers, a es8316 audio codec, two speaker amplifiers and a > BT/WiFi module. > > Best regards, > > Tobias > >
On Thu, 16 Jan 2020 23:56:16 +0100, Emmanuel Vadot wrote: > Add a compatible for Pine64 Pinebook Pro > > Signed-off-by: Emmanuel Vadot <manu@freebsd.org> > --- > Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++ > 1 file changed, 5 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org>
diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml index d9847b306b83..2f6b72b084a4 100644 --- a/Documentation/devicetree/bindings/arm/rockchip.yaml +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml @@ -412,6 +412,11 @@ properties: - const: pine64,rockpro64 - const: rockchip,rk3399 + - description: Pine64 PinebookPro + items: + - const: pine64,pinebook-pro + - const: rockchip,rk3399 + - description: Radxa Rock items: - const: radxa,rock
Add a compatible for Pine64 Pinebook Pro Signed-off-by: Emmanuel Vadot <manu@freebsd.org> --- Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++ 1 file changed, 5 insertions(+)