[U-Boot] [U-Boot, v2] Enable expression support for CONFIG_BOARD_SIZE_LIMIT

[Simon Goldschmidt]

Tom,

On Fri, Dec 14, 2018 at 8:16 PM Tom Rini <trini at konsulko.com> wrote:
>
> On Fri, Dec 07, 2018 at 08:27:51PM +0100, Wolfgang Denk wrote:
>
> > So far, the use of CONFIG_BOARD_SIZE_LIMIT would only work with
> > plain numeric constants.  Extend it to allow for expressions, so one
> > can for example use
> >
> >       #define CONFIG_BOARD_SIZE_LIMIT (768 << 10)
> >
> > in the board configuration.
> >
> > Signed-off-by: Wolfgang Denk <wd at denx.de>
> > Tested-by: Fabio Estevam <festevam at gmail.com>
> >
> > Cc: Fabio Estevam <festevam at gmail.com>
> > Cc: Stefano Babic <sbabic at denx.de>
> > Cc: Vanessa Maegima <vanessa.maegima at nxp.com>
> > Cc: Otavio Salvador <otavio at ossystems.com.br>
> > Cc: John Weber <john.weber at technexion.com>
> > Cc: Stefan Roese <sr at denx.de>
> > ---
> > v2: replace bashism for evaluating expressions in CONFIG_BOARD_SIZE_LIMIT
> >     by another call to awk.
> >
> > Note 1: As gawk lacks an eval function and we don't want to rely on
> >  bash being used as shell, we use another call to awk to evaluate the
> >  expression. This has the disadvantage that we cannot use expressions
> >  like "<<" which awk does not understand. OK, one could replace awk
> >  by something better...
> >
> > Note 2: This patch focusses on enabling this new feature.  It does
> >  not addres another issue that should be solved in a later
> >  commit: the duplication of the same code in Makefile and
> >  arch/arm/mach-imx/Makefile
> >
> > ---
> >  Makefile                   | 17 ++++++++---------
> >  arch/arm/mach-imx/Makefile | 17 ++++++++---------
> >  2 files changed, 16 insertions(+), 18 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 0d11ff9797..87eb0fd2b1 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -774,15 +774,14 @@ LDPPFLAGS += \
> >
> >  ifneq ($(CONFIG_BOARD_SIZE_LIMIT),)
> >  BOARD_SIZE_CHECK = \
> > -     @actual=`wc -c $@ | awk '{print $$1}'`; \
> > -     limit=`printf "%d" $(CONFIG_BOARD_SIZE_LIMIT)`; \
> > -     if test $$actual -gt $$limit; then \
> > -             echo "$@ exceeds file size limit:" >&2 ; \
> > -             echo "  limit:  $$limit bytes" >&2 ; \
> > -             echo "  actual: $$actual bytes" >&2 ; \
> > -             echo "  excess: $$((actual - limit)) bytes" >&2; \
> > -             exit 1; \
> > -     fi
> > +     @(awk "END{print $$(echo $(CONFIG_BOARD_SIZE_LIMIT))}" /dev/null; wc -c $@ ) | \
>
> So this fails with awk being provided by mawk, not gawk:
> $ mawk "END{print $(echo 0xE0000)}" /dev/null;
> 0
> $ gawk "END{print $(echo 0xE0000)}" /dev/null;
> 917504
>
> And then things fail as mawk doesn't like hex.
>
> Now, at the end of the day, I'm now not sure I agree with this patch in
> concept.  When CONFIG_BOARD_SIZE_LIMIT is moved to Kconfig it will be
> taking I would assume a hex input and so we don't have to worry about <<
> at all.

Sorry to warm up an old thread, but I have some slightly new input on this.

I can understand you disliking the concept of this patch regarding U-Boot
proper size check (if CONFIG_BOARD_SIZE_LIMIT moves to Kconfig).

However, I think for SPL this is different: SPL often starts with one single
small SRAM shared for code + data. In this case, the size available for the
SPL binary often can be calculated like this:

CONFIG_SPL_MAX_SIZE = SRAM_SIZE - SYS_MALLOC_F_LEN -
GENERATED_GBL_DATA_SIZE - STACKSIZE;

Being like that, it cannot just be moved to Kconfig and by definition is not
a hardcoded single hex number but changes depending on other options.

I see two ways out here:
a) continue the way of this patch until it works for all shells/awks or
b) implement SPL binary size check using 4 constants instead of 1 (see above)

I'm willing to code this through, as I am hitting this limit on socfpga, so I
could need a decision ;-)

Regards,
Simon