Re: [PATCH 3/3] arm64: dts: allwinner: Add initial support for Pine64 PinePhone

From: Maxime Ripard
Date: Mon Feb 24 2020 - 06:00:33 EST


Hi,

On Sun, Feb 23, 2020 at 06:29:16PM +0100, Ondrej Jirman wrote:
> At them moment PinePhone comes in two slightly incompatible variants:
>
> - 1.0: Early Developer Batch
> - 1.1: Braveheart Batch
>
> There will be at least one more incompatible variant in the very near
> future, so let's start by sharing the dtsi among multiple variants,
> right away, even though the HW description doesn't yet include the
> different bits.
>
> This is a basic DT that includes only features that are already
> supported by mainline drivers.

What are those incompatibilities? It's not really obvious from your
patch.

> Co-developed-by: Samuel Holland <samuel@xxxxxxxxxxxx>
> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx>
> Co-developed-by: Martijn Braam <martijn@xxxxxxxxx>
> Signed-off-by: Martijn Braam <martijn@xxxxxxxxx>
> Co-developed-by: Luca Weiss <luca@xxxxxxxxx>
> Signed-off-by: Luca Weiss <luca@xxxxxxxxx>
> Signed-off-by: Bhushan Shah <bshah@xxxxxxx>
> Signed-off-by: Icenowy Zheng <icenowy@xxxxxxx>
> Signed-off-by: Ondrej Jirman <megous@xxxxxxxxxx>
> ---
> arch/arm64/boot/dts/allwinner/Makefile | 2 +
> .../allwinner/sun50i-a64-pinephone-1.0.dts | 11 +
> .../allwinner/sun50i-a64-pinephone-1.1.dts | 11 +
> .../dts/allwinner/sun50i-a64-pinephone.dtsi | 385 ++++++++++++++++++
> 4 files changed, 409 insertions(+)
> create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.0.dts
> create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.1.dts
> create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
>
> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> index cf4f78617c3f3..79ca263672c38 100644
> --- a/arch/arm64/boot/dts/allwinner/Makefile
> +++ b/arch/arm64/boot/dts/allwinner/Makefile
> @@ -9,6 +9,8 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-orangepi-win.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pine64-lts.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pine64-plus.dtb sun50i-a64-pine64.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pinebook.dtb
> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pinephone-1.0.dtb
> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pinephone-1.1.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-sopine-baseboard.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-teres-i.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-bananapi-m2-plus.dtb
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.0.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.0.dts
> new file mode 100644
> index 0000000000000..0c42272106afa
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.0.dts
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +// Copyright (C) 2020 Ondrej Jirman <megous@xxxxxxxxxx>

Given the list of authors, surely you're not the sole copyright owner
here?

> +/dts-v1/;
> +
> +#include "sun50i-a64-pinephone.dtsi"
> +
> +/ {
> + model = "Pine64 PinePhone Developer Batch (1.0)";
> + compatible = "pine64,pinephone-1.0", "allwinner,sun50i-a64";
> +};
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.1.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.1.dts
> new file mode 100644
> index 0000000000000..06a775c41664b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-1.1.dts
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +// Copyright (C) 2020 Ondrej Jirman <megous@xxxxxxxxxx>
> +
> +/dts-v1/;
> +
> +#include "sun50i-a64-pinephone.dtsi"
> +
> +/ {
> + model = "Pine64 PinePhone Braveheart (1.1)";
> + compatible = "pine64,pinephone-1.1", "allwinner,sun50i-a64";
> +};
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> new file mode 100644
> index 0000000000000..d0cf21d82c9e9
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> @@ -0,0 +1,385 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +// Copyright (C) 2019 Icenowy Zheng <icenowy@xxxxxxxx>
> +// Copyright (C) 2020 Ondrej Jirman <megous@xxxxxxxxxx>
> +
> +#include "sun50i-a64.dtsi"
> +#include "sun50i-a64-cpu-opp.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pwm/pwm.h>
> +
> +/ {
> + aliases {
> + serial0 = &uart0;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + blue {
> + function = LED_FUNCTION_INDICATOR;
> + function-enumerator = <1>;
> + color = <LED_COLOR_ID_BLUE>;
> + gpios = <&pio 3 20 GPIO_ACTIVE_HIGH>; /* PD20 */
> + };
> +
> + green {
> + function = LED_FUNCTION_INDICATOR;
> + function-enumerator = <2>;
> + color = <LED_COLOR_ID_GREEN>;
> + gpios = <&pio 3 18 GPIO_ACTIVE_HIGH>; /* PD18 */
> + };
> +
> + red {
> + function = LED_FUNCTION_INDICATOR;
> + function-enumerator = <3>;
> + color = <LED_COLOR_ID_RED>;
> + gpios = <&pio 3 19 GPIO_ACTIVE_HIGH>; /* PD19 */
> + };
> + };

LEDs should be named using the $color:$board:$usage pattern

> +
> + speaker_amp: audio-amplifier {
> + compatible = "simple-audio-amplifier";
> + enable-gpios = <&pio 2 7 GPIO_ACTIVE_HIGH>; /* PC7 */
> + sound-name-prefix = "Speaker Amp";
> + };
> +
> + vibrator {
> + compatible = "gpio-vibrator";
> + enable-gpios = <&pio 3 2 GPIO_ACTIVE_HIGH>; /* PD2 */
> + vcc-supply = <&reg_dcdc1>;
> + };
> +};
> +
> +&codec {
> + status = "okay";
> +};
> +
> +&codec_analog {
> + cpvdd-supply = <&reg_eldo1>;
> + status = "okay";
> +};
> +
> +&cpu0 {
> + cpu-supply = <&reg_dcdc2>;
> +};
> +
> +&cpu1 {
> + cpu-supply = <&reg_dcdc2>;
> +};
> +
> +&cpu2 {
> + cpu-supply = <&reg_dcdc2>;
> +};
> +
> +&cpu3 {
> + cpu-supply = <&reg_dcdc2>;
> +};
> +
> +&dai {
> + status = "okay";
> +};
> +
> +&ehci0 {
> + status = "okay";
> +};
> +
> +&ehci1 {
> + status = "okay";
> +};
> +
> +&i2c1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pins>;

That's the default

> + status = "okay";
> +
> + /* Magnetometer */
> + lis3mdl@1e {
> + compatible = "st,lis3mdl-magn";
> + reg = <0x1e>;
> + vdd-supply = <&reg_dldo1>;
> + vddio-supply = <&reg_dldo1>;
> + };
> +
> + /* Accelerometer/gyroscope */
> + mpu6050@68 {
> + compatible = "invensense,mpu6050";
> + reg = <0x68>;
> + interrupt-parent = <&pio>;
> + interrupts = <7 5 IRQ_TYPE_EDGE_RISING>; /* PH5 */
> + vdd-supply = <&reg_dldo1>;
> + vddio-supply = <&reg_dldo1>;
> + };
> +};
> +
> +/* Connected to pogo pins */
> +&i2c2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c2_pins>;

That's the default as well

> + status = "okay";
> +};

And I'm not sure what the pogo pins are?

> +
> +&lradc {
> + vref-supply = <&reg_aldo3>;
> + status = "okay";
> +
> + button-200 {
> + label = "Volume Up";
> + linux,code = <KEY_VOLUMEUP>;
> + channel = <0>;
> + voltage = <200000>;
> + };
> +
> + button-400 {
> + label = "Volume Down";
> + linux,code = <KEY_VOLUMEDOWN>;
> + channel = <0>;
> + voltage = <400000>;
> + };
> +};
> +
> +&mmc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc0_pins>;

That's the default

> + vmmc-supply = <&reg_dcdc1>;
> + vqmmc-supply = <&reg_dcdc1>;
> + cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> + disable-wp;
> + bus-width = <4>;
> + status = "okay";
> +};
> +
> +&mmc2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc2_pins>;

Ditto

> + vmmc-supply = <&reg_dcdc1>;
> + vqmmc-supply = <&reg_dcdc1>;
> + bus-width = <8>;
> + non-removable;
> + cap-mmc-hw-reset;
> + status = "okay";
> +};
> +
> +&ohci0 {
> + status = "okay";
> +};
> +
> +&ohci1 {
> + status = "okay";
> +};
> +
> +&pio {
> + vcc-pb-supply = <&reg_dcdc1>;
> + vcc-pc-supply = <&reg_dcdc1>;
> + vcc-pd-supply = <&reg_dcdc1>;
> + vcc-pe-supply = <&reg_aldo1>;
> + vcc-pf-supply = <&reg_dcdc1>;
> + vcc-pg-supply = <&reg_dldo4>;
> + vcc-ph-supply = <&reg_dcdc1>;
> +};
> +
> +&r_pio {
> + /*
> + * FIXME: We can't add that supply for now since it would
> + * create a circular dependency between pinctrl, the regulator
> + * and the RSB Bus.
> + *
> + * vcc-pl-supply = <&reg_aldo2>;
> + */
> +};
> +
> +&r_rsb {
> + status = "okay";
> +
> + axp803: pmic@3a3 {
> + compatible = "x-powers,axp803";
> + reg = <0x3a3>;
> + interrupt-parent = <&r_intc>;
> + interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> + };
> +};
> +
> +#include "axp803.dtsi"
> +
> +&ac_power_supply {
> + status = "okay";
> +};
> +
> +&battery_power_supply {
> + status = "okay";
> +};
> +
> +&reg_aldo1 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-name = "dovdd-csi";
> +};
> +
> +&reg_aldo2 {
> + regulator-always-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-name = "vcc-pl";
> +};
> +
> +&reg_aldo3 {
> + regulator-always-on;
> + regulator-min-microvolt = <2700000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-pll-avcc";
> +};
> +
> +&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-dsi-sensor";
> +};
> +
> +&reg_dldo2 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-name = "vcc-mipi-io";
> +};
> +
> +&reg_dldo3 {
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> + regulator-name = "avdd-csi";
> +};
> +
> +&reg_dldo4 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-wifi-io";
> +};
> +
> +&reg_eldo1 {
> + regulator-always-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-name = "vcc-lpddr";
> +};
> +
> +&reg_eldo3 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-name = "dvdd-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-lcd-ctp-stk";
> + status = "okay";
> +};
> +
> +&reg_ldo_io1 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-name = "vcc-1v8-typec";
> + status = "okay";
> +};
> +
> +&reg_rtc_ldo {
> + regulator-name = "vcc-rtc";
> +};
> +
> +&sound {
> + status = "okay";
> + simple-audio-card,aux-devs = <&codec_analog>, <&speaker_amp>;
> + simple-audio-card,widgets = "Microphone", "Headset Microphone",
> + "Microphone", "Internal Microphone",
> + "Headphone", "Headphone Jack",
> + "Speaker", "Internal Earpiece",
> + "Speaker", "Internal Speaker";
> + simple-audio-card,routing =
> + "Headphone Jack", "HP",
> + "Internal Earpiece", "EARPIECE",
> + "Internal Speaker", "Speaker Amp OUTL",
> + "Internal Speaker", "Speaker Amp OUTR",
> + "Speaker Amp INL", "LINEOUT",
> + "Speaker Amp INR", "LINEOUT",
> + "Left DAC", "AIF1 Slot 0 Left",
> + "Right DAC", "AIF1 Slot 0 Right",
> + "AIF1 Slot 0 Left ADC", "Left ADC",
> + "AIF1 Slot 0 Right ADC", "Right ADC",
> + "Internal Microphone", "MBIAS",
> + "MIC1", "Internal Microphone",
> + "Headset Microphone", "HBIAS",
> + "MIC2", "Headset Microphone";
> +};
> +
> +&uart0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart0_pb_pins>;
> + status = "okay";
> +};
> +
> +/* Connected to the modem */
> +&uart3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart3_pins>;

That's the default too, and I guess you'd need hardware flow control
here?

> + status = "okay";
> +};
> +
> +&usb_otg {
> + dr_mode = "peripheral";
> + status = "okay";
> +};
> +
> +&usb_power_supply {
> + status = "okay";
> +};
> +
> +&usbphy {
> + status = "okay";
> +};
> --
> 2.25.1
>

Looks good otherwise, thanks!
Maxime

Attachment: signature.asc
Description: PGP signature