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

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

[Adrian Freihofer]

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.
+
+      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.::
+
+            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
+      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
+      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:
+
+      -  Adding a script ``boot.cmd`` as an uImage to ``/boot``::
+
+            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"
+            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
-- 
2.47.1

[PATCH v2 2/2] migration-guides: cover FIT_UBOOT_ENV

[Adrian Freihofer]

Add a hint for users using the UBOOT_ENV variable and the
kernel-fitimage.bbclass.

Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
---
 documentation/migration-guides/migration-5.2.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/documentation/migration-guides/migration-5.2.rst b/documentation/migration-guides/migration-5.2.rst
index de2ea773e55..4513186b7de 100644
--- a/documentation/migration-guides/migration-5.2.rst
+++ b/documentation/migration-guides/migration-5.2.rst
@@ -182,3 +182,11 @@ Miscellaneous changes
 
 -  :term:`ZSTD_COMPRESSION_LEVEL` is now a plain integer number instead of a dash-prefixed
    command-line option (e.g. it should be set to ``3`` rather than ``-3``).
+
+-  Until now, the variable :term:`UBOOT_ENV` was processed both by the U-Boot
+   recipe and by the ``kernel-fitimage.bbclass``. However, adding a U-Boot
+   script to the kernel FIT image is a different and independent thing, which
+   also requires an independent variable.
+   Therefore, the :term:`UBOOT_ENV` is no longer handled by the
+   ``kernel-fitimage.bbclass``. There is a new variable :term:`FIT_UBOOT_ENV`
+   which should be used for adding a U-Boot script to a FIT image.
-- 
2.47.1

Re: [docs] [PATCH v2 2/2] migration-guides: cover FIT_UBOOT_ENV

[Antonin Godard]

On Mon Feb 24, 2025 at 9:27 AM CET, Adrian Freihofer via lists.yoctoproject.org wrote:
> Add a hint for users using the UBOOT_ENV variable and the
> kernel-fitimage.bbclass.
>
> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  documentation/migration-guides/migration-5.2.rst | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/documentation/migration-guides/migration-5.2.rst b/documentation/migration-guides/migration-5.2.rst
> index de2ea773e55..4513186b7de 100644
> --- a/documentation/migration-guides/migration-5.2.rst
> +++ b/documentation/migration-guides/migration-5.2.rst
> @@ -182,3 +182,11 @@ Miscellaneous changes
>  
>  -  :term:`ZSTD_COMPRESSION_LEVEL` is now a plain integer number instead of a dash-prefixed
>     command-line option (e.g. it should be set to ``3`` rather than ``-3``).
> +
> +-  Until now, the variable :term:`UBOOT_ENV` was processed both by the U-Boot
> +   recipe and by the ``kernel-fitimage.bbclass``. However, adding a U-Boot
> +   script to the kernel FIT image is a different and independent thing, which
> +   also requires an independent variable.
> +   Therefore, the :term:`UBOOT_ENV` is no longer handled by the
> +   ``kernel-fitimage.bbclass``. There is a new variable :term:`FIT_UBOOT_ENV`
> +   which should be used for adding a U-Boot script to a FIT image.

Reviewed-by: Antonin Godard <antonin.godard@bootlin.com>

Thank you!
Antonin

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

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

[Antonin Godard]

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

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

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

[Adrian Freihofer]

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

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

[Antonin Godard]

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

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