Re: [PATCH v3 1/5] Image size checks: Move all configs in one place

[Philip Oberfichtner <pro@denx.de>]

On Fri, Sep 05, 2025 at 08:22:35AM -0600, Tom Rini wrote:
> On Fri, Sep 05, 2025 at 10:13:36AM +0200, Philip Oberfichtner wrote:
> > On Thu, Sep 04, 2025 at 03:54:54PM +0200, Marek Vasut wrote:
> > > On 9/4/25 10:12 AM, Philip Oberfichtner wrote:
> > > > On Wed, Sep 03, 2025 at 02:50:49PM +0200, Marek Vasut wrote:
> > > > > On 9/3/25 12:56 PM, Philip Oberfichtner wrote:
> > > > > > How image size limiting works in U-Boot should be easier to grasp if we
> > > > > > have all related options in one place.
> > > > > > 
> > > > > > Signed-off-by: Philip Oberfichtner <pro@denx.de>
> > > > > > ---
> > > > > >    Kconfig                | 59 ++++++++++++++++++++++++++++++++++++++++++
> > > > > >    common/spl/Kconfig     | 38 ---------------------------
> > > > > >    common/spl/Kconfig.tpl |  7 -----
> > > > > >    common/spl/Kconfig.vpl |  7 -----
> > > > > >    4 files changed, 59 insertions(+), 52 deletions(-)
> > > > > > 
> > > > > > diff --git a/Kconfig b/Kconfig
> > > > > > index 70125c4e5a6..f1a6be299e8 100644
> > > > > > --- a/Kconfig
> > > > > > +++ b/Kconfig
> > > > > > @@ -535,6 +535,8 @@ config BUILD_TARGET
> > > > > >    	  special image will be automatically built upon calling
> > > > > >    	  make / buildman.
> > > > > > +menu "Image size limits"
> > > > > > +
> > > > > >    config HAS_BOARD_SIZE_LIMIT
> > > > > >    	bool "Define a maximum size for the U-Boot image"
> > > > > >    	depends on !COMPILE_TEST
> > > > > > @@ -554,6 +556,63 @@ config BOARD_SIZE_LIMIT
> > > > > >    	  include SPL nor TPL, on platforms that use that functionality, they
> > > > > >    	  have separate options to restrict size.
> > > > > > +config VPL_SIZE_LIMIT
> > > > > > +	depends on VPL
> > > > > > +	hex "Maximum size of VPL image"
> > > > > > +	default 0x0
> > > > > > +	help
> > > > > > +	  Specifies the maximum length of the U-Boot VPL image.
> > > > > > +	  If this value is zero, it is ignored.
> > > > > > +
> > > > > > +config TPL_SIZE_LIMIT
> > > > > > +	depends on TPL
> > > > > > +	hex "Maximum size of TPL image"
> > > > > > +	default 0x0
> > > > > > +	help
> > > > > > +	  Specifies the maximum length of the U-Boot TPL image.
> > > > > > +	  If this value is zero, it is ignored.
> > > > > The point of separate common/spl/Kconfig.tpl and common/spl/Kconfig.vpl
> > > > > files is to group SPL / TPL / VPL symbols in those files .
> > > > 
> > > > There is "config TPL_" definitions literally all over the place. I think
> > > > there might actually even be more of them outside Kconfig.tpl than
> > > > inside.
> > > 
> > > That does not make it OK to make that situation worse.
> > 
> > It doesn't. Someone who stumbles over BOARD_SIZE_LIMIT might be asking
> > himself, if there's an analogous option for SPL, too. It'll jump right
> > into his eyes, after this patch of mine.
> > 
> > And apparently there were other contributers before me who found it
> > reasonable to group strongly related options together within one menu,
> > instead of spreading them out over many different places. Let me give
> > you a few examples (and this list is by far not exhaustive):
> > 
> > 
> > 	Kconfig:
> > 		config SYS_MALLOC_F
> > 		config SPL_SYS_MALLOC_F
> > 		config TPL_SYS_MALLOC_F
> > 		config VPL_SYS_MALLOC_F
> > 
> > 	arch/Kconfig 
> > 		menu "Skipping low level initialization functions"
> > 		config SKIP_LOWLEVEL_INIT
> > 		config SPL_SKIP_LOWLEVEL_INIT
> > 		config TPL_SKIP_LOWLEVEL_INIT
> > 
> > 	boot/Kconfig
> > 		menuconfig FIT
> > 		config SPL_FIT
> > 		config VPL_FIT
> > 		config TPL_FIT
> > 
> > 
> > 	common/Kconfig
> > 		menu "Logging"
> > 		config LOG
> > 		config SPL_LOG
> > 		config TPL_LOG
> > 		config VPL_LOG
> > 
> > 	dts/Kconfig
> > 		menu "Device Tree Control"
> > 		config OF_CONTROL
> > 		config SPL_OF_CONTROL
> > 		config TPL_OF_CONTROL
> > 		config VPL_OF_CONTROL
> > 
> > 	lib/Kconfig
> > 		menu "Compression Support"
> > 		config LZ4
> > 		config SPL_LZ4
> > 		config TPL_LZ4
> > 		config VPL_LZ4
> > 
> > 
> > So how do these and many other cases differ from the SIZE_LIMIT case
> > discussed here? Or would it be better to tear all those menus apart?
> 
> We are not consistent about this, and it really becomes a stylistic
> argument. Roughly speaking, common/spl/Kconfig.* is for things core to a
> given phase (SPL, TPL, VPL) which is why often but not always, say
> compression algorithms are with the compression algorithms. In this
> case, limits are a core part of the phase so I think Marek's point is
> valid here.

Okay. Thanks for the clarification.