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

[Graeme Russ]

Hi Mike,

On Tue, Aug 30, 2011 at 11:32 AM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Monday, August 29, 2011 19:00:46 Graeme Russ wrote:
>> On Tue, Aug 30, 2011 at 6:10 AM, Mike Frysinger wrote:
>> > On Monday, August 29, 2011 15:42:23 Simon Glass wrote:
>> >> On Mon, Aug 29, 2011 at 12:20 PM, Mike Frysinger wrote:
>> >> > On Monday, August 29, 2011 13:21:57 Simon Glass wrote:
>> >> >> +     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.
>
> 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

>> >> 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

>> > 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