[PATCH v3 1/3] arm: mvebu: mvebu_armada-37xx: Implement the mac command (Marvell hw_info)
Luka Kovacic
luka.kovacic at sartura.hr
Fri Aug 13 11:23:15 CEST 2021
Hello Marek and Pali,
On Fri, Aug 13, 2021 at 10:23 AM Pali Rohár <pali at kernel.org> wrote:
>
> On Friday 13 August 2021 01:39:36 Luka Kovacic wrote:
> > The mac 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 mac 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:
> > => mac read
> > => saveenv
>
> Hello Luka!
>
> You are just using above commands for accessing following variables,
> right?
>
> pcb_slm
> pcb_rev
> eco_rev
> pcb_sn
> ethaddr
> eth1addr
> eth2addr
> eth3addr
> eth4addr
> eth5addr
> eth6addr
> eth7addr
> eth8addr
> eth9addr
>
Yes, I am accessing these variables.
> So why is additional command required? Espressobin boards can already
> load and use correct eth*addr variables, which is implemented in board
> file.
The current implementation in the board file preserves currently stored MAC
addresses, which are in the U-Boot environment.
My implementation is suitable for the ESPRESSOBin-Ultra, as the MACs
are stored in the hw_info area of the SPI flash and the tool imports them
into the normal U-Boot env.
They can also be modified using the tool - we won't be able to get this
functionality in the board file.
>
> So what about implementing this import for all above variables for ultra
> variant in board file, like it is already for all other variants? I have
> already suggested it in the past. As I think this approach is just one
> giant and complicated hack...
> https://lore.kernel.org/u-boot/20210301151721.xt62rdun5dnlevqk@pali/
>
> I guess that it should be enough to import variables from SPI to env in
> board_late_init() function.
Currently, the MACs are automatically imported from the
common/board_r.c file at boot, I think this is more standard and cleaner.
Nevertheless, I believe this a sufficient solution for now,
as this has been suggested as a solution in the previous patchset
comments too.
>
> > Signed-off-by: Luka Kovacic <luka.kovacic at sartura.hr>
> > Cc: Luka Perkov <luka.perkov at sartura.hr>
> > Cc: Robert Marko <robert.marko at sartura.hr>
> > ---
> > arch/arm/mach-mvebu/Kconfig | 1 +
> > board/Marvell/mvebu_armada-37xx/Kconfig | 29 ++
> > board/Marvell/mvebu_armada-37xx/Makefile | 3 +-
> > board/Marvell/mvebu_armada-37xx/mac/Makefile | 5 +
> > board/Marvell/mvebu_armada-37xx/mac/hw_info.c | 391 ++++++++++++++++++
> > include/configs/mvebu_armada-37xx.h | 7 +
> > lib/hashtable.c | 2 +-
> > 7 files changed, 436 insertions(+), 2 deletions(-)
> > create mode 100644 board/Marvell/mvebu_armada-37xx/Kconfig
> > create mode 100644 board/Marvell/mvebu_armada-37xx/mac/Makefile
> > create mode 100644 board/Marvell/mvebu_armada-37xx/mac/hw_info.c
> >
> > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > index 89737a37ad..dff9f05b28 100644
> > --- a/arch/arm/mach-mvebu/Kconfig
> > +++ b/arch/arm/mach-mvebu/Kconfig
> > @@ -312,6 +312,7 @@ config SECURED_MODE_CSK_INDEX
> > default 0
> > depends on SECURED_MODE_IMAGE
> >
> > +source "board/Marvell/mvebu_armada-37xx/Kconfig"
> > source "board/solidrun/clearfog/Kconfig"
> > source "board/kobol/helios4/Kconfig"
> >
> > diff --git a/board/Marvell/mvebu_armada-37xx/Kconfig b/board/Marvell/mvebu_armada-37xx/Kconfig
> > new file mode 100644
> > index 0000000000..b84dd20023
> > --- /dev/null
> > +++ b/board/Marvell/mvebu_armada-37xx/Kconfig
> > @@ -0,0 +1,29 @@
> > +menu "Marvell Armada 37xx configuration"
> > +depends on TARGET_MVEBU_ARMADA_37XX
> > +
> > +config MVEBU_MAC_HW_INFO
> > + bool "Marvell hw_info (mac) support"
> > + 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.
> > + After enabled, these parameters are managed from the common
> > + U-Boot mac command.
> > +
> > +config MVEBU_MAC_HW_INFO_OFFSET
> > + hex "Marvell hw_info (mac) SPI flash offset"
> > + depends on MVEBU_MAC_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.
> > +
> > +endmenu
> > diff --git a/board/Marvell/mvebu_armada-37xx/Makefile b/board/Marvell/mvebu_armada-37xx/Makefile
> > index 27221557c7..6f6ac6b193 100644
> > --- a/board/Marvell/mvebu_armada-37xx/Makefile
> > +++ b/board/Marvell/mvebu_armada-37xx/Makefile
> > @@ -2,4 +2,5 @@
> > #
> > # Copyright (C) 2016 Stefan Roese <sr at denx.de>
> >
> > -obj-y := board.o
> > +obj-y += board.o
> > +obj-y += mac/
> > diff --git a/board/Marvell/mvebu_armada-37xx/mac/Makefile b/board/Marvell/mvebu_armada-37xx/mac/Makefile
> > new file mode 100644
> > index 0000000000..7c30fe6194
> > --- /dev/null
> > +++ b/board/Marvell/mvebu_armada-37xx/mac/Makefile
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright (c) 2021 Sartura Ltd.
> > +
> > +obj-$(CONFIG_ID_EEPROM) += hw_info.o
> > diff --git a/board/Marvell/mvebu_armada-37xx/mac/hw_info.c b/board/Marvell/mvebu_armada-37xx/mac/hw_info.c
> > new file mode 100644
> > index 0000000000..91e5cd8dcf
> > --- /dev/null
> > +++ b/board/Marvell/mvebu_armada-37xx/mac/hw_info.c
> > @@ -0,0 +1,391 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Marvell hw_info (mac) command implementation
> > + * 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 at 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_MVEBU_MAC_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
> > +
> > +#define HW_INFO_MERGED_VARIABLE "read_board_hw_info"
> > +
> > +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;
> > +}
> > +
> > +/**
> > + * read_spi_flash_offset() - Read data from the SPI flash
> > + * @buf: Buffer to write in
> > + * @offset: Offset from the flash start
> > + *
> > + * Read SPI flash data into the buffer from offset to HW_INFO_MAX_ENV_SIZE.
> > + */
> > +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;
> > +}
> > +
> > +/**
> > + * write_spi_flash_offset() - Write a buffer to SPI flash
> > + * @buf: Buffer to write to SPI flash
> > + * @offset: Offset from the flash start
> > + * @size: Size of the buffer content
> > + *
> > + * This function probes the SPI flash and updates the specified flash location
> > + * with new data from the buffer.
> > + */
> > +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;
> > +}
> > +
> > +/**
> > + * cmd_hw_info_dump() - Dump the hw_info parameters
> > + *
> > + * This function prints all Marvell hw_info parameters, which are stored in
> > + * the SPI flash.
> > + */
> > +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;
> > +}
> > +
> > +/**
> > + * cmd_hw_info_read() - Import the hw_info parameters into U-Boot env
> > + * @print_env: Print U-Boot environment after new parameters are imported
> > + *
> > + * This function reads the Marvell hw_info parameters from SPI flash and
> > + * imports them into the U-Boot env.
> > + */
> > +static int cmd_hw_info_read(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;
> > +}
> > +
> > +/**
> > + * cmd_hw_info_save() - Save a parameter from U-Boot env to hw_info parameters
> > + * @name: Name of the U-Boot env parameter to save
> > + *
> > + * This function finds the specified parameter by name in the U-Boot env
> > + * and then updates the Marvell hw_info parameters with the new value.
> > + */
> > +static int cmd_hw_info_save(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;
> > +}
> > +
> > +/**
> > + * mac_read_from_eeprom() - Read the parameters from SPI flash.
> > + *
> > + * This function reads the content of the Marvell hw_info parameters from the
> > + * SPI flash and imports them into the U-Boot environment.
> > + * This includes MAC addresses and board serial numbers.
> > + *
> > + * The import is performed only once.
> > + *
> > + * This function is a part of the U-Boot mac command and must be executed
> > + * after SPI flash initialization.
> > + */
> > +int mac_read_from_eeprom(void)
> > +{
> > + if (env_get_ulong(HW_INFO_MERGED_VARIABLE, 10, 0) == 0) {
> > + if (env_set_ulong(HW_INFO_MERGED_VARIABLE, 1))
> > + return -ENOENT;
> > + return cmd_hw_info_read(false);
> > + }
> > + return 0;
> > +}
> > +
> > +/**
> > + * print_platform_help() - Print the platform specific help
> > + *
> > + * Extend the existing U-Boot mac command description by also printing
> > + * platform specific help text.
> > + */
> > +static void print_platform_help(void)
> > +{
> > + printf("\nNote: arguments mac [id|num|errata|date|ports|port_number]\n"
> > + "are unavailable on Marvell Armada A37xx platforms.\n"
> > + "Use mac [read|save {parameter}] instead.\n"
> > + "Available parameters:\n"
> > + "pcb_slm\tPCB SLM number\n"
> > + "pcb_rev\tPCB revision number\n"
> > + "eco_rev\tECO revision number\n"
> > + "pcb_sn\tPCB SN\n"
> > + "ethaddr\tfirst MAC address\n"
> > + "eth[1-9]addr\tsecond-ninth MAC address\n");
> > +}
> > +
> > +/**
> > + * do_mac() - Standard U-Boot mac command implementation
> > + * @cmdtp: U-Boot command table
> > + * @flag: Execution flags
> > + * @argc: Count of arguments
> > + * @argv: Arguments
> > + *
> > + * This function implements the standard U-Boot mac command in a mostly
> > + * compatible way.
> > + * To conform to the general command structure as much as possible, the
> > + * command description from cmd/mac.c is followed.
> > + * Where not possible, convenient or sensible additional comments for the user
> > + * are added.
> > + */
> > +int do_mac(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > +{
> > + const char *cmd = argv[1];
> > + int ret = 0;
> > +
> > + if (argc == 1) {
> > + ret = cmd_hw_info_dump();
> > + if (ret)
> > + return -EINVAL;
> > + return CMD_RET_SUCCESS;
> > + }
> > +
> > + if (!strcmp(cmd, "read")) {
> > + if (cmd_hw_info_read(true))
> > + return -EINVAL;
> > + } else if (!strcmp(cmd, "save")) {
> > + if (argc != 3) {
> > + printf("Please pass an additional argument to specify, "
> > + "which env parameter to save.\n");
> > + return -EINVAL;
> > + }
> > + if (cmd_hw_info_save(argv[2]))
> > + return -EINVAL;
> > + } else {
> > + ret = cmd_usage(cmdtp);
> > + print_platform_help();
> > + return ret;
> > + }
> > +
> > + return CMD_RET_SUCCESS;
> > +}
> > diff --git a/include/configs/mvebu_armada-37xx.h b/include/configs/mvebu_armada-37xx.h
> > index c8c34d7d92..8e8bcfa018 100644
> > --- a/include/configs/mvebu_armada-37xx.h
> > +++ b/include/configs/mvebu_armada-37xx.h
> > @@ -62,6 +62,13 @@
> > */
> > #define DEFAULT_ENV_IS_RW /* required for configuring default fdtfile= */
> >
> > +/*
> > + * Platform identification (Marvell hw_info parameters)
> > + */
> > +#ifdef CONFIG_MVEBU_MAC_HW_INFO
> > +#define CONFIG_ID_EEPROM /* U-Boot mac command */
> > +#endif
> > +
> > /*
> > * Ethernet Driver configuration
> > */
> > 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.31.1
> >
Kind regards,
Luka
More information about the U-Boot
mailing list