[U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB

[Benoît Thébaudeau]

Hi Heinrich,

On Wed, Apr 3, 2019 at 8:20 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 4/3/19 2:59 AM, Kever Yang wrote:
> > Hi Heinrich,
> >
> >
> > On 04/03/2019 01:19 AM, Heinrich Schuchardt wrote:
> >> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> >> up to 2 KiB for the file header.
> >
> > 32KB is the limit of SPL size, not SPL image size, does not include 2KB
> > header.
> >>
> >> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> >> the board specific limit.
> >
> > There is already CONFIG_SPL_MAX_SIZE for the SPL size limit, isn't it?
> > I don't understand  why we need new variable.
>
> SPL_MAX_SIZE is used to set CONFIG_SPL_PAD_TO if CONFIG_SPL_PAD_TO is
> not defined. If CONFIG_SPL_PAD_TO is defined it specifies the maximum
> value of CONFIG_SPL_PAD_TO.

If CONFIG_SPL_PAD_TO is defined and non-zero, CONFIG_SPL_MAX_SIZE
specifies the _minimum_ value of CONFIG_SPL_PAD_TO.

> So in case CONFIG_SPL_PAD_TO is not defined
> SPL_MAX_SIZE does not specify a maximum size but a minimum one.

CONFIG_SPL_MAX_SIZE specifies a minimum value for CONFIG_SPL_PAD_TO,
but a maximum size for the SPL.

> It is unclear to me why Benoît in 6113d3f27ca ("Makefile: Change
> CONFIG_SPL_PAD_TO to image offset") chose this design. Entangling the
> variables in this way does not provide a clean design.

Everything is explained in this thread:
https://lists.denx.de/pipermail/u-boot/2013-February/146851.html

These two variables coexisted before this change. They have different
purposes, but they are related, so they cannot take any values
independently of each other.

The purpose of CONFIG_SPL_MAX_SIZE is not to define default or minimum
values for CONFIG_SPL_PAD_TO, but only what its name says, i.e. to
define the maximum SPL size.

After the SPL, there may be a gap, then a payload appended to the
image. The purpose of CONFIG_SPL_PAD_TO is to define the image offset
of the payload, if any. CONFIG_SPL_PAD_TO can be 0 if the payload can
always be appended to the SPL without any gap.

Therefore, the SPL must fit inside CONFIG_SPL_MAX_SIZE, and
CONFIG_SPL_MAX_SIZE must fit inside CONFIG_SPL_PAD_TO if the latter is
non-zero.

[...]

Best regards,
Benoît

On Wed, Apr 3, 2019 at 8:20 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 4/3/19 2:59 AM, Kever Yang wrote:
> > Hi Heinrich,
> >
> >
> > On 04/03/2019 01:19 AM, Heinrich Schuchardt wrote:
> >> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> >> up to 2 KiB for the file header.
> >
> > 32KB is the limit of SPL size, not SPL image size, does not include 2KB
> > header.
> >>
> >> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> >> the board specific limit.
> >
> > There is already CONFIG_SPL_MAX_SIZE for the SPL size limit, isn't it?
> > I don't understand  why we need new variable.
>
> SPL_MAX_SIZE is used to set CONFIG_SPL_PAD_TO if CONFIG_SPL_PAD_TO is
> not defined. If CONFIG_SPL_PAD_TO is defined it specifies the maximum
> value of CONFIG_SPL_PAD_TO. So in case CONFIG_SPL_PAD_TO is not defined
> SPL_MAX_SIZE does not specify a maximum size but a minimum one.
>
> It is unclear to me why Benoît in 6113d3f27ca ("Makefile: Change
> CONFIG_SPL_PAD_TO to image offset") chose this design. Entangling the
> variables in this way does not provide a clean design.
>
> The check
>
> #if defined(IMAGE_MAX_SIZE)
> ASSERT(__image_copy_end - __image_copy_start < (IMAGE_MAX_SIZE), \
>         "SPL image too big");
> #endif
>
> in arch/arm/cpu/u-boot-spl.lds neither considers the size of the device
> tree nor of the BSS section. So at least for Rockchip SOCs it is
> checking a measure that is not directly related to the image size
> created by `mkimage -T rksd`.
>
> Best regards
>
> Heinrich
>
> >
> > Thanks,
> > - Kever
> >> A common Makefile function is used for this test and the test against
> >> CONFIG_BOARD_SIZE_LIMIT.
> >>
> >> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
> >>
> >> v4:
> >>      use a common function for all size checks in the Makefiles
> >>
> >> Heinrich Schuchardt (4):
> >>   Makefile: reusable function for BOARD_SIZE_CHECK
> >>   imx: move BOARD_SIZE_CHECK to main Makefile
> >>   configs: define CONFIG_SPL_SIZE_LIMIT
> >>   configs: rk3288: Tinker Board SPL file must fit into 32 KiB
> >>
> >>  Kconfig                         |  8 ++++++++
> >>  Makefile                        | 33 +++++++++++++++++++++++----------
> >>  arch/arm/mach-imx/Makefile      | 16 ----------------
> >>  configs/tinker-rk3288_defconfig |  1 +
> >>  4 files changed, 32 insertions(+), 26 deletions(-)
> >>
> >> --
> >> 2.20.1
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot