public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network
@ 2011-10-22  4:51 Simon Glass
  2011-10-22  4:51 ` [U-Boot] [PATCH 1/8] Move simple_itoa to vsprintf Simon Glass
                   ` (8 more replies)
  0 siblings, 9 replies; 25+ messages in thread
From: Simon Glass @ 2011-10-22  4:51 UTC (permalink / raw)
  To: u-boot

The tftpboot command permits reading of files over a network interface
using the Trivial FTP protocol. This patch series adds the ability to
transfer files the other way.

Why is this useful?

- Uploading boot time data to a server
- Uploading profiling information
- Uploading large mounts of data for comparison / checking on a host
    (e.g. use tftpput and ghex2 instead of the 'md' command)

Mostly the existing code can be re-used and I have tried to avoid too
much refactoring or cleaning up.

The feature is activated by the CONFIG_CMD_TFTPPUT option.

This has been very lightly tested on a Seaboard with a USB network
adaptor. I don't think it handles block number overflow.


Simon Glass (8):
  Move simple_itoa to vsprintf
  Add setenv_uint() and setenv_addr()
  tftpput: Rename TFTP to TFTPGET
  tftpput: move common code into separate functions
  tftpput: support selecting get/put for tftp
  tftpput: add save_addr and save_size global variables
  tftpput: implement tftp logic
  tftpput: add tftpput command

 README                   |    2 +
 common/cmd_net.c         |   31 +++++++-
 common/cmd_nvedit.c      |   31 ++++++++
 common/hush.c            |   15 ----
 include/common.h         |    5 +
 include/config_cmd_all.h |    1 +
 include/net.h            |    8 +-
 lib/vsprintf.c           |   16 ++++
 net/bootp.c              |    2 +-
 net/net.c                |   22 +++---
 net/tftp.c               |  189 +++++++++++++++++++++++++++++++++-------------
 net/tftp.h               |    2 +-
 12 files changed, 238 insertions(+), 86 deletions(-)

-- 
1.7.3.1

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/8] Move simple_itoa to vsprintf
  2011-10-22  4:51 [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Simon Glass
@ 2011-10-22  4:51 ` Simon Glass
  2011-10-22  4:51 ` [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr() Simon Glass
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2011-10-22  4:51 UTC (permalink / raw)
  To: u-boot

This function is generally useful and shouldn't hide away in hush. It
has been moved as is.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/hush.c    |   15 ---------------
 include/common.h |    1 +
 lib/vsprintf.c   |   16 ++++++++++++++++
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index 85a6030..9bdb499 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -17,7 +17,6 @@
  *      Erik W. Troan, which they placed in the public domain.  I don't know
  *      how much of the Johnson/Troan code has survived the repeated rewrites.
  * Other credits:
- *      simple_itoa() was lifted from boa-0.93.15
  *      b_addchr() derived from similar w_addchar function in glibc-2.2
  *      setup_redirect(), redirect_opt_num(), and big chunks of main()
  *        and many builtins derived from contributions by Erik Andersen
@@ -922,20 +921,6 @@ static int b_addqchr(o_string *o, int ch, int quote)
 	return b_addchr(o, ch);
 }
 
-/* belongs in utility.c */
-char *simple_itoa(unsigned int i)
-{
-	/* 21 digits plus null terminator, good for 64-bit or smaller ints */
-	static char local[22];
-	char *p = &local[21];
-	*p-- = '\0';
-	do {
-		*p-- = '0' + i % 10;
-		i /= 10;
-	} while (i > 0);
-	return p + 1;
-}
-
 #ifndef __U_BOOT__
 static int b_adduint(o_string *o, unsigned int i)
 {
diff --git a/include/common.h b/include/common.h
index eb19a44..55f772d 100644
--- a/include/common.h
+++ b/include/common.h
@@ -702,6 +702,7 @@ void	panic(const char *fmt, ...)
 int	sprintf(char * buf, const char *fmt, ...)
 		__attribute__ ((format (__printf__, 2, 3)));
 int	vsprintf(char *buf, const char *fmt, va_list args);
+char *simple_itoa(ulong i);
 
 /* lib/strmhz.c */
 char *	strmhz(char *buf, unsigned long hz);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 79dead3..e497a86 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -7,6 +7,8 @@
 /* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */
 /*
  * Wirzenius wrote this portably, Torvalds fucked it up :-)
+ *
+ * from hush: simple_itoa() was lifted from boa-0.93.15
  */
 
 #include <stdarg.h>
@@ -738,3 +740,17 @@ void __assert_fail(const char *assertion, const char *file, unsigned line,
 	panic("%s:%u: %s: Assertion `%s' failed.", file, line, function,
 	      assertion);
 }
+
+char *simple_itoa(ulong i)
+{
+	/* 21 digits plus null terminator, good for 64-bit or smaller ints */
+	static char local[22];
+	char *p = &local[21];
+
+	*p-- = '\0';
+	do {
+		*p-- = '0' + i % 10;
+		i /= 10;
+	} while (i > 0);
+	return p + 1;
+}
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-22  4:51 [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Simon Glass
  2011-10-22  4:51 ` [U-Boot] [PATCH 1/8] Move simple_itoa to vsprintf Simon Glass
@ 2011-10-22  4:51 ` Simon Glass
  2011-10-23  5:29   ` Mike Frysinger
  2011-10-22  4:51 ` [U-Boot] [PATCH 3/8] tftpput: Rename TFTP to TFTPGET Simon Glass
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2011-10-22  4:51 UTC (permalink / raw)
  To: u-boot

It seems we put numbers and addresses into environment variables a lot.
We should have some functions to do this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/cmd_nvedit.c |   29 +++++++++++++++++++++++++++++
 include/common.h    |    2 ++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 101bc49..da7028c 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -377,6 +377,35 @@ int setenv(const char *varname, const char *varvalue)
 		return _do_env_set(0, 3, (char * const *)argv);
 }
 
+/**
+ * Set an environment variable to an integer value
+ *
+ * @param varname	Environmet variable to set
+ * @param value		Value to set it to
+ * @return 0 if ok, 1 on error
+ */
+int setenv_ulong(const char *varname, ulong value)
+{
+	char *str = simple_itoa(value);
+
+	return setenv(varname, str);
+}
+
+/**
+ * Set an environment variable to an address in hex
+ *
+ * @param varname	Environmet variable to set
+ * @param addr		Value to set it to
+ * @return 0 if ok, 1 on error
+ */
+int setenv_addr(const char *varname, const void *addr)
+{
+	char str[17];
+
+	sprintf(str, "%x", (uintptr_t)addr);
+	return setenv(varname, str);
+}
+
 int do_env_set(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	if (argc < 2)
diff --git a/include/common.h b/include/common.h
index 55f772d..99c2a37 100644
--- a/include/common.h
+++ b/include/common.h
@@ -294,6 +294,8 @@ int inline setenv    (const char *, const char *);
 #else
 int	setenv	     (const char *, const char *);
 #endif /* CONFIG_PPC */
+int setenv_ulong(const char *varname, ulong value);
+int setenv_addr(const char *varname, const void *addr);
 #ifdef CONFIG_ARM
 # include <asm/mach-types.h>
 # include <asm/setup.h>
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 3/8] tftpput: Rename TFTP to TFTPGET
  2011-10-22  4:51 [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Simon Glass
  2011-10-22  4:51 ` [U-Boot] [PATCH 1/8] Move simple_itoa to vsprintf Simon Glass
  2011-10-22  4:51 ` [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr() Simon Glass
@ 2011-10-22  4:51 ` Simon Glass
  2011-10-22  4:51 ` [U-Boot] [PATCH 4/8] tftpput: move common code into separate functions Simon Glass
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2011-10-22  4:51 UTC (permalink / raw)
  To: u-boot

This is a better name for this protocol. Also remove the typedef to keep
checkpatch happy, and move zeroing of NetBootFileXferSize a little
earlier since TFTPPUT will need to change this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/cmd_net.c |    8 ++++----
 include/net.h    |    8 +++++---
 net/net.c        |   18 ++++++++----------
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/common/cmd_net.c b/common/cmd_net.c
index 872f4a6..f610385 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -28,7 +28,7 @@
 #include <command.h>
 #include <net.h>
 
-static int netboot_common (proto_t, cmd_tbl_t *, int , char * const []);
+static int netboot_common(enum proto_t, cmd_tbl_t *, int, char * const []);
 
 int do_bootp (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
@@ -43,7 +43,7 @@ U_BOOT_CMD(
 
 int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	return netboot_common (TFTP, cmdtp, argc, argv);
+	return netboot_common(TFTPGET, cmdtp, argc, argv);
 }
 
 U_BOOT_CMD(
@@ -167,8 +167,8 @@ static void netboot_update_env (void)
 #endif
 }
 
-static int
-netboot_common (proto_t proto, cmd_tbl_t *cmdtp, int argc, char * const argv[])
+static int netboot_common(enum proto_t proto, cmd_tbl_t *cmdtp, int argc,
+		char * const argv[])
 {
 	char *s;
 	char *end;
diff --git a/include/net.h b/include/net.h
index d5d37b6..3dfe073 100644
--- a/include/net.h
+++ b/include/net.h
@@ -347,8 +347,10 @@ extern int		NetState;		/* Network loop state		*/
 
 extern int		NetRestartWrap;		/* Tried all network devices	*/
 
-typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
-	       TFTPSRV } proto_t;
+enum {
+	BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
+	TFTPSRV
+};
 
 /* from net/net.c */
 extern char	BootFile[128];			/* Boot File name		*/
@@ -374,7 +376,7 @@ extern int NetTimeOffset;			/* offset time from UTC		*/
 #endif
 
 /* Initialize the network adapter */
-extern int	NetLoop(proto_t);
+extern int NetLoop(enum proto_t);
 
 /* Shutdown adapters and cleanup */
 extern void	NetStop(void);
diff --git a/net/net.c b/net/net.c
index 5e67886..ce07ed6 100644
--- a/net/net.c
+++ b/net/net.c
@@ -224,7 +224,7 @@ static ulong	timeDelta;
 /* THE transmit packet */
 volatile uchar *NetTxPacket;
 
-static int net_check_prereq(proto_t protocol);
+static int net_check_prereq(enum proto_t protocol);
 
 static int NetTryCount;
 
@@ -310,8 +310,7 @@ void ArpTimeoutCheck(void)
 	}
 }
 
-static void
-NetInitLoop(proto_t protocol)
+static void NetInitLoop(enum proto_t protocol)
 {
 	static int env_changed_id;
 	bd_t *bd = gd->bd;
@@ -340,8 +339,7 @@ NetInitLoop(proto_t protocol)
  *	Main network processing loop.
  */
 
-int
-NetLoop(proto_t protocol)
+int NetLoop(enum proto_t protocol)
 {
 	bd_t *bd = gd->bd;
 
@@ -405,10 +403,11 @@ restart:
 
 	case 0:
 		NetDevExists = 1;
+		NetBootFileXferSize = 0;
 		switch (protocol) {
-		case TFTP:
+		case TFTPGET:
 			/* always use ARP to get server ethernet address */
-			TftpStart();
+			TftpStart(protocol);
 			break;
 #ifdef CONFIG_CMD_TFTPSRV
 		case TFTPSRV:
@@ -470,7 +469,6 @@ restart:
 			break;
 		}
 
-		NetBootFileXferSize = 0;
 		break;
 	}
 
@@ -1728,7 +1726,7 @@ NetReceive(volatile uchar *inpkt, int len)
 
 /**********************************************************************/
 
-static int net_check_prereq(proto_t protocol)
+static int net_check_prereq(enum proto_t protocol)
 {
 	switch (protocol) {
 		/* Fall through */
@@ -1759,7 +1757,7 @@ static int net_check_prereq(proto_t protocol)
 #if defined(CONFIG_CMD_NFS)
 	case NFS:
 #endif
-	case TFTP:
+	case TFTPGET:
 		if (NetServerIP == 0) {
 			puts("*** ERROR: `serverip' not set\n");
 			return 1;
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 4/8] tftpput: move common code into separate functions
  2011-10-22  4:51 [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Simon Glass
                   ` (2 preceding siblings ...)
  2011-10-22  4:51 ` [U-Boot] [PATCH 3/8] tftpput: Rename TFTP to TFTPGET Simon Glass
@ 2011-10-22  4:51 ` Simon Glass
  2011-10-22  4:51 ` [U-Boot] [PATCH 5/8] tftpput: support selecting get/put for tftp Simon Glass
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2011-10-22  4:51 UTC (permalink / raw)
  To: u-boot

We want to show block markers on completion of get and put, so
move this common code into separate functions.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 net/tftp.c |   70 ++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index da8eeaa..4ad85a5 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -195,6 +195,42 @@ static void TftpTimeout(void);
 
 /**********************************************************************/
 
+static void show_block_marker(void)
+{
+	ulong pos = TftpBlock * TftpBlkSize + TftpBlockWrapOffset;
+
+#ifdef CONFIG_TFTP_TSIZE
+	if (TftpTsize) {
+		while (TftpNumchars <
+			/* TODO: needs to be different for put */
+			pos * 50 / TftpTsize) {
+			putc('#');
+			TftpNumchars++;
+		}
+	}
+#endif
+	else {
+		if (((TftpBlock - 1) % 10) == 0)
+			putc('#');
+		else if ((TftpBlock % (10 * HASHES_PER_LINE)) == 0)
+			puts("\n\t ");
+	}
+}
+
+/* The TFTP get or put is complete */
+static void tftp_complete(void)
+{
+#ifdef CONFIG_TFTP_TSIZE
+	/* Print hash marks for the last packet received */
+	while (TftpTsize && TftpNumchars < 49) {
+		putc('#');
+		TftpNumchars++;
+	}
+#endif
+	puts("\ndone\n");
+	NetState = NETLOOP_SUCCESS;
+}
+
 static void
 TftpSend(void)
 {
@@ -400,21 +436,8 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 				TftpBlkSize * TFTP_SEQUENCE_SIZE;
 			printf("\n\t %lu MB received\n\t ",
 				TftpBlockWrapOffset>>20);
-		}
-#ifdef CONFIG_TFTP_TSIZE
-		else if (TftpTsize) {
-			while (TftpNumchars <
-			       NetBootFileXferSize * 50 / TftpTsize) {
-				putc('#');
-				TftpNumchars++;
-			}
-		}
-#endif
-		else {
-			if (((TftpBlock - 1) % 10) == 0)
-				putc('#');
-			else if ((TftpBlock % (10 * HASHES_PER_LINE)) == 0)
-				puts("\n\t ");
+		} else {
+			show_block_marker();
 		}
 
 		if (TftpState == STATE_SEND_RRQ)
@@ -498,21 +521,8 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 		}
 		else
 #endif
-		if (len < TftpBlkSize) {
-			/*
-			 *	We received the whole thing.  Try to
-			 *	run it.
-			 */
-#ifdef CONFIG_TFTP_TSIZE
-			/* Print hash marks for the last packet received */
-			while (TftpTsize && TftpNumchars < 49) {
-				putc('#');
-				TftpNumchars++;
-			}
-#endif
-			puts("\ndone\n");
-			NetState = NETLOOP_SUCCESS;
-		}
+		if (len < TftpBlkSize)
+			tftp_complete();
 		break;
 
 	case TFTP_ERROR:
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 5/8] tftpput: support selecting get/put for tftp
  2011-10-22  4:51 [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Simon Glass
                   ` (3 preceding siblings ...)
  2011-10-22  4:51 ` [U-Boot] [PATCH 4/8] tftpput: move common code into separate functions Simon Glass
@ 2011-10-22  4:51 ` Simon Glass
  2011-10-22  4:51 ` [U-Boot] [PATCH 6/8] tftpput: add save_addr and save_size global variables Simon Glass
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2011-10-22  4:51 UTC (permalink / raw)
  To: u-boot

TftpStart should support starting either a get or a put.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 include/net.h |    4 ++--
 net/bootp.c   |    2 +-
 net/tftp.c    |    5 +++--
 net/tftp.h    |    2 +-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/net.h b/include/net.h
index 3dfe073..2e5a898 100644
--- a/include/net.h
+++ b/include/net.h
@@ -349,8 +349,8 @@ extern int		NetRestartWrap;		/* Tried all network devices	*/
 
 enum {
 	BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
-	TFTPSRV
-};
+	TFTPSRV, TFTPPUT
+} proto_t;
 
 /* from net/net.c */
 extern char	BootFile[128];			/* Boot File name		*/
diff --git a/net/bootp.c b/net/bootp.c
index a003c42..068b037 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -165,7 +165,7 @@ static void auto_load(void)
 		}
 #endif
 	}
-	TftpStart();
+	TftpStart(TFTPGET);
 }
 
 #if !defined(CONFIG_CMD_DHCP)
diff --git a/net/tftp.c b/net/tftp.c
index 4ad85a5..3a58e32 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -81,6 +81,7 @@ static int	TftpTsize;
 /* The number of hashes we printed */
 static short	TftpNumchars;
 #endif
+static int	TftpWriting;	/* 1 if writing, else 0 */
 
 #define STATE_SEND_RRQ	1
 #define STATE_DATA	2
@@ -572,8 +573,7 @@ TftpTimeout(void)
 }
 
 
-void
-TftpStart(void)
+void TftpStart(proto_t protocol)
 {
 	char *ep;             /* Environment pointer */
 
@@ -648,6 +648,7 @@ TftpStart(void)
 	}
 
 	putc('\n');
+	TftpWriting = (protocol == TFTPPUT);
 
 	printf("Load address: 0x%lx\n", load_addr);
 
diff --git a/net/tftp.h b/net/tftp.h
index 3abdf7b..2d5d594 100644
--- a/net/tftp.h
+++ b/net/tftp.h
@@ -16,7 +16,7 @@
  */
 
 /* tftp.c */
-extern void	TftpStart (void);	/* Begin TFTP get */
+void TftpStart(proto_t protocol);	/* Begin TFTP get/put */
 
 #ifdef CONFIG_CMD_TFTPSRV
 extern void	TftpStartServer(void);	/* Wait for incoming TFTP put */
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 6/8] tftpput: add save_addr and save_size global variables
  2011-10-22  4:51 [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Simon Glass
                   ` (4 preceding siblings ...)
  2011-10-22  4:51 ` [U-Boot] [PATCH 5/8] tftpput: support selecting get/put for tftp Simon Glass
@ 2011-10-22  4:51 ` Simon Glass
  2011-10-22  4:51 ` [U-Boot] [PATCH 7/8] tftpput: implement tftp logic Simon Glass
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2011-10-22  4:51 UTC (permalink / raw)
  To: u-boot

We need something akin to load_addr to handle saving data.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/cmd_nvedit.c |    2 ++
 include/common.h    |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index da7028c..30cf8df 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -79,6 +79,8 @@ SPI_FLASH|MG_DISK|NVRAM|MMC} or CONFIG_ENV_IS_NOWHERE
 #define	MAX_ENV_SIZE	(1 << 20)	/* 1 MiB */
 
 ulong load_addr = CONFIG_SYS_LOAD_ADDR;	/* Default Load Address */
+ulong save_addr;			/* Default Save Address */
+ulong save_size;			/* Default Save Size (in bytes) */
 
 /*
  * Table with supported baudrates (defined in config_xyz.h)
diff --git a/include/common.h b/include/common.h
index 99c2a37..d7382d9 100644
--- a/include/common.h
+++ b/include/common.h
@@ -278,6 +278,8 @@ void flash_perror (int);
 int	source (ulong addr, const char *fit_uname);
 
 extern ulong load_addr;		/* Default Load Address */
+extern ulong save_addr;		/* Default Save Address */
+extern ulong save_size;		/* Default Save Size */
 
 /* common/cmd_doc.c */
 void	doc_probe(unsigned long physadr);
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 7/8] tftpput: implement tftp logic
  2011-10-22  4:51 [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Simon Glass
                   ` (5 preceding siblings ...)
  2011-10-22  4:51 ` [U-Boot] [PATCH 6/8] tftpput: add save_addr and save_size global variables Simon Glass
@ 2011-10-22  4:51 ` Simon Glass
  2011-10-22  4:51 ` [U-Boot] [PATCH 8/8] tftpput: add tftpput command Simon Glass
  2011-10-22  8:21 ` [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Albert ARIBAUD
  8 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2011-10-22  4:51 UTC (permalink / raw)
  To: u-boot

This adds logic to tftp.c to implement the tftp 'put' command, and
updates the README.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 README     |    2 +
 net/net.c  |    4 ++
 net/tftp.c |  114 +++++++++++++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 100 insertions(+), 20 deletions(-)

diff --git a/README b/README
index 7e032a9..8261bfb 100644
--- a/README
+++ b/README
@@ -784,6 +784,7 @@ The following options need to be configured:
 		CONFIG_CMD_SOURCE	  "source" command Support
 		CONFIG_CMD_SPI		* SPI serial bus support
 		CONFIG_CMD_TFTPSRV	* TFTP transfer in server mode
+		CONFIG_CMD_TFTPPUT	* TFTP put command (upload)
 		CONFIG_CMD_TIME		* run command and report execution time
 		CONFIG_CMD_USB		* USB support
 		CONFIG_CMD_CDP		* Cisco Discover Protocol support
@@ -3340,6 +3341,7 @@ bootp	- boot image via network using BootP/TFTP protocol
 tftpboot- boot image via network using TFTP protocol
 	       and env variables "ipaddr" and "serverip"
 	       (and eventually "gatewayip")
+tftpput - upload a file via network using TFTP protocol
 rarpboot- boot image via network using RARP/TFTP protocol
 diskboot- boot from IDE devicebootd   - boot default, i.e., run 'bootcmd'
 loads	- load S-Record file over serial line
diff --git a/net/net.c b/net/net.c
index ce07ed6..89596d3 100644
--- a/net/net.c
+++ b/net/net.c
@@ -406,6 +406,9 @@ restart:
 		NetBootFileXferSize = 0;
 		switch (protocol) {
 		case TFTPGET:
+#ifdef CONFIG_CMD_TFTPPUT
+		case TFTPPUT:
+#endif
 			/* always use ARP to get server ethernet address */
 			TftpStart(protocol);
 			break;
@@ -1758,6 +1761,7 @@ static int net_check_prereq(enum proto_t protocol)
 	case NFS:
 #endif
 	case TFTPGET:
+	case TFTPPUT:
 		if (NetServerIP == 0) {
 			puts("*** ERROR: `serverip' not set\n");
 			return 1;
diff --git a/net/tftp.c b/net/tftp.c
index 3a58e32..02862c6 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -82,6 +82,11 @@ static int	TftpTsize;
 static short	TftpNumchars;
 #endif
 static int	TftpWriting;	/* 1 if writing, else 0 */
+#ifdef CONFIG_CMD_TFTPPUT
+static int	TftpFinalBlock;	/* 1 if we have sent the last block */
+#else
+#define TftpWriting	0
+#endif
 
 #define STATE_SEND_RRQ	1
 #define STATE_DATA	2
@@ -89,6 +94,7 @@ static int	TftpWriting;	/* 1 if writing, else 0 */
 #define STATE_BAD_MAGIC	4
 #define STATE_OACK	5
 #define STATE_RECV_WRQ	6
+#define STATE_SEND_WRQ	7
 
 /* default TFTP block size */
 #define TFTP_BLOCK_SIZE		512
@@ -191,6 +197,28 @@ store_block(unsigned block, uchar *src, unsigned len)
 		NetBootFileXferSize = newsize;
 }
 
+#ifdef CONFIG_CMD_TFTPPUT
+/**
+ * Load the next block from memory to be sent over tftp.
+ *
+ * @param block	Block number to send
+ * @param dst	Destination buffer for data
+ * @param len	Number of bytes in block (this one and every other)
+ * @return number of bytes loaded
+ */
+static int load_block(unsigned block, uchar *dst, unsigned len)
+{
+	ulong offset = (block - 1) * len + TftpBlockWrapOffset;
+	ulong tosend = len;
+
+	tosend = min(NetBootFileXferSize - offset, tosend);
+	(void)memcpy(dst, (void *)(save_addr + offset), tosend);
+	debug("%s: block=%d, offset=%ld, len=%d, tosend=%ld\n", __func__,
+		block, offset, len, tosend);
+	return tosend;
+}
+#endif
+
 static void TftpSend(void);
 static void TftpTimeout(void);
 
@@ -235,7 +263,7 @@ static void tftp_complete(void)
 static void
 TftpSend(void)
 {
-	volatile uchar *pkt;
+	uchar *pkt;
 	volatile uchar *xp;
 	int		len = 0;
 	volatile ushort *s;
@@ -251,14 +279,15 @@ TftpSend(void)
 	 *	We will always be sending some sort of packet, so
 	 *	cobble together the packet headers now.
 	 */
-	pkt = NetTxPacket + NetEthHdrSize() + IP_HDR_SIZE;
+	pkt = (uchar *)(NetTxPacket + NetEthHdrSize() + IP_HDR_SIZE);
 
 	switch (TftpState) {
-
 	case STATE_SEND_RRQ:
+	case STATE_SEND_WRQ:
 		xp = pkt;
 		s = (ushort *)pkt;
-		*s++ = htons(TFTP_RRQ);
+		*s++ = htons(TftpState == STATE_SEND_RRQ ? TFTP_RRQ
+				: TFTP_WRQ);
 		pkt = (uchar *)s;
 		strcpy((char *)pkt, tftp_filename);
 		pkt += strlen(tftp_filename) + 1;
@@ -270,8 +299,8 @@ TftpSend(void)
 		debug("send option \"timeout %s\"\n", (char *)pkt);
 		pkt += strlen((char *)pkt) + 1;
 #ifdef CONFIG_TFTP_TSIZE
-		memcpy((char *)pkt, "tsize\0000\0", 8);
-		pkt += 8;
+		pkt += sprintf((char *)pkt, "tsize%c%lu%c",
+				0, NetBootFileXferSize, 0);
 #endif
 		/* try for more effic. blk size */
 		pkt += sprintf((char *)pkt, "blksize%c%d%c",
@@ -302,9 +331,19 @@ TftpSend(void)
 	case STATE_DATA:
 		xp = pkt;
 		s = (ushort *)pkt;
-		*s++ = htons(TFTP_ACK);
-		*s++ = htons(TftpBlock);
-		pkt = (uchar *)s;
+		s[0] = htons(TFTP_ACK);
+		s[1] = htons(TftpBlock);
+		pkt = (uchar *)(s + 2);
+#ifdef CONFIG_CMD_TFTPPUT
+		if (TftpWriting) {
+			int toload = TftpBlkSize;
+			int loaded = load_block(TftpBlock, pkt, toload);
+
+			s[0] = htons(TFTP_DATA);
+			pkt += loaded;
+			TftpFinalBlock = (loaded < toload);
+		}
+#endif
 		len = pkt - xp;
 		break;
 
@@ -312,7 +351,8 @@ TftpSend(void)
 		xp = pkt;
 		s = (ushort *)pkt;
 		*s++ = htons(TFTP_ERROR);
-		*s++ = htons(3);
+			*s++ = htons(3);
+
 		pkt = (uchar *)s;
 		strcpy((char *)pkt, "File too large");
 		pkt += 14 /*strlen("File too large")*/ + 1;
@@ -342,7 +382,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 {
 	ushort proto;
 	ushort *s;
-	int i;
+	int i, block;
 
 	if (dest != TftpOurPort) {
 #ifdef CONFIG_MCAST_TFTP
@@ -352,7 +392,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 			return;
 	}
 	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort &&
-	    TftpState != STATE_RECV_WRQ)
+	    TftpState != STATE_RECV_WRQ && TftpState != STATE_SEND_WRQ)
 		return;
 
 	if (len < 2)
@@ -362,11 +402,28 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 	s = (ushort *)pkt;
 	proto = *s++;
 	pkt = (uchar *)s;
+	block = ntohs(*s);
 	switch (ntohs(proto)) {
 
 	case TFTP_RRQ:
+		break;
+
 	case TFTP_ACK:
+#ifdef CONFIG_CMD_TFTPPUT
+		debug("Ack block %d\n", block);
+		show_block_marker();
+		if (TftpWriting) {
+			if (TftpFinalBlock) {
+				tftp_complete();
+			} else {
+				TftpBlock = block + 1;
+				show_block_marker();
+				TftpSend(); /* Send next data block */
+			}
+		}
+#endif
 		break;
+
 	default:
 		break;
 
@@ -417,7 +474,14 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 			TftpState = STATE_DATA;	/* passive.. */
 		else
 #endif
-		TftpSend(); /* Send ACK */
+#ifdef CONFIG_CMD_TFTPPUT
+		if (TftpWriting) {
+			/* Get ready to send the first block */
+			TftpState = STATE_DATA;
+			TftpBlock++;
+		}
+#endif
+		TftpSend(); /* Send ACK or first data block */
 		break;
 	case TFTP_DATA:
 		if (len < 2)
@@ -626,8 +690,8 @@ void TftpStart(proto_t protocol)
 	}
 
 	printf("Using %s device\n", eth_get_name());
-	printf("TFTP from server %pI4"
-		"; our IP address is %pI4", &TftpRemoteIP, &NetOurIP);
+	printf("TFTP %s server %pI4; our IP address is %pI4",
+	       protocol == TFTPPUT ? "to" : "from", &TftpRemoteIP, &NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
@@ -649,10 +713,21 @@ void TftpStart(proto_t protocol)
 
 	putc('\n');
 	TftpWriting = (protocol == TFTPPUT);
-
-	printf("Load address: 0x%lx\n", load_addr);
-
-	puts("Loading: *\b");
+#ifdef CONFIG_CMD_TFTPPUT
+	if (TftpWriting) {
+		printf("Save address: 0x%lx\n", save_addr);
+		printf("Save size:    0x%lx\n", save_size);
+		NetBootFileXferSize = save_size;
+		puts("Saving: *\b");
+		TftpState = STATE_SEND_WRQ;
+		TftpFinalBlock = 0;
+	}
+#endif
+	else {
+		printf("Load address: 0x%lx\n", load_addr);
+		puts("Loading: *\b");
+		TftpState = STATE_SEND_RRQ;
+	}
 
 	TftpTimeoutCountMax = TftpRRQTimeoutCountMax;
 
@@ -661,7 +736,6 @@ void TftpStart(proto_t protocol)
 
 	TftpRemotePort = WELL_KNOWN_PORT;
 	TftpTimeoutCount = 0;
-	TftpState = STATE_SEND_RRQ;
 	/* Use a pseudo-random port unless a specific port is set */
 	TftpOurPort = 1024 + (get_timer(0) % 3072);
 
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 8/8] tftpput: add tftpput command
  2011-10-22  4:51 [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Simon Glass
                   ` (6 preceding siblings ...)
  2011-10-22  4:51 ` [U-Boot] [PATCH 7/8] tftpput: implement tftp logic Simon Glass
@ 2011-10-22  4:51 ` Simon Glass
  2011-10-22  8:21 ` [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Albert ARIBAUD
  8 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2011-10-22  4:51 UTC (permalink / raw)
  To: u-boot

This adds a command to start a tftp put transfer.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/cmd_net.c         |   23 +++++++++++++++++++++++
 include/config_cmd_all.h |    1 +
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/common/cmd_net.c b/common/cmd_net.c
index f610385..f89a24b 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -52,6 +52,22 @@ U_BOOT_CMD(
 	"[loadAddress] [[hostIPaddr:]bootfilename]"
 );
 
+#ifdef CONFIG_CMD_TFTPPUT
+int do_tftpput(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	int ret;
+
+	ret = netboot_common(TFTPPUT, cmdtp, argc, argv);
+	return ret;
+}
+
+U_BOOT_CMD(
+	tftpput,	4,	1,	do_tftpput,
+	"TFTP put command, for uploading files to a server",
+	"Address Size [[hostIPaddr:]filename]"
+);
+#endif
+
 #ifdef CONFIG_CMD_TFTPSRV
 static int do_tftpsrv(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 {
@@ -203,6 +219,13 @@ static int netboot_common(enum proto_t proto, cmd_tbl_t *cmdtp, int argc,
 
 		break;
 
+#ifdef CONFIG_CMD_TFTPPUT
+	case 4:
+		save_addr = strict_strtoul(argv[1], NULL, 16);
+		save_size = strict_strtoul(argv[2], NULL, 16);
+		copy_filename(BootFile, argv[3], sizeof(BootFile));
+		break;
+#endif
 	default:
 		show_boot_progress (-80);
 		return cmd_usage(cmdtp);
diff --git a/include/config_cmd_all.h b/include/config_cmd_all.h
index 9716f9c..7fa8661 100644
--- a/include/config_cmd_all.h
+++ b/include/config_cmd_all.h
@@ -82,6 +82,7 @@
 #define CONFIG_CMD_SOURCE	/* "source" command support	*/
 #define CONFIG_CMD_SPI		/* SPI utility			*/
 #define CONFIG_CMD_TERMINAL	/* built-in Serial Terminal	*/
+#define CONFIG_CMD_TFTPPUT	/* TFTP upload			*/
 #define CONFIG_CMD_UBI		/* UBI Support			*/
 #define CONFIG_CMD_UBIFS	/* UBIFS Support		*/
 #define CONFIG_CMD_UNIVERSE	/* Tundra Universe Support	*/
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network
  2011-10-22  4:51 [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Simon Glass
                   ` (7 preceding siblings ...)
  2011-10-22  4:51 ` [U-Boot] [PATCH 8/8] tftpput: add tftpput command Simon Glass
@ 2011-10-22  8:21 ` Albert ARIBAUD
  2011-10-22 16:15   ` Simon Glass
  2011-10-30 20:13   ` Mike Frysinger
  8 siblings, 2 replies; 25+ messages in thread
From: Albert ARIBAUD @ 2011-10-22  8:21 UTC (permalink / raw)
  To: u-boot

Le 22/10/2011 06:51, Simon Glass a ?crit :
> The tftpboot command permits reading of files over a network interface
> using the Trivial FTP protocol. This patch series adds the ability to
> transfer files the other way.
>
> Why is this useful?
>
> - Uploading boot time data to a server
> - Uploading profiling information
> - Uploading large mounts of data for comparison / checking on a host
>      (e.g. use tftpput and ghex2 instead of the 'md' command)

Especially I find it interesting for backing up things like MTD and 
small disk files (not partitions, though). Most of my work currently is 
trying to bring mainline U-Boot support to existing boards with bad 
U-Boot implementations, and being able to backup things from U-Boot (as 
opposed to having to set up NFS root and Linux boot) would definitely be 
a plus.

> Mostly the existing code can be re-used and I have tried to avoid too
> much refactoring or cleaning up.

:)

> The feature is activated by the CONFIG_CMD_TFTPPUT option.
>
> This has been very lightly tested on a Seaboard with a USB network
> adaptor. I don't think it handles block number overflow.

What size does this limit transfers to?

> Simon Glass (8):
>    Move simple_itoa to vsprintf
>    Add setenv_uint() and setenv_addr()
>    tftpput: Rename TFTP to TFTPGET
>    tftpput: move common code into separate functions
>    tftpput: support selecting get/put for tftp
>    tftpput: add save_addr and save_size global variables
>    tftpput: implement tftp logic
>    tftpput: add tftpput command

Many U-Boot environments use 'tftp' as a shorthand to tftpboot. Did you 
verify that this is not broken by the introduction of 'tftpput'?

Also, I'd be happy to test this if a branch exists that already holds 
these commits.

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network
  2011-10-22  8:21 ` [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Albert ARIBAUD
@ 2011-10-22 16:15   ` Simon Glass
  2011-10-24  4:28     ` Simon Glass
  2011-10-30 20:13   ` Mike Frysinger
  1 sibling, 1 reply; 25+ messages in thread
From: Simon Glass @ 2011-10-22 16:15 UTC (permalink / raw)
  To: u-boot

Hi Albert,

On Sat, Oct 22, 2011 at 1:21 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 22/10/2011 06:51, Simon Glass a ?crit :
>>
>> The tftpboot command permits reading of files over a network interface
>> using the Trivial FTP protocol. This patch series adds the ability to
>> transfer files the other way.
>>
>> Why is this useful?
>>
>> - Uploading boot time data to a server
>> - Uploading profiling information
>> - Uploading large mounts of data for comparison / checking on a host
>> ? ? (e.g. use tftpput and ghex2 instead of the 'md' command)
>
> Especially I find it interesting for backing up things like MTD and small
> disk files (not partitions, though). Most of my work currently is trying to
> bring mainline U-Boot support to existing boards with bad U-Boot
> implementations, and being able to backup things from U-Boot (as opposed to
> having to set up NFS root and Linux boot) would definitely be a plus.
>
>> Mostly the existing code can be re-used and I have tried to avoid too
>> much refactoring or cleaning up.
>
> :)
>
>> The feature is activated by the CONFIG_CMD_TFTPPUT option.
>>
>> This has been very lightly tested on a Seaboard with a USB network
>> adaptor. I don't think it handles block number overflow.
>
> What size does this limit transfers to?

I think about 1468 * 65535 - around 95MB - it's fairly easy to fix
just by copying out the existing tftp get wrap code. I put it in the
commit message so it wouldn't get lost.

>
>> Simon Glass (8):
>> ? Move simple_itoa to vsprintf
>> ? Add setenv_uint() and setenv_addr()
>> ? tftpput: Rename TFTP to TFTPGET
>> ? tftpput: move common code into separate functions
>> ? tftpput: support selecting get/put for tftp
>> ? tftpput: add save_addr and save_size global variables
>> ? tftpput: implement tftp logic
>> ? tftpput: add tftpput command
>
> Many U-Boot environments use 'tftp' as a shorthand to tftpboot. Did you
> verify that this is not broken by the introduction of 'tftpput'?
>
> Also, I'd be happy to test this if a branch exists that already holds these
> commits.

I will see if I can organise one at Denx.

Regards,
Simon

>
> Amicalement,
> --
> Albert.
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-22  4:51 ` [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr() Simon Glass
@ 2011-10-23  5:29   ` Mike Frysinger
  2011-10-25 21:35     ` Simon Glass
  0 siblings, 1 reply; 25+ messages in thread
From: Mike Frysinger @ 2011-10-23  5:29 UTC (permalink / raw)
  To: u-boot

On Sat, Oct 22, 2011 at 00:51, Simon Glass <sjg@chromium.org> wrote:
> +int setenv_ulong(const char *varname, ulong value)
> +{
> + ? ? ? char *str = simple_itoa(value);
> +
> + ? ? ? return setenv(varname, str);
> +}

could be a one liner, but works either way

> +int setenv_addr(const char *varname, const void *addr)
> +{
> + ? ? ? char str[17];

char str[sizeof(addr) * 2 + 1];

> + ? ? ? sprintf(str, "%x", (uintptr_t)addr);

i wonder if we should use %p and drop the cast
-mike

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network
  2011-10-22 16:15   ` Simon Glass
@ 2011-10-24  4:28     ` Simon Glass
  2011-10-25  4:13       ` Simon Glass
  0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2011-10-24  4:28 UTC (permalink / raw)
  To: u-boot

Hi Albert,

On Sat, Oct 22, 2011 at 9:15 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Albert,
>
> On Sat, Oct 22, 2011 at 1:21 AM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>> Le 22/10/2011 06:51, Simon Glass a ?crit :
>>>
>>> The tftpboot command permits reading of files over a network interface
>>> using the Trivial FTP protocol. This patch series adds the ability to
>>> transfer files the other way.
>>>
>>> Why is this useful?
>>>
>>> - Uploading boot time data to a server
>>> - Uploading profiling information
>>> - Uploading large mounts of data for comparison / checking on a host
>>> ? ? (e.g. use tftpput and ghex2 instead of the 'md' command)
>>
>> Especially I find it interesting for backing up things like MTD and small
>> disk files (not partitions, though). Most of my work currently is trying to
>> bring mainline U-Boot support to existing boards with bad U-Boot
>> implementations, and being able to backup things from U-Boot (as opposed to
>> having to set up NFS root and Linux boot) would definitely be a plus.
>>
>>> Mostly the existing code can be re-used and I have tried to avoid too
>>> much refactoring or cleaning up.
>>
>> :)
>>
>>> The feature is activated by the CONFIG_CMD_TFTPPUT option.
>>>
>>> This has been very lightly tested on a Seaboard with a USB network
>>> adaptor. I don't think it handles block number overflow.
>>
>> What size does this limit transfers to?
>
> I think about 1468 * 65535 - around 95MB - it's fairly easy to fix
> just by copying out the existing tftp get wrap code. I put it in the
> commit message so it wouldn't get lost.
>
>>
>>> Simon Glass (8):
>>> ? Move simple_itoa to vsprintf
>>> ? Add setenv_uint() and setenv_addr()
>>> ? tftpput: Rename TFTP to TFTPGET
>>> ? tftpput: move common code into separate functions
>>> ? tftpput: support selecting get/put for tftp
>>> ? tftpput: add save_addr and save_size global variables
>>> ? tftpput: implement tftp logic
>>> ? tftpput: add tftpput command
>>
>> Many U-Boot environments use 'tftp' as a shorthand to tftpboot. Did you
>> verify that this is not broken by the introduction of 'tftpput'?
>>
>> Also, I'd be happy to test this if a branch exists that already holds these
>> commits.
>
> I will see if I can organise one at Denx.

Thanks to Wolfgang I have something you can try:

git clone git://git.denx.de/u-boot-simonglass
git checkout us-tftp

Regards,
Simon

>
> Regards,
> Simon
>
>>
>> Amicalement,
>> --
>> Albert.
>>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network
  2011-10-24  4:28     ` Simon Glass
@ 2011-10-25  4:13       ` Simon Glass
  2011-10-25  6:44         ` Albert ARIBAUD
  0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2011-10-25  4:13 UTC (permalink / raw)
  To: u-boot

Hi Albert,

On Sun, Oct 23, 2011 at 9:28 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Albert,
>
> On Sat, Oct 22, 2011 at 9:15 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Albert,
>>
>> On Sat, Oct 22, 2011 at 1:21 AM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net> wrote:
>>> Le 22/10/2011 06:51, Simon Glass a ?crit :
>>>>
>>>> The tftpboot command permits reading of files over a network interface
>>>> using the Trivial FTP protocol. This patch series adds the ability to
>>>> transfer files the other way.
>>>>
>>>> Why is this useful?
>>>>
>>>> - Uploading boot time data to a server
>>>> - Uploading profiling information
>>>> - Uploading large mounts of data for comparison / checking on a host
>>>> ? ? (e.g. use tftpput and ghex2 instead of the 'md' command)
>>>
>>> Especially I find it interesting for backing up things like MTD and small
>>> disk files (not partitions, though). Most of my work currently is trying to
>>> bring mainline U-Boot support to existing boards with bad U-Boot
>>> implementations, and being able to backup things from U-Boot (as opposed to
>>> having to set up NFS root and Linux boot) would definitely be a plus.
>>>
>>>> Mostly the existing code can be re-used and I have tried to avoid too
>>>> much refactoring or cleaning up.
>>>
>>> :)
>>>
>>>> The feature is activated by the CONFIG_CMD_TFTPPUT option.
>>>>
>>>> This has been very lightly tested on a Seaboard with a USB network
>>>> adaptor. I don't think it handles block number overflow.
>>>
>>> What size does this limit transfers to?
>>
>> I think about 1468 * 65535 - around 95MB - it's fairly easy to fix
>> just by copying out the existing tftp get wrap code. I put it in the
>> commit message so it wouldn't get lost.

I have fixed this in v2 and tested that it can wrap around many times
without trouble. There is a small issue that some servers wrap around
to 0 and others to 1. For tftpd-hpa at least it is 0, although the
write part seems broken anyway.

Unfortunately this involves a little more refactoring, since I didn't
want to duplicate code, and some of the functions are awfully long.

>>
>>>
>>>> Simon Glass (8):
>>>> ? Move simple_itoa to vsprintf
>>>> ? Add setenv_uint() and setenv_addr()
>>>> ? tftpput: Rename TFTP to TFTPGET
>>>> ? tftpput: move common code into separate functions
>>>> ? tftpput: support selecting get/put for tftp
>>>> ? tftpput: add save_addr and save_size global variables
>>>> ? tftpput: implement tftp logic
>>>> ? tftpput: add tftpput command
>>>
>>> Many U-Boot environments use 'tftp' as a shorthand to tftpboot. Did you
>>> verify that this is not broken by the introduction of 'tftpput'?

I forgot to answer this. No this will break 'tftp' as 'tftpserver'
does. I vaguely remember a discussion that this was correct, since
tftp is only an abbreviation and should not be used in scripts.

>>>
>>> Also, I'd be happy to test this if a branch exists that already holds these
>>> commits.
>>
>> I will see if I can organise one at Denx.
>
> Thanks to Wolfgang I have something you can try:
>
> git clone git://git.denx.de/u-boot-simonglass
> git checkout us-tftp

Try git checkout us-tftp2 for the new version.

Regards,
SImon

>>> Amicalement,
>>> --
>>> Albert.
>>>
>>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network
  2011-10-25  4:13       ` Simon Glass
@ 2011-10-25  6:44         ` Albert ARIBAUD
  0 siblings, 0 replies; 25+ messages in thread
From: Albert ARIBAUD @ 2011-10-25  6:44 UTC (permalink / raw)
  To: u-boot

Hi Simon,

Le 25/10/2011 06:13, Simon Glass a ?crit :

>>>> Many U-Boot environments use 'tftp' as a shorthand to tftpboot. Did you
>>>> verify that this is not broken by the introduction of 'tftpput'?
>
> I forgot to answer this. No this will break 'tftp' as 'tftpserver'
> does. I vaguely remember a discussion that this was correct, since
> tftp is only an abbreviation and should not be used in scripts.

Point taken. I'll have to fix some web pages floating around. :)

>>>> Also, I'd be happy to test this if a branch exists that already holds these
>>>> commits.
>>>
>>> I will see if I can organise one at Denx.
>>
>> Thanks to Wolfgang I have something you can try:
>>
>> git clone git://git.denx.de/u-boot-simonglass
>> git checkout us-tftp
>
> Try git checkout us-tftp2 for the new version.

Thanks! Will test it this evening.

> Regards,
> SImon

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-23  5:29   ` Mike Frysinger
@ 2011-10-25 21:35     ` Simon Glass
  2011-10-25 21:39       ` Mike Frysinger
  0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2011-10-25 21:35 UTC (permalink / raw)
  To: u-boot

Hi Mike

On Sat, Oct 22, 2011 at 10:29 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Sat, Oct 22, 2011 at 00:51, Simon Glass <sjg@chromium.org> wrote:
>> +int setenv_ulong(const char *varname, ulong value)
>> +{
>> + ? ? ? char *str = simple_itoa(value);
>> +
>> + ? ? ? return setenv(varname, str);
>> +}
>
> could be a one liner, but works either way

OK, was trying to separate that out deliberately.

>
>> +int setenv_addr(const char *varname, const void *addr)
>> +{
>> + ? ? ? char str[17];
>
> char str[sizeof(addr) * 2 + 1];

Yes of course!

>
>> + ? ? ? sprintf(str, "%x", (uintptr_t)addr);
>
> i wonder if we should use %p and drop the cast
> -mike
>

Is %p supposed to print a 0x before it or not? I saw some discussion
about this. I vote for %p no, and %#p yes.

I will tidy these up for v3.

Regards,
Simon

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-25 21:35     ` Simon Glass
@ 2011-10-25 21:39       ` Mike Frysinger
  2011-10-25 21:58         ` Simon Glass
  0 siblings, 1 reply; 25+ messages in thread
From: Mike Frysinger @ 2011-10-25 21:39 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 25, 2011 at 17:35, Simon Glass wrote:
> On Sat, Oct 22, 2011 at 10:29 PM, Mike Frysinger wrote:
>> On Sat, Oct 22, 2011 at 00:51, Simon Glass wrote:
>>> + ? ? ? sprintf(str, "%x", (uintptr_t)addr);
>>
>> i wonder if we should use %p and drop the cast
>
> Is %p supposed to print a 0x before it or not? I saw some discussion
> about this. I vote for %p no, and %#p yes.

%p does not currently output a leading 0x.  it should (imo), and i'll
prob send a patch to fix that.

but %x doesn't output a leading 0x either :).
-mike

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-25 21:39       ` Mike Frysinger
@ 2011-10-25 21:58         ` Simon Glass
  2011-10-25 22:01           ` Wolfgang Denk
  0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2011-10-25 21:58 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Tue, Oct 25, 2011 at 2:39 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tue, Oct 25, 2011 at 17:35, Simon Glass wrote:
>> On Sat, Oct 22, 2011 at 10:29 PM, Mike Frysinger wrote:
>>> On Sat, Oct 22, 2011 at 00:51, Simon Glass wrote:
>>>> + ? ? ? sprintf(str, "%x", (uintptr_t)addr);
>>>
>>> i wonder if we should use %p and drop the cast
>>
>> Is %p supposed to print a 0x before it or not? I saw some discussion
>> about this. I vote for %p no, and %#p yes.
>
> %p does not currently output a leading 0x. ?it should (imo), and i'll
> prob send a patch to fix that.
>
> but %x doesn't output a leading 0x either :).
> -mike
>

OK, well to be consistent with glibc we should probably add 0x as you say.

isn't it intended that env variables without a leading 0x in the value
be interpreted as hex still? Iwc do I really want the 0x?

Regards,
Simon

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-25 21:58         ` Simon Glass
@ 2011-10-25 22:01           ` Wolfgang Denk
  2011-10-25 22:08             ` Mike Frysinger
  0 siblings, 1 reply; 25+ messages in thread
From: Wolfgang Denk @ 2011-10-25 22:01 UTC (permalink / raw)
  To: u-boot

Dear Simon Glass,

In message <CAPnjgZ1+nTaAjYxq5Nh3i7XTh+RhPC9edVtHF34YNPA2PJcCuw@mail.gmail.com> you wrote:
> 
> isn't it intended that env variables without a leading 0x in the value
> be interpreted as hex still? Iwc do I really want the 0x?

Yes, this is intended.  No, we do NOT want to see the 0x in such
cases.

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 time the world beats a path to your door is when you are  in
the bathroom.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-25 22:01           ` Wolfgang Denk
@ 2011-10-25 22:08             ` Mike Frysinger
  2011-10-25 22:41               ` Simon Glass
  0 siblings, 1 reply; 25+ messages in thread
From: Mike Frysinger @ 2011-10-25 22:08 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 25, 2011 at 18:01, Wolfgang Denk wrote:
> Simon Glass wrote:
>> isn't it intended that env variables without a leading 0x in the value
>> be interpreted as hex still? Iwc do I really want the 0x?
>
> Yes, this is intended. ?No, we do NOT want to see the 0x in such
> cases.

ok, so we'll want to stick with %x like you already have Simon.  i
disagree, but not enough to fight over it ;).
-mike

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-25 22:08             ` Mike Frysinger
@ 2011-10-25 22:41               ` Simon Glass
  2011-10-25 22:49                 ` Mike Frysinger
  0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2011-10-25 22:41 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Tue, Oct 25, 2011 at 3:08 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tue, Oct 25, 2011 at 18:01, Wolfgang Denk wrote:
>> Simon Glass wrote:
>>> isn't it intended that env variables without a leading 0x in the value
>>> be interpreted as hex still? Iwc do I really want the 0x?
>>
>> Yes, this is intended. ?No, we do NOT want to see the 0x in such
>> cases.
>
> ok, so we'll want to stick with %x like you already have Simon. ?i
> disagree, but not enough to fight over it ;).
> -mike
>

OK. But I can use %p if you don't send your patch. How about %#p to
display 0x? It would be incompatible with glibc, but seems more
sensible. Then we could use this in quite a few places in U-Boot I
suspect.

Regards,
Simon

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-25 22:41               ` Simon Glass
@ 2011-10-25 22:49                 ` Mike Frysinger
  2011-10-25 23:08                   ` Simon Glass
  0 siblings, 1 reply; 25+ messages in thread
From: Mike Frysinger @ 2011-10-25 22:49 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 25, 2011 at 18:41, Simon Glass wrote:
> On Tue, Oct 25, 2011 at 3:08 PM, Mike Frysinger wrote:
>> On Tue, Oct 25, 2011 at 18:01, Wolfgang Denk wrote:
>>> Simon Glass wrote:
>>>> isn't it intended that env variables without a leading 0x in the value
>>>> be interpreted as hex still? Iwc do I really want the 0x?
>>>
>>> Yes, this is intended. ?No, we do NOT want to see the 0x in such
>>> cases.
>>
>> ok, so we'll want to stick with %x like you already have Simon. ?i
>> disagree, but not enough to fight over it ;).
>
> OK. But I can use %p if you don't send your patch. How about %#p to
> display 0x? It would be incompatible with glibc, but seems more
> sensible. Then we could use this in quite a few places in U-Boot I
> suspect.

i think wolfgang's point is that he doesn't want "0x" in env vars.  i
don't think he made any statement about %p in general.

also, %#p works the same under u-boot as under glibc.  it's just that
%#p generates a gcc warning.
-mike

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-25 22:49                 ` Mike Frysinger
@ 2011-10-25 23:08                   ` Simon Glass
  2011-10-25 23:27                     ` Mike Frysinger
  0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2011-10-25 23:08 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Tue, Oct 25, 2011 at 3:49 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tue, Oct 25, 2011 at 18:41, Simon Glass wrote:
>> On Tue, Oct 25, 2011 at 3:08 PM, Mike Frysinger wrote:
>>> On Tue, Oct 25, 2011 at 18:01, Wolfgang Denk wrote:
>>>> Simon Glass wrote:
>>>>> isn't it intended that env variables without a leading 0x in the value
>>>>> be interpreted as hex still? Iwc do I really want the 0x?
>>>>
>>>> Yes, this is intended. ?No, we do NOT want to see the 0x in such
>>>> cases.
>>>
>>> ok, so we'll want to stick with %x like you already have Simon. ?i
>>> disagree, but not enough to fight over it ;).
>>
>> OK. But I can use %p if you don't send your patch. How about %#p to
>> display 0x? It would be incompatible with glibc, but seems more
>> sensible. Then we could use this in quite a few places in U-Boot I
>> suspect.
>
> i think wolfgang's point is that he doesn't want "0x" in env vars. ?i
> don't think he made any statement about %p in general.

Yes I got that.
>
> also, %#p works the same under u-boot as under glibc. ?it's just that
> %#p generates a gcc warning.
> -mike
>

No I meant that for me glibc displays 0x whether or not I put # in
there...seems wrong to me.

Regards,
Simon

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr()
  2011-10-25 23:08                   ` Simon Glass
@ 2011-10-25 23:27                     ` Mike Frysinger
  0 siblings, 0 replies; 25+ messages in thread
From: Mike Frysinger @ 2011-10-25 23:27 UTC (permalink / raw)
  To: u-boot

On Tue, Oct 25, 2011 at 19:08, Simon Glass wrote:
> On Tue, Oct 25, 2011 at 3:49 PM, Mike Frysinger wrote:
>> also, %#p works the same under u-boot as under glibc. ?it's just that
>> %#p generates a gcc warning.
>
> No I meant that for me glibc displays 0x whether or not I put # in
> there...seems wrong to me.

i don't think so.  it's long done this, as does gcc and the Linux
kernel.  fixing u-boot to operate the same way as the rest of the
ecosystem makes sense to me.
-mike

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network
  2011-10-22  8:21 ` [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Albert ARIBAUD
  2011-10-22 16:15   ` Simon Glass
@ 2011-10-30 20:13   ` Mike Frysinger
  1 sibling, 0 replies; 25+ messages in thread
From: Mike Frysinger @ 2011-10-30 20:13 UTC (permalink / raw)
  To: u-boot

On Saturday 22 October 2011 04:21:18 Albert ARIBAUD wrote:
> Many U-Boot environments use 'tftp' as a shorthand to tftpboot. Did you
> verify that this is not broken by the introduction of 'tftpput'?

those boards are broken.  we can't sanely maintain support for people relying 
on autocompleted commands.

ignoring that, no one includes config_cmd_all.h, and no one enables this 
command.  if people do, and they don't test things, then that's even more 
their problem.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111030/52922e6f/attachment.pgp 

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2011-10-30 20:13 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-22  4:51 [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Simon Glass
2011-10-22  4:51 ` [U-Boot] [PATCH 1/8] Move simple_itoa to vsprintf Simon Glass
2011-10-22  4:51 ` [U-Boot] [PATCH 2/8] Add setenv_uint() and setenv_addr() Simon Glass
2011-10-23  5:29   ` Mike Frysinger
2011-10-25 21:35     ` Simon Glass
2011-10-25 21:39       ` Mike Frysinger
2011-10-25 21:58         ` Simon Glass
2011-10-25 22:01           ` Wolfgang Denk
2011-10-25 22:08             ` Mike Frysinger
2011-10-25 22:41               ` Simon Glass
2011-10-25 22:49                 ` Mike Frysinger
2011-10-25 23:08                   ` Simon Glass
2011-10-25 23:27                     ` Mike Frysinger
2011-10-22  4:51 ` [U-Boot] [PATCH 3/8] tftpput: Rename TFTP to TFTPGET Simon Glass
2011-10-22  4:51 ` [U-Boot] [PATCH 4/8] tftpput: move common code into separate functions Simon Glass
2011-10-22  4:51 ` [U-Boot] [PATCH 5/8] tftpput: support selecting get/put for tftp Simon Glass
2011-10-22  4:51 ` [U-Boot] [PATCH 6/8] tftpput: add save_addr and save_size global variables Simon Glass
2011-10-22  4:51 ` [U-Boot] [PATCH 7/8] tftpput: implement tftp logic Simon Glass
2011-10-22  4:51 ` [U-Boot] [PATCH 8/8] tftpput: add tftpput command Simon Glass
2011-10-22  8:21 ` [U-Boot] [PATCH 0/8] Add tftpput command for uploading files over network Albert ARIBAUD
2011-10-22 16:15   ` Simon Glass
2011-10-24  4:28     ` Simon Glass
2011-10-25  4:13       ` Simon Glass
2011-10-25  6:44         ` Albert ARIBAUD
2011-10-30 20:13   ` Mike Frysinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox