Hello,
On 06/17/2014 06:36 PM, Stephen Warren wrote:
On 06/17/2014 04:20 AM, Przemyslaw Marczak wrote:
Hello Stephen,
On 06/16/2014 09:52 PM, Stephen Warren wrote:
On 06/12/2014 03:46 AM, Przemyslaw Marczak wrote:
This patch introduces new feature: initialization of the dfu bootloader entity from a separate environmental variable which can be set on a boot time.
By default, DFU always check environmental variable: $dfu_alt_info.
Changes:
- DFU will also look for environmental variable: $dfu_alt_bootloader
- if any of dfu_alt_* variable is properly defined, then function dfu_init_env_entities() will return success.
Use case: Some devices can boot from various media type (SD, eMMC, NAND, etc.) or some board configs are common for more than one board type. In a such case, bootloader is probably placed on a different devices or even offsets. So such DFU feature is welcome.
Why should the "dfu" command look at different environment variables? Whatever code dynamically sets the value of $dfu_alt_bootloader should simply set $dfu_alt_info instead.
Dynamically setting of any entity cold be done at boot time, like this:
# int buf_len = strlen(alt_bootloader) + strlen(CONFIG_DFU_ALT) + 4; # char *buf = memalign(1, buf_len); # # sprintf(buf, "%s; %s", alt_bootloader, CONFIG_DFU_ALT);
Note that you can't have a space after the ; due to the use of strsep(). Switching to strtok() might solve that.
Yes - this wasn't tested - just some pseudo code.
# setenv("dfu_alt_info", buf);
But overwriting the $dfu_alt_info on each boot is not a good idea. If user modify the dfu entities - then it will be overwritten at next boot.
What if the user sees that $dfu_alt_bootloader is set, and modifies that without realizing that it's an auto-generated variable? The same thing then applies.
Perhaps we need a standard where system-/automatically-set variables are named with a auto_ prefix or _auto suffix or something, to make this clear?
Yes, right note.
I'd prefer that the dfu command didn't use any environment variables, but rather required the user to always pass the list on the command-line. Then, the user could invoke either:
dfu "foo mmc x..." # Manually specified dfu $dfu_alt_info # Use 'user-defined' variable dfu $dfu_alt_bootloaer # Use 'system-defined' variable
Yes, definitely such feature was missing there.
That way, the code doesn't have to loop over a bunch of variables and get more complex. Implicitly depending on environment variables make it hard to tell what a sequence of commands will do.
...
Maybe better could be modification of the function dfu_init_env_entities() to support parsing variables in the $dfu_alt_info instead of hard coded env variables names, e.g:
dfu_alt_info="${alt_info_boot}, ${alt_info_system},..."
I feel like the shell already has the capability to interpolate variable values into strings, so this would be quite easy to do in the shell rather than duplicating that code inside the dfu command.
Every env macro passed with cmdline will be expanded. And then, we can use such style like this:
# setenv alt_kernel "uImage ext4 0 2;zImage ext4 0 2" # setenv alt_system "boot part 0 2;root part 0 3" # setenv auto_alt_bootloader "u-boot raw 0x0 0x800" # setenv alt_info "${alt_kernel};${alt_system};${auto_alt_bootloader}" (this will expand when passing to "setenv")
or just put this in env default config:
"alt_info=${alt_kernel};${alt_system};${auto_alt_bootloader}\0" ...
So summarizing, I don't want to break your DFU rework, I want just to add the Odroid U3 support, so in the next patch set I will use the $dfu_alt_info, instead of combining with a short time solution.
And after your work will be done, then I will update Odroid code.
Best regards,