[PATCH v2 0/3] Add support for the GST ESPRESSOBin-Ultra board

[PATCH v2 0/3] Add support for the GST ESPRESSOBin-Ultra board

[Luka Kovacic]

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: 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   |   8 +
 board/Marvell/mvebu_armada-37xx/board.c       |  92 +++++-
 cmd/mvebu/Kconfig                             |  23 ++
 cmd/mvebu/Makefile                            |   1 +
 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, 727 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.20.1

[PATCH v2 1/3] cmd: mvebu: Implement the Marvell hw_info command

[Luka Kovacic]

The hw_info command is implemented 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.
These parameters are usually written to the flash in the factory.

Currently the command supports reading/writing parameters and dumping the
current hw_info parameters.
EEPROM config pattern and checksum aren't supported.

This functionality has been tested on the GST ESPRESSOBin-Ultra board
successfully, both reading the stock U-Boot parameters in mainline U-Boot
and reading the parameters written by this command in the stock U-Boot.

Usage example:
 => hw_info load
 => saveenv

Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
Cc: Robert Marko <robert.marko@sartura.hr>
---
 cmd/mvebu/Kconfig   |  23 ++++
 cmd/mvebu/Makefile  |   1 +
 cmd/mvebu/hw_info.c | 312 ++++++++++++++++++++++++++++++++++++++++++++
 lib/hashtable.c     |   2 +-
 4 files changed, 337 insertions(+), 1 deletion(-)
 create mode 100644 cmd/mvebu/hw_info.c

diff --git a/cmd/mvebu/Kconfig b/cmd/mvebu/Kconfig
index ad10a572a3..a8e958e7c8 100644
--- a/cmd/mvebu/Kconfig
+++ b/cmd/mvebu/Kconfig
@@ -9,6 +9,29 @@ config CMD_MVEBU_BUBT
 	  For details about bubt command please see the documentation
 	  in doc/mvebu/cmd/bubt.txt
 
+config CMD_MVEBU_HW_INFO
+	bool "hw_info"
+	depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEBU
+	default n
+	help
+	  Enable loading of the Marvell hw_info parameters from the
+	  SPI flash hw_info area. Parameters (usually the board serial
+	  number and MAC addresses) are then imported into the
+	  existing U-Boot environment.
+	  Implementation of this command is compatible with the
+	  original Marvell U-Boot command. Reading and writing is
+	  supported.
+	  EEPROM config pattern and checksum aren't supported.
+
+config CMD_MVEBU_HW_INFO_OFFSET
+	hex "Marvell hw_info SPI flash offset"
+	depends on CMD_MVEBU_HW_INFO
+	default 0x3E0000
+	help
+	  This option defines the SPI flash offset of the Marvell
+	  hw_info area. This defaults to 0x3E0000 on most Armada
+	  A3720 platforms.
+
 choice
 	prompt "Flash for image"
 	default MVEBU_SPI_BOOT
diff --git a/cmd/mvebu/Makefile b/cmd/mvebu/Makefile
index 96829c48eb..2b5a8b37be 100644
--- a/cmd/mvebu/Makefile
+++ b/cmd/mvebu/Makefile
@@ -6,3 +6,4 @@
 
 
 obj-$(CONFIG_CMD_MVEBU_BUBT) += bubt.o
+obj-$(CONFIG_CMD_MVEBU_HW_INFO) += hw_info.o
diff --git a/cmd/mvebu/hw_info.c b/cmd/mvebu/hw_info.c
new file mode 100644
index 0000000000..1ef49d78d4
--- /dev/null
+++ b/cmd/mvebu/hw_info.c
@@ -0,0 +1,312 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Marvell hw_info command
+ * Helper command for interfacing with the Marvell hw_info parameters
+ *
+ * Copyright (c) 2021 Sartura Ltd.
+ * Copyright (c) 2018 Marvell International Ltd.
+ *
+ * Author: Luka Kovacic <luka.kovacic@sartura.hr>
+ */
+
+#include <command.h>
+#include <common.h>
+#include <env.h>
+#include <env_internal.h>
+#include <spi.h>
+#include <spi_flash.h>
+
+#define HW_INFO_SPI_FLASH_OFFSET	CONFIG_CMD_MVEBU_HW_INFO_OFFSET
+
+#define HW_INFO_MAX_ENV_SIZE		0x1F0
+#define HW_INFO_ENV_OFFSET		0xA
+#define HW_INFO_ENV_SEP			0x20
+
+#define HW_INFO_MAX_NAME_LEN		32
+
+static char hw_info_allowed_parameters[][HW_INFO_MAX_NAME_LEN] = {
+	"pcb_slm",
+	"pcb_rev",
+	"eco_rev",
+	"pcb_sn",
+	"ethaddr",
+	"eth1addr",
+	"eth2addr",
+	"eth3addr",
+	"eth4addr",
+	"eth5addr",
+	"eth6addr",
+	"eth7addr",
+	"eth8addr",
+	"eth9addr",
+};
+
+static int hw_info_allowed_param_count = (sizeof(hw_info_allowed_parameters) /
+					sizeof(hw_info_allowed_parameters[0]));
+
+static int hw_info_check_parameter(char *name)
+{
+	int idx;
+
+	for (idx = 0; idx < hw_info_allowed_param_count; idx++) {
+		if (strcmp(name, hw_info_allowed_parameters[idx]) == 0)
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int read_spi_flash_offset(char *buf, int offset)
+{
+	struct spi_flash *flash;
+	int ret;
+
+	flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
+				CONFIG_SF_DEFAULT_CS,
+				CONFIG_SF_DEFAULT_SPEED,
+				CONFIG_SF_DEFAULT_MODE);
+
+	if (!flash) {
+		printf("Error - unable to probe SPI flash.\n");
+		return -EIO;
+	}
+
+	ret = spi_flash_read(flash, offset, HW_INFO_MAX_ENV_SIZE, buf);
+	if (ret) {
+		printf("Error - unable to read hw_info environment from SPI flash.\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static int write_spi_flash_offset(char *buf, int offset, ssize_t size)
+{
+	ssize_t safe_size, erase_size;
+	struct spi_flash *flash;
+	int ret;
+
+	flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
+				CONFIG_SF_DEFAULT_CS,
+				CONFIG_SF_DEFAULT_SPEED,
+				CONFIG_SF_DEFAULT_MODE);
+
+	if (!flash) {
+		printf("Error - unable to probe SPI flash.\n");
+		return -EIO;
+	}
+
+	safe_size = size > HW_INFO_MAX_ENV_SIZE ? HW_INFO_MAX_ENV_SIZE : size;
+	erase_size = safe_size +
+		     (flash->erase_size - safe_size % flash->erase_size);
+	ret = spi_flash_erase(flash, HW_INFO_SPI_FLASH_OFFSET, erase_size);
+	if (ret) {
+		printf("Error - unable to erase the hw_info area on SPI flash.\n");
+		return ret;
+	}
+	ret = spi_flash_write(flash, offset, safe_size, buf);
+	if (ret) {
+		printf("Error - unable to write hw_info parameters to SPI flash.\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static int cmd_hw_info_dump(void)
+{
+	char buffer[HW_INFO_MAX_ENV_SIZE];
+	struct hsearch_data htab;
+	char *res = NULL;
+	ssize_t len;
+	int ret = 0;
+
+	ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
+				    HW_INFO_ENV_OFFSET);
+	if (ret)
+		goto err;
+	memset(&htab, 0, sizeof(htab));
+	if (!hcreate_r(HW_INFO_MAX_ENV_SIZE, &htab)) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	if (!himport_r(&htab, buffer, HW_INFO_MAX_ENV_SIZE,
+		       HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
+		ret = -EFAULT;
+		goto err_htab;
+	}
+
+	len = hexport_r(&htab, '\n', H_HIDE_DOT, &res, 0, 0, NULL);
+	if (len > 0) {
+		printf("Parameters (hw_info):\n");
+		puts(res);
+		free(res);
+		ret = 0;
+		goto ret_htab;
+	}
+ret_htab:
+	hdestroy_r(&htab);
+	return ret;
+err_htab:
+	hdestroy_r(&htab);
+err:
+	printf("## Error: cannot store hw_info parameters to SPI flash\n");
+	return ret;
+}
+
+static int cmd_hw_info_load(bool print_env)
+{
+	char buffer[HW_INFO_MAX_ENV_SIZE];
+	char *res = NULL;
+	ssize_t len;
+	int ret = 0;
+
+	ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
+				    HW_INFO_ENV_OFFSET);
+	if (ret)
+		goto err;
+	if (!himport_r(&env_htab, buffer, HW_INFO_MAX_ENV_SIZE,
+		       HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
+		ret = -EFAULT;
+		goto err;
+	}
+
+	printf("Successfully imported the Marvell hw_info parameters.\n");
+	if (!print_env)
+		return 0;
+
+	len = hexport_r(&env_htab, '\n', H_HIDE_DOT, &res, 0, 0, NULL);
+	if (len > 0) {
+		printf("Updated environment:\n");
+		puts(res);
+		free(res);
+		return 0;
+	}
+err:
+	printf("## Error: cannot import hw_info parameters\n");
+	return ret;
+}
+
+static int cmd_hw_info_store(char *name)
+{
+	char buffer[HW_INFO_MAX_ENV_SIZE];
+	struct env_entry e, *ep, *rv;
+	struct hsearch_data htab;
+	char *res = NULL;
+	ssize_t len;
+	int ret = 0;
+
+	ret = hw_info_check_parameter(name);
+	if (ret) {
+		printf("Invalid parameter %s, stopping.\n", name);
+		goto err;
+	}
+
+	ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
+				    HW_INFO_ENV_OFFSET);
+	if (ret)
+		goto err;
+	memset(&htab, 0, sizeof(htab));
+	if (!hcreate_r(HW_INFO_MAX_ENV_SIZE, &htab)) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	if (!himport_r(&htab, buffer, HW_INFO_MAX_ENV_SIZE,
+		       HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
+		ret = -EFAULT;
+		goto err_htab;
+	}
+
+	e.key = name;
+	e.data = NULL;
+	if (!hsearch_r(e, ENV_FIND, &ep, &env_htab, H_HIDE_DOT)) {
+		ret = -ENOENT;
+		goto err_htab;
+	}
+	if (!ep) {
+		ret = -ENOENT;
+		goto err_htab;
+	}
+
+	printf("Storing %s=%s to hw_info...\n", ep->key, ep->data);
+
+	e.key = ep->key;
+	e.data = ep->data;
+	if (!hsearch_r(e, ENV_ENTER, &rv, &htab, H_HIDE_DOT)) {
+		ret = -EINVAL;
+		goto err_htab;
+	}
+	if (!rv) {
+		ret = -EINVAL;
+		goto err_htab;
+	}
+	len = hexport_r(&htab, HW_INFO_ENV_SEP, H_MATCH_KEY | H_MATCH_IDENT,
+			&res, 0, 0, NULL);
+	if (len <= 0) {
+		free(res);
+		goto ret_htab;
+	}
+	ret = write_spi_flash_offset(res, HW_INFO_SPI_FLASH_OFFSET +
+				     HW_INFO_ENV_OFFSET, len);
+	free(res);
+	if (ret)
+		goto err_htab;
+
+	printf("Successfully stored the Marvell hw_info parameters.\n");
+	return 0;
+ret_htab:
+	hdestroy_r(&htab);
+	return ret;
+err_htab:
+	hdestroy_r(&htab);
+err:
+	printf("## Error: cannot store hw_info parameters to SPI flash\n");
+	return ret;
+}
+
+static int do_hw_info(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+{
+	const char *cmd = argv[1];
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	if (!strcmp(cmd, "dump")) {
+		if (cmd_hw_info_dump())
+			return -EINVAL;
+	} else if (!strcmp(cmd, "load")) {
+		if (cmd_hw_info_load(true))
+			return -EINVAL;
+	} else if (!strcmp(cmd, "store")) {
+		if (cmd_hw_info_store(argv[2]))
+			return -EINVAL;
+	} else {
+		return CMD_RET_USAGE;
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+U_BOOT_CMD(
+	hw_info, 3, 0, do_hw_info,
+	"Marvell hw_info utility",
+	"hw_info\n"
+	"\tdump                        - Dump all hw_info parameters from SPI flash\n"
+	"\tload                        - Load all hw_info parameters from hw_info to environment variables\n"
+	"\tstore <env_name>            - Store specific hw_info parameter from envirnoment variable to SPI flash\n"
+	"\nSupported hw_info parameters:\n"
+	"\tpcb_slm       PCB SLM number\n"
+	"\tpcb_rev       PCB revision number\n"
+	"\teco_rev       ECO revision number\n"
+	"\tpcb_sn        PCB SN\n"
+	"\tethaddr       first MAC address\n"
+	"\teth1addr      second MAC address\n"
+	"\teth2addr      third MAC address\n"
+	"\teth3addr      fourth MAC address\n"
+	"\teth4addr      fifth MAC address\n"
+	"\teth5addr      sixth MAC address\n"
+	"\teth6addr      seventh MAC address\n"
+	"\teth7addr      eighth MAC address\n"
+	"\teth8addr      ninth MAC address\n"
+	"\teth9addr      tenth MAC address\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

[PATCH v2 1/3] cmd: mvebu: Implement the Marvell hw_info command

[Tom Rini]

On Mon, Feb 15, 2021 at 08:59:32PM +0100, Luka Kovacic wrote:

> The hw_info command is implemented 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.
> These parameters are usually written to the flash in the factory.
> 
> Currently the command supports reading/writing parameters and dumping the
> current hw_info parameters.
> EEPROM config pattern and checksum aren't supported.
> 
> This functionality has been tested on the GST ESPRESSOBin-Ultra board
> successfully, both reading the stock U-Boot parameters in mainline U-Boot
> and reading the parameters written by this command in the stock U-Boot.
> 
> Usage example:
>  => hw_info load
>  => saveenv
> 
> Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> Cc: Robert Marko <robert.marko@sartura.hr>

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

-- 
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/20210215/69f2764e/attachment.sig>

[PATCH v2 1/3] cmd: mvebu: Implement the Marvell hw_info command

[Pali Rohár]

On Monday 15 February 2021 20:59:32 Luka Kovacic wrote:
> The hw_info command is implemented 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.
> These parameters are usually written to the flash in the factory.

CCing Marek, as he does not like introduction of another vendor custom
command into mainline U-Boot.

> Currently the command supports reading/writing parameters and dumping the
> current hw_info parameters.
> EEPROM config pattern and checksum aren't supported.
> 
> This functionality has been tested on the GST ESPRESSOBin-Ultra board
> successfully, both reading the stock U-Boot parameters in mainline U-Boot
> and reading the parameters written by this command in the stock U-Boot.
> 
> Usage example:
>  => hw_info load
>  => saveenv

I'm looking at the code and it loads MAC addresses for espressobin
ultra variant. But for other espressobin variants there is already code
which sets correct eth*addr and fdtfile variables in board_late_init()
function.

I think it should be relatively easy to implement reading these
variables from SPI to ENV in board_late_init() function. Which would
mean that it completely replaces this custom 'hw_info load' command.

Could you look at it? This would have a benefit that 'env default -a'
would always load correct mac addresses stored in SPI, so it would work
just via default u-boot commands without need to use something vendor
specific.

> Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> Cc: Robert Marko <robert.marko@sartura.hr>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> ---
>  cmd/mvebu/Kconfig   |  23 ++++
>  cmd/mvebu/Makefile  |   1 +
>  cmd/mvebu/hw_info.c | 312 ++++++++++++++++++++++++++++++++++++++++++++
>  lib/hashtable.c     |   2 +-
>  4 files changed, 337 insertions(+), 1 deletion(-)
>  create mode 100644 cmd/mvebu/hw_info.c
> 
> diff --git a/cmd/mvebu/Kconfig b/cmd/mvebu/Kconfig
> index ad10a572a3..a8e958e7c8 100644
> --- a/cmd/mvebu/Kconfig
> +++ b/cmd/mvebu/Kconfig
> @@ -9,6 +9,29 @@ config CMD_MVEBU_BUBT
>  	  For details about bubt command please see the documentation
>  	  in doc/mvebu/cmd/bubt.txt
>  
> +config CMD_MVEBU_HW_INFO
> +	bool "hw_info"
> +	depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEBU
> +	default n
> +	help
> +	  Enable loading of the Marvell hw_info parameters from the
> +	  SPI flash hw_info area. Parameters (usually the board serial
> +	  number and MAC addresses) are then imported into the
> +	  existing U-Boot environment.
> +	  Implementation of this command is compatible with the
> +	  original Marvell U-Boot command. Reading and writing is
> +	  supported.
> +	  EEPROM config pattern and checksum aren't supported.
> +
> +config CMD_MVEBU_HW_INFO_OFFSET
> +	hex "Marvell hw_info SPI flash offset"
> +	depends on CMD_MVEBU_HW_INFO
> +	default 0x3E0000
> +	help
> +	  This option defines the SPI flash offset of the Marvell
> +	  hw_info area. This defaults to 0x3E0000 on most Armada
> +	  A3720 platforms.
> +
>  choice
>  	prompt "Flash for image"
>  	default MVEBU_SPI_BOOT
> diff --git a/cmd/mvebu/Makefile b/cmd/mvebu/Makefile
> index 96829c48eb..2b5a8b37be 100644
> --- a/cmd/mvebu/Makefile
> +++ b/cmd/mvebu/Makefile
> @@ -6,3 +6,4 @@
>  
>  
>  obj-$(CONFIG_CMD_MVEBU_BUBT) += bubt.o
> +obj-$(CONFIG_CMD_MVEBU_HW_INFO) += hw_info.o
> diff --git a/cmd/mvebu/hw_info.c b/cmd/mvebu/hw_info.c
> new file mode 100644
> index 0000000000..1ef49d78d4
> --- /dev/null
> +++ b/cmd/mvebu/hw_info.c
> @@ -0,0 +1,312 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Marvell hw_info command
> + * Helper command for interfacing with the Marvell hw_info parameters
> + *
> + * Copyright (c) 2021 Sartura Ltd.
> + * Copyright (c) 2018 Marvell International Ltd.
> + *
> + * Author: Luka Kovacic <luka.kovacic@sartura.hr>
> + */
> +
> +#include <command.h>
> +#include <common.h>
> +#include <env.h>
> +#include <env_internal.h>
> +#include <spi.h>
> +#include <spi_flash.h>
> +
> +#define HW_INFO_SPI_FLASH_OFFSET	CONFIG_CMD_MVEBU_HW_INFO_OFFSET
> +
> +#define HW_INFO_MAX_ENV_SIZE		0x1F0
> +#define HW_INFO_ENV_OFFSET		0xA
> +#define HW_INFO_ENV_SEP			0x20
> +
> +#define HW_INFO_MAX_NAME_LEN		32
> +
> +static char hw_info_allowed_parameters[][HW_INFO_MAX_NAME_LEN] = {
> +	"pcb_slm",
> +	"pcb_rev",
> +	"eco_rev",
> +	"pcb_sn",
> +	"ethaddr",
> +	"eth1addr",
> +	"eth2addr",
> +	"eth3addr",
> +	"eth4addr",
> +	"eth5addr",
> +	"eth6addr",
> +	"eth7addr",
> +	"eth8addr",
> +	"eth9addr",
> +};
> +
> +static int hw_info_allowed_param_count = (sizeof(hw_info_allowed_parameters) /
> +					sizeof(hw_info_allowed_parameters[0]));
> +
> +static int hw_info_check_parameter(char *name)
> +{
> +	int idx;
> +
> +	for (idx = 0; idx < hw_info_allowed_param_count; idx++) {
> +		if (strcmp(name, hw_info_allowed_parameters[idx]) == 0)
> +			return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int read_spi_flash_offset(char *buf, int offset)
> +{
> +	struct spi_flash *flash;
> +	int ret;
> +
> +	flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
> +				CONFIG_SF_DEFAULT_CS,
> +				CONFIG_SF_DEFAULT_SPEED,
> +				CONFIG_SF_DEFAULT_MODE);
> +
> +	if (!flash) {
> +		printf("Error - unable to probe SPI flash.\n");
> +		return -EIO;
> +	}
> +
> +	ret = spi_flash_read(flash, offset, HW_INFO_MAX_ENV_SIZE, buf);
> +	if (ret) {
> +		printf("Error - unable to read hw_info environment from SPI flash.\n");
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int write_spi_flash_offset(char *buf, int offset, ssize_t size)
> +{
> +	ssize_t safe_size, erase_size;
> +	struct spi_flash *flash;
> +	int ret;
> +
> +	flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
> +				CONFIG_SF_DEFAULT_CS,
> +				CONFIG_SF_DEFAULT_SPEED,
> +				CONFIG_SF_DEFAULT_MODE);
> +
> +	if (!flash) {
> +		printf("Error - unable to probe SPI flash.\n");
> +		return -EIO;
> +	}
> +
> +	safe_size = size > HW_INFO_MAX_ENV_SIZE ? HW_INFO_MAX_ENV_SIZE : size;
> +	erase_size = safe_size +
> +		     (flash->erase_size - safe_size % flash->erase_size);
> +	ret = spi_flash_erase(flash, HW_INFO_SPI_FLASH_OFFSET, erase_size);
> +	if (ret) {
> +		printf("Error - unable to erase the hw_info area on SPI flash.\n");
> +		return ret;
> +	}
> +	ret = spi_flash_write(flash, offset, safe_size, buf);
> +	if (ret) {
> +		printf("Error - unable to write hw_info parameters to SPI flash.\n");
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int cmd_hw_info_dump(void)
> +{
> +	char buffer[HW_INFO_MAX_ENV_SIZE];
> +	struct hsearch_data htab;
> +	char *res = NULL;
> +	ssize_t len;
> +	int ret = 0;
> +
> +	ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
> +				    HW_INFO_ENV_OFFSET);
> +	if (ret)
> +		goto err;
> +	memset(&htab, 0, sizeof(htab));
> +	if (!hcreate_r(HW_INFO_MAX_ENV_SIZE, &htab)) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +	if (!himport_r(&htab, buffer, HW_INFO_MAX_ENV_SIZE,
> +		       HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
> +		ret = -EFAULT;
> +		goto err_htab;
> +	}
> +
> +	len = hexport_r(&htab, '\n', H_HIDE_DOT, &res, 0, 0, NULL);
> +	if (len > 0) {
> +		printf("Parameters (hw_info):\n");
> +		puts(res);
> +		free(res);
> +		ret = 0;
> +		goto ret_htab;
> +	}
> +ret_htab:
> +	hdestroy_r(&htab);
> +	return ret;
> +err_htab:
> +	hdestroy_r(&htab);
> +err:
> +	printf("## Error: cannot store hw_info parameters to SPI flash\n");
> +	return ret;
> +}
> +
> +static int cmd_hw_info_load(bool print_env)
> +{
> +	char buffer[HW_INFO_MAX_ENV_SIZE];
> +	char *res = NULL;
> +	ssize_t len;
> +	int ret = 0;
> +
> +	ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
> +				    HW_INFO_ENV_OFFSET);
> +	if (ret)
> +		goto err;
> +	if (!himport_r(&env_htab, buffer, HW_INFO_MAX_ENV_SIZE,
> +		       HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +
> +	printf("Successfully imported the Marvell hw_info parameters.\n");
> +	if (!print_env)
> +		return 0;
> +
> +	len = hexport_r(&env_htab, '\n', H_HIDE_DOT, &res, 0, 0, NULL);
> +	if (len > 0) {
> +		printf("Updated environment:\n");
> +		puts(res);
> +		free(res);
> +		return 0;
> +	}
> +err:
> +	printf("## Error: cannot import hw_info parameters\n");
> +	return ret;
> +}
> +
> +static int cmd_hw_info_store(char *name)
> +{
> +	char buffer[HW_INFO_MAX_ENV_SIZE];
> +	struct env_entry e, *ep, *rv;
> +	struct hsearch_data htab;
> +	char *res = NULL;
> +	ssize_t len;
> +	int ret = 0;
> +
> +	ret = hw_info_check_parameter(name);
> +	if (ret) {
> +		printf("Invalid parameter %s, stopping.\n", name);
> +		goto err;
> +	}
> +
> +	ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
> +				    HW_INFO_ENV_OFFSET);
> +	if (ret)
> +		goto err;
> +	memset(&htab, 0, sizeof(htab));
> +	if (!hcreate_r(HW_INFO_MAX_ENV_SIZE, &htab)) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +	if (!himport_r(&htab, buffer, HW_INFO_MAX_ENV_SIZE,
> +		       HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
> +		ret = -EFAULT;
> +		goto err_htab;
> +	}
> +
> +	e.key = name;
> +	e.data = NULL;
> +	if (!hsearch_r(e, ENV_FIND, &ep, &env_htab, H_HIDE_DOT)) {
> +		ret = -ENOENT;
> +		goto err_htab;
> +	}
> +	if (!ep) {
> +		ret = -ENOENT;
> +		goto err_htab;
> +	}
> +
> +	printf("Storing %s=%s to hw_info...\n", ep->key, ep->data);
> +
> +	e.key = ep->key;
> +	e.data = ep->data;
> +	if (!hsearch_r(e, ENV_ENTER, &rv, &htab, H_HIDE_DOT)) {
> +		ret = -EINVAL;
> +		goto err_htab;
> +	}
> +	if (!rv) {
> +		ret = -EINVAL;
> +		goto err_htab;
> +	}
> +	len = hexport_r(&htab, HW_INFO_ENV_SEP, H_MATCH_KEY | H_MATCH_IDENT,
> +			&res, 0, 0, NULL);
> +	if (len <= 0) {
> +		free(res);
> +		goto ret_htab;
> +	}
> +	ret = write_spi_flash_offset(res, HW_INFO_SPI_FLASH_OFFSET +
> +				     HW_INFO_ENV_OFFSET, len);
> +	free(res);
> +	if (ret)
> +		goto err_htab;
> +
> +	printf("Successfully stored the Marvell hw_info parameters.\n");
> +	return 0;
> +ret_htab:
> +	hdestroy_r(&htab);
> +	return ret;
> +err_htab:
> +	hdestroy_r(&htab);
> +err:
> +	printf("## Error: cannot store hw_info parameters to SPI flash\n");
> +	return ret;
> +}
> +
> +static int do_hw_info(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> +{
> +	const char *cmd = argv[1];
> +
> +	if (argc < 2)
> +		return CMD_RET_USAGE;
> +
> +	if (!strcmp(cmd, "dump")) {
> +		if (cmd_hw_info_dump())
> +			return -EINVAL;
> +	} else if (!strcmp(cmd, "load")) {
> +		if (cmd_hw_info_load(true))
> +			return -EINVAL;
> +	} else if (!strcmp(cmd, "store")) {
> +		if (cmd_hw_info_store(argv[2]))
> +			return -EINVAL;
> +	} else {
> +		return CMD_RET_USAGE;
> +	}
> +
> +	return CMD_RET_SUCCESS;
> +}
> +
> +U_BOOT_CMD(
> +	hw_info, 3, 0, do_hw_info,
> +	"Marvell hw_info utility",
> +	"hw_info\n"
> +	"\tdump                        - Dump all hw_info parameters from SPI flash\n"
> +	"\tload                        - Load all hw_info parameters from hw_info to environment variables\n"
> +	"\tstore <env_name>            - Store specific hw_info parameter from envirnoment variable to SPI flash\n"
> +	"\nSupported hw_info parameters:\n"
> +	"\tpcb_slm       PCB SLM number\n"
> +	"\tpcb_rev       PCB revision number\n"
> +	"\teco_rev       ECO revision number\n"
> +	"\tpcb_sn        PCB SN\n"
> +	"\tethaddr       first MAC address\n"
> +	"\teth1addr      second MAC address\n"
> +	"\teth2addr      third MAC address\n"
> +	"\teth3addr      fourth MAC address\n"
> +	"\teth4addr      fifth MAC address\n"
> +	"\teth5addr      sixth MAC address\n"
> +	"\teth6addr      seventh MAC address\n"
> +	"\teth7addr      eighth MAC address\n"
> +	"\teth8addr      ninth MAC address\n"
> +	"\teth9addr      tenth MAC address\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,

[PATCH v2 1/3] cmd: mvebu: Implement the Marvell hw_info command

[Luka Kovacic]

Hello Pali,

On Sat, Feb 27, 2021 at 2:09 AM Pali Roh?r <pali@kernel.org> wrote:
>
> On Monday 15 February 2021 20:59:32 Luka Kovacic wrote:
> > The hw_info command is implemented 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.
> > These parameters are usually written to the flash in the factory.
>
> CCing Marek, as he does not like introduction of another vendor custom
> command into mainline U-Boot.
>
> > Currently the command supports reading/writing parameters and dumping the
> > current hw_info parameters.
> > EEPROM config pattern and checksum aren't supported.
> >
> > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > successfully, both reading the stock U-Boot parameters in mainline U-Boot
> > and reading the parameters written by this command in the stock U-Boot.
> >
> > Usage example:
> >  => hw_info load
> >  => saveenv
>
> I'm looking at the code and it loads MAC addresses for espressobin
> ultra variant. But for other espressobin variants there is already code
> which sets correct eth*addr and fdtfile variables in board_late_init()
> function.
>
> I think it should be relatively easy to implement reading these
> variables from SPI to ENV in board_late_init() function. Which would
> mean that it completely replaces this custom 'hw_info load' command.

The board.c file looked really cluttered to me in the current state and
adding more board-specific code wouldn't help it.
Also there are some other potentially useful parameters in the hw_info
SPI flash area, which might be of use (board serial number and some
other values).

>
> Could you look at it? This would have a benefit that 'env default -a'
> would always load correct mac addresses stored in SPI, so it would work
> just via default u-boot commands without need to use something vendor
> specific.

I do agree that a vendor specific solution isn't too elegant.
I might consider the mac command, which Marek has suggested.

Kind regards,
Luka

>
> > Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > Cc: Robert Marko <robert.marko@sartura.hr>
> > Reviewed-by: Tom Rini <trini@konsulko.com>
> > ---
> >  cmd/mvebu/Kconfig   |  23 ++++
> >  cmd/mvebu/Makefile  |   1 +
> >  cmd/mvebu/hw_info.c | 312 ++++++++++++++++++++++++++++++++++++++++++++
> >  lib/hashtable.c     |   2 +-
> >  4 files changed, 337 insertions(+), 1 deletion(-)
> >  create mode 100644 cmd/mvebu/hw_info.c
> >
> > diff --git a/cmd/mvebu/Kconfig b/cmd/mvebu/Kconfig
> > index ad10a572a3..a8e958e7c8 100644
> > --- a/cmd/mvebu/Kconfig
> > +++ b/cmd/mvebu/Kconfig
> > @@ -9,6 +9,29 @@ config CMD_MVEBU_BUBT
> >         For details about bubt command please see the documentation
> >         in doc/mvebu/cmd/bubt.txt
> >
> > +config CMD_MVEBU_HW_INFO
> > +     bool "hw_info"
> > +     depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEBU
> > +     default n
> > +     help
> > +       Enable loading of the Marvell hw_info parameters from the
> > +       SPI flash hw_info area. Parameters (usually the board serial
> > +       number and MAC addresses) are then imported into the
> > +       existing U-Boot environment.
> > +       Implementation of this command is compatible with the
> > +       original Marvell U-Boot command. Reading and writing is
> > +       supported.
> > +       EEPROM config pattern and checksum aren't supported.
> > +
> > +config CMD_MVEBU_HW_INFO_OFFSET
> > +     hex "Marvell hw_info SPI flash offset"
> > +     depends on CMD_MVEBU_HW_INFO
> > +     default 0x3E0000
> > +     help
> > +       This option defines the SPI flash offset of the Marvell
> > +       hw_info area. This defaults to 0x3E0000 on most Armada
> > +       A3720 platforms.
> > +
> >  choice
> >       prompt "Flash for image"
> >       default MVEBU_SPI_BOOT
> > diff --git a/cmd/mvebu/Makefile b/cmd/mvebu/Makefile
> > index 96829c48eb..2b5a8b37be 100644
> > --- a/cmd/mvebu/Makefile
> > +++ b/cmd/mvebu/Makefile
> > @@ -6,3 +6,4 @@
> >
> >
> >  obj-$(CONFIG_CMD_MVEBU_BUBT) += bubt.o
> > +obj-$(CONFIG_CMD_MVEBU_HW_INFO) += hw_info.o
> > diff --git a/cmd/mvebu/hw_info.c b/cmd/mvebu/hw_info.c
> > new file mode 100644
> > index 0000000000..1ef49d78d4
> > --- /dev/null
> > +++ b/cmd/mvebu/hw_info.c
> > @@ -0,0 +1,312 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Marvell hw_info command
> > + * Helper command for interfacing with the Marvell hw_info parameters
> > + *
> > + * Copyright (c) 2021 Sartura Ltd.
> > + * Copyright (c) 2018 Marvell International Ltd.
> > + *
> > + * Author: Luka Kovacic <luka.kovacic@sartura.hr>
> > + */
> > +
> > +#include <command.h>
> > +#include <common.h>
> > +#include <env.h>
> > +#include <env_internal.h>
> > +#include <spi.h>
> > +#include <spi_flash.h>
> > +
> > +#define HW_INFO_SPI_FLASH_OFFSET     CONFIG_CMD_MVEBU_HW_INFO_OFFSET
> > +
> > +#define HW_INFO_MAX_ENV_SIZE         0x1F0
> > +#define HW_INFO_ENV_OFFSET           0xA
> > +#define HW_INFO_ENV_SEP                      0x20
> > +
> > +#define HW_INFO_MAX_NAME_LEN         32
> > +
> > +static char hw_info_allowed_parameters[][HW_INFO_MAX_NAME_LEN] = {
> > +     "pcb_slm",
> > +     "pcb_rev",
> > +     "eco_rev",
> > +     "pcb_sn",
> > +     "ethaddr",
> > +     "eth1addr",
> > +     "eth2addr",
> > +     "eth3addr",
> > +     "eth4addr",
> > +     "eth5addr",
> > +     "eth6addr",
> > +     "eth7addr",
> > +     "eth8addr",
> > +     "eth9addr",
> > +};
> > +
> > +static int hw_info_allowed_param_count = (sizeof(hw_info_allowed_parameters) /
> > +                                     sizeof(hw_info_allowed_parameters[0]));
> > +
> > +static int hw_info_check_parameter(char *name)
> > +{
> > +     int idx;
> > +
> > +     for (idx = 0; idx < hw_info_allowed_param_count; idx++) {
> > +             if (strcmp(name, hw_info_allowed_parameters[idx]) == 0)
> > +                     return 0;
> > +     }
> > +
> > +     return -EINVAL;
> > +}
> > +
> > +static int read_spi_flash_offset(char *buf, int offset)
> > +{
> > +     struct spi_flash *flash;
> > +     int ret;
> > +
> > +     flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
> > +                             CONFIG_SF_DEFAULT_CS,
> > +                             CONFIG_SF_DEFAULT_SPEED,
> > +                             CONFIG_SF_DEFAULT_MODE);
> > +
> > +     if (!flash) {
> > +             printf("Error - unable to probe SPI flash.\n");
> > +             return -EIO;
> > +     }
> > +
> > +     ret = spi_flash_read(flash, offset, HW_INFO_MAX_ENV_SIZE, buf);
> > +     if (ret) {
> > +             printf("Error - unable to read hw_info environment from SPI flash.\n");
> > +             return ret;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static int write_spi_flash_offset(char *buf, int offset, ssize_t size)
> > +{
> > +     ssize_t safe_size, erase_size;
> > +     struct spi_flash *flash;
> > +     int ret;
> > +
> > +     flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
> > +                             CONFIG_SF_DEFAULT_CS,
> > +                             CONFIG_SF_DEFAULT_SPEED,
> > +                             CONFIG_SF_DEFAULT_MODE);
> > +
> > +     if (!flash) {
> > +             printf("Error - unable to probe SPI flash.\n");
> > +             return -EIO;
> > +     }
> > +
> > +     safe_size = size > HW_INFO_MAX_ENV_SIZE ? HW_INFO_MAX_ENV_SIZE : size;
> > +     erase_size = safe_size +
> > +                  (flash->erase_size - safe_size % flash->erase_size);
> > +     ret = spi_flash_erase(flash, HW_INFO_SPI_FLASH_OFFSET, erase_size);
> > +     if (ret) {
> > +             printf("Error - unable to erase the hw_info area on SPI flash.\n");
> > +             return ret;
> > +     }
> > +     ret = spi_flash_write(flash, offset, safe_size, buf);
> > +     if (ret) {
> > +             printf("Error - unable to write hw_info parameters to SPI flash.\n");
> > +             return ret;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static int cmd_hw_info_dump(void)
> > +{
> > +     char buffer[HW_INFO_MAX_ENV_SIZE];
> > +     struct hsearch_data htab;
> > +     char *res = NULL;
> > +     ssize_t len;
> > +     int ret = 0;
> > +
> > +     ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
> > +                                 HW_INFO_ENV_OFFSET);
> > +     if (ret)
> > +             goto err;
> > +     memset(&htab, 0, sizeof(htab));
> > +     if (!hcreate_r(HW_INFO_MAX_ENV_SIZE, &htab)) {
> > +             ret = -ENOMEM;
> > +             goto err;
> > +     }
> > +     if (!himport_r(&htab, buffer, HW_INFO_MAX_ENV_SIZE,
> > +                    HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
> > +             ret = -EFAULT;
> > +             goto err_htab;
> > +     }
> > +
> > +     len = hexport_r(&htab, '\n', H_HIDE_DOT, &res, 0, 0, NULL);
> > +     if (len > 0) {
> > +             printf("Parameters (hw_info):\n");
> > +             puts(res);
> > +             free(res);
> > +             ret = 0;
> > +             goto ret_htab;
> > +     }
> > +ret_htab:
> > +     hdestroy_r(&htab);
> > +     return ret;
> > +err_htab:
> > +     hdestroy_r(&htab);
> > +err:
> > +     printf("## Error: cannot store hw_info parameters to SPI flash\n");
> > +     return ret;
> > +}
> > +
> > +static int cmd_hw_info_load(bool print_env)
> > +{
> > +     char buffer[HW_INFO_MAX_ENV_SIZE];
> > +     char *res = NULL;
> > +     ssize_t len;
> > +     int ret = 0;
> > +
> > +     ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
> > +                                 HW_INFO_ENV_OFFSET);
> > +     if (ret)
> > +             goto err;
> > +     if (!himport_r(&env_htab, buffer, HW_INFO_MAX_ENV_SIZE,
> > +                    HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
> > +             ret = -EFAULT;
> > +             goto err;
> > +     }
> > +
> > +     printf("Successfully imported the Marvell hw_info parameters.\n");
> > +     if (!print_env)
> > +             return 0;
> > +
> > +     len = hexport_r(&env_htab, '\n', H_HIDE_DOT, &res, 0, 0, NULL);
> > +     if (len > 0) {
> > +             printf("Updated environment:\n");
> > +             puts(res);
> > +             free(res);
> > +             return 0;
> > +     }
> > +err:
> > +     printf("## Error: cannot import hw_info parameters\n");
> > +     return ret;
> > +}
> > +
> > +static int cmd_hw_info_store(char *name)
> > +{
> > +     char buffer[HW_INFO_MAX_ENV_SIZE];
> > +     struct env_entry e, *ep, *rv;
> > +     struct hsearch_data htab;
> > +     char *res = NULL;
> > +     ssize_t len;
> > +     int ret = 0;
> > +
> > +     ret = hw_info_check_parameter(name);
> > +     if (ret) {
> > +             printf("Invalid parameter %s, stopping.\n", name);
> > +             goto err;
> > +     }
> > +
> > +     ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
> > +                                 HW_INFO_ENV_OFFSET);
> > +     if (ret)
> > +             goto err;
> > +     memset(&htab, 0, sizeof(htab));
> > +     if (!hcreate_r(HW_INFO_MAX_ENV_SIZE, &htab)) {
> > +             ret = -ENOMEM;
> > +             goto err;
> > +     }
> > +     if (!himport_r(&htab, buffer, HW_INFO_MAX_ENV_SIZE,
> > +                    HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
> > +             ret = -EFAULT;
> > +             goto err_htab;
> > +     }
> > +
> > +     e.key = name;
> > +     e.data = NULL;
> > +     if (!hsearch_r(e, ENV_FIND, &ep, &env_htab, H_HIDE_DOT)) {
> > +             ret = -ENOENT;
> > +             goto err_htab;
> > +     }
> > +     if (!ep) {
> > +             ret = -ENOENT;
> > +             goto err_htab;
> > +     }
> > +
> > +     printf("Storing %s=%s to hw_info...\n", ep->key, ep->data);
> > +
> > +     e.key = ep->key;
> > +     e.data = ep->data;
> > +     if (!hsearch_r(e, ENV_ENTER, &rv, &htab, H_HIDE_DOT)) {
> > +             ret = -EINVAL;
> > +             goto err_htab;
> > +     }
> > +     if (!rv) {
> > +             ret = -EINVAL;
> > +             goto err_htab;
> > +     }
> > +     len = hexport_r(&htab, HW_INFO_ENV_SEP, H_MATCH_KEY | H_MATCH_IDENT,
> > +                     &res, 0, 0, NULL);
> > +     if (len <= 0) {
> > +             free(res);
> > +             goto ret_htab;
> > +     }
> > +     ret = write_spi_flash_offset(res, HW_INFO_SPI_FLASH_OFFSET +
> > +                                  HW_INFO_ENV_OFFSET, len);
> > +     free(res);
> > +     if (ret)
> > +             goto err_htab;
> > +
> > +     printf("Successfully stored the Marvell hw_info parameters.\n");
> > +     return 0;
> > +ret_htab:
> > +     hdestroy_r(&htab);
> > +     return ret;
> > +err_htab:
> > +     hdestroy_r(&htab);
> > +err:
> > +     printf("## Error: cannot store hw_info parameters to SPI flash\n");
> > +     return ret;
> > +}
> > +
> > +static int do_hw_info(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > +{
> > +     const char *cmd = argv[1];
> > +
> > +     if (argc < 2)
> > +             return CMD_RET_USAGE;
> > +
> > +     if (!strcmp(cmd, "dump")) {
> > +             if (cmd_hw_info_dump())
> > +                     return -EINVAL;
> > +     } else if (!strcmp(cmd, "load")) {
> > +             if (cmd_hw_info_load(true))
> > +                     return -EINVAL;
> > +     } else if (!strcmp(cmd, "store")) {
> > +             if (cmd_hw_info_store(argv[2]))
> > +                     return -EINVAL;
> > +     } else {
> > +             return CMD_RET_USAGE;
> > +     }
> > +
> > +     return CMD_RET_SUCCESS;
> > +}
> > +
> > +U_BOOT_CMD(
> > +     hw_info, 3, 0, do_hw_info,
> > +     "Marvell hw_info utility",
> > +     "hw_info\n"
> > +     "\tdump                        - Dump all hw_info parameters from SPI flash\n"
> > +     "\tload                        - Load all hw_info parameters from hw_info to environment variables\n"
> > +     "\tstore <env_name>            - Store specific hw_info parameter from envirnoment variable to SPI flash\n"
> > +     "\nSupported hw_info parameters:\n"
> > +     "\tpcb_slm       PCB SLM number\n"
> > +     "\tpcb_rev       PCB revision number\n"
> > +     "\teco_rev       ECO revision number\n"
> > +     "\tpcb_sn        PCB SN\n"
> > +     "\tethaddr       first MAC address\n"
> > +     "\teth1addr      second MAC address\n"
> > +     "\teth2addr      third MAC address\n"
> > +     "\teth3addr      fourth MAC address\n"
> > +     "\teth4addr      fifth MAC address\n"
> > +     "\teth5addr      sixth MAC address\n"
> > +     "\teth6addr      seventh MAC address\n"
> > +     "\teth7addr      eighth MAC address\n"
> > +     "\teth8addr      ninth MAC address\n"
> > +     "\teth9addr      tenth MAC address\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,

[PATCH v2 1/3] cmd: mvebu: Implement the Marvell hw_info command

[Pali Rohár]

On Sunday 28 February 2021 22:54:27 Luka Kovacic wrote:
> Hello Pali,
> 
> On Sat, Feb 27, 2021 at 2:09 AM Pali Roh?r <pali@kernel.org> wrote:
> >
> > On Monday 15 February 2021 20:59:32 Luka Kovacic wrote:
> > > The hw_info command is implemented 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.
> > > These parameters are usually written to the flash in the factory.
> >
> > CCing Marek, as he does not like introduction of another vendor custom
> > command into mainline U-Boot.
> >
> > > Currently the command supports reading/writing parameters and dumping the
> > > current hw_info parameters.
> > > EEPROM config pattern and checksum aren't supported.
> > >
> > > This functionality has been tested on the GST ESPRESSOBin-Ultra board
> > > successfully, both reading the stock U-Boot parameters in mainline U-Boot
> > > and reading the parameters written by this command in the stock U-Boot.
> > >
> > > Usage example:
> > >  => hw_info load
> > >  => saveenv
> >
> > I'm looking at the code and it loads MAC addresses for espressobin
> > ultra variant. But for other espressobin variants there is already code
> > which sets correct eth*addr and fdtfile variables in board_late_init()
> > function.
> >
> > I think it should be relatively easy to implement reading these
> > variables from SPI to ENV in board_late_init() function. Which would
> > mean that it completely replaces this custom 'hw_info load' command.
> 
> The board.c file looked really cluttered to me in the current state and
> adding more board-specific code wouldn't help it.

Well, code which you adding in this patch series is currently specific
for one board = Espressobin Ultra. Therefore it should be part of board
code.

I agree with you that board code is not the best code on the world, but
it is still better to have board specific stuff at one place (in one
file or one directory). Currently you put some logic into defconfig file
(loading of variables from SPI) and some into board code / header files
and some into external command.

> Also there are some other potentially useful parameters in the hw_info
> SPI flash area, which might be of use (board serial number and some
> other values).

These parameters can be useful. But cannot you export them to U-Boot env
variable storage from board code?

For me it looks like a giant hack if you need to "program" preboot
command which loads these information from SPI via external u-boot
command to u-boot variable storage.

See how is initialized 'fdtfile' variable for Espressobin based at
runtime based on hw on which is running.

I think in the same way you could be able to initialize these useful
parameters (like board serial number, etc...) into u-boot variable
storage.

> >
> > Could you look at it? This would have a benefit that 'env default -a'
> > would always load correct mac addresses stored in SPI, so it would work
> > just via default u-boot commands without need to use something vendor
> > specific.
> 
> I do agree that a vendor specific solution isn't too elegant.
> I might consider the mac command, which Marek has suggested.

mac command looks like a good solution for MAC addresses. If it is
possible I would suggest to use it.

> Kind regards,
> Luka
> 
> >
> > > Signed-off-by: Luka Kovacic <luka.kovacic@sartura.hr>
> > > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > > Cc: Robert Marko <robert.marko@sartura.hr>
> > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > > ---
> > >  cmd/mvebu/Kconfig   |  23 ++++
> > >  cmd/mvebu/Makefile  |   1 +
> > >  cmd/mvebu/hw_info.c | 312 ++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/hashtable.c     |   2 +-
> > >  4 files changed, 337 insertions(+), 1 deletion(-)
> > >  create mode 100644 cmd/mvebu/hw_info.c
> > >
> > > diff --git a/cmd/mvebu/Kconfig b/cmd/mvebu/Kconfig
> > > index ad10a572a3..a8e958e7c8 100644
> > > --- a/cmd/mvebu/Kconfig
> > > +++ b/cmd/mvebu/Kconfig
> > > @@ -9,6 +9,29 @@ config CMD_MVEBU_BUBT
> > >         For details about bubt command please see the documentation
> > >         in doc/mvebu/cmd/bubt.txt
> > >
> > > +config CMD_MVEBU_HW_INFO
> > > +     bool "hw_info"
> > > +     depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEBU
> > > +     default n
> > > +     help
> > > +       Enable loading of the Marvell hw_info parameters from the
> > > +       SPI flash hw_info area. Parameters (usually the board serial
> > > +       number and MAC addresses) are then imported into the
> > > +       existing U-Boot environment.
> > > +       Implementation of this command is compatible with the
> > > +       original Marvell U-Boot command. Reading and writing is
> > > +       supported.
> > > +       EEPROM config pattern and checksum aren't supported.
> > > +
> > > +config CMD_MVEBU_HW_INFO_OFFSET
> > > +     hex "Marvell hw_info SPI flash offset"
> > > +     depends on CMD_MVEBU_HW_INFO
> > > +     default 0x3E0000
> > > +     help
> > > +       This option defines the SPI flash offset of the Marvell
> > > +       hw_info area. This defaults to 0x3E0000 on most Armada
> > > +       A3720 platforms.
> > > +
> > >  choice
> > >       prompt "Flash for image"
> > >       default MVEBU_SPI_BOOT
> > > diff --git a/cmd/mvebu/Makefile b/cmd/mvebu/Makefile
> > > index 96829c48eb..2b5a8b37be 100644
> > > --- a/cmd/mvebu/Makefile
> > > +++ b/cmd/mvebu/Makefile
> > > @@ -6,3 +6,4 @@
> > >
> > >
> > >  obj-$(CONFIG_CMD_MVEBU_BUBT) += bubt.o
> > > +obj-$(CONFIG_CMD_MVEBU_HW_INFO) += hw_info.o
> > > diff --git a/cmd/mvebu/hw_info.c b/cmd/mvebu/hw_info.c
> > > new file mode 100644
> > > index 0000000000..1ef49d78d4
> > > --- /dev/null
> > > +++ b/cmd/mvebu/hw_info.c
> > > @@ -0,0 +1,312 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Marvell hw_info command
> > > + * Helper command for interfacing with the Marvell hw_info parameters
> > > + *
> > > + * Copyright (c) 2021 Sartura Ltd.
> > > + * Copyright (c) 2018 Marvell International Ltd.
> > > + *
> > > + * Author: Luka Kovacic <luka.kovacic@sartura.hr>
> > > + */
> > > +
> > > +#include <command.h>
> > > +#include <common.h>
> > > +#include <env.h>
> > > +#include <env_internal.h>
> > > +#include <spi.h>
> > > +#include <spi_flash.h>
> > > +
> > > +#define HW_INFO_SPI_FLASH_OFFSET     CONFIG_CMD_MVEBU_HW_INFO_OFFSET
> > > +
> > > +#define HW_INFO_MAX_ENV_SIZE         0x1F0
> > > +#define HW_INFO_ENV_OFFSET           0xA
> > > +#define HW_INFO_ENV_SEP                      0x20
> > > +
> > > +#define HW_INFO_MAX_NAME_LEN         32
> > > +
> > > +static char hw_info_allowed_parameters[][HW_INFO_MAX_NAME_LEN] = {
> > > +     "pcb_slm",
> > > +     "pcb_rev",
> > > +     "eco_rev",
> > > +     "pcb_sn",
> > > +     "ethaddr",
> > > +     "eth1addr",
> > > +     "eth2addr",
> > > +     "eth3addr",
> > > +     "eth4addr",
> > > +     "eth5addr",
> > > +     "eth6addr",
> > > +     "eth7addr",
> > > +     "eth8addr",
> > > +     "eth9addr",
> > > +};
> > > +
> > > +static int hw_info_allowed_param_count = (sizeof(hw_info_allowed_parameters) /
> > > +                                     sizeof(hw_info_allowed_parameters[0]));
> > > +
> > > +static int hw_info_check_parameter(char *name)
> > > +{
> > > +     int idx;
> > > +
> > > +     for (idx = 0; idx < hw_info_allowed_param_count; idx++) {
> > > +             if (strcmp(name, hw_info_allowed_parameters[idx]) == 0)
> > > +                     return 0;
> > > +     }
> > > +
> > > +     return -EINVAL;
> > > +}
> > > +
> > > +static int read_spi_flash_offset(char *buf, int offset)
> > > +{
> > > +     struct spi_flash *flash;
> > > +     int ret;
> > > +
> > > +     flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
> > > +                             CONFIG_SF_DEFAULT_CS,
> > > +                             CONFIG_SF_DEFAULT_SPEED,
> > > +                             CONFIG_SF_DEFAULT_MODE);
> > > +
> > > +     if (!flash) {
> > > +             printf("Error - unable to probe SPI flash.\n");
> > > +             return -EIO;
> > > +     }
> > > +
> > > +     ret = spi_flash_read(flash, offset, HW_INFO_MAX_ENV_SIZE, buf);
> > > +     if (ret) {
> > > +             printf("Error - unable to read hw_info environment from SPI flash.\n");
> > > +             return ret;
> > > +     }
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static int write_spi_flash_offset(char *buf, int offset, ssize_t size)
> > > +{
> > > +     ssize_t safe_size, erase_size;
> > > +     struct spi_flash *flash;
> > > +     int ret;
> > > +
> > > +     flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
> > > +                             CONFIG_SF_DEFAULT_CS,
> > > +                             CONFIG_SF_DEFAULT_SPEED,
> > > +                             CONFIG_SF_DEFAULT_MODE);
> > > +
> > > +     if (!flash) {
> > > +             printf("Error - unable to probe SPI flash.\n");
> > > +             return -EIO;
> > > +     }
> > > +
> > > +     safe_size = size > HW_INFO_MAX_ENV_SIZE ? HW_INFO_MAX_ENV_SIZE : size;
> > > +     erase_size = safe_size +
> > > +                  (flash->erase_size - safe_size % flash->erase_size);
> > > +     ret = spi_flash_erase(flash, HW_INFO_SPI_FLASH_OFFSET, erase_size);
> > > +     if (ret) {
> > > +             printf("Error - unable to erase the hw_info area on SPI flash.\n");
> > > +             return ret;
> > > +     }
> > > +     ret = spi_flash_write(flash, offset, safe_size, buf);
> > > +     if (ret) {
> > > +             printf("Error - unable to write hw_info parameters to SPI flash.\n");
> > > +             return ret;
> > > +     }
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static int cmd_hw_info_dump(void)
> > > +{
> > > +     char buffer[HW_INFO_MAX_ENV_SIZE];
> > > +     struct hsearch_data htab;
> > > +     char *res = NULL;
> > > +     ssize_t len;
> > > +     int ret = 0;
> > > +
> > > +     ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
> > > +                                 HW_INFO_ENV_OFFSET);
> > > +     if (ret)
> > > +             goto err;
> > > +     memset(&htab, 0, sizeof(htab));
> > > +     if (!hcreate_r(HW_INFO_MAX_ENV_SIZE, &htab)) {
> > > +             ret = -ENOMEM;
> > > +             goto err;
> > > +     }
> > > +     if (!himport_r(&htab, buffer, HW_INFO_MAX_ENV_SIZE,
> > > +                    HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
> > > +             ret = -EFAULT;
> > > +             goto err_htab;
> > > +     }
> > > +
> > > +     len = hexport_r(&htab, '\n', H_HIDE_DOT, &res, 0, 0, NULL);
> > > +     if (len > 0) {
> > > +             printf("Parameters (hw_info):\n");
> > > +             puts(res);
> > > +             free(res);
> > > +             ret = 0;
> > > +             goto ret_htab;
> > > +     }
> > > +ret_htab:
> > > +     hdestroy_r(&htab);
> > > +     return ret;
> > > +err_htab:
> > > +     hdestroy_r(&htab);
> > > +err:
> > > +     printf("## Error: cannot store hw_info parameters to SPI flash\n");
> > > +     return ret;
> > > +}
> > > +
> > > +static int cmd_hw_info_load(bool print_env)
> > > +{
> > > +     char buffer[HW_INFO_MAX_ENV_SIZE];
> > > +     char *res = NULL;
> > > +     ssize_t len;
> > > +     int ret = 0;
> > > +
> > > +     ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
> > > +                                 HW_INFO_ENV_OFFSET);
> > > +     if (ret)
> > > +             goto err;
> > > +     if (!himport_r(&env_htab, buffer, HW_INFO_MAX_ENV_SIZE,
> > > +                    HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
> > > +             ret = -EFAULT;
> > > +             goto err;
> > > +     }
> > > +
> > > +     printf("Successfully imported the Marvell hw_info parameters.\n");
> > > +     if (!print_env)
> > > +             return 0;
> > > +
> > > +     len = hexport_r(&env_htab, '\n', H_HIDE_DOT, &res, 0, 0, NULL);
> > > +     if (len > 0) {
> > > +             printf("Updated environment:\n");
> > > +             puts(res);
> > > +             free(res);
> > > +             return 0;
> > > +     }
> > > +err:
> > > +     printf("## Error: cannot import hw_info parameters\n");
> > > +     return ret;
> > > +}
> > > +
> > > +static int cmd_hw_info_store(char *name)
> > > +{
> > > +     char buffer[HW_INFO_MAX_ENV_SIZE];
> > > +     struct env_entry e, *ep, *rv;
> > > +     struct hsearch_data htab;
> > > +     char *res = NULL;
> > > +     ssize_t len;
> > > +     int ret = 0;
> > > +
> > > +     ret = hw_info_check_parameter(name);
> > > +     if (ret) {
> > > +             printf("Invalid parameter %s, stopping.\n", name);
> > > +             goto err;
> > > +     }
> > > +
> > > +     ret = read_spi_flash_offset(buffer, HW_INFO_SPI_FLASH_OFFSET +
> > > +                                 HW_INFO_ENV_OFFSET);
> > > +     if (ret)
> > > +             goto err;
> > > +     memset(&htab, 0, sizeof(htab));
> > > +     if (!hcreate_r(HW_INFO_MAX_ENV_SIZE, &htab)) {
> > > +             ret = -ENOMEM;
> > > +             goto err;
> > > +     }
> > > +     if (!himport_r(&htab, buffer, HW_INFO_MAX_ENV_SIZE,
> > > +                    HW_INFO_ENV_SEP, H_NOCLEAR, 0, 0, NULL)) {
> > > +             ret = -EFAULT;
> > > +             goto err_htab;
> > > +     }
> > > +
> > > +     e.key = name;
> > > +     e.data = NULL;
> > > +     if (!hsearch_r(e, ENV_FIND, &ep, &env_htab, H_HIDE_DOT)) {
> > > +             ret = -ENOENT;
> > > +             goto err_htab;
> > > +     }
> > > +     if (!ep) {
> > > +             ret = -ENOENT;
> > > +             goto err_htab;
> > > +     }
> > > +
> > > +     printf("Storing %s=%s to hw_info...\n", ep->key, ep->data);
> > > +
> > > +     e.key = ep->key;
> > > +     e.data = ep->data;
> > > +     if (!hsearch_r(e, ENV_ENTER, &rv, &htab, H_HIDE_DOT)) {
> > > +             ret = -EINVAL;
> > > +             goto err_htab;
> > > +     }
> > > +     if (!rv) {
> > > +             ret = -EINVAL;
> > > +             goto err_htab;
> > > +     }
> > > +     len = hexport_r(&htab, HW_INFO_ENV_SEP, H_MATCH_KEY | H_MATCH_IDENT,
> > > +                     &res, 0, 0, NULL);
> > > +     if (len <= 0) {
> > > +             free(res);
> > > +             goto ret_htab;
> > > +     }
> > > +     ret = write_spi_flash_offset(res, HW_INFO_SPI_FLASH_OFFSET +
> > > +                                  HW_INFO_ENV_OFFSET, len);
> > > +     free(res);
> > > +     if (ret)
> > > +             goto err_htab;
> > > +
> > > +     printf("Successfully stored the Marvell hw_info parameters.\n");
> > > +     return 0;
> > > +ret_htab:
> > > +     hdestroy_r(&htab);
> > > +     return ret;
> > > +err_htab:
> > > +     hdestroy_r(&htab);
> > > +err:
> > > +     printf("## Error: cannot store hw_info parameters to SPI flash\n");
> > > +     return ret;
> > > +}
> > > +
> > > +static int do_hw_info(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > > +{
> > > +     const char *cmd = argv[1];
> > > +
> > > +     if (argc < 2)
> > > +             return CMD_RET_USAGE;
> > > +
> > > +     if (!strcmp(cmd, "dump")) {
> > > +             if (cmd_hw_info_dump())
> > > +                     return -EINVAL;
> > > +     } else if (!strcmp(cmd, "load")) {
> > > +             if (cmd_hw_info_load(true))
> > > +                     return -EINVAL;
> > > +     } else if (!strcmp(cmd, "store")) {
> > > +             if (cmd_hw_info_store(argv[2]))
> > > +                     return -EINVAL;
> > > +     } else {
> > > +             return CMD_RET_USAGE;
> > > +     }
> > > +
> > > +     return CMD_RET_SUCCESS;
> > > +}
> > > +
> > > +U_BOOT_CMD(
> > > +     hw_info, 3, 0, do_hw_info,
> > > +     "Marvell hw_info utility",
> > > +     "hw_info\n"
> > > +     "\tdump                        - Dump all hw_info parameters from SPI flash\n"
> > > +     "\tload                        - Load all hw_info parameters from hw_info to environment variables\n"
> > > +     "\tstore <env_name>            - Store specific hw_info parameter from envirnoment variable to SPI flash\n"
> > > +     "\nSupported hw_info parameters:\n"
> > > +     "\tpcb_slm       PCB SLM number\n"
> > > +     "\tpcb_rev       PCB revision number\n"
> > > +     "\teco_rev       ECO revision number\n"
> > > +     "\tpcb_sn        PCB SN\n"
> > > +     "\tethaddr       first MAC address\n"
> > > +     "\teth1addr      second MAC address\n"
> > > +     "\teth2addr      third MAC address\n"
> > > +     "\teth3addr      fourth MAC address\n"
> > > +     "\teth4addr      fifth MAC address\n"
> > > +     "\teth5addr      sixth MAC address\n"
> > > +     "\teth6addr      seventh MAC address\n"
> > > +     "\teth7addr      eighth MAC address\n"
> > > +     "\teth8addr      ninth MAC address\n"
> > > +     "\teth9addr      tenth MAC address\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,

[PATCH v2 1/3] cmd: mvebu: Implement the Marvell hw_info command

[Marek Behun]

Luka, Tom,

I am very against this.

there already is `mac` command in cmd/mac.c with following help
   display and program the system ID and MAC addresses in EEPROM
   [read|save|id|num|errata|date|ports|port_number]
   mac read
       - read EEPROM content into memory data structure
   mac save
       - save memory data structure to the EEPROM
   mac id
       - program system id per hard coded value
   mac num string
       - program system serial number to value string
   mac errata string
       - program errata data to value string
   mac date YYMMDDhhmmss
       - program date to string value YYMMDDhhmmss
   mac ports N
       - program the number of network ports to integer N
   mac X string
       - program MAC addr for port X [X=0,1..] to colon separated string"

Why introducing new, vendor specific commands, when we have a global
API for such a thing?

Marek

[PATCH v2 1/3] cmd: mvebu: Implement the Marvell hw_info command

[Luka Kovacic]

Hello Marek,

On Sat, Feb 27, 2021 at 2:05 PM Marek Behun <marek.behun@nic.cz> wrote:
>
> Luka, Tom,
>
> I am very against this.
>
> there already is `mac` command in cmd/mac.c with following help
>    display and program the system ID and MAC addresses in EEPROM
>    [read|save|id|num|errata|date|ports|port_number]
>    mac read
>        - read EEPROM content into memory data structure
>    mac save
>        - save memory data structure to the EEPROM
>    mac id
>        - program system id per hard coded value
>    mac num string
>        - program system serial number to value string
>    mac errata string
>        - program errata data to value string
>    mac date YYMMDDhhmmss
>        - program date to string value YYMMDDhhmmss
>    mac ports N
>        - program the number of network ports to integer N
>    mac X string
>        - program MAC addr for port X [X=0,1..] to colon separated string"
>
> Why introducing new, vendor specific commands, when we have a global
> API for such a thing?

I have to admit, I wasn't aware that this command existed.

I will transfer the implementation of the hw_info command into this existing
framework, I completely agree that this is a better solution.
Also when the "mac read" command would be executed, I'd also transfer
all of the other OEM values to the environment (the MACs and the 4
additional values).

Also I'll check how the current implementation is done in some of the boards.

Kind regards,
Luka

>
> Marek

[PATCH v2 1/3] cmd: mvebu: Implement the Marvell hw_info command

[Heinrich Schuchardt]

Am 27. Februar 2021 14:05:50 MEZ schrieb Marek Behun <marek.behun@nic.cz>:
>Luka, Tom,
>
>I am very against this.
>
>there already is `mac` command in cmd/mac.c with following help
>   display and program the system ID and MAC addresses in EEPROM
>   [read|save|id|num|errata|date|ports|port_number]
>   mac read
>       - read EEPROM content into memory data structure
>   mac save
>       - save memory data structure to the EEPROM
>   mac id
>       - program system id per hard coded value
>   mac num string
>       - program system serial number to value string
>   mac errata string
>       - program errata data to value string
>   mac date YYMMDDhhmmss
>       - program date to string value YYMMDDhhmmss
>   mac ports N
>       - program the number of network ports to integer N
>   mac X string
>     - program MAC addr for port X [X=0,1..] to colon separated string"
>
>Why introducing new, vendor specific commands, when we have a global
>API for such a thing?

We lack a man-page in doc/usage/.

Best regards

Heinrich

>
>Marek

[PATCH v2 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable

[Luka Kovacic]

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 2ad4325eaf..1041df8d91 100644
--- a/include/configs/mvebu_armada-37xx.h
+++ b/include/configs/mvebu_armada-37xx.h
@@ -103,6 +103,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

[PATCH v2 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable

[Pali Rohár]

On Monday 15 February 2021 20:59:33 Luka Kovacic wrote:
> Add the loadaddr U-Boot environment variable, as this is available in
> the stock Marvell U-Boot by default on Marvell Armada A37XX platforms.

Hello Luka! Why is this change needed? mvebu_armada-37xx.h already
defines CONFIG_SYS_LOAD_ADDR macro with its default value 0x06000000.
So defining loadaddr variable should not be needed at all. And it is
suspicious for me why definition is to the same default value.

> 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 2ad4325eaf..1041df8d91 100644
> --- a/include/configs/mvebu_armada-37xx.h
> +++ b/include/configs/mvebu_armada-37xx.h
> @@ -103,6 +103,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"		\

[PATCH v2 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable

[Luka Kovacic]

Hello Pali,

On Sat, Feb 27, 2021 at 1:38 AM Pali Roh?r <pali@kernel.org> wrote:
>
> On Monday 15 February 2021 20:59:33 Luka Kovacic wrote:
> > Add the loadaddr U-Boot environment variable, as this is available in
> > the stock Marvell U-Boot by default on Marvell Armada A37XX platforms.
>
> Hello Luka! Why is this change needed? mvebu_armada-37xx.h already
> defines CONFIG_SYS_LOAD_ADDR macro with its default value 0x06000000.
> So defining loadaddr variable should not be needed at all. And it is
> suspicious for me why definition is to the same default value.

I've added this to the environment, because it was not present in
U-Boot as a variable.
There's a need for this value in some scripts, are you aware of some
other way to retrieve it easily?
Some other boards also do this similarly.

Kind regards,
Luka
>
> > 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 2ad4325eaf..1041df8d91 100644
> > --- a/include/configs/mvebu_armada-37xx.h
> > +++ b/include/configs/mvebu_armada-37xx.h
> > @@ -103,6 +103,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"          \

[PATCH v2 2/3] arm: mvebu: mvebu_armada-37xx: Define the loadaddr environment variable

[Pali Rohár]

On Sunday 28 February 2021 22:26:25 Luka Kovacic wrote:
> Hello Pali,
> 
> On Sat, Feb 27, 2021 at 1:38 AM Pali Roh?r <pali@kernel.org> wrote:
> >
> > On Monday 15 February 2021 20:59:33 Luka Kovacic wrote:
> > > Add the loadaddr U-Boot environment variable, as this is available in
> > > the stock Marvell U-Boot by default on Marvell Armada A37XX platforms.
> >
> > Hello Luka! Why is this change needed? mvebu_armada-37xx.h already
> > defines CONFIG_SYS_LOAD_ADDR macro with its default value 0x06000000.
> > So defining loadaddr variable should not be needed at all. And it is
> > suspicious for me why definition is to the same default value.
> 
> I've added this to the environment, because it was not present in
> U-Boot as a variable.
> There's a need for this value in some scripts, are you aware of some
> other way to retrieve it easily?

Hello Luka!

It is really needed? Because if you do not specify loadaddr then default
value from CONFIG_SYS_LOAD_ADDR is used. It applies also for 'loadb' and
'loady' commands.

I guess in past it was needed in scripts because default
CONFIG_SYS_LOAD_ADDR was wrong. But I have fixed it in commit:
996ecfd3ec4e86be9dbee17b2223061e16627f71

It is possible that old Marvell's U-Boot fork has this problem too, but
in recent version of upstream U-Boot it is fixed by above commit.

> Some other boards also do this similarly.
> 
> Kind regards,
> Luka
> >
> > > 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 2ad4325eaf..1041df8d91 100644
> > > --- a/include/configs/mvebu_armada-37xx.h
> > > +++ b/include/configs/mvebu_armada-37xx.h
> > > @@ -103,6 +103,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"          \

[PATCH v2 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

[Luka Kovacic]

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 hw_info command.
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       |  92 +++++++-
 .../mvebu_espressobin-ultra-88f3720_defconfig |  92 ++++++++
 5 files changed, 389 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 33e483f4fb..9f544b1caf 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -208,6 +208,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 c630437c08..9ffda8fdba 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -11,6 +11,7 @@
 #include <i2c.h>
 #include <init.h>
 #include <mmc.h>
+#include <miiphy.h>
 #include <phy.h>
 #include <asm/global_data.h>
 #include <asm/io.h>
@@ -55,6 +56,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
  *
@@ -289,12 +299,68 @@ 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)
+{
+	int i;
+	/* Setup 88E1512 SGMII-to-Copper mode */
+	force_phy_88e1512_sgmii_to_copper(ULTRA_MV88E1512_PHYADDR);
 
+	/*
+	 * FIXME: remove this code once Topaz driver gets available
+	 * A3720 ESPRESSObin Ultra Board Only
+	 * Configure Topaz switch (88E6341)
+	 * Set port 1,2,3,4,5 to forwarding Mode (through Switch Port registers)
+	 */
+	for (i = 0; i <= 5; i++) {
+		mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(i),
+					  MVEBU_SW_PORT_CTRL_REG,
+					  i == 5 ? 0x7c : 0x7f);
+	}
+
+	/* RGMII Delay on Port 0 (CPU port), force link to 1000Mbps */
+	mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(0),
+				  MVEBU_SW_LINK_CTRL_REG, 0xe002);
+
+	/* Power up PHY 1, 2, 3, 4, 5 (through Global 2 registers) */
+	mii_multi_chip_mode_write(bus, 3, MVEBU_SW_G2_SMI_ADDR,
+				  MVEBU_G2_SMI_PHY_DATA_REG, 0x1140);
+	for (i = 1; i <= 5; i++) {
+		mii_multi_chip_mode_write(bus, 3, MVEBU_SW_G2_SMI_ADDR,
+					  MVEBU_G2_SMI_PHY_CMD_REG, 0x9400 +
+					  (MVEBU_PORT_CTRL_SMI_ADDR(i) << 5));
+	}
+
+	return 0;
+}
+
+int board_network_enable_espressobin(struct mii_dev *bus)
+{
 	/*
 	 * FIXME: remove this code once Topaz driver gets available
 	 * A3720 Community Board Only
@@ -335,6 +401,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)
 {
@@ -343,8 +419,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..207f38d300
--- /dev/null
+++ b/configs/mvebu_espressobin-ultra-88f3720_defconfig
@@ -0,0 +1,92 @@
+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 setenv read_board_hw_info 1; hw_info load; 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_MVEBU_HW_INFO=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_PHY_GIGE=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

[PATCH v2 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

[Pali Rohár]

On Monday 15 February 2021 20:59:34 Luka Kovacic wrote:
> 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 hw_info command.
> The parameters stored in Marvell hw_info are usually the board serial
> number and MAC addresses.

Hello Luka!

Few months ago I have de-duplicated all existing espressobin setups
supported in uboot to use just one DTS file and one defconfig file and
do board detection at runtime.

So I do not think it is a good idea to again define another espressobin
DTS and defconfig file.

Linux kernel also is using one common DTS file for all espressobin
boards (including ultra variant).

So could you please try to rework this patch to use existing files
armada-3720-espressobin.dts and mvebu_espressobin-88f3720_defconfig
(with probably some board code which detects for ultra variant)?

> 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       |  92 +++++++-
>  .../mvebu_espressobin-ultra-88f3720_defconfig |  92 ++++++++
>  5 files changed, 389 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 33e483f4fb..9f544b1caf 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -208,6 +208,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 c630437c08..9ffda8fdba 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -11,6 +11,7 @@
>  #include <i2c.h>
>  #include <init.h>
>  #include <mmc.h>
> +#include <miiphy.h>
>  #include <phy.h>
>  #include <asm/global_data.h>
>  #include <asm/io.h>
> @@ -55,6 +56,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
>   *
> @@ -289,12 +299,68 @@ 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)
> +{
> +	int i;
> +	/* Setup 88E1512 SGMII-to-Copper mode */
> +	force_phy_88e1512_sgmii_to_copper(ULTRA_MV88E1512_PHYADDR);
>  
> +	/*
> +	 * FIXME: remove this code once Topaz driver gets available
> +	 * A3720 ESPRESSObin Ultra Board Only
> +	 * Configure Topaz switch (88E6341)
> +	 * Set port 1,2,3,4,5 to forwarding Mode (through Switch Port registers)
> +	 */
> +	for (i = 0; i <= 5; i++) {
> +		mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(i),
> +					  MVEBU_SW_PORT_CTRL_REG,
> +					  i == 5 ? 0x7c : 0x7f);
> +	}
> +
> +	/* RGMII Delay on Port 0 (CPU port), force link to 1000Mbps */
> +	mii_multi_chip_mode_write(bus, 3, MVEBU_PORT_CTRL_SMI_ADDR(0),
> +				  MVEBU_SW_LINK_CTRL_REG, 0xe002);
> +
> +	/* Power up PHY 1, 2, 3, 4, 5 (through Global 2 registers) */
> +	mii_multi_chip_mode_write(bus, 3, MVEBU_SW_G2_SMI_ADDR,
> +				  MVEBU_G2_SMI_PHY_DATA_REG, 0x1140);
> +	for (i = 1; i <= 5; i++) {
> +		mii_multi_chip_mode_write(bus, 3, MVEBU_SW_G2_SMI_ADDR,
> +					  MVEBU_G2_SMI_PHY_CMD_REG, 0x9400 +
> +					  (MVEBU_PORT_CTRL_SMI_ADDR(i) << 5));
> +	}
> +
> +	return 0;
> +}
> +
> +int board_network_enable_espressobin(struct mii_dev *bus)
> +{
>  	/*
>  	 * FIXME: remove this code once Topaz driver gets available
>  	 * A3720 Community Board Only
> @@ -335,6 +401,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)
>  {
> @@ -343,8 +419,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..207f38d300
> --- /dev/null
> +++ b/configs/mvebu_espressobin-ultra-88f3720_defconfig
> @@ -0,0 +1,92 @@
> +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 setenv read_board_hw_info 1; hw_info load; 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_MVEBU_HW_INFO=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_PHY_GIGE=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

[PATCH v2 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

[Marek Behun]

In Linux the DTS for espressobin-ultra includes
armada-3720-espressobin.dtsi and just adds changes.

If possible, please do this as is done in Linux. In fact we want to
slowly move in the direction to have the dts files just copied from
Linux.

Marek

[PATCH v2 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

[Luka Kovacic]

Hello Marek,

On Sat, Feb 27, 2021 at 2:24 PM Marek Behun <marek.behun@nic.cz> wrote:
>
> In Linux the DTS for espressobin-ultra includes
> armada-3720-espressobin.dtsi and just adds changes.
>
> If possible, please do this as is done in Linux. In fact we want to
> slowly move in the direction to have the dts files just copied from
> Linux.
>
> Marek

Thanks for pointing this out.
As you and Pali have suggested, I will rework this patch to be as
close to the Linux dts as possible.

I'd go with the approach of just creating a dts with changes with
respect to the base ESPRESSOBin.

Kind regards,
Luka

[PATCH v2 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

[Pali Rohár]

On Sunday 28 February 2021 22:38:26 Luka Kovacic wrote:
> Hello Marek,
> 
> On Sat, Feb 27, 2021 at 2:24 PM Marek Behun <marek.behun@nic.cz> wrote:
> >
> > In Linux the DTS for espressobin-ultra includes
> > armada-3720-espressobin.dtsi and just adds changes.
> >
> > If possible, please do this as is done in Linux. In fact we want to
> > slowly move in the direction to have the dts files just copied from
> > Linux.
> >
> > Marek
> 
> Thanks for pointing this out.
> As you and Pali have suggested, I will rework this patch to be as
> close to the Linux dts as possible.
> 
> I'd go with the approach of just creating a dts with changes with
> respect to the base ESPRESSOBin.
> 
> Kind regards,
> Luka

Hello Luka!

Linux kernel has currently 5 DTS files for espressobin (v5, v5-emmc, v7,
v7-emmc, ultra) and all are using one common dtsi file.

In U-Boot now we have just one DTS file for all 4 variants (ultra
variant is missing) and differences between emmc and non-emmc, v5 and v7
variants is done at u-boot runtime code.

So this allows to have just one U-Boot binary for all espressobin
boards.

Distributions (OpenWRT and Armbian) complained about need to have such
many precompiled binaries for espressobin. And seems that after unifying
u-boot to build just one espressobin binary, they are happy.

So, if it is possible, I would be happy if we can continue with this
approach with just one DTS file and one U-Boot binary for all
espressobin variants.

If it is needed to enable / disable some nodes in DTS file, it is
possible at U-Boot runtime via board_fix_fdt() function in board code.
Here can be e.g. code which checks if U-Boot is running at espressobin
v7 or at espressobin ultra. Maybe this could help you with debugging...

[PATCH v2 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

[Marek Behun]

On Mon, 1 Mar 2021 16:41:01 +0100
Pali Roh?r <pali@kernel.org> wrote:

> On Sunday 28 February 2021 22:38:26 Luka Kovacic wrote:
> > Hello Marek,
> > 
> > On Sat, Feb 27, 2021 at 2:24 PM Marek Behun <marek.behun@nic.cz> wrote:  
> > >
> > > In Linux the DTS for espressobin-ultra includes
> > > armada-3720-espressobin.dtsi and just adds changes.
> > >
> > > If possible, please do this as is done in Linux. In fact we want to
> > > slowly move in the direction to have the dts files just copied from
> > > Linux.
> > >
> > > Marek  
> > 
> > Thanks for pointing this out.
> > As you and Pali have suggested, I will rework this patch to be as
> > close to the Linux dts as possible.
> > 
> > I'd go with the approach of just creating a dts with changes with
> > respect to the base ESPRESSOBin.
> > 
> > Kind regards,
> > Luka  
> 
> Hello Luka!
> 
> Linux kernel has currently 5 DTS files for espressobin (v5, v5-emmc, v7,
> v7-emmc, ultra) and all are using one common dtsi file.
> 
> In U-Boot now we have just one DTS file for all 4 variants (ultra
> variant is missing) and differences between emmc and non-emmc, v5 and v7
> variants is done at u-boot runtime code.
> 
> So this allows to have just one U-Boot binary for all espressobin
> boards.
> 
> Distributions (OpenWRT and Armbian) complained about need to have such
> many precompiled binaries for espressobin. And seems that after unifying
> u-boot to build just one espressobin binary, they are happy.
> 
> So, if it is possible, I would be happy if we can continue with this
> approach with just one DTS file and one U-Boot binary for all
> espressobin variants.
> 
> If it is needed to enable / disable some nodes in DTS file, it is
> possible at U-Boot runtime via board_fix_fdt() function in board code.
> Here can be e.g. code which checks if U-Boot is running at espressobin
> v7 or at espressobin ultra. Maybe this could help you with debugging...

In that case try to make the "one dts file to rule them all" to be the
same as the basic dts file in Linux, and do the necessary U-Boot
differences in armada-3720-espressobin-u-boot.dtsi file. Then fix the
necessary differences for variants in board code.

Marek

[PATCH v2 3/3] arm: mvebu: Initial ESPRESSOBin-Ultra board support

[Marek Behun]

On Mon, 1 Mar 2021 16:41:01 +0100
Pali Roh?r <pali@kernel.org> wrote:

> On Sunday 28 February 2021 22:38:26 Luka Kovacic wrote:
> > Hello Marek,
> > 
> > On Sat, Feb 27, 2021 at 2:24 PM Marek Behun <marek.behun@nic.cz> wrote:  
> > >
> > > In Linux the DTS for espressobin-ultra includes
> > > armada-3720-espressobin.dtsi and just adds changes.
> > >
> > > If possible, please do this as is done in Linux. In fact we want to
> > > slowly move in the direction to have the dts files just copied from
> > > Linux.
> > >
> > > Marek  
> > 
> > Thanks for pointing this out.
> > As you and Pali have suggested, I will rework this patch to be as
> > close to the Linux dts as possible.
> > 
> > I'd go with the approach of just creating a dts with changes with
> > respect to the base ESPRESSOBin.
> > 
> > Kind regards,
> > Luka  
> 
> Hello Luka!
> 
> Linux kernel has currently 5 DTS files for espressobin (v5, v5-emmc, v7,
> v7-emmc, ultra) and all are using one common dtsi file.
> 
> In U-Boot now we have just one DTS file for all 4 variants (ultra
> variant is missing) and differences between emmc and non-emmc, v5 and v7
> variants is done at u-boot runtime code.
> 
> So this allows to have just one U-Boot binary for all espressobin
> boards.
> 
> Distributions (OpenWRT and Armbian) complained about need to have such
> many precompiled binaries for espressobin. And seems that after unifying
> u-boot to build just one espressobin binary, they are happy.
> 
> So, if it is possible, I would be happy if we can continue with this
> approach with just one DTS file and one U-Boot binary for all
> espressobin variants.
> 
> If it is needed to enable / disable some nodes in DTS file, it is
> possible at U-Boot runtime via board_fix_fdt() function in board code.
> Here can be e.g. code which checks if U-Boot is running at espressobin
> v7 or at espressobin ultra. Maybe this could help you with debugging...

The previous message was for Luka, sorry.

BTW Pali, if distributions want to have one binary for all variants
maybe they would be interested in my approach to the TIM GPP code for
RAM initialization from mox-boot-builder repository, since we have one
flash-image.bin binary for both DDR3 and DDR4 for all RAM sizes.

Marek