[PATCH 20/20] Convert CONFIG_SYS_BOOTM_LEN to Kconfig

[Tom Rini]

On Sat, Jun 25, 2022 at 07:59:36PM +0200, Soeren Moch wrote:
> Hi Tom,
> 
> On 25.06.22 17:02, Tom Rini wrote:
> > This converts the following to Kconfig:
> >     CONFIG_SYS_BOOTM_LEN
> > 
> > As part of this, rework error handling in boot/bootm.c so that we pass
> > the buffer size to handle_decomp_error as CONFIG_SYS_BOOTM_LEN will not
> > be available to host tools but we do know the size that we passed to
> > malloc().
> > 
> > Signed-off-by: Tom Rini <trini at konsulko.com>
> > -- > README                                        |  6 -----
> > boot/bootm.c                                  | 24 +++++++++----------
> > cmd/Kconfig                                   | 10 ++++++++
> > configs/M5208EVBE_defconfig                   |  1 +
> [...]
> >   include/configs/xilinx_zynqmp_r5.h            |  1 -
> >   include/configs/zynq-common.h                 |  1 -
> >   scripts/config_whitelist.txt                  |  1 -
> >   555 files changed, 419 insertions(+), 270 deletions(-)
> 
> Is it really necessary to create so many additional lines just for a
> conversion?

It's about as small as I can get it to without spending a lot of time
massaging defaults.  As is the case with any of the conversions,
arch/SoC maintainers or other knowledgable parties are welcome and
encouraged to submit a follow-up to add default X if Y.  It's keeping
the in-use value for everyone and unfortunately aside from platforms
that didn't change from the old default, there's not a lot of
consistency, but arguably should be.

> > diff --git a/README b/README
> > index fb0284d4ecb6..ff0df3797d21 100644
> > --- a/README
> > +++ b/README
> > @@ -1736,12 +1736,6 @@ Configuration Settings:
> > 
> >   		Non-cached memory is only supported on 32-bit ARM at present.
> > 
> > -- CONFIG_SYS_BOOTM_LEN:
> > -		Normally compressed uImages are limited to an
> > -		uncompressed size of 8 MBytes. If this is not enough,
> > -		you can define CONFIG_SYS_BOOTM_LEN in your board config file
> > -		to adjust this setting to your needs.
> > -
> >   - CONFIG_SYS_BOOTMAPSZ:
> >   		Maximum size of memory mapped by the startup code of
> >   		the Linux kernel; all data that must be processed by
> > diff --git a/boot/bootm.c b/boot/bootm.c
> > index dfa65f125e57..86dbfbcfed5b 100644
> > --- a/boot/bootm.c
> > +++ b/boot/bootm.c
> > @@ -33,11 +33,6 @@
> >   #include <bootm.h>
> >   #include <image.h>
> > 
> > -#ifndef CONFIG_SYS_BOOTM_LEN
> > -/* use 8MByte as default max gunzip size */
> > -#define CONFIG_SYS_BOOTM_LEN	0x800000
> > -#endif
> > -
> 
> The old default was 8MiB, OK.
> 
> >   #define MAX_CMDLINE_SIZE	SZ_4K
> > 
> >   #define IH_INITRD_ARCH IH_ARCH_DEFAULT
> [...]
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index bb956e330750..357b82849f7f 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -335,6 +335,16 @@ config BOOTM_VXWORKS
> >   	help
> >   	  Support booting VxWorks images via the bootm command.
> > 
> > +config SYS_BOOTM_LEN
> > +	hex "Maximum size of a decompresed OS image"
> > +	depends on CMD_BOOTM || CMD_BOOTI || CMD_BOOTZ
> > +	default 0x4000000 if PPC || ARM64
> > +	default 0x1000000 if X86
> > +	default 0x800000
> > +	help
> > +	  Normally compressed OS images are limited to an uncompressed size of
> > +	  8 MiB. If this is not enough, increase this setting to fit your needs.
> > +
> 
> The new default is sad to be 8MiB, too (as the old default was), but
> this is not true for all architectures. So the help text is misleading.

English is an imprecise language.  Maybe just not mentioning the size in
the help is best.

> Besides that, there is probably nothing wrong with the new defaults.
> 
> >   config CMD_BOOTEFI
> >   	bool "bootefi"
> >   	depends on EFI_LOADER
> > diff --git a/configs/M5208EVBE_defconfig b/configs/M5208EVBE_defconfig
> > index 4ab888f59ed5..858fc5bbfd2e 100644
> > --- a/configs/M5208EVBE_defconfig
> > +++ b/configs/M5208EVBE_defconfig
> > @@ -16,6 +16,7 @@ CONFIG_SYS_MALLOC_BOOTPARAMS=y
> >   # CONFIG_AUTO_COMPLETE is not set
> >   CONFIG_SYS_PROMPT="-> "
> >   CONFIG_SYS_PBSIZE=276
> > +CONFIG_SYS_BOOTM_LEN=0x2000000
> >   CONFIG_CMD_IMLS=y
> >   # CONFIG_CMD_SETEXPR is not set
> >   CONFIG_CMD_MII=y
> [...]
> > diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
> > index 892d7c60d283..f0ecfd049d65 100644
> > --- a/configs/tbs2910_defconfig
> > +++ b/configs/tbs2910_defconfig
> > @@ -35,6 +35,7 @@ CONFIG_CMD_BOOTZ=y
> >   # CONFIG_BOOTM_PLAN9 is not set
> >   # CONFIG_BOOTM_RTEMS is not set
> >   # CONFIG_BOOTM_VXWORKS is not set
> > +CONFIG_SYS_BOOTM_LEN=0x1000000
> 
> For tbs2910 there is another value set here - not the old default, not
> the new one. Why?
> This looks like a carefully chosen value, but very likely it is not.
> 
> Probably also this value is fine for this board, but for me it makes no
> sense to set a board specific value here.

I don't follow you, sorry.  Since tbs2910 isn't X86 or PPC or ARM64,
it's using the current value instead.  Before this patch
include/configs/tbs2910.h includes include/configs/mx6_common.h which
sets it to 0x1000000.  Which means I probably could put a || ARCH_MX6 in
the Kconfig entry and drop out another 84 files (3 MX6 platforms set
0x4000000 instead today).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220625/b3183143/attachment.sig>