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.kova...@sartura.hr>
> Cc: Luka Perkov <luka.per...@sartura.hr>
> Cc: Robert Marko <robert.ma...@sartura.hr>
> Reviewed-by: Tom Rini <tr...@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.kova...@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,
