[U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU

[Lukasz Majewski <l.majewski@majess.pl>]

Hi Joe,

> Hi Lukasz,
> 
> On Thu, Jul 16, 2015 at 2:59 PM, Lukasz Majewski
> <l.majewski@majess.pl> wrote:
> > Hi Joe,
> >
> > Thank you for your review.
> >
> >> Hi Lukasz,
> >>
> >> On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski
> >> <l.majewski@majess.pl> wrote:
> >> > Documentation file for DFU extension. With this functionality it
> >> > is now possible to transfer FIT images with firmware updates via
> >> > TFTP and use DFU backend for storing them.
> >> >
> >> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> >> > ---
> >> >  doc/README.dfutftp | 153
> >> > +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file
> >> > changed, 153 insertions(+) create mode 100644 doc/README.dfutftp
> >> >
> >> > diff --git a/doc/README.dfutftp b/doc/README.dfutftp
> >> > new file mode 100644
> >> > index 0000000..4636321
> >> > --- /dev/null
> >> > +++ b/doc/README.dfutftp
> >> > @@ -0,0 +1,153 @@
> >> > +Device Firmware Upgrade (DFU) - extension to use TFTP
> >> > +=====================================================
> >> > +
> >> > +Why?
> >> > +----
> >> > +
> >> > +* Update TFTP (CONFIG_UPDATE_TFTP) only supports writing
> >> > +code to NAND memory via TFTP.
> >> > +* DFU supports writing data to variety of mediums (NAND,
> >> > +eMMC, SD, partitions, RAM, etc) via USB.
> >> > +
> >> > +Combination of both solves their shortcomings!
> >> > +
> >> > +
> >> > +Overview
> >> > +--------
> >> > +
> >> > +This document briefly describes how to use BF for
> >>
> >> BF?
> >
> > It should be DFU.
> >
> >>
> >> > +upgrading firmware (e.g. kernel, u-boot, rootfs, etc.)
> >> > +via TFTP protocol.
> >> > +
> >> > +By using Ethernet (TFTP protocol to be precise) it was
> >> > +possible to overcome the major problem of USB based DFU -
> >> > +the relatively low transfer speed for large files.
> >> > +This was caused by DFU standard, which imposed utilization
> >> > +of only EP0 for transfer. By using Ethernet we can circumvent
> >> > +this shortcoming.
> >> > +
> >> > +Beagle Bone Black (BBB) powered by TI's am335x CPU has been used
> >> > +as a demo board.
> >> > +
> >> > +To utilize this feature, one needs to first enable support
> >> > +for USB based DFU (CONFIG_DFU_*) and TFTP update
> >> > +(CONFIG_UPDATE_TFTP) described in ./doc/README.update.
> >>
> >> Does it really make sense to reuse this UPDATE_TFTP config? Why not
> >> DFU_TFTP?
> >
> > Enabling CONFIG_UPDATE_TFTP allows reusing parts of the legacy
> > update_tftp() code.
> 
> I can understand reusing the code, but that doesn't mean we should
> force both complete features to be included.

The legacy ./common/update.c was using two flags - namely
CONFIG_UPDATE_TFTP (to allow compilation of this file) and
CONFIG_SYS_NO_FLASH. Lack of the latter was using #error to terminate a
compilation.

That was the old behavior - one needed to define both flags to compile
in the legacy code.

Moreover the CONFIG_UPDATE_TFTP is used to enable running
update_tftp() in the early boot stage.

My changes in the ./common/update.c file have split the code to common
one (enabled by CONFIG_UPDATE_TFTP) and legacy one (NAND support
specific) enabled by CONFIG_SYS_NO_FLASH. 

> 
> > What I mean is that one should enable legacy CONFIG_UPDATE_TFTP as a
> > prerequisite for using dfu tftp transfer.
> 
> What if a new config like DFU_TFTP enabled the shared code, but not
> the old behaviors.

DFU_TFTP enables dfu specific code - not the shared code
(at ./common/update.c).

Shared code is enabled by CONFIG_UPDATE_TFTP.

> 
> >> > +New "dfutftp" command has been introduced to comply with present
> >> > +USB related commands' syntax.
> >> > +
> >> > +This code works without "fitupd" command enabled.
> >> > +
> >> > +As of this writing
> >> > (SHA1:241746e618fa725491e9ff689710c5f73ffd9139) the +update.c
> >> > code is not enabled (CONFIG_UPDATE_TFTP) by any board in the
> >> > +contemporary u-boot tree.
> >>
> >> So maybe we should remove it.
> >
> > This is IMHO a tricky issue. On the one hand there hasn't left any
> > board supporting this feature in mainline (recently some old PPC
> > boards have been removed from u-boot).
> >
> > One the other hand _probably_ there are deployed systems (as a
> > derivative of the boards supported in u-boot and using this feature)
> > which depend on this feature.
> 
> That's fair.
> 
> > I'd opt for leaving the original code in the tree with a fat big
> > note about the plan to remove the legacy code in a near future (as
> > we do it with MAKEALL script).
> 
> OK
> 
> >> > +
> >> > +Environment variables
> >> > +---------------------
> >> > +
> >> > +To preserve legacy behavior of TFTP update (./common/update.c)
> >> > +code following new environment variables had been introduced:
> >>
> >> Another example of why you should use a new config instead of the
> >> existing one.
> >
> > Could you be more specific here?
> 
> You are adding magic env vars here because you want to have both old
> and new behaviors at once. I think it would be better to select one at
> build time and not add any magic.

In theory you are right. In practice it is a bit more complicated - I
will try to explain it latter in this e-mail.

> 
> >> > +* "update_tftp_exec_at_boot" ["true"|"false"]
> >> > +
> >> > +  New usage of update_tftp allows setting the
> >> > +  "update_tftp_exec_at_boot" env variable to allow it running
> >> > +  at boot time.
> >> > +  In this way update_tftp will not be executed at startup and
> >> > reduce
> >> > +  boot time.
> >> > +
> >> > +  To preserve backward compatibility for legacy boards this
> >> > variable
> >> > +  should be set to "true".
> >> > +
> >> > +  BBB note:
> >> > +           When update tftp is not working after boot one need
> >> > to
> >> > +           increase values of following two configs:
> >> > +           CONFIG_UPDATE_TFTP_MSEC_MAX and
> >> > CONFIG_UPDATE_TFTP_CNT_MAX.
> >> > +           Values of namely 1000 and 5 solve the issue for BBB.
> >> > +
> >> > +* "update_tftp_dfu" ["true"|"false"]
> >> > +
> >> > +  By "update_tftp_dfu" env variable we inform update_tftp that
> >> > +  it should use dfu for writing data - in this way support for
> >> > +  legacy usage is preserved.
> >>
> >> Same here... presumably a user only wants support for one or the
> >> other compiled in. Please use a different config.
> >
> > The most appealing thing about update.c is that I had to add only a
> > few lines of code to use it for my purpose. For that reason I've
> > decided to keep as much as possible of the legacy code.
> >
> >>
> >> > +* "update_tftp_dfu_interface" ["mmc"]
> >> > +* "update_tftp_dfu_devstring" ["1"]
> >> > +
> >> > +  Both variables - namely
> >> > "update_tftp_dfu_{interface|devstring}" are
> >> > +  only taken into account when "update_tftp_dfu" is defined.
> >> > +  They store information about interface (e.g. "mmc") and device
> >> > number
> >> > +  string (e.g. "1").
> >>
> >> It is preferable to make these parameters to a command and not
> >> magic env variables.
> >
> > They can be specified in the 'dfutftp' command - which syntax is
> > similar to 'dfu' (e.g. dfutftp 0 mmc 1).
> 
> OK, great.
> 
> > However, those variables are needed for automatic update after power
> > up. In other words update_tftp() function is called very early in
> > boot and env variables are a convenient way to specify the
> > interface and its number.
> 
> So this is only called early in the case of the old functionality.
> Your new DFU features do not need an early automatic feature, right?

I'm afraid that they will require some kind of update in the early
boot state.

> 
> In fact, I would argue that the old method didn't need it either and
> never should have supported that. Things like this should simply be
> part of the board's preboot script so we don't need this type of magic
> stuff.

Please correct me if I'm wrong.

I should use another CONFIG_* to enable using common update_tftp()
function. In this way I would avoid calling it in the early boot code.

Instead, I should define proper "preboot" env variable with "dfu tftp 0
mmc 1" command and use it for SW updating.

> 
> >> > +  In the "dfutftp" command they are explicitly overridden.
> >> > +  It is done deliberately, since in this command we may use
> >> > arbitrary values. +
> >> > +  Default values, when available during boot, are used by
> >> > update_tftp
> >> > +  (when dfu support is enabled) to properly setup medium device
> >> > +  (e.g. mmc 1).
> >> > +
> >> > +
> >> > +
> >> > +Beagle Bone Black (BBB) setup
> >> > +-----------------------------
> >> > +
> >> > +1. Setup tftp env variables:
> >> > +   *  select desired eth device - 'ethact' variable
> >> > ["ethact=cpsw"]
> >> > +      (use "bdinfo" to check current setting)
> >> > +   *  setup "serverip" and "ipaddr" variables
> >> > +   *  set "loadaddr" as a fixed buffer where incoming data is
> >> > placed
> >> > +      ["loadaddr=0x81000000"]
> >> > +
> >> > +#########
> >> > +# BONUS #
> >> > +#########
> >> > +It is possible to use USB interface to emulate ETH connection by
> >> > setting +"ethact=usb_ether". In this way one can have very fast
> >> > DFU transfer via USB.
> >>
> >> Is thor not faster than DFU?
> >
> > Yes, it is. However, DFU is standardized (which is despite of its
> > low speed its huge advantage) - thor not.
> >
> >>
> >> It seems like DFU should support a bulk endpoint if performance is
> >> an issue, right?
> >
> > Yes, it should, but such modification would be not compliant with
> > the standard.
> 
> Who is the standards body? Can they accept suggestions for the next
> revision? Is it worth trying to improve the standard?

The last revision of DFU standard is from 2006 with Greg KH being a
notable USB committee board member :-)

I've asked him about the possibility to revise the standard but he
replied that chances are small since Linux Foundation is not part
of the USB standard committee anymore.
> 
> >>That would be more efficient than emulating Ethernet.
> >
> > To be more precise - I've combined the ability to use Ethernet with
> > DFU flashing backend.
> >
> > In this way boards only equipped with ETH can (re)use DFU code to
> > flash data (on MMC, NAND, filesystems, RAM, etc).
> 
> Yes, that's very good. I was simply talking about the "BONUS" where
> emulated Ethernet over USB is used. In that case it would be more
> efficient to actually have a raw bulk endpoint supported by DFU.

Yes, it would. Unfortunately I think that it would be very hard to
revise the DFU standard.

ETH over USB can be used on devices equipped only with USB (like
trats/trats2 devel mobile phones). In that way DFU speed would increase.

> 
> >> > +For 33MiB test image the transfer rate is 1MiB/s
> >> > +
> >> > +2. Setup update_tftp variables:
> >> > +   *  set "updatefile" - the file name to be downloaded via TFTP
> >> > (stored on
> >> > +      the HOST at e.g. /srv/tftp)
> >> > +
> >> > +3. Setup dfutftp specific variables (as explained in
> >> > "Environment Variables")
> >> > +   * "update_tftp_exec_at_boot=true"
> >> > +   * "update_tftp_dfu=true"
> >> > +
> >> > +4. Inspect "dfu" specific variables:
> >> > +   * "dfu_alt_info" - information about available DFU entities
> >> > +   * "dfu_bufsiz"   - variable to set buffer size [in bytes] -
> >> > when it is not
> >> > +                     possible to set large enough default buffer
> >> > (8 MiB @ BBB) +
> >> > +
> >> > +
> >> > +FIT image format for download
> >> > +-----------------------------
> >> > +
> >> > +To create FIT image for download one should follow the update
> >> > tftp README file +(./doc/README.update) with one notable
> >> > difference: +
> >> > +The original snippet of ./doc/uImage.FIT/update_uboot.its
> >> > +
> >> > +       images {
> >> > +               update at 1 {
> >> > +                       description = "U-Boot binary";
> >> > +
> >> > +should look like
> >> > +
> >> > +       images {
> >> > +               u-boot.bin at 1 {
> >> > +                       description = "U-Boot binary";
> >> > +
> >> > +where "u-boot.bin" is the DFU entity name to be stored.
> >> > +
> >> > +
> >> > +
> >> > +To do
> >> > +-----
> >> > +
> >> > +* Extend dfu-util command (or write new one - e.g. dfueth-util)
> >> > to support
> >> > +  TFTP based transfers
> >> > +
> >> > +* Upload support (via TFTP)
> >> > \ No newline at end of file
> >> > --
> >> > 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/2fda2091/attachment.sig>