Hi Joe, > Hi Lukasz, > > On Thu, Jul 16, 2015 at 2:59 PM, Lukasz Majewski > <l.majew...@majess.pl> wrote: > > Hi Joe, > > > > Thank you for your review. > > > >> Hi Lukasz, > >> > >> On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski > >> <l.majew...@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.majew...@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@1 { > >> > + description = "U-Boot binary"; > >> > + > >> > +should look like > >> > + > >> > + images { > >> > + u-boot.bin@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@lists.denx.de > >> > http://lists.denx.de/mailman/listinfo/u-boot > > > > Best regards, > > > > Lukasz Majewski Best regards, Lukasz Majewski
pgpfgaBIKVxuZ.pgp
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot