2022-08-15 12:42:19

by Tom Fitzhenry

[permalink] [raw]
Subject: [PATCH v3 0/2] Add support for the Pine64 PinePhone Pro phone

PinePhone Pro is a RK3399 based phone produced by Pine64.

Add a basic DTS for it. This is a working base that will allow myself and
others to add more nodes.

Changes since v2:
* Used rk3399-opp.dtsi (+ disabled) rather than introducing rk3399-t-opp.dtsi
* Removed superfluous comments.
* Followed DT Generic Names Recommendation
* Set 'From:' of DTS patch to be Martijn, the primary contributor to the DT.

Changes since v1:
* Simplified the DT to a minimal base.
* Introduced the RK3399-T OPPs.

Martijn Braam (1):
arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

Tom Fitzhenry (1):
dt-bindings: arm: rockchip: Add PinePhone Pro bindings

.../devicetree/bindings/arm/rockchip.yaml | 5 +
arch/arm64/boot/dts/rockchip/Makefile | 1 +
.../dts/rockchip/rk3399-pinephone-pro.dts | 394 ++++++++++++++++++
3 files changed, 400 insertions(+)
create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts


base-commit: 2e1170c71ec0c0fb785ce76fca7c682e930af2b8
--
2.37.1


2022-08-15 13:07:05

by Tom Fitzhenry

[permalink] [raw]
Subject: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

From: Martijn Braam <[email protected]>

This is a basic DT containing regulators and UART, intended to be a
base that myself and others can add additional nodes in future patches.

Tested to work: booting from eMMC, output over UART.

https://wiki.pine64.org/wiki/PinePhone_Pro

This is derived from
https://gitlab.com/pine64-org/linux/-/commit/261d3b5f8ac503f97da810986d1d6422430c8531
with fixes from https://megous.com/git/linux.

Signed-off-by: Martijn Braam <[email protected]>
Co-developed-by: Kamil Trzciński <[email protected]>
[no SoB, but Kamil is happy for this patch to be submitted]
Co-developed-by: Ondrej Jirman <[email protected]>
Signed-off-by: Ondrej Jirman <[email protected]>
Co-developed-by: Tom Fitzhenry <[email protected]>
Signed-off-by: Tom Fitzhenry <[email protected]>
---
arch/arm64/boot/dts/rockchip/Makefile | 1 +
.../dts/rockchip/rk3399-pinephone-pro.dts | 394 ++++++++++++++++++
2 files changed, 395 insertions(+)
create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index ef79a672804a1..cb42e0a15808e 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-neo4.dtb
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-r4s.dtb
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-orangepi.dtb
dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-pinebook-pro.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-pinephone-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-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
new file mode 100644
index 0000000000000..f9f1dcd4f6494
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
@@ -0,0 +1,394 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2020 Martijn Braam <[email protected]>
+ * Copyright (c) 2021 Kamil Trzciński <[email protected]>
+ */
+
+/* PinePhone Pro datasheet:
+ * https://files.pine64.org/doc/PinePhonePro/PinephonePro-Schematic-V1.0-20211127.pdf
+ */
+
+/dts-v1/;
+#include <dt-bindings/input/linux-event-codes.h>
+#include "rk3399.dtsi"
+#include "rk3399-opp.dtsi"
+
+/ {
+ model = "Pine64 PinePhonePro";
+ compatible = "pine64,pinephone-pro", "rockchip,rk3399";
+ chassis-type = "handset";
+
+ aliases {
+ mmc0 = &sdio0;
+ mmc1 = &sdmmc;
+ mmc2 = &sdhci;
+ };
+
+ chosen {
+ stdout-path = "serial2:115200n8";
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwrbtn_pin>;
+
+ power {
+ debounce-interval = <20>;
+ gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>;
+ label = "Power";
+ linux,code = <KEY_POWER>;
+ wakeup-source;
+ };
+ };
+
+ vcc_sysin: vcc-sysin-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc_sysin";
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ vcc3v3_sys: vcc3v3-sys-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc3v3_sys";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ vin-supply = <&vcc_sysin>;
+ };
+
+ vcca1v8_s3: vcc1v8-s3-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "vcca1v8_s3";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ vin-supply = <&vcc3v3_sys>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+};
+
+&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>;
+};
+
+&cpu_b0 {
+ cpu-supply = <&vdd_cpu_b>;
+};
+
+&cpu_b1 {
+ cpu-supply = <&vdd_cpu_b>;
+};
+
+&emmc_phy {
+ status = "okay";
+};
+
+&i2c0 {
+ clock-frequency = <400000>;
+ i2c-scl-rising-time-ns = <168>;
+ i2c-scl-falling-time-ns = <4>;
+ status = "okay";
+
+ rk818: pmic@1c {
+ compatible = "rockchip,rk818";
+ reg = <0x1c>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <RK_PC5 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_sysin>;
+ vcc2-supply = <&vcc_sysin>;
+ vcc3-supply = <&vcc_sysin>;
+ vcc4-supply = <&vcc_sysin>;
+ vcc6-supply = <&vcc_sysin>;
+ vcc7-supply = <&vcc3v3_sys>;
+ vcc8-supply = <&vcc_sysin>;
+ vcc9-supply = <&vcc3v3_sys>;
+
+ regulators {
+ vdd_cpu_l: DCDC_REG1 {
+ 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;
+ };
+ };
+
+ vdd_center: DCDC_REG2 {
+ regulator-name = "vdd_center";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <800000>;
+ 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: vcc_wl: 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;
+ };
+ };
+
+ vcca3v0_codec: LDO_REG1 {
+ regulator-name = "vcca3v0_codec";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
+ };
+
+ vcc3v0_touch: LDO_REG2 {
+ regulator-name = "vcc3v0_touch";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };
+
+ vcca1v8_codec: LDO_REG3 {
+ regulator-name = "vcca1v8_codec";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
+ };
+
+ vcc_power_on: LDO_REG4 {
+ regulator-name = "vcc_power_on";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
+ };
+
+ vcc_3v0: LDO_REG5 {
+ 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;
+ };
+ };
+
+ 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;
+ };
+ };
+
+ vcc1v8_dvp: LDO_REG7 {
+ regulator-name = "vcc1v8_dvp";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ vcc3v3_s3: LDO_REG8 {
+ regulator-name = "vcc3v3_s3";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };
+
+ vccio_sd: LDO_REG9 {
+ regulator-name = "vccio_sd";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ vcc3v3_s0: SWITCH_REG {
+ regulator-name = "vcc3v3_s0";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
+ };
+ };
+ };
+
+ vdd_cpu_b: regulator@40 {
+ compatible = "silergy,syr827";
+ reg = <0x40>;
+ fcs,suspend-voltage-selector = <1>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&vsel1_pin>;
+ regulator-name = "vdd_cpu_b";
+ regulator-min-microvolt = <712500>;
+ regulator-max-microvolt = <1500000>;
+ regulator-ramp-delay = <1000>;
+ regulator-always-on;
+ regulator-boot-on;
+
+ 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_pin>;
+ regulator-name = "vdd_gpu";
+ regulator-min-microvolt = <712500>;
+ regulator-max-microvolt = <1500000>;
+ regulator-ramp-delay = <1000>;
+ regulator-always-on;
+ regulator-boot-on;
+
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };
+};
+
+&cluster0_opp {
+ opp04 {
+ status = "disabled";
+ };
+
+ opp05 {
+ status = "disabled";
+ };
+};
+
+&cluster1_opp {
+ opp06 {
+ status = "disabled";
+ };
+
+ opp07 {
+ status = "disabled";
+ };
+};
+
+&io_domains {
+ status = "okay";
+
+ bt656-supply = <&vcc1v8_dvp>;
+ audio-supply = <&vcca1v8_codec>;
+ sdmmc-supply = <&vccio_sd>;
+ gpio1830-supply = <&vcc_3v0>;
+};
+
+&pmu_io_domains {
+ pmu1830-supply = <&vcc_1v8>;
+ status = "okay";
+};
+
+&pinctrl {
+ buttons {
+ pwrbtn_pin: pwrbtn-pin {
+ rockchip,pins = <0 RK_PA5 RK_FUNC_GPIO &pcfg_pull_up>;
+ };
+ };
+
+ pmic {
+ pmic_int_l: pmic-int-l {
+ rockchip,pins = <1 RK_PC5 RK_FUNC_GPIO &pcfg_pull_up>;
+ };
+
+ vsel1_pin: vsel1-pin {
+ rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
+ };
+
+ vsel2_pin: vsel2-pin {
+ rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
+ };
+ };
+};
+
+&sdmmc {
+ bus-width = <4>;
+ cap-sd-highspeed;
+ cd-gpios = <&gpio0 RK_PA7 GPIO_ACTIVE_LOW>;
+ disable-wp;
+ max-frequency = <150000000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd &sdmmc_bus4>;
+ vmmc-supply = <&vcc3v3_sys>;
+ vqmmc-supply = <&vccio_sd>;
+ status = "okay";
+};
+
+&sdhci {
+ bus-width = <8>;
+ mmc-hs200-1_8v;
+ non-removable;
+ status = "okay";
+};
+
+&tsadc {
+ rockchip,hw-tshut-mode = <1>;
+ rockchip,hw-tshut-polarity = <1>;
+ status = "okay";
+};
+
+&uart2 {
+ status = "okay";
+};
--
2.37.1

2022-08-16 02:59:54

by Caleb Connolly

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro



On 15/08/2022 13:30, Tom Fitzhenry wrote:
> From: Martijn Braam <[email protected]>
>
> This is a basic DT containing regulators and UART, intended to be a
> base that myself and others can add additional nodes in future patches.
>
> Tested to work: booting from eMMC, output over UART.
>
> https://wiki.pine64.org/wiki/PinePhone_Pro
>
> This is derived from
> https://gitlab.com/pine64-org/linux/-/commit/261d3b5f8ac503f97da810986d1d6422430c8531
> with fixes from https://megous.com/git/linux.
>
> Signed-off-by: Martijn Braam <[email protected]>
> Co-developed-by: Kamil Trzciński <[email protected]>
> [no SoB, but Kamil is happy for this patch to be submitted]
> Co-developed-by: Ondrej Jirman <[email protected]>
> Signed-off-by: Ondrej Jirman <[email protected]>
> Co-developed-by: Tom Fitzhenry <[email protected]>
> Signed-off-by: Tom Fitzhenry <[email protected]>

Reviewed-by: Caleb Connolly <[email protected]>
> ---
> arch/arm64/boot/dts/rockchip/Makefile | 1 +
> .../dts/rockchip/rk3399-pinephone-pro.dts | 394 ++++++++++++++++++
> 2 files changed, 395 insertions(+)
> create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
>
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index ef79a672804a1..cb42e0a15808e 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-neo4.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-r4s.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-orangepi.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-pinebook-pro.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-pinephone-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-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> new file mode 100644
> index 0000000000000..f9f1dcd4f6494
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> @@ -0,0 +1,394 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2020 Martijn Braam <[email protected]>
> + * Copyright (c) 2021 Kamil Trzciński <[email protected]>
> + */
> +
> +/* PinePhone Pro datasheet:
> + * https://files.pine64.org/doc/PinePhonePro/PinephonePro-Schematic-V1.0-20211127.pdf
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/input/linux-event-codes.h>
> +#include "rk3399.dtsi"
> +#include "rk3399-opp.dtsi"
> +
> +/ {
> + model = "Pine64 PinePhonePro";
> + compatible = "pine64,pinephone-pro", "rockchip,rk3399";
> + chassis-type = "handset";
> +
> + aliases {
> + mmc0 = &sdio0;
> + mmc1 = &sdmmc;
> + mmc2 = &sdhci;
> + };
> +
> + chosen {
> + stdout-path = "serial2:115200n8";
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pwrbtn_pin>;
> +
> + power {
> + debounce-interval = <20>;
> + gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>;
> + label = "Power";
> + linux,code = <KEY_POWER>;
> + wakeup-source;
> + };
> + };
> +
> + vcc_sysin: vcc-sysin-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc_sysin";
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + vcc3v3_sys: vcc3v3-sys-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc3v3_sys";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + vin-supply = <&vcc_sysin>;
> + };
> +
> + vcca1v8_s3: vcc1v8-s3-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vcca1v8_s3";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + vin-supply = <&vcc3v3_sys>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +};
> +
> +&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>;
> +};
> +
> +&cpu_b0 {
> + cpu-supply = <&vdd_cpu_b>;
> +};
> +
> +&cpu_b1 {
> + cpu-supply = <&vdd_cpu_b>;
> +};
> +
> +&emmc_phy {
> + status = "okay";
> +};
> +
> +&i2c0 {
> + clock-frequency = <400000>;
> + i2c-scl-rising-time-ns = <168>;
> + i2c-scl-falling-time-ns = <4>;
> + status = "okay";
> +
> + rk818: pmic@1c {
> + compatible = "rockchip,rk818";
> + reg = <0x1c>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <RK_PC5 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_sysin>;
> + vcc2-supply = <&vcc_sysin>;
> + vcc3-supply = <&vcc_sysin>;
> + vcc4-supply = <&vcc_sysin>;
> + vcc6-supply = <&vcc_sysin>;
> + vcc7-supply = <&vcc3v3_sys>;
> + vcc8-supply = <&vcc_sysin>;
> + vcc9-supply = <&vcc3v3_sys>;
> +
> + regulators {
> + vdd_cpu_l: DCDC_REG1 {
> + 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;
> + };
> + };
> +
> + vdd_center: DCDC_REG2 {
> + regulator-name = "vdd_center";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <800000>;
> + 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: vcc_wl: 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;
> + };
> + };
> +
> + vcca3v0_codec: LDO_REG1 {
> + regulator-name = "vcca3v0_codec";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <3000000>;
> + regulator-max-microvolt = <3000000>;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> + };
> +
> + vcc3v0_touch: LDO_REG2 {
> + regulator-name = "vcc3v0_touch";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <3000000>;
> + regulator-max-microvolt = <3000000>;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +
> + vcca1v8_codec: LDO_REG3 {
> + regulator-name = "vcca1v8_codec";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> + };
> +
> + vcc_power_on: LDO_REG4 {
> + regulator-name = "vcc_power_on";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> + };
> +
> + vcc_3v0: LDO_REG5 {
> + 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;
> + };
> + };
> +
> + 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;
> + };
> + };
> +
> + vcc1v8_dvp: LDO_REG7 {
> + regulator-name = "vcc1v8_dvp";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> +
> + vcc3v3_s3: LDO_REG8 {
> + regulator-name = "vcc3v3_s3";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +
> + vccio_sd: LDO_REG9 {
> + regulator-name = "vccio_sd";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + vcc3v3_s0: SWITCH_REG {
> + regulator-name = "vcc3v3_s0";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-state-mem {
> + regulator-on-in-suspend;
> + };
> + };
> + };
> + };
> +
> + vdd_cpu_b: regulator@40 {
> + compatible = "silergy,syr827";
> + reg = <0x40>;
> + fcs,suspend-voltage-selector = <1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&vsel1_pin>;
> + regulator-name = "vdd_cpu_b";
> + regulator-min-microvolt = <712500>;
> + regulator-max-microvolt = <1500000>;
> + regulator-ramp-delay = <1000>;
> + regulator-always-on;
> + regulator-boot-on;
> +
> + 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_pin>;
> + regulator-name = "vdd_gpu";
> + regulator-min-microvolt = <712500>;
> + regulator-max-microvolt = <1500000>;
> + regulator-ramp-delay = <1000>;
> + regulator-always-on;
> + regulator-boot-on;
> +
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +};
> +
> +&cluster0_opp {
> + opp04 {
> + status = "disabled";
> + };
> +
> + opp05 {
> + status = "disabled";
> + };
> +};
> +
> +&cluster1_opp {
> + opp06 {
> + status = "disabled";
> + };
> +
> + opp07 {
> + status = "disabled";
> + };
> +};
> +
> +&io_domains {
> + status = "okay";
> +
> + bt656-supply = <&vcc1v8_dvp>;
> + audio-supply = <&vcca1v8_codec>;
> + sdmmc-supply = <&vccio_sd>;
> + gpio1830-supply = <&vcc_3v0>;
> +};
> +
> +&pmu_io_domains {
> + pmu1830-supply = <&vcc_1v8>;
> + status = "okay";
> +};
> +
> +&pinctrl {
> + buttons {
> + pwrbtn_pin: pwrbtn-pin {
> + rockchip,pins = <0 RK_PA5 RK_FUNC_GPIO &pcfg_pull_up>;
> + };
> + };
> +
> + pmic {
> + pmic_int_l: pmic-int-l {
> + rockchip,pins = <1 RK_PC5 RK_FUNC_GPIO &pcfg_pull_up>;
> + };
> +
> + vsel1_pin: vsel1-pin {
> + rockchip,pins = <1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_down>;
> + };
> +
> + vsel2_pin: vsel2-pin {
> + rockchip,pins = <1 RK_PB6 RK_FUNC_GPIO &pcfg_pull_down>;
> + };
> + };
> +};
> +
> +&sdmmc {
> + bus-width = <4>;
> + cap-sd-highspeed;
> + cd-gpios = <&gpio0 RK_PA7 GPIO_ACTIVE_LOW>;
> + disable-wp;
> + max-frequency = <150000000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd &sdmmc_bus4>;
> + vmmc-supply = <&vcc3v3_sys>;
> + vqmmc-supply = <&vccio_sd>;
> + status = "okay";
> +};
> +
> +&sdhci {
> + bus-width = <8>;
> + mmc-hs200-1_8v;
> + non-removable;
> + status = "okay";
> +};
> +
> +&tsadc {
> + rockchip,hw-tshut-mode = <1>;
> + rockchip,hw-tshut-polarity = <1>;
> + status = "okay";
> +};
> +
> +&uart2 {
> + status = "okay";
> +};
> --
> 2.37.1
>

2022-08-16 09:34:14

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

On 15/08/2022 15:30, Tom Fitzhenry wrote:
> +
> +/dts-v1/;
> +#include <dt-bindings/input/linux-event-codes.h>
> +#include "rk3399.dtsi"
> +#include "rk3399-opp.dtsi"
> +
> +/ {
> + model = "Pine64 PinePhonePro";
> + compatible = "pine64,pinephone-pro", "rockchip,rk3399";
> + chassis-type = "handset";
> +
> + aliases {
> + mmc0 = &sdio0;
> + mmc1 = &sdmmc;
> + mmc2 = &sdhci;
> + };
> +
> + chosen {
> + stdout-path = "serial2:115200n8";
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pwrbtn_pin>;
> +
> + power {

Still wrong. Do not ignore comments and test your DTS with dtbs_check.

Best regards,
Krzysztof

2022-08-18 03:14:36

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

Hi Tom,

thanks for getting the upstreaming of this DT going. Some comments below.

On Mon, Aug 15, 2022 at 10:30:04PM +1000, Tom Fitzhenry wrote:
> From: Martijn Braam <[email protected]>
>
> This is a basic DT containing regulators and UART, intended to be a
> base that myself and others can add additional nodes in future patches.
>
> Tested to work: booting from eMMC, output over UART.

You're also adding the SD controller here. Does it work as is? If so add it to
the commit description as well.

>
[..]
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> @@ -0,0 +1,394 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2020 Martijn Braam <[email protected]>
> + * Copyright (c) 2021 Kamil Trzciński <[email protected]>
> + */
> +
> +/* PinePhone Pro datasheet:

First comment line should be empty following the coding style [1]. Like you did
for the copyrights above.

[1] https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting

> + * https://files.pine64.org/doc/PinePhonePro/PinephonePro-Schematic-V1.0-20211127.pdf
> + */
[..]
> + vcc_sysin: vcc-sysin-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc_sysin";

This signal is called vcc_sys in the datasheet, so I suggest we keep that name
here. It's not everyday that we get a device with a publicly available datasheet
:^).

> + regulator-always-on;
> + regulator-boot-on;
> + };
[..]
> + rk818: pmic@1c {
> + compatible = "rockchip,rk818";
> + reg = <0x1c>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <RK_PC5 IRQ_TYPE_LEVEL_LOW>;
> + #clock-cells = <1>;
> + clock-output-names = "xin32k", "rk808-clkout2";

What about keeping the datasheet names here too? clk32kout1, clk32kout2

> + pinctrl-names = "default";
[..]
> + vcc_1v8: vcc_wl: DCDC_REG4 {

From the datasheet, vcc_wl is actually wired to vcc3v3_sys. But looks like
vcc_wl is only used for bluetooth and you're not enabling it yet anyway, so just
drop this extra label, and it can be added when bluetooth is added (or not, and
then the bluetooth supply just points directly to vcc3v3_sys).

> + regulator-name = "vcc_1v8";
[..]
> + vcc_power_on: LDO_REG4 {
> + regulator-name = "vcc_power_on";

The name on the datasheet for this one is rk818_pwr_on.

> + regulator-always-on;
[..]
> +&cluster0_opp {
> + opp04 {
> + status = "disabled";
> + };
> +
> + opp05 {
> + status = "disabled";
> + };
> +};

I saw the discussion on the previous version about using the rk3399-opp.dtsi
here, but the thing is, this OPP has greater values for the max voltage than the
maximum allowed on the OPP table you posted previously (for RK3399-T)...

Same thing for the GPU OPP.

> +
> +&cluster1_opp {
> + opp06 {
> + status = "disabled";
> + };

There's actually an opp06 node in the OPP for RK3399-T, only that the frequency
is slightly lower. Maybe you could keep it enabled but override the frequency?

Or given the above point about the max voltages, maybe it would be best to have
a separate OPP table after all?

> +
> + opp07 {
> + status = "disabled";
> + };
> +};
> +
> +&io_domains {
> + status = "okay";

Let's keep the status at the end of the node for consistency with the rest.

With these points addressed:

Reviewed-by: Nícolas F. R. A. Prado <[email protected]>

I'll also try to give this a test shortly.

Thanks,
Nícolas

> +
> + bt656-supply = <&vcc1v8_dvp>;
> + audio-supply = <&vcca1v8_codec>;
> + sdmmc-supply = <&vccio_sd>;
> + gpio1830-supply = <&vcc_3v0>;
> +};
[..]

2022-08-21 00:46:53

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

On Mon, Aug 15, 2022 at 10:30:04PM +1000, Tom Fitzhenry wrote:
> From: Martijn Braam <[email protected]>
>
> This is a basic DT containing regulators and UART, intended to be a
> base that myself and others can add additional nodes in future patches.
>
> Tested to work: booting from eMMC, output over UART.
>
> https://wiki.pine64.org/wiki/PinePhone_Pro
>
> This is derived from
> https://gitlab.com/pine64-org/linux/-/commit/261d3b5f8ac503f97da810986d1d6422430c8531
> with fixes from https://megous.com/git/linux.
>
> Signed-off-by: Martijn Braam <[email protected]>
> Co-developed-by: Kamil Trzciński <[email protected]>
> [no SoB, but Kamil is happy for this patch to be submitted]
> Co-developed-by: Ondrej Jirman <[email protected]>
> Signed-off-by: Ondrej Jirman <[email protected]>
> Co-developed-by: Tom Fitzhenry <[email protected]>
> Signed-off-by: Tom Fitzhenry <[email protected]>

Tested-by: Nícolas F. R. A. Prado <[email protected]>

Indeed booting from the SD card also works with this minimal DT. Also confirmed
that eMMC, thermal sensors, power button, regulators and UART are all working.

Thanks,
Nícolas

2022-08-21 09:31:31

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

Hi Nícolas,

On Wed, Aug 17, 2022 at 11:05:47PM -0400, Nícolas F. R. A. Prado wrote:
> Hi Tom,
>
> thanks for getting the upstreaming of this DT going. Some comments below.
>
> On Mon, Aug 15, 2022 at 10:30:04PM +1000, Tom Fitzhenry wrote:
> > From: Martijn Braam <[email protected]>
> >
> > This is a basic DT containing regulators and UART, intended to be a
> > base that myself and others can add additional nodes in future patches.
> >
> > Tested to work: booting from eMMC, output over UART.
>
> You're also adding the SD controller here. Does it work as is? If so add it to
> the commit description as well.
>
> >
> [..]
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > @@ -0,0 +1,394 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2020 Martijn Braam <[email protected]>
> > + * Copyright (c) 2021 Kamil Trzciński <[email protected]>
> > + */
> > +
> > +/* PinePhone Pro datasheet:
>
> First comment line should be empty following the coding style [1]. Like you did
> for the copyrights above.
>
> [1] https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting
>
> > + * https://files.pine64.org/doc/PinePhonePro/PinephonePro-Schematic-V1.0-20211127.pdf
> > + */
> [..]
> > + vcc_sysin: vcc-sysin-regulator {
> > + compatible = "regulator-fixed";
> > + regulator-name = "vcc_sysin";
>
> This signal is called vcc_sys in the datasheet, so I suggest we keep that name
> here. It's not everyday that we get a device with a publicly available datasheet
> :^).
>
> > + regulator-always-on;
> > + regulator-boot-on;
> > + };
> [..]
> > + rk818: pmic@1c {
> > + compatible = "rockchip,rk818";
> > + reg = <0x1c>;
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <RK_PC5 IRQ_TYPE_LEVEL_LOW>;
> > + #clock-cells = <1>;
> > + clock-output-names = "xin32k", "rk808-clkout2";
>
> What about keeping the datasheet names here too? clk32kout1, clk32kout2

Rather not. My guess is that trying to change the names will break the clock
tree, because xin32k is hardcoded in the rk3399 clk driver as parent for
mux_pll_p.

https://elixir.bootlin.com/linux/latest/source/drivers/clk/rockchip/clk-rk3399.c#L109

These names are not just a cosmetic thing.

>
> > + pinctrl-names = "default";
> [..]
> > + vcc_1v8: vcc_wl: DCDC_REG4 {
>
> From the datasheet, vcc_wl is actually wired to vcc3v3_sys. But looks like
> vcc_wl is only used for bluetooth and you're not enabling it yet anyway, so just
> drop this extra label, and it can be added when bluetooth is added (or not, and
> then the bluetooth supply just points directly to vcc3v3_sys).
>
> > + regulator-name = "vcc_1v8";
> [..]
> > + vcc_power_on: LDO_REG4 {
> > + regulator-name = "vcc_power_on";
>
> The name on the datasheet for this one is rk818_pwr_on.
>
> > + regulator-always-on;
> [..]
> > +&cluster0_opp {
> > + opp04 {
> > + status = "disabled";
> > + };
> > +
> > + opp05 {
> > + status = "disabled";
> > + };
> > +};
>
> I saw the discussion on the previous version about using the rk3399-opp.dtsi
> here, but the thing is, this OPP has greater values for the max voltage than the
> maximum allowed on the OPP table you posted previously (for RK3399-T)...

Max voltages in OPP table are irreleavant for Pinephone Pro DT, because the CPU
regulators in the PMIC are capable of setting the preferred voltage or very
close to it.

The actual min/max limits that are enforced by the kernel for the board are set
on the CPU regulator, which are actually wrongly set in this DT. See:

https://dl.radxa.com/rockpi4/docs/hw/datasheets/Rockchip%20RK3399-T%20Datasheet%20V1.0-20210818.pdf

+ vdd_cpu_l: DCDC_REG1 {
+ regulator-name = "vdd_cpu_l";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <750000>;
+ regulator-max-microvolt = <1350000>;

Should be 975000 (or at most 1300000 to fit within absmax)

+ regulator-ramp-delay = <6001>;
+ 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_pin>;
+ regulator-name = "vdd_cpu_b";
+ regulator-min-microvolt = <712500>;
+ regulator-max-microvolt = <1500000>;
+ regulator-ramp-delay = <1000>;
+ regulator-always-on;
+ regulator-boot-on;
+
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };

Should be 1150000 (or at most 1300000 to fit within absmax)

+ vdd_gpu: regulator@41 {
+ compatible = "silergy,syr828";
+ reg = <0x41>;
+ fcs,suspend-voltage-selector = <1>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&vsel2_pin>;
+ regulator-name = "vdd_gpu";
+ regulator-min-microvolt = <712500>;
+ regulator-max-microvolt = <1500000>;

Should be 975000 (or at most 1300000 to fit within absmax)

+ regulator-ramp-delay = <1000>;
+ regulator-always-on;
+ regulator-boot-on;
+
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };

The max values in OPP table also still fit within abs. max values for RK3399-T,
so they are safe in any case.

> Same thing for the GPU OPP.
>
> > +
> > +&cluster1_opp {
> > + opp06 {
> > + status = "disabled";
> > + };
>
> There's actually an opp06 node in the OPP for RK3399-T, only that the frequency
> is slightly lower. Maybe you could keep it enabled but override the frequency?
>
> Or given the above point about the max voltages, maybe it would be best to have
> a separate OPP table after all?

No separate OPP table, please, unless absolutely necessary.

kind regards,
Ondrej

> > +
> > + opp07 {
> > + status = "disabled";
> > + };
> > +};
> > +
> > +&io_domains {
> > + status = "okay";
>
> Let's keep the status at the end of the node for consistency with the rest.
>
> With these points addressed:
>
> Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
>
> I'll also try to give this a test shortly.
>
> Thanks,
> Nícolas
>
> > +
> > + bt656-supply = <&vcc1v8_dvp>;
> > + audio-supply = <&vcca1v8_codec>;
> > + sdmmc-supply = <&vccio_sd>;
> > + gpio1830-supply = <&vcc_3v0>;
> > +};
> [..]

2022-08-21 10:04:06

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

On Sun, Aug 21, 2022 at 11:00:05AM +0200, megi xff wrote:

[...]

> The actual min/max limits that are enforced by the kernel for the board are set
> on the CPU regulator, which are actually wrongly set in this DT. See:
>
> https://dl.radxa.com/rockpi4/docs/hw/datasheets/Rockchip%20RK3399-T%20Datasheet%20V1.0-20210818.pdf

The minimums on the regulators also seem out of range in the datasheet, for that matter.

regards,
o.

2022-08-22 00:34:23

by Tom Fitzhenry

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

On 18/8/22 13:05, Nícolas F. R. A. Prado wrote:

> thanks for getting the upstreaming of this DT going. Some comments below.

No worries, thank you for your review!

> You're also adding the SD controller here. Does it work as is? If so add it to
> the commit description as well.
I will note this in v4.
>> +/* PinePhone Pro datasheet:
> First comment line should be empty following the coding style [1]. Like you did
> for the copyrights above.
>
> [1] https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting

I will do this in v4.

> This signal is called vcc_sys in the datasheet, so I suggest we keep that name
> here. It's not everyday that we get a device with a publicly available datasheet
> :^).

Indeed! :) I will do this in v4.

> + rk818: pmic@1c {
>> + compatible = "rockchip,rk818";
>> + reg = <0x1c>;
>> + interrupt-parent = <&gpio1>;
>> + interrupts = <RK_PC5 IRQ_TYPE_LEVEL_LOW>;
>> + #clock-cells = <1>;
>> + clock-output-names = "xin32k", "rk808-clkout2";
> What about keeping the datasheet names here too? clk32kout1, clk32kout2
Per Megi's response, I'll stick with the current names.
>> + vcc_1v8: vcc_wl: DCDC_REG4 {
> From the datasheet, vcc_wl is actually wired to vcc3v3_sys. But looks like
> vcc_wl is only used for bluetooth and you're not enabling it yet anyway, so just
> drop this extra label, and it can be added when bluetooth is added (or not, and
> then the bluetooth supply just points directly to vcc3v3_sys).
Good catch, I will remove the vcc_wl label.
>> + vcc_power_on: LDO_REG4 {
>> + regulator-name = "vcc_power_on";
> The name on the datasheet for this one is rk818_pwr_on.
I will use the name rk818_pwr_on in v4.
>> +
>> +&cluster1_opp {
>> + opp06 {
>> + status = "disabled";
>> + };
> There's actually an opp06 node in the OPP for RK3399-T, only that the frequency
> is slightly lower. Maybe you could keep it enabled but override the frequency?
>
> Or given the above point about the max voltages, maybe it would be best to have
> a separate OPP table after all?
Per Megi's response/rationale, I'll keep the existing table, but
re-introduce cluster1_opp/opp06 with updated frequency/voltage, aligned
with the RK3399-T datasheet.
>> +
>> + opp07 {
>> + status = "disabled";
>> + };
>> +};
>> +
>> +&io_domains {
>> + status = "okay";
> Let's keep the status at the end of the node for consistency with the rest.
I will do this in v4.

2022-08-22 00:53:44

by Tom Fitzhenry

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

On 21/8/22 19:00, Ondřej Jirman wrote:

> Max voltages in OPP table are irreleavant for Pinephone Pro DT, because the CPU
> regulators in the PMIC are capable of setting the preferred voltage or very
> close to it.
>
> The actual min/max limits that are enforced by the kernel for the board are set
> on the CPU regulator, which are actually wrongly set in this DT. See:
>
> https://dl.radxa.com/rockpi4/docs/hw/datasheets/Rockchip%20RK3399-T%20Datasheet%20V1.0-20210818.pdf

Good spot, I will fix these in v4.

2022-08-22 01:15:46

by Tom Fitzhenry

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

On 16/8/22 17:52, Krzysztof Kozlowski wrote:

> Still wrong.

My bad, I will fix this in v4.

> test your DTS with dtbs_check.
Will do.

2022-08-22 01:17:03

by Tom Fitzhenry

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] arm64: dts: rockchip: Add initial support for Pine64 PinePhone Pro

On Sun, 21 Aug 2022, at 12:11 AM, Nícolas F. R. A. Prado wrote:
> Indeed booting from the SD card also works with this minimal DT. Also confirmed
> that eMMC, thermal sensors, power button, regulators and UART are all working.

Thanks for the testing! I will include your Tested-by in v4.

2022-09-04 17:25:31

by Heiko Stuebner

[permalink] [raw]
Subject: Re: (subset) [PATCH v3 0/2] Add support for the Pine64 PinePhone Pro phone

On Mon, 15 Aug 2022 22:30:02 +1000, Tom Fitzhenry wrote:
> PinePhone Pro is a RK3399 based phone produced by Pine64.
>
> Add a basic DTS for it. This is a working base that will allow myself and
> others to add more nodes.
>
> Changes since v2:
> * Used rk3399-opp.dtsi (+ disabled) rather than introducing rk3399-t-opp.dtsi
> * Removed superfluous comments.
> * Followed DT Generic Names Recommendation
> * Set 'From:' of DTS patch to be Martijn, the primary contributor to the DT.
>
> [...]

Applied, thanks!

[1/2] dt-bindings: arm: rockchip: Add PinePhone Pro bindings
commit: 23c5f10981474b44ce1726e3753cbb7ac20d6548

applied together with v5 of the actual board dts patch
which was missing the binding.


Best regards,
--
Heiko Stuebner <[email protected]>