Re: [U-Boot] [PATCH v2 5/5] sunxi: DT: add support for Pinebook

On Wednesday, October 17, 2018 5:23:04 PM PDT André Przywara wrote:

On 10/17/18 6:09 AM, Vasily Khoruzhick wrote:

Hi,

...

Pinebook is a laptop produced by Pine64, with USB-connected keyboard, USB-connected touchpad and an eDP LCD panel connected via a RGB-eDP bridge from Analogix.

Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com

arch/arm/dts/Makefile | 3 +- arch/arm/dts/sun50i-a64-pinebook.dts | 306 +++++++++++++++++++++++++++ configs/pinebook_defconfig | 27 +++ 3 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 arch/arm/dts/sun50i-a64-pinebook.dts create mode 100644 configs/pinebook_defconfig

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index dfe9335a04..ccb93f1e46 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -404,7 +404,8 @@ dtb-$(CONFIG_MACH_SUN50I) += \

sun50i-a64-orangepi-win.dtb \ sun50i-a64-pine64-plus.dtb \ sun50i-a64-pine64.dtb \

• sun50i-a64-sopine-baseboard.dtb

• sun50i-a64-sopine-baseboard.dtb \
• sun50i-a64-pinebook.dtb

dtb-$(CONFIG_MACH_SUN9I) += \

sun9i-a80-optimus.dtb \ sun9i-a80-cubieboard4.dtb \

diff --git a/arch/arm/dts/sun50i-a64-pinebook.dts b/arch/arm/dts/sun50i-a64-pinebook.dts new file mode 100644 index 0000000000..5d111fab64 --- /dev/null +++ b/arch/arm/dts/sun50i-a64-pinebook.dts @@ -0,0 +1,306 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/*

• • Copyright (C) 2017 Icenowy Zheng icenowy@aosc.xyz
• • Copyright (C) 2018 Vasily Khoruzhick anarsoul@gmail.com
• */

+/dts-v1/;

+#include "sun50i-a64.dtsi"

+#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/input/input.h> +#include <dt-bindings/pwm/pwm.h>

+/ {

• model = "Pinebook";
• compatible = "pine64,pinebook", "allwinner,sun50i-a64";
• aliases {

• serial0 = &uart0;
  

• ethernet0 = &rtl8723cs;
  

• };
• vdd_bl: regulator@0 {

• compatible = "regulator-fixed";
  

• regulator-name = "bl-3v3";
  

• regulator-min-microvolt = <3300000>;
  

• regulator-max-microvolt = <3300000>;
  

• gpio = <&pio 7 6 GPIO_ACTIVE_HIGH>; /* PH6 */
  

• enable-active-high;
  

• };
• backlight: backlight {

• compatible = "pwm-backlight";
  

• pwms = <&pwm 0 50000 0>;
  

• brightness-levels = <0 5 10 15 20 30 40 55 70 85 100>;
  

• default-brightness-level = <2>;
  

• enable-gpios = <&pio 3 23 GPIO_ACTIVE_HIGH>; /* PD23 */
  

• power-supply = <&vdd_bl>;
  

• };
• chosen {

• stdout-path = "serial0:115200n8";
  

• framebuffer-lcd {
  

• 	panel-supply = <&reg_dc1sw>;
  

• 	dvdd25-supply = <&reg_dldo2>;
  

• 	dvdd12-supply = <&reg_fldo1>;
  

• };
  

• };
• gpio_keys {

• compatible = "gpio-keys";
  

• lid_switch {
  

• 	label = "Lid Switch";
  

• 	gpios = <&r_pio 0 12 GPIO_ACTIVE_LOW>; /* PL12 */
  

• 	linux,input-type = <EV_SW>;
  

• 	linux,code = <SW_LID>;
  

• 	linux,can-disable;
  

• 	wakeup-source;
  

• };
  

• };
• reg_vcc3v3: vcc3v3 {

• compatible = "regulator-fixed";
  

• regulator-name = "vcc3v3";
  

• regulator-min-microvolt = <3300000>;
  

• regulator-max-microvolt = <3300000>;
  

• };
• wifi_pwrseq: wifi_pwrseq {

• compatible = "mmc-pwrseq-simple";
  

• reset-gpios = <&r_pio 0 2 GPIO_ACTIVE_LOW>; /* PL2 */
  

• };
• speaker_amp: speaker_amp {

• compatible = "simple-audio-amplifier";
  

• enable-gpios = <&pio 7 7 GPIO_ACTIVE_HIGH>;
  

• };

I see that this is the only sound related node not dependent on any .dtsi nodes, but how useful is it to have it in here on its own? Shouldn't we wait until all sound nodes are agreed upon and merged?

I'll drop it for v3.

...

+};

+&ehci0 {

• phys = <&usbphy 0>;
• phy-names = "usb";
• status = "okay";

+};

+&ehci1 {

• status = "okay";

+};

+&mmc0 {

• pinctrl-names = "default";
• pinctrl-0 = <&mmc0_pins>;
• vmmc-supply = <&reg_dcdc1>;
• cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>;
• cd-inverted;
• disable-wp;
• bus-width = <4>;
• status = "okay";

+};

+&mmc1 {

• pinctrl-names = "default";
• pinctrl-0 = <&mmc1_pins>;
• vmmc-supply = <&reg_dldo4>;
• vqmmc-supply = <&reg_eldo1>;
• mmc-pwrseq = <&wifi_pwrseq>;
• bus-width = <4>;
• non-removable;
• status = "okay";
• rtl8723cs: wifi@1 {

• reg = <1>;
  

• };

+};

+&mmc2 {

• pinctrl-names = "default";
• pinctrl-0 = <&mmc2_pins>;
• vmmc-supply = <&reg_dcdc1>;
• vqmmc-supply = <&reg_eldo1>;
• bus-width = <8>;
• non-removable;
• cap-mmc-hw-reset;
• mmc-hs200-1_8v;
• status = "okay";

+};

+&ohci0 {

• phys = <&usbphy 0>;
• phy-names = "usb";
• status = "okay";

+};

+&ohci1 {

• status = "okay";

+};

+&pwm {

• status = "okay";

+};

+&r_rsb {

• status = "okay";
• axp803: pmic@3a3 {

• compatible = "x-powers,axp803";
  

• reg = <0x3a3>;
  

• interrupt-parent = <&r_intc>;
  

• interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
  

• };

+};

+/* The ANX6345 eDP-bridge is on r_i2c */ +&r_i2c {

• clock-frequency = <100000>;
• pinctrl-names = "default";
• pinctrl-0 = <&r_i2c_pins_a>;
• status = "okay";

Nit: please add an empty line here, to separate the slave section from the master.

I'll move it to sun50i-a64-pinebook-u-boot.dtsi for v3 as Jernej suggested, so we can drop it when anx6345 merges into linux.

...

• anx6345: edp-bridge@38 {

• compatible = "analogix,anx6345";
  

• reg = <0x38>;
  

• reset-gpios = <&pio 3 24 GPIO_ACTIVE_LOW>; /* PD24 */
  

• status = "okay";
  

• };

Do you have any plans on upstreaming this binding?

Icenowy has plans to merge it.

...

+};

+#include "axp803.dtsi"

+&reg_aldo1 {

• regulator-min-microvolt = <2800000>;
• regulator-max-microvolt = <2800000>;
• regulator-name = "vcc-csi";

+};

+&reg_aldo2 {

• regulator-always-on;
• regulator-min-microvolt = <1800000>;
• regulator-max-microvolt = <3300000>;
• regulator-name = "vcc-pl";

+};

+&reg_aldo3 {

• regulator-always-on;
• regulator-min-microvolt = <2700000>;
• regulator-max-microvolt = <3300000>;
• regulator-name = "vcc-pll-avcc";

+};

+&reg_dc1sw {

• regulator-name = "vcc-lcd";

+};

+&reg_dcdc1 {

• regulator-always-on;
• regulator-min-microvolt = <3300000>;
• regulator-max-microvolt = <3300000>;
• regulator-name = "vcc-3v3";

+};

+&reg_dcdc2 {

• regulator-always-on;
• regulator-min-microvolt = <1000000>;
• regulator-max-microvolt = <1300000>;
• regulator-name = "vdd-cpux";

+};

+/* DCDC3 is polyphased with DCDC2 */

+&reg_dcdc5 {

• regulator-always-on;
• regulator-min-microvolt = <1200000>;
• regulator-max-microvolt = <1200000>;
• regulator-name = "vcc-dram";

+};

+&reg_dcdc6 {

• regulator-always-on;
• regulator-min-microvolt = <1100000>;
• regulator-max-microvolt = <1100000>;
• regulator-name = "vdd-sys";

+};

+&reg_dldo1 {

• regulator-min-microvolt = <3300000>;
• regulator-max-microvolt = <3300000>;
• regulator-name = "vcc-hdmi";

+};

+&reg_dldo2 {

• regulator-min-microvolt = <2500000>;
• regulator-max-microvolt = <2500000>;
• regulator-name = "vcc-edp";

+};

+&reg_dldo3 {

• regulator-min-microvolt = <3300000>;
• regulator-max-microvolt = <3300000>;
• regulator-name = "avdd-csi";

+};

+&reg_dldo4 {

• regulator-min-microvolt = <3300000>;
• regulator-max-microvolt = <3300000>;
• regulator-name = "vcc-wifi";

+};

+&reg_eldo1 {

• regulator-always-on;
• regulator-min-microvolt = <1800000>;
• regulator-max-microvolt = <1800000>;
• regulator-name = "cpvdd";

+};

+&reg_eldo3 {

• regulator-min-microvolt = <1800000>;
• regulator-max-microvolt = <1800000>;
• regulator-name = "vdd-1v8-csi";

+};

+&reg_fldo1 {

• regulator-min-microvolt = <1200000>;
• regulator-max-microvolt = <1200000>;
• regulator-name = "vcc-1v2-hsic";

+};

+&reg_fldo2 {

• regulator-always-on;
• regulator-min-microvolt = <1100000>;
• regulator-max-microvolt = <1100000>;
• regulator-name = "vdd-cpus";

+};

+&reg_ldo_io0 {

• regulator-min-microvolt = <3300000>;
• regulator-max-microvolt = <3300000>;
• regulator-name = "vcc-usb";
• status = "okay";

+};

+&reg_rtc_ldo {

• regulator-name = "vcc-rtc";

+};

+&simplefb_hdmi {

• vcc-hdmi-supply = <&reg_dldo1>;

+};

+&uart0 {

• pinctrl-names = "default";
• pinctrl-0 = <&uart0_pins_a>;
• status = "okay";

+};

+&usb_otg {

• dr_mode = "host";

+};

+&usbphy {

• usb0_vbus-supply = <&reg_ldo_io0>;
• usb1_vbus-supply = <&reg_ldo_io0>;
• status = "okay";

+}; diff --git a/configs/pinebook_defconfig b/configs/pinebook_defconfig new file mode 100644 index 0000000000..f5190d96d2 --- /dev/null +++ b/configs/pinebook_defconfig @@ -0,0 +1,27 @@ +CONFIG_ARM=y +CONFIG_ARCH_SUNXI=y +CONFIG_SPL=y +CONFIG_MACH_SUN50I=y +CONFIG_RESERVE_ALLWINNER_BOOT0_HEADER=y

I think you can drop this.

OK

...

+CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y +CONFIG_DRAM_CLK=552 +CONFIG_DRAM_ZQ=3881949 +CONFIG_MMC_SUNXI_SLOT_EXTRA=2 +CONFIG_R_I2C_ENABLE=y +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set +# CONFIG_SPL_LEGACY_IMAGE_SUPPORT is not set

Why these two? I see that they save 219 bytes of the SPL, but the image size is still way below the limit. So I'd prefer to not deviate from the other boards here.

OK, I'll drop these.

...

+# CONFIG_CMD_FLASH is not set +# CONFIG_SPL_DOS_PARTITION is not set +# CONFIG_SPL_EFI_PARTITION is not set +CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pinebook" +# CONFIG_MMC_VERBOSE is not set

Why this one? Doesn't seem to affect SPL size?

OK, will keep it.

Cheers, Andre.

...

+CONFIG_DM_REGULATOR=y +CONFIG_DM_REGULATOR_FIXED=y +CONFIG_DM_PWM=y +CONFIG_PWM_SUNXI=y +CONFIG_USB_EHCI_HCD=y +CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE=y +# CONFIG_USB_GADGET is not set +CONFIG_VIDEO_BRIDGE=y +CONFIG_VIDEO_BRIDGE_ANALOGIX_ANX6345=y