| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <477e2602-d740-5954-5573-87db5fd83888@manjaro.org>
Date: Tue, 21 Jan 2020 15:13:06 +0100
From: Tobias Schramm <t.schramm@...jaro.org>
To: Emmanuel Vadot <manu@...ebsd.org>, heiko@...ech.de,
pbrobinson@...il.com
Cc: robh+dt@...nel.org, mark.rutland@....com, andy.yan@...k-chips.com,
robin.murphy@....com, nick@...das.com, kever.yang@...k-chips.com,
m.reichl@...etechno.de, aballier@...too.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: Add initial support for
Pinebook Pro
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@...il.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
Powered by blists - more mailing lists