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

[Graeme Russ]

Hi Mike,

On Tue, Aug 30, 2011 at 12:50 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Monday, August 29, 2011 21:49:33 Graeme Russ wrote:
>> On Tue, Aug 30, 2011 at 11:32 AM, Mike Frysinger 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:
>> >> >> 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
>
> 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")

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

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