[U-Boot] [RFC][PATCH] Pre-console buffer

[U-Boot] [RFC][PATCH] Pre-console buffer

[Graeme Russ]

This cropped up as an aside to another thread so I thought I would give
it a go. It's pretty rough-and-ready but it does the trick :)

---
 arch/x86/cpu/cpu.c                 |    2 +
 arch/x86/include/asm/global_data.h |   14 +++++----
 common/console.c                   |   56 ++++++++++++++++++++++++++++--------
 include/configs/eNET.h             |    5 ++-
 4 files changed, 58 insertions(+), 19 deletions(-)

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index cac12c0..602fb51 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -90,6 +90,8 @@ int x86_cpu_init_f(void)
 	const u32 em_rst = ~X86_CR0_EM;
 	const u32 mp_ne_set = X86_CR0_MP | X86_CR0_NE;

+	printf("Example pre-console printf()\n");
+
 	/* initialize FPU, reset EM, set MP and NE */
 	asm ("fninit\n" \
 	     "movl %%cr0, %%eax\n" \
diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
index 2902e61..4ebc5bd 100644
--- a/arch/x86/include/asm/global_data.h
+++ b/arch/x86/include/asm/global_data.h
@@ -44,6 +44,7 @@ typedef	struct global_data {
 	unsigned long	env_addr;	/* Address  of Environment struct */
 	unsigned long	cpu_clk;	/* CPU clock in Hz!		*/
 	unsigned long	bus_clk;
+	unsigned long	con_buf_idx;	/* Console buffer index */
 	unsigned long	relocaddr;	/* Start address of U-Boot in RAM */
 	unsigned long	start_addr_sp;	/* start_addr_stackpointer */
 	phys_size_t	ram_size;	/* RAM size */
@@ -65,13 +66,14 @@ extern gd_t *gd;
 #define GD_ENV_ADDR		5
 #define GD_CPU_CLK		6
 #define GD_BUS_CLK		7
-#define GD_RELOC_ADDR		8
-#define GD_START_ADDR_SP	9
-#define GD_RAM_SIZE		10
-#define GD_RESET_STATUS		11
-#define GD_JT			12
+#define GD_CON_BUF_IDX		8
+#define GD_RELOC_ADDR		9
+#define GD_START_ADDR_SP	10
+#define GD_RAM_SIZE		11
+#define GD_RESET_STATUS		12
+#define GD_JT			13

-#define GD_SIZE			13
+#define GD_SIZE			14

 /*
  * Global Data Flags
diff --git a/common/console.c b/common/console.c
index acc4df3..1fa862d 100644
--- a/common/console.c
+++ b/common/console.c
@@ -323,6 +323,28 @@ int tstc(void)
 	return serial_tstc();
 }

+void pre_console_putc(const char c)
+{
+	char *buffer =  (char *)CONFIG_SYS_TMP_CON_BUF_ADDR;
+
+	if (gd->con_buf_idx < CONFIG_SYS_TMP_CON_BUF_SZ)
+		buffer[gd->con_buf_idx++] = c;
+}
+
+void pre_console_puts(const char *s)
+{
+	while (*s)
+		pre_console_putc(*s++);
+}
+
+void print_pre_console_buffer(void)
+{
+	char *buffer =  (char *)CONFIG_SYS_TMP_CON_BUF_ADDR;
+
+	buffer[gd->con_buf_idx] = 0x00;
+	puts(buffer);
+}
+
 void putc(const char c)
 {
 #ifdef CONFIG_SILENT_CONSOLE
@@ -334,13 +356,16 @@ void putc(const char c)
 	if (gd->flags & GD_FLG_DISABLE_CONSOLE)
 		return;
 #endif
-
-	if (gd->flags & GD_FLG_DEVINIT) {
-		/* Send to the standard output */
-		fputc(stdout, c);
+	if (gd->flags & GD_FLG_HAVE_CONSOLE) {
+		if (gd->flags & GD_FLG_DEVINIT) {
+			/* Send to the standard output */
+			fputc(stdout, c);
+		} else {
+			/* Send directly to the handler */
+			serial_putc(c);
+		}
 	} else {
-		/* Send directly to the handler */
-		serial_putc(c);
+		pre_console_putc(c);
 	}
 }

@@ -355,13 +380,16 @@ void puts(const char *s)
 	if (gd->flags & GD_FLG_DISABLE_CONSOLE)
 		return;
 #endif
-
-	if (gd->flags & GD_FLG_DEVINIT) {
-		/* Send to the standard output */
-		fputs(stdout, s);
+	if (gd->flags & GD_FLG_HAVE_CONSOLE) {
+		if (gd->flags & GD_FLG_DEVINIT) {
+			/* Send to the standard output */
+			fputs(stdout, s);
+		} else {
+			/* Send directly to the handler */
+			serial_puts(s);
+		}
 	} else {
-		/* Send directly to the handler */
-		serial_puts(s);
+		pre_console_puts(s);
 	}
 }

@@ -529,6 +557,10 @@ int console_init_f(void)
 		gd->flags |= GD_FLG_SILENT;
 #endif

+	printf("console initialised - dumping buffer\n");
+	print_pre_console_buffer();
+	printf("buffer dumped\n");
+
 	return 0;
 }

diff --git a/include/configs/eNET.h b/include/configs/eNET.h
index 548d52c..4fb971f 100644
--- a/include/configs/eNET.h
+++ b/include/configs/eNET.h
@@ -164,8 +164,11 @@
 #define CONFIG_SYS_STACK_SIZE			(32 * 1024)
 #define CONFIG_SYS_CAR_ADDR			0x19200000
 #define CONFIG_SYS_CAR_SIZE			(16 * 1024)
+#define CONFIG_SYS_TMP_CON_BUF_SZ		(1 * 1024)
 #define CONFIG_SYS_INIT_SP_ADDR			(CONFIG_SYS_CAR_ADDR + \
-						 CONFIG_SYS_CAR_SIZE)
+						 CONFIG_SYS_CAR_SIZE - \
+						 CONFIG_SYS_TMP_CON_BUF_SZ)
+#define CONFIG_SYS_TMP_CON_BUF_ADDR		CONFIG_SYS_INIT_SP_ADDR + 1
 #define CONFIG_SYS_MONITOR_BASE			CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_MONITOR_LEN			(256 * 1024)
 #define CONFIG_SYS_MALLOC_LEN			(CONFIG_ENV_SECT_SIZE + \
--
1.7.5.2.317.g391b14

[U-Boot] [RFC][PATCH] Pre-console buffer

[Wolfgang Denk]

Dear Graeme Russ,

In message <1314449645-16900-1-git-send-email-graeme.russ@gmail.com> you wrote:
> This cropped up as an aside to another thread so I thought I would give
> it a go. It's pretty rough-and-ready but it does the trick :)

Hm....the problem is that we need some buffer to store the data.  On
some systems this may work, but on many (most?) it doesnt.

> diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
> index 2902e61..4ebc5bd 100644
> --- a/arch/x86/include/asm/global_data.h
> +++ b/arch/x86/include/asm/global_data.h
> @@ -44,6 +44,7 @@ typedef	struct global_data {
>  	unsigned long	env_addr;	/* Address  of Environment struct */
>  	unsigned long	cpu_clk;	/* CPU clock in Hz!		*/
>  	unsigned long	bus_clk;
> +	unsigned long	con_buf_idx;	/* Console buffer index */
>  	unsigned long	relocaddr;	/* Start address of U-Boot in RAM */
>  	unsigned long	start_addr_sp;	/* start_addr_stackpointer */
>  	phys_size_t	ram_size;	/* RAM size */
> @@ -65,13 +66,14 @@ extern gd_t *gd;
>  #define GD_ENV_ADDR		5
>  #define GD_CPU_CLK		6
>  #define GD_BUS_CLK		7
> -#define GD_RELOC_ADDR		8
> -#define GD_START_ADDR_SP	9
> -#define GD_RAM_SIZE		10
> -#define GD_RESET_STATUS		11
> -#define GD_JT			12
> +#define GD_CON_BUF_IDX		8
> +#define GD_RELOC_ADDR		9
> +#define GD_START_ADDR_SP	10
> +#define GD_RAM_SIZE		11
> +#define GD_RESET_STATUS		12
> +#define GD_JT			13
> 
> -#define GD_SIZE			13
> +#define GD_SIZE			14

Argh... your whole "Word Offsets into Global Data" needs to be
removed.  This should be auto-generated as asm-offsets.

> --- a/common/console.c
> +++ b/common/console.c
> @@ -323,6 +323,28 @@ int tstc(void)
>  	return serial_tstc();
>  }

Hm... this adds a lot of code, unconditionally.  In thos form this is
not acceptable, especially as many boards cannot make use of this, or
eventually don't want to make use of it.

> +	if (gd->flags & GD_FLG_HAVE_CONSOLE) {
> +		if (gd->flags & GD_FLG_DEVINIT) {
> +			/* Send to the standard output */
> +			fputc(stdout, c);
> +		} else {
> +			/* Send directly to the handler */
> +			serial_putc(c);
> +		}
>  	} else {
> -		/* Send directly to the handler */
> -		serial_putc(c);
> +		pre_console_putc(c);
>  	}
>  }

And this is actually wrong.  If we can use the serial console for
output, we definitely don;t want to use your buffer any more.

> diff --git a/include/configs/eNET.h b/include/configs/eNET.h
> index 548d52c..4fb971f 100644
> --- a/include/configs/eNET.h
> +++ b/include/configs/eNET.h

...and compilation for all other boards breaks?

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
A bore is someone who persists in holding his own views after we have
enlightened him with ours.

[U-Boot] [RFC][PATCH] Pre-console buffer

[Graeme Russ]

On 27/08/11 23:48, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <1314449645-16900-1-git-send-email-graeme.russ@gmail.com> you wrote:
>> This cropped up as an aside to another thread so I thought I would give
>> it a go. It's pretty rough-and-ready but it does the trick :)
> 
> Hm....the problem is that we need some buffer to store the data.  On
> some systems this may work, but on many (most?) it doesnt.

[U-Boot] [RFC][PATCH] Pre-console buffer

[Wolfgang Denk]

Dear Graeme Russ,

In message <4E5985C8.8020404@gmail.com> you wrote:
>
> > And this is actually wrong.  If we can use the serial console for
> > output, we definitely don;t want to use your buffer any more.
> 
> Look closer - It's a diff weirdness - The actual code is:

I see.  Thanks for pointing out.

> Also, I have noticed in drivers/i2c/ppc4xx_i2c.c:
> 
> 		if (gd->flags & GD_FLG_HAVE_CONSOLE) {
> 			printf("I2C %s: failed %d\n",
> 			       read ? "read" : "write", ret);
> 		}
> 
> and in drivers/i2c/soft_i2c.c:
> 
> #ifdef DEBUG_I2C
> #define PRINTD(fmt,args...)	do {		\
> 	if (gd->flags & GD_FLG_HAVE_CONSOLE)	\
> 		printf (fmt ,##args);		\
> 	} while (0)
> #else
> #define PRINTD(fmt,args...)
> #endif
> 
> So there are drivers that anticipate generating output before console is
> initialised - I think we should not put the onus on the driver and move
> these conditions to printf() in console.c - Unfortunately this could lead
> to head-scratching when one _thinks_ a printf() should generate an output,
> but it is squelched (which is what the pre-console buffer is for)

Indeed - which is why this code is only used for debugging drivers in
certain very special configurations.

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
As of 1992, they're called European Economic Community fries.

[U-Boot] [RFC][PATCH] Pre-console buffer

[Graeme Russ]

Hi Wolfgang,

On 28/08/11 20:08, Wolfgang Denk wrote:
> Dear Graeme Russ,

[snip]

> 
>> So there are drivers that anticipate generating output before console is
>> initialised - I think we should not put the onus on the driver and move
>> these conditions to printf() in console.c - Unfortunately this could lead
>> to head-scratching when one _thinks_ a printf() should generate an output,
>> but it is squelched (which is what the pre-console buffer is for)
> 
> Indeed - which is why this code is only used for debugging drivers in
> certain very special configurations.

So do you think it is worth handling this in printf() as suggested? I do
wonder what unforeseen consequences there can be for an unintended printf()
before console has been initialised. I know that a plain-old 16550 will
just output garbage because the baud rate is not set, but I wonder if other
cases exist where totally unexpected (and unwanted) behaviour will occur
because the hardware is not setup yet.

I think the safe option is to squelch it in printf(), puts() and putc(). At
least then you are only left wondering why something did not print rather
than why you hardware is behaving oddly. And with the pre-console buffer
(if you can squeeze it in) you automagically get those messages as soon as
the console is ready.

Regards,

Graeme