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 |
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] > -=-=-=-=-=-=-=-=-=-=-=- >
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
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 >
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 --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 }