[U-Boot] [PATCH 1/3] [RFC] Add support for early serial debug console

Peter Tyser ptyser at xes-inc.com
Sat Aug 16 00:25:08 CEST 2008


> I understand what you are trying todo, but I think it doesn't work.
> 
> You are invoking a numer of pretty complex functions (like readline()
> and run_command() and ...) which in turn ionvoke other functions etc.
> - all of them written in C with theassumption that they have a  valid
> C runtime environment, which is simply not the case before relocation
> to RAM.

I attempted to account for this fact.  I followed the program flow of
run_command() and readline() in particular looking for global data
writes and bss reads.  For the instances where global data was being
written, I made it conditional on (gd->flags & GD_FLG_RELOC).  I also
forced some variables to reside in the data section so that they could
be read while executing from flash (__attribute__ ((section(".data")))).
I also monitored the flash WE pin while executing the memory and i2c
commands to make sure that the data section (which is still in flash)
was not being written to.  There is a chance that uninitialized data is
being read from the bss, but I didn't see any and the board functions
fine:)

> And your patch seems to be inclomplete, it does not apply.

Hmm, I built the patches off 07efc9e321619c3dec213310c32e011aa6f02783
with the 2 patches I just submitted also applied ("mod_i2c_mem()
bugfix" and "Replace references to extern console_buffer with a function
call").  What error are you seeing while applying it?


> Some parts seem to be missing (like the necessary changes to eliminate
> accesses to the console_buffer[] in BSS).

I don't follow.  The console_buffer_addr() conditionally places the
console_buffer in SDRAM if U-Boot has relocated, or at
CFG_DEBUG_CONSOLE_ADDR if executing from flash.  That's why the "Replace
references to extern console_buffer with a function call" patch I just
submitted was necessary.

> But in any case - You make a few commands usable, and the behaviour of
> the remainig undefined. 
> 
> I don't think this is a good idea.

I agree that it is hokey that some commands work, some don't.  However,
the ones that are very useful do work:)  The memory test, modify, and
display functions as well as the i2c functions work.  These few commands
allow further diagnosis (and possible fixing) of most board hardware
problems that prevent a board from booting.

> 
> And we pay for this with a log of uglier code (many, many #ifdef's)
> and increased memory size.

Agreed, the code is a bit dirtier, but I think the #ifdefs could be
minimized some.  For example, the sequence of code in lib_xxx/board.c:
 for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
        if ((*init_fnc_ptr)() != 0 {
#ifdef CONFIG_DEBUG_CONSOLE
....

could be put in 1 file for all architectures to share.  In that case,
there would only be an additional 4-5 #ifdefs added.  The patches
themselves aren't all that large.

> I'm interested to hear what others say, but so far I tend to reject
> this.

Thanks for the feedback, I'm curious what others have to say as well.
Let me know if I can provide any additional details.

Peter




More information about the U-Boot mailing list