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

Wolfgang Denk
2008-09-25T08:55:45Z

Dear Bartek,

in message <48DB48F5.3010204@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