[U-Boot] [PATCH v2 04/11] drivers:dfu: new feature: separated bootloader alt setting

[Stephen Warren]

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.

> # 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?

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

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.