| Message ID | 1314708581-13678-1-git-send-email-graeme.russ@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tuesday, August 30, 2011 08:49:41 Graeme Russ wrote: > - if (!gd->have_console) > + if (!gd->have_console) { > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + pre_console_putc(c); > +#endif > return; > + } add inline stubs for the pre_console_xxx helpers and you can drop all the ifdef's through out the rest of the code #ifdef CONFIG_PRE_CONSOLE_BUFFER ... normal defines for these ... #else static inline void pre_console_putc(const char c) {} static inline void pre_console_puts(const char *s) {} #endif -mike
Hi Graeme, On Tue, Aug 30, 2011 at 5:49 AM, Graeme Russ <graeme.russ@gmail.com> wrote: > Allow redirection of console output prior to console initialisation to a > temporary buffer. > > To enable this functionality, the board configuration file must define: > - CONFIG_PRE_CONSOLE_BUFFER - Enable pre-console buffer > - CONFIG_PRE_CON_BUF_ADDR - Base address of pre-console buffer > - CONFIG_PRE_CON_BUF_SZ - Size of pre-console buffer (in bytes) Please can you add these to the README? Also I wonder if it is safe to have boards setting the buffer address. Should not arch/xxx/lib/board.c work this out based on CONFIG_SYS_INIT_SP_ADDR as it does all the other memory areas? > > The pre-console buffer will buffer the last CONFIG_PRE_CON_BUF_SZ bytes > Any earlier characters are silently dropped. > > Signed-off-by: Graeme Russ <graeme.russ@gmail.com> > --- > Changes Since V1 > - Implemented circular buffer > - Trivial code style corrections > > arch/arm/include/asm/global_data.h | 3 ++ > arch/avr32/include/asm/global_data.h | 3 ++ > arch/blackfin/include/asm/global_data.h | 3 ++ > arch/m68k/include/asm/global_data.h | 3 ++ > arch/microblaze/include/asm/global_data.h | 3 ++ > arch/mips/include/asm/global_data.h | 3 ++ > arch/nios2/include/asm/global_data.h | 3 ++ > arch/powerpc/include/asm/global_data.h | 3 ++ > arch/sh/include/asm/global_data.h | 3 ++ > arch/sparc/include/asm/global_data.h | 3 ++ > arch/x86/include/asm/global_data.h | 3 ++ > common/console.c | 49 +++++++++++++++++++++++++++- > 12 files changed, 80 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h > index 4fc51fd..b85b7fe 100644 > --- a/arch/arm/include/asm/global_data.h > +++ b/arch/arm/include/asm/global_data.h > @@ -38,6 +38,9 @@ typedef struct global_data { > unsigned long flags; > unsigned long baudrate; > unsigned long have_console; /* serial_init() was called */ > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > unsigned long env_addr; /* Address of Environment struct */ > unsigned long env_valid; /* Checksum of Environment valid? */ > unsigned long fb_base; /* base address of frame buffer */ > diff --git a/arch/avr32/include/asm/global_data.h b/arch/avr32/include/asm/global_data.h > index 4ef8fc5..5c654bd 100644 > --- a/arch/avr32/include/asm/global_data.h > +++ b/arch/avr32/include/asm/global_data.h > @@ -38,6 +38,9 @@ typedef struct global_data { > unsigned long baudrate; > unsigned long stack_end; /* highest stack address */ > unsigned long have_console; /* serial_init() was called */ > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > unsigned long reloc_off; /* Relocation Offset */ > unsigned long env_addr; /* Address of env struct */ > unsigned long env_valid; /* Checksum of env valid? */ > diff --git a/arch/blackfin/include/asm/global_data.h b/arch/blackfin/include/asm/global_data.h > index eba5e93..f7aa711 100644 > --- a/arch/blackfin/include/asm/global_data.h > +++ b/arch/blackfin/include/asm/global_data.h > @@ -45,6 +45,9 @@ typedef struct global_data { > unsigned long board_type; > unsigned long baudrate; > unsigned long have_console; /* serial_init() was called */ > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > phys_size_t ram_size; /* RAM size */ > unsigned long env_addr; /* Address of Environment struct */ > unsigned long env_valid; /* Checksum of Environment valid? */ > diff --git a/arch/m68k/include/asm/global_data.h b/arch/m68k/include/asm/global_data.h > index fc486fd..0ba2b43 100644 > --- a/arch/m68k/include/asm/global_data.h > +++ b/arch/m68k/include/asm/global_data.h > @@ -57,6 +57,9 @@ typedef struct global_data { > unsigned long env_addr; /* Address of Environment struct */ > unsigned long env_valid; /* Checksum of Environment valid? */ > unsigned long have_console; /* serial_init() was called */ > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > #if defined(CONFIG_LCD) || defined(CONFIG_VIDEO) > unsigned long fb_base; /* Base addr of framebuffer memory */ > #endif > diff --git a/arch/microblaze/include/asm/global_data.h b/arch/microblaze/include/asm/global_data.h > index 557ad27..6e8537c 100644 > --- a/arch/microblaze/include/asm/global_data.h > +++ b/arch/microblaze/include/asm/global_data.h > @@ -39,6 +39,9 @@ typedef struct global_data { > unsigned long flags; > unsigned long baudrate; > unsigned long have_console; /* serial_init() was called */ > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > unsigned long env_addr; /* Address of Environment struct */ > unsigned long env_valid; /* Checksum of Environment valid? */ > unsigned long fb_base; /* base address of frame buffer */ > diff --git a/arch/mips/include/asm/global_data.h b/arch/mips/include/asm/global_data.h > index 271a290..b193517 100644 > --- a/arch/mips/include/asm/global_data.h > +++ b/arch/mips/include/asm/global_data.h > @@ -41,6 +41,9 @@ typedef struct global_data { > unsigned long flags; > unsigned long baudrate; > unsigned long have_console; /* serial_init() was called */ > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > phys_size_t ram_size; /* RAM size */ > unsigned long reloc_off; /* Relocation Offset */ > unsigned long env_addr; /* Address of Environment struct */ > diff --git a/arch/nios2/include/asm/global_data.h b/arch/nios2/include/asm/global_data.h > index 2c4a719..d9f0664 100644 > --- a/arch/nios2/include/asm/global_data.h > +++ b/arch/nios2/include/asm/global_data.h > @@ -29,6 +29,9 @@ typedef struct global_data { > unsigned long baudrate; > unsigned long cpu_clk; /* CPU clock in Hz! */ > unsigned long have_console; /* serial_init() was called */ > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > phys_size_t ram_size; /* RAM size */ > unsigned long env_addr; /* Address of Environment struct */ > unsigned long env_valid; /* Checksum of Environment valid */ > diff --git a/arch/powerpc/include/asm/global_data.h b/arch/powerpc/include/asm/global_data.h > index a33ca2f..7fcaf38 100644 > --- a/arch/powerpc/include/asm/global_data.h > +++ b/arch/powerpc/include/asm/global_data.h > @@ -138,6 +138,9 @@ typedef struct global_data { > unsigned long env_addr; /* Address of Environment struct */ > unsigned long env_valid; /* Checksum of Environment valid? */ > unsigned long have_console; /* serial_init() was called */ > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > #if defined(CONFIG_SYS_ALLOC_DPRAM) || defined(CONFIG_CPM2) > unsigned int dp_alloc_base; > unsigned int dp_alloc_top; > diff --git a/arch/sh/include/asm/global_data.h b/arch/sh/include/asm/global_data.h > index 0c09ba9..1b782fc 100644 > --- a/arch/sh/include/asm/global_data.h > +++ b/arch/sh/include/asm/global_data.h > @@ -34,6 +34,9 @@ typedef struct global_data > unsigned long baudrate; > unsigned long cpu_clk; /* CPU clock in Hz! */ > unsigned long have_console; /* serial_init() was called */ > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > phys_size_t ram_size; /* RAM size */ > unsigned long env_addr; /* Address of Environment struct */ > unsigned long env_valid; /* Checksum of Environment valid */ > diff --git a/arch/sparc/include/asm/global_data.h b/arch/sparc/include/asm/global_data.h > index 9b14674..a1e4b44 100644 > --- a/arch/sparc/include/asm/global_data.h > +++ b/arch/sparc/include/asm/global_data.h > @@ -53,6 +53,9 @@ typedef struct global_data { > unsigned long env_valid; /* Checksum of Environment valid? */ > unsigned long have_console; /* serial_init() was called */ > > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > #if defined(CONFIG_LCD) || defined(CONFIG_VIDEO) > unsigned long fb_base; /* Base address of framebuffer memory */ > #endif > diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h > index f977dbe..6cf7955 100644 > --- a/arch/x86/include/asm/global_data.h > +++ b/arch/x86/include/asm/global_data.h > @@ -40,6 +40,9 @@ typedef struct global_data { > unsigned long flags; > unsigned long baudrate; > unsigned long have_console; /* serial_init() was called */ > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + unsigned long precon_buf_idx; /* Pre-Console buffer index */ > +#endif > unsigned long reloc_off; /* Relocation Offset */ > unsigned long load_off; /* Load Offset */ > unsigned long env_addr; /* Address of Environment struct */ > diff --git a/common/console.c b/common/console.c > index b23d933..ae87148 100644 > --- a/common/console.c > +++ b/common/console.c > @@ -329,6 +329,35 @@ int tstc(void) > return serial_tstc(); > } > > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > +#define CIRC_BUF_IDX(idx) ((idx) % CONFIG_PRE_CON_BUF_SZ) The division here sticks in the craw, but unless we go with CONFIG_PRE_CON_BUF_SZ_LOG2 then I don't see an easy way around it, and since this is serial output we can't honestly claim to worry much about performance. > + > +void pre_console_putc(const char c) > +{ > + char *buffer = (char *)CONFIG_PRE_CON_BUF_ADDR; > + > + buffer[CIRC_BUF_IDX(gd->precon_buf_idx++)] = c; > +} > + > +void pre_console_puts(const char *s) > +{ > + while (*s) > + pre_console_putc(*s++); > +} > + > +void print_pre_console_buffer(void) > +{ > + unsigned long i = 0; > + char *buffer = (char *)CONFIG_PRE_CON_BUF_ADDR; > + > + if (gd->precon_buf_idx > CONFIG_PRE_CON_BUF_SZ) > + i = gd->precon_buf_idx - CONFIG_PRE_CON_BUF_SZ; Gosh this is clever - perhaps a little comment? > + > + while (i < gd->precon_buf_idx) > + putc(buffer[CIRC_BUF_IDX(i++)]); > +} > +#endif > + > void putc(const char c) > { > #ifdef CONFIG_SILENT_CONSOLE > @@ -341,8 +370,12 @@ void putc(const char c) > return; > #endif > > - if (!gd->have_console) > + if (!gd->have_console) { > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + pre_console_putc(c); > +#endif > return; > + } > > if (gd->flags & GD_FLG_DEVINIT) { > /* Send to the standard output */ > @@ -365,8 +398,12 @@ void puts(const char *s) > return; > #endif > > - if (!gd->have_console) > + if (!gd->have_console) { > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + pre_console_puts(s); > +#endif > return; > + } > > if (gd->flags & GD_FLG_DEVINIT) { > /* Send to the standard output */ > @@ -383,8 +420,10 @@ int printf(const char *fmt, ...) > uint i; > char printbuffer[CONFIG_SYS_PBSIZE]; > > +#ifndef CONFIG_PRE_CONSOLE_BUFFER > if (!gd->have_console) > return 0; > +#endif > > va_start(args, fmt); > > @@ -404,8 +443,10 @@ int vprintf(const char *fmt, va_list args) > uint i; > char printbuffer[CONFIG_SYS_PBSIZE]; > > +#ifndef CONFIG_PRE_CONSOLE_BUFFER > if (!gd->have_console) > return 0; > +#endif > > /* For this to work, printbuffer must be larger than > * anything we ever want to print. > @@ -547,6 +588,10 @@ int console_init_f(void) > gd->flags |= GD_FLG_SILENT; > #endif > > +#ifdef CONFIG_PRE_CONSOLE_BUFFER > + print_pre_console_buffer(); > +#endif > + > return 0; > } > > -- > 1.7.5.2.317.g391b14 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
Dear Simon Glass, In message <CAPnjgZ1fXCCFDcwCWgZD7H-siVEV-H+Ks5TPeiT=nGS743fzcg@mail.gmail.com> you wrote: > ... > > +#define CIRC_BUF_IDX(idx) ((idx) % CONFIG_PRE_CON_BUF_SZ) > > The division here sticks in the craw, but unless we go with Does it? Why? > CONFIG_PRE_CON_BUF_SZ_LOG2 then I don't see an easy way around it, and > since this is serial output we can't honestly claim to worry much > about performance. Please see my previous posting (http://article.gmane.org/gmane.comp.boot-loaders.u-boot/106810) I don't think 3 additional addembler instructions really play a big role here. Best regards, Wolfgang Denk
On Tuesday, August 30, 2011 15:52:38 Wolfgang Denk wrote: > Simon Glass wrote: > > > +#define CIRC_BUF_IDX(idx) ((idx) % CONFIG_PRE_CON_BUF_SZ) > > > > The division here sticks in the craw, but unless we go with > > Does it? Why? > > > CONFIG_PRE_CON_BUF_SZ_LOG2 then I don't see an easy way around it, and > > since this is serial output we can't honestly claim to worry much > > about performance. > > Please see my previous posting > (http://article.gmane.org/gmane.comp.boot-loaders.u-boot/106810) > > I don't think 3 additional addembler instructions really play a big > role here. i'm pretty sure if you define CONFIG_PRE_CON_BUF_SZ as a power of 2 value, you get nice & simple assembly code. so if the generated code is undesirable, pick a CONFIG value that is power-of-2 ? -mike
Hi Wolfgang, On Tue, Aug 30, 2011 at 12:52 PM, Wolfgang Denk <wd@denx.de> wrote: > Dear Simon Glass, > > In message <CAPnjgZ1fXCCFDcwCWgZD7H-siVEV-H+Ks5TPeiT=nGS743fzcg@mail.gmail.com> you wrote: >> > ... >> > +#define CIRC_BUF_IDX(idx) ((idx) % CONFIG_PRE_CON_BUF_SZ) >> >> The division here sticks in the craw, but unless we go with > > Does it? Why? Only because of the division. But as Mike mentions later, the compiler will avoid it for powers of two. So all is well. > >> CONFIG_PRE_CON_BUF_SZ_LOG2 then I don't see an easy way around it, and >> since this is serial output we can't honestly claim to worry much >> about performance. > > Please see my previous posting > (http://article.gmane.org/gmane.comp.boot-loaders.u-boot/106810) > > I don't think 3 additional addembler instructions really play a big > role here. Yes I saw it, thanks. 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@denx.de > Every time history repeats itself the price goes up. >
Dear Mike Frysinger, In message <201108301558.08010.vapier@gentoo.org> you wrote: > > > Please see my previous posting > > (http://article.gmane.org/gmane.comp.boot-loaders.u-boot/106810) > > > > I don't think 3 additional addembler instructions really play a big > > role here. > > i'm pretty sure if you define CONFIG_PRE_CON_BUF_SZ as a power of 2 value, you > get nice & simple assembly code. so if the generated code is undesirable, > pick a CONFIG value that is power-of-2 ? This was the test code I compiled: --------------------------------------------- #define CONFIG_SYS_TMP_CON_BUF_SZ 1024 int foo(int i) { return i & (CONFIG_SYS_TMP_CON_BUF_SZ-1); } int bar(int i) { return i % CONFIG_SYS_TMP_CON_BUF_SZ; } --------------------------------------------- So was actually checking for a power-of-2 value. Best regards, Wolfgang Denk
Hi Wolfgang, On Tue, Aug 30, 2011 at 1:08 PM, Wolfgang Denk <wd@denx.de> wrote: > Dear Mike Frysinger, > > In message <201108301558.08010.vapier@gentoo.org> you wrote: >> >> > Please see my previous posting >> > (http://article.gmane.org/gmane.comp.boot-loaders.u-boot/106810) >> > >> > I don't think 3 additional addembler instructions really play a big >> > role here. >> >> i'm pretty sure if you define CONFIG_PRE_CON_BUF_SZ as a power of 2 value, you >> get nice & simple assembly code. so if the generated code is undesirable, >> pick a CONFIG value that is power-of-2 ? > > This was the test code I compiled: > > --------------------------------------------- > #define CONFIG_SYS_TMP_CON_BUF_SZ 1024 > > int foo(int i) > { > return i & (CONFIG_SYS_TMP_CON_BUF_SZ-1); > } > > int bar(int i) > { > return i % CONFIG_SYS_TMP_CON_BUF_SZ; > } > --------------------------------------------- > > So was actually checking for a power-of-2 value. In case it is interesting (which may be unlikely) here is the code generated by my compiler (common code stripped) for your example, and one I added. It seems that % makes it worry about sign. #define CONFIG_SYS_TMP_CON_BUF_SZ 1024 int foo(int i) { return i & (CONFIG_SYS_TMP_CON_BUF_SZ-1); 14: e1a03b03 lsl r3, r3, #22 18: e1a03b23 lsr r3, r3, #22 } int bar(int i) { return i % CONFIG_SYS_TMP_CON_BUF_SZ; 40: e1a02fc3 asr r2, r3, #31 44: e1a02b22 lsr r2, r2, #22 48: e0833002 add r3, r3, r2 4c: e1a03b03 lsl r3, r3, #22 50: e1a03b23 lsr r3, r3, #22 54: e0623003 rsb r3, r2, r3 } int lee(int i) { return i % (CONFIG_SYS_TMP_CON_BUF_SZ - 1); 7c: e3003803 movw r3, #2051 ; 0x803 80: e3483020 movt r3, #32800 ; 0x8020 84: e0c31293 smull r1, r3, r3, r2 88: e0833002 add r3, r3, r2 8c: e1a014c3 asr r1, r3, #9 90: e1a03fc2 asr r3, r2, #31 94: e0633001 rsb r3, r3, r1 98: e1a01003 mov r1, r3 9c: e1a01501 lsl r1, r1, #10 a0: e0631001 rsb r1, r3, r1 a4: e0613002 rsb r3, r1, r2 } 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@denx.de > "Maintain an awareness for contribution -- to your schedule, your > project, our company." - A Group of Employees >
Dear Simon Glass, In message <CAPnjgZ1dQXMVeFk47=Gg5y+TCHG7WTdGk0b1WjSqOBhu2d0xnQ@mail.gmail.com> you wrote: > > In case it is interesting (which may be unlikely) here is the code > generated by my compiler (common code stripped) for your example, and Which architecture / which compiler is this? > one I added. It seems that % makes it worry about sign. Indeed! We should use unsigned numbers here. MUCH better now: unsigned int ufoo(unsigned int i) { return i & (CONFIG_SYS_TMP_CON_BUF_SZ-1); } For ARM, BUF_SZ = 1024 mov r0, r0, asl #22 mov r0, r0, lsr #22 For PPC, BUF_SZ = 1024 rlwinm 3,3,0,22,31 The following two entries make no sense, just for completeness: For ARM, BUF_SZ = 1021 (Note: 1021 is a prime number) and r0, r0, #1020 For PPC, BUF_SZ = 1021 rlwinm 3,3,0,22,29 unsigned int ubar(unsigned int i) { return i % CONFIG_SYS_TMP_CON_BUF_SZ; } For ARM, BUF_SZ = 1024 mov r0, r0, asl #22 mov r0, r0, lsr #22 For PPC, BUF_SZ = 1024 rlwinm 3,3,0,22,31 For ARM, BUF_SZ = 1021 ldr r2, .L11 mov ip, r0 umull r1, r3, r2, r0 rsb r1, r3, r0 add r3, r3, r1, lsr #1 mov r3, r3, lsr #9 mov r0, r3, asl #10 sub r0, r0, r3, asl #2 add r0, r0, r3 rsb r0, r0, ip For PPC, BUF_SZ = 1021 lis 0,0xc0 ori 0,0,36973 mulhwu 0,3,0 subf 9,0,3 srwi 9,9,1 add 0,0,9 srwi 0,0,9 mulli 0,0,1021 subf 3,0,3 So indeed we should use unsigned arithmetics, and try to use power-of-two sizes where possible. Best regards, Wolfgang Denk
Hi Wolfgang, On Tue, Aug 30, 2011 at 1:57 PM, Wolfgang Denk <wd@denx.de> wrote: > Dear Simon Glass, > > In message <CAPnjgZ1dQXMVeFk47=Gg5y+TCHG7WTdGk0b1WjSqOBhu2d0xnQ@mail.gmail.com> you wrote: >> >> In case it is interesting (which may be unlikely) here is the code >> generated by my compiler (common code stripped) for your example, and > > Which architecture / which compiler is this? It was ARM, gcc version 4.4.3. > >> one I added. It seems that % makes it worry about sign. > > Indeed! We should use unsigned numbers here. MUCH better now: > Yes it is much better. Perhaps an (unsigned) cast in the C code is better than hoping that the board author uses 1024U or similar. > > unsigned int ufoo(unsigned int i) > { > return i & (CONFIG_SYS_TMP_CON_BUF_SZ-1); > } > > For ARM, BUF_SZ = 1024 > > mov r0, r0, asl #22 > mov r0, r0, lsr #22 > > For PPC, BUF_SZ = 1024 > > rlwinm 3,3,0,22,31 > > The following two entries make no sense, just for completeness: > > For ARM, BUF_SZ = 1021 (Note: 1021 is a prime number) > > and r0, r0, #1020 > > For PPC, BUF_SZ = 1021 > > rlwinm 3,3,0,22,29 > > > > unsigned int ubar(unsigned int i) > { > return i % CONFIG_SYS_TMP_CON_BUF_SZ; > } > > For ARM, BUF_SZ = 1024 > > mov r0, r0, asl #22 > mov r0, r0, lsr #22 > > For PPC, BUF_SZ = 1024 > > rlwinm 3,3,0,22,31 > > For ARM, BUF_SZ = 1021 > > ldr r2, .L11 > mov ip, r0 > umull r1, r3, r2, r0 > rsb r1, r3, r0 > add r3, r3, r1, lsr #1 > mov r3, r3, lsr #9 > mov r0, r3, asl #10 > sub r0, r0, r3, asl #2 > add r0, r0, r3 > rsb r0, r0, ip > > For PPC, BUF_SZ = 1021 > > lis 0,0xc0 > ori 0,0,36973 > mulhwu 0,3,0 > subf 9,0,3 > srwi 9,9,1 > add 0,0,9 > srwi 0,0,9 > mulli 0,0,1021 > subf 3,0,3 > > > So indeed we should use unsigned arithmetics, and try to use > power-of-two sizes where possible. > Yes indeed. 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@denx.de > You can't evaluate a man by logic alone. > -- McCoy, "I, Mudd", stardate 4513.3 >
Hi Simon, Mike, Wolfgang, On Wed, Aug 31, 2011 at 7:02 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Wolfgang, > > On Tue, Aug 30, 2011 at 1:57 PM, Wolfgang Denk <wd@denx.de> wrote: >> Dear Simon Glass, >> >> In message <CAPnjgZ1dQXMVeFk47=Gg5y+TCHG7WTdGk0b1WjSqOBhu2d0xnQ@mail.gmail.com> you wrote: >>> >>> In case it is interesting (which may be unlikely) here is the code >>> generated by my compiler (common code stripped) for your example, and >> >> Which architecture / which compiler is this? > > It was ARM, gcc version 4.4.3. > >> >>> one I added. It seems that % makes it worry about sign. >> >> Indeed! We should use unsigned numbers here. MUCH better now: >> > > Yes it is much better. Perhaps an (unsigned) cast in the C code is > better than hoping that the board author uses 1024U or similar. > [snip] >> >> >> So indeed we should use unsigned arithmetics, and try to use >> power-of-two sizes where possible. I'll do another spin (probably not tonight - maybe tomorrow) which covers: - inline stub functions to reduce #ifdef clutter - unsigned casting of buffer size - Documentation in README including the fact that setting the buffer size to a value which is a power of 2 will provide more optimised code (for reasonably sane compilers ;)) Now as for the calculation of the buffer address... It could be calculated dynamically in board.c but this will require another ulong in gd and the pre-console buffer will be unusable until the calculation is done (and any inadvertant printf() before this will likely cause some very bad behaviour) Also, x86 does not do these calculations dynamically and I also think it might make the pre-buffer console avaialble in early init_f Getting back to gd for a sec - This patch (like the flagify patch set I posted before) impacts the structure of gd. Does this trigger in increment to the API Version? And as observed before, if the structure of gd is fixed for any given API version, then we have a problem with all the #ifdef's that are already in gd. If it _is_ the case that the API version expects a particular fixed structure for gd, then maybe we need to remove all the #ifdefs from gd - Yes it will lead to a few dead ulongs for a few boards, but I don't see any practical alternatives. I did think about looking at doing some kind of CRC on the gd struct (like the generated asm-offsets), but I could not think of a way to do it) Regards, Graeme
Hi Wolfgang On Wed, Aug 31, 2011 at 9:00 AM, Graeme Russ <graeme.russ@gmail.com> wrote: > Hi Simon, Mike, Wolfgang, > > On Wed, Aug 31, 2011 at 7:02 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Wolfgang, >> >> On Tue, Aug 30, 2011 at 1:57 PM, Wolfgang Denk <wd@denx.de> wrote: >>> Dear Simon Glass, >>> >>> In message <CAPnjgZ1dQXMVeFk47=Gg5y+TCHG7WTdGk0b1WjSqOBhu2d0xnQ@mail.gmail.com> you wrote: >>>> [snip] > > Getting back to gd for a sec - This patch (like the flagify patch set I > posted before) impacts the structure of gd. Does this trigger in increment > to the API Version? And as observed before, if the structure of gd is > fixed for any given API version, then we have a problem with all the > #ifdef's that are already in gd. If it _is_ the case that the API version > expects a particular fixed structure for gd, then maybe we need to remove > all the #ifdefs from gd - Yes it will lead to a few dead ulongs for a few > boards, but I don't see any practical alternatives. I did think about > looking at doing some kind of CRC on the gd struct (like the generated > asm-offsets), but I could not think of a way to do it) Another gd related thought - Is it safe to assume that gd will be cleared? I hope so, as the pre-buffer console assumes gd->precon_buf_idx is initially zero. Regards, Graeme
On Tuesday, August 30, 2011 19:39:20 Graeme Russ wrote: > Another gd related thought - Is it safe to assume that gd will be cleared? > I hope so, as the pre-buffer console assumes gd->precon_buf_idx is > initially zero. i'm pretty sure you can. the Blackfin arch certainly memset's it to 0. -mike
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 4fc51fd..b85b7fe 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -38,6 +38,9 @@ typedef struct global_data { unsigned long flags; unsigned long baudrate; unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif unsigned long env_addr; /* Address of Environment struct */ unsigned long env_valid; /* Checksum of Environment valid? */ unsigned long fb_base; /* base address of frame buffer */ diff --git a/arch/avr32/include/asm/global_data.h b/arch/avr32/include/asm/global_data.h index 4ef8fc5..5c654bd 100644 --- a/arch/avr32/include/asm/global_data.h +++ b/arch/avr32/include/asm/global_data.h @@ -38,6 +38,9 @@ typedef struct global_data { unsigned long baudrate; unsigned long stack_end; /* highest stack address */ unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif unsigned long reloc_off; /* Relocation Offset */ unsigned long env_addr; /* Address of env struct */ unsigned long env_valid; /* Checksum of env valid? */ diff --git a/arch/blackfin/include/asm/global_data.h b/arch/blackfin/include/asm/global_data.h index eba5e93..f7aa711 100644 --- a/arch/blackfin/include/asm/global_data.h +++ b/arch/blackfin/include/asm/global_data.h @@ -45,6 +45,9 @@ typedef struct global_data { unsigned long board_type; unsigned long baudrate; unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif phys_size_t ram_size; /* RAM size */ unsigned long env_addr; /* Address of Environment struct */ unsigned long env_valid; /* Checksum of Environment valid? */ diff --git a/arch/m68k/include/asm/global_data.h b/arch/m68k/include/asm/global_data.h index fc486fd..0ba2b43 100644 --- a/arch/m68k/include/asm/global_data.h +++ b/arch/m68k/include/asm/global_data.h @@ -57,6 +57,9 @@ typedef struct global_data { unsigned long env_addr; /* Address of Environment struct */ unsigned long env_valid; /* Checksum of Environment valid? */ unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif #if defined(CONFIG_LCD) || defined(CONFIG_VIDEO) unsigned long fb_base; /* Base addr of framebuffer memory */ #endif diff --git a/arch/microblaze/include/asm/global_data.h b/arch/microblaze/include/asm/global_data.h index 557ad27..6e8537c 100644 --- a/arch/microblaze/include/asm/global_data.h +++ b/arch/microblaze/include/asm/global_data.h @@ -39,6 +39,9 @@ typedef struct global_data { unsigned long flags; unsigned long baudrate; unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif unsigned long env_addr; /* Address of Environment struct */ unsigned long env_valid; /* Checksum of Environment valid? */ unsigned long fb_base; /* base address of frame buffer */ diff --git a/arch/mips/include/asm/global_data.h b/arch/mips/include/asm/global_data.h index 271a290..b193517 100644 --- a/arch/mips/include/asm/global_data.h +++ b/arch/mips/include/asm/global_data.h @@ -41,6 +41,9 @@ typedef struct global_data { unsigned long flags; unsigned long baudrate; unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif phys_size_t ram_size; /* RAM size */ unsigned long reloc_off; /* Relocation Offset */ unsigned long env_addr; /* Address of Environment struct */ diff --git a/arch/nios2/include/asm/global_data.h b/arch/nios2/include/asm/global_data.h index 2c4a719..d9f0664 100644 --- a/arch/nios2/include/asm/global_data.h +++ b/arch/nios2/include/asm/global_data.h @@ -29,6 +29,9 @@ typedef struct global_data { unsigned long baudrate; unsigned long cpu_clk; /* CPU clock in Hz! */ unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif phys_size_t ram_size; /* RAM size */ unsigned long env_addr; /* Address of Environment struct */ unsigned long env_valid; /* Checksum of Environment valid */ diff --git a/arch/powerpc/include/asm/global_data.h b/arch/powerpc/include/asm/global_data.h index a33ca2f..7fcaf38 100644 --- a/arch/powerpc/include/asm/global_data.h +++ b/arch/powerpc/include/asm/global_data.h @@ -138,6 +138,9 @@ typedef struct global_data { unsigned long env_addr; /* Address of Environment struct */ unsigned long env_valid; /* Checksum of Environment valid? */ unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif #if defined(CONFIG_SYS_ALLOC_DPRAM) || defined(CONFIG_CPM2) unsigned int dp_alloc_base; unsigned int dp_alloc_top; diff --git a/arch/sh/include/asm/global_data.h b/arch/sh/include/asm/global_data.h index 0c09ba9..1b782fc 100644 --- a/arch/sh/include/asm/global_data.h +++ b/arch/sh/include/asm/global_data.h @@ -34,6 +34,9 @@ typedef struct global_data unsigned long baudrate; unsigned long cpu_clk; /* CPU clock in Hz! */ unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif phys_size_t ram_size; /* RAM size */ unsigned long env_addr; /* Address of Environment struct */ unsigned long env_valid; /* Checksum of Environment valid */ diff --git a/arch/sparc/include/asm/global_data.h b/arch/sparc/include/asm/global_data.h index 9b14674..a1e4b44 100644 --- a/arch/sparc/include/asm/global_data.h +++ b/arch/sparc/include/asm/global_data.h @@ -53,6 +53,9 @@ typedef struct global_data { unsigned long env_valid; /* Checksum of Environment valid? */ unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif #if defined(CONFIG_LCD) || defined(CONFIG_VIDEO) unsigned long fb_base; /* Base address of framebuffer memory */ #endif diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h index f977dbe..6cf7955 100644 --- a/arch/x86/include/asm/global_data.h +++ b/arch/x86/include/asm/global_data.h @@ -40,6 +40,9 @@ typedef struct global_data { unsigned long flags; unsigned long baudrate; unsigned long have_console; /* serial_init() was called */ +#ifdef CONFIG_PRE_CONSOLE_BUFFER + unsigned long precon_buf_idx; /* Pre-Console buffer index */ +#endif unsigned long reloc_off; /* Relocation Offset */ unsigned long load_off; /* Load Offset */ unsigned long env_addr; /* Address of Environment struct */ diff --git a/common/console.c b/common/console.c index b23d933..ae87148 100644 --- a/common/console.c +++ b/common/console.c @@ -329,6 +329,35 @@ int tstc(void) return serial_tstc(); } +#ifdef CONFIG_PRE_CONSOLE_BUFFER +#define CIRC_BUF_IDX(idx) ((idx) % CONFIG_PRE_CON_BUF_SZ) + +void pre_console_putc(const char c) +{ + char *buffer = (char *)CONFIG_PRE_CON_BUF_ADDR; + + buffer[CIRC_BUF_IDX(gd->precon_buf_idx++)] = c; +} + +void pre_console_puts(const char *s) +{ + while (*s) + pre_console_putc(*s++); +} + +void print_pre_console_buffer(void) +{ + unsigned long i = 0; + char *buffer = (char *)CONFIG_PRE_CON_BUF_ADDR; + + if (gd->precon_buf_idx > CONFIG_PRE_CON_BUF_SZ) + i = gd->precon_buf_idx - CONFIG_PRE_CON_BUF_SZ; + + while (i < gd->precon_buf_idx) + putc(buffer[CIRC_BUF_IDX(i++)]); +} +#endif + void putc(const char c) { #ifdef CONFIG_SILENT_CONSOLE @@ -341,8 +370,12 @@ void putc(const char c) return; #endif - if (!gd->have_console) + if (!gd->have_console) { +#ifdef CONFIG_PRE_CONSOLE_BUFFER + pre_console_putc(c); +#endif return; + } if (gd->flags & GD_FLG_DEVINIT) { /* Send to the standard output */ @@ -365,8 +398,12 @@ void puts(const char *s) return; #endif - if (!gd->have_console) + if (!gd->have_console) { +#ifdef CONFIG_PRE_CONSOLE_BUFFER + pre_console_puts(s); +#endif return; + } if (gd->flags & GD_FLG_DEVINIT) { /* Send to the standard output */ @@ -383,8 +420,10 @@ int printf(const char *fmt, ...) uint i; char printbuffer[CONFIG_SYS_PBSIZE]; +#ifndef CONFIG_PRE_CONSOLE_BUFFER if (!gd->have_console) return 0; +#endif va_start(args, fmt); @@ -404,8 +443,10 @@ int vprintf(const char *fmt, va_list args) uint i; char printbuffer[CONFIG_SYS_PBSIZE]; +#ifndef CONFIG_PRE_CONSOLE_BUFFER if (!gd->have_console) return 0; +#endif /* For this to work, printbuffer must be larger than * anything we ever want to print. @@ -547,6 +588,10 @@ int console_init_f(void) gd->flags |= GD_FLG_SILENT; #endif +#ifdef CONFIG_PRE_CONSOLE_BUFFER + print_pre_console_buffer(); +#endif + return 0; }
Allow redirection of console output prior to console initialisation to a temporary buffer. To enable this functionality, the board configuration file must define: - CONFIG_PRE_CONSOLE_BUFFER - Enable pre-console buffer - CONFIG_PRE_CON_BUF_ADDR - Base address of pre-console buffer - CONFIG_PRE_CON_BUF_SZ - Size of pre-console buffer (in bytes) The pre-console buffer will buffer the last CONFIG_PRE_CON_BUF_SZ bytes Any earlier characters are silently dropped. Signed-off-by: Graeme Russ <graeme.russ@gmail.com> --- Changes Since V1 - Implemented circular buffer - Trivial code style corrections arch/arm/include/asm/global_data.h | 3 ++ arch/avr32/include/asm/global_data.h | 3 ++ arch/blackfin/include/asm/global_data.h | 3 ++ arch/m68k/include/asm/global_data.h | 3 ++ arch/microblaze/include/asm/global_data.h | 3 ++ arch/mips/include/asm/global_data.h | 3 ++ arch/nios2/include/asm/global_data.h | 3 ++ arch/powerpc/include/asm/global_data.h | 3 ++ arch/sh/include/asm/global_data.h | 3 ++ arch/sparc/include/asm/global_data.h | 3 ++ arch/x86/include/asm/global_data.h | 3 ++ common/console.c | 49 +++++++++++++++++++++++++++- 12 files changed, 80 insertions(+), 2 deletions(-) -- 1.7.5.2.317.g391b14