[PATCH 1/2] arm: mvebu: Implement the mac command (Marvell hw_info)

From: Luka Kovacic luka.kovacic@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.
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@sartura.hr Cc: Luka Perkov luka.perkov@sartura.hr Cc: Robert Marko robert.marko@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. + +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@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,

Globalscale MOCHAbin is a Armada 7040 based development board.
Specifications: * Armada 7040 Quad core ARMv8 Cortex A-72 @ 1.4GHz * 2 / 4 / 8 GB of DDR4 DRAM * 16 GB eMMC * 4MB SPI-NOR (Bootloader) * 1x M.2-2280 B-key socket (for SSD expansion, SATA3 only) * 1x M.2-2250 B-key socket (for modems, USB2.0 and I2C only) * 1x Mini-PCIe 3.0 (x1, USB2.0 and I2C) * 1x SATA 7+15 socket (SATA3) * 1x 16-pin (2×8) MikroBus Connector * 1x SIM card slot (Connected to the mini-PCIe and both M.2 slots) * 2x USB3.0 Type-A ports via SMSC USB5434B hub * Cortex 2x5 JTAG * microUSB port for UART (PL2303GL/PL2303SA onboard) * 1x 10G SFP+ * 1x 1G SFP (Connected to 88E1512 PHY) * 1x 1G RJ45 with PoE PD (Connected to 88E1512 PHY) * 4x 1G RJ45 ports via Topaz 88E6141 switch * RTC with battery holder (SoC provided, requires CR2032 battery) * 1x 12V DC IN * 1x Power switch * 1x 12V fan header (3-pin, power only) * 1x mini-PCIe LED header (2x0.1" pins) * 1x M.2-2280 LED header (2x0.1" pins) * 6x Bootstrap jumpers * 1x Power LED (Green) * 3x Tri-color RGB LEDs (Controllable) * 1x Microchip ATECC608B secure element
Note that 1G SFP and 1G WAN cannot be used at the same time as they are in parallel connected to the same PHY.
Additionally, automatic import of the Marvell hw_info parameters is enabled via the recently added mac command for Armada platforms. The parameters stored in Marvell hw_info are usually the board serial number and MAC addresses.
Signed-off-by: Robert Marko robert.marko@sartura.hr --- arch/arm/dts/Makefile | 1 + arch/arm/dts/armada-7040-mochabin.dts | 304 +++++++++++++++++++++++ configs/mvebu_mochabin-88f7040_defconfig | 85 +++++++ 3 files changed, 390 insertions(+) create mode 100644 arch/arm/dts/armada-7040-mochabin.dts create mode 100644 configs/mvebu_mochabin-88f7040_defconfig
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index b8a382d153..249f9d02c2 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -232,6 +232,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ armada-38x-controlcenterdc.dtb \ armada-7040-db-nand.dtb \ armada-7040-db.dtb \ + armada-7040-mochabin.dtb \ armada-8040-clearfog-gt-8k.dtb \ armada-8040-db.dtb \ armada-8040-mcbin.dtb \ diff --git a/arch/arm/dts/armada-7040-mochabin.dts b/arch/arm/dts/armada-7040-mochabin.dts new file mode 100644 index 0000000000..1110907a23 --- /dev/null +++ b/arch/arm/dts/armada-7040-mochabin.dts @@ -0,0 +1,304 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2016- 2021 Marvell International Ltd. + * Copyright (C) 2021 Sartura Ltd. + */ + +/* + * Device Tree file for Globalscale MOCHAbin + * Boot device: SPI NOR, 0x32 (SW3) + */ + +#include "armada-7040.dtsi" + +/ { + model = "Globalscale MOCHAbin"; + compatible = "globalscale,mochabin", "marvell,armada7040", + "marvell,armada-ap806-quad", "marvell,armada-ap806"; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + aliases { + i2c0 = &cp0_i2c0; + spi0 = &cp0_spi1; + }; + + memory@00000000 { + device_type = "memory"; + reg = <0x0 0x0 0x0 0x80000000>; + }; +}; + +&ap_pinctl { + /* MPP Bus: + * SDIO [0-5] + * UART0 [11,19] + */ + /* 0 1 2 3 4 5 6 7 8 9 */ + pin-func = < 1 1 1 1 1 1 0 0 0 0 + 0 3 0 0 0 0 0 0 0 3 >; +}; + +/* microUSB UART console */ +&uart0 { + status = "okay"; +}; + +/* eMMC */ +&ap_sdhci0 { + status = "okay"; + + bus-width = <4>; + no-1-8-v; + non-removable; +}; + +&cp0_pinctl { + /* MPP Bus: + * E6341_RSTn [0] + * E6341_INT [1] + * I2C1 [2,3] + * PCIE_CLKREQn [5] + * UART0 [6,7] + * PCIE_WAKEn [8] + * PCIE_RSTOUTn [9] (GPIO) + * DEVSLP_M2-2 [10] + * PHY0_RSTn [12] (88E1512 PHY RST) + * SPI1 [13-16] + * PHY0_INT [24] (88E1512 PHY INT) + * 9554_INT [27] (PCA9554 INT) + * IS_CLK_VBM [29] (is31fl3199-clk-vbm) + * IS_SDB [30] (is31fl3199-sdb) + * MKR_PWM [31] (mikroBus PWM) + * MKR_INT [32] (mikroBus INT) + * MKR_RST [33] (mikroBus RST) + * DEVSLP_M2-1 [34] (M.2 SATA DevSlp) + * SMI [35,36] + * I2C0 [37,38] + * RGMII2 [44-55] + * SPI0 [56-59] + * GPIO [4,11,17-23,25-26,28] + */ + /* 0 1 2 3 4 5 6 7 8 9 */ + pin-func = < 0 0 7 7 0 7 8 8 0 0 + 0 0 0 3 3 3 3 0 0 0 + 0 0 0 0 0 0 0 0 0 0 + 0 0 0 0 0 8 8 2 2 0 + 0 0 0 0 1 1 1 1 1 1 + 1 1 1 1 1 1 6 6 6 6 + 0 0 0 >; + + cp0_i2c1_pins: cp0-i2c-pins-1 { + marvell,pins = < 2 3 >; + marvell,function = <7>; + }; + + cp0_spi0_pins: cp0-spi-pins-0 { + marvell,pins = < 56 57 58 59 >; + marvell,function = <6>; + }; + + cp0_spi1_pins: cp0-spi-pins-1 { + marvell,pins = < 13 14 15 16 >; + marvell,function = <3>; + }; + + cp0_smi_pins: cp0-smi-pins { + marvell,pins = < 35 36 >; + marvell,function = <8>; + }; +}; + +/* mikroBUS SPI */ +&cp0_spi0 { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&cp0_spi0_pins>; +}; + +/* SPI-NOR */ +&cp0_spi1 { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&cp0_spi1_pins>; + + spi-flash@0 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <20000000>; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + label = "u-boot"; + reg = <0x0 0x3e0000>; + }; + + partition@3e0000 { + label = "hw-info"; + reg = <0x3e0000 0x10000>; + read-only; + }; + + partition@3f0000 { + label = "u-boot-env"; + reg = <0x3f0000 0x10000>; + }; + }; + }; +}; + +/* mikroBUS, 1G SFP and GPIO expander */ +&cp0_i2c0 { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&cp0_i2c0_pins>; + clock-frequency = <100000>; + + sfp_gpio: pca9554@39 { + compatible = "nxp,pca9554"; + reg = <0x39>; + + gpio-controller; + #gpio-cells = <2>; + + /* + * IO0_0: SFP+_TX_FAULT + * IO0_1: SFP+_TX_DISABLE + * IO0_2: SFP+_PRSNT + * IO0_3: SFP+_LOSS + * IO0_4: SFP_TX_FAULT + * IO0_5: SFP_TX_DISABLE + * IO0_6: SFP_PRSNT + * IO0_7: SFP_LOSS + */ + }; +}; + +/* IS31FL3199, mini-PCIe and 10G SFP+ */ +&cp0_i2c1 { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&cp0_i2c1_pins>; + clock-frequency = <100000>; +}; + +&cp0_comphy { + phy0 { + phy-type = <COMPHY_TYPE_SGMII1>; + phy-speed = <COMPHY_SPEED_3_125G>; + }; + + phy1 { + phy-type = <COMPHY_TYPE_USB3_HOST0>; + }; + + phy2 { + phy-type = <COMPHY_TYPE_SATA0>; + }; + + phy3 { + phy-type = <COMPHY_TYPE_SATA1>; + }; + + phy4 { + phy-type = <COMPHY_TYPE_SFI0>; + phy-speed = <COMPHY_SPEED_10_3125G>; + }; + + phy5 { + phy-type = <COMPHY_TYPE_PEX2>; + }; +}; + +&cp0_sata0 { + status = "okay"; +}; + +/* SMSC USB5434B hub */ +&cp0_usb3_0 { + status = "okay"; +}; + +/* miniPCI-E USB */ +&cp0_usb3_1 { + status = "okay"; +}; + +/* SMSC USB5434B hub */ +&cp0_utmi0 { + status = "okay"; +}; + +/* miniPCI-E USB */ +&cp0_utmi1 { + status = "okay"; +}; + +/* miniPCI-E (J5) */ +&cp0_pcie2 { + status = "okay"; + marvell,reset-gpio = <&cp0_gpio0 9 GPIO_ACTIVE_LOW>; +}; + +&cp0_mdio { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&cp0_smi_pins>; + + /* 88E1512 PHY */ + eth2phy: ethernet-phy@1 { + reg = <1>; + }; +}; + +&cp0_ethernet { + status = "okay"; +}; + +/* 10G SFP+ */ +&cp0_eth0 { + status = "okay"; + + phy-mode = "sfi"; + managed = "in-band-status"; + marvell,sfp-tx-disable-gpio = <&sfp_gpio 1 GPIO_ACTIVE_HIGH>; +}; + +/* Topaz switch uplink */ +&cp0_eth1 { + status = "okay"; + + phy-mode = "2500base-x"; + phy-reset-gpios = <&cp0_gpio0 0 GPIO_ACTIVE_LOW>; + + fixed-link { + speed = <2500>; + full-duplex; + }; +}; + +/* 1G SFP or 1G RJ45 */ +&cp0_eth2 { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&cp0_ge2_rgmii_pins>; + + phy-mode = "rgmii-id"; + phy = <ð2phy>; + marvell,sfp-tx-disable-gpio = <&sfp_gpio 5 GPIO_ACTIVE_HIGH>; + phy-reset-gpios = <&cp0_gpio0 12 GPIO_ACTIVE_LOW>; +}; diff --git a/configs/mvebu_mochabin-88f7040_defconfig b/configs/mvebu_mochabin-88f7040_defconfig new file mode 100644 index 0000000000..d5d1e28848 --- /dev/null +++ b/configs/mvebu_mochabin-88f7040_defconfig @@ -0,0 +1,85 @@ +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=2 +CONFIG_TARGET_MVEBU_ARMADA_8K=y +CONFIG_MVEBU_MAC_HW_INFO=y +CONFIG_ENV_SIZE=0x10000 +CONFIG_ENV_OFFSET=0x3f0000 +CONFIG_ENV_SECT_SIZE=0x10000 +CONFIG_DM_GPIO=y +CONFIG_DEFAULT_DEVICE_TREE="armada-7040-mochabin" +CONFIG_DEBUG_UART_BASE=0xf0512000 +CONFIG_DEBUG_UART_CLOCK=200000000 +CONFIG_DEBUG_UART=y +CONFIG_AHCI=y +CONFIG_DISTRO_DEFAULTS=y +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set +CONFIG_SYS_LOAD_ADDR=0x800000 +CONFIG_FIT=y +CONFIG_FIT_VERBOSE=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_USE_PREBOOT=y +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_CMD_GPIO=y +CONFIG_CMD_GPT=y +# CONFIG_RANDOM_UUID is not set +CONFIG_CMD_I2C=y +CONFIG_CMD_MMC=y +CONFIG_CMD_PCI=y +CONFIG_CMD_SATA=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_REGULATOR=y +CONFIG_CMD_EXT4_WRITE=y +# CONFIG_ISO_PARTITION is not set +CONFIG_ENV_IS_IN_SPI_FLASH=y +CONFIG_SYS_RELOC_GD_ENV_ADDR=y +CONFIG_NET_RANDOM_ETHADDR=y +CONFIG_AHCI_MVEBU=y +CONFIG_GPIO_HOG=y +CONFIG_DM_PCA953X=y +CONFIG_DM_I2C=y +CONFIG_SYS_I2C_MVTWSI=y +CONFIG_MISC=y +CONFIG_MMC_SDHCI=y +CONFIG_MMC_SDHCI_XENON=y +CONFIG_DM_MTD=y +CONFIG_SPI_FLASH_BAR=y +CONFIG_SPI_FLASH_WINBOND=y +CONFIG_PHY_MARVELL=y +CONFIG_PHY_GIGE=y +CONFIG_MVPP2=y +CONFIG_RGMII=y +CONFIG_PCI=y +CONFIG_PCIE_DW_MVEBU=y +CONFIG_PHY=y +CONFIG_MVEBU_COMPHY_SUPPORT=y +CONFIG_PINCTRL=y +CONFIG_PINCTRL_ARMADA_8K=y +CONFIG_DM_REGULATOR_FIXED=y +CONFIG_DM_RTC=y +CONFIG_RTC_ARMADA38X=y +CONFIG_DEBUG_UART_SHIFT=2 +CONFIG_DEBUG_UART_ANNOUNCE=y +CONFIG_SYS_NS16550=y +CONFIG_KIRKWOOD_SPI=y +CONFIG_USB=y +CONFIG_USB_XHCI_HCD=y +CONFIG_USB_EHCI_HCD=y +# CONFIG_EFI_LOADER is not set

Hello!
On Friday 08 October 2021 14:09:23 Robert Marko wrote:
From: Luka Kovacic luka.kovacic@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?
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@sartura.hr Cc: Luka Perkov luka.perkov@sartura.hr Cc: Robert Marko robert.marko@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.
Otherwise, patch looks good now.
Reviewed-by: Pali Rohár pali@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@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

Hello Pali,
On Fri, Oct 8, 2021 at 2:53 PM Pali Rohár pali@kernel.org wrote:
Hello!
On Friday 08 October 2021 14:09:23 Robert Marko wrote:
From: Luka Kovacic luka.kovacic@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@sartura.hr Cc: Luka Perkov luka.perkov@sartura.hr Cc: Robert Marko robert.marko@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@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@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

Hello!
On Friday 08 October 2021 15:28:03 Luka Kovacic wrote:
Hello Pali,
On Fri, Oct 8, 2021 at 2:53 PM Pali Rohár pali@kernel.org wrote:
Hello!
On Friday 08 October 2021 14:09:23 Robert Marko wrote:
From: Luka Kovacic luka.kovacic@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.
Are you planning to implement that checksum verification support? I understand that purpose of checksum is there to ensure that only valid data are processed and used.
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@sartura.hr Cc: Luka Perkov luka.perkov@sartura.hr Cc: Robert Marko robert.marko@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.
+Marek
My understanding is that all these definitions, like memory address spaces, partitions, etc... belong to DTS file (or plat structures for boards which do not use DT) and not into source code or config options as they are describing hw layout.
There is ongoing process to move / convert SPI partitions from source files and config defines into DTS files, so for me this looks like a step backward...
But I would like to hear opinion also from other people.
Otherwise, patch looks good now.
Reviewed-by: Pali Rohár pali@kernel.org

+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.
+Marek
My understanding is that all these definitions, like memory address spaces, partitions, etc... belong to DTS file (or plat structures for boards which do not use DT) and not into source code or config options as they are describing hw layout.
There is ongoing process to move / convert SPI partitions from source files and config defines into DTS files, so for me this looks like a step backward...
But I would like to hear opinion also from other people.
This should be done in device tree. Device tree has bindings for such things. You should be able to do something like this:
spi-flash@1 { partitions { compatible = "fixed-partitions"; #address-cells = <1>; #size-cells = <1>;
board_info: board-info@3E0000 { compatible = "nvmem-cells"; label = "board-info"; reg = <0x3E0000 0x1000>; read-only; #address-cells = <1>; #size-cells = <1>;
mac_addr_eth0: mac-addr1@0 { reg = <0x0 0x6>; };
mac_addr_eth1: mac-addr2@1 { reg = <0x6 0x6>; }; }; }; };
ethernet@30000 { nvmem-cells = <&mac_addr_eth0>; nvmem-cell-names = "mac-address"; };
Look at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
Also we need to add nvmem API into U-Boot and get rid of the ad-hoc efuse/mac/hw_mac nonsense.
Marek

On Sat, Oct 9, 2021 at 11:42 AM Marek Behún kabel@kernel.org wrote:
+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.
+Marek
My understanding is that all these definitions, like memory address spaces, partitions, etc... belong to DTS file (or plat structures for boards which do not use DT) and not into source code or config options as they are describing hw layout.
There is ongoing process to move / convert SPI partitions from source files and config defines into DTS files, so for me this looks like a step backward...
But I would like to hear opinion also from other people.
This should be done in device tree. Device tree has bindings for such things. You should be able to do something like this:
spi-flash@1 { partitions { compatible = "fixed-partitions"; #address-cells = <1>; #size-cells = <1>;
board_info: board-info@3E0000 { compatible = "nvmem-cells"; label = "board-info"; reg = <0x3E0000 0x1000>; read-only; #address-cells = <1>; #size-cells = <1>; mac_addr_eth0: mac-addr1@0 { reg = <0x0 0x6>; }; mac_addr_eth1: mac-addr2@1 { reg = <0x6 0x6>; }; }; };
};
ethernet@30000 { nvmem-cells = <&mac_addr_eth0>; nvmem-cell-names = "mac-address"; };
The thing is that the position of each value is not fixed, they can be in whatever order or position inside of the partition and they are not TLV as there is no length argument stored. They are really similar to the traditional U-boot env so you cant use generic NVMEM cells and parse by position and length. But I am sure that we can add a compatible for the hw_info parser and then it can parse the reg property directly instead of using KConfig for that.
I am sure Luka knows more about the format than me.
Regards, Robert
Look at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
Also we need to add nvmem API into U-Boot and get rid of the ad-hoc efuse/mac/hw_mac nonsense.
This would be great for a lot of devices.
Marek

On Saturday 09 October 2021 12:09:28 Robert Marko wrote:
On Sat, Oct 9, 2021 at 11:42 AM Marek Behún kabel@kernel.org wrote:
+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.
+Marek
My understanding is that all these definitions, like memory address spaces, partitions, etc... belong to DTS file (or plat structures for boards which do not use DT) and not into source code or config options as they are describing hw layout.
There is ongoing process to move / convert SPI partitions from source files and config defines into DTS files, so for me this looks like a step backward...
But I would like to hear opinion also from other people.
This should be done in device tree. Device tree has bindings for such things. You should be able to do something like this:
spi-flash@1 { partitions { compatible = "fixed-partitions"; #address-cells = <1>; #size-cells = <1>;
board_info: board-info@3E0000 { compatible = "nvmem-cells"; label = "board-info"; reg = <0x3E0000 0x1000>; read-only; #address-cells = <1>; #size-cells = <1>; mac_addr_eth0: mac-addr1@0 { reg = <0x0 0x6>; }; mac_addr_eth1: mac-addr2@1 { reg = <0x6 0x6>; }; }; };
};
ethernet@30000 { nvmem-cells = <&mac_addr_eth0>; nvmem-cell-names = "mac-address"; };
The thing is that the position of each value is not fixed, they can be in whatever order or position inside of the partition and they are not TLV as there is no length argument stored. They are really similar to the traditional U-boot env so you cant use generic NVMEM cells and parse by position and length. But I am sure that we can add a compatible for the hw_info parser and then it can parse the reg property directly instead of using KConfig for that.
Something like this? compatible = "marvell,hw-info"
I am sure Luka knows more about the format than me.
Regards, Robert
Look at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
Also we need to add nvmem API into U-Boot and get rid of the ad-hoc efuse/mac/hw_mac nonsense.
This would be great for a lot of devices.
Marek
-- Robert Marko Staff Embedded Linux Engineer Sartura Ltd. Lendavska ulica 16a 10000 Zagreb, Croatia Email: robert.marko@sartura.hr Web: www.sartura.hr

Hello Pali,
Something like this? compatible = "marvell,hw-info"
This compatible string looks good to me. We will send a new patch version, which implements the discussed DT functionality.
I am sure Luka knows more about the format than me.
The Marvell hw_info partition is very similar to the U-Boot environment. The only difference is in the separator between the "key=value" pairs, which is 0x20/space in the case of hw_info. This is also prevents us to hard-code the parameter addresses in the device tree, because they can move around.
The checksum is stored before the hw_info environment - this would have to be investigated further to implement checksum verification.
Kind regards, Luka

On Mon, 11 Oct 2021 12:19:07 +0200 Luka Kovacic luka.kovacic@sartura.hr wrote:
Hello Pali,
Something like this? compatible = "marvell,hw-info"
This compatible string looks good to me. We will send a new patch version, which implements the discussed DT functionality.
I am sure Luka knows more about the format than me.
The Marvell hw_info partition is very similar to the U-Boot environment. The only difference is in the separator between the "key=value" pairs, which is 0x20/space in the case of hw_info. This is also prevents us to hard-code the parameter addresses in the device tree, because they can move around.
The checksum is stored before the hw_info environment - this would have to be investigated further to implement checksum verification.
These differences between Marvell's version and U-Boot's standard environment version can be specified in device-tree (via compatible or other properties) and handled by one driver.
A driver for nvmem provider of standard U-Boot env could have this binding (with "denx,u-boot-env" as compatible, changing it to "marvell,hw-info" would make it work with spaces instead of '=' as separator):
spi-flash { blah blah blah;
partitions { compatible = "fixed-partitions"; #address-cells = <1>; #size-cells = <1>;
hw-info@18000 { compatible = "denx,u-boot-env"; reg = <0x18000 0x1000>; label = "hw-info";
eth0_mac_addr: ethaddr { compatible = "mac-address-string"; name = "ethaddr"; };
eth1_mac_addr: eth1addr { compatible = "mac-address-string"; name = "eth1addr"; }; }; }; };
ethernet@30000 { nvmem-cells = <ð0_mac_addr>; nvmem-cell-names = "mac-address"; };
On Turris MOX, we have the MAC address of the device stored in One-Time-Programmable memory accesible only by the secure coprocessor, which Linux communicates with via the turris-mox-rwtm kernel driver.
Currently this driver does not register itself as a nvmem provider, and so isn't use by the ethernet controller driver to get MAC addresses. MAC addresses are currently read by U-Boot board code and the controller keeps these to Linux.
But I plan to extend the turris-mox-rwtm driver to also provide MAC addresses via nvmem API, and then specify in device-tree ð1 { nvmem-cells = <&rwtm_eth1_mac>; nvmem-cell-names = "address"; };
Maybe in the future you will also want to implement this in Linux. It could be done simply by adding a new type of nvmem provider, with compatible = "marvell,hw-info".
Luka, what do you think?
Marek

Hello Marek,
These differences between Marvell's version and U-Boot's standard environment version can be specified in device-tree (via compatible or other properties) and handled by one driver.
A driver for nvmem provider of standard U-Boot env could have this binding (with "denx,u-boot-env" as compatible, changing it to "marvell,hw-info" would make it work with spaces instead of '=' as separator):
An nvmem provider would be an interesting approach to read U-Boot environment parameters from Linux and to maybe even get the U-Boot env partition location in U-Boot (like DM for the env).
A single driver would work, maybe support for the checksum would get more difficult to integrate.
spi-flash { blah blah blah;
partitions { compatible = "fixed-partitions"; #address-cells = <1>; #size-cells = <1>; hw-info@18000 { compatible = "denx,u-boot-env"; reg = <0x18000 0x1000>; label = "hw-info"; eth0_mac_addr: ethaddr { compatible = "mac-address-string"; name = "ethaddr"; }; eth1_mac_addr: eth1addr { compatible = "mac-address-string"; name = "eth1addr"; };
I don't see any better approach than just matching strings to retrieve values for specific keys (for MACs), so this looks good to me.
}; };
};
ethernet@30000 { nvmem-cells = <ð0_mac_addr>; nvmem-cell-names = "mac-address"; };
On Turris MOX, we have the MAC address of the device stored in One-Time-Programmable memory accesible only by the secure coprocessor, which Linux communicates with via the turris-mox-rwtm kernel driver.
Currently this driver does not register itself as a nvmem provider, and so isn't use by the ethernet controller driver to get MAC addresses. MAC addresses are currently read by U-Boot board code and the controller keeps these to Linux.
But I plan to extend the turris-mox-rwtm driver to also provide MAC addresses via nvmem API, and then specify in device-tree ð1 { nvmem-cells = <&rwtm_eth1_mac>; nvmem-cell-names = "address"; };
Maybe in the future you will also want to implement this in Linux. It could be done simply by adding a new type of nvmem provider, with compatible = "marvell,hw-info".
Luka, what do you think?
Also we need to add nvmem API into U-Boot and get rid of the ad-hoc efuse/mac/hw_mac nonsense.
I agree, a real nvmem API would be much cleaner than the current U-Boot implementation, as there is currently no way to programmatically access these parameters and the implementations have different user interfaces.
As there is currently no nvmem framework, I recommend that the basic, futureproof DT bindings are defined and DT parsing is temporarily implemented in the hw_info mac command. What do you think?
Is anyone already working on a nvmem framework to support nvmem providers in U-Boot?
Kind regards, Luka

On Mon, 11 Oct 2021 18:16:02 +0200 Luka Kovacic luka.kovacic@sartura.hr wrote:
eth1_mac_addr: eth1addr { compatible = "mac-address-string"; name = "eth1addr"; };
I don't see any better approach than just matching strings to retrieve values for specific keys (for MACs), so this looks good to me.
The `name` property can be omitted, the node name should be used, since it is unique both in DT and in env.
MAC addresses will need a special compatible property so that the nvmem driver knows to convert them from 'XX:XX:XX:XX:XX:XX' string to 6-byte value.
I agree, a real nvmem API would be much cleaner than the current U-Boot implementation, as there is currently no way to programmatically access these parameters and the implementations have different user interfaces.
As there is currently no nvmem framework, I recommend that the basic, futureproof DT bindings are defined and DT parsing is temporarily implemented in the hw_info mac command. What do you think?
Yes, that is acceptable.
Is anyone already working on a nvmem framework to support nvmem providers in U-Boot?
AFAIK no, but I am planning to look into this.
In the meantime implement the hw_info mac command.
I will send proposal for dt-binding.
Marek

Is anyone already working on a nvmem framework to support nvmem providers in U-Boot?
AFAIK no, but I am planning to look into this.
Ok. I'd be happy to assist in any way you see fit with this in my free time.
In the meantime implement the hw_info mac command.
Okay.
I will send proposal for dt-binding.
Sounds good.
Kind regards, Luka
participants (4)
-
Luka Kovacic
-
Marek Behún
-
Pali Rohár
-
Robert Marko