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

[Tom Rini]

On Fri, Sep 30, 2016 at 09:00:44AM +0300, Siarhei Siamashka wrote:
> 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.

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

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160930/243769a8/attachment.sig>