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@xxxxxxxxx> > > > > > + 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 > >
