From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pali =?utf-8?B?Um9ow6Fy?= Date: Sat, 27 Feb 2021 02:09:21 +0100 Subject: [PATCH v2 1/3] cmd: mvebu: Implement the Marvell hw_info command In-Reply-To: <20210215195934.67034-2-luka.kovacic@sartura.hr> References: <20210215195934.67034-1-luka.kovacic@sartura.hr> <20210215195934.67034-2-luka.kovacic@sartura.hr> Message-ID: <20210227010921.e3g2digi4agjtskw@pali> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > Cc: Luka Perkov > Cc: Robert Marko > Reviewed-by: Tom Rini > --- > 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 > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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 - 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,