diff mbox series

u-boot: Convert ${UBOOT_ENV}.cmd into ${UBOOT_ENV}.env

Message ID 20250128145736.2258098-1-thomas.perrot@bootlin.com
State New
Headers show
Series u-boot: Convert ${UBOOT_ENV}.cmd into ${UBOOT_ENV}.env | expand

Commit Message

Thomas Perrot Jan. 28, 2025, 2:57 p.m. UTC
From: Thomas Perrot <thomas.perrot@bootlin.com>

It is U-Boot environment files that contain environment variables
in a binary format, that enables compact data storage and allows
for faster loading and processing by U-Boot.

Signed-off-by: Thomas Perrot <thomas.perrot@bootlin.com>
---
 meta/classes-recipe/uboot-config.bbclass |  8 +++++++-
 meta/recipes-bsp/u-boot/u-boot.inc       | 11 +++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

Comments

Adrian Freihofer Jan. 29, 2025, 10:11 p.m. UTC | #1
Hi Thomas

On Tue, 2025-01-28 at 15:57 +0100, Thomas Perrot via
lists.openembedded.org wrote:
> From: Thomas Perrot <thomas.perrot@bootlin.com>
> 
> It is U-Boot environment files that contain environment variables
> in a binary format, that enables compact data storage and allows
> for faster loading and processing by U-Boot.

I'm honestly not able to understand the intent of this change from this
commit message (I'm not a native speaker). But the u-boot and fitImage
related code is already quite complicated because there are so many use
cases that might be supported somehow. So it would be very helpful if
changes were submitted with a clear description of the use case.

> 
> Signed-off-by: Thomas Perrot <thomas.perrot@bootlin.com>
> ---
>  meta/classes-recipe/uboot-config.bbclass |  8 +++++++-
>  meta/recipes-bsp/u-boot/u-boot.inc       | 11 +++++++++--
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/classes-recipe/uboot-config.bbclass b/meta/classes-
> recipe/uboot-config.bbclass
> index bf21961977cc..50e16efe50aa 100644
> --- a/meta/classes-recipe/uboot-config.bbclass
> +++ b/meta/classes-recipe/uboot-config.bbclass
> @@ -75,6 +75,12 @@ UBOOT_EXTLINUX_SYMLINK ?=
> "${UBOOT_EXTLINUX_CONF_NAME}-${MACHINE}-${PR}"
>  UBOOT_MKIMAGE_DTCOPTS ??= ""
>  SPL_MKIMAGE_DTCOPTS ??= ""
>  
> +# Options for the binary format, such as enabling a redundant
> environment.
> +UBOOT_MKENVIMAGE_BIN_ARGS ??= ""
> +
> +#mkenvimage command
> +UBOOT_MKENVIMAGE ?= "uboot-mkenvimage"
> +
>  # mkimage command
>  UBOOT_MKIMAGE ?= "uboot-mkimage"
>  UBOOT_MKIMAGE_SIGN ?= "${UBOOT_MKIMAGE}"
> @@ -119,7 +125,7 @@ python () {
>          for config in ubootconfig:
>              found = False
>              for f, v in ubootconfigflags.items():
> -                if config == f: 
> +                if config == f:
>                      found = True
>                      items = v.split(',')
>                      if items[0] and len(items) > 3:
> diff --git a/meta/recipes-bsp/u-boot/u-boot.inc b/meta/recipes-bsp/u-
> boot/u-boot.inc
> index 3270c22e8d42..f1308d65609f 100644
> --- a/meta/recipes-bsp/u-boot/u-boot.inc
> +++ b/meta/recipes-bsp/u-boot/u-boot.inc
> @@ -4,6 +4,7 @@ PROVIDES = "virtual/bootloader"
>  PACKAGE_ARCH = "${MACHINE_ARCH}"
>  
>  DEPENDS += "${@bb.utils.contains('UBOOT_ENV_SUFFIX', 'scr', 'u-boot-
> mkimage-native', '', d)}"
> +DEPENDS += "${@bb.utils.contains('UBOOT_ENV_SUFFIX', 'env', 'u-boot-
> mkenvimage-native', '', d)}"
>  
>  inherit uboot-config uboot-extlinux-config uboot-sign deploy
> python3native kernel-arch
>  
> @@ -68,9 +69,15 @@ do_compile () {
>          uboot_compile
>      fi
>  
> -    if [ -n "${UBOOT_ENV}" ] && [ "${UBOOT_ENV_SUFFIX}" = "scr" ]
> +    if [ -n "${UBOOT_ENV}" ]
>      then
> -        ${UBOOT_MKIMAGE} -C none -A ${UBOOT_ARCH} -T script -d
> ${UNPACKDIR}/${UBOOT_ENV_SRC} ${B}/${UBOOT_ENV_BINARY}
> +        if [ "${UBOOT_ENV_SUFFIX}" = "scr" ]
> +        then
> +            ${UBOOT_MKIMAGE} -C none -A ${UBOOT_ARCH} -T script -d
> ${UNPACKDIR}/${UBOOT_ENV_SRC} ${B}/${UBOOT_ENV_BINARY}
> +        elif [ "${UBOOT_ENV_SUFFIX}" = "env" ]
> +        then
> +            ${UBOOT_MKENVIMAGE} ${UBOOT_MKENVIMAGE_BIN_ARGS} -s
> ${UBOOT_ENV_SIZE} ${UNPACKDIR}/${UBOOT_ENV_SRC} -o
> ${B}/${UBOOT_ENV_BINARY}
> +        fi
>      fi

Later on in do_install there is:
  install -m 644 ${B}/${UBOOT_ENV_BINARY} ${D}/boot/${UBOOT_ENV_IMAGE}

Does it make sense to install this file into /boot? Or is this file
type suitable for do_deploy only?


In kernel-fitimage.bbclass there is:

  # Step 3: Prepare a u-boot script section
  if [ -n "${UBOOT_ENV}" ] && [ -d "${STAGING_DIR_HOST}/boot" ]; then
    if [ -e "${STAGING_DIR_HOST}/boot/${UBOOT_ENV_BINARY}" ]; then
      cp ${STAGING_DIR_HOST}/boot/${UBOOT_ENV_BINARY} ${B}
      bootscr_id="${UBOOT_ENV_BINARY}"
      fitimage_emit_section_boot_script $1 "$bootscr_id" 
        ${UBOOT_ENV_BINARY}
    else
     bbwarn "${STAGING_DIR_HOST}/boot/${UBOOT_ENV_BINARY} not found."
    fi
  fi

If I remember correctly, ${UBOOT_ENV_BINARY} gets staged and probably
also included into the fitImage. Does that make sense for this file
type?

So it would be helpful to get a bit of context from the commit message.

Thank you,
Adrian

>  }
>  
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#210326):
> https://lists.openembedded.org/g/openembedded-core/message/210326
> Mute This Topic: https://lists.openembedded.org/mt/110859777/4454582
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe:
> https://lists.openembedded.org/g/openembedded-core/unsub [
> adrian.freihofer@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Richard Purdie Jan. 29, 2025, 10:39 p.m. UTC | #2
On Wed, 2025-01-29 at 23:11 +0100, Adrian Freihofer via lists.openembedded.org wrote:
> Hi Thomas
> 
> On Tue, 2025-01-28 at 15:57 +0100, Thomas Perrot via
> lists.openembedded.org wrote:
> > From: Thomas Perrot <thomas.perrot@bootlin.com>
> > 
> > It is U-Boot environment files that contain environment variables
> > in a binary format, that enables compact data storage and allows
> > for faster loading and processing by U-Boot.
> 
> I'm honestly not able to understand the intent of this change from this
> commit message (I'm not a native speaker). But the u-boot and fitImage
> related code is already quite complicated because there are so many use
> cases that might be supported somehow. So it would be very helpful if
> changes were submitted with a clear description of the use case.

I'd add that for new branches in the code like this we really need new
test cases too.

Cheers,

Richard
Adrian Freihofer Jan. 29, 2025, 10:48 p.m. UTC | #3
Richard Purdie <richard.purdie@linuxfoundation.org> schrieb am Mi., 29.
Jan. 2025, 23:39:

> On Wed, 2025-01-29 at 23:11 +0100, Adrian Freihofer via
> lists.openembedded.org wrote:
> > Hi Thomas
> >
> > On Tue, 2025-01-28 at 15:57 +0100, Thomas Perrot via
> > lists.openembedded.org wrote:
> > > From: Thomas Perrot <thomas.perrot@bootlin.com>
> > >
> > > It is U-Boot environment files that contain environment variables
> > > in a binary format, that enables compact data storage and allows
> > > for faster loading and processing by U-Boot.
> >
> > I'm honestly not able to understand the intent of this change from this
> > commit message (I'm not a native speaker). But the u-boot and fitImage
> > related code is already quite complicated because there are so many use
> > cases that might be supported somehow. So it would be very helpful if
> > changes were submitted with a clear description of the use case.
>
> I'd add that for new branches in the code like this we really need new
> test cases too.
>
> Here is a branch doing this for the already supported scr script type:
> https://git.yoctoproject.org/poky-contrib/log/?h=adrianf/fitimage-test-uboot-env
>
> Adrian
>


> Cheers,
>
> Richard
>
Thomas Perrot Feb. 4, 2025, 12:59 p.m. UTC | #4
Hello,

On Wed, 2025-01-29 at 23:48 +0100, Adrian Freihofer via
lists.openembedded.org wrote:
> 
> 
> Richard Purdie <richard.purdie@linuxfoundation.org> schrieb am Mi.,
> 29. Jan. 2025, 23:39:
> > On Wed, 2025-01-29 at 23:11 +0100, Adrian Freihofer via
> > lists.openembedded.org wrote:
> > > Hi Thomas
> > > 
> > > On Tue, 2025-01-28 at 15:57 +0100, Thomas Perrot via
> > > lists.openembedded.org wrote:
> > > > From: Thomas Perrot <thomas.perrot@bootlin.com>
> > > > 
> > > > It is U-Boot environment files that contain environment
> > > > variables
> > > > in a binary format, that enables compact data storage and
> > > > allows
> > > > for faster loading and processing by U-Boot.
> > > 
> > > I'm honestly not able to understand the intent of this change
> > > from this
> > > commit message (I'm not a native speaker). But the u-boot and
> > > fitImage
> > > related code is already quite complicated because there are so
> > > many use
> > > cases that might be supported somehow. So it would be very
> > > helpful if
> > > changes were submitted with a clear description of the use case.
> > 
> > I'd add that for new branches in the code like this we really need
> > new
> > test cases too.
> > 
> > Here is a branch doing this for the already supported scr script
> > type:
> > https://git.yoctoproject.org/poky-contrib/log/?h=adrianf/fitimage-test-uboot-env
> > 

Can I help you with anything?

Kind regards,
Thomas

> > Adrian
> 
> > 
> > Cheers,
> > 
> > Richard
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#210420):
> https://lists.openembedded.org/g/openembedded-core/message/210420
> Mute This Topic: https://lists.openembedded.org/mt/110859777/5443093
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe:
> https://lists.openembedded.org/g/openembedded-core/unsub [
> thomas.perrot@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/classes-recipe/uboot-config.bbclass b/meta/classes-recipe/uboot-config.bbclass
index bf21961977cc..50e16efe50aa 100644
--- a/meta/classes-recipe/uboot-config.bbclass
+++ b/meta/classes-recipe/uboot-config.bbclass
@@ -75,6 +75,12 @@  UBOOT_EXTLINUX_SYMLINK ?= "${UBOOT_EXTLINUX_CONF_NAME}-${MACHINE}-${PR}"
 UBOOT_MKIMAGE_DTCOPTS ??= ""
 SPL_MKIMAGE_DTCOPTS ??= ""
 
+# Options for the binary format, such as enabling a redundant environment.
+UBOOT_MKENVIMAGE_BIN_ARGS ??= ""
+
+#mkenvimage command
+UBOOT_MKENVIMAGE ?= "uboot-mkenvimage"
+
 # mkimage command
 UBOOT_MKIMAGE ?= "uboot-mkimage"
 UBOOT_MKIMAGE_SIGN ?= "${UBOOT_MKIMAGE}"
@@ -119,7 +125,7 @@  python () {
         for config in ubootconfig:
             found = False
             for f, v in ubootconfigflags.items():
-                if config == f: 
+                if config == f:
                     found = True
                     items = v.split(',')
                     if items[0] and len(items) > 3:
diff --git a/meta/recipes-bsp/u-boot/u-boot.inc b/meta/recipes-bsp/u-boot/u-boot.inc
index 3270c22e8d42..f1308d65609f 100644
--- a/meta/recipes-bsp/u-boot/u-boot.inc
+++ b/meta/recipes-bsp/u-boot/u-boot.inc
@@ -4,6 +4,7 @@  PROVIDES = "virtual/bootloader"
 PACKAGE_ARCH = "${MACHINE_ARCH}"
 
 DEPENDS += "${@bb.utils.contains('UBOOT_ENV_SUFFIX', 'scr', 'u-boot-mkimage-native', '', d)}"
+DEPENDS += "${@bb.utils.contains('UBOOT_ENV_SUFFIX', 'env', 'u-boot-mkenvimage-native', '', d)}"
 
 inherit uboot-config uboot-extlinux-config uboot-sign deploy python3native kernel-arch
 
@@ -68,9 +69,15 @@  do_compile () {
         uboot_compile
     fi
 
-    if [ -n "${UBOOT_ENV}" ] && [ "${UBOOT_ENV_SUFFIX}" = "scr" ]
+    if [ -n "${UBOOT_ENV}" ]
     then
-        ${UBOOT_MKIMAGE} -C none -A ${UBOOT_ARCH} -T script -d ${UNPACKDIR}/${UBOOT_ENV_SRC} ${B}/${UBOOT_ENV_BINARY}
+        if [ "${UBOOT_ENV_SUFFIX}" = "scr" ]
+        then
+            ${UBOOT_MKIMAGE} -C none -A ${UBOOT_ARCH} -T script -d ${UNPACKDIR}/${UBOOT_ENV_SRC} ${B}/${UBOOT_ENV_BINARY}
+        elif [ "${UBOOT_ENV_SUFFIX}" = "env" ]
+        then
+            ${UBOOT_MKENVIMAGE} ${UBOOT_MKENVIMAGE_BIN_ARGS} -s ${UBOOT_ENV_SIZE} ${UNPACKDIR}/${UBOOT_ENV_SRC} -o ${B}/${UBOOT_ENV_BINARY}
+        fi
     fi
 }