[U-Boot] Automatic software updates in U-Boot
Hello,
This patch series adds support for automatic software updates from a TFTP server. First patch introduces some changes in the TFTP timeout handling that are required by the feature proper, which is contained in the second patch. Third patch provides minor enhancements to the iminfo output that can become handy when using the new feature.
Please see newly added doc/README.au_tftp for more details.
Code has been compile-tested on several ppc, arm and mips targets. The feature itself has been tested on TQM8555 and MPC8555CDS.
Regards, Bartlomiej Sieka
There are two aspects of a TFTP transfer involving timeouts: 1. timeout waiting for initial server reply after sending RRQ 2. timeouts while transferring actual data from the server
Since the upcoming auto-update feature attempts a TFTP download during each boot, it is undesirable to have a long delay when the TFTP server is not available. Thus, this commit makes the server timeout (1.) configurable by two global variables:
TftpRRQTimeoutSecs TftpRRQTimeoutCountMax
TftpRRQTimeoutSecs overrides default timeout when trying to connect to a TFTP server, TftpRRQTimeoutCountMax overrides default number of connection retries. The total delay when trying to download a file from a non-existing TFTP server is TftpRRQTimeoutSecs x TftpRRQTimeoutCountMax seconds.
Timeouts during file transfers (2.) are unaffected.
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com --- net/tftp.c | 30 +++++++++++++++++++++++++----- 1 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/net/tftp.c b/net/tftp.c index 9aeecb8..fe6a204 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -34,6 +34,21 @@ #define TFTP_ERROR 5 #define TFTP_OACK 6
+static ulong TftpTimeoutSecs = TIMEOUT; +static int TftpTimeoutCountMax = TIMEOUT_COUNT; + +/* + * These globals govern the timeout behavior when attempting a connection to a + * TFTP server. TftpRRQTimeoutSecs specifies the number of seconds to wait for + * the server to respond to initial connection, and TftpRRQTimeoutCountMax + * gives the number of such connection retries. TftpRRQTimeoutCountMax must be + * non-negative and TftpRRQTimeoutSecs must be positive. The globals are meant + * to be set (and restored) by code needing non-standard timeout behavior when + * initiating a TFTP transfer. + */ +ulong TftpRRQTimeoutSecs = TIMEOUT; +int TftpRRQTimeoutCountMax = TIMEOUT_COUNT; + static IPaddr_t TftpServerIP; static int TftpServerPort; /* The UDP port at their end */ static int TftpOurPort; /* The UDP port at our end */ @@ -180,7 +195,7 @@ TftpSend (void) pkt += 5 /*strlen("octet")*/ + 1; strcpy ((char *)pkt, "timeout"); pkt += 7 /*strlen("timeout")*/ + 1; - sprintf((char *)pkt, "%lu", TIMEOUT); + sprintf((char *)pkt, "%lu", TftpTimeoutSecs); #ifdef ET_DEBUG printf("send option "timeout %s"\n", (char *)pkt); #endif @@ -370,7 +385,9 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) }
TftpLastBlock = TftpBlock; - NetSetTimeout (TIMEOUT * CFG_HZ, TftpTimeout); + TftpTimeoutSecs = TIMEOUT; + TftpTimeoutCountMax = TIMEOUT_COUNT; + NetSetTimeout (TftpTimeoutSecs * CFG_HZ, TftpTimeout);
store_block (TftpBlock - 1, pkt + 2, len);
@@ -441,7 +458,7 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) static void TftpTimeout (void) { - if (++TftpTimeoutCount > TIMEOUT_COUNT) { + if (++TftpTimeoutCount > TftpTimeoutCountMax) { puts ("\nRetry count exceeded; starting again\n"); #ifdef CONFIG_MCAST_TFTP mcast_cleanup(); @@ -449,7 +466,7 @@ TftpTimeout (void) NetStartAgain (); } else { puts ("T "); - NetSetTimeout (TIMEOUT * CFG_HZ, TftpTimeout); + NetSetTimeout (TftpTimeoutSecs * CFG_HZ, TftpTimeout); TftpSend (); } } @@ -520,7 +537,10 @@ TftpStart (void)
puts ("Loading: *\b");
- NetSetTimeout (TIMEOUT * CFG_HZ, TftpTimeout); + TftpTimeoutSecs = TftpRRQTimeoutSecs; + TftpTimeoutCountMax = TftpRRQTimeoutCountMax; + + NetSetTimeout (TftpTimeoutSecs * CFG_HZ, TftpTimeout); NetSetHandler (TftpHandler);
TftpServerPort = WELL_KNOWN_PORT;
Bartlomiej Sieka wrote:
There are two aspects of a TFTP transfer involving timeouts:
- timeout waiting for initial server reply after sending RRQ
- timeouts while transferring actual data from the server
Since the upcoming auto-update feature attempts a TFTP download during each boot, it is undesirable to have a long delay when the TFTP server is not available. Thus, this commit makes the server timeout (1.) configurable by two global variables:
TftpRRQTimeoutSecs TftpRRQTimeoutCountMax
TftpRRQTimeoutSecs overrides default timeout when trying to connect to a TFTP server, TftpRRQTimeoutCountMax overrides default number of connection retries. The total delay when trying to download a file from a non-existing TFTP server is TftpRRQTimeoutSecs x TftpRRQTimeoutCountMax seconds.
Hi Bartlomiej,
Are seconds an appropriate scale factor for the timeout? Using tenths (thousandths?) of seconds seems much better for allowing timeout choices. (Thousandths could cause problems with clock tick resolution and is unnecessarily fine grained. Gut feel is tenths of seconds is sufficient granularity and practical. Hundredths should be practical too.)
With seconds, you only can chose 1, 2, 3, ... seconds for the timeout. This becomes more severe with the number of timeouts that you chose.
My gut feel is that some multiple of tenths of seconds times some number of retries totaling 1-2 seconds (say 0.2 seconds x 10 retries or 0.5 seconds x 4 retries) is much more practical than 2 retries at 1 second intervals.
Best regards, gvb
Just my two cents worth...
On Fri, Sep 19, 2008 at 1:17 AM, Jerry Van Baren gerald.vanbaren@ge.com wrote:
Bartlomiej Sieka wrote:
There are two aspects of a TFTP transfer involving timeouts:
- timeout waiting for initial server reply after sending RRQ
- timeouts while transferring actual data from the server
Are seconds an appropriate scale factor for the timeout? Using tenths (thousandths?) of seconds seems much better for allowing timeout choices. (Thousandths could cause problems with clock tick resolution and is unnecessarily fine grained. Gut feel is tenths of seconds is
I would have thought that milliseconds would be the most appropriate choice (milliseconds being an SI unit and most timeouts I have seen have been defined in milliseconds) - If it is going to be a problem at the lower level, you can always divide by 10 (or 100) when you actually implement the timeout functionality. I just think at the user level it should be seconds or milliseconds
Regards,
Graeme
Graeme Russ wrote:
Just my two cents worth...
On Fri, Sep 19, 2008 at 1:17 AM, Jerry Van Baren gerald.vanbaren@ge.com wrote:
Bartlomiej Sieka wrote:
There are two aspects of a TFTP transfer involving timeouts:
- timeout waiting for initial server reply after sending RRQ
- timeouts while transferring actual data from the server
Are seconds an appropriate scale factor for the timeout? Using tenths (thousandths?) of seconds seems much better for allowing timeout choices. (Thousandths could cause problems with clock tick resolution and is unnecessarily fine grained. Gut feel is tenths of seconds is
I would have thought that milliseconds would be the most appropriate choice (milliseconds being an SI unit and most timeouts I have seen have been defined in milliseconds) - If it is going to be a problem at the lower level, you can always divide by 10 (or 100) when you actually implement the timeout functionality. I just think at the user level it should be seconds or milliseconds
Regards, Graeme
Hi Graeme,
Milliseconds is fine with me. I actually started editing my email with milliseconds, then debated thousandths/hundredths/tenths and ended up with tenths. My main concern is that seconds is too coarse.
Best regards, gvb
Jerry Van Baren wrote:
Bartlomiej Sieka wrote:
There are two aspects of a TFTP transfer involving timeouts:
- timeout waiting for initial server reply after sending RRQ
- timeouts while transferring actual data from the server
Since the upcoming auto-update feature attempts a TFTP download during each boot, it is undesirable to have a long delay when the TFTP server is not available. Thus, this commit makes the server timeout (1.) configurable by two global variables:
TftpRRQTimeoutSecs TftpRRQTimeoutCountMax
TftpRRQTimeoutSecs overrides default timeout when trying to connect to a TFTP server, TftpRRQTimeoutCountMax overrides default number of connection retries. The total delay when trying to download a file from a non-existing TFTP server is TftpRRQTimeoutSecs x TftpRRQTimeoutCountMax seconds.
Hi Bartlomiej,
Are seconds an appropriate scale factor for the timeout?
Hi Jerry,
The patch didn't introduce changes in this regard -- TFTP timeouts were defined in seconds originally. The patch makes the timeout behavior configurable, instead of being hardcoded, but the units remain the same:
#define TIMEOUT 5UL /* Seconds to timeout for a lost pkt */
Regards, Bartlomiej Sieka
Dear Bartek,
In message 48D33E23.3020502@semihalf.com you wrote:
Are seconds an appropriate scale factor for the timeout?
...
The patch didn't introduce changes in this regard -- TFTP timeouts were defined in seconds originally. The patch makes the timeout behavior configurable, instead of being hardcoded, but the units remain the same:
But the argument was a perfectly valid one. I am, too, concerned that second resolution is not optimal any more (it was as long as we needed just one timeout long enough, but this situation will be changed by your other patches).
So please extend that patch to change this into milliseconds as discussed before.
Thanks.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Bartek,
In message 48D33E23.3020502@semihalf.com you wrote:
Are seconds an appropriate scale factor for the timeout?
...
The patch didn't introduce changes in this regard -- TFTP timeouts were defined in seconds originally. The patch makes the timeout behavior configurable, instead of being hardcoded, but the units remain the same:
But the argument was a perfectly valid one. I am, too, concerned that second resolution is not optimal any more (it was as long as we needed just one timeout long enough, but this situation will be changed by your other patches).
So please extend that patch to change this into milliseconds as discussed before.
OK, v2 of the patches will implement milisecond granularity for initial TFTP server timeout.
Regards, Bartlomiej Sieka
The auto-update feature allows to automatically download software updates from a TFTP server and store them in Flash memory during boot. Updates are contained in a FIT file and protected with SHA-1 checksum.
More detailed description can be found in doc/README.au_tftp
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com --- README | 12 ++ common/Makefile | 1 + common/au_tftp.c | 279 +++++++++++++++++++++++++++++++++++++++ common/main.c | 7 + doc/README.au_tftp | 89 +++++++++++++ doc/uImage.FIT/update3.its | 41 ++++++ doc/uImage.FIT/update_uboot.its | 21 +++ 7 files changed, 450 insertions(+), 0 deletions(-) create mode 100644 common/au_tftp.c create mode 100644 doc/README.au_tftp create mode 100644 doc/uImage.FIT/update3.its create mode 100644 doc/uImage.FIT/update_uboot.its
diff --git a/README b/README index ccd839c..23516eb 100644 --- a/README +++ b/README @@ -1737,6 +1737,14 @@ The following options need to be configured: example, some LED's) on your board. At the moment, the following checkpoints are implemented:
+- Automatic software updates via TFTP server + CONFIG_AU_TFTP + CONFIG_AU_TFTP_CNT_MAX + CONFIG_AU_TFTP_SEC_MAX + + These options enable and control the auto-update feature; + for a more detailed description refer to doc/README.au_tftp. + Legacy uImage format:
Arg Where When @@ -2811,6 +2819,10 @@ Some configuration options can be set using Environment Variables: allowed for use by the bootm command. See also "bootm_low" environment variable.
+ auto-update - Location of the sofware update file on a TFTP server, used + by the automatic software update feature. Please refer to + documentation in doc/README.au_tftp for more details. + autoload - if set to "no" (any string beginning with 'n'), "bootp" will just load perform a lookup of the configuration from the BOOTP server, but not try to diff --git a/common/Makefile b/common/Makefile index 8bddf8e..96850b2 100644 --- a/common/Makefile +++ b/common/Makefile @@ -155,6 +155,7 @@ COBJS-$(CONFIG_LCD) += lcd.o COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o COBJS-$(CONFIG_DDR_SPD) += ddr_spd.o +COBJS-$(CONFIG_AU_TFTP) += au_tftp.o
COBJS := $(sort $(COBJS-y)) SRCS := $(AOBJS:.o=.S) $(COBJS:.o=.c) diff --git a/common/au_tftp.c b/common/au_tftp.c new file mode 100644 index 0000000..7dfecab --- /dev/null +++ b/common/au_tftp.c @@ -0,0 +1,279 @@ +/* + * (C) Copyright 2008 Semihalf + * + * Written by: Rafal Czubak rcz@semihalf.com + * Bartlomiej Sieka tur@semihalf.com + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + * + */ + +#include <common.h> + +#if !(defined(CONFIG_FIT) && defined(CONFIG_OF_LIBFDT)) +#error "CONFIG_FIT and CONFIG_OF_LIBFDT are required for auto-update feature" +#endif + +#if defined(CFG_NO_FLASH) +#error "CFG_NO_FLASH defined, but FLASH is required for auto-update feature" +#endif + +#include <command.h> +#include <flash.h> +#include <net.h> + +/* env variable holding the location of the update file */ +#define AU_FILE_ENV "auto-update" +#define AU_NETRETRY_LEN 10 + +/* set configuration defaults if needed */ +#ifndef CONFIG_AU_LOAD_ADDR +#define CONFIG_AU_LOAD_ADDR 0x100000 +#endif + +#ifndef CONFIG_AU_TFTP_SEC_MAX +#define CONFIG_AU_TFTP_SEC_MAX 1 +#endif + +#ifndef CONFIG_AU_TFTP_CNT_MAX +#define CONFIG_AU_TFTP_CNT_MAX 0 +#endif + +extern ulong TftpRRQTimeoutSecs; +extern int TftpRRQTimeoutCountMax; +extern flash_info_t flash_info[]; +extern ulong load_addr; + + +static int au_load(char *filename, ulong sec_max, int cnt_max, uint32_t addr) +{ + int size, rv; + ulong saved_timeout_secs; + int saved_timeout_count; + char saved_netretry[AU_NETRETRY_LEN]; + char *netretry; + + rv = 0; + /* save used globals and env variable */ + saved_timeout_secs = TftpRRQTimeoutSecs; + saved_timeout_count = TftpRRQTimeoutCountMax; + + memset(&saved_netretry, 0, AU_NETRETRY_LEN); + if ((netretry = getenv("netretry")) != NULL) { + if (strlen(netretry) >= AU_NETRETRY_LEN) + printf("netretry value too long, won't be restored\n"); + else + strncpy(saved_netretry, netretry, AU_NETRETRY_LEN - 1); + } + + /* set timeouts for auto-update */ + TftpRRQTimeoutSecs = sec_max; + TftpRRQTimeoutCountMax = cnt_max; + + /* we don't want to retry the connection if errors occur */ + setenv("netretry", "no"); + + /* download the update file */ + load_addr = addr; + copy_filename(BootFile, filename, sizeof(BootFile)); + size = NetLoop(TFTP); + + if (size < 0) + rv = 1; + else if (size > 0) + flush_cache(addr, size); + + /* restore changed globals and env variable */ + TftpRRQTimeoutSecs = saved_timeout_secs; + TftpRRQTimeoutCountMax = saved_timeout_count; + + if (saved_netretry[0] != '\0') + setenv("netretry", saved_netretry); + else + setenv("netretry", NULL); + + return rv; +} + +static int au_flash(uint32_t addr_source, uint32_t addr_first, uint32_t size) +{ + uint32_t addr_last, bank, sector_end_addr; + flash_info_t *info; + char found; + int i; + + /* compute correct addr_last */ + addr_last = addr_first + size - 1; + + if (addr_first >= addr_last) { + printf("Error: end address exceeds addressing space\n"); + return 1; + } + + /* + * It may happen that addr_last doesn't fall on the sector + * boundary. We want to round such an address to the next + * sector boundary, so that the commands don't fail later on. + */ + + /* find the end addr of the sector where the addr_last is */ + found = 0; + for (bank = 0; bank < CFG_MAX_FLASH_BANKS && !found; ++bank) { + info = &flash_info[bank]; + for (i = 0; i < info->sector_count && !found; ++i) { + /* get the end address of the sector */ + if (i == info->sector_count - 1) + sector_end_addr = info->start[0] + + info->size - 1; + else + sector_end_addr = info->start[i+1] - 1; + + if (addr_last <= sector_end_addr && + addr_last >= info->start[i]) { + found = 1; + /* adjust addr_last if necessary */ + if (addr_last < sector_end_addr) + addr_last = sector_end_addr; + } + } + } + if (!found) { + printf("Error: end address (0x%08x) not in flash!\n", + addr_last); + return 1; + } + + /* remove protection on processed sectors */ + if (flash_sect_protect(0, addr_first, addr_last) > 0) { + printf("Error: could not unprotect flash sectors\n"); + return 1; + } + + printf("Erasing 0x%08x - 0x%08x", addr_first, addr_last); + if (flash_sect_erase(addr_first, addr_last) > 0) { + printf("Error: could not erase flash\n"); + return 1; + } + + printf("Copying to flash..."); + if (flash_write((char *)addr_source, addr_first, size) > 0) { + printf("Error: could not copy to flash\n"); + return 1; + } + printf("done\n"); + + /* enable protection on processed sectors */ + if (flash_sect_protect(1, addr_first, addr_last) > 0) { + printf("Error: could not protect flash sectors\n"); + return 1; + } + + return 0; +} + +static int au_fit_getparams(const void *fit, int noffset, uint32_t *addr, + uint32_t *fladdr, uint32_t *size) +{ + const void *data; + + if (fit_image_get_data(fit, noffset, &data, (size_t *)size)) + return 1; + + if (fit_image_get_load(fit, noffset, (ulong *)fladdr)) + return 1; + + *addr = (uint32_t)data; + + return 0; +} + +void au_tftp(void) +{ + char *filename, *env_addr; + int images_noffset, ndepth, noffset; + static uint32_t update_addr, update_fladdr, update_size; + ulong addr; + void *fit; + + printf("Auto-update from TFTP: "); + + /* get the file name of the update file */ + filename = getenv(AU_FILE_ENV); + if (filename == NULL) { + printf("failed, env. variable '%s' not found\n", AU_FILE_ENV); + return; + } + + printf("trying update file '%s'\n", filename); + + /* get load address of downloaded update file */ + if ((env_addr = getenv("loadaddr")) != NULL) + addr = simple_strtoul(env_addr, NULL, 16); + else + addr = CONFIG_AU_LOAD_ADDR; + + + if (au_load(filename, CONFIG_AU_TFTP_SEC_MAX, + CONFIG_AU_TFTP_CNT_MAX, addr)) { + printf("Can't load update file, aborting auto-update\n"); + return; + } + + fit = (void *)addr; + + if (!fit_check_format((void *)fit)) { + printf("Bad FIT format of the update file, aborting " + "auto-update\n"); + return; + } + + /* process updates */ + images_noffset = fdt_path_offset(fit, FIT_IMAGES_PATH); + + ndepth = 0; + noffset = fdt_next_node(fit, images_noffset, &ndepth); + while (noffset >= 0 && ndepth > 0) { + if (ndepth != 1) + goto next_node; + + printf("Processing update '%s' :", + fit_get_name(fit, noffset, NULL)); + + if (!fit_image_check_hashes(fit, noffset)) { + printf("Error: invalid update hash, aborting\n"); + goto next_node; + } + + printf("\n"); + if (au_fit_getparams(fit, noffset, &update_addr, + &update_fladdr, &update_size)) { + printf("Error: can't get update parameteres, " + "aborting\n"); + goto next_node; + } + if (au_flash(update_addr, update_fladdr, update_size)) { + printf("Error: can't flash update, aborting\n"); + goto next_node; + } +next_node: + noffset = fdt_next_node(fit, noffset, &ndepth); + } + + return; +} diff --git a/common/main.c b/common/main.c index 187ef8a..0d28eb4 100644 --- a/common/main.c +++ b/common/main.c @@ -56,6 +56,9 @@ extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]); /* fo
extern int do_bootd (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
+#if defined(CONFIG_AU_TFTP) +void au_tftp (void); +#endif /* CONFIG_AU_TFTP */
#define MAX_DELAY_STOP_STR 32
@@ -290,6 +293,10 @@ void main_loop (void) char bcs_set[16]; #endif /* CONFIG_BOOTCOUNT_LIMIT */
+#if defined(CONFIG_AU_TFTP) + au_tftp (); +#endif /* CONFIG_AU_TFTP */ + #if defined(CONFIG_VFD) && defined(VFD_TEST_LOGO) ulong bmp = 0; /* default bitmap */ extern int trab_vfd (ulong bitmap); diff --git a/doc/README.au_tftp b/doc/README.au_tftp new file mode 100644 index 0000000..a2529fb --- /dev/null +++ b/doc/README.au_tftp @@ -0,0 +1,89 @@ +Automatic software update from a TFTP server +============================================ + +Overview +-------- + +This feature allows to automatically store software updates present on a TFTP +server in NOR Flash. In more detail: a TFTP transfer of a file given in +environment variable 'auto-update' from server 'serverip' is attempted during +boot. The update file should be a FIT file, and can contain one or more +updates. Each update in the update file has an address in NOR Flash where it +should be placed, updates are also protected with a SHA-1 checksum. If the +TFTP transfer is successful, the hash of each update is verified, and if the +verification is positive, the update is stored in Flash. + +The auto-update feature is enabled by the CONFIG_AU_TFTP macro: + +#define CONFIG_AU_TFTP 1 + + +Note that when enabling auto-update, Flash support must be turned on. Also, +one must enable FIT and LIBFDT support: + +#define CONFIG_FIT 1 +#define CONFIG_OF_LIBFDT 1 + +The auto-update feature uses the following configuration knobs: + +- CONFIG_AU_LOAD_ADDR + + Normally, TFTP transfer of the update file is done to the address specified + in environment variable 'loadaddr'. If this variable is not present, the + transfer is made to the address given in CONFIG_AU_LOAD_ADDR (0x100000 by + default). + +- CONFIG_AU_TFTP_CNT_MAX + CONFIG_AU_TFTP_SEC_MAX + + These knobs control the timeouts during initial connection to the TFTP + server. Since a transfer is attempted during each boot, it is undesirable to + have a long delay when a TFTP server is not present. CONFIG_AU_TFTP_SEC_MAX + specifies the number of seconds to wait for the server to respond to initial + connection, and CONFIG_AU_TFTP_CNT_MAX gives the number of such connection + retries. CONFIG_AU_TFTP_CNT_MAX must be non-negative and is 0 by default, + CONFIG_AU_TFTP_SEC_MAX must be positive and is 1 by default. + +Since the update file is in FIT format, it is created from an *.its file using +the mkimage tool. dtc tool with support for binary includes, e.g. in version +1.2.0 or later, must also be available on the system where the update file is +to be prepared. Refer to the doc/uImage.FIT/ directory for more details on FIT +images. + + +Example .its files +------------------ + +- doc/uImage.FIT/update_uboot.its + + A simple example that can be used to create an update file for automatically + replacing U-Boot image on a system. + + Assuming that an U-Boot image u-boot.bin is present in the current working + directory, and that the address given in the 'load' property in the + 'update_uboot.its' file is where the U-Boot is stored in Flash, the + following command will create the actual update file 'update_uboot.itb': + + mkimage -f update_uboot.its update_uboot.itb + + Place 'update_uboot.itb' on a TFTP server, for example as + '/tftpboot/update_uboot.itb', and set the 'auto-update' variable + appropriately, for example in the U-Boot prompt: + + setenv auto-update /tftpboot/update_uboot.itb + saveenv + + Now, when the system boots up and the update TFTP server specified in the + 'serverip' environment variable is accessible, the new U-Boot image will be + automatically stored in Flash. + + NOTE: do make sure that the 'u-boot.bin' image used to create the update + file is a good, working image. Also make sure that the address in Flash + where the update will be placed is correct. Making mistake here and + attempting the auto-update can render the system unusable. + +- doc/uImage.FIT/update3.its + + An example containing three updates. It can be used to update Linux kernel, + ramdisk and FDT blob stored in Flash. The procedure for preparing the update + file is similar to the example above. diff --git a/doc/uImage.FIT/update3.its b/doc/uImage.FIT/update3.its new file mode 100644 index 0000000..285cf73 --- /dev/null +++ b/doc/uImage.FIT/update3.its @@ -0,0 +1,41 @@ +/* + * Example Automatic software update file. + */ +/ { + description = "Automatic software updates: kernel, ramdisk, FDT"; + #address-cells = <1>; + + images { + update@1 { + description = "Linux kernel binary"; + data = /incbin/("./vmlinux.bin.gz"); + compression = "none"; + type = "firmware"; + load = <FF700000>; + hash@1 { + algo = "sha1"; + }; + }; + update@2 { + description = "Ramdisk image"; + data = /incbin/("./ramdisk_image.gz"); + compression = "none"; + type = "firmware"; + load = <FF8E0000>; + hash@1 { + algo = "sha1"; + }; + }; + + update@3 { + description = "FDT blob"; + data = /incbin/("./blob.fdt"); + compression = "none"; + type = "firmware"; + load = <FFAC0000>; + hash@1 { + algo = "sha1"; + }; + }; + }; +}; diff --git a/doc/uImage.FIT/update_uboot.its b/doc/uImage.FIT/update_uboot.its new file mode 100644 index 0000000..e0d27ea --- /dev/null +++ b/doc/uImage.FIT/update_uboot.its @@ -0,0 +1,21 @@ +/* + * Automatic software update for U-Boot + * Make sure the flashing addresses ('load' prop) is correct for your board! + */ +/ { + description = "Automatic U-Boot update"; + #address-cells = <1>; + + images { + update@1 { + description = "U-Boot binary"; + data = /incbin/("./u-boot.bin"); + compression = "none"; + type = "firmware"; + load = <FFFC0000>; + hash@1 { + algo = "sha1"; + }; + }; + }; +};
Dear Bartlomiej Sieka,
In message 12217502101047-git-send-email-tur@semihalf.com you wrote:
The auto-update feature allows to automatically download software updates from a TFTP server and store them in Flash memory during boot. Updates are contained in a FIT file and protected with SHA-1 checksum.
More detailed description can be found in doc/README.au_tftp
This patch then also needs to be adapted when changing to millisecond timeout resolution, so please submit a v2 when this is done.
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com
README | 12 ++ common/Makefile | 1 + common/au_tftp.c | 279 +++++++++++++++++++++++++++++++++++++++ common/main.c | 7 + doc/README.au_tftp | 89 +++++++++++++ doc/uImage.FIT/update3.its | 41 ++++++ doc/uImage.FIT/update_uboot.its | 21 +++ 7 files changed, 450 insertions(+), 0 deletions(-) create mode 100644 common/au_tftp.c create mode 100644 doc/README.au_tftp create mode 100644 doc/uImage.FIT/update3.its create mode 100644 doc/uImage.FIT/update_uboot.its
diff --git a/README b/README index ccd839c..23516eb 100644 --- a/README +++ b/README @@ -1737,6 +1737,14 @@ The following options need to be configured: example, some LED's) on your board. At the moment, the following checkpoints are implemented:
+- Automatic software updates via TFTP server
CONFIG_AU_TFTPCONFIG_AU_TFTP_CNT_MAXCONFIG_AU_TFTP_SEC_MAXThese options enable and control the auto-update feature;for a more detailed description refer to doc/README.au_tftp.Legacy uImage format:
Arg Where When @@ -2811,6 +2819,10 @@ Some configuration options can be set using Environment Variables: allowed for use by the bootm command. See also "bootm_low" environment variable.
- auto-update - Location of the sofware update file on a TFTP server, used
I think "auto-update" is not a good name (especially since it has a different meaning than the similar sounding "autoload"0; also there is a typo in "sofware".
But most of all - do we really need a new environment variable? What's wrong with our good old "bootfile" ?
--- a/common/Makefile +++ b/common/Makefile @@ -155,6 +155,7 @@ COBJS-$(CONFIG_LCD) += lcd.o COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o COBJS-$(CONFIG_DDR_SPD) += ddr_spd.o +COBJS-$(CONFIG_AU_TFTP) += au_tftp.o
Maybe we could start making lists sorted again (or at least no add to the confusion)? Thanks.
COBJS := $(sort $(COBJS-y)) SRCS := $(AOBJS:.o=.S) $(COBJS:.o=.c) diff --git a/common/au_tftp.c b/common/au_tftp.c new file mode 100644 index 0000000..7dfecab --- /dev/null +++ b/common/au_tftp.c @@ -0,0 +1,279 @@
...
- memset(&saved_netretry, 0, AU_NETRETRY_LEN);
- if ((netretry = getenv("netretry")) != NULL) {
if (strlen(netretry) >= AU_NETRETRY_LEN)printf("netretry value too long, won't be restored\n");elsestrncpy(saved_netretry, netretry, AU_NETRETRY_LEN - 1);- }
Maybe a "char *saved_netretry = strdup(getenv("netretry"));" would have been less hassle?
- /* set timeouts for auto-update */
- TftpRRQTimeoutSecs = sec_max;
- TftpRRQTimeoutCountMax = cnt_max;
- /* we don't want to retry the connection if errors occur */
- setenv("netretry", "no");
- /* download the update file */
- load_addr = addr;
- copy_filename(BootFile, filename, sizeof(BootFile));
- size = NetLoop(TFTP);
- if (size < 0)
rv = 1;- else if (size > 0)
flush_cache(addr, size);- /* restore changed globals and env variable */
- TftpRRQTimeoutSecs = saved_timeout_secs;
- TftpRRQTimeoutCountMax = saved_timeout_count;
- if (saved_netretry[0] != '\0')
setenv("netretry", saved_netretry);- else
setenv("netretry", NULL);
See above
- return rv;
+}
+static int au_flash(uint32_t addr_source, uint32_t addr_first, uint32_t size) +{
- uint32_t addr_last, bank, sector_end_addr;
- flash_info_t *info;
- char found;
- int i;
- /* compute correct addr_last */
- addr_last = addr_first + size - 1;
- if (addr_first >= addr_last) {
printf("Error: end address exceeds addressing space\n");return 1;- }
This is obviously for NOR flash only.
How could the code be extended to be usable for NAND flash / DataFlash / OneNAND / IDE storage devices as well?
- for (bank = 0; bank < CFG_MAX_FLASH_BANKS && !found; ++bank) {
info = &flash_info[bank];for (i = 0; i < info->sector_count && !found; ++i) {/* get the end address of the sector */if (i == info->sector_count - 1)sector_end_addr = info->start[0] +info->size - 1;
Curly braces are needed for multiline statements.
elsesector_end_addr = info->start[i+1] - 1;
And then for the following "else" as well.
- /* enable protection on processed sectors */
- if (flash_sect_protect(1, addr_first, addr_last) > 0) {
printf("Error: could not protect flash sectors\n");return 1;- }
This should only be done if these sectors have been protected before.
+void au_tftp(void) +{
Is it a good idea to make this a void function? How about error handling?
--- a/common/main.c +++ b/common/main.c @@ -56,6 +56,9 @@ extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]); /* fo
extern int do_bootd (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
+#if defined(CONFIG_AU_TFTP) +void au_tftp (void); +#endif /* CONFIG_AU_TFTP */
#define MAX_DELAY_STOP_STR 32
@@ -290,6 +293,10 @@ void main_loop (void) char bcs_set[16]; #endif /* CONFIG_BOOTCOUNT_LIMIT */
+#if defined(CONFIG_AU_TFTP)
- au_tftp ();
+#endif /* CONFIG_AU_TFTP */
#if defined(CONFIG_VFD) && defined(VFD_TEST_LOGO) ulong bmp = 0; /* default bitmap */ extern int trab_vfd (ulong bitmap);
You definitely don't want to add the function call right in the middle of variable declarations, or do you?
Best regards,
Wolfgang Denk
Hi Wolfgang,
You wrote:
Dear Bartlomiej Sieka,
In message 12217502101047-git-send-email-tur@semihalf.com you wrote:
The auto-update feature allows to automatically download software updates from a TFTP server and store them in Flash memory during boot. Updates are contained in a FIT file and protected with SHA-1 checksum.
More detailed description can be found in doc/README.au_tftp
This patch then also needs to be adapted when changing to millisecond timeout resolution, so please submit a v2 when this is done.
OK, will do. Thanks for your other comments -- please see my replies to some of them below; comments not replied to are generally OK and will be addressed in v2 of the patches.
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com
README | 12 ++ common/Makefile | 1 + common/au_tftp.c | 279 +++++++++++++++++++++++++++++++++++++++ common/main.c | 7 + doc/README.au_tftp | 89 +++++++++++++ doc/uImage.FIT/update3.its | 41 ++++++ doc/uImage.FIT/update_uboot.its | 21 +++ 7 files changed, 450 insertions(+), 0 deletions(-) create mode 100644 common/au_tftp.c create mode 100644 doc/README.au_tftp create mode 100644 doc/uImage.FIT/update3.its create mode 100644 doc/uImage.FIT/update_uboot.its
diff --git a/README b/README index ccd839c..23516eb 100644 --- a/README +++ b/README @@ -1737,6 +1737,14 @@ The following options need to be configured: example, some LED's) on your board. At the moment, the following checkpoints are implemented:
+- Automatic software updates via TFTP server
CONFIG_AU_TFTPCONFIG_AU_TFTP_CNT_MAXCONFIG_AU_TFTP_SEC_MAXThese options enable and control the auto-update feature;for a more detailed description refer to doc/README.au_tftp.Legacy uImage format:
Arg Where When @@ -2811,6 +2819,10 @@ Some configuration options can be set using Environment Variables: allowed for use by the bootm command. See also "bootm_low" environment variable.
- auto-update - Location of the sofware update file on a TFTP server, used
I think "auto-update" is not a good name (especially since it has a different meaning than the similar sounding "autoload"0; also there is a typo in "sofware".
But most of all - do we really need a new environment variable? What's wrong with our good old "bootfile" ?
The only concern here is the interaction with bootp and dhcp commands -- they will set the "bootfile" env. variable to the file name got from the server, and the next time around U-Boot will try to use that file name to get the update. So I'd rather stick with a separate env. variable for the name of the update file.
--- a/common/Makefile +++ b/common/Makefile @@ -155,6 +155,7 @@ COBJS-$(CONFIG_LCD) += lcd.o COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o COBJS-$(CONFIG_DDR_SPD) += ddr_spd.o +COBJS-$(CONFIG_AU_TFTP) += au_tftp.o
Maybe we could start making lists sorted again (or at least no add to the confusion)? Thanks.
COBJS := $(sort $(COBJS-y)) SRCS := $(AOBJS:.o=.S) $(COBJS:.o=.c) diff --git a/common/au_tftp.c b/common/au_tftp.c new file mode 100644 index 0000000..7dfecab --- /dev/null +++ b/common/au_tftp.c @@ -0,0 +1,279 @@
...
- memset(&saved_netretry, 0, AU_NETRETRY_LEN);
- if ((netretry = getenv("netretry")) != NULL) {
if (strlen(netretry) >= AU_NETRETRY_LEN)printf("netretry value too long, won't be restored\n");elsestrncpy(saved_netretry, netretry, AU_NETRETRY_LEN - 1);- }
Maybe a "char *saved_netretry = strdup(getenv("netretry"));" would have been less hassle?
- /* set timeouts for auto-update */
- TftpRRQTimeoutSecs = sec_max;
- TftpRRQTimeoutCountMax = cnt_max;
- /* we don't want to retry the connection if errors occur */
- setenv("netretry", "no");
- /* download the update file */
- load_addr = addr;
- copy_filename(BootFile, filename, sizeof(BootFile));
- size = NetLoop(TFTP);
- if (size < 0)
rv = 1;- else if (size > 0)
flush_cache(addr, size);- /* restore changed globals and env variable */
- TftpRRQTimeoutSecs = saved_timeout_secs;
- TftpRRQTimeoutCountMax = saved_timeout_count;
- if (saved_netretry[0] != '\0')
setenv("netretry", saved_netretry);- else
setenv("netretry", NULL);See above
- return rv;
+}
+static int au_flash(uint32_t addr_source, uint32_t addr_first, uint32_t size) +{
- uint32_t addr_last, bank, sector_end_addr;
- flash_info_t *info;
- char found;
- int i;
- /* compute correct addr_last */
- addr_last = addr_first + size - 1;
- if (addr_first >= addr_last) {
printf("Error: end address exceeds addressing space\n");return 1;- }
This is obviously for NOR flash only.
How could the code be extended to be usable for NAND flash / DataFlash / OneNAND / IDE storage devices as well?
Primarily, FIT specification for the update file will have to be extended. Current approach is able to handle a series of <data, address> pairs, where the address is enough for U-Boot to tell where the data should go. If we are to handle other storage types, we need to specify which storage type the data should go to, and also provide a type-specific location. This is somewhat akin to the way we access and boot from these storage types: we use type-specific commands (nand, nboot, ide, diskboot, etc.).
Also, it would be of course nice to have a framework within U-Boot for generic data copying between storage types, that would hide all the type-specific details and provide uniform interface.
- for (bank = 0; bank < CFG_MAX_FLASH_BANKS && !found; ++bank) {
info = &flash_info[bank];for (i = 0; i < info->sector_count && !found; ++i) {/* get the end address of the sector */if (i == info->sector_count - 1)sector_end_addr = info->start[0] +info->size - 1;Curly braces are needed for multiline statements.
elsesector_end_addr = info->start[i+1] - 1;And then for the following "else" as well.
- /* enable protection on processed sectors */
- if (flash_sect_protect(1, addr_first, addr_last) > 0) {
printf("Error: could not protect flash sectors\n");return 1;- }
This should only be done if these sectors have been protected before.
+void au_tftp(void) +{
Is it a good idea to make this a void function? How about error handling?
It seems to be a convention for functions conditionally called from main_loop() to have the error handling withing them, and ignore their return values in main_loop(). So au_tftp() also handles errors itself, and doesn't return anything.
--- a/common/main.c +++ b/common/main.c @@ -56,6 +56,9 @@ extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]); /* fo
extern int do_bootd (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
+#if defined(CONFIG_AU_TFTP) +void au_tftp (void); +#endif /* CONFIG_AU_TFTP */
#define MAX_DELAY_STOP_STR 32
@@ -290,6 +293,10 @@ void main_loop (void) char bcs_set[16]; #endif /* CONFIG_BOOTCOUNT_LIMIT */
+#if defined(CONFIG_AU_TFTP)
- au_tftp ();
+#endif /* CONFIG_AU_TFTP */
#if defined(CONFIG_VFD) && defined(VFD_TEST_LOGO) ulong bmp = 0; /* default bitmap */ extern int trab_vfd (ulong bitmap);
You definitely don't want to add the function call right in the middle of variable declarations, or do you?
The idea is to have au_tftp() called as early as possible, before any other functions in main_loop().
Here's a larger snippet of the related code:
#if defined(CONFIG_VFD) && defined(VFD_TEST_LOGO) ulong bmp = 0; /* default bitmap */ extern int trab_vfd (ulong bitmap);
#ifdef CONFIG_MODEM_SUPPORT if (do_mdm_init) bmp = 1; /* alternate bitmap */ #endif trab_vfd (bmp); #endif /* CONFIG_VFD && VFD_TEST_LOGO */
So if we move the call to au_tftp() someplace below, then when both CONFIG_VFD and VFD_TEST_LOGO are defined, we'll have a call to trab_vfd(), which will happen before the software update.
Note that there are several other cases in the main_loop() where conditionally included code introduces declarations intermixed with instructions. If this issue is to be cleaned-up, then let's have it done as a separate patch.
Regards, Bartlomiej Sieka
Dear Bartek,
in message 48DB48F5.3010204@semihalf.com you wrote:
I think "auto-update" is not a good name (especially since it has a different meaning than the similar sounding "autoload"0; also there is a typo in "sofware".
But most of all - do we really need a new environment variable? What's wrong with our good old "bootfile" ?
The only concern here is the interaction with bootp and dhcp commands -- they will set the "bootfile" env. variable to the file name got from the server, and the next time around U-Boot will try to use that file name to get the update. So I'd rather stick with a separate env. variable for the name of the update file.
I see. Maybe we should call the variable "updatefile" or similar, then?
How could the code be extended to be usable for NAND flash / DataFlash / OneNAND / IDE storage devices as well?
Primarily, FIT specification for the update file will have to be extended. Current approach is able to handle a series of <data, address> pairs, where the address is enough for U-Boot to tell where the data should go. If we are to handle other storage types, we need to specify which storage type the data should go to, and also provide a type-specific location. This is somewhat akin to the way we access and boot from these storage types: we use type-specific commands (nand, nboot, ide, diskboot, etc.).
Also, it would be of course nice to have a framework within U-Boot for generic data copying between storage types, that would hide all the type-specific details and provide uniform interface.
Agreed. You want devices and a mount command, I think ;-)
Is anybody on the list volunteering to check what could be lifted from the V2 code?
@@ -290,6 +293,10 @@ void main_loop (void) char bcs_set[16]; #endif /* CONFIG_BOOTCOUNT_LIMIT */
+#if defined(CONFIG_AU_TFTP)
- au_tftp ();
+#endif /* CONFIG_AU_TFTP */
#if defined(CONFIG_VFD) && defined(VFD_TEST_LOGO) ulong bmp = 0; /* default bitmap */ extern int trab_vfd (ulong bitmap);
You definitely don't want to add the function call right in the middle of variable declarations, or do you?
The idea is to have au_tftp() called as early as possible, before any other functions in main_loop().
Yes, but this is C, not C++, so declarations go only at the bginning of the function, then follows code (and no more declarations unless you open a new block).
So if we move the call to au_tftp() someplace below, then when both CONFIG_VFD and VFD_TEST_LOGO are defined, we'll have a call to trab_vfd(), which will happen before the software update.
So you will either have to add some more #ifdef's, or think what could happen if the VFD (Vacuum Fluorescent Display) initialization code runs first - I would not expect any negative impact?
Note that there are several other cases in the main_loop() where conditionally included code introduces declarations intermixed with instructions. If this issue is to be cleaned-up, then let's have it done as a separate patch.
Are there? I don't see any below these lines:
293 #if defined(CONFIG_VFD) && defined(VFD_TEST_LOGO) 294 ulong bmp = 0; /* default bitmap */ 295 extern int trab_vfd (ulong bitmap); 296 297 #ifdef CONFIG_MODEM_SUPPORT
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Bartek,
in message 48DB48F5.3010204@semihalf.com you wrote:
I think "auto-update" is not a good name (especially since it has a different meaning than the similar sounding "autoload"0; also there is a typo in "sofware".
But most of all - do we really need a new environment variable? What's wrong with our good old "bootfile" ?
The only concern here is the interaction with bootp and dhcp commands -- they will set the "bootfile" env. variable to the file name got from the server, and the next time around U-Boot will try to use that file name to get the update. So I'd rather stick with a separate env. variable for the name of the update file.
I see. Maybe we should call the variable "updatefile" or similar, then?
How about "au_file"? "updatefile" suffers from similarity to the commonly used (although not documented) "update" env. variable. But I'm fine either way, just let me know what you prefer.
[...]
@@ -290,6 +293,10 @@ void main_loop (void) char bcs_set[16]; #endif /* CONFIG_BOOTCOUNT_LIMIT */
+#if defined(CONFIG_AU_TFTP)
- au_tftp ();
+#endif /* CONFIG_AU_TFTP */
#if defined(CONFIG_VFD) && defined(VFD_TEST_LOGO) ulong bmp = 0; /* default bitmap */ extern int trab_vfd (ulong bitmap);
You definitely don't want to add the function call right in the middle of variable declarations, or do you?
The idea is to have au_tftp() called as early as possible, before any other functions in main_loop().
Yes, but this is C, not C++, so declarations go only at the bginning of the function, then follows code (and no more declarations unless you open a new block).
So if we move the call to au_tftp() someplace below, then when both CONFIG_VFD and VFD_TEST_LOGO are defined, we'll have a call to trab_vfd(), which will happen before the software update.
So you will either have to add some more #ifdef's, or think what could happen if the VFD (Vacuum Fluorescent Display) initialization code runs first - I would not expect any negative impact?
I don't think there will be any negative impact per se, the update procedure will just start a bit later (the time needed to display the bitmap on the FVD).
Will move au_tftp() call till after the VFD stuff.
Regards, Bartlomiej Sieka
Dear Bartlomiej Sieka,
In message 48DB5EC3.8000604@semihalf.com you wrote:
I see. Maybe we should call the variable "updatefile" or similar, then?
How about "au_file"? "updatefile" suffers from similarity to the commonly used (although not documented) "update" env. variable. But I'm fine either way, just let me know what you prefer.
I would use "updatefile", but if you prefer "au_file" (and nobody else comes up with comments or better suggestions) that's fine with me, too.
So you will either have to add some more #ifdef's, or think what could happen if the VFD (Vacuum Fluorescent Display) initialization code runs first - I would not expect any negative impact?
I don't think there will be any negative impact per se, the update procedure will just start a bit later (the time needed to display the bitmap on the FVD).
Actually that would prebably be wanted, as the end user would see that the system was powered on [and actually we don't really have to care much, as the TRAB board has reached EOL officially].
Will move au_tftp() call till after the VFD stuff.
Thanks.
Best regards,
Wolfgang Denk
Hi,
I see. Maybe we should call the variable "updatefile" or similar, then?
How about "au_file"? "updatefile" suffers from similarity to the commonly used (although not documented) "update" env. variable. But I'm fine either way, just let me know what you prefer.
I would use "updatefile", but if you prefer "au_file" (and nobody else comes up with comments or better suggestions) that's fine with me, too.
I second 'updatefile'. au_file sounds too much like noise, I mean audio ;)
Cheers Detlev
On Thu, Sep 25, 2008 at 3:16 AM, Bartlomiej Sieka tur@semihalf.com wrote:
More detailed description can be found in doc/README.au_tftp
'au' as a prefix seems awfully terse and cryptic to me (not to mention reminding me of Australians and gold), something a bit longer would go a long way to making it clearer what the code does.
On Thu, 25 Sep 2008 10:33:30 -0500 "Andrew Dyer" amdyer@gmail.com wrote:
On Thu, Sep 25, 2008 at 3:16 AM, Bartlomiej Sieka tur@semihalf.com wrote:
More detailed description can be found in doc/README.au_tftp
'au' as a prefix seems awfully terse and cryptic to me (not to mention reminding me of Australians and gold), something a bit longer would go a long way to making it clearer what the code does.
agreed. autoupdate_tftp?
Kim
Kim Phillips wrote:
On Thu, 25 Sep 2008 10:33:30 -0500 "Andrew Dyer" amdyer@gmail.com wrote:
On Thu, Sep 25, 2008 at 3:16 AM, Bartlomiej Sieka tur@semihalf.com wrote:
More detailed description can be found in doc/README.au_tftp
'au' as a prefix seems awfully terse and cryptic to me (not to mention reminding me of Australians and gold), something a bit longer would go a long way to making it clearer what the code does.
agreed. autoupdate_tftp?
Kim
+1 :-)
gvb
Dear Kim,
In message 20080925112855.21f8e87d.kim.phillips@freescale.com you wrote:
On Thu, Sep 25, 2008 at 3:16 AM, Bartlomiej Sieka tur@semihalf.com wrote:
More detailed description can be found in doc/README.au_tftp
'au' as a prefix seems awfully terse and cryptic to me (not to mention reminding me of Australians and gold), something a bit longer would go a long way to making it clearer what the code does.
agreed. autoupdate_tftp?
I think we should have a "*file" name so it's clear that this is a file name and not - for example - a logical variable that takes "yes"/"no" value or similar.
Best regards,
Wolfgang Denk
On Thu, 25 Sep 2008 20:17:30 +0200 Wolfgang Denk wd@denx.de wrote:
Dear Kim,
In message 20080925112855.21f8e87d.kim.phillips@freescale.com you wrote:
On Thu, Sep 25, 2008 at 3:16 AM, Bartlomiej Sieka tur@semihalf.com wrote:
More detailed description can be found in doc/README.au_tftp
'au' as a prefix seems awfully terse and cryptic to me (not to mention reminding me of Australians and gold), something a bit longer would go a long way to making it clearer what the code does.
agreed. autoupdate_tftp?
I think we should have a "*file" name so it's clear that this is a file name and not - for example - a logical variable that takes "yes"/"no" value or similar.
yes, then we'd be consistent with bootfile, fdtfile, etc.
Kim
Kim Phillips wrote:
On Thu, 25 Sep 2008 20:17:30 +0200 Wolfgang Denk wd@denx.de wrote:
Dear Kim,
In message 20080925112855.21f8e87d.kim.phillips@freescale.com you wrote:
On Thu, Sep 25, 2008 at 3:16 AM, Bartlomiej Sieka tur@semihalf.com wrote:
> More detailed description can be found in doc/README.au_tftp
'au' as a prefix seems awfully terse and cryptic to me (not to mention reminding me of Australians and gold), something a bit longer would go a long way to making it clearer what the code does.
agreed. autoupdate_tftp?
I think we should have a "*file" name so it's clear that this is a file name and not - for example - a logical variable that takes "yes"/"no" value or similar.
yes, then we'd be consistent with bootfile, fdtfile, etc.
How about 'updatefile' for the env. variable and 'README.update' for the documentation?
Regards, Bartlomiej Sieka
Dear Bartlomiej Sieka,
In message 48DC9882.4030402@semihalf.com you wrote:
How about 'updatefile' for the env. variable and 'README.update' for the documentation?
ACK from me (this is doc/README.update, right?)
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Bartlomiej Sieka,
In message 48DC9882.4030402@semihalf.com you wrote:
How about 'updatefile' for the env. variable and 'README.update' for the documentation?
ACK from me (this is doc/README.update, right?)
Yes, doc/README.update.
Regards, Bartlomiej Sieka
Now that the auto-update feature uses the 'firmware' type for updates, it is useful to inspect the load address of such images.
Signed-off-by: Bartlomiej Sieka tur@semihalf.com --- common/image.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/common/image.c b/common/image.c index dc8d7dd..0b7bd8d 100644 --- a/common/image.c +++ b/common/image.c @@ -1852,7 +1852,10 @@ void fit_print_contents (const void *fit) * @p: pointer to prefix string * * fit_image_print() lists all mandatory properies for the processed component - * image. If present, hash nodes are printed out as well. + * image. If present, hash nodes are printed out as well. Load + * address for images of type firmware is also printed out. Since the load + * address is not mandatory for firmware images, it will be output as + * "unavailable" when not present. * * returns: * no returned results @@ -1911,14 +1914,17 @@ void fit_image_print (const void *fit, int image_noffset, const char *p) printf ("%s OS: %s\n", p, genimg_get_os_name (os)); }
- if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE)) { + if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE) || + (type == IH_TYPE_FIRMWARE)) { ret = fit_image_get_load (fit, image_noffset, &load); printf ("%s Load Address: ", p); if (ret) printf ("unavailable\n"); else printf ("0x%08lx\n", load); + }
+ if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE)) { fit_image_get_entry (fit, image_noffset, &entry); printf ("%s Entry Point: ", p); if (ret) @@ -2844,7 +2850,7 @@ int fit_check_format (const void *fit) #if defined(CONFIG_TIMESTAMP) || defined(CONFIG_CMD_DATE) || defined(USE_HOSTCC) /* mandatory / node 'timestamp' property */ if (fdt_getprop (fit, 0, FIT_TIMESTAMP_PROP, NULL) == NULL) { - debug ("Wrong FIT format: no description\n"); + debug ("Wrong FIT format: no timestamp\n"); return 0; } #endif
Hello,
This is the second version of the patch series adding support for automatic software updates from a TFTP server. V2 adds millisecond granularity of the TFTP timeouts and addresses comments posted to the ML for the first version.
First three patches make some general changes required by the new feature, first one in the Flash subsystem, second and third in the networking code. Then follows a minor clean-up, with the feature proper in fifth patch. Last patch provides minor enhancements to the iminfo output that can become handy when using the new feature.
Please see newly added doc/README.update for more details.
Code has been compile-tested on several ppc, arm and mips targets. The feature itself has been tested on TQM8555 and MPC8555CDS and trab boards.
Note that the second patch might introduce networking issues for boards not conforming to the following two mandatory requirements: - CFG_HZ should be set to 1000 - get_timer() should return milliseconds
An example of such a non-conformant board is the trab, which when run with the networking changes mentioned above indeed exhibits timeouts during TFTP transfers. Also, the default timeouts of the auto-update feature have to be significantly increased for the trab target (CONFIG_UPDATE_TFTP_MSEC_MAX set to 1000000, instead of the default 100).
Regards, Bartlomiej Sieka
Dear Bartek,
in message 1222867592-58285-1-git-send-email-tur@semihalf.com you wrote:
This is the second version of the patch series adding support for automatic software updates from a TFTP server. V2 adds millisecond granularity of the TFTP timeouts and addresses comments posted to the ML for the first version.
First three patches make some general changes required by the new feature, first one in the Flash subsystem, second and third in the networking code. Then follows a minor clean-up, with the feature proper in fifth patch. Last patch provides minor enhancements to the iminfo output that can become handy when using the new feature.
Please see newly added doc/README.update for more details.
Code has been compile-tested on several ppc, arm and mips targets. The feature itself has been tested on TQM8555 and MPC8555CDS and trab boards.
All patches have been checked in into the "next" branch of the U-Boot repository. Can you please check if everything is correct in place and working as expected?
Thanks in advance.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Bartek,
in message 1222867592-58285-1-git-send-email-tur@semihalf.com you wrote:
This is the second version of the patch series adding support for automatic software updates from a TFTP server. V2 adds millisecond granularity of the TFTP timeouts and addresses comments posted to the ML for the first version.
First three patches make some general changes required by the new feature, first one in the Flash subsystem, second and third in the networking code. Then follows a minor clean-up, with the feature proper in fifth patch. Last patch provides minor enhancements to the iminfo output that can become handy when using the new feature.
Please see newly added doc/README.update for more details.
Code has been compile-tested on several ppc, arm and mips targets. The feature itself has been tested on TQM8555 and MPC8555CDS and trab boards.
All patches have been checked in into the "next" branch of the U-Boot repository. Can you please check if everything is correct in place and working as expected?
Hi Wolfgang,
Seems that the bulk of the feature didn't make it into the 'next' branch. Commit 'Automatic software update from TFTP server' on the branch (b830abed) shows only the following files modified:
common/Makefile
Whereas the patch posted to the list ('[PATCH v2 5/6] Automatic software update from TFTP server') has the following diffstat:
README | 12 ++ common/Makefile | 1 + common/main.c | 7 + common/update.c | 315 +++++++++++++++++++++++++++++++++++++++ doc/README.update | 90 +++++++++++ doc/uImage.FIT/update3.its | 41 +++++ doc/uImage.FIT/update_uboot.its | 21 +++ 7 files changed, 487 insertions(+), 0 deletions(-) create mode 100644 common/update.c create mode 100644 doc/README.update create mode 100644 doc/uImage.FIT/update3.its create mode 100644 doc/uImage.FIT/update_uboot.its
The above changes and newly added files associated with the feature are not present in the tree.
Other commits needed for the auto-update feature are present on the 'next' branch and look OK.
Regards, Bartlomiej Sieka
Dear Bartek,
in message 48F493C4.30404@semihalf.com you wrote:
Seems that the bulk of the feature didn't make it into the 'next' branch. Commit 'Automatic software update from TFTP server' on the branch (b830abed) shows only the following files modified:
Very strange.
Could you then please send just an updated "Automatic software update from TFTP server" patch that applies cleanly on top of "flash: factor out adjusting of Flash address to the end of sector" (= commit id 47508696d7984370869364446c3d937518869e97 in the next branch) ?
I will then rewind the next branch and fix this.
Best regards,
Wolfgang Denk
Dear Bartlomiej,
in message 48F493C4.30404@semihalf.com you wrote:
Seems that the bulk of the feature didn't make it into the 'next' branch. Commit 'Automatic software update from TFTP server' on the branch (b830abed) shows only the following files modified:
common/Makefile
Whereas the patch posted to the list ('[PATCH v2 5/6] Automatic software update from TFTP server') has the following diffstat:
README | 12 ++ common/Makefile | 1 + common/main.c | 7 + common/update.c | 315 +++++++++++++++++++++++++++++++++++++++ doc/README.update | 90 +++++++++++ doc/uImage.FIT/update3.its | 41 +++++ doc/uImage.FIT/update_uboot.its | 21 +++ 7 files changed, 487 insertions(+), 0 deletions(-) create mode 100644 common/update.c create mode 100644 doc/README.update create mode 100644 doc/uImage.FIT/update3.its create mode 100644 doc/uImage.FIT/update_uboot.its
The above changes and newly added files associated with the feature are not present in the tree.
Other commits needed for the auto-update feature are present on the 'next' branch and look OK.
I think I understand what happened, and fixed it.
Please check again.
Thanks.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Bartlomiej,
in message 48F493C4.30404@semihalf.com you wrote:
Seems that the bulk of the feature didn't make it into the 'next' branch. Commit 'Automatic software update from TFTP server' on the branch (b830abed) shows only the following files modified:
common/Makefile
Whereas the patch posted to the list ('[PATCH v2 5/6] Automatic software update from TFTP server') has the following diffstat:
README | 12 ++ common/Makefile | 1 + common/main.c | 7 + common/update.c | 315 +++++++++++++++++++++++++++++++++++++++ doc/README.update | 90 +++++++++++ doc/uImage.FIT/update3.its | 41 +++++ doc/uImage.FIT/update_uboot.its | 21 +++ 7 files changed, 487 insertions(+), 0 deletions(-) create mode 100644 common/update.c create mode 100644 doc/README.update create mode 100644 doc/uImage.FIT/update3.its create mode 100644 doc/uImage.FIT/update_uboot.its
The above changes and newly added files associated with the feature are not present in the tree.
Other commits needed for the auto-update feature are present on the 'next' branch and look OK.
I think I understand what happened, and fixed it.
Please check again.
Checked -- the 'next' branch has all the commits needed for the auto-update feature, and the feature itself works as expected.
Regards, Bartlomiej Sieka
The upcoming automatic update feature needs the ability to adjust an address within Flash to the end of its respective sector. Factor out this functionality to a new function flash_sect_roundb().
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com --- common/cmd_flash.c | 75 +++++++++++++++++++++++++++++----------------------- include/flash.h | 1 + 2 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/common/cmd_flash.c b/common/cmd_flash.c index 18d2250..29e5b6d 100644 --- a/common/cmd_flash.c +++ b/common/cmd_flash.c @@ -105,6 +105,47 @@ abbrev_spec (char *str, flash_info_t ** pinfo, int *psf, int *psl) }
/* + * Take *addr in Flash and adjust it to fall on the end of its sector + */ +int flash_sect_roundb (ulong *addr) +{ + flash_info_t *info; + ulong bank, sector_end_addr; + char found; + int i; + + /* find the end addr of the sector where the *addr is */ + found = 0; + for (bank = 0; bank < CFG_MAX_FLASH_BANKS && !found; ++bank) { + info = &flash_info[bank]; + for (i = 0; i < info->sector_count && !found; ++i) { + /* get the end address of the sector */ + if (i == info->sector_count - 1) { + sector_end_addr = info->start[0] + + info->size - 1; + } else { + sector_end_addr = info->start[i+1] - 1; + } + + if (*addr <= sector_end_addr && + *addr >= info->start[i]) { + found = 1; + /* adjust *addr if necessary */ + if (*addr < sector_end_addr) + *addr = sector_end_addr; + } /* sector */ + } /* bank */ + } + if (!found) { + /* error, addres not in flash */ + printf("Error: end address (0x%08lx) not in flash!\n", *addr); + return 1; + } + + return 0; +} + +/* * This function computes the start and end addresses for both * erase and protect commands. The range of the addresses on which * either of the commands is to operate can be given in two forms: @@ -126,8 +167,6 @@ addr_spec(char *arg1, char *arg2, ulong *addr_first, ulong *addr_last) { char *ep; char len_used; /* indicates if the "start +length" form used */ - char found; - ulong bank;
*addr_first = simple_strtoul(arg1, &ep, 16); if (ep == arg1 || *ep != '\0') @@ -157,38 +196,8 @@ addr_spec(char *arg1, char *arg2, ulong *addr_first, ulong *addr_last) * sector boundary, so that the commands don't fail later on. */
- /* find the end addr of the sector where the *addr_last is */ - found = 0; - for (bank = 0; bank < CFG_MAX_FLASH_BANKS && !found; ++bank){ - int i; - flash_info_t *info = &flash_info[bank]; - for (i = 0; i < info->sector_count && !found; ++i){ - /* get the end address of the sector */ - ulong sector_end_addr; - if (i == info->sector_count - 1){ - sector_end_addr = - info->start[0] + info->size - 1; - } else { - sector_end_addr = - info->start[i+1] - 1; - } - if (*addr_last <= sector_end_addr && - *addr_last >= info->start[i]){ - /* sector found */ - found = 1; - /* adjust *addr_last if necessary */ - if (*addr_last < sector_end_addr){ - *addr_last = sector_end_addr; - } - } - } /* sector */ - } /* bank */ - if (!found){ - /* error, addres not in flash */ - printf("Error: end address (0x%08lx) not in flash!\n", - *addr_last); + if (flash_sect_roundb(addr_last) > 0) return -1; - } } /* "start +length" from used */
return 1; diff --git a/include/flash.h b/include/flash.h index af8a7c0..6f5d7d5 100644 --- a/include/flash.h +++ b/include/flash.h @@ -91,6 +91,7 @@ extern void flash_print_info (flash_info_t *); extern int flash_erase (flash_info_t *, int, int); extern int flash_sect_erase (ulong addr_first, ulong addr_last); extern int flash_sect_protect (int flag, ulong addr_first, ulong addr_last); +extern int flash_sect_roundb (ulong *addr);
/* common/flash.c */ extern void flash_protect (int flag, ulong from, ulong to, flash_info_t *info);
Hello Stefan,
Do you have any comments on the below patch?
Regards, Bartlomiej Sieka
Bartlomiej Sieka wrote:
The upcoming automatic update feature needs the ability to adjust an address within Flash to the end of its respective sector. Factor out this functionality to a new function flash_sect_roundb().
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com
common/cmd_flash.c | 75 +++++++++++++++++++++++++++++----------------------- include/flash.h | 1 + 2 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/common/cmd_flash.c b/common/cmd_flash.c index 18d2250..29e5b6d 100644 --- a/common/cmd_flash.c +++ b/common/cmd_flash.c @@ -105,6 +105,47 @@ abbrev_spec (char *str, flash_info_t ** pinfo, int *psf, int *psl) }
/*
- Take *addr in Flash and adjust it to fall on the end of its sector
- */
+int flash_sect_roundb (ulong *addr) +{
- flash_info_t *info;
- ulong bank, sector_end_addr;
- char found;
- int i;
- /* find the end addr of the sector where the *addr is */
- found = 0;
- for (bank = 0; bank < CFG_MAX_FLASH_BANKS && !found; ++bank) {
info = &flash_info[bank];for (i = 0; i < info->sector_count && !found; ++i) {/* get the end address of the sector */if (i == info->sector_count - 1) {sector_end_addr = info->start[0] +info->size - 1;} else {sector_end_addr = info->start[i+1] - 1;}if (*addr <= sector_end_addr &&*addr >= info->start[i]) {found = 1;/* adjust *addr if necessary */if (*addr < sector_end_addr)*addr = sector_end_addr;} /* sector */} /* bank */- }
- if (!found) {
/* error, addres not in flash */printf("Error: end address (0x%08lx) not in flash!\n", *addr);return 1;- }
- return 0;
+}
+/*
- This function computes the start and end addresses for both
- erase and protect commands. The range of the addresses on which
- either of the commands is to operate can be given in two forms:
@@ -126,8 +167,6 @@ addr_spec(char *arg1, char *arg2, ulong *addr_first, ulong *addr_last) { char *ep; char len_used; /* indicates if the "start +length" form used */
char found;
ulong bank;
*addr_first = simple_strtoul(arg1, &ep, 16); if (ep == arg1 || *ep != '\0')
@@ -157,38 +196,8 @@ addr_spec(char *arg1, char *arg2, ulong *addr_first, ulong *addr_last) * sector boundary, so that the commands don't fail later on. */
/* find the end addr of the sector where the *addr_last is */found = 0;for (bank = 0; bank < CFG_MAX_FLASH_BANKS && !found; ++bank){int i;flash_info_t *info = &flash_info[bank];for (i = 0; i < info->sector_count && !found; ++i){/* get the end address of the sector */ulong sector_end_addr;if (i == info->sector_count - 1){sector_end_addr =info->start[0] + info->size - 1;} else {sector_end_addr =info->start[i+1] - 1;}if (*addr_last <= sector_end_addr &&*addr_last >= info->start[i]){/* sector found */found = 1;/* adjust *addr_last if necessary */if (*addr_last < sector_end_addr){*addr_last = sector_end_addr;}}} /* sector */} /* bank */if (!found){/* error, addres not in flash */printf("Error: end address (0x%08lx) not in flash!\n",*addr_last);
if (flash_sect_roundb(addr_last) > 0) return -1;
}} /* "start +length" from used */
return 1;
diff --git a/include/flash.h b/include/flash.h index af8a7c0..6f5d7d5 100644 --- a/include/flash.h +++ b/include/flash.h @@ -91,6 +91,7 @@ extern void flash_print_info (flash_info_t *); extern int flash_erase (flash_info_t *, int, int); extern int flash_sect_erase (ulong addr_first, ulong addr_last); extern int flash_sect_protect (int flag, ulong addr_first, ulong addr_last); +extern int flash_sect_roundb (ulong *addr);
/* common/flash.c */ extern void flash_protect (int flag, ulong from, ulong to, flash_info_t *info);
Hi Bartlomiej,
On Wednesday 08 October 2008, Bartlomiej Sieka wrote:
Do you have any comments on the below patch?
Looks good to me. So:
Acked-by: Stefan Roese sr@denx.de
Not sure if I am responsible for picking this up or if this should go in through Wolfgang. It's not really cfi-flash related. But if nobody objects then I'll put it into the next branch of my cfi-flash repository.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Stefan Roese wrote:
Hi Bartlomiej,
On Wednesday 08 October 2008, Bartlomiej Sieka wrote:
Do you have any comments on the below patch?
Looks good to me. So:
Thanks for prompt response.
Acked-by: Stefan Roese sr@denx.de
Not sure if I am responsible for picking this up or if this should go in through Wolfgang. It's not really cfi-flash related. But if nobody objects then I'll put it into the next branch of my cfi-flash repository.
It depends on how/when Wolfgang wants to handle the inclusion of the automatic update feature (patches 4-6/6). This feature depends on the flash patch in question (1/6), as well as on two networking changes (patches 2/6 and 3/6), which have been pulled into the 'next' branch of the net repository already.
Regards, Bartlomiej Sieka
Dear Bartek,
in message 48EC69E6.2010200@semihalf.com you wrote:
Not sure if I am responsible for picking this up or if this should go in through Wolfgang. It's not really cfi-flash related. But if nobody objects then I'll put it into the next branch of my cfi-flash repository.
It depends on how/when Wolfgang wants to handle the inclusion of the automatic update feature (patches 4-6/6). This feature depends on the flash patch in question (1/6), as well as on two networking changes (patches 2/6 and 3/6), which have been pulled into the 'next' branch of the net repository already.
If eveybody co-operates (Stefan for the flash part, Ben for the network part), I would like to pull this into my "next" branch.
Stefan, Ben - please let me know when your next branches are ready for pulling, so I can coordinate this.
Thanks.
Best regards,
Wolfgang Denk
On Wednesday 08 October 2008, Wolfgang Denk wrote:
Not sure if I am responsible for picking this up or if this should go in through Wolfgang. It's not really cfi-flash related. But if nobody objects then I'll put it into the next branch of my cfi-flash repository.
It depends on how/when Wolfgang wants to handle the inclusion of the automatic update feature (patches 4-6/6). This feature depends on the flash patch in question (1/6), as well as on two networking changes (patches 2/6 and 3/6), which have been pulled into the 'next' branch of the net repository already.
If eveybody co-operates (Stefan for the flash part, Ben for the network part), I would like to pull this into my "next" branch.
Stefan, Ben - please let me know when your next branches are ready for pulling, so I can coordinate this.
OK, I added this patch [1/6] to the u-boot-cfi-flash next branch. Here the pull request for the *next* branch:
The following changes since commit 8fd4166c467a46773f80208bda1ec3b4757747bc: Stefan Roese (1): ppc4xx: Canyonlands: Remove unnecessary FDT warning upon DTB fixup
are available in the git repository at:
git://www.denx.de/git/u-boot-cfi-flash.git next
Bartlomiej Sieka (1): flash: factor out adjusting of Flash address to the end of sector
common/cmd_flash.c | 75 +++++++++++++++++++++++++++++----------------------- include/flash.h | 1 + 2 files changed, 43 insertions(+), 33 deletions(-)
Thanks.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Dear Stefan Roese,
In message 200810081402.00345.sr@denx.de you wrote:
OK, I added this patch [1/6] to the u-boot-cfi-flash next branch. Here the pull request for the *next* branch:
The following changes since commit 8fd4166c467a46773f80208bda1ec3b4757747bc: Stefan Roese (1): ppc4xx: Canyonlands: Remove unnecessary FDT warning upon DTB fixup
are available in the git repository at:
git://www.denx.de/git/u-boot-cfi-flash.git next
Bartlomiej Sieka (1): flash: factor out adjusting of Flash address to the end of sector
common/cmd_flash.c | 75 +++++++++++++++++++++++++++++----------------------- include/flash.h | 1 + 2 files changed, 43 insertions(+), 33 deletions(-)
Thanks. Applied to "next" branch.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Bartek,
in message 48EC69E6.2010200@semihalf.com you wrote:
Not sure if I am responsible for picking this up or if this should go in through Wolfgang. It's not really cfi-flash related. But if nobody objects then I'll put it into the next branch of my cfi-flash repository.
It depends on how/when Wolfgang wants to handle the inclusion of the automatic update feature (patches 4-6/6). This feature depends on the flash patch in question (1/6), as well as on two networking changes (patches 2/6 and 3/6), which have been pulled into the 'next' branch of the net repository already.
If eveybody co-operates (Stefan for the flash part, Ben for the network part), I would like to pull this into my "next" branch.
Stefan, Ben - please let me know when your next branches are ready for pulling, so I can coordinate this.
Thanks.
Best regards,
Wolfgang Denk
You can pull net/next now if you'd like. I'm not able to generate a pull request from here, so if you need that you'll have to wait maybe 10 or 11 hours.
regards, Ben
Dear Ben Warren,
In message 48ECF027.3050706@gmail.com you wrote:
You can pull net/next now if you'd like. I'm not able to generate a pull request from here, so if you need that you'll have to wait maybe 10 or 11 hours.
Done. Pulled into "next" branch.
Thanks.
Best regards,
Wolfgang Denk
Enforce millisecond semantics of the first argument to NetSetTimeout() -- the change is transparent for well-behaving boards (CFG_HZ == 1000 and get_timer() countiing in milliseconds).
Rationale for this patch is to enable millisecond granularity for network-related timeouts, which is needed for the upcoming automatic software update feature.
Summary of changes: - do not scale the first argument to NetSetTimeout() by CFG_HZ - change timeout values used in the networking code to milliseconds
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com --- net/bootp.c | 14 +++++++------- net/bootp.h | 2 +- net/net.c | 19 +++++++------------ net/nfs.c | 6 +++--- net/rarp.c | 6 +++--- net/sntp.c | 4 ++-- net/tftp.c | 10 +++++----- 7 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/net/bootp.c b/net/bootp.c index 64552ac..c2078c6 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -33,7 +33,7 @@
#if defined(CONFIG_CMD_NET)
-#define TIMEOUT 5UL /* Seconds before trying BOOTP again */ +#define TIMEOUT 5000UL /* Milliseconds before trying BOOTP again */ #ifndef CONFIG_NET_RETRY_COUNT # define TIMEOUT_COUNT 5 /* # of timeouts before giving up */ #else @@ -371,7 +371,7 @@ BootpTimeout(void) puts ("\nRetry count exceeded; starting again\n"); NetStartAgain (); } else { - NetSetTimeout (TIMEOUT * CFG_HZ, BootpTimeout); + NetSetTimeout (TIMEOUT, BootpTimeout); BootpRequest (); } } @@ -671,7 +671,7 @@ BootpRequest (void) bp->bp_htype = HWT_ETHER; bp->bp_hlen = HWL_ETHER; bp->bp_hops = 0; - bp->bp_secs = htons(get_timer(0) / CFG_HZ); + bp->bp_secs = htons(get_timer(0) / 1000); NetWriteIP(&bp->bp_ciaddr, 0); NetWriteIP(&bp->bp_yiaddr, 0); NetWriteIP(&bp->bp_siaddr, 0); @@ -688,7 +688,7 @@ BootpRequest (void)
/* * Bootp ID is the lower 4 bytes of our ethernet address - * plus the current time in HZ. + * plus the current time in ms. */ BootpID = ((ulong)NetOurEther[2] << 24) | ((ulong)NetOurEther[3] << 16) @@ -705,7 +705,7 @@ BootpRequest (void) pktlen = BOOTP_SIZE - sizeof(bp->bp_vend) + ext_len; iplen = BOOTP_HDR_SIZE - sizeof(bp->bp_vend) + ext_len; NetSetIP(iphdr, 0xFFFFFFFFL, PORT_BOOTPS, PORT_BOOTPC, iplen); - NetSetTimeout(SELECT_TIMEOUT * CFG_HZ, BootpTimeout); + NetSetTimeout(SELECT_TIMEOUT, BootpTimeout);
#if defined(CONFIG_CMD_DHCP) dhcp_state = SELECTING; @@ -849,7 +849,7 @@ static void DhcpSendRequestPkt(Bootp_t *bp_offer) bp->bp_htype = HWT_ETHER; bp->bp_hlen = HWL_ETHER; bp->bp_hops = 0; - bp->bp_secs = htons(get_timer(0) / CFG_HZ); + bp->bp_secs = htons(get_timer(0) / 1000); /* Do not set the client IP, your IP, or server IP yet, since it hasn't been ACK'ed by * the server yet */
@@ -924,7 +924,7 @@ DhcpHandler(uchar * pkt, unsigned dest, unsigned src, unsigned len) if (NetReadLong((ulong*)&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC)) DhcpOptionsProcess((u8 *)&bp->bp_vend[4], bp);
- NetSetTimeout(TIMEOUT * CFG_HZ, BootpTimeout); + NetSetTimeout(TIMEOUT, BootpTimeout); DhcpSendRequestPkt(bp); #ifdef CFG_BOOTFILE_PREFIX } diff --git a/net/bootp.h b/net/bootp.h index c2f011c..50625ab 100644 --- a/net/bootp.h +++ b/net/bootp.h @@ -88,7 +88,7 @@ typedef enum { INIT, #define DHCP_NAK 6 #define DHCP_RELEASE 7
-#define SELECT_TIMEOUT 3UL /* Seconds to wait for offers */ +#define SELECT_TIMEOUT 3000UL /* Milliseconds to wait for offers */
/**********************************************************************/
diff --git a/net/net.c b/net/net.c index 313d5d8..80262b9 100644 --- a/net/net.c +++ b/net/net.c @@ -95,14 +95,9 @@ DECLARE_GLOBAL_DATA_PTR;
#ifndef CONFIG_ARP_TIMEOUT -# define ARP_TIMEOUT 50UL /* Deciseconds before trying ARP again */ -#elif (CONFIG_ARP_TIMEOUT < 100) -# error "Due to possible overflow CONFIG_ARP_TIMEOUT must be greater than 100ms" +# define ARP_TIMEOUT 5000UL /* Milliseconds before trying ARP again */ #else -# if (CONFIG_ARP_TIMEOUT % 100) -# warning "Supported ARP_TIMEOUT precision is 100ms" -# endif -# define ARP_TIMEOUT (CONFIG_ARP_TIMEOUT / 100) +# define ARP_TIMEOUT CONFIG_ARP_TIMEOUT #endif
@@ -264,7 +259,7 @@ void ArpTimeoutCheck(void) t = get_timer(0);
/* check for arp timeout */ - if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT * CFG_HZ / 10) { + if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT) { NetArpWaitTry++;
if (NetArpWaitTry >= ARP_TIMEOUT_COUNT) { @@ -603,7 +598,7 @@ void NetStartAgain (void) return; } #ifndef CONFIG_NET_MULTI - NetSetTimeout (10UL * CFG_HZ, startAgainTimeout); + NetSetTimeout (10000UL, startAgainTimeout); NetSetHandler (startAgainHandler); #else /* !CONFIG_NET_MULTI*/ eth_halt (); @@ -614,7 +609,7 @@ void NetStartAgain (void) if (NetRestartWrap) { NetRestartWrap = 0; if (NetDevExists && !once) { - NetSetTimeout (10UL * CFG_HZ, startAgainTimeout); + NetSetTimeout (10000UL, startAgainTimeout); NetSetHandler (startAgainHandler); } else { NetState = NETLOOP_FAIL; @@ -790,7 +785,7 @@ static void PingStart(void) #if defined(CONFIG_NET_MULTI) printf ("Using %s device\n", eth_get_name()); #endif /* CONFIG_NET_MULTI */ - NetSetTimeout (10UL * CFG_HZ, PingTimeout); + NetSetTimeout (10000UL, PingTimeout); NetSetHandler (PingHandler);
PingSend(); @@ -813,7 +808,7 @@ static void PingStart(void) #define CDP_SYSOBJECT_TLV 0x0015 #define CDP_MANAGEMENT_ADDRESS_TLV 0x0016
-#define CDP_TIMEOUT (CFG_HZ/4) /* one packet every 250ms */ +#define CDP_TIMEOUT 250UL /* one packet every 250ms */
static int CDPSeq; static int CDPOK; diff --git a/net/nfs.c b/net/nfs.c index 6573c17..0c8f08c 100644 --- a/net/nfs.c +++ b/net/nfs.c @@ -35,7 +35,7 @@
#define HASHES_PER_LINE 65 /* Number of "loading" hashes per line */ #define NFS_RETRY_COUNT 30 -#define NFS_TIMEOUT 2UL +#define NFS_TIMEOUT 2000UL
static int fs_mounted = 0; static unsigned long rpc_id = 0; @@ -674,7 +674,7 @@ NfsHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
case STATE_READ_REQ: rlen = nfs_read_reply (pkt, len); - NetSetTimeout (NFS_TIMEOUT * CFG_HZ, NfsTimeout); + NetSetTimeout (NFS_TIMEOUT, NfsTimeout); if (rlen > 0) { nfs_offset += rlen; NfsSend (); @@ -763,7 +763,7 @@ NfsStart (void) printf ("\nLoad address: 0x%lx\n" "Loading: *\b", load_addr);
- NetSetTimeout (NFS_TIMEOUT * CFG_HZ, NfsTimeout); + NetSetTimeout (NFS_TIMEOUT, NfsTimeout); NetSetHandler (NfsHandler);
NfsTimeoutCount = 0; diff --git a/net/rarp.c b/net/rarp.c index ecf38e4..7105696 100644 --- a/net/rarp.c +++ b/net/rarp.c @@ -31,7 +31,7 @@
#if defined(CONFIG_CMD_NET)
-#define TIMEOUT 5UL /* Seconds before trying BOOTP again */ +#define TIMEOUT 5000UL /* Milliseconds before trying BOOTP again */ #ifndef CONFIG_NET_RETRY_COUNT # define TIMEOUT_COUNT 5 /* # of timeouts before giving up */ #else @@ -80,7 +80,7 @@ RarpTimeout(void) puts ("\nRetry count exceeded; starting again\n"); NetStartAgain (); } else { - NetSetTimeout (TIMEOUT * CFG_HZ, RarpTimeout); + NetSetTimeout (TIMEOUT, RarpTimeout); RarpRequest (); } } @@ -115,7 +115,7 @@ RarpRequest (void)
NetSendPacket(NetTxPacket, (pkt - NetTxPacket) + ARP_HDR_SIZE);
- NetSetTimeout(TIMEOUT * CFG_HZ, RarpTimeout); + NetSetTimeout(TIMEOUT, RarpTimeout); NetSetHandler(RarpHandler); }
diff --git a/net/sntp.c b/net/sntp.c index 95e7542..425d35e 100644 --- a/net/sntp.c +++ b/net/sntp.c @@ -14,7 +14,7 @@
#if defined(CONFIG_CMD_NET) && defined(CONFIG_CMD_SNTP)
-#define SNTP_TIMEOUT 10 +#define SNTP_TIMEOUT 10000UL
static int SntpOurPort;
@@ -82,7 +82,7 @@ SntpStart (void) { debug ("%s\n", __FUNCTION__);
- NetSetTimeout (SNTP_TIMEOUT * CFG_HZ, SntpTimeout); + NetSetTimeout (SNTP_TIMEOUT, SntpTimeout); NetSetHandler(SntpHandler); memset (NetServerEther, 0, 6);
diff --git a/net/tftp.c b/net/tftp.c index 9aeecb8..3f0a516 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -15,7 +15,7 @@ #if defined(CONFIG_CMD_NET)
#define WELL_KNOWN_PORT 69 /* Well known TFTP port # */ -#define TIMEOUT 5UL /* Seconds to timeout for a lost pkt */ +#define TIMEOUT 5000UL /* Millisecs to timeout for lost pkt */ #ifndef CONFIG_NET_RETRY_COUNT # define TIMEOUT_COUNT 10 /* # of timeouts before giving up */ #else @@ -180,7 +180,7 @@ TftpSend (void) pkt += 5 /*strlen("octet")*/ + 1; strcpy ((char *)pkt, "timeout"); pkt += 7 /*strlen("timeout")*/ + 1; - sprintf((char *)pkt, "%lu", TIMEOUT); + sprintf((char *)pkt, "%lu", TIMEOUT / 1000); #ifdef ET_DEBUG printf("send option "timeout %s"\n", (char *)pkt); #endif @@ -370,7 +370,7 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) }
TftpLastBlock = TftpBlock; - NetSetTimeout (TIMEOUT * CFG_HZ, TftpTimeout); + NetSetTimeout (TIMEOUT, TftpTimeout);
store_block (TftpBlock - 1, pkt + 2, len);
@@ -449,7 +449,7 @@ TftpTimeout (void) NetStartAgain (); } else { puts ("T "); - NetSetTimeout (TIMEOUT * CFG_HZ, TftpTimeout); + NetSetTimeout (TIMEOUT, TftpTimeout); TftpSend (); } } @@ -520,7 +520,7 @@ TftpStart (void)
puts ("Loading: *\b");
- NetSetTimeout (TIMEOUT * CFG_HZ, TftpTimeout); + NetSetTimeout (TIMEOUT, TftpTimeout); NetSetHandler (TftpHandler);
TftpServerPort = WELL_KNOWN_PORT;
Bartlomiej Sieka wrote:
Enforce millisecond semantics of the first argument to NetSetTimeout() -- the change is transparent for well-behaving boards (CFG_HZ == 1000 and get_timer() countiing in milliseconds).
Rationale for this patch is to enable millisecond granularity for network-related timeouts, which is needed for the upcoming automatic software update feature.
Summary of changes:
- do not scale the first argument to NetSetTimeout() by CFG_HZ
- change timeout values used in the networking code to milliseconds
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com
Applied to net/next
regards, Ben
There are two aspects of a TFTP transfer involving timeouts: 1. timeout waiting for initial server reply after sending RRQ 2. timeouts while transferring actual data from the server
Since the upcoming auto-update feature attempts a TFTP download during each boot, it is undesirable to have a long delay when the TFTP server is not available. Thus, this commit makes the server timeout (1.) configurable by two global variables:
TftpRRQTimeoutMSecs TftpRRQTimeoutCountMax
TftpRRQTimeoutMSecs overrides default timeout when trying to connect to a TFTP server, TftpRRQTimeoutCountMax overrides default number of connection retries. The total delay when trying to download a file from a non-existing TFTP server is TftpRRQTimeoutMSecs x TftpRRQTimeoutCountMax milliseconds.
Timeouts during file transfers (2.) are unaffected.
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com --- net/tftp.c | 28 ++++++++++++++++++++++++---- 1 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/net/tftp.c b/net/tftp.c index 3f0a516..f7cc111 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -34,6 +34,21 @@ #define TFTP_ERROR 5 #define TFTP_OACK 6
+static ulong TftpTimeoutMSecs = TIMEOUT; +static int TftpTimeoutCountMax = TIMEOUT_COUNT; + +/* + * These globals govern the timeout behavior when attempting a connection to a + * TFTP server. TftpRRQTimeoutMSecs specifies the number of milliseconds to + * wait for the server to respond to initial connection. Second global, + * TftpRRQTimeoutCountMax, gives the number of such connection retries. + * TftpRRQTimeoutCountMax must be non-negative and TftpRRQTimeoutMSecs must be + * positive. The globals are meant to be set (and restored) by code needing + * non-standard timeout behavior when initiating a TFTP transfer. + */ +ulong TftpRRQTimeoutMSecs = TIMEOUT; +int TftpRRQTimeoutCountMax = TIMEOUT_COUNT; + static IPaddr_t TftpServerIP; static int TftpServerPort; /* The UDP port at their end */ static int TftpOurPort; /* The UDP port at our end */ @@ -370,7 +385,9 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) }
TftpLastBlock = TftpBlock; - NetSetTimeout (TIMEOUT, TftpTimeout); + TftpTimeoutMSecs = TIMEOUT; + TftpTimeoutCountMax = TIMEOUT_COUNT; + NetSetTimeout (TftpTimeoutMSecs, TftpTimeout);
store_block (TftpBlock - 1, pkt + 2, len);
@@ -441,7 +458,7 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) static void TftpTimeout (void) { - if (++TftpTimeoutCount > TIMEOUT_COUNT) { + if (++TftpTimeoutCount > TftpTimeoutCountMax) { puts ("\nRetry count exceeded; starting again\n"); #ifdef CONFIG_MCAST_TFTP mcast_cleanup(); @@ -449,7 +466,7 @@ TftpTimeout (void) NetStartAgain (); } else { puts ("T "); - NetSetTimeout (TIMEOUT, TftpTimeout); + NetSetTimeout (TftpTimeoutMSecs, TftpTimeout); TftpSend (); } } @@ -520,7 +537,10 @@ TftpStart (void)
puts ("Loading: *\b");
- NetSetTimeout (TIMEOUT, TftpTimeout); + TftpTimeoutMSecs = TftpRRQTimeoutMSecs; + TftpTimeoutCountMax = TftpRRQTimeoutCountMax; + + NetSetTimeout (TftpTimeoutMSecs, TftpTimeout); NetSetHandler (TftpHandler);
TftpServerPort = WELL_KNOWN_PORT;
Bartlomiej Sieka wrote:
There are two aspects of a TFTP transfer involving timeouts:
- timeout waiting for initial server reply after sending RRQ
- timeouts while transferring actual data from the server
Since the upcoming auto-update feature attempts a TFTP download during each boot, it is undesirable to have a long delay when the TFTP server is not available. Thus, this commit makes the server timeout (1.) configurable by two global variables:
TftpRRQTimeoutMSecs TftpRRQTimeoutCountMax
TftpRRQTimeoutMSecs overrides default timeout when trying to connect to a TFTP server, TftpRRQTimeoutCountMax overrides default number of connection retries. The total delay when trying to download a file from a non-existing TFTP server is TftpRRQTimeoutMSecs x TftpRRQTimeoutCountMax milliseconds.
Timeouts during file transfers (2.) are unaffected.
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com
Applied to net/next
regards, Ben
Signed-off-by: Bartlomiej Sieka tur@semihalf.com --- common/Makefile | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/Makefile b/common/Makefile index 8bddf8e..bb6655d 100644 --- a/common/Makefile +++ b/common/Makefile @@ -148,13 +148,13 @@ endif COBJS-$(CONFIG_CMD_XIMG) += cmd_ximg.o COBJS-$(CONFIG_YAFFS2) += cmd_yaffs2.o COBJS-$(CONFIG_VFD) += cmd_vfd.o +COBJS-$(CONFIG_DDR_SPD) += ddr_spd.o COBJS-$(CONFIG_CMD_DOC) += docecc.o COBJS-y += flash.o COBJS-y += kgdb.o COBJS-$(CONFIG_LCD) += lcd.o COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o -COBJS-$(CONFIG_DDR_SPD) += ddr_spd.o
COBJS := $(sort $(COBJS-y)) SRCS := $(AOBJS:.o=.S) $(COBJS:.o=.c)
On 15:26 Wed 01 Oct , Bartlomiej Sieka wrote:
Signed-off-by: Bartlomiej Sieka tur@semihalf.com
common/Makefile | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/Makefile b/common/Makefile index 8bddf8e..bb6655d 100644
Please base your patch on u-boot-arm/next
and I've plane to split it in a subsection to not have it in the middle of the command
Best Regards, J.
[dropped sr@denx.de and biggerbadderben@gmail.com from CC]
Hi Jean-Christophe,
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 15:26 Wed 01 Oct , Bartlomiej Sieka wrote:
Signed-off-by: Bartlomiej Sieka tur@semihalf.com
common/Makefile | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/Makefile b/common/Makefile index 8bddf8e..bb6655d 100644
Please base your patch on u-boot-arm/next
Sorry, I'm not sure here -- the patch makes a simple change to common/Makefile, so why would re-basing to arm/next be necessary?
and I've plane to split it in a subsection to not have it in the middle of the command
Is this comment related to the patch? Could you please elaborate?
Thanks, Bartlomiej Sieka
On 14:58 Wed 08 Oct , Bartlomiej Sieka wrote:
[dropped sr@denx.de and biggerbadderben@gmail.com from CC]
Hi Jean-Christophe,
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 15:26 Wed 01 Oct , Bartlomiej Sieka wrote:
Signed-off-by: Bartlomiej Sieka tur@semihalf.com
common/Makefile | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/Makefile b/common/Makefile index 8bddf8e..bb6655d 100644
Please base your patch on u-boot-arm/next
Sorry, I'm not sure here -- the patch makes a simple change to common/Makefile, so why would re-basing to arm/next be necessary?
and I've plane to split it in a subsection to not have it in the middle of the command
Is this comment related to the patch? Could you please elaborate?
I've split the Makefile in 4 subgroup # core # core command # environment # command
It will be good to regroup non command file to # others or something like this
Best Regards, J.
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 14:58 Wed 08 Oct , Bartlomiej Sieka wrote:
[dropped sr@denx.de and biggerbadderben@gmail.com from CC]
Hi Jean-Christophe,
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 15:26 Wed 01 Oct , Bartlomiej Sieka wrote:
Signed-off-by: Bartlomiej Sieka tur@semihalf.com
common/Makefile | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/Makefile b/common/Makefile index 8bddf8e..bb6655d 100644
Please base your patch on u-boot-arm/next
Sorry, I'm not sure here -- the patch makes a simple change to common/Makefile, so why would re-basing to arm/next be necessary?
and I've plane to split it in a subsection to not have it in the middle of the command
Is this comment related to the patch? Could you please elaborate?
I've split the Makefile in 4 subgroup # core # core command # environment # command
It will be good to regroup non command file to # others or something like this
Yes, this makes sense in general. So let's skip this patch (4/6) for now and have the auto-update feature merged first. Makefile clean-ups and rearrangements can be done later on.
Regards, Bartlomiej Sieka
The auto-update feature allows to automatically download software updates from a TFTP server and store them in Flash memory during boot. Updates are contained in a FIT file and protected with SHA-1 checksum.
More detailed description can be found in doc/README.update.
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com --- README | 12 ++ common/Makefile | 1 + common/main.c | 7 + common/update.c | 315 +++++++++++++++++++++++++++++++++++++++ doc/README.update | 90 +++++++++++ doc/uImage.FIT/update3.its | 41 +++++ doc/uImage.FIT/update_uboot.its | 21 +++ 7 files changed, 487 insertions(+), 0 deletions(-) create mode 100644 common/update.c create mode 100644 doc/README.update create mode 100644 doc/uImage.FIT/update3.its create mode 100644 doc/uImage.FIT/update_uboot.its
diff --git a/README b/README index ccd839c..0ff4467 100644 --- a/README +++ b/README @@ -1737,6 +1737,14 @@ The following options need to be configured: example, some LED's) on your board. At the moment, the following checkpoints are implemented:
+- Automatic software updates via TFTP server + CONFIG_UPDATE_TFTP + CONFIG_UPDATE_TFTP_CNT_MAX + CONFIG_UPDATE_TFTP_MSEC_MAX + + These options enable and control the auto-update feature; + for a more detailed description refer to doc/README.update. + Legacy uImage format:
Arg Where When @@ -2811,6 +2819,10 @@ Some configuration options can be set using Environment Variables: allowed for use by the bootm command. See also "bootm_low" environment variable.
+ updatefile - Location of the software update file on a TFTP server, used + by the automatic software update feature. Please refer to + documentation in doc/README.update for more details. + autoload - if set to "no" (any string beginning with 'n'), "bootp" will just load perform a lookup of the configuration from the BOOTP server, but not try to diff --git a/common/Makefile b/common/Makefile index bb6655d..28c65a9 100644 --- a/common/Makefile +++ b/common/Makefile @@ -154,6 +154,7 @@ COBJS-y += flash.o COBJS-y += kgdb.o COBJS-$(CONFIG_LCD) += lcd.o COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o +COBJS-$(CONFIG_UPDATE_TFTP) += update.o COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
COBJS := $(sort $(COBJS-y)) diff --git a/common/main.c b/common/main.c index 187ef8a..c06ea07 100644 --- a/common/main.c +++ b/common/main.c @@ -56,6 +56,9 @@ extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]); /* fo
extern int do_bootd (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
+#if defined(CONFIG_UPDATE_TFTP) +void update_tftp (void); +#endif /* CONFIG_UPDATE_TFTP */
#define MAX_DELAY_STOP_STR 32
@@ -301,6 +304,10 @@ void main_loop (void) trab_vfd (bmp); #endif /* CONFIG_VFD && VFD_TEST_LOGO */
+#if defined(CONFIG_UPDATE_TFTP) + update_tftp (); +#endif /* CONFIG_UPDATE_TFTP */ + #ifdef CONFIG_BOOTCOUNT_LIMIT bootcount = bootcount_load(); bootcount++; diff --git a/common/update.c b/common/update.c new file mode 100644 index 0000000..938cc06 --- /dev/null +++ b/common/update.c @@ -0,0 +1,315 @@ +/* + * (C) Copyright 2008 Semihalf + * + * Written by: Rafal Czubak rcz@semihalf.com + * Bartlomiej Sieka tur@semihalf.com + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + * + */ + +#include <common.h> + +#if !(defined(CONFIG_FIT) && defined(CONFIG_OF_LIBFDT)) +#error "CONFIG_FIT and CONFIG_OF_LIBFDT are required for auto-update feature" +#endif + +#if defined(CFG_NO_FLASH) +#error "CFG_NO_FLASH defined, but FLASH is required for auto-update feature" +#endif + +#include <command.h> +#include <flash.h> +#include <net.h> +#include <malloc.h> + +/* env variable holding the location of the update file */ +#define UPDATE_FILE_ENV "updatefile" + +/* set configuration defaults if needed */ +#ifndef CONFIG_UPDATE_LOAD_ADDR +#define CONFIG_UPDATE_LOAD_ADDR 0x100000 +#endif + +#ifndef CONFIG_UPDATE_TFTP_MSEC_MAX +#define CONFIG_UPDATE_TFTP_MSEC_MAX 100 +#endif + +#ifndef CONFIG_UPDATE_TFTP_CNT_MAX +#define CONFIG_UPDATE_TFTP_CNT_MAX 0 +#endif + +extern ulong TftpRRQTimeoutMSecs; +extern int TftpRRQTimeoutCountMax; +extern flash_info_t flash_info[]; +extern ulong load_addr; + +static uchar *saved_prot_info; + +static int update_load(char *filename, ulong msec_max, int cnt_max, ulong addr) +{ + int size, rv; + ulong saved_timeout_msecs; + int saved_timeout_count; + char *saved_netretry, *saved_bootfile; + + rv = 0; + /* save used globals and env variable */ + saved_timeout_msecs = TftpRRQTimeoutMSecs; + saved_timeout_count = TftpRRQTimeoutCountMax; + saved_netretry = strdup(getenv("netretry")); + saved_bootfile = strdup(BootFile); + + /* set timeouts for auto-update */ + TftpRRQTimeoutMSecs = msec_max; + TftpRRQTimeoutCountMax = cnt_max; + + /* we don't want to retry the connection if errors occur */ + setenv("netretry", "no"); + + /* download the update file */ + load_addr = addr; + copy_filename(BootFile, filename, sizeof(BootFile)); + size = NetLoop(TFTP); + + if (size < 0) + rv = 1; + else if (size > 0) + flush_cache(addr, size); + + /* restore changed globals and env variable */ + TftpRRQTimeoutMSecs = saved_timeout_msecs; + TftpRRQTimeoutCountMax = saved_timeout_count; + + setenv("netretry", saved_netretry); + if (saved_netretry != NULL) + free(saved_netretry); + + if (saved_bootfile != NULL) { + copy_filename(BootFile, saved_bootfile, sizeof(BootFile)); + free(saved_bootfile); + } + + return rv; +} + +static int update_flash_protect(int prot, ulong addr_first, ulong addr_last) +{ + uchar *sp_info_ptr; + ulong s; + int i, bank, cnt; + flash_info_t *info; + + sp_info_ptr = NULL; + + if (prot == 0) { + saved_prot_info = + calloc(CFG_MAX_FLASH_BANKS * CFG_MAX_FLASH_SECT, 1); + if (!saved_prot_info) + return 1; + } + + for (bank = 0; bank < CFG_MAX_FLASH_BANKS; ++bank) { + cnt = 0; + info = &flash_info[bank]; + + /* Nothing to do if the bank doesn't exist */ + if (info->sector_count == 0) + return 0; + + /* Point to current bank protection information */ + sp_info_ptr = saved_prot_info + (bank * CFG_MAX_FLASH_SECT); + + /* + * Adjust addr_first or addr_last if we are on bank boundary. + * Address space between banks must be continuous for other + * flash functions (like flash_sect_erase or flash_write) to + * succeed. Banks must also be numbered in correct order, + * according to increasing addresses. + */ + if (addr_last > info->start[0] + info->size - 1) + addr_last = info->start[0] + info->size - 1; + if (addr_first < info->start[0]) + addr_first = info->start[0]; + + for (i = 0; i < info->sector_count; i++) { + /* Save current information about protected sectors */ + if (prot == 0) { + s = info->start[i]; + if ((s >= addr_first) && (s <= addr_last)) + sp_info_ptr[i] = info->protect[i]; + + } + + /* Protect/unprotect sectors */ + if (sp_info_ptr[i] == 1) { +#if defined(CFG_FLASH_PROTECTION) + if (flash_real_protect(info, i, prot)) + return 1; +#else + info->protect[i] = prot; +#endif + cnt++; + } + } + + if (cnt) { + printf("%sProtected %d sectors\n", + prot ? "": "Un-", cnt); + } + } + + if((prot == 1) && saved_prot_info) + free(saved_prot_info); + + return 0; +} + +static int update_flash(ulong addr_source, ulong addr_first, ulong size) +{ + ulong addr_last = addr_first + size - 1; + + /* round last address to the sector boundary */ + if (flash_sect_roundb(&addr_last) > 0) + return 1; + + if (addr_first >= addr_last) { + printf("Error: end address exceeds addressing space\n"); + return 1; + } + + /* remove protection on processed sectors */ + if (update_flash_protect(0, addr_first, addr_last) > 0) { + printf("Error: could not unprotect flash sectors\n"); + return 1; + } + + printf("Erasing 0x%08lx - 0x%08lx", addr_first, addr_last); + if (flash_sect_erase(addr_first, addr_last) > 0) { + printf("Error: could not erase flash\n"); + return 1; + } + + printf("Copying to flash..."); + if (flash_write((char *)addr_source, addr_first, size) > 0) { + printf("Error: could not copy to flash\n"); + return 1; + } + printf("done\n"); + + /* enable protection on processed sectors */ + if (update_flash_protect(1, addr_first, addr_last) > 0) { + printf("Error: could not protect flash sectors\n"); + return 1; + } + + return 0; +} + +static int update_fit_getparams(const void *fit, int noffset, ulong *addr, + ulong *fladdr, ulong *size) +{ + const void *data; + + if (fit_image_get_data(fit, noffset, &data, (size_t *)size)) + return 1; + + if (fit_image_get_load(fit, noffset, (ulong *)fladdr)) + return 1; + + *addr = (ulong)data; + + return 0; +} + +void update_tftp(void) +{ + char *filename, *env_addr; + int images_noffset, ndepth, noffset; + ulong update_addr, update_fladdr, update_size; + ulong addr; + void *fit; + + printf("Auto-update from TFTP: "); + + /* get the file name of the update file */ + filename = getenv(UPDATE_FILE_ENV); + if (filename == NULL) { + printf("failed, env. variable '%s' not found\n", + UPDATE_FILE_ENV); + return; + } + + printf("trying update file '%s'\n", filename); + + /* get load address of downloaded update file */ + if ((env_addr = getenv("loadaddr")) != NULL) + addr = simple_strtoul(env_addr, NULL, 16); + else + addr = CONFIG_UPDATE_LOAD_ADDR; + + + if (update_load(filename, CONFIG_UPDATE_TFTP_MSEC_MAX, + CONFIG_UPDATE_TFTP_CNT_MAX, addr)) { + printf("Can't load update file, aborting auto-update\n"); + return; + } + + fit = (void *)addr; + + if (!fit_check_format((void *)fit)) { + printf("Bad FIT format of the update file, aborting " + "auto-update\n"); + return; + } + + /* process updates */ + images_noffset = fdt_path_offset(fit, FIT_IMAGES_PATH); + + ndepth = 0; + noffset = fdt_next_node(fit, images_noffset, &ndepth); + while (noffset >= 0 && ndepth > 0) { + if (ndepth != 1) + goto next_node; + + printf("Processing update '%s' :", + fit_get_name(fit, noffset, NULL)); + + if (!fit_image_check_hashes(fit, noffset)) { + printf("Error: invalid update hash, aborting\n"); + goto next_node; + } + + printf("\n"); + if (update_fit_getparams(fit, noffset, &update_addr, + &update_fladdr, &update_size)) { + printf("Error: can't get update parameteres, " + "aborting\n"); + goto next_node; + } + if (update_flash(update_addr, update_fladdr, update_size)) { + printf("Error: can't flash update, aborting\n"); + goto next_node; + } +next_node: + noffset = fdt_next_node(fit, noffset, &ndepth); + } + + return; +} diff --git a/doc/README.update b/doc/README.update new file mode 100644 index 0000000..a476002 --- /dev/null +++ b/doc/README.update @@ -0,0 +1,90 @@ +Automatic software update from a TFTP server +============================================ + +Overview +-------- + +This feature allows to automatically store software updates present on a TFTP +server in NOR Flash. In more detail: a TFTP transfer of a file given in +environment variable 'updatefile' from server 'serverip' is attempted during +boot. The update file should be a FIT file, and can contain one or more +updates. Each update in the update file has an address in NOR Flash where it +should be placed, updates are also protected with a SHA-1 checksum. If the +TFTP transfer is successful, the hash of each update is verified, and if the +verification is positive, the update is stored in Flash. + +The auto-update feature is enabled by the CONFIG_UPDATE_TFTP macro: + +#define CONFIG_UPDATE_TFTP 1 + + +Note that when enabling auto-update, Flash support must be turned on. Also, +one must enable FIT and LIBFDT support: + +#define CONFIG_FIT 1 +#define CONFIG_OF_LIBFDT 1 + +The auto-update feature uses the following configuration knobs: + +- CONFIG_UPDATE_LOAD_ADDR + + Normally, TFTP transfer of the update file is done to the address specified + in environment variable 'loadaddr'. If this variable is not present, the + transfer is made to the address given in CONFIG_UPDATE_LOAD_ADDR (0x100000 + by default). + +- CONFIG_UPDATE_TFTP_CNT_MAX + CONFIG_UPDATE_TFTP_MSEC_MAX + + These knobs control the timeouts during initial connection to the TFTP + server. Since a transfer is attempted during each boot, it is undesirable to + have a long delay when a TFTP server is not present. + CONFIG_UPDATE_TFTP_MSEC_MAX specifies the number of seconds to wait for the + server to respond to initial connection, and CONFIG_UPDATE_TFTP_CNT_MAX + gives the number of such connection retries. CONFIG_UPDATE_TFTP_CNT_MAX must + be non-negative and is 0 by default, CONFIG_UPDATE_TFTP_MSEC_MAX must be + positive and is 1 by default. + +Since the update file is in FIT format, it is created from an *.its file using +the mkimage tool. dtc tool with support for binary includes, e.g. in version +1.2.0 or later, must also be available on the system where the update file is +to be prepared. Refer to the doc/uImage.FIT/ directory for more details on FIT +images. + + +Example .its files +------------------ + +- doc/uImage.FIT/update_uboot.its + + A simple example that can be used to create an update file for automatically + replacing U-Boot image on a system. + + Assuming that an U-Boot image u-boot.bin is present in the current working + directory, and that the address given in the 'load' property in the + 'update_uboot.its' file is where the U-Boot is stored in Flash, the + following command will create the actual update file 'update_uboot.itb': + + mkimage -f update_uboot.its update_uboot.itb + + Place 'update_uboot.itb' on a TFTP server, for example as + '/tftpboot/update_uboot.itb', and set the 'updatefile' variable + appropriately, for example in the U-Boot prompt: + + setenv updatefile /tftpboot/update_uboot.itb + saveenv + + Now, when the system boots up and the update TFTP server specified in the + 'serverip' environment variable is accessible, the new U-Boot image will be + automatically stored in Flash. + + NOTE: do make sure that the 'u-boot.bin' image used to create the update + file is a good, working image. Also make sure that the address in Flash + where the update will be placed is correct. Making mistake here and + attempting the auto-update can render the system unusable. + +- doc/uImage.FIT/update3.its + + An example containing three updates. It can be used to update Linux kernel, + ramdisk and FDT blob stored in Flash. The procedure for preparing the update + file is similar to the example above. diff --git a/doc/uImage.FIT/update3.its b/doc/uImage.FIT/update3.its new file mode 100644 index 0000000..285cf73 --- /dev/null +++ b/doc/uImage.FIT/update3.its @@ -0,0 +1,41 @@ +/* + * Example Automatic software update file. + */ +/ { + description = "Automatic software updates: kernel, ramdisk, FDT"; + #address-cells = <1>; + + images { + update@1 { + description = "Linux kernel binary"; + data = /incbin/("./vmlinux.bin.gz"); + compression = "none"; + type = "firmware"; + load = <FF700000>; + hash@1 { + algo = "sha1"; + }; + }; + update@2 { + description = "Ramdisk image"; + data = /incbin/("./ramdisk_image.gz"); + compression = "none"; + type = "firmware"; + load = <FF8E0000>; + hash@1 { + algo = "sha1"; + }; + }; + + update@3 { + description = "FDT blob"; + data = /incbin/("./blob.fdt"); + compression = "none"; + type = "firmware"; + load = <FFAC0000>; + hash@1 { + algo = "sha1"; + }; + }; + }; +}; diff --git a/doc/uImage.FIT/update_uboot.its b/doc/uImage.FIT/update_uboot.its new file mode 100644 index 0000000..e0d27ea --- /dev/null +++ b/doc/uImage.FIT/update_uboot.its @@ -0,0 +1,21 @@ +/* + * Automatic software update for U-Boot + * Make sure the flashing addresses ('load' prop) is correct for your board! + */ +/ { + description = "Automatic U-Boot update"; + #address-cells = <1>; + + images { + update@1 { + description = "U-Boot binary"; + data = /incbin/("./u-boot.bin"); + compression = "none"; + type = "firmware"; + load = <FFFC0000>; + hash@1 { + algo = "sha1"; + }; + }; + }; +};
Dear Bartlomiej Sieka,
In message 1222867592-58285-6-git-send-email-tur@semihalf.com you wrote:
The auto-update feature allows to automatically download software updates from a TFTP server and store them in Flash memory during boot. Updates are contained in a FIT file and protected with SHA-1 checksum.
More detailed description can be found in doc/README.update.
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com
README | 12 ++ common/Makefile | 1 + common/main.c | 7 + common/update.c | 315 +++++++++++++++++++++++++++++++++++++++ doc/README.update | 90 +++++++++++ doc/uImage.FIT/update3.its | 41 +++++ doc/uImage.FIT/update_uboot.its | 21 +++ 7 files changed, 487 insertions(+), 0 deletions(-) create mode 100644 common/update.c create mode 100644 doc/README.update create mode 100644 doc/uImage.FIT/update3.its create mode 100644 doc/uImage.FIT/update_uboot.its
Applied to "next" branch. Thanks.
Best regards,
Wolfgang Denk
Now that the auto-update feature uses the 'firmware' type for updates, it is useful to inspect the load address of such images.
Signed-off-by: Bartlomiej Sieka tur@semihalf.com --- common/image.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/common/image.c b/common/image.c index dc8d7dd..0b7bd8d 100644 --- a/common/image.c +++ b/common/image.c @@ -1852,7 +1852,10 @@ void fit_print_contents (const void *fit) * @p: pointer to prefix string * * fit_image_print() lists all mandatory properies for the processed component - * image. If present, hash nodes are printed out as well. + * image. If present, hash nodes are printed out as well. Load + * address for images of type firmware is also printed out. Since the load + * address is not mandatory for firmware images, it will be output as + * "unavailable" when not present. * * returns: * no returned results @@ -1911,14 +1914,17 @@ void fit_image_print (const void *fit, int image_noffset, const char *p) printf ("%s OS: %s\n", p, genimg_get_os_name (os)); }
- if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE)) { + if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE) || + (type == IH_TYPE_FIRMWARE)) { ret = fit_image_get_load (fit, image_noffset, &load); printf ("%s Load Address: ", p); if (ret) printf ("unavailable\n"); else printf ("0x%08lx\n", load); + }
+ if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE)) { fit_image_get_entry (fit, image_noffset, &entry); printf ("%s Entry Point: ", p); if (ret) @@ -2844,7 +2850,7 @@ int fit_check_format (const void *fit) #if defined(CONFIG_TIMESTAMP) || defined(CONFIG_CMD_DATE) || defined(USE_HOSTCC) /* mandatory / node 'timestamp' property */ if (fdt_getprop (fit, 0, FIT_TIMESTAMP_PROP, NULL) == NULL) { - debug ("Wrong FIT format: no description\n"); + debug ("Wrong FIT format: no timestamp\n"); return 0; } #endif
participants (11)
-
Andrew Dyer -
Bartlomiej Sieka -
Ben Warren -
Detlev Zundel -
Graeme Russ -
Jean-Christophe PLAGNIOL-VILLARD -
Jerry Van Baren -
Jerry Van Baren -
Kim Phillips -
Stefan Roese -
Wolfgang Denk