Dear "Johannes Thoma",
In message 20110428125810.123810@gmx.net you wrote:
I wrote a small patch that checks on autoboot if the image is a kernel image and if not assumes that it is a standalone image. If the image is kernel do_bootm is used else (unconditionally) do_go is used. This is handy if you are working with test applications and kernels, since you only need to reconfigure your DHCP server and don't need to re-flash u-boot.
The same can be done with minimal scripting, without changing any code. Your solution may work for you, but I don't think this is how to solve such an issue in general.
Since I am new to u-boot I'd like to ask if the following patch is formatted correctly (I have used git format-patch) and if there is the possiblity to add this feature to mainline UBoot.
Second part of the question first: I consider this a highly specific feature which is not needed by many people, and if it was needed, if can be implemented with mninimal scripting, so I don't think adding this code to mainline is a good idea.
For the formal issues: most things look good, except:
From c631d05e225fa3f87b0a5cad0bb033063e61018c Mon Sep 17 00:00:00 2001 From: Johannes Thoma johannes.thoma@gmx.at Date: Thu, 28 Apr 2011 14:25:39 +0200 Subject: [PATCH] Smart autoboot
We check if the image is a kernel image and use bootm command to boot the OS, else we do a go.
To use this feature, #define CONFIG_SMART_AUTOBOOT in your config and set the environment variable autostart to "yes". Then your DHCP driver can be used to either boot a kernel image or a u-boot application without changing anything on the target.
Signed-off-by: line missing.
README | 5 +++++ common/cmd_net.c | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-)
...
+extern int do_bootm (cmd_tbl_t *, int, int, char * const []); +#ifdef CONFIG_SMART_AUTOBOOT +extern int do_go (cmd_tbl_t *, int, int, char * const []);
externs should be avoided; use prototype declarations in the respective header files.
Don't add too much white space - a single blank line is enough.
static int netboot_common (proto_t, cmd_tbl_t *, int , char * const []);
int do_bootp (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) @@ -212,14 +218,29 @@ netboot_common (proto_t proto, cmd_tbl_t *cmdtp, int argc, char * const argv[])
/* Loading ok, check if we should attempt an auto-start */ if (((s = getenv("autostart")) != NULL) && (strcmp(s,"yes") == 0)) {
char *local_args[2];
char *local_args[3];
Indentation must be done by TABs, not spaces.
...
+#ifdef CONFIG_SMART_AUTOBOOT
if (image_get_type((void*) load_addr) == IH_TYPE_KERNEL)
Don't add trailing white space.
It appears you did not run checkpatch, which would have caught most of these issues.
Best regards,
Wolfgang Denk