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_MVEBUdefault nhelpEnable loading of the Marvell hw_info parameters from theSPI flash hw_info area. Parameters (usually the board serialnumber and MAC addresses) are then imported into theexisting U-Boot environment.Implementation of this command is compatible with theoriginal Marvell U-Boot command. Reading and writing issupported.EEPROM config pattern and checksum aren't supported.After enabled, these parameters are managed from the commonU-Boot mac command.+config MVEBU_MAC_HW_INFO_OFFSET
hex "Marvell hw_info (mac) SPI flash offset"depends on MVEBU_MAC_HW_INFOdefault 0x3E0000helpThis option defines the SPI flash offset of the Marvellhw_info area. This defaults to 0x3E0000 on most ArmadaA3720 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