Pre-console buffer for ARM

RFC PATCH New Simon Glass 4 participants 15 messages Aug 29, 2011 – Aug 30, 2011

Subsystems (2)

Arm (32-bit) Tegra architecture support

lore

Share & tools

lore

Simon Glass

author

2011-08-29 17:21 UTC #0 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

This patch is for Graeme to take a look at. I found that have_console is not a flag yet. Also a few tidy-ups to handle buffer overflow and to avoid printing a 'dumping buffer' message when nothing was outputted.

Also I tried to simplify the maths for the location of the pre-console buffer. IMO this should be done in board.c though.

Signed-off-by Simon Glass <[email protected]>

---
 arch/arm/include/asm/global_data.h |    1 +
 common/console.c                   |   36 ++++++++++++++++++++++++++++++++++++
 include/configs/tegra2-common.h    |    8 ++++++--
 3 files changed, 43 insertions(+), 2 deletions(-)

arch/arm/include/asm/global_data.h +1 -0

index 4fc51fd..8b028cc 100644
@@ -71,6 +71,7 @@ typedef	struct	global_data {
 	unsigned long	start_addr_sp;	/* start_addr_stackpointer */
 	unsigned long	reloc_off;
 #if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
+	unsigned long	con_buf_idx;	/* Console buffer index */
 	unsigned long	tlb_addr;
 #endif
 	void		**jt;		/* jump table */

common/console.c +36 -0

index 8c650e0..2a7d2c8 100644
@@ -323,6 +323,36 @@ 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;
+	int index = gd->con_buf_idx;
+
+	if (index) {
+		printf("console initialised - dumping pre-console buffer:\n");
+		index = min(index, CONFIG_SYS_TMP_CON_BUF_SZ - 1);
+		buffer[index] = '\0';
+		puts(buffer);
+		if (gd->con_buf_idx == CONFIG_SYS_TMP_CON_BUF_SZ)
+			puts("... (buffer overflowed)\n");
+		printf("buffer dumped\n");
+	}
+}
+
 void putc(const char c)
 {
 #ifdef CONFIG_SILENT_CONSOLE

@@ -341,6 +371,8 @@ void putc(const char c)
 	} else {
 		/* Send directly to the handler */
 		serial_putc(c);
+	} else {
+		pre_console_putc(c);
 	}
 }

@@ -362,6 +394,8 @@ void puts(const char *s)
 	} else {
 		/* Send directly to the handler */
 		serial_puts(s);
+	} else {
+		pre_console_puts(s);
 	}
 }

@@ -529,6 +563,8 @@ int console_init_f(void)
 		gd->flags |= GD_FLG_SILENT;
 #endif
 
+	print_pre_console_buffer();
+
 	return 0;
 }

include/configs/tegra2-common.h +6 -2

index 73e0f05..a42cce1 100644
@@ -156,8 +156,12 @@
 
 #define CONFIG_SYS_INIT_RAM_ADDR	CONFIG_STACKBASE
 #define CONFIG_SYS_INIT_RAM_SIZE	CONFIG_SYS_MALLOC_LEN
-#define CONFIG_SYS_INIT_SP_ADDR		(CONFIG_SYS_INIT_RAM_ADDR + \
-						CONFIG_SYS_INIT_RAM_SIZE - \
+#define CONFIG_SYS_TMP_CON_BUF_SZ	(1 * 1024)
+#define CONFIG_SYS_INIT_RAM_TOP		(CONFIG_SYS_INIT_RAM_ADDR  + \
+						CONFIG_SYS_INIT_RAM_SIZE)
+#define CONFIG_SYS_TMP_CON_BUF_ADDR	(CONFIG_SYS_INIT_RAM_TOP - \
+					CONFIG_SYS_TMP_CON_BUF_SZ)
+#define CONFIG_SYS_INIT_SP_ADDR		(CONFIG_SYS_TMP_CON_BUF_ADDR - \
 						GENERATED_GBL_DATA_SIZE)
 
 #define CONFIG_TEGRA2_GPIO

Signature · 1 line

1.7.3.1

Mike Frysinger

2011-08-29 19:20 UTC #1 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

common/console.c int tstc(void) · line 328

Quoted 3 lines · On Monday, August 29, 2011 13:21:57 Simon Glass wrote:

common/console.c +3 -0

+void pre_console_putc(const char c)
+{
+	char *buffer =  (char *)CONFIG_SYS_TMP_CON_BUF_ADDR;

excess space after the "="

common/console.c int tstc(void) · line 331

Quoted 2 lines

common/console.c +2 -0

+	if (gd->con_buf_idx < CONFIG_SYS_TMP_CON_BUF_SZ)
+		buffer[gd->con_buf_idx++] = c;

seems like a circular buffer would make more sense ... usually the part of the log you want is the last chunk and not the first
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110829/0404f2e3/attachment.pgp

Simon Glass

author

2011-08-29 19:42 UTC #2 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Hi Mike,

Quoted 6 lines · Mike Frysinger

common/console.c int tstc(void)
Quoted 3 lines · On Monday, August 29, 2011 13:21:57 Simon Glass wrote:
common/console.c +3 -0
+void pre_console_putc(const char c)
+{
+ ? ? char *buffer = ?(char *)CONFIG_SYS_TMP_CON_BUF_ADDR;
excess space after the "="

Thanks - I will leave this to Graeme to tidy as our patches overlap (and most of the patch is just his).

Quoted 6 lines

Quoted 3 lines
+ ? ? if (gd->con_buf_idx < CONFIG_SYS_TMP_CON_BUF_SZ)
+ ? ? ? ? ? ? buffer[gd->con_buf_idx++] = c;
seems like a circular buffer would make more sense ... usually the part of the log you want is the last chunk and not the first

Yes I agree, although if you have more than 1KB of data it might be a bug.

Regards,
SImon

Quoted 2 lines

-mike

Mike Frysinger

2011-08-29 20:10 UTC #3 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Quoted 9 lines · On Monday, August 29, 2011 15:42:23 Simon Glass wrote:

Quoted 6 lines · Mike Frysinger
common/console.c int tstc(void) · line 331
Quoted 2 lines · On Monday, August 29, 2011 13:21:57 Simon Glass wrote:
common/console.c +2 -0
+     if (gd->con_buf_idx < CONFIG_SYS_TMP_CON_BUF_SZ)
+             buffer[gd->con_buf_idx++] = c;
seems like a circular buffer would make more sense ... usually the part of the log you want is the last chunk and not the first
Yes I agree, although if you have more than 1KB of data it might be a bug.

give people a foot and they'll take 1MiB :p

it's fairly easy as well:
#define CIRC_BUF_IDX(idx) ((idx) & (CONFIG_SYS_TMP_CON_BUF_SZ-1)) buffer[CIRC_BUF_IDX(gd->conf_buf_idx++)] = c;
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110829/dfa93fa6/attachment.pgp

Graeme Russ

2011-08-29 23:00 UTC #4 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Hi Mike,

Quoted 9 lines · Mike Frysinger

Quoted 8 lines · On Monday, August 29, 2011 15:42:23 Simon Glass wrote:
Quoted 6 lines · Mike Frysinger
common/console.c int tstc(void) · line 331
Quoted 2 lines · On Monday, August 29, 2011 13:21:57 Simon Glass wrote:
common/console.c +2 -0
+     if (gd->con_buf_idx < CONFIG_SYS_TMP_CON_BUF_SZ)
+             buffer[gd->con_buf_idx++] = c;
seems like a circular buffer would make more sense ... usually the part of the log you want is the last chunk and not the first

Well you would need an 'overflow' flag and then based on that you would need to do two printf()'s when dumping the buffer - One from the 'index to end' which is the 'top' of the buffer and one from 'start to index' which is the bottom.

Quoted 1 line

Quoted 1 line
Yes I agree, although if you have more than 1KB of data it might be a bug.

I personally don't see the need - I expect the amount of pre-console output would be faily limited considering that the board should do everything it can to initialise console as early as possible.

Quoted 6 lines

give people a foot and they'll take 1MiB :p
it's fairly easy as well:
#define CIRC_BUF_IDX(idx) ((idx) & (CONFIG_SYS_TMP_CON_BUF_SZ-1)) buffer[CIRC_BUF_IDX(gd->conf_buf_idx++)] = c;

But does that work for non power-of-two buffer sizes...

Buffer size = 100 = 1100100

(size - 1)        = 1100011
idx = 100         = 1100100
idx & (size - 1)  = 1100000

Nope :(

Regards,

Graeme

Mike Frysinger

2011-08-30 01:32 UTC #5 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Quoted 7 lines · On Monday, August 29, 2011 19:00:46 Graeme Russ wrote:

Quoted 6 lines · Mike Frysinger
Quoted 5 lines · On Monday, August 29, 2011 15:42:23 Simon Glass wrote:
Quoted 4 lines · Mike Frysinger
Quoted 2 lines · On Monday, August 29, 2011 13:21:57 Simon Glass wrote:
common/console.c +2 -0
+     if (gd->con_buf_idx < CONFIG_SYS_TMP_CON_BUF_SZ)
+             buffer[gd->con_buf_idx++] = c;

Quoted 7 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
seems like a circular buffer would make more sense ... usually the part of the log you want is the last chunk and not the first
Well you would need an 'overflow' flag and then based on that you would need to do two printf()'s when dumping the buffer - One from the 'index to end' which is the 'top' of the buffer and one from 'start to index' which is the bottom.

you wouldnt need an overflow flag unless you wrote out more than 2^32 bytes. look at the logic again ... it isnt masking the write, it's masking the read. so by virtual of the con_buf_idx being larger than the max, you know you've wrapped around.

otherwise yes, you'd need to split up the writes. not that big of a deal i dont think ...

Quoted 6 lines

Quoted 2 lines
Quoted 2 lines
Yes I agree, although if you have more than 1KB of data it might be a bug.
I personally don't see the need - I expect the amount of pre-console output would be faily limited considering that the board should do everything it can to initialise console as early as possible.

until people hit an early fail and add a lot of debug printf's and then the output gets silently clipped. it's confusing imo, and i say this having implemented early debug output in other systems (including linux) and seeing how people used/reacted to it.

Quoted 7 lines

Quoted 5 lines
give people a foot and they'll take 1MiB :p
it's fairly easy as well:
#define CIRC_BUF_IDX(idx) ((idx) & (CONFIG_SYS_TMP_CON_BUF_SZ-1)) buffer[CIRC_BUF_IDX(gd->conf_buf_idx++)] = c;
But does that work for non power-of-two buffer sizes...

no, but not that big of a deal. so you get limited to the last 1KiB, 4KiB, 8KiB, 16KiB, etc... amount of data.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110829/481a0f2a/attachment.pgp

Graeme Russ

2011-08-30 01:49 UTC #6 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Hi Mike,

Quoted 16 lines · Mike Frysinger

Quoted 7 lines · On Monday, August 29, 2011 19:00:46 Graeme Russ wrote:
Quoted 6 lines · Mike Frysinger
Quoted 5 lines · On Monday, August 29, 2011 15:42:23 Simon Glass wrote:
Quoted 4 lines · Mike Frysinger
Quoted 2 lines · On Monday, August 29, 2011 13:21:57 Simon Glass wrote:
common/console.c +2 -0
+     if (gd->con_buf_idx < CONFIG_SYS_TMP_CON_BUF_SZ)
+             buffer[gd->con_buf_idx++] = c;
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
seems like a circular buffer would make more sense ... usually the part of the log you want is the last chunk and not the first
Well you would need an 'overflow' flag and then based on that you would need to do two printf()'s when dumping the buffer - One from the 'index to end' which is the 'top' of the buffer and one from 'start to index' which is the bottom.

Quoted 7 lines

you wouldnt need an overflow flag unless you wrote out more than 2^32 bytes. look at the logic again ... it isnt masking the write, it's masking the read. so by virtual of the con_buf_idx being larger than the max, you know you've wrapped around.
otherwise yes, you'd need to split up the writes. not that big of a deal i dont think ...

OK, so you let gd->con_buf_idx increment always and modulo to get the array index - fine if CONFIG_SYS_TMP_CON_BUF_SZ is ^2 - see below

Quoted 11 lines

Quoted 6 lines
Quoted 2 lines
Quoted 2 lines
Yes I agree, although if you have more than 1KB of data it might be a bug.
I personally don't see the need - I expect the amount of pre-console output would be faily limited considering that the board should do everything it can to initialise console as early as possible.
until people hit an early fail and add a lot of debug printf's and then the output gets silently clipped. it's confusing imo, and i say this having implemented early debug output in other systems (including linux) and seeing how people used/reacted to it.

We can easily have a 'output clipped' so people know what happened and can trim their debugging messages accordingly

Quoted 10 lines

Quoted 7 lines
Quoted 5 lines
give people a foot and they'll take 1MiB :p
it's fairly easy as well:
#define CIRC_BUF_IDX(idx) ((idx) & (CONFIG_SYS_TMP_CON_BUF_SZ-1)) buffer[CIRC_BUF_IDX(gd->conf_buf_idx++)] = c;
But does that work for non power-of-two buffer sizes...
no, but not that big of a deal. so you get limited to the last 1KiB, 4KiB, 8KiB, 16KiB, etc... amount of data.

Until someone doesn't read the documentation and figures they can only squeeze 200 bytes out of their L1 cache after making room for gd and stack and then tries to print 201 bytes of debug info and trashes either the stack or gd and then things start to get a lot weirder than simply having their early debug messgaes clipped...

To be safe, CONFIG_SYS_TMP_CON_BUF_SZ would need to be checked for ^2 size and now we only get 128 bytes rather than 200 :( - Better to add another long in gd and get 196

Regards,

Graeme

Mike Frysinger

2011-08-30 02:50 UTC #7 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Quoted 11 lines · On Monday, August 29, 2011 21:49:33 Graeme Russ wrote:

Quoted 10 lines · Mike Frysinger
Quoted 8 lines · On Monday, August 29, 2011 19:00:46 Graeme Russ wrote:
Quoted 3 lines · Mike Frysinger
Quoted 2 lines · On Monday, August 29, 2011 15:42:23 Simon Glass wrote:
Yes I agree, although if you have more than 1KB of data it might be a bug.
I personally don't see the need - I expect the amount of pre-console output would be faily limited considering that the board should do everything it can to initialise console as early as possible.

Quoted 7 lines

Quoted 4 lines
until people hit an early fail and add a lot of debug printf's and then the output gets silently clipped. it's confusing imo, and i say this having implemented early debug output in other systems (including linux) and seeing how people used/reacted to it.
We can easily have a 'output clipped' so people know what happened and can trim their debugging messages accordingly

much easier said than done. clipping the head rather than the tail is much more often the right default.

Quoted 8 lines

Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
give people a foot and they'll take 1MiB :p
it's fairly easy as well:
#define CIRC_BUF_IDX(idx) ((idx) & (CONFIG_SYS_TMP_CON_BUF_SZ-1)) buffer[CIRC_BUF_IDX(gd->conf_buf_idx++)] = c;
But does that work for non power-of-two buffer sizes...

Quoted 8 lines

Quoted 2 lines
no, but not that big of a deal. so you get limited to the last 1KiB, 4KiB, 8KiB, 16KiB, etc... amount of data.
Until someone doesn't read the documentation and figures they can only squeeze 200 bytes out of their L1 cache after making room for gd and stack and then tries to print 201 bytes of debug info and trashes either the stack or gd and then things start to get a lot weirder than simply having their early debug messgaes clipped...

this is really a non-issue as it's trivial to catch at CPP time:
#if (CONFIG_SYS_TMP_CON_BUF_SZ & (CONFIG_SYS_TMP_CON_BUF_SZ - 1))

# error "CONFIG_SYS_TMP_CON_BUF_SZ must be a power-of-two"
#endif

Quoted 3 lines

To be safe, CONFIG_SYS_TMP_CON_BUF_SZ would need to be checked for ^2 size and now we only get 128 bytes rather than 200 :( - Better to add another long in gd and get 196

my systems dont have nearly the size restrictions yours do, so if you think that's better, that's fine by me. i dont care so much about the internal implementation details of the circular buffer ... i only care that it is a circular buffer and not one that stops too soon.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110829/92cc8348/attachment.pgp

Graeme Russ

2011-08-30 03:12 UTC #8 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Hi Mike,

Quoted 17 lines · Mike Frysinger

Quoted 16 lines · On Monday, August 29, 2011 21:49:33 Graeme Russ wrote:
Quoted 14 lines · Mike Frysinger
Quoted 8 lines · On Monday, August 29, 2011 19:00:46 Graeme Russ wrote:
Quoted 3 lines · Mike Frysinger
Quoted 2 lines · On Monday, August 29, 2011 15:42:23 Simon Glass wrote:
Yes I agree, although if you have more than 1KB of data it might be a bug.
I personally don't see the need - I expect the amount of pre-console output would be faily limited considering that the board should do everything it can to initialise console as early as possible.
until people hit an early fail and add a lot of debug printf's and then the output gets silently clipped. it's confusing imo, and i say this having implemented early debug output in other systems (including linux) and seeing how people used/reacted to it.

Quoted 5 lines

Quoted 2 lines
We can easily have a 'output clipped' so people know what happened and can trim their debugging messages accordingly
much easier said than done. clipping the head rather than the tail is much more often the right default.

One could argue that not clipping at all is the _right_ default ;)

But I can easily add to print_pre_console_buffer()...

	if (gd->conf_buf_idx >= CONFIG_SYS_TMP_CON_BUF_SZ)
		puts("Pre-console buffer clipped")

Quoted 17 lines

Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
give people a foot and they'll take 1MiB :p
it's fairly easy as well:
#define CIRC_BUF_IDX(idx) ((idx) & (CONFIG_SYS_TMP_CON_BUF_SZ-1)) buffer[CIRC_BUF_IDX(gd->conf_buf_idx++)] = c;
But does that work for non power-of-two buffer sizes...
Quoted 8 lines
Quoted 2 lines
no, but not that big of a deal. so you get limited to the last 1KiB, 4KiB, 8KiB, 16KiB, etc... amount of data.
Until someone doesn't read the documentation and figures they can only squeeze 200 bytes out of their L1 cache after making room for gd and stack and then tries to print 201 bytes of debug info and trashes either the stack or gd and then things start to get a lot weirder than simply having their early debug messgaes clipped...

Quoted 4 lines

this is really a non-issue as it's trivial to catch at CPP time:
#if (CONFIG_SYS_TMP_CON_BUF_SZ & (CONFIG_SYS_TMP_CON_BUF_SZ - 1))
# error "CONFIG_SYS_TMP_CON_BUF_SZ must be a power-of-two"
#endif

Nice - hadn't thought of that

Quoted 8 lines

Quoted 3 lines
To be safe, CONFIG_SYS_TMP_CON_BUF_SZ would need to be checked for ^2 size and now we only get 128 bytes rather than 200 :( - Better to add another long in gd and get 196
my systems dont have nearly the size restrictions yours do, so if you think that's better, that's fine by me. i dont care so much about the internal implementation details of the circular buffer ... i only care that it is a circular buffer and not one that stops too soon.

Well actually, I have a 12-year old CPU with 16kB of cache - More than enough for initial stack, gd and pre-console buffer. It was actually Wolgang who raised the size issue, but that was probably more aimed at those that do not have the space for the buffer at all

I'll do another rev of the two-patch series implementing the 'power of two' trap and circular buffer.

P.S. Can we move this discussion over to that thread now:

[PATCH 0/2] console: Squelch and pre-console buffer
http://lists.denx.de/pipermail/u-boot/2011-August/099607.html

Regards,

Graeme

Wolfgang Denk

2011-08-30 05:17 UTC #9 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

In reply to Graeme Russ

Dear Graeme Russ,

Quoted 16 lines · In message <CALButCJF1p+60WOLtiJqhcAed-71VxmP2mBqY2wdNURPk5=Q+g@mail.gmail.com>…

Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
it's fairly easy as well:
#define CIRC_BUF_IDX(idx) ((idx) & (CONFIG_SYS_TMP_CON_BUF_SZ-1)) buffer[CIRC_BUF_IDX(gd->conf_buf_idx++)] = c;
But does that work for non power-of-two buffer sizes...
no, but not that big of a deal. so you get limited to the last 1KiB, 4KiB, 8KiB, 16KiB, etc... amount of data.
Until someone doesn't read the documentation and figures they can only squeeze 200 bytes out of their L1 cache after making room for gd and stack and then tries to print 201 bytes of debug info and trashes either the stack or gd and then things start to get a lot weirder than simply having their early debug messgaes clipped...

Quoted 3 lines

To be safe, CONFIG_SYS_TMP_CON_BUF_SZ would need to be checked for ^2 size and now we only get 128 bytes rather than 200 :( - Better to add another long in gd and get 196

Grrrgh.

If you want to support arbitrary buffer sizes, then just use

	#define CIRC_BUF_IDX(idx) ((idx) % CONFIG_SYS_TMP_CON_BUF_SZ)

Best regards,

Wolfgang Denk

Signature · 4 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
Use C++ to confuse your enemies; use C to produce stable code.

Graeme Russ

2011-08-30 05:28 UTC #10 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Hi Wolfgang

Quoted 23 lines · Wolfgang Denk

Dear Graeme Russ,
Quoted 16 lines · In message <CALButCJF1p+60WOLtiJqhcAed-71VxmP2mBqY2wdNURPk5=Q+g@mail.gmail.com>…
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
it's fairly easy as well:
#define CIRC_BUF_IDX(idx) ((idx) & (CONFIG_SYS_TMP_CON_BUF_SZ-1)) buffer[CIRC_BUF_IDX(gd->conf_buf_idx++)] = c;
But does that work for non power-of-two buffer sizes...
no, but not that big of a deal. so you get limited to the last 1KiB, 4KiB, 8KiB, 16KiB, etc... amount of data.
Until someone doesn't read the documentation and figures they can only squeeze 200 bytes out of their L1 cache after making room for gd and stack and then tries to print 201 bytes of debug info and trashes either the stack or gd and then things start to get a lot weirder than simply having their early debug messgaes clipped...
Quoted 3 lines
To be safe, CONFIG_SYS_TMP_CON_BUF_SZ would need to be checked for ^2 size and now we only get 128 bytes rather than 200 :( - Better to add another long in gd and get 196

Quoted 5 lines

Grrrgh.
If you want to support arbitrary buffer sizes, then just use
       #define CIRC_BUF_IDX(idx) ((idx) % CONFIG_SYS_TMP_CON_BUF_SZ)

I know, but I was concerned that you wouldn't like the use of modulo arithmetic for every putc() - But I suppose thats cheaper than a compare plus branch...

If you prefer modulo over the 'must be a power of two' restriction, then I am happy to do it that way instead

Regards,

Graeme

Wolfgang Denk

2011-08-30 09:29 UTC #11 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Dear Graeme Russ,

Quoted 8 lines · In message <CALButC+t=OUL9XgQnfyAdZmgPJRp7mdRwEE9LKogYHEg1D+m4g@mail.gmail.com>…

Quoted 4 lines
If you want to support arbitrary buffer sizes, then just use
       #define CIRC_BUF_IDX(idx) ((idx) % CONFIG_SYS_TMP_CON_BUF_SZ)
I know, but I was concerned that you wouldn't like the use of modulo arithmetic for every putc() - But I suppose thats cheaper than a compare plus branch...

Well, the difference of "i & (CONFIG_SYS_TMP_CON_BUF_SZ-1)" versus "i % CONFIG_SYS_TMP_CON_BUF_SZ" is 1 versus 4 assembler instructions on Power, and 2 versus 5 assembler instructions on ARM. I think we can tolerate this.

Best regards,

Wolfgang Denk

Signature · 5 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
You get a wonderful view from the point of no return.
                                    - Terry Pratchett, _Making_Money_

Graeme Russ

2011-08-30 09:37 UTC #12 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Hi Wolfgang,

Quoted 8 lines · Wolfgang Denk

Dear Graeme Russ,
Quoted 5 lines · In message <CALButC+t=OUL9XgQnfyAdZmgPJRp7mdRwEE9LKogYHEg1D+m4g@mail.gmail.com>…
Quoted 4 lines
If you want to support arbitrary buffer sizes, then just use
       #define CIRC_BUF_IDX(idx) ((idx) % CONFIG_SYS_TMP_CON_BUF_SZ)

Quoted 8 lines

Quoted 3 lines
I know, but I was concerned that you wouldn't like the use of modulo arithmetic for every putc() - But I suppose thats cheaper than a compare plus branch...
Well, the difference of "i & (CONFIG_SYS_TMP_CON_BUF_SZ-1)" versus "i % CONFIG_SYS_TMP_CON_BUF_SZ" is 1 versus 4 assembler instructions on Power, and 2 versus 5 assembler instructions on ARM. I think we can tolerate this.

And it's only hit during printf() before console is initialised :)

Respinning now

Regards,

Graeme

Graeme Russ

2011-08-29 22:49 UTC #13 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

In reply to Simon Glass

Hi Simon

Quoted 6 lines · Simon Glass

This patch is for Graeme to take a look at. I found that have_console is not a flag yet. Also a few tidy-ups to handle buffer overflow and to avoid printing a 'dumping buffer' message when nothing was outputted.
Also I tried to simplify the maths for the location of the pre-console buffer. IMO this should be done in board.c though.

[snip]

Have you seen my latest pre-console buffer series?:

http://lists.denx.de/pipermail/u-boot/2011-August/099607.html

I think that is a better base to work from as I think we can also fold in your 'Early panic support' series as well - I'll comment on that series seperately

Regards,

Graeme

Simon Glass

author

2011-08-29 23:01 UTC #14 lore raw

RFC PATCH 1/1 Pre-console buffer for ARM

Hi Graeme,

Quoted 7 lines · Graeme Russ

Hi Simon
Quoted 4 lines · Simon Glass
This patch is for Graeme to take a look at. I found that have_console is not a flag yet. Also a few tidy-ups to handle buffer overflow and to avoid printing a 'dumping buffer' message when nothing was outputted.

Quoted 8 lines

Quoted 2 lines
Also I tried to simplify the maths for the location of the pre-console buffer. IMO this should be done in board.c though.
[snip]
Have you seen my latest pre-console buffer series?:
http://lists.denx.de/pipermail/u-boot/2011-August/099607.html

No - I foolishly didn't look at the mailing list in those 4 hours :-)

Will take a look, thanks.

Regards,
Simon

Quoted 9 lines

I think that is a better base to work from as I think we can also fold in your 'Early panic support' series as well - I'll comment on that series seperately
Regards,
Graeme