[U-Boot] add usb_system_autoupdate command.

Wolfgang Denk wd at denx.de
Wed Dec 10 00:06:11 CET 2008


Dear Matteo,

In message <492176A0.7040101 at sirius-es.it> you wrote:
> 
> I looked updater.c and and I take that code as example.
> 
> I removed all function as same as do_... (do_fat_fsload, do_protect, 
> do_flerase, do_mem_cp).
> In this way, I could remove also strcpy() and sprintf() needed to made 
> their parameters and the result is that I cleaned and emproved all code.
> 
> Now all works correctly.

Hm... it may work for you, but it's mostly unusable by anybody else
given the complete lack of documentation.

> In attach, the code...
> and if anyone try it, I'm interesting to feedback or opinions.

Here a general comment first: we already have quitre a number of such
auto-update mechanisms in U-Boot, including  updates  from  USB  mass
storage device (like yours, or board/trab/auto_update.c or
board/mcc200/auto_update.c or board/esd/common/auto_update.c), and
updates over TFTP (see doc/README.update).

In my opinion, the code described in doc/README.update is probably the
most advanced and powerful, as it moves the description of the update
files from the code into the download image (using FIT image format).

So instead of adding yet another highly specific implementation for a
pretty common task, I'd much rather see patches that come up  with  a
generalization of this feature, i. e. something that allows reuse for
other systems, eventually even replacing (parts of) the old, existing
code.


Now for your patch:

* The Signed-off-by line is missing.
* You exceed the maximum line length in many places.
* You use an incorrect brace style.
* There is lots of trailing white space.
* It is not configurable (i.e. cannot be removed for boards that don't
  need it / don't want it.
* And it has some pretty strange parts, like this error "handling":

> error_exit:
> 	udelay(1000*1000*5);
> 	return 1;

I'm sorry, but this patch would  need  a  major  rework  to  make  it
acceptable  for  mainline, but as mentioned above, such efforts would
(in my opinion) be better invested in designing  and  implementing  a
more general solution.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Our management frequently gets lost in thought.   That's because it's
unfamiliar territory.


More information about the U-Boot mailing list