Hi Siarhei, On 30 September 2016 at 00:00, Siarhei Siamashka <siarhei.siamas...@gmail.com> wrote: > > Hello Simon, > > On Thu, 29 Sep 2016 14:23:02 -0600 > Simon Glass <s...@chromium.org> wrote: > > > Move these option to Kconfig and tidy up existing uses. > > > > Signed-off-by: Simon Glass <s...@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. > > 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.
All CONFIGs need to move to Kconfig or be deleted. I also don't like the idea of specifying an address for a pre-console buffer. As mentioned I think it is better to allocate it automatically. But that is a separate issue from the Kconfig conversion. if we re-litigate every CONFIG that we move, it will never happen. > > > README | 17 ---------------- > > board/sunxi/Kconfig | 3 +++ > > common/Kconfig | 42 > > +++++++++++++++++++++++++++++++++++++++ > > common/console.c | 6 +++--- > > configs/tbs2910_defconfig | 2 ++ > > include/asm-generic/global_data.h | 2 +- > > include/configs/sunxi-common.h | 6 ------ > > include/configs/tbs2910.h | 4 ---- > > scripts/config_whitelist.txt | 3 --- > > 9 files changed, 51 insertions(+), 34 deletions(-) > > > > diff --git a/README b/README > > index 0a1f3fe..8f93dad 100644 > > --- a/README > > +++ b/README > > @@ -872,23 +872,6 @@ The following options need to be configured: > > must be defined, to setup the maximum idle timeout for > > the SMC. > > > > -- Pre-Console Buffer: > > - Prior to the console being initialised (i.e. serial UART > > - initialised etc) all console output is silently discarded. > > - Defining CONFIG_PRE_CONSOLE_BUFFER will cause U-Boot to > > - buffer any console messages prior to the console being > > - initialised to a buffer of size CONFIG_PRE_CON_BUF_SZ > > - bytes located at CONFIG_PRE_CON_BUF_ADDR. The buffer is > > - a circular buffer, so if more than CONFIG_PRE_CON_BUF_SZ > > - bytes are output before the console is initialised, the > > - earlier bytes are discarded. > > - > > - Note that when printing the buffer a copy is made on the > > - stack so CONFIG_PRE_CON_BUF_SZ must fit on the stack. > > - > > - 'Sane' compilers will generate smaller code if > > - CONFIG_PRE_CON_BUF_SZ is a power of 2 > > - > > - Autoboot Command: > > CONFIG_BOOTCOMMAND > > Only needed when CONFIG_BOOTDELAY is enabled; > > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig > > index b139d1c..c0ffeb3 100644 > > --- a/board/sunxi/Kconfig > > +++ b/board/sunxi/Kconfig > > @@ -3,6 +3,9 @@ if ARCH_SUNXI > > config IDENT_STRING > > default " Allwinner Technology" > > > > +config PRE_CONSOLE_BUFFER > > + default y > > + > > config SPL_GPIO_SUPPORT > > default y > > > > diff --git a/common/Kconfig b/common/Kconfig > > index bbd5633..6ee67ac 100644 > > --- a/common/Kconfig > > +++ b/common/Kconfig > > @@ -246,6 +246,48 @@ config SILENT_CONSOLE_UPDATE_ON_RELOC > > (e.g. NAND). This option makes the value of the 'silent' > > environment variable take effect at relocation. > > > > +config PRE_CONSOLE_BUFFER > > + bool "Buffer characters before the console is available" > > + help > > + Prior to the console being initialised (i.e. serial UART > > + initialised etc) all console output is silently discarded. > > + Defining CONFIG_PRE_CONSOLE_BUFFER will cause U-Boot to > > + buffer any console messages prior to the console being > > + initialised to a buffer. The buffer is a circular buffer, so > > + if it overflows, earlier output is discarded. > > + > > + Note that this is not currently supported in SPL. It would be > > + useful to be able to share the pre-console buffer with SPL. > > We can't (easily) share this buffer with SPL. In the SPL case, this > buffer needs to be placed somewhere in SRAM, because it is necessary > to also have console messages even before DRAM is initialized. > Essentially, we want to have two separate buffers. One small buffer > somewhere in SRAM for SPL, and one very large buffer somewhere in > DRAM for U-Boot proper. The data from the small buffer gets combined > with the data from the large buffer at some point. The actual > implementation is very straightforward and simple. And in fact it > has been already available since a long time ago: > > http://lists.denx.de/pipermail/u-boot/2015-January/201127.html > http://lists.denx.de/pipermail/u-boot/2015-January/201128.html > http://lists.denx.de/pipermail/u-boot/2015-January/201129.html > > Again, assigning the exact location of these buffers is a very platform > specific information and is a part of the memory map. The user has no > business overriding this via Kconfig. > > BTW, would you want me to rebase these old patches on the current > U-Boot git master branch? Yes please. I'd like to see this setup moved to board_init.c, with just a size CONFIG options, not an address. > > > + > > +config PRE_CON_BUF_SZ > > + int "Sets the size of the pre-console buffer" > > + depends on PRE_CONSOLE_BUFFER > > + default 4096 > > + help > > + The size of the pre-console buffer affects how much console output > > + can be held before it overflows and starts discarding earlier > > + output. Normally there is very little output at this early stage, > > + unless debugging is enabled, so allow enough for ~10 lines of > > + text. > > + > > + This is a useful feature if you are using a video console and > > + want to see the full boot output on the console. Without this > > + option only the post-relocation output will be displayed. > > + > > +config PRE_CON_BUF_ADDR > > + hex "Address of the pre-console buffer" > > + depends on PRE_CONSOLE_BUFFER > > + default 0x2f000000 if ARCH_SUNXI && MACH_SUN9I > > + default 0x4f000000 if ARCH_SUNXI && !MACH_SUN9I > > + help > > + This sets the start address of the pre-console buffer. This must > > + be in available memory and is accessed before relocation and > > + possibly before DRAM is set up. Therefore choose an address > > + carefully. > > + > > + We should consider removing this option and allocating the memory > > + in board_init_f_init_reserve() instead. > > And what is the profit? The implementation becomes more complex, more > fragile and defeats the whole purpose. We want to be able to print > and accumulate debugging messages immediately, and not only after > certain things are initialized relatively late in the boot process. > That's exactly the reason why this pre-console buffer functionality > exists in the first place. We cannot print anything until we have global_data set up. This happens in board_init_f_init_reserve() so there is no point in having a console before then. We should allocate all the init memory in one place, in C code. This is very early in the boot process. Please take a look. [...] Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
