: Smart Autoboot
PATCH 1/1 : Smart Autoboot
Dear U-Boot community,
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.
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.
Kind regards,
- Johannes
PATCH 1/1 : Smart Autoboot
Dear "Johannes Thoma",
Quoted 7 lines · In message <[email protected]> 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_bootmis used else (unconditionally)do_gois 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.
Quoted 3 lines
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:
Quoted 5 lines
From
c631d05e225fa3f87b0a5cad0bb033063e61018cMon Sep 17 00:00:00 2001
From: Johannes Thoma <[email protected]>
Date: Thu, 28 Apr 2011 14:25:39 +0200
Subject: [PATCH] Smart autoboot
Quoted 7 lines
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_AUTOBOOTin 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.
Quoted 4 lines
--- README | 5 +++++ common/cmd_net.c | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-)
...
Review
Quoted 3 lines
(quoted diff) +3
-0+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.
Review
Quoted 2 lines
(quoted diff) +2
-0+ +
Don't add too much white space - a single blank line is enough.
Review netboot_common (proto_t proto, cmd_tbl_t *cmdtp, int argc, char * const argv[]) · line 221
Quoted 9 lines
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[])(quoted diff) +1
-1@@ -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.
...
Review netboot_common (proto_t proto, cmd_tbl_t *cmdtp, int argc, char * const argv[]) · line 223
Quoted 2 lines
(quoted diff) +2
-0+#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
Signature · 7 lines
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 Abstainer: A weak person who yields to the temptation of denying him- self a pleasure. A total abstainer is one who abstains from every- thing but abstention, and especially from inactivity in the affairs of others. - Ambrose Bierce
PATCH 1/1 : Smart Autoboot
Dear Wolfgang,
Thank you for your quick response. I agree that a scripting solution would be better, but I didn't research enough to find that out, sorry for that .. ;)
I will run the mentioned check scripts for the next patches, I hope they will be better.
All the best,
- Johannes
Signature · 2 lines
NEU: FreePhone - kostenlos mobil telefonieren und surfen! Jetzt informieren: http://www.gmx.net/de/go/freephone