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

Vasily Khoruzhick anarsoul at gmail.com
Thu Oct 18 04:48:59 UTC 2018 

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 at 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 at aosc.xyz>
> > + * Copyright (C) 2018 Vasily Khoruzhick <anarsoul at 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 at 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 at 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 at 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 at 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

More information about the U-Boot mailing list