[U-Boot] [PATCH 6/8] update: tftp: dfu: Extend update_tftp() function to support DFU

From: Lukasz Majewski <l.majewski@majess.pl>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 6/8] update: tftp: dfu: Extend update_tftp() function to support DFU
Date: Mon, 20 Jul 2015 21:05:15 +0200	[thread overview]
Message-ID: <20150720210515.7a4de701@jawa> (raw)
In-Reply-To: <CANr=Z=ahxot4bxNJ8TJBfXuoN7sOBGGLz+9eky+6G=oJm2LrTA@mail.gmail.com>

On Fri, 17 Jul 2015 14:38:20 -0500
Joe Hershberger <joe.hershberger@gmail.com> wrote:

> Hi Lukasz,
> 
> On Thu, Jul 16, 2015 at 3:11 PM, Lukasz Majewski
> <l.majewski@majess.pl> wrote:
> > Hi Joe,
> >
> >> Hi Lukasz,
> >>
> >> On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski
> >> <l.majewski@majess.pl> wrote:
> >> > This code allows using DFU defined mediums for storing data
> >> > received via TFTP protocol.
> >> >
> >> > It reuses legacy code at common/update.c.
> >> >
> >> > To run update_tftp() during boot one needs to define
> >> > "update_tftp_exec_at_boot=true".
> >> >
> >> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> >> > ---
> >> >  common/update.c | 37 ++++++++++++++++++++++++++++---------
> >> >  1 file changed, 28 insertions(+), 9 deletions(-)
> >> >
> >> > diff --git a/common/update.c b/common/update.c
> >> > index 75c6d62..f3ed036 100644
> >> > --- a/common/update.c
> >> > +++ b/common/update.c
> >> > @@ -18,6 +18,7 @@
> >> >  #include <net.h>
> >> >  #include <tftp.h>
> >> >  #include <malloc.h>
> >> > +#include <dfu.h>
> >> >
> >> >  /* env variable holding the location of the update file */
> >> >  #define UPDATE_FILE_ENV                "updatefile"
> >> > @@ -224,11 +225,18 @@ static int update_fit_getparams(const void
> >> > *fit, int noffset, ulong *addr,
> >> >
> >> >  int update_tftp(ulong addr)
> >> >  {
> >> > -       char *filename, *env_addr;
> >> > -       int images_noffset, ndepth, noffset;
> >> > +       char *filename, *env_addr, *fit_image_name;
> >> >         ulong update_addr, update_fladdr, update_size;
> >> > -       void *fit;
> >> > +       int images_noffset, ndepth, noffset;
> >> > +       bool update_tftp_dfu = false;
> >> >         int ret = 0;
> >> > +       void *fit;
> >> > +
> >> > +       if (!getenv("update_tftp_exec_at_boot"))
> >> > +               return 0;
> >> > +
> >> > +       if (getenv("update_tftp_dfu"))
> >> > +               update_tftp_dfu = true;
> >>
> >> As I mentioned in the documentation patch, it would be nice to
> >> split these out and drop the env vars.
> >
> > This would be difficult since update_tftp() is used at dfutftp() and
> > legacy fitupd command. Moreover it is called in the early stage of
> > booting to perform auto firmware upgrade.
> 
> So you are using it for the dfu stuff too. How early does it need to
> be? Maybe preboot? That's what I do on my products. Works great for
> me.
> 
> I'd really prefer not to proliferate this practice.
> 

Ok, I see.

> > Those env variables give some control over its behavior.
> >
> >>
> >> >
> >> >         /* use already present image */
> >> >         if (addr)
> >> > @@ -277,8 +285,8 @@ got_update_file:
> >> >                 if (ndepth != 1)
> >> >                         goto next_node;
> >> >
> >> > -               printf("Processing update '%s' :",
> >> > -                       fit_get_name(fit, noffset, NULL));
> >> > +               fit_image_name = (char *)fit_get_name(fit,
> >> > noffset, NULL);
> >> > +               printf("Processing update '%s' :",
> >> > fit_image_name);
> >> >
> >> >                 if (!fit_image_verify(fit, noffset)) {
> >> >                         printf("Error: invalid update hash,
> >> > aborting\n"); @@ -294,10 +302,21 @@ got_update_file:
> >> >                         ret = 1;
> >> >                         goto next_node;
> >> >                 }
> >> > -               if (update_flash(update_addr, update_fladdr,
> >> > update_size)) {
> >> > -                       printf("Error: can't flash update,
> >> > aborting\n");
> >> > -                       ret = 1;
> >> > -                       goto next_node;
> >> > +
> >> > +               if (!update_tftp_dfu) {
> >> > +                       if (update_flash(update_addr,
> >> > update_fladdr,
> >> > +                                        update_size)) {
> >> > +                               printf("Error: can't flash
> >> > update, aborting\n");
> >> > +                               ret = 1;
> >> > +                               goto next_node;
> >> > +                       }
> >> > +               } else if (fit_image_check_type(fit, noffset,
> >> > +
> >> > IH_TYPE_FIRMWARE)) {
> >> > +                       if (dfu_tftp_write(fit_image_name,
> >> > +                                          update_addr,
> >> > update_size)) {
> >> > +                               ret = 1;
> >> > +                               goto next_node;
> >> > +                       }
> >> >                 }
> >> >  next_node:
> >> >                 noffset = fdt_next_node(fit, noffset, &ndepth);
> >> > --
> >> > 2.1.4
> >> >
> >> > _______________________________________________
> >> > U-Boot mailing list
> >> > U-Boot at lists.denx.de
> >> > http://lists.denx.de/mailman/listinfo/u-boot
> >
> > Best regards,
> > Lukasz Majewski

Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150720/4e930ef2/attachment.sig>