Re: [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
Dear Simon Glass, In message 1314633910-8550-2-git-send-email-...@chromium.org you wrote: Tested-by: Simon Glass s...@chromium.org Signed-off-by: Simon Glass s...@chromium.org --- include/common.h |8 lib/vsprintf.c | 26 -- 2 files changed, 32 insertions(+), 2 deletions(-) Can we please make this feature conditional, so that boards that are not going to use it don't have to suffer from the increased code size? 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: w...@denx.de The combination of a number of things to make existence worthwhile. Yes, the philosophy of 'none,' meaning 'all.' -- Spock and Lincoln, The Savage Curtain, stardate 5906.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
Hi Wolfgang, On Mon, Oct 17, 2011 at 1:06 PM, Wolfgang Denk w...@denx.de wrote: Dear Simon Glass, In message 1314633910-8550-2-git-send-email-...@chromium.org you wrote: Tested-by: Simon Glass s...@chromium.org Signed-off-by: Simon Glass s...@chromium.org --- include/common.h | 8 lib/vsprintf.c | 26 -- 2 files changed, 32 insertions(+), 2 deletions(-) Can we please make this feature conditional, so that boards that are not going to use it don't have to suffer from the increased code size? We can easily make some of it conditional (the exported functions), but some of it is changes to make the low-level formatter respect an 'end' pointer. I will take a look as there might be something clever we can do there. We would also need to add, for example, an snprintf() macro which drops the numeric parameter, so that code which uses snprintf() continues to work. Regards, Simon 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: w...@denx.de The combination of a number of things to make existence worthwhile. Yes, the philosophy of 'none,' meaning 'all.' -- Spock and Lincoln, The Savage Curtain, stardate 5906.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
Hi Wofgang, On Mon, Oct 17, 2011 at 1:21 PM, Simon Glass s...@chromium.org wrote: Hi Wolfgang, On Mon, Oct 17, 2011 at 1:06 PM, Wolfgang Denk w...@denx.de wrote: Dear Simon Glass, In message 1314633910-8550-2-git-send-email-...@chromium.org you wrote: Tested-by: Simon Glass s...@chromium.org Signed-off-by: Simon Glass s...@chromium.org --- include/common.h | 8 lib/vsprintf.c | 26 -- 2 files changed, 32 insertions(+), 2 deletions(-) Can we please make this feature conditional, so that boards that are not going to use it don't have to suffer from the increased code size? We can easily make some of it conditional (the exported functions), but some of it is changes to make the low-level formatter respect an 'end' pointer. I will take a look as there might be something clever we can do there. We would also need to add, for example, an snprintf() macro which drops the numeric parameter, so that code which uses snprintf() continues to work. Regards, Simon Thanks. I was thinking of your code size comments in snprintf() when I wrote this - I sent another patch today which deals with that. I am hoping that that will remove one more barrier to To address your comment here the code size increase is 42 bytes on ARMv7 with this option. The patch I have just sent makes it optional. Regards, Simon 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: w...@denx.de The combination of a number of things to make existence worthwhile. Yes, the philosophy of 'none,' meaning 'all.' -- Spock and Lincoln, The Savage Curtain, stardate 5906.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
Hi Graeme, On Mon, Aug 29, 2011 at 4:15 PM, Graeme Russ graeme.r...@gmail.com wrote: Hi Simon On Tue, Aug 30, 2011 at 2:05 AM, Simon Glass s...@chromium.org wrote: Tested-by: Simon Glass s...@chromium.org Signed-off-by: Simon Glass s...@chromium.org --- include/common.h | 8 lib/vsprintf.c | 26 -- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/include/common.h b/include/common.h index 12a1074..856df9a 100644 --- a/include/common.h +++ b/include/common.h @@ -250,6 +250,14 @@ int last_stage_init(void); extern ulong monitor_flash_len; int mac_read_from_eeprom(void); [snip] +/* Provide a default function for when no console is available */ +static void __board_panic_no_console(const char *msg) +{ + /* Just return since we can't access the console */ +} + +void board_panic_no_console(const char *msg) __attribute__((weak, + alias(__board_panic_no_console))); + void panic(const char *fmt, ...) { + char str[CONFIG_SYS_PBSIZE]; va_list args; + va_start(args, fmt); - vprintf(fmt, args); - putc('\n'); + + /* TODO)sjg): Move to vsnprintf() when available */ + vsprintf(str, fmt, args); va_end(args); + + if (gd-have_console) { + puts(str); + putc('\n'); + } else { + board_panic_no_console(str); + } + This patch highlights why console squelching should be done in console.c. We are ending up with more and more if (gd-have_console) all over the place. With the console squelching patch I posted earlier, I think it might be easier to create a weak 'panic_printf()' which aliases to printf() which the board can override if it has the ability to generate emergency output before console is initialised. Console output will be squelched if the board does not define such an override. My reasoning for putting this in panic is that the board_panic_no_console() turns on all UARTS and might fiddle with baud rates, etc. It should only be called once. Very much an emergency function and not just a different implementation of puts(). That said, with your approach above, I suppose panic_printf() would only be called once. I would suggest panic_puts() since this would call less external code and be easier for boards to implement. Regards, Simon Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
Tested-by: Simon Glass s...@chromium.org Signed-off-by: Simon Glass s...@chromium.org --- include/common.h |8 lib/vsprintf.c | 26 -- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/include/common.h b/include/common.h index 12a1074..856df9a 100644 --- a/include/common.h +++ b/include/common.h @@ -250,6 +250,14 @@ intlast_stage_init(void); extern ulong monitor_flash_len; int mac_read_from_eeprom(void); +/* + * Called during a panic() when no console is available. The board should do + * its best to get a message to the user any way it can. This function should + * return if it can, in which case the system will either hang or reset. + * See panic(). + */ +void board_panic_no_console(const char *str); + /* common/flash.c */ void flash_perror (int); diff --git a/lib/vsprintf.c b/lib/vsprintf.c index c029fbb..fd742a9 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -24,6 +24,8 @@ # define NUM_TYPE long long #define noinline __attribute__((noinline)) +DECLARE_GLOBAL_DATA_PTR; + const char hex_asc[] = 0123456789abcdef; #define hex_asc_lo(x) hex_asc[((x) 0x0f)] #define hex_asc_hi(x) hex_asc[((x) 0xf0) 4] @@ -714,13 +716,33 @@ int sprintf(char * buf, const char *fmt, ...) return i; } +/* Provide a default function for when no console is available */ +static void __board_panic_no_console(const char *msg) +{ + /* Just return since we can't access the console */ +} + +void board_panic_no_console(const char *msg) __attribute__((weak, + alias(__board_panic_no_console))); + void panic(const char *fmt, ...) { + char str[CONFIG_SYS_PBSIZE]; va_list args; + va_start(args, fmt); - vprintf(fmt, args); - putc('\n'); + + /* TODO)sjg): Move to vsnprintf() when available */ + vsprintf(str, fmt, args); va_end(args); + + if (gd-have_console) { + puts(str); + putc('\n'); + } else { + board_panic_no_console(str); + } + #if defined (CONFIG_PANIC_HANG) hang(); #else -- 1.7.3.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
Hi Simon On Tue, Aug 30, 2011 at 2:05 AM, Simon Glass s...@chromium.org wrote: Tested-by: Simon Glass s...@chromium.org Signed-off-by: Simon Glass s...@chromium.org --- include/common.h |8 lib/vsprintf.c | 26 -- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/include/common.h b/include/common.h index 12a1074..856df9a 100644 --- a/include/common.h +++ b/include/common.h @@ -250,6 +250,14 @@ intlast_stage_init(void); extern ulong monitor_flash_len; int mac_read_from_eeprom(void); [snip] +/* Provide a default function for when no console is available */ +static void __board_panic_no_console(const char *msg) +{ + /* Just return since we can't access the console */ +} + +void board_panic_no_console(const char *msg) __attribute__((weak, + alias(__board_panic_no_console))); + void panic(const char *fmt, ...) { + char str[CONFIG_SYS_PBSIZE]; va_list args; + va_start(args, fmt); - vprintf(fmt, args); - putc('\n'); + + /* TODO)sjg): Move to vsnprintf() when available */ + vsprintf(str, fmt, args); va_end(args); + + if (gd-have_console) { + puts(str); + putc('\n'); + } else { + board_panic_no_console(str); + } + This patch highlights why console squelching should be done in console.c. We are ending up with more and more if (gd-have_console) all over the place. With the console squelching patch I posted earlier, I think it might be easier to create a weak 'panic_printf()' which aliases to printf() which the board can override if it has the ability to generate emergency output before console is initialised. Console output will be squelched if the board does not define such an override. Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot