Re: [PATCH] arm64: dts: marvell: add DT for ESPRESSObin-Ultra

From: Andre Heider
Date: Mon Oct 12 2020 - 03:08:56 EST


Hi Vladimir,

a few comments, all by just comparing this device tree to the other espressobin files.

On 09/10/2020 17:27, Vladimir Vid wrote:
This adds support for ESPRESSObin-Ultra from Globalscale.

Specifications are similar to the base ESPRESSObin board, with main
difference being being WAN port with PoE capability and 2 additional ethernet ports.

Full specifications:

1x Marvell 64 bit Dual Core ARM A53 Armada 3700 SOC clocked up to 1.2Ghz
1x Topaz 6341 Networking Switch
1GB DDR4
8GB eMMC
1x WAN with 30W POE
4x Gb LAN
1x RTC Clock and battery
1x DC Jack
1x USB 3.0 Type A
1x USB 2.0 Type A
1x SIM NanoSIM card Slot

This one isn't enabled, is it not working?

1x Power Button
4x LED
1x Reset button
1x microUSB for UART
1x M.2 2280 slot for memory
1x 2x2 802.11ac Wi-Fi
1x MiniPCIE slot for Wi-Fi (PCIe interface)

Signed-off-by: Vladimir Vid <vladimir.vid@xxxxxxxxxx>
---
arch/arm64/boot/dts/marvell/Makefile | 1 +
.../marvell/armada-3720-espressobin-ultra.dts | 242 ++++++++++++++++++
2 files changed, 243 insertions(+)
create mode 100644 arch/arm64/boot/dts/marvell/armada-3720-espressobin-ultra.dts

diff --git a/arch/arm64/boot/dts/marvell/Makefile b/arch/arm64/boot/dts/marvell/Makefile
index 3e5f2e7a040c..094f451fdd1d 100644
--- a/arch/arm64/boot/dts/marvell/Makefile
+++ b/arch/arm64/boot/dts/marvell/Makefile
@@ -3,6 +3,7 @@
dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-db.dtb
dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin.dtb
dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin-emmc.dtb
+dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin-ultra.dtb
dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin-v7.dtb
dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin-v7-emmc.dtb
dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-turris-mox.dtb
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin-ultra.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin-ultra.dts
new file mode 100644
index 000000000000..25d10b628e77
--- /dev/null
+++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin-ultra.dts
@@ -0,0 +1,242 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Device Tree file for ESPRESSObin-Ultra board.
+ * Copyright (C) 2019 Globalscale technologies, Inc.
+ *
+ * Jason Hung <jhung@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include "armada-372x.dtsi"
+
+/ {
+ model = "Marvell Armada 3720 ESPRESSOBin-Ultra";

the other espressobin boards use "Globalscale Marvell ESPRESSOBin Board", so maybe "Globalscale Marvell ESPRESSOBin Ultra Board"?

+ compatible = "globalscale,espressobin-ultra", "marvell,armada3720",
+ "marvell,armada3710";
+
+ aliases {
+ ethernet0 = &eth0;
+ /* for dsa slave device */
+ ethernet1 = &switch0port1;
+ ethernet2 = &switch0port2;
+ ethernet3 = &switch0port3;
+ ethernet4 = &switch0port4;
+ ethernet5 = &switch0port5;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory@0 {
+ device_type = "memory";
+ reg = <0x00000000 0x00000000 0x00000000 0x20000000>;
+ };
+
+ reg_usb3_vbus: usb3-vbus {
+ compatible = "regulator-fixed";
+ regulator-name = "usb3-vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ enable-active-high;
+ gpio = <&gpionb 19 GPIO_ACTIVE_HIGH>;
+ };
+
+ usb3_phy: usb3-phy {
+ compatible = "usb-nop-xceiv";
+ vcc-supply = <&reg_usb3_vbus>;
+ };
+
+ gpio-leds {
+ pinctrl-names = "default";
+ compatible = "gpio-leds";
+ /* No assigned functions to the LEDs by default */
+ led1 {
+ label = "ebin-ultra:blue:led1";
+ gpios = <&gpionb 11 GPIO_ACTIVE_LOW>;
+ };
+ led2 {
+ label = "ebin-ultra:green:led2";
+ gpios = <&gpionb 12 GPIO_ACTIVE_LOW>;
+ };
+ led3 {
+ label = "ebin-ultra:red:led3";
+ gpios = <&gpionb 13 GPIO_ACTIVE_LOW>;
+ };
+ led4 {
+ label = "ebin-ultra:yellow:led4";
+ gpios = <&gpionb 14 GPIO_ACTIVE_LOW>;
+ };
+ };
+};
+
+&pcie0 {
+ status = "okay";

armada-3720-espressobin.dtsi additionally has:

pinctrl-names = "default";
pinctrl-0 = <&pcie_reset_pins &pcie_clkreq_pins>;
reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;

is it missing here?

+};
+
+&sata {
+ status = "okay";

also from armada-3720-espressobin.dtsi, missing?

phys = <&comphy2 0>;
phy-names = "sata-phy";

+};
+
+&sdhci0 {
+ status = "okay";
+ non-removable;
+ bus-width = <8>;
+ mmc-ddr-1_8v;
+ mmc-hs400-1_8v;
+ marvell,pad-type = "fixed-1-8v";
+};

again from armada-3720-espressobin.dtsi, maybe use as-is to keep the differences to a minimum?

non-removable;
bus-width = <8>;
mmc-ddr-1_8v;
mmc-hs400-1_8v;
marvell,xenon-emmc;
marvell,xenon-tun-count = <9>;
marvell,pad-type = "fixed-1-8v";

pinctrl-names = "default";
pinctrl-0 = <&mmc_pins>;

#address-cells = <1>;
#size-cells = <0>;
mmccard: mmccard@0 {
compatible = "mmc-card";
reg = <0>;
};

+
+&spi0 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi_quad_pins>;
+
+ flash@0 {
+ compatible = "jedec,spi-nor";
+ reg = <0>;
+ spi-max-frequency = <108000000>;
+ spi-rx-bus-width = <4>;
+ spi-tx-bus-width = <4>;
+ m25p,fast-read;
+
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "u-boot";

as Pali mentioned, that's more than just u-boot, the other espressobin device trees use "firmware" here.

+ reg = <0x0 0x3e0000>;
+ };
+ partition@3e0000 {
+ label = "hw-info";

any idea what's in there?

+ reg = <0x3e0000 0x10000>;
+ read-only;
+ };
+ partition@3f0000 {
+ label = "u-boot-env";
+ reg = <0x3f0000 0x10000>;
+ };
+ };
+ };
+};
+
+&uart0 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart1_pins>;
+};
+
+&i2c0 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c1_pins>;
+
+ clock-frequency = <100000>;
+
+ rtc@51 {
+ compatible = "nxp,pcf8563";
+ reg = <0x51>;
+ };
+};
+
+&usb3 {
+ status = "okay";
+ usb-phy = <&usb3_phy>;

That node comes from armada-37xx.dtsi and has:

phys = <&comphy0 0>, <&usb2_utmi_otg_phy>;
phy-names = "usb3-phy", "usb2-utmi-otg-phy";

Are those supposed to be mixed?

+};
+
+&usb2 {
+ status = "okay";
+};
+
+&eth0 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&rgmii_pins>;
+ phy-mode = "rgmii-id";
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+};
+
+&mdio {
+ status = "okay";

I think that's not required here.

+
+ extphy: ethernet-phy@0 {
+ reg = <1>;
+ };
+
+ switch0: switch0@1 {
+ compatible = "marvell,mv88e6085";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+
+ dsa,member = <0 0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ switch0port0: port@0 {
+ reg = <0>;
+ label = "cpu";
+ ethernet = <&eth0>;

armada-3720-espressobin.dtsi has these too:

phy-mode = "rgmii-id";
fixed-link {
speed = <1000>;
full-duplex;
};

Which matches with the &eth0 setup from above.

+ };
+
+ switch0port1: port@1 {
+ reg = <1>;
+ label = "lan0";
+ phy-handle = <&switch0phy1>;
+ };
+
+ switch0port2: port@2 {
+ reg = <2>;
+ label = "lan1";
+ phy-handle = <&switch0phy2>;
+ };
+
+ switch0port3: port@3 {
+ reg = <3>;
+ label = "lan2";
+ phy-handle = <&switch0phy3>;
+ };
+
+ switch0port4: port@4 {
+ reg = <4>;
+ label = "lan3";
+ phy-handle = <&switch0phy4>;
+ };
+
+ switch0port5: port@5 {
+ reg = <5>;
+ label = "wan";
+ phy-handle = <&extphy>;
+ phy-mode = "sgmii";
+ };
+ };
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ switch0phy1: switch0phy1@11 {
+ reg = <0x11>;
+ };
+ switch0phy2: switch0phy2@12 {
+ reg = <0x12>;
+ };
+ switch0phy3: switch0phy3@13 {
+ reg = <0x13>;
+ };
+ switch0phy4: switch0phy4@14 {
+ reg = <0x14>;
+ };
+ };
+ };
+};