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 <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.
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
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
