[v3] ref-manual: cover UBOOT_ENV variables

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

Hi Adrian,

On 2/24/25 11:16 PM, Adrian Freihofer via lists.yoctoproject.org wrote:
> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>   documentation/ref-manual/variables.rst | 62 ++++++++++++++++++++++++++
>   1 file changed, 62 insertions(+)
> 
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index adbef69d8f3..b432488a012 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`

FIT_UBOOT_ENV isn't specific to linux-yocto recipes, any recipe 
inheriting kernel-fitimage should be able to handle that no?

You can see just below in FONT_EXTRA_RDEPENDS that we specify

When inheriting the :ref:`ref-classes-fontcache` class,

So maybe we can something similar here?

> +      of the Linux kernel recipe.
> +
> +      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 TFTP server.::
> +

Remove the trailing dot.

> +            tftp $loadaddr $fit_url
> +            source $loadaddr#bootscr-boot.cmd
> +

I think it'd be nice to explain that the FIT image node for the boot 
script will be FIT_UBOOT_ENV prefixed with bootscr-.

fit_url is a bit odd since I don't think tftp supports URLs? c.f. 
https://en.wikipedia.org/wiki/URL#Syntax

I think we should use something like "$fit_tftp_path" or something like 
that? IIRC, it's relative to the root directory of the TFTP server?

> +      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,44 @@ 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`

It seems that most of this is specific to u-boot.inc, which we usually 
recommend NOT including from third-party recipes. Maybe say it is 
specific to the U-Boot recipe in OE-Core? @Antonin, an opinion on that?

> +      This variable allows to add additional environment variables or a script
> +      to be installed together with U-Boot.
> +      This file, typically ``uEnv.txt`` or ``boot.cmd``, is installed in
> +      ``/boot`` as well as copied to the deploy directory.

:term:`DEPLOYDIR` (where the recipe copies the file) or maybe 
:term:`DEPLOY_DIR_IMAGE` (where the user should get the file from)

> +
> +      For machine configurations needing one of these files a ``.bbappend``
> +      file 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 (``mkimage -T script..``) otherwise it gets
> +      installed as it is.
> +

Can suggest

s/as it is/verbatim/

not strong opinion though.

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

s/an/a/

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

Isn't that UBOOT_ENV_SRC directly? I think it'd make sense to use 
variables made specifically for that instead of replacing it with its 
content. Best practice and all :)

> +      -  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}"

Isn't that UBOOT_ENV_BINARY directly? I think it'd make sense to use 
variables made specifically for that instead of replacing it with its 
content. Best practice and all :)

> +
> +   :term:`UBOOT_ENV_SUFFIX`
> +      If this variable is set to ``scr`` the script referred by

s/referred by/referred to 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

s/is it/it is/

> +      U-Boot recipe.
> +

This is not alphabetically sorted, UBOOT_ENV needs to be after 
UBOOT_ENTRYPOINT, then UBOOT_ENV_SRC_SUFFIX and after that, 
UBOOT_ENV_SUFFIX.

Cheers,
Quentin

Hi Quentin

Thank you for the re-view. I fixed all the findings with v4.

Just a comment about that section which is also changed acc. your
suggestion now.

> > +
> > +            UBOOT_ENV = "boot"
> > +            UBOOT_ENV_SUFFIX = "scr"
> > +            SRC_URI +=
> > "file://${UBOOT_ENV}.${UBOOT_ENV_SRC_SUFFIX}"
> > +
> 
> Isn't that UBOOT_ENV_SRC directly? I think it'd make sense to use 
> variables made specifically for that instead of replacing it with its
> content. Best practice and all :)

Yes, it is. But not sure if referring to more variables is helpful from
a user's perspective. Technically it does not matter how the SRC_URI
gets defined.

> 
> > +      -  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}"
> 
> Isn't that UBOOT_ENV_BINARY directly? I think it'd make sense to use 
> variables made specifically for that instead of replacing it with its
> content. Best practice and all :)

Yes, it is. Here it's even more confusing, because having a variable
named ..BINARY referring to a txt file looks odd to me.

But lets follow the best practice, it's fine for me.

Adrian