[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Wolfgang Denk]

Dear Bartek,

in message <48DB48F5.3010204 at semihalf.com> you wrote:
> 
> > I think "auto-update" is not a good name (especially since it has a
> > different meaning than the similar sounding "autoload"0; also there is
> > a typo in "sofware".
> > 
> > But most of all - do we really need a new environment variable? What's
> > wrong with our good old "bootfile" ?
> 
> The only concern here is the interaction with bootp and dhcp commands --
> they will set the "bootfile" env. variable to the file name
> got from the server, and the next time around U-Boot will try to use
> that file name to get the update. So I'd rather stick with a separate 
> env. variable for the name of the update file.

I see. Maybe we should call the variable "updatefile" or similar,
then?

> > How could the code  be  extended  to  be  usable  for  NAND  flash  /
> > DataFlash / OneNAND / IDE storage devices as well?
> 
> Primarily, FIT specification for the update file will have to be 
> extended. Current approach is able to handle a series of <data, address> 
> pairs, where the address is enough for U-Boot to tell where the data 
> should go. If we are to handle other storage types, we need to specify 
> which storage type the data should go to, and also provide a 
> type-specific location. This is somewhat akin to the way we access and 
> boot from these storage types: we use type-specific commands (nand, 
> nboot, ide, diskboot, etc.).
> 
> Also, it would be of course nice to have a framework within U-Boot for 
> generic data copying between storage types, that would hide all the 
> type-specific details and provide uniform interface.

Agreed. You want devices and a mount command, I think ;-)


Is anybody on the list volunteering to check what could be lifted
from the V2 code?

> >> @@ -290,6 +293,10 @@ void main_loop (void)
> >>  	char bcs_set[16];
> >>  #endif /* CONFIG_BOOTCOUNT_LIMIT */
> >>  
> >> +#if defined(CONFIG_AU_TFTP)
> >> +	au_tftp ();
> >> +#endif /* CONFIG_AU_TFTP */
> >> +
> >>  #if defined(CONFIG_VFD) && defined(VFD_TEST_LOGO)
> >>  	ulong bmp = 0;		/* default bitmap */
> >>  	extern int trab_vfd (ulong bitmap);
> > 
> > You definitely don't want to add  the  function  call  right  in  the
> > middle of variable declarations, or do you?
> 
> The idea is to have au_tftp() called as early as possible, before any
> other functions in main_loop().

Yes, but this is C, not C++, so declarations go only at the  bginning
of  the  function, then follows code (and no more declarations unless
you open a new block).

> So if we move the call to au_tftp() someplace below, then when
> both CONFIG_VFD and VFD_TEST_LOGO are defined, we'll have a call to
> trab_vfd(), which will happen before the software update.

So you will either have to add some  more  #ifdef's,  or  think  what
could  happen  if the VFD (Vacuum Fluorescent Display) initialization
code runs first - I would not expect any negative impact?

> Note that there are several other cases in the main_loop() where
> conditionally included code introduces declarations intermixed with
> instructions. If this issue is to be cleaned-up, then let's have it done
> as a separate patch.

Are there? I don't see any below these lines:

 293 #if defined(CONFIG_VFD) && defined(VFD_TEST_LOGO)
 294         ulong bmp = 0;          /* default bitmap */
 295         extern int trab_vfd (ulong bitmap);
 296
 297 #ifdef CONFIG_MODEM_SUPPORT


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
Ever try. Ever fail. No matter. Try again. Fail again.  Fail  better.
                                                        -- S. Beckett