On 05.02.21 23:51, Luka Kovacic wrote:
On Fri, Feb 5, 2021 at 11:40 PM Tom Rini <tr...@konsulko.com> wrote:
On Fri, Feb 05, 2021 at 11:36:25PM +0100, Luka Kovacic wrote:
On Fri, Feb 5, 2021 at 11:04 PM Tom Rini <tr...@konsulko.com> wrote:
On Fri, Feb 05, 2021 at 11:02:23PM +0100, Luka Kovacic wrote:
Hello Tom,
On Thu, Feb 4, 2021 at 6:06 PM Tom Rini <tr...@konsulko.com> wrote:
On Thu, Feb 04, 2021 at 11:46:35AM +0100, Luka Kovacic wrote:
The '-h' flag is added to the 'env import' command to enable parsing
Marvell hw_info formatted environments.
This format is often used on Marvell Armada A37XX based devices to store
parameters like the board serial number, factory MAC addresses and some
other information.
Currently this environment format can only be imported, not exported.
These parameters are usually written to the flash in the factory.
This functionality has been tested on the GST ESPRESSOBin-Ultra board
successfully.
Usage example:
=> sf probe
=> sf read ${loadaddr} 0x003E000A 0x1F0 # Read the environment from
SPI flash
=> env import -h ${loadaddr}
Signed-off-by: Luka Kovacic <luka.kova...@sartura.hr>
Cc: Luka Perkov <luka.per...@sartura.hr>
Cc: Robert Marko <robert.ma...@sartura.hr>
So, the implementation itself is fine:
Reviewed-by: Tom Rini <tr...@konsulko.com>
And I take it as a given that we can't get the boards populated with an
existing separator, so we need to solve this somehow or another. I am
however loathe to increase every platform by a handful of bytes (help
text, code text) to cover this case, and adding #ifdef around the help
text in particular would be very ugly. Is there any clean way to write
this as a board-specific command? I assume the overall intention is to
import the factory env for the required information, store it in regular
U-Boot environment and then never touch the factory area again. Thanks!
I have already considered the possibility of implementing this in a
board-specific way, but I tried implementing it here to avoid unnecessary
code duplication.
You are correct, the intention is to use this functionality as a migration
path. It's good to keep the factory area intact, in case it's still needed in
the future.
The Marvell hw_info area is limited to MACs and serial numbers by the
stock utility.
OK. And can we put this in the board-specific path somehow, relatively
cleanly?
I've been thinking about the following options:
- Implementing a separate command under cmd/mvebu (hw_info), like done
in the stock U-Boot.
I'm not inclined to this one, as I don't see real utility in updating
this factory
area. A positive with this one is that it abstracts reading of the SPI flash.
- No command, automatic migration, directly from the SPI flash (implemented
in the board.c file.
I don't see this as a clean way of doing it, the Armada A37XX board.c is
already quite full of board-specific init.
- Implemented as is, but with #ifdef
This would break CONFIG_CMD_IMPORTENV into smaller pieces.
#ifdef wouldn't look too bad around the code, only the inline string changes
would be ugly.
What would you recommend?
I hate to say it but I prefer the first, especially if it ends up being
like 50 lines of code or whatever to write the function and all that.
Or the second option, if the first option really ends up being messy
once you write it and the second one less messy. Thanks!
Okay, I agree.
I think the first option should end up cleaner than both other options.
I'll do it this way.
Ok. Please rebase on latest mainline and send v2 once it's ready.
Thanks,
Stefan
