U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Philip Oberfichtner <pro@denx.de>
To: Tom Rini <trini@konsulko.com>
Cc: Marek Vasut <marek.vasut@mailbox.org>,
	u-boot@lists.denx.de,
	Mattijs Korpershoek <mkorpershoek@kernel.org>,
	Michael Walle <michael@walle.cc>,
	Quentin Schulz <quentin.schulz@cherry.de>,
	Sean Anderson <seanga2@gmail.com>, Simon Glass <sjg@chromium.org>
Subject: Re: [PATCH v2 1/3] Image size checks: Remove HAS_BOARD_SIZE_LIMIT
Date: Mon, 11 Aug 2025 10:50:14 +0200	[thread overview]
Message-ID: <aJmuxlTEFT4R7HR_@antares> (raw)
In-Reply-To: <20250807232421.GD124814@bill-the-cat>

On Thu, Aug 07, 2025 at 05:24:21PM -0600, Tom Rini wrote:
> On Fri, Aug 08, 2025 at 01:15:45AM +0200, Marek Vasut wrote:
> > On 8/7/25 10:11 PM, Tom Rini wrote:
> > > On Thu, Aug 07, 2025 at 09:41:34PM +0200, Marek Vasut wrote:
> > > > On 8/7/25 6:21 PM, Tom Rini wrote:
> > > > > On Thu, Aug 07, 2025 at 03:41:38PM +0200, Marek Vasut wrote:
> > > > > > On 8/7/25 12:24 PM, Philip Oberfichtner wrote:
> > > > > > > CONFIG_HAS_BOARD_SIZE_LIMIT is obsolete, if we interpret the value
> > > > > > > "zero" as "unlimited".
> > > > > > 
> > > > > > This sentence makes no sense. Is the variable not obsolete if its value is
> > > > > > non-zero ?
> > > > > 
> > > > > This is phrased oddly, yes. How about:
> > > > > By making the code treat a size limit of 0 as unlimited we no longer
> > > > > need to guard asking about having a size limit on the platform.
> > > > 
> > > > 0 shouldn't mean unlimited, that is just fragile ...
> > > 
> > > That's a standard unix thing? ulimit -c 0 is unlimited.
> > 
> > This is a really bad argument, because then the counter-argument is, that
> > size = 0 is also a valid size and it shouldn't be conflated with SIZE_LIMIT
> > validity.
> > 
> > My take on this is, don't conflate size-limit "enabled/disabled" with
> > size-limit "value" , these are two separate config options. Mixing them is
> > not helping.
> 
> I still think it's fine, but it's not worth arguing further over, and we
> can just make sure to gate all of the symbols rather than 0-is-disabled.

The idea of treating a size limit of zero as unlimited has been common
practice in mainline U-Boot since 2019, where CONFIG_SPL_SIZE_LIMIT has
been introduced. The same logic has later been applied to TPL and VPL
size limits.

If we want to consistently stick to the HAS_*_SIZE_LIMIT approach, we'd
have to introduce four extra Kconfig options alongside
HAS_BOARD_SIZE_LIMIT:

	CONFIG_HAS_UBOOT_WITH_SPL_SIZE_LIMIT
	CONFIG_HAS_SPL_SIZE_LIMIT
	CONFIG_HAS_TPL_SIZE_LIMIT
	CONFIG_HAS_VPL_SIZE_LIMIT


Furthermore, the extra lines of code in the toplevel Makefile, which
could otherwise be removed:

	ifneq ($(CONFIG_BOARD_SIZE_LIMIT),)
	BOARD_SIZE_CHECK= @ $(call size_check,$@,$(CONFIG_BOARD_SIZE_LIMIT))
	else
	BOARD_SIZE_CHECK =
	endif

	ifneq ($(CONFIG_HAS_UBOOT_WITH_SPL_SIZE_LIMIT),0x0)
	UBOOT_WITH_SPL_SIZE_CHECK = @$(call size_check,$@,$(CONFIG_UBOOT_WITH_SPL_SIZE_LIMIT)
	else
	UBOOT_WITH_SPL_SIZE_CHECK =
	endif

	ifneq ($(CONFIG_SPL_SIZE_LIMIT),0x0)
	SPL_SIZE_CHECK = @$(call size_check,$@,$$(tools/spl_size_limit))
	else
	SPL_SIZE_CHECK =
	endif

	ifneq ($(CONFIG_TPL_SIZE_LIMIT),0x0)
	TPL_SIZE_CHECK = @$(call size_check,$@,$(CONFIG_TPL_SIZE_LIMIT))
	else
	TPL_SIZE_CHECK =
	endif

	ifneq ($(CONFIG_VPL_SIZE_LIMIT),0x0)
	VPL_SIZE_CHECK = @$(call size_check,$@,$(CONFIG_VPL_SIZE_LIMIT))
	else
	VPL_SIZE_CHECK =
	endif


Is it really worth adding this much of extra code?


Best regards,
Philip



> 
> -- 
> Tom



-- 
=====================================================================
DENX Software Engineering GmbH,
Managing Director: Johanna Denk, Tabea Lutz
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
=====================================================================

  reply	other threads:[~2025-08-11  8:50 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-07 10:24 [PATCH v2 0/3] Simplify image size checks Philip Oberfichtner
2025-08-07 10:24 ` [PATCH v2 1/3] Image size checks: Remove HAS_BOARD_SIZE_LIMIT Philip Oberfichtner
2025-08-07 13:41   ` Marek Vasut
2025-08-07 16:21     ` Tom Rini
2025-08-07 19:41       ` Marek Vasut
2025-08-07 20:11         ` Tom Rini
2025-08-07 23:15           ` Marek Vasut
2025-08-07 23:24             ` Tom Rini
2025-08-11  8:50               ` Philip Oberfichtner [this message]
2025-08-18  9:29                 ` Philip Oberfichtner
2025-08-25  8:59                   ` Philip Oberfichtner
2025-08-28 21:44                 ` Marek Vasut
2025-09-01  9:38                   ` Philip Oberfichtner
2025-09-01 19:41                     ` Marek Vasut
2025-08-11  8:30     ` Philip Oberfichtner
2025-08-07 10:24 ` [PATCH v2 2/3] Image size checks: Move all configs in one place Philip Oberfichtner
2025-08-07 10:24 ` [PATCH v2 3/3] Image size checks: Simplify help text Philip Oberfichtner
2025-08-07 13:41   ` Marek Vasut
2025-08-11  8:57     ` Philip Oberfichtner
2025-08-11 10:28       ` Heinrich Schuchardt
2025-08-11 14:25       ` Marek Vasut
2025-08-12  9:51         ` Philip Oberfichtner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aJmuxlTEFT4R7HR_@antares \
    --to=pro@denx.de \
    --cc=marek.vasut@mailbox.org \
    --cc=michael@walle.cc \
    --cc=mkorpershoek@kernel.org \
    --cc=quentin.schulz@cherry.de \
    --cc=seanga2@gmail.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox