This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
Re: [PATCH] add option to override redboot abort script check
Jose Vasconcellos wrote:
Gary Thomas wrote:
Andrew Lunn wrote:
On Mon, Aug 25, 2008 at 10:02:37PM -0400, Jose Vasconcellos wrote:
This patch factors out the test for ^C that is used to bypass the
start-up script.
By creating the function redboot_abort_script and declaring weak,
it's possible
to easily replace it for custom needs.
In my case, there's no easily accessible serial port so get to the
redboot prompt
telnet is used. However, if there's a boot script, there needs to be
a way for the
user to abort it in an easy manner. So, in my platform specific
code, the
redboot_abort_script is replaced by code to check for to see button
if a button
is pressed.
What i don't like about this is that it removes the normal ^C
handling. For you that is probably O.K. however in other situations,
it would be better to add additional mechanism, rather than replace
mechanisms. So in the field you can use a jumper on a GPIO, but the
developers desk which does have a serial port, maybe via a bed of
nails, can continue to use ^C etc.
Indeed, the initial discussion did not mention using weak functions
(although I suspected this). I'm also not keen on a wholesale
replacement.
Redboot makes a lot of use of HAL tables, eg for adding new commands,
idle handlers, etc. What i would suggest is adding a new table for
script abort functions. The ^C handler would be the first entry in the
table. HALs or other packages could then add more entries, eg read a
GPIO line etc. If any of these functions return true, the script is
aborted.
Another possibility would be to use a HAL defined macro, in the
same spirit as HAL_MEM_REAL_REGION_TOP which is used in that same
file (main.c)
Thank you for your feedback.
The patch simply makes a function out of an internal loop; by adding a weak
reference it may be overridden for special needs. The change involved is
quite minimal, and it doesn't have any impact if not used. Andrew's and
Gary's
suggestions may be more general but it's not clear that this is necessary.
After all, redboot has been around a very long time and this hasn't been
brought
out before.
Sorry, but your comment is antithetic to the eCos/RedBoot philosophy.
Even if this code *has* been around for years, unchanged, it behooves us
to make changes which are both flexible and follow the existing model.
The attached patch affects the same change you'd like, with the benefit of
retaining existing functionality (e.g. being able to connect and abort
via ^C) while giving your target platform the ability to handle the abort
button. Simply define the macro HAL_REDBOOT_ABORT_FUNCTION in your HAL
to return true on abort and you're done.
--
------------------------------------------------------------
Gary Thomas | Consulting for the
MLB Associates | Embedded world
------------------------------------------------------------
Index: redboot/current/src/main.c
===================================================================
RCS file: /srv/misc/cvsfiles/ecos/packages/redboot/current/src/main.c,v
retrieving revision 1.66
diff -u -5 -p -r1.66 main.c
--- redboot/current/src/main.c 20 Jul 2006 20:27:47 -0000 1.66
+++ redboot/current/src/main.c 26 Aug 2008 15:48:28 -0000
@@ -342,10 +342,16 @@ cyg_start(void)
diag_printf("== Executing boot script in %d.%03d seconds - enter ^C to abort\n",
script_timeout_ms/1000, script_timeout_ms%1000);
script = (unsigned char *)0;
res = _GETS_CTRLC; // Treat 0 timeout as ^C
while (script_timeout_ms >= CYGNUM_REDBOOT_CLI_IDLE_TIMEOUT) {
+#ifdef HAL_REDBOOT_ABORT_FUNCTION
+ if (HAL_REDBOOT_ABORT_FUNCTION()) {
+ res == _GETS_CTRLC;
+ break;
+ }
+#endif
res = _rb_gets(line, sizeof(line), CYGNUM_REDBOOT_CLI_IDLE_TIMEOUT);
if (res >= _GETS_OK) {
diag_printf("== Executing boot script in %d.%03d seconds - enter ^C to abort\n",
script_timeout_ms/1000, script_timeout_ms%1000);
continue; // Ignore anything but ^C