[U-Boot] [PATCH 0/8] dfu: tftp: update: Support for DFU upgrades via ETH (TFTP)
This commit series enables DFU subsystem to use ETH and TFTP protocol as a medium for downloading data, which should bring substantial speedup for writing large files (like rootfs).
Please read provided ./doc/README.dfutftp documentation entry for more information.
Lukasz Majewski (8): doc: dfu: tftp: README entry for TFTP extension of DFU net: tftp: Move tftp.h file from ./net to ./include tftp: update: Allow some parts of the code to be reused when CONFIG_SYS_NO_FLASH is set dfu: tftp: update: Provide tftp support for the DFU subsystem dfu: tftp: update: Add dfu_write_from_mem_addr() function update: tftp: dfu: Extend update_tftp() function to support DFU dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP config: bbb: Configs necessary for running update via TFTP on Beagle Bone Black
common/Makefile | 1 + common/cmd_dfutftp.c | 43 ++++++++++++ common/update.c | 54 ++++++++++----- doc/README.dfutftp | 153 +++++++++++++++++++++++++++++++++++++++++++ drivers/dfu/Makefile | 1 + drivers/dfu/dfu.c | 48 ++++++++++++++ drivers/dfu/dfu_tftp.c | 76 +++++++++++++++++++++ include/configs/am335x_evm.h | 10 +++ include/dfu.h | 12 ++++ include/tftp.h | 30 +++++++++ net/bootp.c | 2 +- net/net.c | 2 +- net/rarp.c | 2 +- net/tftp.c | 2 +- net/tftp.h | 30 --------- 15 files changed, 414 insertions(+), 52 deletions(-) create mode 100644 common/cmd_dfutftp.c create mode 100644 doc/README.dfutftp create mode 100644 drivers/dfu/dfu_tftp.c create mode 100644 include/tftp.h delete mode 100644 net/tftp.h
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 +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. + +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. + + +Environment variables +--------------------- + +To preserve legacy behavior of TFTP update (./common/update.c) +code following new environment variables had been introduced: + +* "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. + +* "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"). + + 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. + +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
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?
+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?
+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.
+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.
+* "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 toincrease 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.
+* "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.
- 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?
It seems like DFU should support a bulk endpoint if performance is an issue, right? That would be more efficient than emulating Ethernet.
+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
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.
What I mean is that one should enable legacy CONFIG_UPDATE_TFTP as a prerequisite for using dfu tftp transfer.
+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.
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).
+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?
+* "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 toincrease values of following two configs:CONFIG_UPDATE_TFTP_MSEC_MAX andCONFIG_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).
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.
- 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.
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).
+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
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.
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.
+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.
+* "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 toincrease values of following two configs:CONFIG_UPDATE_TFTP_MSEC_MAX andCONFIG_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?
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.
- 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?
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.
+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
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 needto
increase values of following two configs:CONFIG_UPDATE_TFTP_MSEC_MAX andCONFIG_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
Hi Lukasz,
On Mon, Jul 20, 2015 at 1:59 PM, Lukasz Majewski l.majewski@majess.pl wrote:
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.
It seems the CONFIG_SYS_NO_FLASH is simply making sure that some other required feature is included. It wasn't actually selecting this feature.
The CONFIG_UPDATE_TFTP was actually enabling the feature. It seems that should remain the case.
That was the old behavior - one needed to define both flags to compile in the legacy code.
You actually were required to have *not* defined CONFIG_SYS_NO_FLASH.
Moreover the CONFIG_UPDATE_TFTP is used to enable running update_tftp() in the early boot stage.
Yes... That could probably be left as is for now.
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.
I think you need to make the common code be enabled by seeing either CONFIG_UPDATE_TFTP or CONFIG_DFU_TFTP.
You also need to have a check that fails if CONFIG_UPDATE_TFTP && CONFIG_SYS_NO_FLASH. That preserves old behavior.
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).
It should also enable the shared code.
Shared code is enabled by CONFIG_UPDATE_TFTP.
It should enable the shared code as well as the legacy-specific behavior.
+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 needto
increase values of following two configs:CONFIG_UPDATE_TFTP_MSEC_MAX andCONFIG_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.
You should not need more than already existed for legacy, I think. So you should not be adding any magic env vars.
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.
Explained above.
Instead, I should define proper "preboot" env variable with "dfu tftp 0 mmc 1" command and use it for SW updating.
Correct. The preboot command should call normal commands and pass parameters.
- 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's a shame. Oh well.
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.
Sure. Sounds good.
+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
On Mon, Jul 20, 2015 at 9:17 PM, Joe Hershberger wrote:
On Mon, Jul 20, 2015 at 1:59 PM, Lukasz Majewski wrote:
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's a shame. Oh well.
As a dfu-util maintainer I should not encourage breaking the standard :) but it happened that ST made their own twist to the standard ("DfuSe") for their own purposes. I added support for this to dfu-util instead of making a new separate tool since much code could be shared, and I still think that made sense. A good part of dfu-util usage today is on DfuSe devices, and the added exposure and contributions are helpful for non-DfuSe devices also. So if not too intrusive changes are needed and there are people willing to code and test I would be happy to accept it in dfu-util.
For background, the use of only control endpoints in the DFU standard was there to keep everything as simple as possible and allow implementations on the simplest of microcontrollers. Nowadays more advanced microcontroller devices often use for instance USB mass storage emulation for firmware updates, wanting to avoid any special tools or drivers on the host, however this approach also has issues, especially between various host platforms.
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.
Sure. Sounds good.
This is a nice workaround, although I don't know how easy it is to use for common users, and if it gives much speed penalty compared to raw bulk endpoints. Whether it makes sense to develop the latter also depends on how interesting updates over USB (from a host computer) will be for the uboot userbase or if e.g. pluggable mass storage devices (and the device as USB host) will be more popular.
Regards, Tormod
On Mon, 20 Jul 2015 14:17:45 -0500 Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Lukasz,
On Mon, Jul 20, 2015 at 1:59 PM, Lukasz Majewski l.majewski@majess.pl wrote:
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.
It seems the CONFIG_SYS_NO_FLASH is simply making sure that some other required feature is included. It wasn't actually selecting this feature.
The CONFIG_UPDATE_TFTP was actually enabling the feature. It seems that should remain the case.
That was the old behavior - one needed to define both flags to compile in the legacy code.
You actually were required to have *not* defined CONFIG_SYS_NO_FLASH.
Moreover the CONFIG_UPDATE_TFTP is used to enable running update_tftp() in the early boot stage.
Yes... That could probably be left as is for now.
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.
I think you need to make the common code be enabled by seeing either CONFIG_UPDATE_TFTP or CONFIG_DFU_TFTP.
You also need to have a check that fails if CONFIG_UPDATE_TFTP && CONFIG_SYS_NO_FLASH. That preserves old behavior.
Ok. Sounds good.
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).
It should also enable the shared code.
Shared code is enabled by CONFIG_UPDATE_TFTP.
It should enable the shared code as well as the legacy-specific behavior.
+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 oneneed to
increase values of following two configs:CONFIG_UPDATE_TFTP_MSEC_MAX andCONFIG_UPDATE_TFTP_CNT_MAX.
Values of namely 1000 and 5 solve the issue forBBB. + +* "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.
You should not need more than already existed for legacy, I think. So you should not be adding any magic env vars.
I will try to do away with them.
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.
Explained above.
Instead, I should define proper "preboot" env variable with "dfu tftp 0 mmc 1" command and use it for SW updating.
Correct. The preboot command should call normal commands and pass parameters.
I will utilize preboot for early command execution.
- 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's a shame. Oh well.
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.
Sure. Sounds good.
+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 defaultbuffer (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
Best regards, Lukasz Majewski
This change gives the ability to reuse the <tftp.h> header file by other subsystems (like e.g. dfu).
Without this change compilation error emerges for the legacy update.c file.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl --- common/update.c | 2 +- include/tftp.h | 30 ++++++++++++++++++++++++++++++ net/bootp.c | 2 +- net/net.c | 2 +- net/rarp.c | 2 +- net/tftp.c | 2 +- net/tftp.h | 30 ------------------------------ 7 files changed, 35 insertions(+), 35 deletions(-) create mode 100644 include/tftp.h delete mode 100644 net/tftp.h
diff --git a/common/update.c b/common/update.c index 1c6aa18..7bd7e94 100644 --- a/common/update.c +++ b/common/update.c @@ -20,7 +20,7 @@ #include <command.h> #include <flash.h> #include <net.h> -#include <net/tftp.h> +#include <tftp.h> #include <malloc.h>
/* env variable holding the location of the update file */ diff --git a/include/tftp.h b/include/tftp.h new file mode 100644 index 0000000..c411c9b --- /dev/null +++ b/include/tftp.h @@ -0,0 +1,30 @@ +/* + * LiMon - BOOTP/TFTP. + * + * Copyright 1994, 1995, 2000 Neil Russell. + * Copyright 2011 Comelit Group SpA + * Luca Ceresoli luca.ceresoli@comelit.it + * (See License) + */ + +#ifndef __TFTP_H__ +#define __TFTP_H__ + +/**********************************************************************/ +/* + * Global functions and variables. + */ + +/* tftp.c */ +void tftp_start(enum proto_t protocol); /* Begin TFTP get/put */ + +#ifdef CONFIG_CMD_TFTPSRV +void tftp_start_server(void); /* Wait for incoming TFTP put */ +#endif + +extern ulong tftp_timeout_ms; +extern int tftp_timeout_count_max; + +/**********************************************************************/ + +#endif /* __TFTP_H__ */ diff --git a/net/bootp.c b/net/bootp.c index 43466af..d325cd0 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -11,8 +11,8 @@ #include <common.h> #include <command.h> #include <net.h> +#include <tftp.h> #include "bootp.h" -#include "tftp.h" #include "nfs.h" #ifdef CONFIG_STATUS_LED #include <status_led.h> diff --git a/net/net.c b/net/net.c index 67e0ad2..8460a51 100644 --- a/net/net.c +++ b/net/net.c @@ -86,6 +86,7 @@ #include <environment.h> #include <errno.h> #include <net.h> +#include <tftp.h> #if defined(CONFIG_STATUS_LED) #include <miiphy.h> #include <status_led.h> @@ -105,7 +106,6 @@ #if defined(CONFIG_CMD_SNTP) #include "sntp.h" #endif -#include "tftp.h"
DECLARE_GLOBAL_DATA_PTR;
diff --git a/net/rarp.c b/net/rarp.c index 4ce2f37..2693d4b 100644 --- a/net/rarp.c +++ b/net/rarp.c @@ -8,10 +8,10 @@ #include <common.h> #include <command.h> #include <net.h> +#include <tftp.h> #include "nfs.h" #include "bootp.h" #include "rarp.h" -#include "tftp.h"
#define TIMEOUT 5000UL /* Milliseconds before trying BOOTP again */ #ifndef CONFIG_NET_RETRY_COUNT diff --git a/net/tftp.c b/net/tftp.c index 3e99e73..f95f737 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -10,7 +10,7 @@ #include <command.h> #include <mapmem.h> #include <net.h> -#include "tftp.h" +#include <tftp.h> #include "bootp.h" #ifdef CONFIG_SYS_DIRECT_FLASH_TFTP #include <flash.h> diff --git a/net/tftp.h b/net/tftp.h deleted file mode 100644 index c411c9b..0000000 --- a/net/tftp.h +++ /dev/null @@ -1,30 +0,0 @@ -/* - * LiMon - BOOTP/TFTP. - * - * Copyright 1994, 1995, 2000 Neil Russell. - * Copyright 2011 Comelit Group SpA - * Luca Ceresoli luca.ceresoli@comelit.it - * (See License) - */ - -#ifndef __TFTP_H__ -#define __TFTP_H__ - -/**********************************************************************/ -/* - * Global functions and variables. - */ - -/* tftp.c */ -void tftp_start(enum proto_t protocol); /* Begin TFTP get/put */ - -#ifdef CONFIG_CMD_TFTPSRV -void tftp_start_server(void); /* Wait for incoming TFTP put */ -#endif - -extern ulong tftp_timeout_ms; -extern int tftp_timeout_count_max; - -/**********************************************************************/ - -#endif /* __TFTP_H__ */
Hi Lukasz,
On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski l.majewski@majess.pl wrote:
This change gives the ability to reuse the <tftp.h> header file by other subsystems (like e.g. dfu).
Without this change compilation error emerges for the legacy update.c file.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
common/update.c | 2 +- include/tftp.h | 30 ++++++++++++++++++++++++++++++
I'd like to start moving net headers to include/net/*... lets start here.
net/bootp.c | 2 +- net/net.c | 2 +- net/rarp.c | 2 +- net/tftp.c | 2 +- net/tftp.h | 30 ------------------------------ 7 files changed, 35 insertions(+), 35 deletions(-) create mode 100644 include/tftp.h delete mode 100644 net/tftp.h
diff --git a/common/update.c b/common/update.c index 1c6aa18..7bd7e94 100644 --- a/common/update.c +++ b/common/update.c @@ -20,7 +20,7 @@ #include <command.h> #include <flash.h> #include <net.h> -#include <net/tftp.h> +#include <tftp.h> #include <malloc.h>
/* env variable holding the location of the update file */ diff --git a/include/tftp.h b/include/tftp.h new file mode 100644 index 0000000..c411c9b --- /dev/null +++ b/include/tftp.h @@ -0,0 +1,30 @@ +/*
LiMon - BOOTP/TFTP.
Copyright 1994, 1995, 2000 Neil Russell.
Copyright 2011 Comelit Group SpA
Luca Ceresoli <luca.ceresoli@comelit.it>
(See License)- */
+#ifndef __TFTP_H__ +#define __TFTP_H__
+/**********************************************************************/ +/*
Global functions and variables.- */
+/* tftp.c */ +void tftp_start(enum proto_t protocol); /* Begin TFTP get/put */
+#ifdef CONFIG_CMD_TFTPSRV +void tftp_start_server(void); /* Wait for incoming TFTP put */ +#endif
+extern ulong tftp_timeout_ms; +extern int tftp_timeout_count_max;
+/**********************************************************************/
+#endif /* __TFTP_H__ */ diff --git a/net/bootp.c b/net/bootp.c index 43466af..d325cd0 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -11,8 +11,8 @@ #include <common.h> #include <command.h> #include <net.h> +#include <tftp.h> #include "bootp.h" -#include "tftp.h" #include "nfs.h" #ifdef CONFIG_STATUS_LED #include <status_led.h> diff --git a/net/net.c b/net/net.c index 67e0ad2..8460a51 100644 --- a/net/net.c +++ b/net/net.c @@ -86,6 +86,7 @@ #include <environment.h> #include <errno.h> #include <net.h> +#include <tftp.h> #if defined(CONFIG_STATUS_LED) #include <miiphy.h> #include <status_led.h> @@ -105,7 +106,6 @@ #if defined(CONFIG_CMD_SNTP) #include "sntp.h" #endif -#include "tftp.h"
DECLARE_GLOBAL_DATA_PTR;
diff --git a/net/rarp.c b/net/rarp.c index 4ce2f37..2693d4b 100644 --- a/net/rarp.c +++ b/net/rarp.c @@ -8,10 +8,10 @@ #include <common.h> #include <command.h> #include <net.h> +#include <tftp.h> #include "nfs.h" #include "bootp.h" #include "rarp.h" -#include "tftp.h"
#define TIMEOUT 5000UL /* Milliseconds before trying BOOTP again */ #ifndef CONFIG_NET_RETRY_COUNT diff --git a/net/tftp.c b/net/tftp.c index 3e99e73..f95f737 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -10,7 +10,7 @@ #include <command.h> #include <mapmem.h> #include <net.h> -#include "tftp.h" +#include <tftp.h> #include "bootp.h" #ifdef CONFIG_SYS_DIRECT_FLASH_TFTP #include <flash.h> diff --git a/net/tftp.h b/net/tftp.h deleted file mode 100644 index c411c9b..0000000 --- a/net/tftp.h +++ /dev/null @@ -1,30 +0,0 @@ -/*
LiMon - BOOTP/TFTP.
Copyright 1994, 1995, 2000 Neil Russell.
Copyright 2011 Comelit Group SpA
Luca Ceresoli <luca.ceresoli@comelit.it>
(See License)- */
-#ifndef __TFTP_H__ -#define __TFTP_H__
-/**********************************************************************/ -/*
Global functions and variables.- */
-/* tftp.c */ -void tftp_start(enum proto_t protocol); /* Begin TFTP get/put */
-#ifdef CONFIG_CMD_TFTPSRV -void tftp_start_server(void); /* Wait for incoming TFTP put */ -#endif
-extern ulong tftp_timeout_ms; -extern int tftp_timeout_count_max;
-/**********************************************************************/
-#endif /* __TFTP_H__ */
2.1.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Joe,
Hi Lukasz,
On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski l.majewski@majess.pl wrote:
This change gives the ability to reuse the <tftp.h> header file by other subsystems (like e.g. dfu).
Without this change compilation error emerges for the legacy update.c file.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
common/update.c | 2 +- include/tftp.h | 30 ++++++++++++++++++++++++++++++
I'd like to start moving net headers to include/net/*... lets start here.
Ok, no problem.
net/bootp.c | 2 +- net/net.c | 2 +- net/rarp.c | 2 +- net/tftp.c | 2 +- net/tftp.h | 30 ------------------------------ 7 files changed, 35 insertions(+), 35 deletions(-) create mode 100644 include/tftp.h delete mode 100644 net/tftp.h
diff --git a/common/update.c b/common/update.c index 1c6aa18..7bd7e94 100644 --- a/common/update.c +++ b/common/update.c @@ -20,7 +20,7 @@ #include <command.h> #include <flash.h> #include <net.h> -#include <net/tftp.h> +#include <tftp.h> #include <malloc.h>
/* env variable holding the location of the update file */ diff --git a/include/tftp.h b/include/tftp.h new file mode 100644 index 0000000..c411c9b --- /dev/null +++ b/include/tftp.h @@ -0,0 +1,30 @@ +/*
LiMon - BOOTP/TFTP.
Copyright 1994, 1995, 2000 Neil Russell.
Copyright 2011 Comelit Group SpA
Luca Ceresoli <luca.ceresoli@comelit.it>
(See License)- */
+#ifndef __TFTP_H__ +#define __TFTP_H__
+/**********************************************************************/ +/*
Global functions and variables.- */
+/* tftp.c */ +void tftp_start(enum proto_t protocol); /* Begin TFTP get/put */ + +#ifdef CONFIG_CMD_TFTPSRV +void tftp_start_server(void); /* Wait for incoming TFTP put */ +#endif
+extern ulong tftp_timeout_ms; +extern int tftp_timeout_count_max;
+/**********************************************************************/
+#endif /* __TFTP_H__ */ diff --git a/net/bootp.c b/net/bootp.c index 43466af..d325cd0 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -11,8 +11,8 @@ #include <common.h> #include <command.h> #include <net.h> +#include <tftp.h> #include "bootp.h" -#include "tftp.h" #include "nfs.h" #ifdef CONFIG_STATUS_LED #include <status_led.h> diff --git a/net/net.c b/net/net.c index 67e0ad2..8460a51 100644 --- a/net/net.c +++ b/net/net.c @@ -86,6 +86,7 @@ #include <environment.h> #include <errno.h> #include <net.h> +#include <tftp.h> #if defined(CONFIG_STATUS_LED) #include <miiphy.h> #include <status_led.h> @@ -105,7 +106,6 @@ #if defined(CONFIG_CMD_SNTP) #include "sntp.h" #endif -#include "tftp.h"
DECLARE_GLOBAL_DATA_PTR;
diff --git a/net/rarp.c b/net/rarp.c index 4ce2f37..2693d4b 100644 --- a/net/rarp.c +++ b/net/rarp.c @@ -8,10 +8,10 @@ #include <common.h> #include <command.h> #include <net.h> +#include <tftp.h> #include "nfs.h" #include "bootp.h" #include "rarp.h" -#include "tftp.h"
#define TIMEOUT 5000UL /* Milliseconds before trying BOOTP again */ #ifndef CONFIG_NET_RETRY_COUNT diff --git a/net/tftp.c b/net/tftp.c index 3e99e73..f95f737 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -10,7 +10,7 @@ #include <command.h> #include <mapmem.h> #include <net.h> -#include "tftp.h" +#include <tftp.h> #include "bootp.h" #ifdef CONFIG_SYS_DIRECT_FLASH_TFTP #include <flash.h> diff --git a/net/tftp.h b/net/tftp.h deleted file mode 100644 index c411c9b..0000000 --- a/net/tftp.h +++ /dev/null @@ -1,30 +0,0 @@ -/*
LiMon - BOOTP/TFTP.
Copyright 1994, 1995, 2000 Neil Russell.
Copyright 2011 Comelit Group SpA
Luca Ceresoli <luca.ceresoli@comelit.it>
(See License)- */
-#ifndef __TFTP_H__ -#define __TFTP_H__
-/**********************************************************************/ -/*
Global functions and variables.- */
-/* tftp.c */ -void tftp_start(enum proto_t protocol); /* Begin TFTP get/put */ - -#ifdef CONFIG_CMD_TFTPSRV -void tftp_start_server(void); /* Wait for incoming TFTP put */ -#endif
-extern ulong tftp_timeout_ms; -extern int tftp_timeout_count_max;
-/**********************************************************************/
-#endif /* __TFTP_H__ */
2.1.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Lukasz Majewski
Up till now it was impossible to use code from update.c when system was not equipped with raw FLASH memory. Such behavior prevented DFU from reusing this code.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl --- common/update.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/common/update.c b/common/update.c index 7bd7e94..75c6d62 100644 --- a/common/update.c +++ b/common/update.c @@ -13,10 +13,6 @@ #error "CONFIG_FIT and CONFIG_OF_LIBFDT are required for auto-update feature" #endif
-#if defined(CONFIG_SYS_NO_FLASH) -#error "CONFIG_SYS_NO_FLASH defined, but FLASH is required for auto-update feature" -#endif - #include <command.h> #include <flash.h> #include <net.h> @@ -41,11 +37,11 @@
extern ulong tftp_timeout_ms; extern int tftp_timeout_count_max; -extern flash_info_t flash_info[]; extern ulong load_addr; - +#ifndef CONFIG_SYS_NO_FLASH +extern flash_info_t flash_info[]; static uchar *saved_prot_info; - +#endif static int update_load(char *filename, ulong msec_max, int cnt_max, ulong addr) { int size, rv; @@ -94,6 +90,7 @@ static int update_load(char *filename, ulong msec_max, int cnt_max, ulong addr) return rv; }
+#ifndef CONFIG_SYS_NO_FLASH static int update_flash_protect(int prot, ulong addr_first, ulong addr_last) { uchar *sp_info_ptr; @@ -165,9 +162,11 @@ static int update_flash_protect(int prot, ulong addr_first, ulong addr_last)
return 0; } +#endif
static int update_flash(ulong addr_source, ulong addr_first, ulong size) { +#ifndef CONFIG_SYS_NO_FLASH ulong addr_last = addr_first + size - 1;
/* round last address to the sector boundary */ @@ -203,7 +202,7 @@ static int update_flash(ulong addr_source, ulong addr_first, ulong size) printf("Error: could not protect flash sectors\n"); return 1; } - +#endif return 0; }
Hi Lukasz,
On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski l.majewski@majess.pl wrote:
Up till now it was impossible to use code from update.c when system was not equipped with raw FLASH memory. Such behavior prevented DFU from reusing this code.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
Acked-by: Joe Hershberger joe.hershberger@ni.com
This commit adds initial support for using tftp for downloading and upgrading firmware on the device.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl --- drivers/dfu/Makefile | 1 + drivers/dfu/dfu_tftp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/dfu.h | 11 ++++++++ 3 files changed, 88 insertions(+) create mode 100644 drivers/dfu/dfu_tftp.c
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index 5cc535e..43249ce 100644 --- a/drivers/dfu/Makefile +++ b/drivers/dfu/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_DFU_MMC) += dfu_mmc.o obj-$(CONFIG_DFU_NAND) += dfu_nand.o obj-$(CONFIG_DFU_RAM) += dfu_ram.o obj-$(CONFIG_DFU_SF) += dfu_sf.o +obj-$(CONFIG_DFU_TFTP) += dfu_tftp.o diff --git a/drivers/dfu/dfu_tftp.c b/drivers/dfu/dfu_tftp.c new file mode 100644 index 0000000..26539f2 --- /dev/null +++ b/drivers/dfu/dfu_tftp.c @@ -0,0 +1,76 @@ +/* + * (C) Copyright 2015 + * Lukasz Majewski l.majewski@majess.pl + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <malloc.h> +#include <errno.h> +#include <dfu.h> + +int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len) +{ + char *s, *sb, *interface, *devstring; + int alt_setting_num, ret; + struct dfu_entity *dfu; + + debug("%s: name: %s addr: 0x%x len: %d\n", __func__, dfu_entity_name, + addr, len); + + interface = getenv("update_tftp_dfu_interface"); + if (interface == NULL) { + error("TFTP DFU: 'interface' not defined\n"); + return -EINVAL; + } + + devstring = getenv("update_tftp_dfu_devstring"); + if (devstring == NULL) { + error("TFTP DFU: 'devstring' not defined\n"); + return -EINVAL; + } + + ret = dfu_init_env_entities(interface, devstring); + if (ret) + goto done; + + /* + * We need to copy name pointed by *dfu_entity_name since this text + * is the integral part of the FDT image. + * Any implicit modification (i.e. done by strsep()) will corrupt + * the FDT image and prevent other images to be stored. + */ + s = strdup(dfu_entity_name); + sb = s; + if (!s) { + ret = -ENOMEM; + goto done; + } + + strsep(&s, "@"); + debug("%s: image name: %s strlen: %d\n", __func__, sb, strlen(sb)); + + alt_setting_num = dfu_get_alt(sb); + free(sb); + if (alt_setting_num < 0) { + error("Alt setting [%d] to write not found!", + alt_setting_num); + ret = -ENODEV; + goto done; + } + + dfu = dfu_get_entity(alt_setting_num); + if (!dfu) { + error("DFU entity for alt: %d not found!", alt_setting_num); + ret = -ENODEV; + goto done; + } + + ret = dfu_write_from_mem_addr(dfu, (void *)addr, len); + +done: + dfu_free_entities(); + + return ret; +} diff --git a/include/dfu.h b/include/dfu.h index 7d31abd..adad863 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -207,5 +207,16 @@ static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, } #endif
+#ifdef CONFIG_DFU_TFTP +int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len); +#else +static inline int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, + unsigned int len) +{ + puts("TFTP write support for DFU not available!\n"); + return -1; +} +#endif + int dfu_add(struct usb_configuration *c); #endif /* __DFU_ENTITY_H_ */
Hi Lukasz,
On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski l.majewski@majess.pl wrote:
This commit adds initial support for using tftp for downloading and upgrading firmware on the device.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
drivers/dfu/Makefile | 1 + drivers/dfu/dfu_tftp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/dfu.h | 11 ++++++++ 3 files changed, 88 insertions(+) create mode 100644 drivers/dfu/dfu_tftp.c
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index 5cc535e..43249ce 100644 --- a/drivers/dfu/Makefile +++ b/drivers/dfu/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_DFU_MMC) += dfu_mmc.o obj-$(CONFIG_DFU_NAND) += dfu_nand.o obj-$(CONFIG_DFU_RAM) += dfu_ram.o obj-$(CONFIG_DFU_SF) += dfu_sf.o +obj-$(CONFIG_DFU_TFTP) += dfu_tftp.o diff --git a/drivers/dfu/dfu_tftp.c b/drivers/dfu/dfu_tftp.c new file mode 100644 index 0000000..26539f2 --- /dev/null +++ b/drivers/dfu/dfu_tftp.c @@ -0,0 +1,76 @@ +/*
- (C) Copyright 2015
- Lukasz Majewski l.majewski@majess.pl
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <malloc.h> +#include <errno.h> +#include <dfu.h>
+int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len) +{
char *s, *sb, *interface, *devstring;int alt_setting_num, ret;struct dfu_entity *dfu;debug("%s: name: %s addr: 0x%x len: %d\n", __func__, dfu_entity_name,addr, len);interface = getenv("update_tftp_dfu_interface");if (interface == NULL) {error("TFTP DFU: 'interface' not defined\n");return -EINVAL;}devstring = getenv("update_tftp_dfu_devstring");if (devstring == NULL) {error("TFTP DFU: 'devstring' not defined\n");return -EINVAL;}
It would be great if these env vars could be moved to command parameters.
ret = dfu_init_env_entities(interface, devstring);if (ret)goto done;/** We need to copy name pointed by *dfu_entity_name since this text* is the integral part of the FDT image.* Any implicit modification (i.e. done by strsep()) will corrupt* the FDT image and prevent other images to be stored.*/s = strdup(dfu_entity_name);sb = s;if (!s) {ret = -ENOMEM;goto done;}strsep(&s, "@");debug("%s: image name: %s strlen: %d\n", __func__, sb, strlen(sb));alt_setting_num = dfu_get_alt(sb);free(sb);if (alt_setting_num < 0) {error("Alt setting [%d] to write not found!",alt_setting_num);ret = -ENODEV;goto done;}dfu = dfu_get_entity(alt_setting_num);if (!dfu) {error("DFU entity for alt: %d not found!", alt_setting_num);ret = -ENODEV;goto done;}ret = dfu_write_from_mem_addr(dfu, (void *)addr, len);+done:
dfu_free_entities();return ret;+} diff --git a/include/dfu.h b/include/dfu.h index 7d31abd..adad863 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -207,5 +207,16 @@ static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, } #endif
+#ifdef CONFIG_DFU_TFTP +int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len); +#else +static inline int dfu_tftp_write(char *dfu_entity_name, unsigned int addr,
unsigned int len)+{
puts("TFTP write support for DFU not available!\n");return -1;
This should be -ENOSYS probably.
+} +#endif
int dfu_add(struct usb_configuration *c);
#endif /* __DFU_ENTITY_H_ */
2.1.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Joe,
Hi Lukasz,
On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski l.majewski@majess.pl wrote:
This commit adds initial support for using tftp for downloading and upgrading firmware on the device.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
drivers/dfu/Makefile | 1 + drivers/dfu/dfu_tftp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/dfu.h | 11 ++++++++ 3 files changed, 88 insertions(+) create mode 100644 drivers/dfu/dfu_tftp.c
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index 5cc535e..43249ce 100644 --- a/drivers/dfu/Makefile +++ b/drivers/dfu/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_DFU_MMC) += dfu_mmc.o obj-$(CONFIG_DFU_NAND) += dfu_nand.o obj-$(CONFIG_DFU_RAM) += dfu_ram.o obj-$(CONFIG_DFU_SF) += dfu_sf.o +obj-$(CONFIG_DFU_TFTP) += dfu_tftp.o diff --git a/drivers/dfu/dfu_tftp.c b/drivers/dfu/dfu_tftp.c new file mode 100644 index 0000000..26539f2 --- /dev/null +++ b/drivers/dfu/dfu_tftp.c @@ -0,0 +1,76 @@ +/*
- (C) Copyright 2015
- Lukasz Majewski l.majewski@majess.pl
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <malloc.h> +#include <errno.h> +#include <dfu.h>
+int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len) +{
char *s, *sb, *interface, *devstring;int alt_setting_num, ret;struct dfu_entity *dfu;debug("%s: name: %s addr: 0x%x len: %d\n", __func__,dfu_entity_name,
addr, len);interface = getenv("update_tftp_dfu_interface");if (interface == NULL) {error("TFTP DFU: 'interface' not defined\n");return -EINVAL;}devstring = getenv("update_tftp_dfu_devstring");if (devstring == NULL) {error("TFTP DFU: 'devstring' not defined\n");return -EINVAL;}It would be great if these env vars could be moved to command parameters.
Those parameters are necessary to perform update (via update_tftp()) during boot time.
Normally - when user call 'dfutftp' command he/she needs to specify this informaiton. (e.g. 'dfutftp 0 mmc 1').
ret = dfu_init_env_entities(interface, devstring);if (ret)goto done;/** We need to copy name pointed by *dfu_entity_name sincethis text
* is the integral part of the FDT image.* Any implicit modification (i.e. done by strsep()) willcorrupt
* the FDT image and prevent other images to be stored.*/s = strdup(dfu_entity_name);sb = s;if (!s) {ret = -ENOMEM;goto done;}strsep(&s, "@");debug("%s: image name: %s strlen: %d\n", __func__, sb,strlen(sb)); +
alt_setting_num = dfu_get_alt(sb);free(sb);if (alt_setting_num < 0) {error("Alt setting [%d] to write not found!",alt_setting_num);ret = -ENODEV;goto done;}dfu = dfu_get_entity(alt_setting_num);if (!dfu) {error("DFU entity for alt: %d not found!",alt_setting_num);
ret = -ENODEV;goto done;}ret = dfu_write_from_mem_addr(dfu, (void *)addr, len);+done:
dfu_free_entities();return ret;+} diff --git a/include/dfu.h b/include/dfu.h index 7d31abd..adad863 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -207,5 +207,16 @@ static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, } #endif
+#ifdef CONFIG_DFU_TFTP +int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len); +#else +static inline int dfu_tftp_write(char *dfu_entity_name, unsigned int addr,
unsigned int len)+{
puts("TFTP write support for DFU not available!\n");return -1;This should be -ENOSYS probably.
Good point - thanks!
+} +#endif
int dfu_add(struct usb_configuration *c);
#endif /* __DFU_ENTITY_H_ */
2.1.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Lukasz Majewski
Hi Lukasz,
On Thu, Jul 16, 2015 at 3:06 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 commit adds initial support for using tftp for downloading and upgrading firmware on the device.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
drivers/dfu/Makefile | 1 + drivers/dfu/dfu_tftp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/dfu.h | 11 ++++++++ 3 files changed, 88 insertions(+) create mode 100644 drivers/dfu/dfu_tftp.c
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index 5cc535e..43249ce 100644 --- a/drivers/dfu/Makefile +++ b/drivers/dfu/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_DFU_MMC) += dfu_mmc.o obj-$(CONFIG_DFU_NAND) += dfu_nand.o obj-$(CONFIG_DFU_RAM) += dfu_ram.o obj-$(CONFIG_DFU_SF) += dfu_sf.o +obj-$(CONFIG_DFU_TFTP) += dfu_tftp.o diff --git a/drivers/dfu/dfu_tftp.c b/drivers/dfu/dfu_tftp.c new file mode 100644 index 0000000..26539f2 --- /dev/null +++ b/drivers/dfu/dfu_tftp.c @@ -0,0 +1,76 @@ +/*
- (C) Copyright 2015
- Lukasz Majewski l.majewski@majess.pl
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <malloc.h> +#include <errno.h> +#include <dfu.h>
+int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len) +{
char *s, *sb, *interface, *devstring;int alt_setting_num, ret;struct dfu_entity *dfu;debug("%s: name: %s addr: 0x%x len: %d\n", __func__,dfu_entity_name,
addr, len);interface = getenv("update_tftp_dfu_interface");if (interface == NULL) {error("TFTP DFU: 'interface' not defined\n");return -EINVAL;}devstring = getenv("update_tftp_dfu_devstring");if (devstring == NULL) {error("TFTP DFU: 'devstring' not defined\n");return -EINVAL;}It would be great if these env vars could be moved to command parameters.
Those parameters are necessary to perform update (via update_tftp()) during boot time.
This is just the old method, right? Not the new DFU stuff.
Normally - when user call 'dfutftp' command he/she needs to specify this informaiton. (e.g. 'dfutftp 0 mmc 1').
Perfect - so specify it when you call it from preboot.
ret = dfu_init_env_entities(interface, devstring);if (ret)goto done;/** We need to copy name pointed by *dfu_entity_name sincethis text
* is the integral part of the FDT image.* Any implicit modification (i.e. done by strsep()) willcorrupt
* the FDT image and prevent other images to be stored.*/s = strdup(dfu_entity_name);sb = s;if (!s) {ret = -ENOMEM;goto done;}strsep(&s, "@");debug("%s: image name: %s strlen: %d\n", __func__, sb,strlen(sb)); +
alt_setting_num = dfu_get_alt(sb);free(sb);if (alt_setting_num < 0) {error("Alt setting [%d] to write not found!",alt_setting_num);ret = -ENODEV;goto done;}dfu = dfu_get_entity(alt_setting_num);if (!dfu) {error("DFU entity for alt: %d not found!",alt_setting_num);
ret = -ENODEV;goto done;}ret = dfu_write_from_mem_addr(dfu, (void *)addr, len);+done:
dfu_free_entities();return ret;+} diff --git a/include/dfu.h b/include/dfu.h index 7d31abd..adad863 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -207,5 +207,16 @@ static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, } #endif
+#ifdef CONFIG_DFU_TFTP +int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len); +#else +static inline int dfu_tftp_write(char *dfu_entity_name, unsigned int addr,
unsigned int len)+{
puts("TFTP write support for DFU not available!\n");return -1;This should be -ENOSYS probably.
Good point - thanks!
+} +#endif
int dfu_add(struct usb_configuration *c);
#endif /* __DFU_ENTITY_H_ */
2.1.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Lukasz Majewski
On Fri, 17 Jul 2015 14:35:39 -0500 Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Lukasz,
On Thu, Jul 16, 2015 at 3:06 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 commit adds initial support for using tftp for downloading and upgrading firmware on the device.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
drivers/dfu/Makefile | 1 + drivers/dfu/dfu_tftp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/dfu.h | 11 ++++++++ 3 files changed, 88 insertions(+) create mode 100644 drivers/dfu/dfu_tftp.c
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index 5cc535e..43249ce 100644 --- a/drivers/dfu/Makefile +++ b/drivers/dfu/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_DFU_MMC) += dfu_mmc.o obj-$(CONFIG_DFU_NAND) += dfu_nand.o obj-$(CONFIG_DFU_RAM) += dfu_ram.o obj-$(CONFIG_DFU_SF) += dfu_sf.o +obj-$(CONFIG_DFU_TFTP) += dfu_tftp.o diff --git a/drivers/dfu/dfu_tftp.c b/drivers/dfu/dfu_tftp.c new file mode 100644 index 0000000..26539f2 --- /dev/null +++ b/drivers/dfu/dfu_tftp.c @@ -0,0 +1,76 @@ +/*
- (C) Copyright 2015
- Lukasz Majewski l.majewski@majess.pl
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <malloc.h> +#include <errno.h> +#include <dfu.h>
+int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len) +{
char *s, *sb, *interface, *devstring;int alt_setting_num, ret;struct dfu_entity *dfu;debug("%s: name: %s addr: 0x%x len: %d\n", __func__,dfu_entity_name,
addr, len);interface = getenv("update_tftp_dfu_interface");if (interface == NULL) {error("TFTP DFU: 'interface' not defined\n");return -EINVAL;}devstring = getenv("update_tftp_dfu_devstring");if (devstring == NULL) {error("TFTP DFU: 'devstring' not defined\n");return -EINVAL;}It would be great if these env vars could be moved to command parameters.
Those parameters are necessary to perform update (via update_tftp()) during boot time.
This is just the old method, right? Not the new DFU stuff.
Yes, this is the part of legacy code.
Normally - when user call 'dfutftp' command he/she needs to specify this informaiton. (e.g. 'dfutftp 0 mmc 1').
Perfect - so specify it when you call it from preboot.
I think that it would be feasible to use preboot variable for early booting.
The problem is with CONFIG_UPDATE_TFTP flag. I will try to find solution for this issue.
ret = dfu_init_env_entities(interface, devstring);if (ret)goto done;/** We need to copy name pointed by *dfu_entity_name sincethis text
* is the integral part of the FDT image.* Any implicit modification (i.e. done by strsep()) willcorrupt
* the FDT image and prevent other images to be stored.*/s = strdup(dfu_entity_name);sb = s;if (!s) {ret = -ENOMEM;goto done;}strsep(&s, "@");debug("%s: image name: %s strlen: %d\n", __func__, sb,strlen(sb)); +
alt_setting_num = dfu_get_alt(sb);free(sb);if (alt_setting_num < 0) {error("Alt setting [%d] to write not found!",alt_setting_num);ret = -ENODEV;goto done;}dfu = dfu_get_entity(alt_setting_num);if (!dfu) {error("DFU entity for alt: %d not found!",alt_setting_num);
ret = -ENODEV;goto done;}ret = dfu_write_from_mem_addr(dfu, (void *)addr, len);+done:
dfu_free_entities();return ret;+} diff --git a/include/dfu.h b/include/dfu.h index 7d31abd..adad863 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -207,5 +207,16 @@ static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, } #endif
+#ifdef CONFIG_DFU_TFTP +int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len); +#else +static inline int dfu_tftp_write(char *dfu_entity_name, unsigned int addr,
unsigned int len)+{
puts("TFTP write support for DFU not available!\n");return -1;This should be -ENOSYS probably.
Good point - thanks!
+} +#endif
int dfu_add(struct usb_configuration *c);
#endif /* __DFU_ENTITY_H_ */
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
This function allows writing via DFU data stored from fixed buffer address (like e.g. loadaddr env variable).
Such predefined buffers are used in the update_tftp() code. In fact this function is a wrapper on the dfu_write() and dfu_flush().
Signed-off-by: Lukasz Majewski l.majewski@majess.pl --- drivers/dfu/dfu.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ include/dfu.h | 1 + 2 files changed, 49 insertions(+)
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 332be67..3fbbecc 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -568,3 +568,51 @@ int dfu_get_alt(char *name)
return -ENODEV; } + +/** + * dfu_write_from_mem_addr - this function adds support for writing data + * starting from fixed memory address (like $loadaddr) + * to dfu managed medium (e.g. NAND, MMC) + * + * @param dfu - dfu entity to which we want to store data + * @param buf - fixed memory addres from where data starts + * @param size - number of bytes to write + * + * @return - 0 on success, other value on failure + */ +int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size) +{ + unsigned long dfu_buf_size, write; + int i, ret = 0, left = size; + void *dp = buf; + + /* + * Here we must call dfu_get_buf(dfu) first to be sure that dfu_buf_size + * has been properly initialized - e.g. if "dfu_bufsiz" has been taken + * into account. + */ + dfu_get_buf(dfu); + dfu_buf_size = dfu_get_buf_size(); + debug("%s: dfu buf size: %lu\n", __func__, dfu_buf_size); + + for (i = 0; left > 0; i++) { + write = (dfu_buf_size < left ? dfu_buf_size : left); + + debug("%s: dp: 0x%p left: %d write: %lu\n", __func__, + dp, left, write); + ret = dfu_write(dfu, dp, write, i); + if (ret) { + error("DFU write failed\n"); + return ret; + } + + dp += write; + left -= write; + } + + ret = dfu_flush(dfu, NULL, 0, i); + if (ret) + error("DFU flush failed!"); + + return ret; +} diff --git a/include/dfu.h b/include/dfu.h index adad863..ff4db5d 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -162,6 +162,7 @@ bool dfu_usb_get_reset(void); int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num); int dfu_write(struct dfu_entity *de, void *buf, int size, int blk_seq_num); int dfu_flush(struct dfu_entity *de, void *buf, int size, int blk_seq_num); +int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size); /* Device specific */ #ifdef CONFIG_DFU_MMC extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s);
Hi Lukasz,
On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski l.majewski@majess.pl wrote:
This function allows writing via DFU data stored from fixed buffer address (like e.g. loadaddr env variable).
Such predefined buffers are used in the update_tftp() code. In fact this function is a wrapper on the dfu_write() and dfu_flush().
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
drivers/dfu/dfu.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ include/dfu.h | 1 + 2 files changed, 49 insertions(+)
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 332be67..3fbbecc 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -568,3 +568,51 @@ int dfu_get_alt(char *name)
return -ENODEV;}
+/**
- dfu_write_from_mem_addr - this function adds support for writing data
starting from fixed memory address (like $loadaddr)
to dfu managed medium (e.g. NAND, MMC)
- @param dfu - dfu entity to which we want to store data
- @param buf - fixed memory addres from where data starts
- @param size - number of bytes to write
- @return - 0 on success, other value on failure
- */
+int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size) +{
unsigned long dfu_buf_size, write;int i, ret = 0, left = size;void *dp = buf;/** Here we must call dfu_get_buf(dfu) first to be sure that dfu_buf_size* has been properly initialized - e.g. if "dfu_bufsiz" has been taken* into account.*/dfu_get_buf(dfu);dfu_buf_size = dfu_get_buf_size();debug("%s: dfu buf size: %lu\n", __func__, dfu_buf_size);for (i = 0; left > 0; i++) {write = (dfu_buf_size < left ? dfu_buf_size : left);
Please use min() here.
debug("%s: dp: 0x%p left: %d write: %lu\n", __func__,dp, left, write);ret = dfu_write(dfu, dp, write, i);if (ret) {error("DFU write failed\n");return ret;}dp += write;left -= write;}ret = dfu_flush(dfu, NULL, 0, i);if (ret)error("DFU flush failed!");return ret;+} diff --git a/include/dfu.h b/include/dfu.h index adad863..ff4db5d 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -162,6 +162,7 @@ bool dfu_usb_get_reset(void); int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num); int dfu_write(struct dfu_entity *de, void *buf, int size, int blk_seq_num); int dfu_flush(struct dfu_entity *de, void *buf, int size, int blk_seq_num); +int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size); /* Device specific */ #ifdef CONFIG_DFU_MMC extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s); -- 2.1.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Joe,
Hi Lukasz,
On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski l.majewski@majess.pl wrote:
This function allows writing via DFU data stored from fixed buffer address (like e.g. loadaddr env variable).
Such predefined buffers are used in the update_tftp() code. In fact this function is a wrapper on the dfu_write() and dfu_flush().
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
drivers/dfu/dfu.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ include/dfu.h | 1 + 2 files changed, 49 insertions(+)
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 332be67..3fbbecc 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -568,3 +568,51 @@ int dfu_get_alt(char *name)
return -ENODEV;}
+/**
- dfu_write_from_mem_addr - this function adds support for
writing data
starting from fixed memory address(like $loadaddr)
to dfu managed medium (e.g. NAND, MMC)
- @param dfu - dfu entity to which we want to store data
- @param buf - fixed memory addres from where data starts
- @param size - number of bytes to write
- @return - 0 on success, other value on failure
- */
+int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size) +{
unsigned long dfu_buf_size, write;int i, ret = 0, left = size;void *dp = buf;/** Here we must call dfu_get_buf(dfu) first to be sure thatdfu_buf_size
* has been properly initialized - e.g. if "dfu_bufsiz" hasbeen taken
* into account.*/dfu_get_buf(dfu);dfu_buf_size = dfu_get_buf_size();debug("%s: dfu buf size: %lu\n", __func__, dfu_buf_size);for (i = 0; left > 0; i++) {write = (dfu_buf_size < left ? dfu_buf_size : left);Please use min() here.
Ok.
debug("%s: dp: 0x%p left: %d write: %lu\n",__func__,
dp, left, write);ret = dfu_write(dfu, dp, write, i);if (ret) {error("DFU write failed\n");return ret;}dp += write;left -= write;}ret = dfu_flush(dfu, NULL, 0, i);if (ret)error("DFU flush failed!");return ret;+} diff --git a/include/dfu.h b/include/dfu.h index adad863..ff4db5d 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -162,6 +162,7 @@ bool dfu_usb_get_reset(void); int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num); int dfu_write(struct dfu_entity *de, void *buf, int size, int blk_seq_num); int dfu_flush(struct dfu_entity *de, void *buf, int size, int blk_seq_num); +int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size); /* Device specific */ #ifdef CONFIG_DFU_MMC extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s); -- 2.1.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Lukasz Majewski
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;
/* 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);
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.
/* 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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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.
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Lukasz Majewski
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.
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Lukasz Majewski
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 flashupdate, 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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Lukasz Majewski
Best regards, Lukasz Majewski
The new 'dfutftp' command has syntax similar to 'dfu' command. This new command however, requires some extra env variables to allow update_tftp() code to work properly. For more explanation, please consult ./doc/README.dfutftp
Signed-off-by: Lukasz Majewski l.majewski@majess.pl --- common/Makefile | 1 + common/cmd_dfutftp.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 common/cmd_dfutftp.c
diff --git a/common/Makefile b/common/Makefile index d6c1d48..483905c 100644 --- a/common/Makefile +++ b/common/Makefile @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o obj-$(CONFIG_CMD_DFU) += cmd_dfu.o obj-$(CONFIG_CMD_GPT) += cmd_gpt.o +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o
# Power obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c new file mode 100644 index 0000000..2b75a09 --- /dev/null +++ b/common/cmd_dfutftp.c @@ -0,0 +1,43 @@ +/* + * cmd_dfutftp.c -- dfutftp command + * + * Copyright (C) 2015 + * Lukasz Majewski l.majewski@majess.pl + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <net.h> + +static +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + unsigned long addr = 0; + + if (argc < 4 || argc > 5) + return CMD_RET_USAGE; + + char *interface = argv[2]; + char *devstring = argv[3]; + + if (argc == 5) + addr = simple_strtoul(argv[4], NULL, 0); + + /* Below env variables are descibed in detail at ./doc/README.dfutftp */ + setenv("update_tftp_exec_at_boot", "true"); + setenv("update_tftp_dfu", "true"); + setenv("update_tftp_dfu_interface", interface); + setenv("update_tftp_dfu_devstring", devstring); + + return update_tftp(addr); +} + +U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp, + "Device Firmware Upgrade via TFTP", + "<ETH_controller> <interface> <dev>\n" + " - device firmware upgrade via <ETH_controller>\n" + " using TFTP protocol on device <dev>, attached\n" + " to interface <interface>\n" + " [addr] - address where FIT image has been stored\n" +);
Hi Lukasz,
On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski l.majewski@majess.pl wrote:
The new 'dfutftp' command has syntax similar to 'dfu' command. This new command however, requires some extra env variables to allow update_tftp() code to work properly. For more explanation, please consult ./doc/README.dfutftp
It would be great if it didn't require them. It would also be great if there were just a dfu command and "tftp" were a subcommand. It's a more common pattern now instead of adding new, top-level commands.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
common/Makefile | 1 + common/cmd_dfutftp.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 common/cmd_dfutftp.c
diff --git a/common/Makefile b/common/Makefile index d6c1d48..483905c 100644 --- a/common/Makefile +++ b/common/Makefile @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o obj-$(CONFIG_CMD_DFU) += cmd_dfu.o obj-$(CONFIG_CMD_GPT) += cmd_gpt.o +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o
# Power obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c new file mode 100644 index 0000000..2b75a09 --- /dev/null +++ b/common/cmd_dfutftp.c @@ -0,0 +1,43 @@ +/*
- cmd_dfutftp.c -- dfutftp command
- Copyright (C) 2015
- Lukasz Majewski l.majewski@majess.pl
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <net.h>
+static +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
unsigned long addr = 0;if (argc < 4 || argc > 5)return CMD_RET_USAGE;char *interface = argv[2];char *devstring = argv[3];if (argc == 5)addr = simple_strtoul(argv[4], NULL, 0);/* Below env variables are descibed in detail at ./doc/README.dfutftp */setenv("update_tftp_exec_at_boot", "true");setenv("update_tftp_dfu", "true");setenv("update_tftp_dfu_interface", interface);setenv("update_tftp_dfu_devstring", devstring);return update_tftp(addr);+}
+U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp,
"Device Firmware Upgrade via TFTP","<ETH_controller> <interface> <dev>\n"" - device firmware upgrade via <ETH_controller>\n"" using TFTP protocol on device <dev>, attached\n"" to interface <interface>\n"" [addr] - address where FIT image has been stored\n"+);
2.1.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Joe,
Hi Lukasz,
On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski l.majewski@majess.pl wrote:
The new 'dfutftp' command has syntax similar to 'dfu' command. This new command however, requires some extra env variables to allow update_tftp() code to work properly. For more explanation, please consult ./doc/README.dfutftp
It would be great if it didn't require them.
I've described reasoning for them in the previous reply.
It would also be great if there were just a dfu command and "tftp" were a subcommand. It's a more common pattern now instead of adding new, top-level commands.
Good point. However, I've tried to avoid #ifdefs in the cmd_dfu.c code. Some boards only use USB and they would not need support for tftp in the cmd_dfu.c command.
Separate command allows adding the code only when it is needed.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
common/Makefile | 1 + common/cmd_dfutftp.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 common/cmd_dfutftp.c
diff --git a/common/Makefile b/common/Makefile index d6c1d48..483905c 100644 --- a/common/Makefile +++ b/common/Makefile @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o obj-$(CONFIG_CMD_DFU) += cmd_dfu.o obj-$(CONFIG_CMD_GPT) += cmd_gpt.o +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o
# Power obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c new file mode 100644 index 0000000..2b75a09 --- /dev/null +++ b/common/cmd_dfutftp.c @@ -0,0 +1,43 @@ +/*
- cmd_dfutftp.c -- dfutftp command
- Copyright (C) 2015
- Lukasz Majewski l.majewski@majess.pl
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <net.h>
+static +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
unsigned long addr = 0;if (argc < 4 || argc > 5)return CMD_RET_USAGE;char *interface = argv[2];char *devstring = argv[3];if (argc == 5)addr = simple_strtoul(argv[4], NULL, 0);/* Below env variables are descibed in detailat ./doc/README.dfutftp */
setenv("update_tftp_exec_at_boot", "true");setenv("update_tftp_dfu", "true");setenv("update_tftp_dfu_interface", interface);setenv("update_tftp_dfu_devstring", devstring);return update_tftp(addr);+}
+U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp,
"Device Firmware Upgrade via TFTP","<ETH_controller> <interface> <dev>\n"" - device firmware upgrade via <ETH_controller>\n"" using TFTP protocol on device <dev>, attached\n"" to interface <interface>\n"" [addr] - address where FIT image has been stored\n"+);
2.1.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Lukasz Majewski
Hi Lukasz,
On Thu, Jul 16, 2015 at 3:26 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:
The new 'dfutftp' command has syntax similar to 'dfu' command. This new command however, requires some extra env variables to allow update_tftp() code to work properly. For more explanation, please consult ./doc/README.dfutftp
It would be great if it didn't require them.
I've described reasoning for them in the previous reply.
It would also be great if there were just a dfu command and "tftp" were a subcommand. It's a more common pattern now instead of adding new, top-level commands.
Good point. However, I've tried to avoid #ifdefs in the cmd_dfu.c code. Some boards only use USB and they would not need support for tftp in the cmd_dfu.c command.
Separate command allows adding the code only when it is needed.
That's true, but it seems a simple thing to have an ifdef in the command list.
You could even keep the sub-command implementation in a separate file.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
common/Makefile | 1 + common/cmd_dfutftp.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 common/cmd_dfutftp.c
diff --git a/common/Makefile b/common/Makefile index d6c1d48..483905c 100644 --- a/common/Makefile +++ b/common/Makefile @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o obj-$(CONFIG_CMD_DFU) += cmd_dfu.o obj-$(CONFIG_CMD_GPT) += cmd_gpt.o +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o
# Power obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c new file mode 100644 index 0000000..2b75a09 --- /dev/null +++ b/common/cmd_dfutftp.c @@ -0,0 +1,43 @@ +/*
- cmd_dfutftp.c -- dfutftp command
- Copyright (C) 2015
- Lukasz Majewski l.majewski@majess.pl
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <net.h>
+static +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
unsigned long addr = 0;if (argc < 4 || argc > 5)return CMD_RET_USAGE;char *interface = argv[2];char *devstring = argv[3];if (argc == 5)addr = simple_strtoul(argv[4], NULL, 0);/* Below env variables are descibed in detailat ./doc/README.dfutftp */
setenv("update_tftp_exec_at_boot", "true");setenv("update_tftp_dfu", "true");setenv("update_tftp_dfu_interface", interface);setenv("update_tftp_dfu_devstring", devstring);return update_tftp(addr);+}
+U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp,
"Device Firmware Upgrade via TFTP","<ETH_controller> <interface> <dev>\n"" - device firmware upgrade via <ETH_controller>\n"" using TFTP protocol on device <dev>, attached\n"" to interface <interface>\n"" [addr] - address where FIT image has been stored\n"+);
2.1.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Lukasz Majewski
Hi Joe,
Hi Lukasz,
On Thu, Jul 16, 2015 at 3:26 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:
The new 'dfutftp' command has syntax similar to 'dfu' command. This new command however, requires some extra env variables to allow update_tftp() code to work properly. For more explanation, please consult ./doc/README.dfutftp
It would be great if it didn't require them.
I've described reasoning for them in the previous reply.
It would also be great if there were just a dfu command and "tftp" were a subcommand. It's a more common pattern now instead of adding new, top-level commands.
Good point. However, I've tried to avoid #ifdefs in the cmd_dfu.c code. Some boards only use USB and they would not need support for tftp in the cmd_dfu.c command.
Separate command allows adding the code only when it is needed.
That's true, but it seems a simple thing to have an ifdef in the command list.
You could even keep the sub-command implementation in a separate file.
If I've understood you correctly - you propose extending cmd_dfu.c to support tftp.
In this way we would have following dfu command syntax:
dfu [tftp] <usb/eth controller> <medium> <medium number>
e.g. dfu [tftp] 0 mmc 1
And implementation of [tftp] part should go to another file?
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
common/Makefile | 1 + common/cmd_dfutftp.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 common/cmd_dfutftp.c
diff --git a/common/Makefile b/common/Makefile index d6c1d48..483905c 100644 --- a/common/Makefile +++ b/common/Makefile @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o obj-$(CONFIG_CMD_DFU) += cmd_dfu.o obj-$(CONFIG_CMD_GPT) += cmd_gpt.o +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o
# Power obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c new file mode 100644 index 0000000..2b75a09 --- /dev/null +++ b/common/cmd_dfutftp.c @@ -0,0 +1,43 @@ +/*
- cmd_dfutftp.c -- dfutftp command
- Copyright (C) 2015
- Lukasz Majewski l.majewski@majess.pl
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <net.h>
+static +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
unsigned long addr = 0;if (argc < 4 || argc > 5)return CMD_RET_USAGE;char *interface = argv[2];char *devstring = argv[3];if (argc == 5)addr = simple_strtoul(argv[4], NULL, 0);/* Below env variables are descibed in detailat ./doc/README.dfutftp */
setenv("update_tftp_exec_at_boot", "true");setenv("update_tftp_dfu", "true");setenv("update_tftp_dfu_interface", interface);setenv("update_tftp_dfu_devstring", devstring);return update_tftp(addr);+}
+U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp,
"Device Firmware Upgrade via TFTP","<ETH_controller> <interface> <dev>\n"" - device firmware upgrade via <ETH_controller>\n"" using TFTP protocol on device <dev>, attached\n"" to interface <interface>\n"" [addr] - address where FIT image has beenstored\n" +);
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
Hi Lukasz,
On Mon, Jul 20, 2015 at 12:59 PM, Lukasz Majewski l.majewski@majess.pl wrote:
Hi Joe,
Hi Lukasz,
On Thu, Jul 16, 2015 at 3:26 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:
The new 'dfutftp' command has syntax similar to 'dfu' command. This new command however, requires some extra env variables to allow update_tftp() code to work properly. For more explanation, please consult ./doc/README.dfutftp
It would be great if it didn't require them.
I've described reasoning for them in the previous reply.
It would also be great if there were just a dfu command and "tftp" were a subcommand. It's a more common pattern now instead of adding new, top-level commands.
Good point. However, I've tried to avoid #ifdefs in the cmd_dfu.c code. Some boards only use USB and they would not need support for tftp in the cmd_dfu.c command.
Separate command allows adding the code only when it is needed.
That's true, but it seems a simple thing to have an ifdef in the command list.
You could even keep the sub-command implementation in a separate file.
If I've understood you correctly - you propose extending cmd_dfu.c to support tftp.
In this way we would have following dfu command syntax:
dfu [tftp] <usb/eth controller> <medium> <medium number>
e.g. dfu [tftp] 0 mmc 1
Correct.
And implementation of [tftp] part should go to another file?
This is up to you, but that would probably be nicer.
Signed-off-by: Lukasz Majewski l.majewski@majess.pl
common/Makefile | 1 + common/cmd_dfutftp.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 common/cmd_dfutftp.c
diff --git a/common/Makefile b/common/Makefile index d6c1d48..483905c 100644 --- a/common/Makefile +++ b/common/Makefile @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o obj-$(CONFIG_CMD_DFU) += cmd_dfu.o obj-$(CONFIG_CMD_GPT) += cmd_gpt.o +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o
# Power obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c new file mode 100644 index 0000000..2b75a09 --- /dev/null +++ b/common/cmd_dfutftp.c @@ -0,0 +1,43 @@ +/*
- cmd_dfutftp.c -- dfutftp command
- Copyright (C) 2015
- Lukasz Majewski l.majewski@majess.pl
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <net.h>
+static +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
unsigned long addr = 0;if (argc < 4 || argc > 5)return CMD_RET_USAGE;char *interface = argv[2];char *devstring = argv[3];if (argc == 5)addr = simple_strtoul(argv[4], NULL, 0);/* Below env variables are descibed in detailat ./doc/README.dfutftp */
setenv("update_tftp_exec_at_boot", "true");setenv("update_tftp_dfu", "true");setenv("update_tftp_dfu_interface", interface);setenv("update_tftp_dfu_devstring", devstring);return update_tftp(addr);+}
+U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp,
"Device Firmware Upgrade via TFTP","<ETH_controller> <interface> <dev>\n"" - device firmware upgrade via <ETH_controller>\n"" using TFTP protocol on device <dev>, attached\n"" to interface <interface>\n"" [addr] - address where FIT image has beenstored\n" +);
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
Signed-off-by: Lukasz Majewski l.majewski@majess.pl --- include/configs/am335x_evm.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h index 035c156..09f6543 100644 --- a/include/configs/am335x_evm.h +++ b/include/configs/am335x_evm.h @@ -46,6 +46,16 @@ #define CONFIG_CMD_GPT #define CONFIG_EFI_PARTITION
+/* Commands to enable DFU TFTP support */ +#define CONFIG_CMD_DFUTFTP +#define CONFIG_UPDATE_TFTP +#define CONFIG_DFU_TFTP +#define CONFIG_OF_LIBFDT + +/* Enable SHA1 support */ +#define CONFIG_SHA1 +#define CONFIG_CMD_SHA1SUM + #ifdef CONFIG_NAND #define NANDARGS \ "mtdids=" MTDIDS_DEFAULT "\0" \
participants (3)
-
Joe Hershberger -
Lukasz Majewski -
Tormod Volden