All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [RFC PATCH 0/2] Early panic support
@ 2011-08-29 16:05 Simon Glass
  2011-08-29 16:05 ` [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors Simon Glass
  2011-08-29 16:05 ` [U-Boot] [PATCH 2/2] Add check that console is ready before output Simon Glass
  0 siblings, 2 replies; 14+ messages in thread
From: Simon Glass @ 2011-08-29 16:05 UTC (permalink / raw)
  To: u-boot

This patch set adds support for calling panic() before stdio is initialized.
At present this will just hang with little or no indication of what the
problem is.

A new board_panic_no_console() function is added to the board API. If provided
by the board it will be called in the event of a panic before the console is
ready. This function should turn on all UARTS and spray the message out if
it possibly can.

This is somewhat related to the Graeme's pre-console buffer patch, which is
why I am sending it for comment now.


Simon Glass (2):
  Add board_panic_no_console to deal with early critical errors
  Add check that console is ready before output

 common/console.c |    4 ++--
 include/common.h |    8 ++++++++
 lib/vsprintf.c   |   26 ++++++++++++++++++++++++--
 3 files changed, 34 insertions(+), 4 deletions(-)

-- 
1.7.3.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
  2011-08-29 16:05 [U-Boot] [RFC PATCH 0/2] Early panic support Simon Glass
@ 2011-08-29 16:05 ` Simon Glass
  2011-08-29 23:15   ` Graeme Russ
  2011-10-17 20:06   ` Wolfgang Denk
  2011-08-29 16:05 ` [U-Boot] [PATCH 2/2] Add check that console is ready before output Simon Glass
  1 sibling, 2 replies; 14+ messages in thread
From: Simon Glass @ 2011-08-29 16:05 UTC (permalink / raw)
  To: u-boot

Tested-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Simon Glass <sjg@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);
 
+/*
+ * 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 2/2] Add check that console is ready before output
  2011-08-29 16:05 [U-Boot] [RFC PATCH 0/2] Early panic support Simon Glass
  2011-08-29 16:05 ` [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors Simon Glass
@ 2011-08-29 16:05 ` Simon Glass
  2011-08-29 23:16   ` Graeme Russ
  2011-10-17 20:11   ` Wolfgang Denk
  1 sibling, 2 replies; 14+ messages in thread
From: Simon Glass @ 2011-08-29 16:05 UTC (permalink / raw)
  To: u-boot

If puts() or printf() is called before the console is ready, U-Boot will
either hang or die. This adds a check for this so that debug() can be used
in early code without concern that it will hang.

U-Boot boots properly

Tested-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
 common/console.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/console.c b/common/console.c
index 8c650e0..28ddb95 100644
--- a/common/console.c
+++ b/common/console.c
@@ -338,7 +338,7 @@ void putc(const char c)
 	if (gd->flags & GD_FLG_DEVINIT) {
 		/* Send to the standard output */
 		fputc(stdout, c);
-	} else {
+	} else if (gd->have_console) {
 		/* Send directly to the handler */
 		serial_putc(c);
 	}
@@ -359,7 +359,7 @@ void puts(const char *s)
 	if (gd->flags & GD_FLG_DEVINIT) {
 		/* Send to the standard output */
 		fputs(stdout, s);
-	} else {
+	} else if (gd->have_console) {
 		/* Send directly to the handler */
 		serial_puts(s);
 	}
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
  2011-08-29 16:05 ` [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors Simon Glass
@ 2011-08-29 23:15   ` Graeme Russ
  2011-08-30 15:32     ` Simon Glass
  2011-10-17 20:06   ` Wolfgang Denk
  1 sibling, 1 reply; 14+ messages in thread
From: Graeme Russ @ 2011-08-29 23:15 UTC (permalink / raw)
  To: u-boot

Hi Simon

On Tue, Aug 30, 2011 at 2:05 AM, Simon Glass <sjg@chromium.org> wrote:
> Tested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Simon Glass <sjg@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.

Regards,

Graeme

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 2/2] Add check that console is ready before output
  2011-08-29 16:05 ` [U-Boot] [PATCH 2/2] Add check that console is ready before output Simon Glass
@ 2011-08-29 23:16   ` Graeme Russ
  2011-10-17 20:11   ` Wolfgang Denk
  1 sibling, 0 replies; 14+ messages in thread
From: Graeme Russ @ 2011-08-29 23:16 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Tue, Aug 30, 2011 at 2:05 AM, Simon Glass <sjg@chromium.org> wrote:
> If puts() or printf() is called before the console is ready, U-Boot will
> either hang or die. This adds a check for this so that debug() can be used
> in early code without concern that it will hang.
>
> U-Boot boots properly
>
> Tested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> ?common/console.c | ? ?4 ++--
> ?1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/common/console.c b/common/console.c
> index 8c650e0..28ddb95 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -338,7 +338,7 @@ void putc(const char c)
> ? ? ? ?if (gd->flags & GD_FLG_DEVINIT) {
> ? ? ? ? ? ? ? ?/* Send to the standard output */
> ? ? ? ? ? ? ? ?fputc(stdout, c);
> - ? ? ? } else {
> + ? ? ? } else if (gd->have_console) {
> ? ? ? ? ? ? ? ?/* Send directly to the handler */
> ? ? ? ? ? ? ? ?serial_putc(c);
> ? ? ? ?}
> @@ -359,7 +359,7 @@ void puts(const char *s)
> ? ? ? ?if (gd->flags & GD_FLG_DEVINIT) {
> ? ? ? ? ? ? ? ?/* Send to the standard output */
> ? ? ? ? ? ? ? ?fputs(stdout, s);
> - ? ? ? } else {
> + ? ? ? } else if (gd->have_console) {
> ? ? ? ? ? ? ? ?/* Send directly to the handler */
> ? ? ? ? ? ? ? ?serial_puts(s);
> ? ? ? ?}

Please have a look at my console squelching patches

Regards,

Graeme

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
  2011-08-29 23:15   ` Graeme Russ
@ 2011-08-30 15:32     ` Simon Glass
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2011-08-30 15:32 UTC (permalink / raw)
  To: u-boot

Hi Graeme,

On Mon, Aug 29, 2011 at 4:15 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Hi Simon
>
> On Tue, Aug 30, 2011 at 2:05 AM, Simon Glass <sjg@chromium.org> wrote:
>> Tested-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Simon Glass <sjg@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
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
  2011-08-29 16:05 ` [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors Simon Glass
  2011-08-29 23:15   ` Graeme Russ
@ 2011-10-17 20:06   ` Wolfgang Denk
  2011-10-17 20:21     ` Simon Glass
  1 sibling, 1 reply; 14+ messages in thread
From: Wolfgang Denk @ 2011-10-17 20:06 UTC (permalink / raw)
  To: u-boot

Dear Simon Glass,

In message <1314633910-8550-2-git-send-email-sjg@chromium.org> you wrote:
> Tested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Simon Glass <sjg@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: wd at 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 2/2] Add check that console is ready before output
  2011-08-29 16:05 ` [U-Boot] [PATCH 2/2] Add check that console is ready before output Simon Glass
  2011-08-29 23:16   ` Graeme Russ
@ 2011-10-17 20:11   ` Wolfgang Denk
  2011-10-17 20:25     ` Simon Glass
  1 sibling, 1 reply; 14+ messages in thread
From: Wolfgang Denk @ 2011-10-17 20:11 UTC (permalink / raw)
  To: u-boot

Dear Simon Glass,

In message <1314633910-8550-3-git-send-email-sjg@chromium.org> you wrote:
> If puts() or printf() is called before the console is ready, U-Boot will
> either hang or die. This adds a check for this so that debug() can be used
> in early code without concern that it will hang.
> 
> U-Boot boots properly
> 
> Tested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  common/console.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Isn't this just hushing up implementation errors?

Before, the incorrect call would cause U-Boot to fail - a situation
which cannot be overlooked during the port, so the problem will
quickly be analyzed and fixed.

With your commit, everything appears to work fine, just some
(expected) output will be missing.  This is misleading at beats, and
will cause that buggy code will go undetected for a long time.

I don't think this is a good strategy.

Please comment.

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
Compassion -- that's the one things no machine ever had.  Maybe it's
the one thing that keeps men ahead of them.
	-- McCoy, "The Ultimate Computer", stardate 4731.3

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
  2011-10-17 20:06   ` Wolfgang Denk
@ 2011-10-17 20:21     ` Simon Glass
  2011-10-18  2:19       ` Simon Glass
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2011-10-17 20:21 UTC (permalink / raw)
  To: u-boot

Hi Wolfgang,

On Mon, Oct 17, 2011 at 1:06 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <1314633910-8550-2-git-send-email-sjg@chromium.org> you wrote:
>> Tested-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Simon Glass <sjg@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: wd at 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
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 2/2] Add check that console is ready before output
  2011-10-17 20:11   ` Wolfgang Denk
@ 2011-10-17 20:25     ` Simon Glass
  2011-10-17 20:37       ` Wolfgang Denk
  2011-10-17 20:38       ` Simon Glass
  0 siblings, 2 replies; 14+ messages in thread
From: Simon Glass @ 2011-10-17 20:25 UTC (permalink / raw)
  To: u-boot

Hi Wolfgang,

On Mon, Oct 17, 2011 at 1:11 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <1314633910-8550-3-git-send-email-sjg@chromium.org> you wrote:
>> If puts() or printf() is called before the console is ready, U-Boot will
>> either hang or die. This adds a check for this so that debug() can be used
>> in early code without concern that it will hang.
>>
>> U-Boot boots properly
>>
>> Tested-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> ?common/console.c | ? ?4 ++--
>> ?1 files changed, 2 insertions(+), 2 deletions(-)
>
> Isn't this just hushing up implementation errors?
>
> Before, the incorrect call would cause U-Boot to fail - a situation
> which cannot be overlooked during the port, so the problem will
> quickly be analyzed and fixed.
>
> With your commit, everything appears to work fine, just some
> (expected) output will be missing. ?This is misleading at beats, and
> will cause that buggy code will go undetected for a long time.
>
> I don't think this is a good strategy.
>
> Please comment.

Yes indeed -  I did actually submit an 'early panic' patch which I
should take another look at - it is called 'Add board_panic_no_console
to deal with early critical errors'. I think it was in the same patch
set but I'm not sure.

It would panic when console output was performed before the console
was ready - and therefore give the behaviour you desire.

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: wd at denx.de
> Compassion -- that's the one things no machine ever had. ?Maybe it's
> the one thing that keeps men ahead of them.
> ? ? ? ?-- McCoy, "The Ultimate Computer", stardate 4731.3
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 2/2] Add check that console is ready before output
  2011-10-17 20:25     ` Simon Glass
@ 2011-10-17 20:37       ` Wolfgang Denk
  2011-10-17 20:42         ` Simon Glass
  2011-10-17 20:38       ` Simon Glass
  1 sibling, 1 reply; 14+ messages in thread
From: Wolfgang Denk @ 2011-10-17 20:37 UTC (permalink / raw)
  To: u-boot

Dear Simon Glass,

In message <CAPnjgZ3WBmJBs1zCd4_jQsUZcO2BOAOK865CaL6tWeUX9zVVWg@mail.gmail.com> you wrote:
> 
> Yes indeed -  I did actually submit an 'early panic' patch which I
> should take another look at - it is called 'Add board_panic_no_console
> to deal with early critical errors'. I think it was in the same patch
> set but I'm not sure.

It was (I even commented on it).

> It would panic when console output was performed before the console
> was ready - and therefore give the behaviour you desire.

Well, but there will probably be a large number of boards that don't
implement this function (or don;t enable it if we make it
configurable).  And all these will now fail silently.

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
panic: kernel trap (ignored)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 2/2] Add check that console is ready before output
  2011-10-17 20:25     ` Simon Glass
  2011-10-17 20:37       ` Wolfgang Denk
@ 2011-10-17 20:38       ` Simon Glass
  1 sibling, 0 replies; 14+ messages in thread
From: Simon Glass @ 2011-10-17 20:38 UTC (permalink / raw)
  To: u-boot

Hi Wolfgang,

On Mon, Oct 17, 2011 at 1:25 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Wolfgang,
>
> On Mon, Oct 17, 2011 at 1:11 PM, Wolfgang Denk <wd@denx.de> wrote:
>> Dear Simon Glass,
>>
>> In message <1314633910-8550-3-git-send-email-sjg@chromium.org> you wrote:
>>> If puts() or printf() is called before the console is ready, U-Boot will
>>> either hang or die. This adds a check for this so that debug() can be used
>>> in early code without concern that it will hang.
>>>
>>> U-Boot boots properly
>>>
>>> Tested-by: Simon Glass <sjg@chromium.org>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>> ?common/console.c | ? ?4 ++--
>>> ?1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> Isn't this just hushing up implementation errors?
>>
>> Before, the incorrect call would cause U-Boot to fail - a situation
>> which cannot be overlooked during the port, so the problem will
>> quickly be analyzed and fixed.
>>
>> With your commit, everything appears to work fine, just some
>> (expected) output will be missing. ?This is misleading at beats, and
>> will cause that buggy code will go undetected for a long time.
>>
>> I don't think this is a good strategy.
>>
>> Please comment.
>
> Yes indeed - ?I did actually submit an 'early panic' patch which I
> should take another look at - it is called 'Add board_panic_no_console
> to deal with early critical errors'. I think it was in the same patch
> set but I'm not sure.
>
> It would panic when console output was performed before the console
> was ready - and therefore give the behaviour you desire.

I took another look at this - in fact this patch has been entirely
superseded by Graeme's:

commit e3e454cd72f319908355427b1a3ae54b3dd53356
Author: Graeme Russ <graeme.russ@gmail.com>
Date:   Mon Aug 29 02:14:05 2011 +0000

    console: Squelch pre-console output in console functions

so there is no need for me to resubmit. The other patch I mentioned,
The board_panic_no_console patch is still of interest though.

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: wd at denx.de
>> Compassion -- that's the one things no machine ever had. ?Maybe it's
>> the one thing that keeps men ahead of them.
>> ? ? ? ?-- McCoy, "The Ultimate Computer", stardate 4731.3
>>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 2/2] Add check that console is ready before output
  2011-10-17 20:37       ` Wolfgang Denk
@ 2011-10-17 20:42         ` Simon Glass
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2011-10-17 20:42 UTC (permalink / raw)
  To: u-boot

Hi Wolfgang,

On Mon, Oct 17, 2011 at 1:37 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ3WBmJBs1zCd4_jQsUZcO2BOAOK865CaL6tWeUX9zVVWg@mail.gmail.com> you wrote:
>>
>> Yes indeed - ?I did actually submit an 'early panic' patch which I
>> should take another look at - it is called 'Add board_panic_no_console
>> to deal with early critical errors'. I think it was in the same patch
>> set but I'm not sure.
>
> It was (I even commented on it).
>
>> It would panic when console output was performed before the console
>> was ready - and therefore give the behaviour you desire.
>
> Well, but there will probably be a large number of boards that don't
> implement this function (or don;t enable it if we make it
> configurable). ?And all these will now fail silently.

Sorry, just emailed again on this thread - actually the behaviour of
that patch has already come in due to Graeme's patch.

And I think it is actually the opposite - they will not fail silently,
but in fact ignore the pre-console printf() and continue. Previously
(before Graeme's patch and this one had it been applied) any
pre-console printf() would cause a silent failure / hang. I think this
was the intent of the fix.

The intent of my other patch in the series was to provide a way for a
board to panic before the console is ready, and hopefully get a
message to the user (flashing lights, LCD, init all the UARTs and
blast out a msg, etc.). It is true that most boards will not implement
this. We could perhaps look at implementing a weak function to do this
at the architecture level (but only for SOCs) but it seems risky.

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: wd at denx.de
> panic: kernel trap (ignored)
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors
  2011-10-17 20:21     ` Simon Glass
@ 2011-10-18  2:19       ` Simon Glass
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2011-10-18  2:19 UTC (permalink / raw)
  To: u-boot

Hi Wofgang,

On Mon, Oct 17, 2011 at 1:21 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Wolfgang,
>
> On Mon, Oct 17, 2011 at 1:06 PM, Wolfgang Denk <wd@denx.de> wrote:
>> Dear Simon Glass,
>>
>> In message <1314633910-8550-2-git-send-email-sjg@chromium.org> you wrote:
>>> Tested-by: Simon Glass <sjg@chromium.org>
>>> Signed-off-by: Simon Glass <sjg@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: wd at 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
>>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-10-18  2:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-29 16:05 [U-Boot] [RFC PATCH 0/2] Early panic support Simon Glass
2011-08-29 16:05 ` [U-Boot] [PATCH 1/2] Add board_panic_no_console to deal with early critical errors Simon Glass
2011-08-29 23:15   ` Graeme Russ
2011-08-30 15:32     ` Simon Glass
2011-10-17 20:06   ` Wolfgang Denk
2011-10-17 20:21     ` Simon Glass
2011-10-18  2:19       ` Simon Glass
2011-08-29 16:05 ` [U-Boot] [PATCH 2/2] Add check that console is ready before output Simon Glass
2011-08-29 23:16   ` Graeme Russ
2011-10-17 20:11   ` Wolfgang Denk
2011-10-17 20:25     ` Simon Glass
2011-10-17 20:37       ` Wolfgang Denk
2011-10-17 20:42         ` Simon Glass
2011-10-17 20:38       ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.