Re: [docs] [PATCH v2 1/2] ref-manual: cover UBOOT_ENV variables

[Adrian Freihofer <adrian.freihofer@gmail.com>]

Hi Antonin

I will just send a v3 which fixes all your findings, except this one
needs clarification:

snip
> > +      -  Adding a script ``boot.cmd`` as an uImage to ``/boot``::
> > +
> > +            UBOOT_ENV = "boot"
> > +            UBOOT_ENV_SUFFIX = "scr"
> 
> Or shouldn't you set UBOOT_ENV_SRC_SUFFIX to "cmd" here too? I'm a
> little
> confused as why you refer to boot.cmd yet "cmd" is not defined here.

UBOOT_ENV_SRC_SUFFIX ?= "cmd" is the default.

I have spent many hours trying to understand the code that handles the
UBOOT_ENV* variables. It's super complicated. In uboot.inc, there is:

do_compile () {
if [ -n "${UBOOT_ENV}" ] && [ "${UBOOT_ENV_SUFFIX}" = "scr" ]; then
${UBOOT_MKIMAGE} -C none -A ${UBOOT_ARCH} -T script \
-d ${UNPACKDIR}/${UBOOT_ENV_SRC} ${B}/${UBOOT_ENV_BINARY}
fi
}

do_install () {
if [ -n "${UBOOT_ENV}" ]; then
install -m 644 ${B}/${UBOOT_ENV_BINARY} \
${D}/boot/${UBOOT_ENV_IMAGE}
fi
}

Depending on the file ending respectively the UBOOT_ENV_SUFFIX
variable, something different happens: Either
 * a script which can be loaded by U-Boot's source command
   or
 * an image including a script which can be handled by U-Boot's load
   command
   ends up in /boot.

In u-boot-config.bbclass there are more variables defined:
 * Documented variables:
   UBOOT_ENV_SUFFIX ?= "txt"
   UBOOT_ENV ?= ""
   UBOOT_ENV_SRC_SUFFIX ?= "cmd"
 * Variables which are considered as internal and hence not documented
   on purpose:
   UBOOT_ENV_SRC ?= "${UBOOT_ENV}.${UBOOT_ENV_SRC_SUFFIX}"
   UBOOT_ENV_BINARY ?= "${UBOOT_ENV}.${UBOOT_ENV_SUFFIX}"
   ...two more...

So my patch tries to keep it as simple as possible but still cover the
2 relevant use cases. To make it more clear, I added:
"It defaults to ``cmd``."
to the UBOOT_ENV_SRC_SUFFIX section.

I hope this makes it more understandable without a need for dropping
all these details on the users.


Note: It was even much more complicated when the kernel-
fitimage.bbclass was in use as well. The kernel-fitimage.bbclass is now
decoupled which simplifies (or maybe I should say fixes) it quite a
bit.

> 
snip

Thank you for the review!
Adrian