[v2,1/2] ref-manual: cover UBOOT_ENV variables

Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
---
 documentation/ref-manual/variables.rst | 64 ++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

Hi Adrian,

On Mon Feb 24, 2025 at 9:27 AM CET, Adrian Freihofer via lists.yoctoproject.org wrote:
> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  documentation/ref-manual/variables.rst | 64 ++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index adbef69d8f3..82dd80ca130 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -3180,6 +3180,30 @@ system and gives an overview of their function and contents.
>        The default value for this variable is set to "2048"
>        by the :ref:`ref-classes-kernel-fitimage` class.
>  
> +   :term:`FIT_UBOOT_ENV`
> +      This variable allows to add a U-Boot script as a text file to the
> +      FIT image. Such a script can be sourced from the U-Boot shell.
> +
> +      For machine configurations needing such a script a
> +      ``linux-yocto_%.bbappend`` file should include it in the :term:`SRC_URI`
> +      of the kernel recipe.

s/kernel/Linux kernel/

> +
> +      Example:
> +
> +      -  Add a script ``boot.cmd`` to the Linux kernel recipe::
> +
> +            FIT_UBOOT_ENV = "boot.cmd"
> +            SRC_URI += "file://${FIT_UBOOT_ENV}"
> +
> +      -  Use the script file from the U-Boot shell. This example loads the FIT
> +         image from a TFP server.::

s/TFP/TFTP/

> +
> +            tftp $loadaddr $fit_url
> +            source $loadaddr#bootscr-boot.cmd
> +
> +      More information can be found in the official U-Boot documentation:
> +      `U-Boot source command <https://docs.u-boot.org/en/latest/usage/cmd/source.html#fit-image.f>`__
> +
>     :term:`FONT_EXTRA_RDEPENDS`
>        When inheriting the :ref:`ref-classes-fontcache` class,
>        this variable specifies the runtime dependencies for font packages.
> @@ -9777,6 +9801,46 @@ system and gives an overview of their function and contents.
>        :ref:`ref-classes-kernel-fitimage` class to specify the load address to be
>        used in creating the dtbo sections of Image Tree Source for the FIT image.
>  
> +   :term:`UBOOT_ENV`
> +      Additional environment variables or a script can be installed alongside

Can you start the paragraph with "This variable allows..." like the other? So
the next sentence doesn't feel out of context, and the user instantly knows what
the variable is used for.

> +      U-Boot to be used automatically on boot.
> +      This file, typically ``uEnv.txt`` or ``boot.cmd``, gets packaged along
> +      with U-Boot (installed into ``/boot``) as well as placed in the deploy
> +      directory.
> +
> +      For machine configurations needing one of these files a
> +      ``u-boot_%.bbappend`` file should include it in the :term:`SRC_URI` of

s/``u-boot_%.bbappend``/``.bbappend``/ (not limited to u-boot.bb only I assume
but any virtual/bootloader representing U-boot)

> +      the U-Boot recipe.
> +
> +      If the variable :term:`UBOOT_ENV_SUFFIX` is set to ``scr`` the script is
> +      packaged as a uImage (``mkimage -T script..``) otherwise it gets
> +      installed as it is.
> +
> +      Some Examples:

s/Examples/examples/

> +
> +      -  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.

> +            SRC_URI += "file://${UBOOT_ENV}.${UBOOT_ENV_SRC_SUFFIX}"
> +
> +      -  Adding a script ``uEnv.txt`` as a plain text file to ``/boot``::
> +
> +            UBOOT_ENV = "uEnv"
> +            UBOOT_ENV_SUFFIX = "txt"
> +            SRC_URI += "file://${UBOOT_ENV}.${UBOOT_ENV_SUFFIX}"
> +
> +   :term:`UBOOT_ENV_SUFFIX`
> +      If this variable is set to ``scr`` the script referred by
> +      :term:`UBOOT_ENV` gets packaged as a uImage before it gets installed.
> +      The default is ``txt`` which means the script is installed as-is, with
> +      no modification.
> +
> +   :term:`UBOOT_ENV_SRC_SUFFIX`
> +      If :term:`UBOOT_ENV_SUFFIX` is set to ``scr`` this is the suffix of the
> +      plain text script file as is it specified in the :term:`SRC_URI` of the
> +      U-Boot recipe.
> +
>     :term:`UBOOT_ENTRYPOINT`
>        Specifies the entry point for the U-Boot image. During U-Boot image
>        creation, the :term:`UBOOT_ENTRYPOINT` variable is passed as a

Thanks!
Antonin

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

Hi Adrian,

On Mon Feb 24, 2025 at 11:14 PM CET, Adrian Freihofer wrote:
> 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.

Okay so if I recapitulate, what is happening in your example is:

UBOOT_ENV = "boot"
UBOOT_ENV_SUFFIX = "scr"
UBOOT_ENV_SRC_SUFFIX = "cmd"
SRC_URI += "file://${UBOOT_ENV}.${UBOOT_ENV_SRC_SUFFIX}"

Which makes sense.

I wasn't sure of the default value for UBOOT_ENV_SRC_SUFFIX, which is why I
suggested setting all the variables in the example to avoid confusion.

> 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.

Thanks, this will make it clearer too.

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

I see your point, yes this code is far from intuitive.

> 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

Thanks for the efforts!

Antonin