All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Add support for the GST ESPRESSOBin-Ultra board
@ 2021-02-04 10:46 Luka Kovacic
  2021-02-04 10:46 ` [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments Luka Kovacic
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Luka Kovacic @ 2021-02-04 10:46 UTC (permalink / raw)
  To: u-boot

This patchset adds initial support for the ESPRESSOBin-Ultra board from
Globalscale Technologies, Inc.

The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
Peripherals:
 - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
 - RTC clock (PCF8563)
 - USB 3.0 port
 - USB 2.0 port
 - 4x LED
 - UART over Micro-USB
 - M.2 slot (2280)
 - Mini PCI-E slot

Additionally support for importing Marvell hw_info formatted environments
is added to fully support the board.

Luka Kovacic (3):
  cmd: env: Add 'env import -h' for Marvell hw_info formatted
    environments
  arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment
    variable
  arm: mvebu: Initial ESPRESSOBin-Ultra board support

 arch/arm/dts/Makefile                         |   1 +
 .../arm/dts/armada-3720-espressobin-ultra.dts | 202 ++++++++++++++++++
 board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
 board/Marvell/mvebu_armada-37xx/board.c       |  76 ++++++-
 cmd/nvedit.c                                  |  22 +-
 .../mvebu_espressobin-ultra-88f3720_defconfig |  90 ++++++++
 include/configs/mvebu_armada-37xx.h           |   1 +
 lib/hashtable.c                               |   2 +-
 8 files changed, 391 insertions(+), 11 deletions(-)
 create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
 create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig

-- 
2.20.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments
  2021-02-04 10:46 [PATCH 0/3] Add support for the GST ESPRESSOBin-Ultra board Luka Kovacic
@ 2021-02-04 10:46 ` Luka Kovacic
  2021-02-04 17:06   ` Tom Rini
  2021-02-04 10:46 ` [PATCH 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable Luka Kovacic
  2021-02-04 10:46 ` [PATCH 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support Luka Kovacic
  2 siblings, 1 reply; 12+ messages in thread
From: Luka Kovacic @ 2021-02-04 10:46 UTC (permalink / raw)
  To: u-boot

The '-h' flag is added to the 'env import' command to enable parsing
Marvell hw_info formatted environments.
This format is often used on Marvell Armada A37XX based devices to store
parameters like the board serial number, factory MAC addresses and some
other information.

Currently this environment format can only be imported, not exported.
These parameters are usually written to the flash in the factory.

This functionality has been tested on the GST ESPRESSOBin-Ultra board
successfully.

Usage example:
 => sf probe
 => sf read ${loadaddr} 0x003E000A 0x1F0 # Read the environment from
SPI flash
 => env import -h ${loadaddr}

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
Cc: Robert Marko <robert.marko@sartura.hr>
---
 cmd/nvedit.c    | 22 ++++++++++++++++++----
 lib/hashtable.c |  2 +-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index d0d2eca904..eab490ac48 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -1060,7 +1060,7 @@ sep_err:
 
 #ifdef CONFIG_CMD_IMPORTENV
 /*
- * env import [-d] [-t [-r] | -b | -c] addr [size] [var ...]
+ * env import [-d] [-t [-r] | -b | -h | -c] addr [size] [var ...]
  *	-d:	delete existing environment before importing if no var is
  *		passed; if vars are passed, if one var is in the current
  *		environment but not in the environment at addr, delete var from
@@ -1073,6 +1073,7 @@ sep_err:
  *		to import text files created with editors which are using CRLF
  *		for line endings. Only effective in addition to -t.
  *	-b:	assume binary format ('\0' separated, "\0\0" terminated)
+ *	-h:	assume Marvell hw_info format (0x20 separated, "0x20\0" terminated)
  *	-c:	assume checksum protected environment format
  *	addr:	memory address to read from
  *	size:	length of input data; if missing, proper '\0'
@@ -1082,6 +1083,14 @@ sep_err:
  *	var...	List of the names of the only variables that get imported from
  *		the environment at address 'addr'. Without arguments, the whole
  *		environment gets imported.
+ *
+ * Using the "-h" flag you can import a Marvell hw_info formatted environment.
+ * This is commonly used on Marvell Armada A37XX devices to store the board serial
+ * number, MAC addresses and some other information.
+ * Currently you can only import these by loading the data into memory and parse it
+ * using this command. Exporting is currently not supported, due to tight memory
+ * restrictions and a strict formatting scheme.
+ * These parameters are usually populated in the factory.
  */
 static int do_env_import(struct cmd_tbl *cmdtp, int flag,
 			 int argc, char *const argv[])
@@ -1107,6 +1116,11 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag,
 					goto sep_err;
 				sep = '\0';
 				break;
+			case 'h':		/* Marvell hw_info format */
+				if (fmt++)
+					goto sep_err;
+				sep = 0x20;
+				break;
 			case 'c':		/* external checksum format */
 				if (fmt++)
 					goto sep_err;
@@ -1199,8 +1213,8 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag,
 	return 0;
 
 sep_err:
-	printf("## %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n",
-		cmd);
+	printf("## %s: only one of \"-b\", \"-h\", \"-c\" or \"-t\" allowed\n",
+	       cmd);
 	return 1;
 }
 #endif
@@ -1450,7 +1464,7 @@ static char env_help_text[] =
 #endif
 #endif
 #if defined(CONFIG_CMD_IMPORTENV)
-	"env import [-d] [-t [-r] | -b | -c] addr [size] [var ...] - import environment\n"
+	"env import [-d] [-t [-r] | -b | -h | -c] addr [size] [var ...] - import environment\n"
 #endif
 #if defined(CONFIG_CMD_NVEDIT_INFO)
 	"env info - display environment information\n"
diff --git a/lib/hashtable.c b/lib/hashtable.c
index ff5ff72639..06322e3304 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -794,7 +794,7 @@ static int drop_var_from_set(const char *name, int nvars, char * vars[])
  * multi-line values.
  *
  * In theory, arbitrary separator characters can be used, but only
- * '\0' and '\n' have really been tested.
+ * '\0', '\n' and 0x20 have been tested.
  */
 
 int himport_r(struct hsearch_data *htab,
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable
  2021-02-04 10:46 [PATCH 0/3] Add support for the GST ESPRESSOBin-Ultra board Luka Kovacic
  2021-02-04 10:46 ` [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments Luka Kovacic
@ 2021-02-04 10:46 ` Luka Kovacic
  2021-02-04 10:46 ` [PATCH 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support Luka Kovacic
  2 siblings, 0 replies; 12+ messages in thread
From: Luka Kovacic @ 2021-02-04 10:46 UTC (permalink / raw)
  To: u-boot

Add the loadaddr U-Boot environment variable, as this is available in
the stock Marvell U-Boot by default on Marvell Armada A37XX platforms.

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
Cc: Robert Marko <robert.marko@sartura.hr>
---
 include/configs/mvebu_armada-37xx.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/configs/mvebu_armada-37xx.h b/include/configs/mvebu_armada-37xx.h
index 0d585606a7..6da82078a4 100644
--- a/include/configs/mvebu_armada-37xx.h
+++ b/include/configs/mvebu_armada-37xx.h
@@ -89,6 +89,7 @@
 
 /* fdt_addr and kernel_addr are needed for existing distribution boot scripts */
 #define CONFIG_EXTRA_ENV_SETTINGS	\
+	"loadaddr=0x6000000\0"		\
 	"scriptaddr=0x6d00000\0"	\
 	"pxefile_addr_r=0x6e00000\0"	\
 	"fdt_addr=0x6f00000\0"		\
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support
  2021-02-04 10:46 [PATCH 0/3] Add support for the GST ESPRESSOBin-Ultra board Luka Kovacic
  2021-02-04 10:46 ` [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments Luka Kovacic
  2021-02-04 10:46 ` [PATCH 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable Luka Kovacic
@ 2021-02-04 10:46 ` Luka Kovacic
  2 siblings, 0 replies; 12+ messages in thread
From: Luka Kovacic @ 2021-02-04 10:46 UTC (permalink / raw)
  To: u-boot

Add initial support for the ESPRESSOBin-Ultra board from Globalscale
Technologies, Inc.

The board is based on the 64-bit dual-core Marvell Armada 3720 SoC.
Peripherals:
 - 5 Gigabit Ethernet ports (WAN has PoE, up to 30W, Topaz 6341 switch)
 - RTC clock (PCF8563)
 - USB 3.0 port
 - USB 2.0 port
 - 4x LED
 - UART over Micro-USB
 - M.2 slot (2280)
 - Mini PCI-E slot

Additionally, automatic import of the Marvell hw_info parameters is
enabled via the recently added env import -h flag.
The parameters stored in Marvell hw_info are usually the board serial
number and MAC addresses.

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
Cc: Robert Marko <robert.marko@sartura.hr>
---
 arch/arm/dts/Makefile                         |   1 +
 .../arm/dts/armada-3720-espressobin-ultra.dts | 202 ++++++++++++++++++
 board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   8 +
 board/Marvell/mvebu_armada-37xx/board.c       |  76 ++++++-
 .../mvebu_espressobin-ultra-88f3720_defconfig |  90 ++++++++
 5 files changed, 371 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
 create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 858b79ac97..51455c0271 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -210,6 +210,7 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \
 dtb-$(CONFIG_ARCH_MVEBU) +=			\
 	armada-3720-db.dtb			\
 	armada-3720-espressobin.dtb		\
+	armada-3720-espressobin-ultra.dtb	\
 	armada-3720-turris-mox.dtb		\
 	armada-3720-uDPU.dtb			\
 	armada-375-db.dtb			\
diff --git a/arch/arm/dts/armada-3720-espressobin-ultra.dts b/arch/arm/dts/armada-3720-espressobin-ultra.dts
new file mode 100644
index 0000000000..70f97fe239
--- /dev/null
+++ b/arch/arm/dts/armada-3720-espressobin-ultra.dts
@@ -0,0 +1,202 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Device Tree file for ESPRESSObin-Ultra board
+ * Copyright (C) 2016 Marvell
+ * Copyright (C) 2019 Globalscale technologies, Inc.
+ * Copyright (C) 2020 Sartura Ltd.
+ *
+ * Author: Jason Hung <jhung@globalscaletechnologies.com>
+ * Author: Luka Kovacic <luka.kovacic@sartura.hr>
+ * Author: Vladimir Vid <vladimir.vid@sartura.hr>
+ */
+
+/dts-v1/;
+
+#include "armada-372x.dtsi"
+
+/ {
+	model = "Globalscale Marvell ESPRESSOBin Ultra Board";
+	compatible = "globalscale,espressobin-ultra", "marvell,armada3720", "marvell,armada3710";
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	aliases {
+		ethernet0 = &eth0;
+		i2c0 = &i2c0;
+		spi0 = &spi0;
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x00000000 0x00000000 0x20000000>;
+	};
+
+	vcc_sd_reg0: regulator at 0 {
+		compatible = "regulator-gpio";
+		regulator-name = "vcc_sd0";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-type = "voltage";
+		states = <1800000 0x1
+			  3300000 0x0>;
+		gpios = <&gpionb 4 GPIO_ACTIVE_HIGH>;
+	};
+
+	gpio-leds {
+		pinctrl-names = "default";
+		pinctrl-0 = <&led1_pins>, <&led2_pins>, <&led3_pins>, <&led4_pins>;
+		compatible = "gpio-leds";
+
+		led1 {
+			label = "led1";
+			gpios = <&gpionb 11 GPIO_ACTIVE_LOW>;
+			default-state = "on";
+		};
+		led2 {
+			label = "led2";
+			gpios = <&gpionb 12 GPIO_ACTIVE_LOW>;
+			default-state = "on";
+		};
+		led3 {
+			label = "led3";
+			gpios = <&gpionb 13 GPIO_ACTIVE_LOW>;
+			default-state = "on";
+		};
+		led4 {
+			label = "led4";
+			gpios = <&gpionb 14 GPIO_ACTIVE_LOW>;
+			default-state = "on";
+		};
+	};
+};
+
+&pinctrl_nb {
+	led1_pins: led1-pins {
+		groups = "pwm0";
+		function = "gpio";
+	};
+	led2_pins: led2-pins {
+		groups = "pwm1";
+		function = "gpio";
+	};
+	led3_pins: led3-pins {
+		groups = "pwm2";
+		function = "gpio";
+	};
+	led4_pins: led4-pins {
+		groups = "pwm3";
+		function = "gpio";
+	};
+};
+
+&comphy {
+	max-lanes = <3>;
+	phy0 {
+		phy-type = <PHY_TYPE_USB3_HOST0>;
+		phy-speed = <PHY_SPEED_5G>;
+	};
+
+	phy1 {
+		phy-type = <PHY_TYPE_PEX0>;
+		phy-speed = <PHY_SPEED_2_5G>;
+	};
+
+	phy2 {
+		phy-type = <PHY_TYPE_SATA0>;
+		phy-speed = <PHY_SPEED_5G>;
+	};
+};
+
+&eth0 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&rgmii_pins>, <&smi_pins>;
+	phy-mode = "rgmii";
+	phy_addr = <0x3>;
+	fixed-link {
+		speed = <1000>;
+		full-duplex;
+	};
+};
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins>;
+	status = "okay";
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+	rtc at 51 {
+		compatible = "nxp,pcf8563";
+		reg = <0x51>;
+	};
+};
+
+&sata {
+	status = "okay";
+};
+
+&sdhci0 {
+	status = "disabled";
+};
+
+/* U11 */
+&sdhci1 {
+	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>;
+	status = "okay";
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+	mmccard: mmccard at 0 {
+		compatible = "mmc-card";
+		reg = <0>;
+	};
+};
+
+&spi0 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi_quad_pins>;
+
+	spi-flash at 0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "st,m25p128", "jedec,spi-nor";
+		reg = <0>; /* Chip select 0 */
+		spi-max-frequency = <50000000>;
+		m25p,fast-read;
+	};
+};
+
+/* Exported on the micro USB connector through an FTDI */
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>;
+	status = "okay";
+};
+
+&usb2 {
+	status = "okay";
+};
+
+&usb3 {
+	status = "okay";
+};
+
+&pcie0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie_pins>;
+	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};
diff --git a/board/Marvell/mvebu_armada-37xx/MAINTAINERS b/board/Marvell/mvebu_armada-37xx/MAINTAINERS
index f2c0a582d7..d69af832fc 100644
--- a/board/Marvell/mvebu_armada-37xx/MAINTAINERS
+++ b/board/Marvell/mvebu_armada-37xx/MAINTAINERS
@@ -10,6 +10,14 @@ M:	Konstantin Porotchkin <kostap@marvell.com>
 S:	Maintained
 F:	configs/mvebu_espressobin-88f3720_defconfig
 
+ESPRESSOBin-Ultra BOARD
+M:	Luka Kovacic <luka.kovacic@sartura.hr>
+M:	Robert Marko <robert.marko@sartura.hr>
+M:	Luka Perkov <luka.perkov@sartura.hr>
+S:	Maintained
+F:	arch/arm/dts/armada-3720-espressobin-ultra.dts
+F:	configs/mvebu_espressobin-ultra-88f3720_defconfig
+
 uDPU BOARD
 M:	Vladimir Vid <vladimir.vid@sartura.hr>
 S:	Maintained
diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
index 1b9e7520cc..b92b7950e1 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -10,6 +10,7 @@
 #include <i2c.h>
 #include <init.h>
 #include <mmc.h>
+#include <miiphy.h>
 #include <phy.h>
 #include <asm/io.h>
 #include <asm/arch/cpu.h>
@@ -53,6 +54,15 @@ DECLARE_GLOBAL_DATA_PTR;
 #define MVEBU_G2_SMI_PHY_CMD_REG	(24)
 #define MVEBU_G2_SMI_PHY_DATA_REG	(25)
 
+/* Marvell 88E1512 */
+#define MII_MARVELL_PHY_PAGE		22
+
+#define MV88E1512_GENERAL_CTRL		20
+#define MV88E1512_MODE_SGMII		1
+#define MV88E1512_RESET_OFFS		15
+
+#define ULTRA_MV88E1512_PHYADDR		0x1
+
 /*
  * Memory Controller Registers
  *
@@ -258,12 +268,52 @@ static int mii_multi_chip_mode_write(struct mii_dev *bus, int dev_smi_addr,
 	return 0;
 }
 
-/* Bring-up board-specific network stuff */
-int board_network_enable(struct mii_dev *bus)
+void force_phy_88e1512_sgmii_to_copper(u16 devaddr)
 {
-	if (!of_machine_is_compatible("globalscale,espressobin"))
-		return 0;
+	const char *name;
+	u16 reg;
+
+	name = miiphy_get_current_dev();
+	if (name) {
+		/* SGMII-to-Copper mode initialization */
+
+		/* Select page 18 */
+		miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0x12);
+		/* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
+		miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, &reg);
+		reg &= ~0x7;
+		reg |= MV88E1512_MODE_SGMII;
+		miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, reg);
+		/* PHY reset is necessary after changing MODE[2:0] */
+		miiphy_read(name, devaddr, MV88E1512_GENERAL_CTRL, &reg);
+		reg |= 1 << MV88E1512_RESET_OFFS;
+		miiphy_write(name, devaddr, MV88E1512_GENERAL_CTRL, reg);
+		/* Reset page selection */
+		miiphy_write(name, devaddr, MII_MARVELL_PHY_PAGE, 0);
+		udelay(100);
+	}
+}
+
+int board_network_enable_espressobin_ultra(struct mii_dev *bus)
+{
+	/* Setup 88E1512 SGMII-to-Copper mode */
+	force_phy_88e1512_sgmii_to_copper(ULTRA_MV88E1512_PHYADDR);
 
+	/* Restrict output to ports 1,2,3,4 only from port 0 (CPU) */
+	mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(1),
+				  MVEBU_SW_PORT_BASE_VLAN, BIT(0));
+	mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(2),
+				  MVEBU_SW_PORT_BASE_VLAN, BIT(0));
+	mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(3),
+				  MVEBU_SW_PORT_BASE_VLAN, BIT(0));
+	mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(4),
+				  MVEBU_SW_PORT_BASE_VLAN, BIT(0));
+
+	return 0;
+}
+
+int board_network_enable_espressobin(struct mii_dev *bus)
+{
 	/*
 	 * FIXME: remove this code once Topaz driver gets available
 	 * A3720 Community Board Only
@@ -304,6 +354,16 @@ int board_network_enable(struct mii_dev *bus)
 	return 0;
 }
 
+/* Bring-up the board-specific networking */
+int board_network_enable(struct mii_dev *bus)
+{
+	if (of_machine_is_compatible("globalscale,espressobin"))
+		return board_network_enable_espressobin(bus);
+	if (of_machine_is_compatible("globalscale,espressobin-ultra"))
+		return board_network_enable_espressobin_ultra(bus);
+	return 0;
+}
+
 #if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_ENV_IS_IN_SPI_FLASH)
 int ft_board_setup(void *blob, struct bd_info *bd)
 {
@@ -312,8 +372,12 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 	int parts_off;
 	int part_off;
 
-	/* Fill SPI MTD partitions for Linux kernel on Espressobin */
-	if (!of_machine_is_compatible("globalscale,espressobin"))
+	/*
+	 * Fill SPI MTD partitions for the Linux kernel on ESPRESSOBin and
+	 * ESPRESSOBin Ultra boards.
+	 */
+	if (!of_machine_is_compatible("globalscale,espressobin") &&
+	    !of_machine_is_compatible("globalscale,espressobin-ultra"))
 		return 0;
 
 	spi_off = fdt_node_offset_by_compatible(blob, -1, "jedec,spi-nor");
diff --git a/configs/mvebu_espressobin-ultra-88f3720_defconfig b/configs/mvebu_espressobin-ultra-88f3720_defconfig
new file mode 100644
index 0000000000..e40e280d62
--- /dev/null
+++ b/configs/mvebu_espressobin-ultra-88f3720_defconfig
@@ -0,0 +1,90 @@
+CONFIG_ARM=y
+CONFIG_ARCH_CPU_INIT=y
+CONFIG_ARCH_MVEBU=y
+CONFIG_SYS_TEXT_BASE=0x00000000
+CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_NR_DRAM_BANKS=1
+CONFIG_TARGET_MVEBU_ARMADA_37XX=y
+CONFIG_ENV_SIZE=0x10000
+CONFIG_ENV_OFFSET=0x3F0000
+CONFIG_ENV_SECT_SIZE=0x10000
+CONFIG_DM_GPIO=y
+CONFIG_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_CLOCK=25804800
+CONFIG_DEFAULT_DEVICE_TREE="armada-3720-espressobin-ultra"
+CONFIG_DEBUG_UART=y
+CONFIG_AHCI=y
+CONFIG_DISTRO_DEFAULTS=y
+# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
+CONFIG_OF_BOARD_SETUP=y
+CONFIG_FIT=y
+CONFIG_FIT_VERBOSE=y
+CONFIG_FIT_BEST_MATCH=y
+CONFIG_AUTOBOOT_KEYED=y
+CONFIG_AUTOBOOT_PROMPT="Autoboot in %d seconds, to stop use 's' key\n"
+CONFIG_AUTOBOOT_STOP_STR="s"
+CONFIG_AUTOBOOT_KEYED_CTRLC=y
+CONFIG_HUSH_PARSER=y
+CONFIG_USE_PREBOOT=y
+CONFIG_PREBOOT="if printenv read_board_hw_info; then echo \"Marvell hw_info already imported...\" ; else echo \"Importing Marvell hw_info...\"; sf probe; sf read ${loadaddr} 0x003E000A 0x1F0; env import -h ${loadaddr}; setenv read_board_hw_info 1; fi"
+CONFIG_SYS_CONSOLE_INFO_QUIET=y
+# CONFIG_DISPLAY_CPUINFO is not set
+# CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_ARCH_EARLY_INIT_R=y
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_BOARD_LATE_INIT=y
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_TFTPPUT=y
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIME=y
+CONFIG_CMD_MVEBU_BUBT=y
+CONFIG_CMD_EXT4_WRITE=y
+CONFIG_MAC_PARTITION=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_IS_IN_SPI_FLASH=y
+CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_AHCI_MVEBU=y
+CONFIG_CLK=y
+CONFIG_CLK_MVEBU=y
+CONFIG_DM_I2C=y
+CONFIG_MISC=y
+CONFIG_DM_MMC=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
+CONFIG_MMC_SDHCI_XENON=y
+CONFIG_MTD=y
+CONFIG_SF_DEFAULT_MODE=0
+CONFIG_SPI_FLASH_MACRONIX=y
+CONFIG_PHY_MARVELL=y
+CONFIG_MVNETA=y
+CONFIG_PCI=y
+CONFIG_DM_PCI=y
+CONFIG_PCI_AARDVARK=y
+CONFIG_MVEBU_COMPHY_SUPPORT=y
+CONFIG_PINCTRL=y
+CONFIG_PINCTRL_ARMADA_37XX=y
+CONFIG_DM_REGULATOR_GPIO=y
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_DEBUG_UART_ANNOUNCE=y
+CONFIG_MVEBU_A3700_UART=y
+CONFIG_MVEBU_A3700_SPI=y
+CONFIG_USB=y
+CONFIG_DM_USB=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_HOST_ETHER=y
+CONFIG_SHA1=y
+CONFIG_SHA256=y
+CONFIG_DM_RTC=y
+CONFIG_RTC_PCF8563=y
+CONFIG_LED=y
+CONFIG_LED_GPIO=y
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments
  2021-02-04 10:46 ` [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments Luka Kovacic
@ 2021-02-04 17:06   ` Tom Rini
  2021-02-05 22:02     ` Luka Kovacic
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Rini @ 2021-02-04 17:06 UTC (permalink / raw)
  To: u-boot

On Thu, Feb 04, 2021 at 11:46:35AM +0100, Luka Kovacic wrote:

> The '-h' flag is added to the 'env import' command to enable parsing
> Marvell hw_info formatted environments.
> This format is often used on Marvell Armada A37XX based devices to store
> parameters like the board serial number, factory MAC addresses and some
> other information.
> 
> Currently this environment format can only be imported, not exported.
> These parameters are usually written to the flash in the factory.
> 
> This functionality has been tested on the GST ESPRESSOBin-Ultra board
> successfully.
> 
> Usage example:
>  => sf probe
>  => sf read ${loadaddr} 0x003E000A 0x1F0 # Read the environment from
> SPI flash
>  => env import -h ${loadaddr}
> 
> Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> Cc: Robert Marko <robert.marko@sartura.hr>

So, the implementation itself is fine:

Reviewed-by: Tom Rini <trini@konsulko.com>

And I take it as a given that we can't get the boards populated with an
existing separator, so we need to solve this somehow or another.  I am
however loathe to increase every platform by a handful of bytes (help
text, code text) to cover this case, and adding #ifdef around the help
text in particular would be very ugly.  Is there any clean way to write
this as a board-specific command?  I assume the overall intention is to
import the factory env for the required information, store it in regular
U-Boot environment and then never touch the factory area again.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210204/dcd89772/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments
  2021-02-04 17:06   ` Tom Rini
@ 2021-02-05 22:02     ` Luka Kovacic
  2021-02-05 22:04       ` Tom Rini
  0 siblings, 1 reply; 12+ messages in thread
From: Luka Kovacic @ 2021-02-05 22:02 UTC (permalink / raw)
  To: u-boot

Hello Tom,

On Thu, Feb 4, 2021 at 6:06 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Feb 04, 2021 at 11:46:35AM +0100, Luka Kovacic wrote:
>
> > The '-h' flag is added to the 'env import' command to enable parsing
> > Marvell hw_info formatted environments.
> > This format is often used on Marvell Armada A37XX based devices to store
> > parameters like the board serial number, factory MAC addresses and some
> > other information.
> >
> > Currently this environment format can only be imported, not exported.
> > These parameters are usually written to the flash in the factory.
> >
> > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > successfully.
> >
> > Usage example:
> >  => sf probe
> >  => sf read ${loadaddr} 0x003E000A 0x1F0 # Read the environment from
> > SPI flash
> >  => env import -h ${loadaddr}
> >
> > Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > Cc: Robert Marko <robert.marko@sartura.hr>
>
> So, the implementation itself is fine:
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> And I take it as a given that we can't get the boards populated with an
> existing separator, so we need to solve this somehow or another.  I am
> however loathe to increase every platform by a handful of bytes (help
> text, code text) to cover this case, and adding #ifdef around the help
> text in particular would be very ugly. Is there any clean way to write
> this as a board-specific command?  I assume the overall intention is to
> import the factory env for the required information, store it in regular
> U-Boot environment and then never touch the factory area again.  Thanks!

I have already considered the possibility of implementing this in a
board-specific way, but I tried implementing it here to avoid unnecessary
code duplication.
You are correct, the intention is to use this functionality as a migration
path. It's good to keep the factory area intact, in case it's still needed in
the future.
The Marvell hw_info area is limited to MACs and serial numbers by the
stock utility.

I've also compared the resulting u-boot.bin binary size with and without
the patch and the difference is about 40 bytes.

>
> --
> Tom

Kind regards,
Luka

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments
  2021-02-05 22:02     ` Luka Kovacic
@ 2021-02-05 22:04       ` Tom Rini
  2021-02-05 22:36         ` Luka Kovacic
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Rini @ 2021-02-05 22:04 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 05, 2021 at 11:02:23PM +0100, Luka Kovacic wrote:
> Hello Tom,
> 
> On Thu, Feb 4, 2021 at 6:06 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Feb 04, 2021 at 11:46:35AM +0100, Luka Kovacic wrote:
> >
> > > The '-h' flag is added to the 'env import' command to enable parsing
> > > Marvell hw_info formatted environments.
> > > This format is often used on Marvell Armada A37XX based devices to store
> > > parameters like the board serial number, factory MAC addresses and some
> > > other information.
> > >
> > > Currently this environment format can only be imported, not exported.
> > > These parameters are usually written to the flash in the factory.
> > >
> > > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > > successfully.
> > >
> > > Usage example:
> > >  => sf probe
> > >  => sf read ${loadaddr} 0x003E000A 0x1F0 # Read the environment from
> > > SPI flash
> > >  => env import -h ${loadaddr}
> > >
> > > Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> > > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > > Cc: Robert Marko <robert.marko@sartura.hr>
> >
> > So, the implementation itself is fine:
> >
> > Reviewed-by: Tom Rini <trini@konsulko.com>
> >
> > And I take it as a given that we can't get the boards populated with an
> > existing separator, so we need to solve this somehow or another.  I am
> > however loathe to increase every platform by a handful of bytes (help
> > text, code text) to cover this case, and adding #ifdef around the help
> > text in particular would be very ugly. Is there any clean way to write
> > this as a board-specific command?  I assume the overall intention is to
> > import the factory env for the required information, store it in regular
> > U-Boot environment and then never touch the factory area again.  Thanks!
> 
> I have already considered the possibility of implementing this in a
> board-specific way, but I tried implementing it here to avoid unnecessary
> code duplication.
> You are correct, the intention is to use this functionality as a migration
> path. It's good to keep the factory area intact, in case it's still needed in
> the future.
> The Marvell hw_info area is limited to MACs and serial numbers by the
> stock utility.

OK.  And can we put this in the board-specific path somehow, relatively
cleanly?

> I've also compared the resulting u-boot.bin binary size with and without
> the patch and the difference is about 40 bytes.

Yes, but it's 40 bytes on nearly every single platform.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210205/169a747c/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments
  2021-02-05 22:04       ` Tom Rini
@ 2021-02-05 22:36         ` Luka Kovacic
  2021-02-05 22:40           ` Tom Rini
  0 siblings, 1 reply; 12+ messages in thread
From: Luka Kovacic @ 2021-02-05 22:36 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 5, 2021 at 11:04 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Feb 05, 2021 at 11:02:23PM +0100, Luka Kovacic wrote:
> > Hello Tom,
> >
> > On Thu, Feb 4, 2021 at 6:06 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Feb 04, 2021 at 11:46:35AM +0100, Luka Kovacic wrote:
> > >
> > > > The '-h' flag is added to the 'env import' command to enable parsing
> > > > Marvell hw_info formatted environments.
> > > > This format is often used on Marvell Armada A37XX based devices to store
> > > > parameters like the board serial number, factory MAC addresses and some
> > > > other information.
> > > >
> > > > Currently this environment format can only be imported, not exported.
> > > > These parameters are usually written to the flash in the factory.
> > > >
> > > > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > > > successfully.
> > > >
> > > > Usage example:
> > > >  => sf probe
> > > >  => sf read ${loadaddr} 0x003E000A 0x1F0 # Read the environment from
> > > > SPI flash
> > > >  => env import -h ${loadaddr}
> > > >
> > > > Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> > > > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > > > Cc: Robert Marko <robert.marko@sartura.hr>
> > >
> > > So, the implementation itself is fine:
> > >
> > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > >
> > > And I take it as a given that we can't get the boards populated with an
> > > existing separator, so we need to solve this somehow or another.  I am
> > > however loathe to increase every platform by a handful of bytes (help
> > > text, code text) to cover this case, and adding #ifdef around the help
> > > text in particular would be very ugly. Is there any clean way to write
> > > this as a board-specific command?  I assume the overall intention is to
> > > import the factory env for the required information, store it in regular
> > > U-Boot environment and then never touch the factory area again.  Thanks!
> >
> > I have already considered the possibility of implementing this in a
> > board-specific way, but I tried implementing it here to avoid unnecessary
> > code duplication.
> > You are correct, the intention is to use this functionality as a migration
> > path. It's good to keep the factory area intact, in case it's still needed in
> > the future.
> > The Marvell hw_info area is limited to MACs and serial numbers by the
> > stock utility.
>
> OK.  And can we put this in the board-specific path somehow, relatively
> cleanly?

I've been thinking about the following options:
 - Implementing a separate command under cmd/mvebu (hw_info), like done
in the stock U-Boot.
I'm not inclined to this one, as I don't see real utility in updating
this factory
area. A positive with this one is that it abstracts reading of the SPI flash.

 - No command, automatic migration, directly from the SPI flash (implemented
in the board.c file.
I don't see this as a clean way of doing it, the Armada A37XX board.c is
already quite full of board-specific init.

 - Implemented as is, but with #ifdef
This would break CONFIG_CMD_IMPORTENV into smaller pieces.
#ifdef wouldn't look too bad around the code, only the inline string changes
would be ugly.

What would you recommend?

>
> > I've also compared the resulting u-boot.bin binary size with and without
> > the patch and the difference is about 40 bytes.
>
> Yes, but it's 40 bytes on nearly every single platform.
>
> --
> Tom

Kind regards,
Luka

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments
  2021-02-05 22:36         ` Luka Kovacic
@ 2021-02-05 22:40           ` Tom Rini
  2021-02-05 22:51             ` Luka Kovacic
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Rini @ 2021-02-05 22:40 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 05, 2021 at 11:36:25PM +0100, Luka Kovacic wrote:
> On Fri, Feb 5, 2021 at 11:04 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Feb 05, 2021 at 11:02:23PM +0100, Luka Kovacic wrote:
> > > Hello Tom,
> > >
> > > On Thu, Feb 4, 2021 at 6:06 PM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Feb 04, 2021 at 11:46:35AM +0100, Luka Kovacic wrote:
> > > >
> > > > > The '-h' flag is added to the 'env import' command to enable parsing
> > > > > Marvell hw_info formatted environments.
> > > > > This format is often used on Marvell Armada A37XX based devices to store
> > > > > parameters like the board serial number, factory MAC addresses and some
> > > > > other information.
> > > > >
> > > > > Currently this environment format can only be imported, not exported.
> > > > > These parameters are usually written to the flash in the factory.
> > > > >
> > > > > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > > > > successfully.
> > > > >
> > > > > Usage example:
> > > > >  => sf probe
> > > > >  => sf read ${loadaddr} 0x003E000A 0x1F0 # Read the environment from
> > > > > SPI flash
> > > > >  => env import -h ${loadaddr}
> > > > >
> > > > > Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> > > > > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > > > > Cc: Robert Marko <robert.marko@sartura.hr>
> > > >
> > > > So, the implementation itself is fine:
> > > >
> > > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > > >
> > > > And I take it as a given that we can't get the boards populated with an
> > > > existing separator, so we need to solve this somehow or another.  I am
> > > > however loathe to increase every platform by a handful of bytes (help
> > > > text, code text) to cover this case, and adding #ifdef around the help
> > > > text in particular would be very ugly. Is there any clean way to write
> > > > this as a board-specific command?  I assume the overall intention is to
> > > > import the factory env for the required information, store it in regular
> > > > U-Boot environment and then never touch the factory area again.  Thanks!
> > >
> > > I have already considered the possibility of implementing this in a
> > > board-specific way, but I tried implementing it here to avoid unnecessary
> > > code duplication.
> > > You are correct, the intention is to use this functionality as a migration
> > > path. It's good to keep the factory area intact, in case it's still needed in
> > > the future.
> > > The Marvell hw_info area is limited to MACs and serial numbers by the
> > > stock utility.
> >
> > OK.  And can we put this in the board-specific path somehow, relatively
> > cleanly?
> 
> I've been thinking about the following options:
>  - Implementing a separate command under cmd/mvebu (hw_info), like done
> in the stock U-Boot.
> I'm not inclined to this one, as I don't see real utility in updating
> this factory
> area. A positive with this one is that it abstracts reading of the SPI flash.
> 
>  - No command, automatic migration, directly from the SPI flash (implemented
> in the board.c file.
> I don't see this as a clean way of doing it, the Armada A37XX board.c is
> already quite full of board-specific init.
> 
>  - Implemented as is, but with #ifdef
> This would break CONFIG_CMD_IMPORTENV into smaller pieces.
> #ifdef wouldn't look too bad around the code, only the inline string changes
> would be ugly.
> 
> What would you recommend?

I hate to say it but I prefer the first, especially if it ends up being
like 50 lines of code or whatever to write the function and all that.
Or the second option, if the first option really ends up being messy
once you write it and the second one less messy.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210205/dba84c6e/attachment.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments
  2021-02-05 22:40           ` Tom Rini
@ 2021-02-05 22:51             ` Luka Kovacic
  2021-02-09  8:12               ` Stefan Roese
  0 siblings, 1 reply; 12+ messages in thread
From: Luka Kovacic @ 2021-02-05 22:51 UTC (permalink / raw)
  To: u-boot

On Fri, Feb 5, 2021 at 11:40 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Feb 05, 2021 at 11:36:25PM +0100, Luka Kovacic wrote:
> > On Fri, Feb 5, 2021 at 11:04 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Feb 05, 2021 at 11:02:23PM +0100, Luka Kovacic wrote:
> > > > Hello Tom,
> > > >
> > > > On Thu, Feb 4, 2021 at 6:06 PM Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Feb 04, 2021 at 11:46:35AM +0100, Luka Kovacic wrote:
> > > > >
> > > > > > The '-h' flag is added to the 'env import' command to enable parsing
> > > > > > Marvell hw_info formatted environments.
> > > > > > This format is often used on Marvell Armada A37XX based devices to store
> > > > > > parameters like the board serial number, factory MAC addresses and some
> > > > > > other information.
> > > > > >
> > > > > > Currently this environment format can only be imported, not exported.
> > > > > > These parameters are usually written to the flash in the factory.
> > > > > >
> > > > > > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > > > > > successfully.
> > > > > >
> > > > > > Usage example:
> > > > > >  => sf probe
> > > > > >  => sf read ${loadaddr} 0x003E000A 0x1F0 # Read the environment from
> > > > > > SPI flash
> > > > > >  => env import -h ${loadaddr}
> > > > > >
> > > > > > Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> > > > > > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > > > > > Cc: Robert Marko <robert.marko@sartura.hr>
> > > > >
> > > > > So, the implementation itself is fine:
> > > > >
> > > > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > > > >
> > > > > And I take it as a given that we can't get the boards populated with an
> > > > > existing separator, so we need to solve this somehow or another.  I am
> > > > > however loathe to increase every platform by a handful of bytes (help
> > > > > text, code text) to cover this case, and adding #ifdef around the help
> > > > > text in particular would be very ugly. Is there any clean way to write
> > > > > this as a board-specific command?  I assume the overall intention is to
> > > > > import the factory env for the required information, store it in regular
> > > > > U-Boot environment and then never touch the factory area again.  Thanks!
> > > >
> > > > I have already considered the possibility of implementing this in a
> > > > board-specific way, but I tried implementing it here to avoid unnecessary
> > > > code duplication.
> > > > You are correct, the intention is to use this functionality as a migration
> > > > path. It's good to keep the factory area intact, in case it's still needed in
> > > > the future.
> > > > The Marvell hw_info area is limited to MACs and serial numbers by the
> > > > stock utility.
> > >
> > > OK.  And can we put this in the board-specific path somehow, relatively
> > > cleanly?
> >
> > I've been thinking about the following options:
> >  - Implementing a separate command under cmd/mvebu (hw_info), like done
> > in the stock U-Boot.
> > I'm not inclined to this one, as I don't see real utility in updating
> > this factory
> > area. A positive with this one is that it abstracts reading of the SPI flash.
> >
> >  - No command, automatic migration, directly from the SPI flash (implemented
> > in the board.c file.
> > I don't see this as a clean way of doing it, the Armada A37XX board.c is
> > already quite full of board-specific init.
> >
> >  - Implemented as is, but with #ifdef
> > This would break CONFIG_CMD_IMPORTENV into smaller pieces.
> > #ifdef wouldn't look too bad around the code, only the inline string changes
> > would be ugly.
> >
> > What would you recommend?
>
> I hate to say it but I prefer the first, especially if it ends up being
> like 50 lines of code or whatever to write the function and all that.
> Or the second option, if the first option really ends up being messy
> once you write it and the second one less messy.  Thanks!

Okay, I agree.

I think the first option should end up cleaner than both other options.
I'll do it this way.

>
> --
> Tom
Kind regards,
Luka

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments
  2021-02-05 22:51             ` Luka Kovacic
@ 2021-02-09  8:12               ` Stefan Roese
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Roese @ 2021-02-09  8:12 UTC (permalink / raw)
  To: u-boot

On 05.02.21 23:51, Luka Kovacic wrote:
> On Fri, Feb 5, 2021 at 11:40 PM Tom Rini <trini@konsulko.com> wrote:
>>
>> On Fri, Feb 05, 2021 at 11:36:25PM +0100, Luka Kovacic wrote:
>>> On Fri, Feb 5, 2021 at 11:04 PM Tom Rini <trini@konsulko.com> wrote:
>>>>
>>>> On Fri, Feb 05, 2021 at 11:02:23PM +0100, Luka Kovacic wrote:
>>>>> Hello Tom,
>>>>>
>>>>> On Thu, Feb 4, 2021 at 6:06 PM Tom Rini <trini@konsulko.com> wrote:
>>>>>>
>>>>>> On Thu, Feb 04, 2021 at 11:46:35AM +0100, Luka Kovacic wrote:
>>>>>>
>>>>>>> The '-h' flag is added to the 'env import' command to enable parsing
>>>>>>> Marvell hw_info formatted environments.
>>>>>>> This format is often used on Marvell Armada A37XX based devices to store
>>>>>>> parameters like the board serial number, factory MAC addresses and some
>>>>>>> other information.
>>>>>>>
>>>>>>> Currently this environment format can only be imported, not exported.
>>>>>>> These parameters are usually written to the flash in the factory.
>>>>>>>
>>>>>>> This functionality has been tested on the GST ESPRESSOBin-Ultra board
>>>>>>> successfully.
>>>>>>>
>>>>>>> Usage example:
>>>>>>>   => sf probe
>>>>>>>   => sf read ${loadaddr} 0x003E000A 0x1F0 # Read the environment from
>>>>>>> SPI flash
>>>>>>>   => env import -h ${loadaddr}
>>>>>>>
>>>>>>> Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
>>>>>>> Cc: Luka Perkov <luka.perkov@sartura.hr>
>>>>>>> Cc: Robert Marko <robert.marko@sartura.hr>
>>>>>>
>>>>>> So, the implementation itself is fine:
>>>>>>
>>>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>>>>>
>>>>>> And I take it as a given that we can't get the boards populated with an
>>>>>> existing separator, so we need to solve this somehow or another.  I am
>>>>>> however loathe to increase every platform by a handful of bytes (help
>>>>>> text, code text) to cover this case, and adding #ifdef around the help
>>>>>> text in particular would be very ugly. Is there any clean way to write
>>>>>> this as a board-specific command?  I assume the overall intention is to
>>>>>> import the factory env for the required information, store it in regular
>>>>>> U-Boot environment and then never touch the factory area again.  Thanks!
>>>>>
>>>>> I have already considered the possibility of implementing this in a
>>>>> board-specific way, but I tried implementing it here to avoid unnecessary
>>>>> code duplication.
>>>>> You are correct, the intention is to use this functionality as a migration
>>>>> path. It's good to keep the factory area intact, in case it's still needed in
>>>>> the future.
>>>>> The Marvell hw_info area is limited to MACs and serial numbers by the
>>>>> stock utility.
>>>>
>>>> OK.  And can we put this in the board-specific path somehow, relatively
>>>> cleanly?
>>>
>>> I've been thinking about the following options:
>>>   - Implementing a separate command under cmd/mvebu (hw_info), like done
>>> in the stock U-Boot.
>>> I'm not inclined to this one, as I don't see real utility in updating
>>> this factory
>>> area. A positive with this one is that it abstracts reading of the SPI flash.
>>>
>>>   - No command, automatic migration, directly from the SPI flash (implemented
>>> in the board.c file.
>>> I don't see this as a clean way of doing it, the Armada A37XX board.c is
>>> already quite full of board-specific init.
>>>
>>>   - Implemented as is, but with #ifdef
>>> This would break CONFIG_CMD_IMPORTENV into smaller pieces.
>>> #ifdef wouldn't look too bad around the code, only the inline string changes
>>> would be ugly.
>>>
>>> What would you recommend?
>>
>> I hate to say it but I prefer the first, especially if it ends up being
>> like 50 lines of code or whatever to write the function and all that.
>> Or the second option, if the first option really ends up being messy
>> once you write it and the second one less messy.  Thanks!
> 
> Okay, I agree.
> 
> I think the first option should end up cleaner than both other options.
> I'll do it this way.

Ok. Please rebase on latest mainline and send v2 once it's ready.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 0/3] Add support for the GST ESPRESSOBin-Ultra board
@ 2021-08-11  8:35 Gerald Kerma
  0 siblings, 0 replies; 12+ messages in thread
From: Gerald Kerma @ 2021-08-11  8:35 UTC (permalink / raw)
  To: U-Boot; +Cc: Kerma Gérald

From: Kerma Gérald <gandalf@gk2.net>

Add support for the GST ESPRESSOBin-Ultra board

Kerma Gérald (3):
  cmd: mvebu: Implement the Marvell hw_info command
  arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment
    variable
  arm: mvebu: Initial ESPRESSOBin-Ultra board support

 arch/arm/dts/Makefile                         |   1 +
 .../arm/dts/armada-3720-espressobin-ultra.dts | 202 ++++++++++++
 board/Marvell/mvebu_armada-37xx/MAINTAINERS   |   9 +
 board/Marvell/mvebu_armada-37xx/board.c       |  92 +++++-
 cmd/mvebu/Kconfig                             |  23 ++
 cmd/mvebu/Makefile                            |   2 +
 cmd/mvebu/hw_info.c                           | 312 ++++++++++++++++++
 .../mvebu_espressobin-ultra-88f3720_defconfig |  92 ++++++
 include/configs/mvebu_armada-37xx.h           |   1 +
 lib/hashtable.c                               |   2 +-
 10 files changed, 729 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm/dts/armada-3720-espressobin-ultra.dts
 create mode 100644 cmd/mvebu/hw_info.c
 create mode 100644 configs/mvebu_espressobin-ultra-88f3720_defconfig

-- 
2.30.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-08-11 12:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-04 10:46 [PATCH 0/3] Add support for the GST ESPRESSOBin-Ultra board Luka Kovacic
2021-02-04 10:46 ` [PATCH 1/3] cmd: env: Add 'env import -h' for Marvell hw_info formatted environments Luka Kovacic
2021-02-04 17:06   ` Tom Rini
2021-02-05 22:02     ` Luka Kovacic
2021-02-05 22:04       ` Tom Rini
2021-02-05 22:36         ` Luka Kovacic
2021-02-05 22:40           ` Tom Rini
2021-02-05 22:51             ` Luka Kovacic
2021-02-09  8:12               ` Stefan Roese
2021-02-04 10:46 ` [PATCH 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable Luka Kovacic
2021-02-04 10:46 ` [PATCH 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support Luka Kovacic
2021-08-11  8:35 [PATCH 0/3] Add support for the GST ESPRESSOBin-Ultra board Gerald Kerma

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.