* [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU
2015-07-12 15:30 [U-Boot] [PATCH 0/8] dfu: tftp: update: Support for DFU upgrades via ETH (TFTP) Lukasz Majewski
@ 2015-07-12 15:30 ` Lukasz Majewski
2015-07-15 18:14 ` Joe Hershberger
2015-07-12 15:30 ` [U-Boot] [PATCH 2/8] net: tftp: Move tftp.h file from ./net to ./include Lukasz Majewski
` (6 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-12 15:30 UTC (permalink / raw)
To: u-boot
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 at 1 {
+ description = "U-Boot binary";
+
+should look like
+
+ images {
+ u-boot.bin at 1 {
+ description = "U-Boot binary";
+
+where "u-boot.bin" is the DFU entity name to be stored.
+
+
+
+To do
+-----
+
+* Extend dfu-util command (or write new one - e.g. dfueth-util) to support
+ TFTP based transfers
+
+* Upload support (via TFTP)
\ No newline at end of file
--
2.1.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU
2015-07-12 15:30 ` [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU Lukasz Majewski
@ 2015-07-15 18:14 ` Joe Hershberger
2015-07-16 19:59 ` Lukasz Majewski
0 siblings, 1 reply; 34+ messages in thread
From: Joe Hershberger @ 2015-07-15 18:14 UTC (permalink / raw)
To: u-boot
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 to
> + increase values of following two configs:
> + CONFIG_UPDATE_TFTP_MSEC_MAX and CONFIG_UPDATE_TFTP_CNT_MAX.
> + Values of namely 1000 and 5 solve the issue for BBB.
> +
> +* "update_tftp_dfu" ["true"|"false"]
> +
> + By "update_tftp_dfu" env variable we inform update_tftp that
> + it should use dfu for writing data - in this way support for
> + legacy usage is preserved.
Same here... presumably a user only wants support for one or the other
compiled in. Please use a different config.
> +* "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 at 1 {
> + description = "U-Boot binary";
> +
> +should look like
> +
> + images {
> + u-boot.bin at 1 {
> + description = "U-Boot binary";
> +
> +where "u-boot.bin" is the DFU entity name to be stored.
> +
> +
> +
> +To do
> +-----
> +
> +* Extend dfu-util command (or write new one - e.g. dfueth-util) to support
> + TFTP based transfers
> +
> +* Upload support (via TFTP)
> \ No newline at end of file
> --
> 2.1.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU
2015-07-15 18:14 ` Joe Hershberger
@ 2015-07-16 19:59 ` Lukasz Majewski
2015-07-17 19:28 ` Joe Hershberger
0 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-16 19:59 UTC (permalink / raw)
To: 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 to
> > + increase values of following two configs:
> > + CONFIG_UPDATE_TFTP_MSEC_MAX and
> > CONFIG_UPDATE_TFTP_CNT_MAX.
> > + Values of namely 1000 and 5 solve the issue for BBB.
> > +
> > +* "update_tftp_dfu" ["true"|"false"]
> > +
> > + By "update_tftp_dfu" env variable we inform update_tftp that
> > + it should use dfu for writing data - in this way support for
> > + legacy usage is preserved.
>
> Same here... presumably a user only wants support for one or the other
> compiled in. Please use a different config.
The most appealing thing about update.c is that I had to add only a few
lines of code to use it for my purpose. For that reason I've decided to
keep as much as possible of the legacy code.
>
> > +* "update_tftp_dfu_interface" ["mmc"]
> > +* "update_tftp_dfu_devstring" ["1"]
> > +
> > + Both variables - namely "update_tftp_dfu_{interface|devstring}"
> > are
> > + only taken into account when "update_tftp_dfu" is defined.
> > + They store information about interface (e.g. "mmc") and device
> > number
> > + string (e.g. "1").
>
> It is preferable to make these parameters to a command and not magic
> env variables.
They can be specified in the 'dfutftp' command - which syntax is similar
to 'dfu' (e.g. dfutftp 0 mmc 1).
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 at 1 {
> > + description = "U-Boot binary";
> > +
> > +should look like
> > +
> > + images {
> > + u-boot.bin at 1 {
> > + description = "U-Boot binary";
> > +
> > +where "u-boot.bin" is the DFU entity name to be stored.
> > +
> > +
> > +
> > +To do
> > +-----
> > +
> > +* Extend dfu-util command (or write new one - e.g. dfueth-util) to
> > support
> > + TFTP based transfers
> > +
> > +* Upload support (via TFTP)
> > \ No newline at end of file
> > --
> > 2.1.4
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150716/07a9adf2/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU
2015-07-16 19:59 ` Lukasz Majewski
@ 2015-07-17 19:28 ` Joe Hershberger
2015-07-20 18:59 ` Lukasz Majewski
0 siblings, 1 reply; 34+ messages in thread
From: Joe Hershberger @ 2015-07-17 19:28 UTC (permalink / raw)
To: u-boot
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 to
>> > + increase values of following two configs:
>> > + CONFIG_UPDATE_TFTP_MSEC_MAX and
>> > CONFIG_UPDATE_TFTP_CNT_MAX.
>> > + Values of namely 1000 and 5 solve the issue for BBB.
>> > +
>> > +* "update_tftp_dfu" ["true"|"false"]
>> > +
>> > + By "update_tftp_dfu" env variable we inform update_tftp that
>> > + it should use dfu for writing data - in this way support for
>> > + legacy usage is preserved.
>>
>> Same here... presumably a user only wants support for one or the other
>> compiled in. Please use a different config.
>
> The most appealing thing about update.c is that I had to add only a few
> lines of code to use it for my purpose. For that reason I've decided to
> keep as much as possible of the legacy code.
>
>>
>> > +* "update_tftp_dfu_interface" ["mmc"]
>> > +* "update_tftp_dfu_devstring" ["1"]
>> > +
>> > + Both variables - namely "update_tftp_dfu_{interface|devstring}"
>> > are
>> > + only taken into account when "update_tftp_dfu" is defined.
>> > + They store information about interface (e.g. "mmc") and device
>> > number
>> > + string (e.g. "1").
>>
>> It is preferable to make these parameters to a command and not magic
>> env variables.
>
> They can be specified in the 'dfutftp' command - which syntax is similar
> to 'dfu' (e.g. dfutftp 0 mmc 1).
OK, great.
> However, those variables are needed for automatic update after power
> up. In other words update_tftp() function is called very early in boot
> and env variables are a convenient way to specify the interface and its
> number.
So this is only called early in the case of the old functionality.
Your new DFU features do not need an early automatic feature, right?
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 at 1 {
>> > + description = "U-Boot binary";
>> > +
>> > +should look like
>> > +
>> > + images {
>> > + u-boot.bin at 1 {
>> > + description = "U-Boot binary";
>> > +
>> > +where "u-boot.bin" is the DFU entity name to be stored.
>> > +
>> > +
>> > +
>> > +To do
>> > +-----
>> > +
>> > +* Extend dfu-util command (or write new one - e.g. dfueth-util) to
>> > support
>> > + TFTP based transfers
>> > +
>> > +* Upload support (via TFTP)
>> > \ No newline at end of file
>> > --
>> > 2.1.4
>> >
>> > _______________________________________________
>> > U-Boot mailing list
>> > U-Boot at lists.denx.de
>> > http://lists.denx.de/mailman/listinfo/u-boot
>
> Best regards,
>
> Lukasz Majewski
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU
2015-07-17 19:28 ` Joe Hershberger
@ 2015-07-20 18:59 ` Lukasz Majewski
2015-07-20 19:17 ` Joe Hershberger
0 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-20 18:59 UTC (permalink / raw)
To: u-boot
Hi Joe,
> Hi Lukasz,
>
> On Thu, Jul 16, 2015 at 2:59 PM, Lukasz Majewski
> <l.majewski@majess.pl> wrote:
> > Hi Joe,
> >
> > Thank you for your review.
> >
> >> Hi Lukasz,
> >>
> >> On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski
> >> <l.majewski@majess.pl> wrote:
> >> > Documentation file for DFU extension. With this functionality it
> >> > is now possible to transfer FIT images with firmware updates via
> >> > TFTP and use DFU backend for storing them.
> >> >
> >> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> >> > ---
> >> > doc/README.dfutftp | 153
> >> > +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file
> >> > changed, 153 insertions(+) create mode 100644 doc/README.dfutftp
> >> >
> >> > diff --git a/doc/README.dfutftp b/doc/README.dfutftp
> >> > new file mode 100644
> >> > index 0000000..4636321
> >> > --- /dev/null
> >> > +++ b/doc/README.dfutftp
> >> > @@ -0,0 +1,153 @@
> >> > +Device Firmware Upgrade (DFU) - extension to use TFTP
> >> > +=====================================================
> >> > +
> >> > +Why?
> >> > +----
> >> > +
> >> > +* Update TFTP (CONFIG_UPDATE_TFTP) only supports writing
> >> > +code to NAND memory via TFTP.
> >> > +* DFU supports writing data to variety of mediums (NAND,
> >> > +eMMC, SD, partitions, RAM, etc) via USB.
> >> > +
> >> > +Combination of both solves their shortcomings!
> >> > +
> >> > +
> >> > +Overview
> >> > +--------
> >> > +
> >> > +This document briefly describes how to use BF for
> >>
> >> BF?
> >
> > It should be DFU.
> >
> >>
> >> > +upgrading firmware (e.g. kernel, u-boot, rootfs, etc.)
> >> > +via TFTP protocol.
> >> > +
> >> > +By using Ethernet (TFTP protocol to be precise) it was
> >> > +possible to overcome the major problem of USB based DFU -
> >> > +the relatively low transfer speed for large files.
> >> > +This was caused by DFU standard, which imposed utilization
> >> > +of only EP0 for transfer. By using Ethernet we can circumvent
> >> > +this shortcoming.
> >> > +
> >> > +Beagle Bone Black (BBB) powered by TI's am335x CPU has been used
> >> > +as a demo board.
> >> > +
> >> > +To utilize this feature, one needs to first enable support
> >> > +for USB based DFU (CONFIG_DFU_*) and TFTP update
> >> > +(CONFIG_UPDATE_TFTP) described in ./doc/README.update.
> >>
> >> Does it really make sense to reuse this UPDATE_TFTP config? Why not
> >> DFU_TFTP?
> >
> > Enabling CONFIG_UPDATE_TFTP allows reusing parts of the legacy
> > update_tftp() code.
>
> I can understand reusing the code, but that doesn't mean we should
> force both complete features to be included.
The legacy ./common/update.c was using two flags - namely
CONFIG_UPDATE_TFTP (to allow compilation of this file) and
CONFIG_SYS_NO_FLASH. Lack of the latter was using #error to terminate a
compilation.
That was the old behavior - one needed to define both flags to compile
in the legacy code.
Moreover the CONFIG_UPDATE_TFTP is used to enable running
update_tftp() in the early boot stage.
My changes in the ./common/update.c file have split the code to common
one (enabled by CONFIG_UPDATE_TFTP) and legacy one (NAND support
specific) enabled by CONFIG_SYS_NO_FLASH.
>
> > What I mean is that one should enable legacy CONFIG_UPDATE_TFTP as a
> > prerequisite for using dfu tftp transfer.
>
> What if a new config like DFU_TFTP enabled the shared code, but not
> the old behaviors.
DFU_TFTP enables dfu specific code - not the shared code
(at ./common/update.c).
Shared code is enabled by CONFIG_UPDATE_TFTP.
>
> >> > +New "dfutftp" command has been introduced to comply with present
> >> > +USB related commands' syntax.
> >> > +
> >> > +This code works without "fitupd" command enabled.
> >> > +
> >> > +As of this writing
> >> > (SHA1:241746e618fa725491e9ff689710c5f73ffd9139) the +update.c
> >> > code is not enabled (CONFIG_UPDATE_TFTP) by any board in the
> >> > +contemporary u-boot tree.
> >>
> >> So maybe we should remove it.
> >
> > This is IMHO a tricky issue. On the one hand there hasn't left any
> > board supporting this feature in mainline (recently some old PPC
> > boards have been removed from u-boot).
> >
> > One the other hand _probably_ there are deployed systems (as a
> > derivative of the boards supported in u-boot and using this feature)
> > which depend on this feature.
>
> That's fair.
>
> > I'd opt for leaving the original code in the tree with a fat big
> > note about the plan to remove the legacy code in a near future (as
> > we do it with MAKEALL script).
>
> OK
>
> >> > +
> >> > +Environment variables
> >> > +---------------------
> >> > +
> >> > +To preserve legacy behavior of TFTP update (./common/update.c)
> >> > +code following new environment variables had been introduced:
> >>
> >> Another example of why you should use a new config instead of the
> >> existing one.
> >
> > Could you be more specific here?
>
> You are adding magic env vars here because you want to have both old
> and new behaviors at once. I think it would be better to select one at
> build time and not add any magic.
In theory you are right. In practice it is a bit more complicated - I
will try to explain it latter in this e-mail.
>
> >> > +* "update_tftp_exec_at_boot" ["true"|"false"]
> >> > +
> >> > + New usage of update_tftp allows setting the
> >> > + "update_tftp_exec_at_boot" env variable to allow it running
> >> > + at boot time.
> >> > + In this way update_tftp will not be executed at startup and
> >> > reduce
> >> > + boot time.
> >> > +
> >> > + To preserve backward compatibility for legacy boards this
> >> > variable
> >> > + should be set to "true".
> >> > +
> >> > + BBB note:
> >> > + When update tftp is not working after boot one need
> >> > to
> >> > + increase values of following two configs:
> >> > + CONFIG_UPDATE_TFTP_MSEC_MAX and
> >> > CONFIG_UPDATE_TFTP_CNT_MAX.
> >> > + Values of namely 1000 and 5 solve the issue for BBB.
> >> > +
> >> > +* "update_tftp_dfu" ["true"|"false"]
> >> > +
> >> > + By "update_tftp_dfu" env variable we inform update_tftp that
> >> > + it should use dfu for writing data - in this way support for
> >> > + legacy usage is preserved.
> >>
> >> Same here... presumably a user only wants support for one or the
> >> other compiled in. Please use a different config.
> >
> > The most appealing thing about update.c is that I had to add only a
> > few lines of code to use it for my purpose. For that reason I've
> > decided to keep as much as possible of the legacy code.
> >
> >>
> >> > +* "update_tftp_dfu_interface" ["mmc"]
> >> > +* "update_tftp_dfu_devstring" ["1"]
> >> > +
> >> > + Both variables - namely
> >> > "update_tftp_dfu_{interface|devstring}" are
> >> > + only taken into account when "update_tftp_dfu" is defined.
> >> > + They store information about interface (e.g. "mmc") and device
> >> > number
> >> > + string (e.g. "1").
> >>
> >> It is preferable to make these parameters to a command and not
> >> magic env variables.
> >
> > They can be specified in the 'dfutftp' command - which syntax is
> > similar to 'dfu' (e.g. dfutftp 0 mmc 1).
>
> OK, great.
>
> > However, those variables are needed for automatic update after power
> > up. In other words update_tftp() function is called very early in
> > boot and env variables are a convenient way to specify the
> > interface and its number.
>
> So this is only called early in the case of the old functionality.
> Your new DFU features do not need an early automatic feature, right?
I'm afraid that they will require some kind of update in the early
boot state.
>
> In fact, I would argue that the old method didn't need it either and
> never should have supported that. Things like this should simply be
> part of the board's preboot script so we don't need this type of magic
> stuff.
Please correct me if I'm wrong.
I should use another CONFIG_* to enable using common update_tftp()
function. In this way I would avoid calling it in the early boot code.
Instead, I should define proper "preboot" env variable with "dfu tftp 0
mmc 1" command and use it for SW updating.
>
> >> > + In the "dfutftp" command they are explicitly overridden.
> >> > + It is done deliberately, since in this command we may use
> >> > arbitrary values. +
> >> > + Default values, when available during boot, are used by
> >> > update_tftp
> >> > + (when dfu support is enabled) to properly setup medium device
> >> > + (e.g. mmc 1).
> >> > +
> >> > +
> >> > +
> >> > +Beagle Bone Black (BBB) setup
> >> > +-----------------------------
> >> > +
> >> > +1. Setup tftp env variables:
> >> > + * select desired eth device - 'ethact' variable
> >> > ["ethact=cpsw"]
> >> > + (use "bdinfo" to check current setting)
> >> > + * setup "serverip" and "ipaddr" variables
> >> > + * set "loadaddr" as a fixed buffer where incoming data is
> >> > placed
> >> > + ["loadaddr=0x81000000"]
> >> > +
> >> > +#########
> >> > +# BONUS #
> >> > +#########
> >> > +It is possible to use USB interface to emulate ETH connection by
> >> > setting +"ethact=usb_ether". In this way one can have very fast
> >> > DFU transfer via USB.
> >>
> >> Is thor not faster than DFU?
> >
> > Yes, it is. However, DFU is standardized (which is despite of its
> > low speed its huge advantage) - thor not.
> >
> >>
> >> It seems like DFU should support a bulk endpoint if performance is
> >> an issue, right?
> >
> > Yes, it should, but such modification would be not compliant with
> > the standard.
>
> Who is the standards body? Can they accept suggestions for the next
> revision? Is it worth trying to improve the standard?
The last revision of DFU standard is from 2006 with Greg KH being a
notable USB committee board member :-)
I've asked him about the possibility to revise the standard but he
replied that chances are small since Linux Foundation is not part
of the USB standard committee anymore.
>
> >>That would be more efficient than emulating Ethernet.
> >
> > To be more precise - I've combined the ability to use Ethernet with
> > DFU flashing backend.
> >
> > In this way boards only equipped with ETH can (re)use DFU code to
> > flash data (on MMC, NAND, filesystems, RAM, etc).
>
> Yes, that's very good. I was simply talking about the "BONUS" where
> emulated Ethernet over USB is used. In that case it would be more
> efficient to actually have a raw bulk endpoint supported by DFU.
Yes, it would. Unfortunately I think that it would be very hard to
revise the DFU standard.
ETH over USB can be used on devices equipped only with USB (like
trats/trats2 devel mobile phones). In that way DFU speed would increase.
>
> >> > +For 33MiB test image the transfer rate is 1MiB/s
> >> > +
> >> > +2. Setup update_tftp variables:
> >> > + * set "updatefile" - the file name to be downloaded via TFTP
> >> > (stored on
> >> > + the HOST at e.g. /srv/tftp)
> >> > +
> >> > +3. Setup dfutftp specific variables (as explained in
> >> > "Environment Variables")
> >> > + * "update_tftp_exec_at_boot=true"
> >> > + * "update_tftp_dfu=true"
> >> > +
> >> > +4. Inspect "dfu" specific variables:
> >> > + * "dfu_alt_info" - information about available DFU entities
> >> > + * "dfu_bufsiz" - variable to set buffer size [in bytes] -
> >> > when it is not
> >> > + possible to set large enough default buffer
> >> > (8 MiB @ BBB) +
> >> > +
> >> > +
> >> > +FIT image format for download
> >> > +-----------------------------
> >> > +
> >> > +To create FIT image for download one should follow the update
> >> > tftp README file +(./doc/README.update) with one notable
> >> > difference: +
> >> > +The original snippet of ./doc/uImage.FIT/update_uboot.its
> >> > +
> >> > + images {
> >> > + update at 1 {
> >> > + description = "U-Boot binary";
> >> > +
> >> > +should look like
> >> > +
> >> > + images {
> >> > + u-boot.bin at 1 {
> >> > + description = "U-Boot binary";
> >> > +
> >> > +where "u-boot.bin" is the DFU entity name to be stored.
> >> > +
> >> > +
> >> > +
> >> > +To do
> >> > +-----
> >> > +
> >> > +* Extend dfu-util command (or write new one - e.g. dfueth-util)
> >> > to support
> >> > + TFTP based transfers
> >> > +
> >> > +* Upload support (via TFTP)
> >> > \ No newline at end of file
> >> > --
> >> > 2.1.4
> >> >
> >> > _______________________________________________
> >> > U-Boot mailing list
> >> > U-Boot at lists.denx.de
> >> > http://lists.denx.de/mailman/listinfo/u-boot
> >
> > Best regards,
> >
> > Lukasz Majewski
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150720/2fda2091/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU
2015-07-20 18:59 ` Lukasz Majewski
@ 2015-07-20 19:17 ` Joe Hershberger
2015-07-20 20:09 ` Tormod Volden
2015-07-21 21:55 ` Lukasz Majewski
0 siblings, 2 replies; 34+ messages in thread
From: Joe Hershberger @ 2015-07-20 19:17 UTC (permalink / raw)
To: u-boot
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 need
>> >> > to
>> >> > + increase values of following two configs:
>> >> > + CONFIG_UPDATE_TFTP_MSEC_MAX and
>> >> > CONFIG_UPDATE_TFTP_CNT_MAX.
>> >> > + Values of namely 1000 and 5 solve the issue for BBB.
>> >> > +
>> >> > +* "update_tftp_dfu" ["true"|"false"]
>> >> > +
>> >> > + By "update_tftp_dfu" env variable we inform update_tftp that
>> >> > + it should use dfu for writing data - in this way support for
>> >> > + legacy usage is preserved.
>> >>
>> >> Same here... presumably a user only wants support for one or the
>> >> other compiled in. Please use a different config.
>> >
>> > The most appealing thing about update.c is that I had to add only a
>> > few lines of code to use it for my purpose. For that reason I've
>> > decided to keep as much as possible of the legacy code.
>> >
>> >>
>> >> > +* "update_tftp_dfu_interface" ["mmc"]
>> >> > +* "update_tftp_dfu_devstring" ["1"]
>> >> > +
>> >> > + Both variables - namely
>> >> > "update_tftp_dfu_{interface|devstring}" are
>> >> > + only taken into account when "update_tftp_dfu" is defined.
>> >> > + They store information about interface (e.g. "mmc") and device
>> >> > number
>> >> > + string (e.g. "1").
>> >>
>> >> It is preferable to make these parameters to a command and not
>> >> magic env variables.
>> >
>> > They can be specified in the 'dfutftp' command - which syntax is
>> > similar to 'dfu' (e.g. dfutftp 0 mmc 1).
>>
>> OK, great.
>>
>> > However, those variables are needed for automatic update after power
>> > up. In other words update_tftp() function is called very early in
>> > boot and env variables are a convenient way to specify the
>> > interface and its number.
>>
>> So this is only called early in the case of the old functionality.
>> Your new DFU features do not need an early automatic feature, right?
>
> I'm afraid that they will require some kind of update in the early
> boot state.
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 at 1 {
>> >> > + description = "U-Boot binary";
>> >> > +
>> >> > +should look like
>> >> > +
>> >> > + images {
>> >> > + u-boot.bin at 1 {
>> >> > + description = "U-Boot binary";
>> >> > +
>> >> > +where "u-boot.bin" is the DFU entity name to be stored.
>> >> > +
>> >> > +
>> >> > +
>> >> > +To do
>> >> > +-----
>> >> > +
>> >> > +* Extend dfu-util command (or write new one - e.g. dfueth-util)
>> >> > to support
>> >> > + TFTP based transfers
>> >> > +
>> >> > +* Upload support (via TFTP)
>> >> > \ No newline at end of file
>> >> > --
>> >> > 2.1.4
>> >> >
>> >> > _______________________________________________
>> >> > U-Boot mailing list
>> >> > U-Boot at lists.denx.de
>> >> > http://lists.denx.de/mailman/listinfo/u-boot
>> >
>> > Best regards,
>> >
>> > Lukasz Majewski
>
> Best regards,
> Lukasz Majewski
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU
2015-07-20 19:17 ` Joe Hershberger
@ 2015-07-20 20:09 ` Tormod Volden
2015-07-21 21:55 ` Lukasz Majewski
1 sibling, 0 replies; 34+ messages in thread
From: Tormod Volden @ 2015-07-20 20:09 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU
2015-07-20 19:17 ` Joe Hershberger
2015-07-20 20:09 ` Tormod Volden
@ 2015-07-21 21:55 ` Lukasz Majewski
1 sibling, 0 replies; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-21 21:55 UTC (permalink / raw)
To: u-boot
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 one
> >> >> > need to
> >> >> > + increase values of following two configs:
> >> >> > + CONFIG_UPDATE_TFTP_MSEC_MAX and
> >> >> > CONFIG_UPDATE_TFTP_CNT_MAX.
> >> >> > + Values of namely 1000 and 5 solve the issue for
> >> >> > BBB. +
> >> >> > +* "update_tftp_dfu" ["true"|"false"]
> >> >> > +
> >> >> > + By "update_tftp_dfu" env variable we inform update_tftp
> >> >> > that
> >> >> > + it should use dfu for writing data - in this way support
> >> >> > for
> >> >> > + legacy usage is preserved.
> >> >>
> >> >> Same here... presumably a user only wants support for one or the
> >> >> other compiled in. Please use a different config.
> >> >
> >> > The most appealing thing about update.c is that I had to add
> >> > only a few lines of code to use it for my purpose. For that
> >> > reason I've decided to keep as much as possible of the legacy
> >> > code.
> >> >
> >> >>
> >> >> > +* "update_tftp_dfu_interface" ["mmc"]
> >> >> > +* "update_tftp_dfu_devstring" ["1"]
> >> >> > +
> >> >> > + Both variables - namely
> >> >> > "update_tftp_dfu_{interface|devstring}" are
> >> >> > + only taken into account when "update_tftp_dfu" is defined.
> >> >> > + They store information about interface (e.g. "mmc") and
> >> >> > device number
> >> >> > + string (e.g. "1").
> >> >>
> >> >> It is preferable to make these parameters to a command and not
> >> >> magic env variables.
> >> >
> >> > They can be specified in the 'dfutftp' command - which syntax is
> >> > similar to 'dfu' (e.g. dfutftp 0 mmc 1).
> >>
> >> OK, great.
> >>
> >> > However, those variables are needed for automatic update after
> >> > power up. In other words update_tftp() function is called very
> >> > early in boot and env variables are a convenient way to specify
> >> > the interface and its number.
> >>
> >> So this is only called early in the case of the old functionality.
> >> Your new DFU features do not need an early automatic feature,
> >> right?
> >
> > I'm afraid that they will require some kind of update in the early
> > boot state.
>
> 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 default
> >> >> > buffer (8 MiB @ BBB) +
> >> >> > +
> >> >> > +
> >> >> > +FIT image format for download
> >> >> > +-----------------------------
> >> >> > +
> >> >> > +To create FIT image for download one should follow the update
> >> >> > tftp README file +(./doc/README.update) with one notable
> >> >> > difference: +
> >> >> > +The original snippet of ./doc/uImage.FIT/update_uboot.its
> >> >> > +
> >> >> > + images {
> >> >> > + update at 1 {
> >> >> > + description = "U-Boot binary";
> >> >> > +
> >> >> > +should look like
> >> >> > +
> >> >> > + images {
> >> >> > + u-boot.bin at 1 {
> >> >> > + description = "U-Boot binary";
> >> >> > +
> >> >> > +where "u-boot.bin" is the DFU entity name to be stored.
> >> >> > +
> >> >> > +
> >> >> > +
> >> >> > +To do
> >> >> > +-----
> >> >> > +
> >> >> > +* Extend dfu-util command (or write new one - e.g.
> >> >> > dfueth-util) to support
> >> >> > + TFTP based transfers
> >> >> > +
> >> >> > +* Upload support (via TFTP)
> >> >> > \ No newline at end of file
> >> >> > --
> >> >> > 2.1.4
> >> >> >
> >> >> > _______________________________________________
> >> >> > U-Boot mailing list
> >> >> > U-Boot at lists.denx.de
> >> >> > http://lists.denx.de/mailman/listinfo/u-boot
> >> >
> >> > Best regards,
> >> >
> >> > Lukasz Majewski
> >
> > Best regards,
> > Lukasz Majewski
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150721/90725bff/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [U-Boot] [PATCH 2/8] net: tftp: Move tftp.h file from ./net to ./include
2015-07-12 15:30 [U-Boot] [PATCH 0/8] dfu: tftp: update: Support for DFU upgrades via ETH (TFTP) Lukasz Majewski
2015-07-12 15:30 ` [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU Lukasz Majewski
@ 2015-07-12 15:30 ` Lukasz Majewski
2015-07-15 18:17 ` Joe Hershberger
2015-07-12 15:30 ` [U-Boot] [PATCH 3/8] tftp: update: Allow some parts of the code to be reused when CONFIG_SYS_NO_FLASH is set Lukasz Majewski
` (5 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-12 15:30 UTC (permalink / raw)
To: u-boot
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__ */
--
2.1.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 2/8] net: tftp: Move tftp.h file from ./net to ./include
2015-07-12 15:30 ` [U-Boot] [PATCH 2/8] net: tftp: Move tftp.h file from ./net to ./include Lukasz Majewski
@ 2015-07-15 18:17 ` Joe Hershberger
2015-07-16 20:02 ` Lukasz Majewski
0 siblings, 1 reply; 34+ messages in thread
From: Joe Hershberger @ 2015-07-15 18:17 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 34+ messages in thread
* [U-Boot] [PATCH 2/8] net: tftp: Move tftp.h file from ./net to ./include
2015-07-15 18:17 ` Joe Hershberger
@ 2015-07-16 20:02 ` Lukasz Majewski
0 siblings, 0 replies; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-16 20:02 UTC (permalink / raw)
To: 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 at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150716/20b8210a/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [U-Boot] [PATCH 3/8] tftp: update: Allow some parts of the code to be reused when CONFIG_SYS_NO_FLASH is set
2015-07-12 15:30 [U-Boot] [PATCH 0/8] dfu: tftp: update: Support for DFU upgrades via ETH (TFTP) Lukasz Majewski
2015-07-12 15:30 ` [U-Boot] [PATCH 1/8] doc: dfu: tftp: README entry for TFTP extension of DFU Lukasz Majewski
2015-07-12 15:30 ` [U-Boot] [PATCH 2/8] net: tftp: Move tftp.h file from ./net to ./include Lukasz Majewski
@ 2015-07-12 15:30 ` Lukasz Majewski
2015-07-15 18:19 ` Joe Hershberger
2015-07-12 15:30 ` [U-Boot] [PATCH 4/8] dfu: tftp: update: Provide tftp support for the DFU subsystem Lukasz Majewski
` (4 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-12 15:30 UTC (permalink / raw)
To: u-boot
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;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 4/8] dfu: tftp: update: Provide tftp support for the DFU subsystem
2015-07-12 15:30 [U-Boot] [PATCH 0/8] dfu: tftp: update: Support for DFU upgrades via ETH (TFTP) Lukasz Majewski
` (2 preceding siblings ...)
2015-07-12 15:30 ` [U-Boot] [PATCH 3/8] tftp: update: Allow some parts of the code to be reused when CONFIG_SYS_NO_FLASH is set Lukasz Majewski
@ 2015-07-12 15:30 ` Lukasz Majewski
2015-07-15 18:24 ` Joe Hershberger
2015-07-12 15:30 ` [U-Boot] [PATCH 5/8] dfu: tftp: update: Add dfu_write_from_mem_addr() function Lukasz Majewski
` (3 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-12 15:30 UTC (permalink / raw)
To: u-boot
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_ */
--
2.1.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 4/8] dfu: tftp: update: Provide tftp support for the DFU subsystem
2015-07-12 15:30 ` [U-Boot] [PATCH 4/8] dfu: tftp: update: Provide tftp support for the DFU subsystem Lukasz Majewski
@ 2015-07-15 18:24 ` Joe Hershberger
2015-07-16 20:06 ` Lukasz Majewski
0 siblings, 1 reply; 34+ messages in thread
From: Joe Hershberger @ 2015-07-15 18:24 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 4/8] dfu: tftp: update: Provide tftp support for the DFU subsystem
2015-07-15 18:24 ` Joe Hershberger
@ 2015-07-16 20:06 ` Lukasz Majewski
2015-07-17 19:35 ` Joe Hershberger
0 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-16 20:06 UTC (permalink / raw)
To: 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 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.
Good point - thanks!
>
> > +}
> > +#endif
> > +
> > int dfu_add(struct usb_configuration *c);
> > #endif /* __DFU_ENTITY_H_ */
> > --
> > 2.1.4
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150716/e0afce0c/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 4/8] dfu: tftp: update: Provide tftp support for the DFU subsystem
2015-07-16 20:06 ` Lukasz Majewski
@ 2015-07-17 19:35 ` Joe Hershberger
2015-07-20 19:03 ` Lukasz Majewski
0 siblings, 1 reply; 34+ messages in thread
From: Joe Hershberger @ 2015-07-17 19:35 UTC (permalink / raw)
To: u-boot
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 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.
>
> Good point - thanks!
>
>>
>> > +}
>> > +#endif
>> > +
>> > int dfu_add(struct usb_configuration *c);
>> > #endif /* __DFU_ENTITY_H_ */
>> > --
>> > 2.1.4
>> >
>> > _______________________________________________
>> > U-Boot mailing list
>> > U-Boot at lists.denx.de
>> > http://lists.denx.de/mailman/listinfo/u-boot
>
> Best regards,
> Lukasz Majewski
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 4/8] dfu: tftp: update: Provide tftp support for the DFU subsystem
2015-07-17 19:35 ` Joe Hershberger
@ 2015-07-20 19:03 ` Lukasz Majewski
0 siblings, 0 replies; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-20 19:03 UTC (permalink / raw)
To: u-boot
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 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.
> >
> > Good point - thanks!
> >
> >>
> >> > +}
> >> > +#endif
> >> > +
> >> > int dfu_add(struct usb_configuration *c);
> >> > #endif /* __DFU_ENTITY_H_ */
> >> > --
> >> > 2.1.4
> >> >
> >> > _______________________________________________
> >> > U-Boot mailing list
> >> > U-Boot at lists.denx.de
> >> > http://lists.denx.de/mailman/listinfo/u-boot
> >
> > Best regards,
> > Lukasz Majewski
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150720/a2aeddcc/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [U-Boot] [PATCH 5/8] dfu: tftp: update: Add dfu_write_from_mem_addr() function
2015-07-12 15:30 [U-Boot] [PATCH 0/8] dfu: tftp: update: Support for DFU upgrades via ETH (TFTP) Lukasz Majewski
` (3 preceding siblings ...)
2015-07-12 15:30 ` [U-Boot] [PATCH 4/8] dfu: tftp: update: Provide tftp support for the DFU subsystem Lukasz Majewski
@ 2015-07-12 15:30 ` Lukasz Majewski
2015-07-15 18:33 ` Joe Hershberger
2015-07-12 15:30 ` [U-Boot] [PATCH 6/8] update: tftp: dfu: Extend update_tftp() function to support DFU Lukasz Majewski
` (2 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-12 15:30 UTC (permalink / raw)
To: u-boot
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);
--
2.1.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 5/8] dfu: tftp: update: Add dfu_write_from_mem_addr() function
2015-07-12 15:30 ` [U-Boot] [PATCH 5/8] dfu: tftp: update: Add dfu_write_from_mem_addr() function Lukasz Majewski
@ 2015-07-15 18:33 ` Joe Hershberger
2015-07-16 20:07 ` Lukasz Majewski
0 siblings, 1 reply; 34+ messages in thread
From: Joe Hershberger @ 2015-07-15 18:33 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 5/8] dfu: tftp: update: Add dfu_write_from_mem_addr() function
2015-07-15 18:33 ` Joe Hershberger
@ 2015-07-16 20:07 ` Lukasz Majewski
0 siblings, 0 replies; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-16 20:07 UTC (permalink / raw)
To: 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 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.
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 at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150716/3ab2eeba/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [U-Boot] [PATCH 6/8] update: tftp: dfu: Extend update_tftp() function to support DFU
2015-07-12 15:30 [U-Boot] [PATCH 0/8] dfu: tftp: update: Support for DFU upgrades via ETH (TFTP) Lukasz Majewski
` (4 preceding siblings ...)
2015-07-12 15:30 ` [U-Boot] [PATCH 5/8] dfu: tftp: update: Add dfu_write_from_mem_addr() function Lukasz Majewski
@ 2015-07-12 15:30 ` Lukasz Majewski
2015-07-15 18:35 ` Joe Hershberger
2015-07-12 15:30 ` [U-Boot] [PATCH 7/8] dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP Lukasz Majewski
2015-07-12 15:30 ` [U-Boot] [PATCH 8/8] config: bbb: Configs necessary for running update via TFTP on Beagle Bone Black Lukasz Majewski
7 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-12 15:30 UTC (permalink / raw)
To: u-boot
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);
--
2.1.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 6/8] update: tftp: dfu: Extend update_tftp() function to support DFU
2015-07-12 15:30 ` [U-Boot] [PATCH 6/8] update: tftp: dfu: Extend update_tftp() function to support DFU Lukasz Majewski
@ 2015-07-15 18:35 ` Joe Hershberger
2015-07-16 20:11 ` Lukasz Majewski
0 siblings, 1 reply; 34+ messages in thread
From: Joe Hershberger @ 2015-07-15 18:35 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 6/8] update: tftp: dfu: Extend update_tftp() function to support DFU
2015-07-15 18:35 ` Joe Hershberger
@ 2015-07-16 20:11 ` Lukasz Majewski
2015-07-17 19:38 ` Joe Hershberger
0 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-16 20:11 UTC (permalink / raw)
To: 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 at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150716/1d90f76b/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 6/8] update: tftp: dfu: Extend update_tftp() function to support DFU
2015-07-16 20:11 ` Lukasz Majewski
@ 2015-07-17 19:38 ` Joe Hershberger
2015-07-20 19:05 ` Lukasz Majewski
0 siblings, 1 reply; 34+ messages in thread
From: Joe Hershberger @ 2015-07-17 19:38 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
>> > http://lists.denx.de/mailman/listinfo/u-boot
>
> Best regards,
> Lukasz Majewski
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 6/8] update: tftp: dfu: Extend update_tftp() function to support DFU
2015-07-17 19:38 ` Joe Hershberger
@ 2015-07-20 19:05 ` Lukasz Majewski
0 siblings, 0 replies; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-20 19:05 UTC (permalink / raw)
To: u-boot
On Fri, 17 Jul 2015 14:38:20 -0500
Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Lukasz,
>
> On Thu, Jul 16, 2015 at 3:11 PM, Lukasz Majewski
> <l.majewski@majess.pl> wrote:
> > Hi Joe,
> >
> >> Hi Lukasz,
> >>
> >> On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski
> >> <l.majewski@majess.pl> wrote:
> >> > This code allows using DFU defined mediums for storing data
> >> > received via TFTP protocol.
> >> >
> >> > It reuses legacy code at common/update.c.
> >> >
> >> > To run update_tftp() during boot one needs to define
> >> > "update_tftp_exec_at_boot=true".
> >> >
> >> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> >> > ---
> >> > common/update.c | 37 ++++++++++++++++++++++++++++---------
> >> > 1 file changed, 28 insertions(+), 9 deletions(-)
> >> >
> >> > diff --git a/common/update.c b/common/update.c
> >> > index 75c6d62..f3ed036 100644
> >> > --- a/common/update.c
> >> > +++ b/common/update.c
> >> > @@ -18,6 +18,7 @@
> >> > #include <net.h>
> >> > #include <tftp.h>
> >> > #include <malloc.h>
> >> > +#include <dfu.h>
> >> >
> >> > /* env variable holding the location of the update file */
> >> > #define UPDATE_FILE_ENV "updatefile"
> >> > @@ -224,11 +225,18 @@ static int update_fit_getparams(const void
> >> > *fit, int noffset, ulong *addr,
> >> >
> >> > int update_tftp(ulong addr)
> >> > {
> >> > - char *filename, *env_addr;
> >> > - int images_noffset, ndepth, noffset;
> >> > + char *filename, *env_addr, *fit_image_name;
> >> > ulong update_addr, update_fladdr, update_size;
> >> > - void *fit;
> >> > + int images_noffset, ndepth, noffset;
> >> > + bool update_tftp_dfu = false;
> >> > int ret = 0;
> >> > + void *fit;
> >> > +
> >> > + if (!getenv("update_tftp_exec_at_boot"))
> >> > + return 0;
> >> > +
> >> > + if (getenv("update_tftp_dfu"))
> >> > + update_tftp_dfu = true;
> >>
> >> As I mentioned in the documentation patch, it would be nice to
> >> split these out and drop the env vars.
> >
> > This would be difficult since update_tftp() is used at dfutftp() and
> > legacy fitupd command. Moreover it is called in the early stage of
> > booting to perform auto firmware upgrade.
>
> So you are using it for the dfu stuff too. How early does it need to
> be? Maybe preboot? That's what I do on my products. Works great for
> me.
>
> I'd really prefer not to proliferate this practice.
>
Ok, I see.
> > Those env variables give some control over its behavior.
> >
> >>
> >> >
> >> > /* use already present image */
> >> > if (addr)
> >> > @@ -277,8 +285,8 @@ got_update_file:
> >> > if (ndepth != 1)
> >> > goto next_node;
> >> >
> >> > - printf("Processing update '%s' :",
> >> > - fit_get_name(fit, noffset, NULL));
> >> > + fit_image_name = (char *)fit_get_name(fit,
> >> > noffset, NULL);
> >> > + printf("Processing update '%s' :",
> >> > fit_image_name);
> >> >
> >> > if (!fit_image_verify(fit, noffset)) {
> >> > printf("Error: invalid update hash,
> >> > aborting\n"); @@ -294,10 +302,21 @@ got_update_file:
> >> > ret = 1;
> >> > goto next_node;
> >> > }
> >> > - if (update_flash(update_addr, update_fladdr,
> >> > update_size)) {
> >> > - printf("Error: can't flash update,
> >> > aborting\n");
> >> > - ret = 1;
> >> > - goto next_node;
> >> > +
> >> > + if (!update_tftp_dfu) {
> >> > + if (update_flash(update_addr,
> >> > update_fladdr,
> >> > + update_size)) {
> >> > + printf("Error: can't flash
> >> > update, aborting\n");
> >> > + ret = 1;
> >> > + goto next_node;
> >> > + }
> >> > + } else if (fit_image_check_type(fit, noffset,
> >> > +
> >> > IH_TYPE_FIRMWARE)) {
> >> > + if (dfu_tftp_write(fit_image_name,
> >> > + update_addr,
> >> > update_size)) {
> >> > + ret = 1;
> >> > + goto next_node;
> >> > + }
> >> > }
> >> > next_node:
> >> > noffset = fdt_next_node(fit, noffset, &ndepth);
> >> > --
> >> > 2.1.4
> >> >
> >> > _______________________________________________
> >> > U-Boot mailing list
> >> > U-Boot at lists.denx.de
> >> > http://lists.denx.de/mailman/listinfo/u-boot
> >
> > Best regards,
> > Lukasz Majewski
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150720/4e930ef2/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [U-Boot] [PATCH 7/8] dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP
2015-07-12 15:30 [U-Boot] [PATCH 0/8] dfu: tftp: update: Support for DFU upgrades via ETH (TFTP) Lukasz Majewski
` (5 preceding siblings ...)
2015-07-12 15:30 ` [U-Boot] [PATCH 6/8] update: tftp: dfu: Extend update_tftp() function to support DFU Lukasz Majewski
@ 2015-07-12 15:30 ` Lukasz Majewski
2015-07-15 18:39 ` Joe Hershberger
2015-07-12 15:30 ` [U-Boot] [PATCH 8/8] config: bbb: Configs necessary for running update via TFTP on Beagle Bone Black Lukasz Majewski
7 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-12 15:30 UTC (permalink / raw)
To: u-boot
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"
+);
--
2.1.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 7/8] dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP
2015-07-12 15:30 ` [U-Boot] [PATCH 7/8] dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP Lukasz Majewski
@ 2015-07-15 18:39 ` Joe Hershberger
2015-07-16 20:26 ` Lukasz Majewski
0 siblings, 1 reply; 34+ messages in thread
From: Joe Hershberger @ 2015-07-15 18:39 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 7/8] dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP
2015-07-15 18:39 ` Joe Hershberger
@ 2015-07-16 20:26 ` Lukasz Majewski
2015-07-17 19:40 ` Joe Hershberger
0 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-16 20:26 UTC (permalink / raw)
To: 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 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 at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150716/37167e6b/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 7/8] dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP
2015-07-16 20:26 ` Lukasz Majewski
@ 2015-07-17 19:40 ` Joe Hershberger
2015-07-20 17:59 ` Lukasz Majewski
0 siblings, 1 reply; 34+ messages in thread
From: Joe Hershberger @ 2015-07-17 19:40 UTC (permalink / raw)
To: u-boot
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 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 at lists.denx.de
>> > http://lists.denx.de/mailman/listinfo/u-boot
>
> Best regards,
> Lukasz Majewski
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 7/8] dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP
2015-07-17 19:40 ` Joe Hershberger
@ 2015-07-20 17:59 ` Lukasz Majewski
2015-07-20 18:11 ` Joe Hershberger
0 siblings, 1 reply; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-20 17:59 UTC (permalink / raw)
To: u-boot
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 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 at lists.denx.de
> >> > http://lists.denx.de/mailman/listinfo/u-boot
> >
> > Best regards,
> > Lukasz Majewski
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150720/14f937af/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread* [U-Boot] [PATCH 7/8] dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP
2015-07-20 17:59 ` Lukasz Majewski
@ 2015-07-20 18:11 ` Joe Hershberger
0 siblings, 0 replies; 34+ messages in thread
From: Joe Hershberger @ 2015-07-20 18:11 UTC (permalink / raw)
To: u-boot
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 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 at lists.denx.de
>> >> > http://lists.denx.de/mailman/listinfo/u-boot
>> >
>> > Best regards,
>> > Lukasz Majewski
>
> Best regards,
> Lukasz Majewski
^ permalink raw reply [flat|nested] 34+ messages in thread
* [U-Boot] [PATCH 8/8] config: bbb: Configs necessary for running update via TFTP on Beagle Bone Black
2015-07-12 15:30 [U-Boot] [PATCH 0/8] dfu: tftp: update: Support for DFU upgrades via ETH (TFTP) Lukasz Majewski
` (6 preceding siblings ...)
2015-07-12 15:30 ` [U-Boot] [PATCH 7/8] dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP Lukasz Majewski
@ 2015-07-12 15:30 ` Lukasz Majewski
7 siblings, 0 replies; 34+ messages in thread
From: Lukasz Majewski @ 2015-07-12 15:30 UTC (permalink / raw)
To: u-boot
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" \
--
2.1.4
^ permalink raw reply related [flat|nested] 34+ messages in thread