[U-Boot] Automatic software updates in U-Boot

[U-Boot] Automatic software updates in U-Boot

[Bartlomiej Sieka]

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

[U-Boot] [PATCH 1/3] net: Make TFTP server timeout configurable

[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;
-- 
1.5.3.4

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[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 at 1 {
+			description = "Linux kernel binary";
+			data = /incbin/("./vmlinux.bin.gz");
+			compression = "none";
+			type = "firmware";
+			load = <FF700000>;
+			hash at 1 {
+				algo = "sha1";
+			};
+		};
+		update at 2 {
+			description = "Ramdisk image";
+			data = /incbin/("./ramdisk_image.gz");
+			compression = "none";
+			type = "firmware";
+			load = <FF8E0000>;
+			hash at 1 {
+				algo = "sha1";
+			};
+		};
+
+		update at 3 {
+			description = "FDT blob";
+			data = /incbin/("./blob.fdt");
+			compression = "none";
+			type = "firmware";
+			load = <FFAC0000>;
+			hash at 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 at 1 {
+			description = "U-Boot binary";
+			data = /incbin/("./u-boot.bin");
+			compression = "none";
+			type = "firmware";
+			load = <FFFC0000>;
+			hash at 1 {
+				algo = "sha1";
+			};
+		};
+	};
+};
-- 
1.5.3.4

[U-Boot] [PATCH 3/3] FIT: output image load address for type 'firmware', fix debug msg while there

[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
-- 
1.5.3.4

[U-Boot] [PATCH 1/3] net: Make TFTP server timeout configurable

[Jerry Van Baren]

Bartlomiej Sieka wrote:
> 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.

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

[U-Boot] [PATCH 1/3] net: Make TFTP server timeout configurable

[Graeme Russ]

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:
>> 1. timeout waiting for initial server reply after sending RRQ
>> 2. 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

[U-Boot] [PATCH 1/3] net: Make TFTP server timeout configurable

[Jerry Van Baren]

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:
>>> 1. timeout waiting for initial server reply after sending RRQ
>>> 2. 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

[U-Boot] [PATCH 1/3] net: Make TFTP server timeout configurable

[Bartlomiej Sieka]

Jerry Van Baren wrote:
> Bartlomiej Sieka wrote:
>> 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.
> 
> 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

[U-Boot] [PATCH 1/3] net: Make TFTP server timeout configurable

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Humans do claim a great deal for that particular emotion (love).
	-- Spock, "The Lights of Zetar", stardate 5725.6

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Wolfgang Denk]

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_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

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");
> +		else
> +			strncpy(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.

> +			else
> +				sector_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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Hegel was right when he said that we learn from history that man  can
never learn anything from history.              - George Bernard Shaw

[U-Boot] [PATCH 1/3] net: Make TFTP server timeout configurable

[Bartlomiej Sieka]

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

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Bartlomiej Sieka]

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_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
> 
> 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");
>> +		else
>> +			strncpy(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.
> 
>> +			else
>> +				sector_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

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Ever try. Ever fail. No matter. Try again. Fail again.  Fail  better.
                                                        -- S. Beckett

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Bartlomiej Sieka]

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

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It all seemed, he thought, to be rather a lot of  trouble  to  go  to
just sharpen a razor blade.  - Terry Pratchett, _The Light Fantastic_

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Andrew Dyer]

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.

-- 
Hardware, n.:
 The parts of a computer system that can be kicked.

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Jerry Van Baren]

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

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Kim Phillips]

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

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The majority of the stupid is  invincible  and  guaranteed  for  all
time.  The  terror  of their tyranny, however, is alleviated by their
lack of consistency."                               - Albert Einstein

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Kim Phillips]

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

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Bartlomiej Sieka]

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

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Bartlomiej Sieka]

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

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
When in doubt, mumble;   when in trouble, delegate;  when in  charge,
ponder.                                             -- James H. Boren

[U-Boot] [PATCH 2/3] Automatic software update from TFTP server

[Detlev Zundel]

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

-- 
Whatever you do will be insignificant,
but it is very important that you do it.
                                        -- Mahatma Gandhi
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] Automatic software updates in U-Boot -- version 2

[Bartlomiej Sieka]

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

[U-Boot] [PATCH v2 1/6] flash: factor out adjusting of Flash address to the end of sector

[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);
-- 
1.5.2.2

[U-Boot] [PATCH v2 2/6] net: express the first argument to NetSetTimeout() in milliseconds

[Bartlomiej Sieka]

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;
-- 
1.5.2.2

[U-Boot] [PATCH v2 3/6] net: Make TFTP server timeout configurable

[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:

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;
-- 
1.5.2.2

[U-Boot] [PATCH v2 4/6] Restore alphabetic ordering in common/Makefile

[Bartlomiej Sieka]

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)
-- 
1.5.2.2

[U-Boot] [PATCH v2 5/6] Automatic software update from TFTP server

[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 at 1 {
+			description = "Linux kernel binary";
+			data = /incbin/("./vmlinux.bin.gz");
+			compression = "none";
+			type = "firmware";
+			load = <FF700000>;
+			hash at 1 {
+				algo = "sha1";
+			};
+		};
+		update at 2 {
+			description = "Ramdisk image";
+			data = /incbin/("./ramdisk_image.gz");
+			compression = "none";
+			type = "firmware";
+			load = <FF8E0000>;
+			hash at 1 {
+				algo = "sha1";
+			};
+		};
+
+		update at 3 {
+			description = "FDT blob";
+			data = /incbin/("./blob.fdt");
+			compression = "none";
+			type = "firmware";
+			load = <FFAC0000>;
+			hash at 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 at 1 {
+			description = "U-Boot binary";
+			data = /incbin/("./u-boot.bin");
+			compression = "none";
+			type = "firmware";
+			load = <FFFC0000>;
+			hash at 1 {
+				algo = "sha1";
+			};
+		};
+	};
+};
-- 
1.5.2.2

[U-Boot] [PATCH v2 6/6] FIT: output image load address for type 'firmware', fix message while there

[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
-- 
1.5.2.2

[U-Boot] [PATCH v2 2/6] net: express the first argument to NetSetTimeout() in milliseconds

[Ben Warren]

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

[U-Boot] [PATCH v2 3/6] net: Make TFTP server timeout configurable

[Ben Warren]

Bartlomiej Sieka wrote:
> 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>
> ---
Applied to net/next

regards,
Ben

[U-Boot] [PATCH v2 1/6] flash: factor out adjusting of Flash address to the end of sector

[Bartlomiej Sieka]

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);

[U-Boot] [PATCH v2 1/6] flash: factor out adjusting of Flash address to the end of sector

[Stefan Roese]

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 at denx.de
=====================================================================

[U-Boot] [PATCH v2 1/6] flash: factor out adjusting of Flash address to the end of sector

[Bartlomiej Sieka]

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

[U-Boot] [PATCH v2 1/6] flash: factor out adjusting of Flash address to the end of sector

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The only way to learn a new programming language is by  writing  pro-
grams in it.                                        - Brian Kernighan

[U-Boot] [PATCH v2 1/6] flash: factor out adjusting of Flash address to the end of sector

[Stefan Roese]

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 at denx.de
=====================================================================

[U-Boot] [PATCH v2 4/6] Restore alphabetic ordering in common/Makefile

[Jean-Christophe PLAGNIOL-VILLARD]

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.

[U-Boot] [PATCH v2 4/6] Restore alphabetic ordering in common/Makefile

[Bartlomiej Sieka]

[dropped sr at denx.de and biggerbadderben at 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

[U-Boot] [PATCH v2 4/6] Restore alphabetic ordering in common/Makefile

[Jean-Christophe PLAGNIOL-VILLARD]

On 14:58 Wed 08 Oct     , Bartlomiej Sieka wrote:
> [dropped sr at denx.de and biggerbadderben at 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.

[U-Boot] [PATCH v2 1/6] flash: factor out adjusting of Flash address to the end of sector

[Ben Warren]

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

[U-Boot] [PATCH v2 4/6] Restore alphabetic ordering in common/Makefile

[Bartlomiej Sieka]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 14:58 Wed 08 Oct     , Bartlomiej Sieka wrote:
>> [dropped sr at denx.de and biggerbadderben at 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

[U-Boot] [PATCH v2 1/6] flash: factor out adjusting of Flash address to the end of sector

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
People are always a lot more complicated than you  think.  It's  very
important to remember that.             - Terry Pratchett, _Truckers_

[U-Boot] [PATCH v2 1/6] flash: factor out adjusting of Flash address to the end of sector

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Power is danger.
	-- The Centurion, "Balance of Terror", stardate 1709.2

[U-Boot] [PATCH v2 5/6] Automatic software update from TFTP server

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Syntactic sugar causes cancer of the semicolon.
                - Epigrams in Programming, ACM SIGPLAN Sept. 1982

[U-Boot] Automatic software updates in U-Boot -- version 2

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Certain old men prefer to rise at dawn, taking a cold bath and a long
walk with an empty stomach and otherwise mortifying the  flesh.  They
then point with pride to these practices as the cause of their sturdy
health  and ripe years; the truth being that they are hearty and old,
not because of their habits, but in spite of them. The reason we find
only robust persons doing this thing is that it has  killed  all  the
others who have tried it.  - Ambrose Bierce, "The Devil's Dictionary"

[U-Boot] Automatic software updates in U-Boot -- version 2

[Bartlomiej Sieka]

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

[U-Boot] Automatic software updates in U-Boot -- version 2

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Without facts, the decision cannot be made logically. You  must  rely
on your human intuition.
	-- Spock, "Assignment: Earth", stardate unknown

[U-Boot] Automatic software updates in U-Boot -- version 2

[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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The face of war has never changed.  Surely it is more logical to heal
than to kill.
	-- Surak of Vulcan, "The Savage Curtain", stardate 5906.5

[U-Boot] Automatic software updates in U-Boot -- version 2

[Bartlomiej Sieka]

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