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,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to