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

["Antonin Godard" <antonin.godard@bootlin.com>]

Hi Adrian,

On Thu Feb 13, 2025 at 11:04 PM CET, Adrian Freihofer via lists.yoctoproject.org wrote:
> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  documentation/ref-manual/variables.rst | 42 ++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index adbef69d8f3..e44a105c53e 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -3180,6 +3180,12 @@ 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

s/u-boot/U-boot/

> +      fitImage. Such a script can be sourced from the u-boot shell, as
> +      documented here:
> +      `U-Boot source command <https://docs.u-boot.org/en/latest/usage/cmd/source.html#fit-image.f>`__
> +

When reading this, my first question was: how do I use this variable? A path to
a file? Can you add an example below?

>     :term:`FONT_EXTRA_RDEPENDS`
>        When inheriting the :ref:`ref-classes-fontcache` class,
>        this variable specifies the runtime dependencies for font packages.
> @@ -9777,6 +9783,42 @@ 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
> +      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.
> +
> +      Machine configurations needing one of these files should include it in
> +      the :term:`SRC_URI` of the U-Boot recipe.
> +
> +      If the variable :term:`UBOOT_ENV_SUFFIX` is set to ``scr`` the script is
> +      packaged as a uImage (``mkinage -T script..``) othersise it gets

s/mkinage/mkimage/
s/othersise/otherwise/

> +      installed as it is.

Extra newline here?

> +      Some Examples:

s/Examples/examples/

> +
> +      -  Adding a script ``boot.cmd`` as an uImage to /boot::

s@/boot@``/boot``@

Above you mention that "Machine configurations needing...should include it
in the :term:`SRC_URI`..."

In this example it seems that you are setting these from the machine conf
file, at least the first two variables. But the third one should be placed in
a bbappend, most likely. Can you make it stand out?

> +
> +            UBOOT_ENV = "boot"
> +            UBOOT_ENV_SUFFIX = "scr"
> +            SRC_URI += "file://${UBOOT_ENV}.${UBOOT_ENV_SRC_SUFFIX}"
> +
> +      -  Adding a script ``uEnv.txt`` as a plain text file to /boot::
> +
> +            UBOOT_ENV = "uEnv"
> +            SRC_URI += "file://${UBOOT_ENV}.${UBOOT_ENV_SUFFIX}"

In the first example you are setting UBOOT_ENV_SUFFIX but not
UBOOT_ENV_SRC_SUFFIX. I think your examples would be better if you set all of
the variables, even if you are overriding a value with the same. Just to improve
clarity of the examples.

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

s/intalled/installed/

> +      The default is ``txt`` which means the script gets installed directly.

I would rephrase to "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 it is referred by the :term:`SRC_URI` of the

s/as it is referred by the/as is it specified in 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

Thank you,
Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com