[PATCH 1/2] arm: mvebu: Implement the mac command (Marvell hw_info)
Luka Kovacic
luka.kovacic at sartura.hr
Fri Oct 8 15:28:03 CEST 2021
Hello Pali,
On Fri, Oct 8, 2021 at 2:53 PM Pali Rohár <pali at kernel.org> wrote:
>
> Hello!
>
> On Friday 08 October 2021 14:09:23 Robert Marko wrote:
> > From: Luka Kovacic <luka.kovacic at sartura.hr>
> >
> > The mac command is implemented to enable parsing Marvell hw_info formatted
> > environments. This format is often used on Marvell Armada 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.
>
> Is there any documentation how is checksum stored in this hw_info
> structure?
>
There probably isn't any public documentation.
This implementation was written using the public Marvell U-Boot source code
which is hosted on GitHub (MarvellEmbeddedProcessors/u-boot-marvell).
Anyway, this shouldn't be much of a problem for the initial version, as SPI
flash is quite reliable and data written here can also be read by the official
version and vice versa.
> > 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.
> >
> > Support for this command is added for Marvell Armada A37XX and 7K/8K
> > devices.
> >
> > Usage example:
> > => mac read
> > => saveenv
> >
> > 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/common/Kconfig | 29 +++
> > board/Marvell/common/Makefile | 5 +
> > board/Marvell/common/mac.c | 391 ++++++++++++++++++++++++++++
> > include/configs/mvebu_armada-37xx.h | 7 +
> > include/configs/mvebu_armada-8k.h | 7 +
> > lib/hashtable.c | 2 +-
> > 7 files changed, 441 insertions(+), 1 deletion(-)
> > create mode 100644 board/Marvell/common/Kconfig
> > create mode 100644 board/Marvell/common/Makefile
> > create mode 100644 board/Marvell/common/mac.c
> >
> > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > index 087643725e..d48de626ea 100644
> > --- a/arch/arm/mach-mvebu/Kconfig
> > +++ b/arch/arm/mach-mvebu/Kconfig
> > @@ -336,6 +336,7 @@ config SECURED_MODE_CSK_INDEX
> > default 0
> > depends on SECURED_MODE_IMAGE
> >
> > +source "board/Marvell/common/Kconfig"
> > source "board/solidrun/clearfog/Kconfig"
> > source "board/kobol/helios4/Kconfig"
> >
> > diff --git a/board/Marvell/common/Kconfig b/board/Marvell/common/Kconfig
> > new file mode 100644
> > index 0000000000..473a83b05b
> > --- /dev/null
> > +++ b/board/Marvell/common/Kconfig
> > @@ -0,0 +1,29 @@
> > +menu "Marvell Armada common configuration"
> > +depends on TARGET_MVEBU_ARMADA_37XX || TARGET_MVEBU_ARMADA_8K
> > +
> > +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.
>
> Have you tried to specify this offset directly into DTS file? Because
> in DTS file is already specified this hw info partition and it seems
> like that this kind of information belongs to DTS.
I haven't encountered a board, which has a different offset so far.
This can be treated as a nicer way of defining this offset, rather
than just hard-coding it in the source code itself.
In case there are more boards with this partition, a way of defining
the offset in the DTS can be added later and this value can then
be used as a default.
>
> Otherwise, patch looks good now.
>
> Reviewed-by: Pali Rohár <pali at kernel.org>
>
> > +
> > +endmenu
> > diff --git a/board/Marvell/common/Makefile b/board/Marvell/common/Makefile
> > new file mode 100644
> > index 0000000000..072c3e49de
> > --- /dev/null
> > +++ b/board/Marvell/common/Makefile
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright (c) 2021 Sartura Ltd.
> > +
> > +obj-$(CONFIG_ID_EEPROM) += mac.o
> > diff --git a/board/Marvell/common/mac.c b/board/Marvell/common/mac.c
> > new file mode 100644
> > index 0000000000..91e5cd8dcf
> > --- /dev/null
> > +++ b/board/Marvell/common/mac.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 755f59eee9..b364a57517 100644
> > --- a/include/configs/mvebu_armada-37xx.h
> > +++ b/include/configs/mvebu_armada-37xx.h
> > @@ -46,6 +46,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/include/configs/mvebu_armada-8k.h b/include/configs/mvebu_armada-8k.h
> > index 2f8be2ee49..c474494f7c 100644
> > --- a/include/configs/mvebu_armada-8k.h
> > +++ b/include/configs/mvebu_armada-8k.h
> > @@ -30,6 +30,13 @@
> > /* End of 16M scrubbed by training in bootrom */
> > #define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_TEXT_BASE + 0xFF0000)
> >
> > +/*
> > + * Platform identification (Marvell hw_info parameters)
> > + */
> > +#ifdef CONFIG_MVEBU_MAC_HW_INFO
> > +#define CONFIG_ID_EEPROM /* U-Boot mac command */
> > +#endif
> > +
> > /* When runtime detection fails this is the default */
> >
> > #define CONFIG_SYS_MAX_NAND_DEVICE 1
> > 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.33.0
> >
Kind regards,
Luka
More information about the U-Boot
mailing list