[PATCH] arm: mvebu: Add support for Thecus N2350 (Armada 385) board
Tony Dinh
mibodhi at gmail.com
Sun Jan 29 01:17:32 CET 2023
Hi Pali,
On Sat, Jan 28, 2023 at 5:00 AM Pali Rohár <pali at kernel.org> wrote:
>
> Hello! It is nice to see support for another A385 board!
> I have few comments, see below...
>
> On Friday 27 January 2023 20:47:10 Tony Dinh wrote:
> > Thecus N2350 is a NAS based on Marvell Armada 385 SoC.
> >
> > Specification:
> >
> > - Processor: Marvel MV88F6820 Dual Core at 1GHz
> > - 1 GiB DDR4 RAM
> > - 4MB Macronix mx25l3205d SPI flash
> > - 512MB Hynix H27U4G8F2DTR-BC NAND flash
> > - I2C
> > - 2x USB 3.0
> > - 1x GBE LAN port (PHY: Marvell 88E1510)
> > - 2x SATA (hot swap slots)
> > - 3x buttons
> > - 10x LEDS
> > - serial console
> >
> > Signed-off-by: Tony Dinh <mibodhi at gmail.com>
> > ---
> >
> > arch/arm/dts/Makefile | 1 +
> > .../dts/armada-385-thecus-n2350-u-boot.dtsi | 5 +
> > arch/arm/dts/armada-385-thecus-n2350.dts | 451 ++++++++++++++++++
> > arch/arm/mach-mvebu/Kconfig | 12 +
> > board/thecus/n2350/MAINTAINERS | 6 +
> > board/thecus/n2350/Makefile | 6 +
> > board/thecus/n2350/n2350.c | 134 ++++++
> > configs/n2350_defconfig | 93 ++++
> > include/configs/n2350.h | 65 +++
> > 9 files changed, 773 insertions(+)
> > create mode 100644 arch/arm/dts/armada-385-thecus-n2350-u-boot.dtsi
> > create mode 100644 arch/arm/dts/armada-385-thecus-n2350.dts
> > create mode 100644 board/thecus/n2350/MAINTAINERS
> > create mode 100644 board/thecus/n2350/Makefile
> > create mode 100644 board/thecus/n2350/n2350.c
> > create mode 100644 configs/n2350_defconfig
> > create mode 100644 include/configs/n2350.h
> >
> > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > index 3ecd6a86e9..c5d1825a3b 100644
> > --- a/arch/arm/dts/Makefile
> > +++ b/arch/arm/dts/Makefile
> > @@ -245,6 +245,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \
> > armada-385-atl-x530.dtb \
> > armada-385-atl-x530DP.dtb \
> > armada-385-db-88f6820-amc.dtb \
> > + armada-385-thecus-n2350.dtb \
> > armada-385-turris-omnia.dtb \
> > armada-388-clearfog.dtb \
> > armada-388-gp.dtb \
> > diff --git a/arch/arm/dts/armada-385-thecus-n2350-u-boot.dtsi b/arch/arm/dts/armada-385-thecus-n2350-u-boot.dtsi
> > new file mode 100644
> > index 0000000000..0b818705cf
> > --- /dev/null
> > +++ b/arch/arm/dts/armada-385-thecus-n2350-u-boot.dtsi
> > @@ -0,0 +1,5 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2023 Tony Dinh <mibodhi at gmail.com>
> > + */
> > +#include "mvebu-u-boot.dtsi"
>
> If you have only #include "mvebu-u-boot.dtsi" line in your -u-boot.dtsi
> file then this -u-boot.dtsi file should not be needed at all. Because
> U-Boot build process include it automatically when board -u-boot.dtsi
> does not exits.
I see. So that must be part of a Make file somewhere? I have another
patch for Kirkwood boards -uboot.dtsi coming and I'm thinking about
making it automatic, too, if possible.
> > diff --git a/arch/arm/dts/armada-385-thecus-n2350.dts b/arch/arm/dts/armada-385-thecus-n2350.dts
> > new file mode 100644
> > index 0000000000..5632d82510
> > --- /dev/null
> > +++ b/arch/arm/dts/armada-385-thecus-n2350.dts
>
> Could you send DTS file also into Linux kernel?
Yes, as soon as this board gets merged here, I will send the DTS to
Linux kernel.
>
> > @@ -0,0 +1,451 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Device Tree file for Thecus N2350 board
> > + *
> > + * Copyright (C) 2018-2023 Tony Dinh <mibodhi at gmail.com>
> > + * Copyright (C) 2018 Manuel Jung <manuel.jung at hotmail.com>
> > + */
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/input/input.h>
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include "armada-385.dtsi"
> > +
> > +/ {
> > + model = "Thecus N2350";
> > + compatible = "thecus,n2350", "marvell,a385-db", "marvell,armada388",
> > + "marvell,armada385", "marvell,armada380";
>
> As it is Armada 385 board, you should not have there "marvell,armada388"
> and also you should not have there "marvell,a385-db" which refers to the
> Marvell Development Board (different one).
OK.
> > +
> > + aliases {
> > + ethernet0 = ð0;
> > + };
> > +
> > + chosen {
> > + stdout-path = "serial0:115200n8";
> > + };
> > +
> > + memory {
> > + device_type = "memory";
> > + reg = <0x00000000 0x40000000>; /* 1GB */
> > + };
> > +
> > + soc {
> > + ranges = <MBUS_ID(0xf0, 0x01) 0 0xf1000000 0x100000
> > + MBUS_ID(0x01, 0x1d) 0 0xfff00000 0x100000
> > + MBUS_ID(0x09, 0x19) 0 0xf1100000 0x10000
> > + MBUS_ID(0x09, 0x15) 0 0xf1110000 0x10000
> > + MBUS_ID(0x0c, 0x04) 0 0xf1200000 0x100000>;
> > +
> > + internal-regs {
> > + i2c at 11000 {
> > + status = "okay";
> > + clock-frequency = <100000>;
> > + };
> > +
> > + i2c at 11100 {
> > + status = "okay";
> > + clock-frequency = <100000>;
> > + };
>
> These two nodes have &i2c0 and &i2c1 pointers defined in armada-38x.dtsi
> so you can move them out of the internal-regs. Like you did for &spi1
> and &nand_controller.
>
> > +
> > + serial at 12000 {
> > + status = "okay";
> > + };
>
> Same for &uart0
>
> > +
> > + usb at 58000 {
> > + status = "ok";
>
> status = "ok" is ignored. Correct is "okay".
>
> > + };
>
> And it has &usb0.
>
> > +
> > + ethernet at 70000 {
> > + status = "okay";
> > + phy = <&phy0>;
> > + phy-mode = "sgmii";
> > + buffer-manager = <&bm>;
> > + bm,pool-long = <0>;
> > + bm,pool-short = <1>;
> > + };
>
> ð0
>
> > +
> > + mdio at 72004 {
> > + phy0: ethernet-phy at 0 {
> > + reg = <1>;
> > + };
> > + };
>
> &mdio
>
> > +
> > + sata at a8000 {
> > + status = "okay";
> > + };
>
> &ahci0
>
> > +
> > + bm at c8000 {
> > + status = "okay";
> > + };
>
> &bm
>
> > +
> > + pinctrl at 18000 {
>
> Could you sort device nodes by addresses? It is common to define nodes
> in ascending order.
>
> Also it has &pinctrl
Thanks for these comments! as you can see this DTS is quite old :) I
wrote it long ago and then not really paid any attention to
modernizing it.
>
> > + pinctrl-names = "default";
> > +
> > + pmx_power_button: pmx_power-button {
>
> IIRC convention for device node name is to use only hyphens and for DTS
> pointers only underscore. So it should be something like this:
>
> pmx_power_button: pmx-power-button {
> ...
> };
Oo. My bad, I knew that :)
>
> > + marvell,pins = "mpp49";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_copy_button: pmx_copy-button {
> > + marvell,pins = "mpp52";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_reset_button: pmx_reset-button {
> > + marvell,pins = "mpp50";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_sata1_white_led: pmx_sata1_white_led {
> > + marvell,pins = "mpp20";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_sata1_red_led: pmx_sata1_red_led {
> > + marvell,pins = "mpp46";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_sata2_white_led: pmx_sata2_white_led {
> > + marvell,pins = "mpp19";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_sata2_red_led: pmx_sata2_red_led {
> > + marvell,pins = "mpp47";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_sys_white_led: pmx_sys_white_led {
> > + marvell,pins = "mpp14";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_sys_red_led: pmx_sys_red_led {
> > + marvell,pins = "mpp15";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_buzzer: pmx_buzzer {
> > + marvell,pins = "mpp51";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_pwr_off: pmx-pwr-off {
> > + marvell,pins = "mpp54";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_pwr_blue_led: pmx_pwr_blue_led {
> > + marvell,pins = "mpp43";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_pwr_red_led: pmx_pwr_red_led {
> > + marvell,pins = "mpp18";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_usb_white_led: pmx_usb_white_led {
> > + marvell,pins = "mpp16";
> > + marvell,function = "gpio";
> > + };
> > +
> > + pmx_usb_red_led: pmx_usb_red_led {
> > + marvell,pins = "mpp17";
> > + marvell,function = "gpio";
> > + };
> > + };
> > +
> > + sdhci at d8000 {
> > + broken-cd;
> > + wp-inverted;
> > + bus-width = <8>;
> > + status = "okay";
> > + no-1-8-v;
> > + };
>
> &sdhci
>
> > +
> > + usb3 at f0000 {
> > + status = "okay";
> > + };
>
> &usb3_0
>
> > +
> > + usb3 at f8000 {
> > + status = "okay";
> > + };
>
> &usb3_1
>
> > +
> > + };
> > +
> > + bm-bppi {
> > + status = "okay";
> > + };
>
> &bm_bppi
>
> > +
> > + pcie {
> > + status = "okay";
> > + /*
> > + * The two PCIe units are accessible through
> > + * standard PCIe slots on the board.
> > + */
> > + pcie at 1,0 {
> > + /* Port 0, Lane 0 */
> > + status = "okay";
> > + };
> > + pcie at 2,0 {
> > + /* Port 1, Lane 0 */
> > + status = "okay";
> > + };
> > + };
> > +
> > + };
> > +
> > + usb3_0_phy: usb3_0_phy {
> > + compatible = "usb-nop-xceiv";
> > + vcc-supply = <&usb3_0_power>;
> > + };
> > +
> > + usb3_1_phy: usb3_1_phy {
> > + compatible = "usb-nop-xceiv";
> > + vcc-supply = <&usb3_1_power>;
> > + };
> > +
> > + gpio-keys {
> > + compatible = "gpio-keys";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + pinctrl-0 = <&pmx_power_button &pmx_copy_button &pmx_reset_button>;
> > + pinctrl-names = "default";
> > +
> > + button at 1 {
> > + label = "Power Button";
> > + linux,code = <KEY_POWER>;
> > + gpios = <&gpio1 17 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + button at 2 {
> > + label = "Copy Button";
> > + linux,code = <KEY_COPY>;
> > + gpios = <&gpio1 20 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + button at 3 {
> > + label = "Reset Button";
> > + linux,code = <KEY_RESTART>;
> > + gpios = <&gpio1 18 GPIO_ACTIVE_HIGH>;
> > + };
> > + };
> > +
> > + gpio-leds {
> > + compatible = "gpio-leds";
> > + pinctrl-0 = <&pmx_sata1_white_led
> > + &pmx_sata1_red_led
> > + &pmx_sata2_white_led
> > + &pmx_sata2_red_led
> > + &pmx_sys_white_led
> > + &pmx_sys_red_led
> > + &pmx_pwr_blue_led
> > + &pmx_pwr_red_led
> > + &pmx_usb_white_led
> > + &pmx_usb_red_led>;
> > +
> > + pinctrl-names = "default";
> > +
> > + white_sata1 {
> > + label = "n2350:white:sata1";
> > + gpios = <&gpio0 20 GPIO_ACTIVE_HIGH>;
> > + linux,default-trigger = "ide-disk1";
> > + };
> > +
> > + red_sata1 {
> > + label = "n2350:red:sata1";
> > + gpios = <&gpio1 14 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + white-sata2 {
> > + label = "n2350:white:sata2";
> > + gpios = <&gpio0 19 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + red-sata2 {
> > + label = "n2350:red:sata2";
> > + gpios = <&gpio1 15 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + white-sys {
> > + label = "n2350:white:sys";
> > + gpios = <&gpio0 14 GPIO_ACTIVE_HIGH>;
> > + linux,default-trigger = "default-on";
> > + };
> > +
> > + red-sys {
> > + label = "n2350:red:sys";
> > + gpios = <&gpio0 15 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + blue-pwr {
> > + label = "n2350:blue:pwr";
> > + gpios = <&gpio1 11 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + red-pwr {
> > + label = "n2350:red:pwr";
> > + gpios = <&gpio0 18 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + white-usb {
> > + label = "n2350:white:usb";
> > + gpios = <&gpio0 16 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + red-usb {
> > + label = "n2350:red:usb";
> > + gpios = <&gpio0 17 GPIO_ACTIVE_HIGH>;
> > + };
> > + };
> > +
> > + regulators {
> > + compatible = "simple-bus";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + usb3_0_power: regulator at 1 {
> > + compatible = "regulator-fixed";
> > + reg = <1>;
> > + regulator-name = "USB3_0_Power";
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + enable-active-high;
> > + regulator-always-on;
> > + regulator-boot-on;
> > + gpio = <&gpio0 21 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + usb3_1_power: regulator at 2 {
> > + compatible = "regulator-fixed";
> > + reg = <1>;
> > + regulator-name = "USB3_1_Power";
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + enable-active-high;
> > + regulator-always-on;
> > + regulator-boot-on;
> > + gpio = <&gpio0 24 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + reg_sata0: regulator at 3 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "pwr_en_sata0";
> > + regulator-min-microvolt = <12000000>;
> > + regulator-max-microvolt = <12000000>;
> > + enable-active-high;
> > + regulator-always-on;
> > + regulator-boot-on;
> > + gpio = <&gpio1 12 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + reg_5v_sata0: v5-sata0 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "v5.0-sata0";
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + vin-supply = <®_sata0>;
> > + };
> > +
> > + reg_12v_sata0: v12-sata0 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "v12.0-sata0";
> > + regulator-min-microvolt = <12000000>;
> > + regulator-max-microvolt = <12000000>;
> > + vin-supply = <®_sata0>;
> > + };
> > +
> > + reg_sata1: regulator at 4 {
> > + regulator-name = "pwr_en_sata1";
> > + compatible = "regulator-fixed";
> > + regulator-min-microvolt = <12000000>;
> > + regulator-max-microvolt = <12000000>;
> > + enable-active-high;
> > + regulator-always-on;
> > + regulator-boot-on;
> > + gpio = <&gpio1 13 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + reg_5v_sata1: v5-sata1 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "v5.0-sata1";
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + vin-supply = <®_sata1>;
> > + };
> > +
> > + reg_12v_sata1: v12-sata1 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "v12.0-sata1";
> > + regulator-min-microvolt = <12000000>;
> > + regulator-max-microvolt = <12000000>;
> > + vin-supply = <®_sata1>;
> > + };
> > +
> > + };
> > +
> > + gpio-poweroff {
> > + compatible = "gpio-poweroff";
> > + pinctrl-0 = <&pmx_pwr_off>;
> > + pinctrl-names = "default";
> > + gpios = <&gpio1 22 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > +};
> > +
> > +&spi1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&spi1_pins>;
> > + status = "okay";
> > +
> > + /* spi: 4M Flash Macronix MX25L3205D */
> > + spi-flash at 0 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + compatible = "macronix,mx25l3205d", "jedec,spi-nor";
> > + reg = <0>;
> > +
> > + spi-max-frequency = <108000000>;
> > + spi-cpha;
> > +
> > + partition at 0 {
> > + label = "U-Boot-img";
> > + reg = <0x00000000 0x00100000>;
> > + };
> > +
> > + partition at 100000 {
> > + label = "U-Boot-env";
> > + reg = <0x00100000 0x00010000>;
> > + };
>
> Personally, I would use labels "u-boot" and "u-boot-env". But I'm not
> sure what others think...
Sure. Those were stock FW labels carried over. I also think "u-boot"
and "u-boot-env" are better.
> > + };
> > +
> > +};
> > +
> > +&nand_controller {
> > + status = "okay";
> > +
> > + nand at 0 {
> > + status = "okay";
> > + reg = <0>;
> > + label = "pxa3xx_nand-0";
> > + nand-rb = <0>;
> > + marvell,nand-keep-config;
> > + nand-on-flash-bbt;
> > + nand-ecc-strength = <4>;
> > + nand-ecc-step-size = <512>;
> > +
> > + partitions {
> > + compatible = "fixed-partitions";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + partition at 0 {
> > + label = "ubifs";
> > + reg = <0x00000000 0x20000000>;
> > + };
> > +
> > + };
> > + };
> > +};
> > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > index 594e9a03d9..f3c3d2a7d9 100644
> > --- a/arch/arm/mach-mvebu/Kconfig
> > +++ b/arch/arm/mach-mvebu/Kconfig
> > @@ -97,6 +97,10 @@ config 88F6820
> > config CUSTOMER_BOARD_SUPPORT
> > bool
> >
> > +config DDR4
> > + bool "Support Marvell DDR4 Training driver"
> > + default false
>
> "default false" is not needed, All unspecified values are false by
> default.
OK.
> > +
> > choice
> > prompt "Armada XP/375/38x/3700/7K/8K/Alleycat-5 board select"
> > optional
> > @@ -175,6 +179,11 @@ config TARGET_MAXBCM
> > select BOARD_ECC_SUPPORT
> > select MV78460
> >
> > +config TARGET_N2350
> > + bool "Support Thecus N2350"
> > + select 88F6820
> > + select DDR4
> > +
> > config TARGET_THEADORABLE
> > bool "Support theadorable Armada XP"
> > select BOARD_LATE_INIT if USB
> > @@ -257,6 +266,7 @@ config SYS_BOARD
> > default "db-mv784mp-gp" if TARGET_DB_MV784MP_GP
> > default "ds414" if TARGET_DS414
> > default "maxbcm" if TARGET_MAXBCM
> > + default "n2350" if TARGET_N2350
> > default "theadorable" if TARGET_THEADORABLE
> > default "a38x" if TARGET_CONTROLCENTERDC
> > default "x530" if TARGET_X530
> > @@ -276,6 +286,7 @@ config SYS_CONFIG_NAME
> > default "db-mv784mp-gp" if TARGET_DB_MV784MP_GP
> > default "ds414" if TARGET_DS414
> > default "maxbcm" if TARGET_MAXBCM
> > + default "n2350" if TARGET_N2350
> > default "theadorable" if TARGET_THEADORABLE
> > default "turris_omnia" if TARGET_TURRIS_OMNIA
> > default "turris_mox" if TARGET_TURRIS_MOX
> > @@ -298,6 +309,7 @@ config SYS_VENDOR
> > default "solidrun" if TARGET_CLEARFOG
> > default "kobol" if TARGET_HELIOS4
> > default "Synology" if TARGET_DS414
> > + default "thecus" if TARGET_N2350
> > default "CZ.NIC" if TARGET_TURRIS_OMNIA
> > default "CZ.NIC" if TARGET_TURRIS_MOX
> > default "gdsys" if TARGET_CONTROLCENTERDC
> > diff --git a/board/thecus/n2350/MAINTAINERS b/board/thecus/n2350/MAINTAINERS
> > new file mode 100644
> > index 0000000000..27cf238225
> > --- /dev/null
> > +++ b/board/thecus/n2350/MAINTAINERS
> > @@ -0,0 +1,6 @@
> > +N2350 BOARD
> > +M: Tony Dinh <mibodhi at gmail.com>
> > +S: Maintained
> > +F: board/thecus/n2350/
> > +F: include/configs/n2350.h
> > +F: configs/n2350_defconfig
> > diff --git a/board/thecus/n2350/Makefile b/board/thecus/n2350/Makefile
> > new file mode 100644
> > index 0000000000..b220bb1925
> > --- /dev/null
> > +++ b/board/thecus/n2350/Makefile
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright (C) 2023 Tony Dinh <mibodhi at gmail.com>
> > +#
> > +
> > +obj-y := n2350.o
> > diff --git a/board/thecus/n2350/n2350.c b/board/thecus/n2350/n2350.c
> > new file mode 100644
> > index 0000000000..4d3c49ba80
> > --- /dev/null
> > +++ b/board/thecus/n2350/n2350.c
> > @@ -0,0 +1,134 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2023 Tony Dinh <mibodhi at gmail.com>
> > + *
> > + */
> > +
> > +#include <common.h>
> > +#include <i2c.h>
> > +#include <miiphy.h>
> > +#include <netdev.h>
> > +#include <asm/io.h>
> > +#include <asm/arch/cpu.h>
> > +#include <asm/arch/soc.h>
> > +#include <linux/bitops.h>
> > +
> > +#include "../drivers/ddr/marvell/a38x/ddr3_init.h"
> > +#include <../serdes/a38x/high_speed_env_spec.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +/*
> > + * Those N2350_GPP_xx values and defines in board_serdes_map, and board_topology_map
> > + * are taken from the Marvell U-Boot version "u-boot-a38x-2015T1_p18_Thecus"
> > + */
> > +
> > +#define N2350_GPP_OUT_ENA_LOW (~(BIT(20) | BIT(21) | BIT(24)))
> > +#define N2350_GPP_OUT_ENA_MID (~(BIT(12) | BIT(13) | BIT(16) | BIT(19) | BIT(22)))
> > +#define N2350_GPP_OUT_VAL_LOW 0x1200000
> > +#define N2350_GPP_OUT_VAL_MID 0x1001
> > +#define N2350_GPP_POL_LOW 0x0
> > +#define N2350_GPP_POL_MID 0x0
> > +
> > +static struct serdes_map board_serdes_map[] = {
> > + { SGMII0, SERDES_SPEED_1_25_GBPS, SERDES_DEFAULT_MODE, 0, 0},
> > + { SATA0, SERDES_SPEED_3_GBPS, SERDES_DEFAULT_MODE, 0, 0},
> > + { SATA1, SERDES_SPEED_3_GBPS, SERDES_DEFAULT_MODE, 0, 0},
> > + { DEFAULT_SERDES, SERDES_SPEED_5_GBPS, SERDES_DEFAULT_MODE, 0, 0},
> > + { USB3_HOST0, SERDES_SPEED_5_GBPS, SERDES_DEFAULT_MODE, 0, 0},
> > + { USB3_HOST1, SERDES_SPEED_5_GBPS, SERDES_DEFAULT_MODE, 0, 0},
> > +};
> > +
> > +int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
> > +{
> > + *serdes_map_array = board_serdes_map;
> > + *count = ARRAY_SIZE(board_serdes_map);
> > + return 0;
> > +}
> > +
> > +/*
> > + * Define the DDR layout / topology here in the board file. This will
> > + * be used by the DDR4 init code in the SPL U-Boot version to configure
> > + * the DDR4 controller.
> > + */
> > +
> > +static struct mv_ddr_topology_map board_topology_map = {
> > + DEBUG_LEVEL_ERROR,
> > + 0x1, /* active interfaces */
> > + /* cs_mask, mirror, dqs_swap, ck_swap X PUPs */
> > + { { { {0x1, 0, 0, 0},
> > + {0x1, 0, 0, 0},
> > + {0x1, 0, 0, 0},
> > + {0x1, 0, 0, 0},
> > + {0x1, 0, 0, 0} },
> > + SPEED_BIN_DDR_1866L, /* speed_bin */
> > + MV_DDR_DEV_WIDTH_16BIT, /* memory_width - 16 bits */
> > + MV_DDR_DIE_CAP_4GBIT, /* mem_size - N2350 board has 2x512MB DRAM banks */
> > + MV_DDR_FREQ_800, /* frequency */
> > + 0, 0, /* cas_wl cas_l */
> > + MV_DDR_TEMP_LOW, /* temperature */
> > + MV_DDR_TIM_DEFAULT} }, /* timing */
> > + BUS_MASK_32BIT, /* Busses mask */
> > + MV_DDR_CFG_DEFAULT, /* ddr configuration data source */
> > + NOT_COMBINED, /* ddr twin-die combined */
> > + { {0} }, /* raw spd data */
> > + {0} /* timing parameters */
> > +};
> > +
> > +struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
> > +{
> > + /* Return the board topology as defined in the board code */
> > + return &board_topology_map;
> > +}
> > +
> > +int board_early_init_f(void)
> > +{
> > + /* Those MPP values are taken from the Marvell U-Boot version
> > + * "u-boot-a38x-2015T1_p18_Thecus"
> > + */
> > +
> > + /* Configure MPP */
> > + writel(0x50111111, MVEBU_MPP_BASE + 0x00); /* MPP0_7 */
> > + writel(0x00555555, MVEBU_MPP_BASE + 0x04); /* MPP8_15 */
> > + writel(0x55000000, MVEBU_MPP_BASE + 0x08); /* MPP16_23 */
> > + writel(0x05050050, MVEBU_MPP_BASE + 0x0c); /* MPP24_31 */
> > + writel(0x05555555, MVEBU_MPP_BASE + 0x10); /* MPP32_39 */
> > + writel(0x00000565, MVEBU_MPP_BASE + 0x14); /* MPP40_47 */
> > + writel(0x00000000, MVEBU_MPP_BASE + 0x18); /* MPP48_55 */
> > + writel(0x00004444, MVEBU_MPP_BASE + 0x1c); /* MPP56_63 */
> > +
> > + /* Set GPP Out value */
> > + writel(N2350_GPP_OUT_VAL_LOW, MVEBU_GPIO0_BASE + 0x00);
> > + writel(N2350_GPP_OUT_VAL_MID, MVEBU_GPIO1_BASE + 0x00);
> > +
> > + /* Set GPP Polarity */
> > + writel(N2350_GPP_POL_LOW, MVEBU_GPIO0_BASE + 0x0c);
> > + writel(N2350_GPP_POL_MID, MVEBU_GPIO1_BASE + 0x0c);
> > +
> > + /* Set GPP Out Enable */
> > + writel(N2350_GPP_OUT_ENA_LOW, MVEBU_GPIO0_BASE + 0x04);
> > + writel(N2350_GPP_OUT_ENA_MID, MVEBU_GPIO1_BASE + 0x04);
> > +
> > + return 0;
> > +}
> > +
> > +int board_init(void)
> > +{
> > + /* Address of boot parameters */
> > + gd->bd->bi_boot_params = mvebu_sdram_bar(0) + 0x100;
> > +
> > + return 0;
> > +}
> > +
> > +int checkboard(void)
> > +{
> > + puts("Board: Thecus N2350\n");
> > +
> > + return 0;
> > +}
>
> I think that checkboard() function is not needed here. It does not check
> for anything (always returns 0) and printing board name is done by
> common u-boot function show_board_info().
OK.
>
> > +
> > +int board_eth_init(struct bd_info *bis)
> > +{
> > + cpu_eth_init(bis); /* Built in controller(s) come first */
> > + return pci_eth_init(bis);
> > +}
> > diff --git a/configs/n2350_defconfig b/configs/n2350_defconfig
> > new file mode 100644
> > index 0000000000..22b23d50e2
> > --- /dev/null
> > +++ b/configs/n2350_defconfig
> > @@ -0,0 +1,93 @@
> > +CONFIG_ARM=y
> > +CONFIG_ARCH_CPU_INIT=y
> > +CONFIG_SYS_THUMB_BUILD=y
> > +CONFIG_ARCH_MVEBU=y
> > +CONFIG_SUPPORT_PASSING_ATAGS=y
> > +CONFIG_CMDLINE_TAG=y
> > +CONFIG_INITRD_TAG=y
> > +CONFIG_TEXT_BASE=0x00800000
> > +CONFIG_SPL_LIBCOMMON_SUPPORT=y
> > +CONFIG_SPL_LIBGENERIC_SUPPORT=y
> > +CONFIG_NR_DRAM_BANKS=2
> > +CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> > +CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff0000
> > +CONFIG_TARGET_N2350=y
> > +CONFIG_ENV_SIZE=0x10000
> > +CONFIG_ENV_OFFSET=0x100000
> > +CONFIG_ENV_SECT_SIZE=0x10000
> > +CONFIG_DEFAULT_DEVICE_TREE="armada-385-thecus-n2350"
> > +CONFIG_SPL_TEXT_BASE=0x40000030
> > +CONFIG_SYS_PROMPT="N2350 > "
> > +CONFIG_SPL_SERIAL=y
> > +CONFIG_SPL_STACK=0x4002c000
> > +CONFIG_SPL=y
> > +CONFIG_DEBUG_UART_BASE=0xf1012000
> > +CONFIG_DEBUG_UART_CLOCK=250000000
> > +CONFIG_IDENT_STRING="\nThecus N2350"
> > +CONFIG_SYS_LOAD_ADDR=0x800000
> > +CONFIG_ENV_ADDR=0x100000
> > +CONFIG_DEBUG_UART=y
> > +CONFIG_AHCI=y
> > +CONFIG_DISTRO_DEFAULTS=y
> > +CONFIG_BOOTDELAY=10
> > +CONFIG_USE_PREBOOT=y
> > +CONFIG_SYS_CONSOLE_INFO_QUIET=y
> > +# CONFIG_DISPLAY_BOARDINFO is not set
> > +CONFIG_DISPLAY_BOARDINFO_LATE=y
> > +CONFIG_SPL_MAX_SIZE=0x22fd0
> > +CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
> > +CONFIG_SPL_BSS_START_ADDR=0x40023000
> > +CONFIG_SPL_BSS_MAX_SIZE=0x4000
> > +CONFIG_SPL_SYS_MALLOC_SIMPLE=y
> > +# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
> > +CONFIG_SPL_I2C=y
> > +CONFIG_SYS_MAXARGS=32
> > +CONFIG_CMD_GPIO=y
> > +CONFIG_CMD_GPIO_READ=y
> > +CONFIG_CMD_I2C=y
> > +CONFIG_CMD_PCI=y
> > +CONFIG_CMD_SPI=y
> > +CONFIG_CMD_USB=y
> > +CONFIG_CMD_TFTPPUT=y
> > +CONFIG_CMD_SNTP=y
> > +CONFIG_CMD_DNS=y
> > +CONFIG_CMD_CACHE=y
> > +CONFIG_CMD_TIME=y
> > +CONFIG_CMD_MTDPARTS=y
> > +CONFIG_MTDPARTS_DEFAULT="mtdparts=pxa3xx_nand-0:-(ubifs);spi1.0:1M(uboot),64K at 1M(uboot_env),-(data)"
>
> Do you need to define partitions also here? Because on Turris Omnia is
> U-Boot parsing partitions from DTS file and CONFIG_MTDPARTS_DEFAULT is
> not specified in configs/turris_omnia_defconfig at all.
I think CONFIG_MTDPARTS_DEFAULT is still quite useful here. Oftenly,
when we boot with a separate DTB, we can use the Linux cmdline
mtdparts to override the partittions definition in the DTS. And Linux
mainline DTS sometimes have obsolete/wrong mtd partitions definition
(eg. Synology A38x/XP boards are prime examples). So exposing the env
mtdparts makes it apparent to the users what we want to use as
default.
But if mtdparts env is automatically set by the DTS definition, then
yes I think we don't want to repeat it here.
>
> > +CONFIG_ENV_OVERWRITE=y
> > +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> > +CONFIG_VERSION_VARIABLE=y
> > +CONFIG_ARP_TIMEOUT=200
> > +CONFIG_NET_RETRY_COUNT=50
> > +CONFIG_NET_RANDOM_ETHADDR=y
> > +CONFIG_NETCONSOLE=y
> > +CONFIG_SPL_OF_TRANSLATE=y
> > +CONFIG_AHCI_MVEBU=y
> > +CONFIG_LBA48=y
> > +CONFIG_SYS_64BIT_LBA=y
> > +CONFIG_DM_I2C=y
> > +CONFIG_SYS_I2C_MVTWSI=y
> > +CONFIG_MTD=y
> > +CONFIG_SF_DEFAULT_SPEED=50000000
> > +CONFIG_SPI_FLASH_MACRONIX=y
> > +CONFIG_SPI_FLASH_STMICRO=y
> > +CONFIG_PHY_MARVELL=y
> > +CONFIG_PHY_GIGE=y
> > +CONFIG_MVNETA=y
> > +CONFIG_MII=y
> > +CONFIG_MVMDIO=y
> > +CONFIG_PCI=y
> > +CONFIG_PCI_MVEBU=y
> > +CONFIG_PINCTRL=y
> > +CONFIG_PINCTRL_ARMADA_38X=y
> > +CONFIG_DM_RTC=y
> > +CONFIG_RTC_ARMADA38X=y
> > +CONFIG_SCSI=y
> > +CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
> > +CONFIG_DEBUG_UART_SHIFT=2
> > +CONFIG_SYS_NS16550=y
> > +CONFIG_KIRKWOOD_SPI=y
> > +CONFIG_USB=y
> > +CONFIG_USB_XHCI_HCD=y
> > +CONFIG_USB_EHCI_HCD=y
> > diff --git a/include/configs/n2350.h b/include/configs/n2350.h
> > new file mode 100644
> > index 0000000000..1d4458d210
> > --- /dev/null
> > +++ b/include/configs/n2350.h
> > @@ -0,0 +1,65 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (C) 2023 Tony Dinh <mibodhi at gmail.com>
> > + *
> > + */
> > +
> > +#ifndef _CONFIG_N2350_H
> > +#define _CONFIG_N2350_H
> > +
> > +/*
> > + * High Level Configuration Options (easy to change)
> > + */
> > +
> > +/* I2C */
> > +#define CFG_I2C_MVTWSI_BASE0 MVEBU_TWSI_BASE
> > +
> > +/* Environment in SPI NOR flash */
> > +
> > +#define PHY_ANEG_TIMEOUT 8000 /* PHY needs a longer aneg time */
> > +
> > +/* Keep device tree and initrd in lower memory so the kernel can access them */
> > +#define RELOCATION_LIMITS_ENV_SETTINGS \
> > + "fdt_high=0x10000000\0" \
> > + "initrd_high=0x10000000\0"
> > +
> > +/*
> > + * mv-common.h should be defined after CMD configs since it used them
> > + * to enable certain macros
> > + */
> > +#include "mv-common.h"
> > +
> > +/* Include the common distro boot environment */
> > +#ifndef CONFIG_SPL_BUILD
> > +
> > +#define BOOT_TARGET_DEVICES(func) \
> > + func(SCSI, scsi, 0) \
> > + func(USB, usb, 0) \
> > + func(PXE, pxe, na) \
> > + func(DHCP, dhcp, na)
>
> I see that you have also NAND with UBIFS in DTS file. What is stored on
> NAND? Does not it have bootable system and should be in default boot
> target list?
The UBIFS in stock FW was the rootfs. But it is not bootable any more
with the new u-boot. In case somebody wants to boot OpenWrt or some
rescue system, I just want to make the NAND partition visible for that
purpose. Perhaps its name should be something innocuous like "data" ?
>
> > +
> > +#define KERNEL_ADDR_R __stringify(0x1000000)
> > +#define FDT_ADDR_R __stringify(0x2000000)
> > +#define RAMDISK_ADDR_R __stringify(0x2200000)
> > +#define SCRIPT_ADDR_R __stringify(0x1800000)
> > +#define PXEFILE_ADDR_R __stringify(0x1900000)
> > +
> > +#define LOAD_ADDRESS_ENV_SETTINGS \
> > + "kernel_addr_r=" KERNEL_ADDR_R "\0" \
> > + "fdt_addr_r=" FDT_ADDR_R "\0" \
> > + "ramdisk_addr_r=" RAMDISK_ADDR_R "\0" \
> > + "scriptaddr=" SCRIPT_ADDR_R "\0" \
> > + "pxefile_addr_r=" PXEFILE_ADDR_R "\0"
> > +
> > +#include <config_distro_bootcmd.h>
> > +
> > +#define CFG_EXTRA_ENV_SETTINGS \
> > + RELOCATION_LIMITS_ENV_SETTINGS \
> > + LOAD_ADDRESS_ENV_SETTINGS \
> > + "fdtfile=" CONFIG_DEFAULT_DEVICE_TREE ".dtb\0" \
> > + "console=ttyS0,115200\0" \
> > + BOOTENV
> > +
> > +#endif /* CONFIG_SPL_BUILD */
> > +
> > +#endif /* _CONFIG_N2350_H */
> > --
> > 2.30.2
> >
Thanks for the review Pali.
All the best,
Tony
More information about the U-Boot
mailing list