[U-Boot] [PATCH v3 03/29] Convert CONSOLE_PRE_CONSOLE_BUFFER options to Kconfig

[Soeren Moch]

>> > Hello Simon,
>> > 
>> > On Thu, 29 Sep 2016 14:23:02 -0600
>> > Simon Glass <sjg at chromium.org> wrote:
>> > 
>>> > > Move these option to Kconfig and tidy up existing uses.
>>> > > 
>>> > > Signed-off-by: Simon Glass <sjg at chromium.org>
>>> > > ---
>>> > > 
>>> > > Changes in v3: None
>>> > > Changes in v2:
>>> > > - Change CONFIG_PRE_CON_BUF_SZ default to 4096
>>> > > - Change CONFIG_PRE_CON_BUF_SZ to 'int' type
>>> > > - Drop the depend clause on the CONFIG_PRE_CON_BUF_SZ default
>>> > > - Move CONFIG_PRE_CON_BUF_ADDR default to common/Kconfig
>> > 
>> > What is the point moving these defines to Kconfig? They are neither
>> > user configurable, nor board specific. The location of the buffer is
>> > defined per platform or per SoC type and is a part of the global memory
>> > map. Similar to such things as the malloc heap and the stack.
> This is a good point to bring up.  As we're discussing in another thread
> about moving FSL things out of CONFIG_SYS_... and into Kconfig or a
> different namespace, we have other examples today where there's
> addresses in Kconfig.  Looking harder still at this code, perhaps as a
> follow-up CONFIG_PRE_CON_BUF_SZ should just be 'PRE_CON_BUF_SIZE' in the
> code and 4096, and not in Kconfig at all.  And for this series, make the
> default tbs2910 uses the default for ARCH_MX6.
I don't think that my arbitrarily chosen PRE_CON_BUF_ADDR for tbs2910
(0x7C000000) is a good default for ARCH_MX6. On tbs2910 we have 2GB of
DDR memory, there are lots of other imx6 boards with less memory available.
So a lower PRE_CON_BUF_ADDR (as for sunxi) might be a better choice.

The only constraint for this buffer (besides being located in DDR) is,
that it must not overlap with u-boot text and data.
As I understand it, this buffer is only used very early in the boot process
and not needed anymore, when the vga console is set up. So this buffer can
not interfere with user commands (even pre-boot commands).
> > Allowing the users to redefine the buffer location is a dangerous thing
> > because everything may go out of control very easily (it may overlap
> > with some other memory buffer). IMHO it only makes sense to have a
> > single user configurable boolean flag in Kconfig (the one which
> > enables/disables the pre-console functionality).
> > 
> > Regarding the buffer size. It was originally picked rather arbitrarily
> > as 1MB at least for the sunxi platform:
> > 
> >     https://patchwork.ozlabs.org/patch/426526/
> > 
> > Just because making it several orders of magnitude larger than
> > necessary makes it extremely unlikely that anyone ever gets into
> > a buffer wraparound situation. Picking smallish sizes does not gain
> > us anything, but just adds an extra hassle because now one needs to
> > make some estimations whether the size is large enough or not.
> > Especially considering that this functionality may be sometimes
> > used for debugging prints when troubleshooting something. And one
> > can't easily predict how much debugging output would be actually
> > necessary.
> So maybe we should hide the size option under EXPERT?
Just curious, can this address be generated automatically, e.g. just below
the malloc area? Or is this too complicated, e.g. because the buffer is
used already before relocation?

However, if this address is configurable in Kconfig at all, for me it makes
sense to hide it somehow.

I can test other PRE_CON_BUF_ADDR settings on tbs2910 if this would help
to find more general default values.

Regards,
Soeren