[U-Boot] [PATCH] bootvx: Cleanup & improvements

[U-Boot] [PATCH] bootvx: Cleanup & improvements

[Niklaus Giger]

The following improvements are made:
- document bootvx in README
- fix size too small by one in sprintf
- changed old (pre 2004) device name ibmEmac to emac
- boot device may be overriden in board config
- servername may be defined in board config
- additional parameters may be defined in board config
- fixed some line wrappings
- changed netstal-common.h to use the new settings

---
 README                           |   17 ++++++++
 common/cmd_elf.c                 |   80 ++++++++++++++++++--------------------
 include/configs/netstal-common.h |   29 +++++++-------
 3 files changed, 69 insertions(+), 57 deletions(-)

diff --git a/README b/README
index ebee20f..99d2a3d 100644
--- a/README
+++ b/README
@@ -379,6 +379,23 @@ The following options need to be configured:
 		This define fills in the correct boot CPU in the boot
 		param header, the default value is zero if undefined.
 
+- vxWorks boot parameters:
+
+		bootvx constructs a valid bootline using the following
+		environments variables: bootfile, ipaddr, serverip, hostname.
+		It loads the vxWorks image pointed bootfile.
+
+		CONFIG_SYS_VXWORKS_BOOT_DEVICE - The vxworks device name
+		CONFIG_SYS_VXWORKS_MAC_PTR - Ethernet 6 byte MA -address
+		CONFIG_SYS_VXWORKS_SERVERNAME - Name of the server
+
+		CONFIG_SYS_VXWORKS_ADD_PARAMS
+
+		Add it at the end of the bootline. E.g "u=username pw=secret"
+
+		Note: If a "bootargs" environment is defined, it will overwride
+		the defaults discussed just above.
+
 - Serial Ports:
 		CONFIG_PL010_SERIAL
 
diff --git a/common/cmd_elf.c b/common/cmd_elf.c
index 3ebb6d9..d537478 100644
--- a/common/cmd_elf.c
+++ b/common/cmd_elf.c
@@ -102,10 +102,8 @@ int do_bootvx (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	unsigned long bootaddr;		/* Address to put the bootline */
 	char *bootline;			/* Text of the bootline        */
 	char *tmp;			/* Temporary char pointer      */
+	char build_buf[128];		/* Buffer for building the bootline */
 
-#if defined(CONFIG_4xx) || defined(CONFIG_IOP480)
-	char build_buf[80];		/* Buffer for building the bootline */
-#endif
 	/* -------------------------------------------------- */
 
 	/*
@@ -124,7 +122,8 @@ int do_bootvx (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	if ((argc == 2) && (strcmp (argv[1], "tftp") == 0)) {
 		if (NetLoop (TFTP) <= 0)
 			return 1;
-		printf ("Automatic boot of VxWorks image at address 0x%08lx ... \n", addr);
+		printf ("Automatic boot of VxWorks image@address 0x%08lx ... \n",
+		     addr);
 	}
 #endif
 
@@ -163,54 +162,53 @@ int do_bootvx (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	 */
 
 	if ((bootline = getenv ("bootargs")) != NULL) {
-		memcpy ((void *) bootaddr, bootline, MAX(strlen(bootline), 255));
-		flush_cache (bootaddr, MAX(strlen(bootline), 255));
+		memcpy ((void *) bootaddr, bootline,
+			MAX (strlen (bootline), 255));
+		flush_cache (bootaddr, MAX (strlen (bootline), 255));
 	} else {
+
+#ifndef CONFIG_SYS_VXWORKS_BOOT_DEVICE
 #if defined(CONFIG_4xx)
-		sprintf (build_buf, "ibmEmac(0,0)");
+#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "emac(0,0)"
+#elif defined(CONFIG_IOP480)
+#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "dc(0,0)"
+#else
+#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "eth(0,0)"
+#endif
+#endif
 
-		if ((tmp = getenv ("hostname")) != NULL) {
-			sprintf (&build_buf[strlen (build_buf - 1)],
-				"host:%s ", tmp);
+#ifndef CONFIG_SYS_VXWORKS_SERVERNAME
+#define CONFIG_SYS_VXWORKS_SERVERNAME srv
+#endif
+
+		sprintf (build_buf, CONFIG_SYS_VXWORKS_BOOT_DEVICE);
+		if ((tmp = getenv ("bootfile")) != NULL) {
+			sprintf (&build_buf[strlen (build_buf)],
+				 "%s:%s ", CONFIG_SYS_VXWORKS_SERVERNAME, tmp);
 		} else {
-			sprintf (&build_buf[strlen (build_buf - 1)],
-				": ");
+			sprintf (&build_buf[strlen (build_buf)],
+				 "%s:file ", CONFIG_SYS_VXWORKS_SERVERNAME);
 		}
 
 		if ((tmp = getenv ("ipaddr")) != NULL) {
-			sprintf (&build_buf[strlen (build_buf - 1)],
-				"e=%s ", tmp);
+			sprintf (&build_buf[strlen (build_buf)], "e=%s ", tmp);
 		}
-		memcpy ((void *)bootaddr, build_buf, MAX(strlen(build_buf), 255));
-		flush_cache (bootaddr, MAX(strlen(build_buf), 255));
-#elif defined(CONFIG_IOP480)
-		sprintf (build_buf, "dc(0,0)");
 
-		if ((tmp = getenv ("hostname")) != NULL) {
-			sprintf (&build_buf[strlen (build_buf - 1)],
-				"host:%s ", tmp);
-		} else {
-			sprintf (&build_buf[strlen (build_buf - 1)],
-				": ");
+		if ((tmp = getenv ("serverip")) != NULL) {
+			sprintf (&build_buf[strlen (build_buf)], "h=%s ", tmp);
 		}
 
-		if ((tmp = getenv ("ipaddr")) != NULL) {
-			sprintf (&build_buf[strlen (build_buf - 1)],
-				"e=%s ", tmp);
+		if ((tmp = getenv ("hostname")) != NULL) {
+			sprintf (&build_buf[strlen (build_buf)], "tn=%s ", tmp);
 		}
-		memcpy ((void *) bootaddr, build_buf, MAX(strlen(build_buf), 255));
-		flush_cache (bootaddr, MAX(strlen(build_buf), 255));
-#else
-
-		/*
-		 * I'm not sure what the device should be for other
-		 * PPC flavors, the hostname and ipaddr should be ok
-		 * to just copy
-		 */
-
-		puts ("No bootargs defined\n");
-		return 1;
+#ifdef CONFIG_SYS_VXWORKS_ADD_PARAMS
+		sprintf (&build_buf[strlen (build_buf)],
+			 CONFIG_SYS_VXWORKS_ADD_PARAMS);
 #endif
+
+		memcpy ((void *) bootaddr, build_buf,
+			MAX (strlen (build_buf), 255));
+		flush_cache (bootaddr, MAX (strlen (build_buf), 255));
 	}
 
 	/*
@@ -255,8 +253,7 @@ int valid_elf_image (unsigned long addr)
 	}
 
 	if (ehdr->e_type != ET_EXEC) {
-		printf ("## Not a 32-bit elf image at address 0x%08lx\n",
-			addr);
+		printf ("## Not a 32-bit elf image at address 0x%08lx\n", addr);
 		return 0;
 	}
 
@@ -271,7 +268,6 @@ int valid_elf_image (unsigned long addr)
 	return 1;
 }
 
-
 /* ======================================================================
  * A very simple elf loader, assumes the image is valid, returns the
  * entry point address.
diff --git a/include/configs/netstal-common.h b/include/configs/netstal-common.h
index 1fa4b00..7f2b015 100644
--- a/include/configs/netstal-common.h
+++ b/include/configs/netstal-common.h
@@ -201,8 +201,9 @@
 #define CONFIG_ETHADDR      00:60:13:00:00:00   /* Netstal Machines AG MAC */
 #define CONFIG_OVERWRITE_ETHADDR_ONCE
 
-#define CONFIG_SYS_TFTP_LOADADDR 0x01000000
-
+#define CONFIG_SYS_TFTP_LOADADDR	0x01000000
+#define CONFIG_SYS_VXWORKS_ADD_PARAMS	"u=dpu pw=netstal8752"
+#define CONFIG_SYS_VXWORKS_SERVERNAME	"c"
 /*
  * General common environment variables shared by all boards produced by Netstal Maschinen
  */
@@ -222,19 +223,17 @@
 	"fdt_addr_r=800000\0"						\
 	"hostname=" xstr(CONFIG_HOSTNAME) "\0"				\
 	"bootfile=" xstr(CONFIG_HOSTNAME) "/uImage\0"			\
-	"load=tftp 200000 " xstr(CONFIG_HOSTNAME) "/u-boot.bin\0"	\
-	"update=protect off " xstr(CONFIG_SYS_MONITOR_BASE) " FFFFFFFF;"	\
-		"era " xstr(CONFIG_SYS_MONITOR_BASE) " FFFFFFFF;"		\
-		"cp.b ${fileaddr} " xstr(CONFIG_SYS_MONITOR_BASE) " ${filesize};" \
-		"setenv filesize\0"					\
-	"upd=run load update\0"						\
-	"vx_rom=" xstr(CONFIG_HOSTNAME) "/"     			\
-	xstr(CONFIG_HOSTNAME) "_vx_rom\0"				\
-	"vx=tftp " xstr(CONFIG_SYS_TFTP_LOADADDR) " ${vx_rom};run vxargs;"	\
-	"bootvx\0"							\
-	"vxargs=setenv bootargs emac(0,0)c:${vx_rom} e=${ipaddr}"	\
-	" h=${serverip} u=dpu pw=netstal8752 "				\
-	"tn=" xstr(CONFIG_HOSTNAME) " f=0x3008\0"			\
+	"uload=tftp " xstr(CONFIG_SYS_TFTP_LOADADDR) " "		\
+		xstr(CONFIG_HOSTNAME) "/u-boot.bin\0"			\
+	"vx_rom=" xstr(CONFIG_HOSTNAME) "/"				\
+		xstr(CONFIG_HOSTNAME) "_vx_rom\0"			\
+	"update=protect off " xstr(CONFIG_SYS_MONITOR_BASE) " FFFFFFFF;"\
+		"era " xstr(CONFIG_SYS_MONITOR_BASE) " FFFFFFFF;"	\
+		"cp.b ${fileaddr} "xstr(CONFIG_SYS_MONITOR_BASE) 	\
+		" ${filesize}; setenv filesize\0"			\
+	"upd=run uload update\0"					\
+	"vx=setenv bootfile ${vx_rom}; tftp " 				\
+		xstr(CONFIG_SYS_TFTP_LOADADDR) "; bootvx\0"		\
 	CONFIG_NETSTAL_DEF_ENV_ROOTPATH
 
 /*
-- 
1.5.4.3

[U-Boot] [PATCH] bootvx: Cleanup & improvements

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:54 Mon 03 Nov     , Niklaus Giger wrote:
> The following improvements are made:
> - document bootvx in README
> - fix size too small by one in sprintf
> - changed old (pre 2004) device name ibmEmac to emac
> - boot device may be overriden in board config
> - servername may be defined in board config
> - additional parameters may be defined in board config
> - fixed some line wrappings
> - changed netstal-common.h to use the new settings
Could you split your patch a smallest changeset?
> 
> ---
>  README                           |   17 ++++++++
>  common/cmd_elf.c                 |   80 ++++++++++++++++++--------------------
>  include/configs/netstal-common.h |   29 +++++++-------
>  3 files changed, 69 insertions(+), 57 deletions(-)
> 
> diff --git a/README b/README
> index ebee20f..99d2a3d 100644
> --- a/README
> +++ b/README
> @@ -379,6 +379,23 @@ The following options need to be configured:
>  		This define fills in the correct boot CPU in the boot
>  		param header, the default value is zero if undefined.
>  
> +- vxWorks boot parameters:
> +
> +		bootvx constructs a valid bootline using the following
> +		environments variables: bootfile, ipaddr, serverip, hostname.
> +		It loads the vxWorks image pointed bootfile.
> +
> +		CONFIG_SYS_VXWORKS_BOOT_DEVICE - The vxworks device name
> +		CONFIG_SYS_VXWORKS_MAC_PTR - Ethernet 6 byte MA -address
> +		CONFIG_SYS_VXWORKS_SERVERNAME - Name of the server
> +
> +		CONFIG_SYS_VXWORKS_ADD_PARAMS
> +
> +		Add it at the end of the bootline. E.g "u=username pw=secret"
> +
> +		Note: If a "bootargs" environment is defined, it will overwride
> +		the defaults discussed just above.
> +
>  - Serial Ports:
>  		CONFIG_PL010_SERIAL
>  
> diff --git a/common/cmd_elf.c b/common/cmd_elf.c
> index 3ebb6d9..d537478 100644
> --- a/common/cmd_elf.c
> +++ b/common/cmd_elf.c
> @@ -102,10 +102,8 @@ int do_bootvx (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  	unsigned long bootaddr;		/* Address to put the bootline */
>  	char *bootline;			/* Text of the bootline        */
>  	char *tmp;			/* Temporary char pointer      */
> +	char build_buf[128];		/* Buffer for building the bootline */
>  
> -#if defined(CONFIG_4xx) || defined(CONFIG_IOP480)
> -	char build_buf[80];		/* Buffer for building the bootline */
> -#endif
>  	/* -------------------------------------------------- */
>  
>  	/*
> @@ -124,7 +122,8 @@ int do_bootvx (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  	if ((argc == 2) && (strcmp (argv[1], "tftp") == 0)) {
>  		if (NetLoop (TFTP) <= 0)
>  			return 1;
> -		printf ("Automatic boot of VxWorks image at address 0x%08lx ... \n", addr);
> +		printf ("Automatic boot of VxWorks image at address 0x%08lx ... \n",
> +		     addr);
>  	}
>  #endif
>  
> @@ -163,54 +162,53 @@ int do_bootvx (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  	 */
>  
>  	if ((bootline = getenv ("bootargs")) != NULL) {
> -		memcpy ((void *) bootaddr, bootline, MAX(strlen(bootline), 255));
> -		flush_cache (bootaddr, MAX(strlen(bootline), 255));
> +		memcpy ((void *) bootaddr, bootline,
> +			MAX (strlen (bootline), 255));
> +		flush_cache (bootaddr, MAX (strlen (bootline), 255));
>  	} else {
> +
> +#ifndef CONFIG_SYS_VXWORKS_BOOT_DEVICE
>  #if defined(CONFIG_4xx)
> -		sprintf (build_buf, "ibmEmac(0,0)");
> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "emac(0,0)"
> +#elif defined(CONFIG_IOP480)
> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "dc(0,0)"
> +#else
> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "eth(0,0)"
> +#endif
> +#endif
could we move thie define to an header?

Best Regards,
J.

[U-Boot] [PATCH] bootvx: Cleanup & improvements

[Niklaus Giger]

Jean-Christophe PLAGNIOL-VILLARD wrote:

> On 10:54 Mon 03 Nov     , Niklaus Giger wrote:
>> The following improvements are made:
>> - document bootvx in README
>> - fix size too small by one in sprintf
>> - changed old (pre 2004) device name ibmEmac to emac
>> - boot device may be overriden in board config
>> - servername may be defined in board config
>> - additional parameters may be defined in board config
>> - fixed some line wrappings
>> - changed netstal-common.h to use the new settings
> Could you split your patch a smallest changeset?
How many changeset do you want? Is it sufficient to make two by just isolating the changes in netstal-common.h?
<,,.>
>> +#ifndef CONFIG_SYS_VXWORKS_BOOT_DEVICE
>>  #if defined(CONFIG_4xx)
>> -		sprintf (build_buf, "ibmEmac(0,0)");
>> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "emac(0,0)"
>> +#elif defined(CONFIG_IOP480)
>> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "dc(0,0)"
>> +#else
>> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "eth(0,0)"
>> +#endif
>> +#endif
> could we move thie define to an header?
Which header shall I use? Is common.h right?

Mmm. Looking at common.h I found that there max is defined with the same meaning as in cmd_elf.h. Probably worth a fix too.

Best regards

Niklaus

[U-Boot] [PATCH] bootvx: Cleanup & improvements

[Jean-Christophe PLAGNIOL-VILLARD]

On 19:59 Mon 03 Nov     , Niklaus Giger wrote:
> Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > On 10:54 Mon 03 Nov     , Niklaus Giger wrote:
> >> The following improvements are made:
> >> - document bootvx in README
> >> - fix size too small by one in sprintf
> >> - changed old (pre 2004) device name ibmEmac to emac
> >> - boot device may be overriden in board config
> >> - servername may be defined in board config
> >> - additional parameters may be defined in board config
> >> - fixed some line wrappings
> >> - changed netstal-common.h to use the new settings
> > Could you split your patch a smallest changeset?
> How many changeset do you want? Is it sufficient to make two by just isolating the changes in netstal-common.h?
> <,,.>
> >> +#ifndef CONFIG_SYS_VXWORKS_BOOT_DEVICE
> >>  #if defined(CONFIG_4xx)
> >> -		sprintf (build_buf, "ibmEmac(0,0)");
> >> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "emac(0,0)"
> >> +#elif defined(CONFIG_IOP480)
> >> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "dc(0,0)"
> >> +#else
> >> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "eth(0,0)"
> >> +#endif
> >> +#endif
> > could we move thie define to an header?
> Which header shall I use? Is common.h right?
> 
> Mmm. Looking at common.h I found that there max is defined with the same meaning as in cmd_elf.h. Probably worth a fix too.
> 
Maybe wxvorks.h

Best Regards,
J.

[U-Boot] [PATCH] bootvx: Cleanup & improvements

[Wolfgang Denk]

Dear Niklaus Giger,

In message <genhn6$6su$1@ger.gmane.org> you wrote:
>
> >> The following improvements are made:
> >> - document bootvx in README
> >> - fix size too small by one in sprintf
> >> - changed old (pre 2004) device name ibmEmac to emac
> >> - boot device may be overriden in board config
> >> - servername may be defined in board config
> >> - additional parameters may be defined in board config
> >> - fixed some line wrappings
> >> - changed netstal-common.h to use the new settings
> > Could you split your patch a smallest changeset?
> How many changeset do you want? Is it sufficient to make two by just isolating the changes in netstal-common.h?

Please see http://www.denx.de/wiki/U-Boot/Patches:

	Changes that contain different, unrelated modifications shall
	be submitted as separate patches, one patch per changeset. 

> >> +#ifndef CONFIG_SYS_VXWORKS_BOOT_DEVICE
> >>  #if defined(CONFIG_4xx)
> >> -		sprintf (build_buf, "ibmEmac(0,0)");
> >> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "emac(0,0)"
> >> +#elif defined(CONFIG_IOP480)
> >> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "dc(0,0)"
> >> +#else
> >> +#define		CONFIG_SYS_VXWORKS_BOOT_DEVICE "eth(0,0)"
> >> +#endif
> >> +#endif
> > could we move thie define to an header?
> Which header shall I use? Is common.h right?

For VxWorks boot device names? Not really. vxworks.h ?

> Mmm. Looking at common.h I found that there max is defined with the same meaning as in cmd_elf.h. Probably worth a fix too.

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 universe contains any amount of horrible ways  to  be  woken  up,
such as the noise of the mob breaking down the front door, the scream
of fire engines, or the realization that today is the Monday which on
Friday night was a comfortably long way off.
                                 - Terry Pratchett, _Moving Pictures_