* [meta-rockchip][PATCH v4] enable stored U-Boot environment
@ 2024-04-30 20:23 Trevor Woerner
2024-05-03 13:22 ` [yocto-patches] " Quentin Schulz
0 siblings, 1 reply; 3+ messages in thread
From: Trevor Woerner @ 2024-04-30 20:23 UTC (permalink / raw)
To: yocto-patches
U-Boot has the ability to store its environment variables to a permanent
storage device. Whether or not it does so for any one specific device
depends on whatever settings are enabled in that specific device's
defconfig. In order to definitively configure U-Boot to be able to store
its environment into the device from which it boots, for any device
supported in this BSP, simply add the following to MACHINE_FEATURES:
rk-u-boot-env
If enabled, there is now a second choice to make: should the build also
include the U-Boot environment in the image or not? The default environment,
as generated by U-Boot, can be included in the generated wic image. If it
is included, then flashing the image will also flash the default U-Boot
environment variables and settings, wiping out anything that might have been
there already. If it is not included then your device will either continue
using whatever environment happens to be there (if valid), or will not use any
stored environment if the stored environment has not been set or is invalid.
The variable which governs this behaviour is:
RK_IMAGE_INCLUDES_UBOOT_ENV
By default this is set to "0", meaning that by default the image does not
contain the U-Boot environment. To enable this behaviour, enable this
variable. This variable only takes effect if rk-u-boot-env is listed in
MACHINE_FEATURES, and has no effect otherwise.
The script:
scripts/dump-uboot-env-from-yocto-image.sh
can be used on a rockchip wic image to see the contents of the U-Boot
environment partition at build time.
Tested by booting the same image on both eMMC and SDcard with the following
devices, verifying the ability to read and write the U-Boot environment in
both U-Boot and Linux user-space, and that changes made in one are seen in the
other:
rock-3a
rock-5a
rock-5b
rock-pi-4b
rock-pi-e
rock64
Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
v4 changes:
- sort WICVARS alphabetically
- tweak dump-uboot-env-from-yocto-image.sh script to not just check for
the existence of the user-supplied parameter, but that it is a file
specifically
- u-boot bbappend:
- use a do_compile[postfuncs] for generating the fw_env.config instead of
a do_compile:append
- move the conversion of the U-Boot text-based environment to the
require binary representation from a do_deploy:append to the newly-created
do_compile[postfuncs] created above, this will allow the user to inject
any extra U-Boot environment variables they might wish as part of a
do_compile:append and have that occur before the binary is generated
so that those additional user-supplied values are included
- add a lot of extra checking (instead of assuming various resources exist)
before making use of said resources
v3 changes:
- switch from using a bbclass so the `rk-u-boot-env` override can be used
directly
- add SPDX header to script
- drop `inherit deploy` in U-Boot bbappend
v2 changes:
- re-word the commit message and README for clarity
- use bash's built-in math handling instead of depending on `bc`
- in anticipation of the upcoming feature in U-Boot whereby rockchip
devices can automatically select the environment storage device to be
the same as the boot device, use a more recent SRCREV for builds that do
environment handling, instead of hardcoding the environment storage device
by SoC family
- handle RK_IMAGE_INCLUDES_UBOOT_ENV as a boolean
---
README | 20 ++++++++++
conf/machine/include/rockchip-wic.inc | 11 +++++
.../rockchip-enable-environment-mmc.cfg | 6 +++
recipes-bsp/u-boot/u-boot_%.bbappend | 40 +++++++++++++++++++
scripts/dump-uboot-env-from-yocto-image.sh | 29 ++++++++++++++
wic/rockchip.wks | 2 +-
6 files changed, 107 insertions(+), 1 deletion(-)
create mode 100644 recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg
create mode 100755 scripts/dump-uboot-env-from-yocto-image.sh
diff --git a/README b/README
index c76fc9131276..605773d4ecd3 100644
--- a/README
+++ b/README
@@ -67,6 +67,26 @@ Notes:
in the configuration (e.g. conf/local.conf).
+U-Boot Environment:
+------------------
+ In order to configure U-Boot to be able to store its environment into the
+ device from which it was booted, for any device supported in this BSP,
+ simply add the following to MACHINE_FEATURES:
+
+ rk-u-boot-env
+
+ If enabled, to additionally have the U-Boot environment generated and
+ stored in the image, also enable the following variable (default: off):
+
+ RK_IMAGE_INCLUDES_UBOOT_ENV
+
+ The script:
+
+ scripts/dump-uboot-env-from-yocto-image.sh
+
+ can be used on a rockchip wic image to see the contents of the U-Boot
+ environment partition at build time.
+
Maintenance:
-----------
Please send pull requests, patches, comments, or questions to the
diff --git a/conf/machine/include/rockchip-wic.inc b/conf/machine/include/rockchip-wic.inc
index 147a36685d7d..92749b514499 100644
--- a/conf/machine/include/rockchip-wic.inc
+++ b/conf/machine/include/rockchip-wic.inc
@@ -2,6 +2,11 @@
require conf/machine/include/rockchip-extlinux.inc
+# 'rk-u-boot-env' indicates the user wants to be able to save their U-Boot
+# environment back to the drive from which the device was booted
+MACHINEOVERRIDES .= "${@bb.utils.contains('MACHINE_FEATURES', 'rk-u-boot-env', ':rk-u-boot-env', '', d)}"
+IMAGE_INSTALL:append:rk-u-boot-env = " u-boot-fw-utils u-boot-env"
+
SPL_BINARY ?= "idbloader.img"
IMAGE_FSTYPES += "wic wic.bmap"
@@ -11,7 +16,13 @@ WKS_FILE_DEPENDS ?= " \
virtual/bootloader \
"
+RK_IMAGE_INCLUDES_UBOOT_ENV ?= "no"
+RK_UBOOT_ENV = " "
+RK_UBOOT_ENV:rk-u-boot-env = "${@ '--source rawcopy --sourceparams=file=u-boot.env' if \
+ bb.utils.to_boolean(d.getVar('RK_IMAGE_INCLUDES_UBOOT_ENV'), False) else ' '}"
+
WICVARS:append = " \
+ RK_UBOOT_ENV \
SPL_BINARY \
UBOOT_SUFFIX \
"
diff --git a/recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg b/recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg
new file mode 100644
index 000000000000..778772d27767
--- /dev/null
+++ b/recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg
@@ -0,0 +1,6 @@
+CONFIG_ENV_SIZE=0x8000
+CONFIG_ENV_OFFSET=0x3f8000
+# CONFIG_ENV_IS_NOWHERE is not set
+CONFIG_ENV_IS_IN_MMC=y
+CONFIG_DM_SEQ_ALIAS=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
diff --git a/recipes-bsp/u-boot/u-boot_%.bbappend b/recipes-bsp/u-boot/u-boot_%.bbappend
index a83179a9f007..3dd7f08732c5 100644
--- a/recipes-bsp/u-boot/u-boot_%.bbappend
+++ b/recipes-bsp/u-boot/u-boot_%.bbappend
@@ -1,7 +1,13 @@
+FILESEXTRAPATHS:prepend := "${THISDIR}/files:"
+
+SRC_URI:append:rk-u-boot-env = " file://rockchip-enable-environment-mmc.cfg"
+SRCREV:rk-u-boot-env = "cdfcc37428e06f4730ab9a17cc084eeb7676ea1a"
+
# various machines require the pyelftools library for parsing dtb files
DEPENDS:append = " python3-pyelftools-native"
DEPENDS:append:rk3308 = " u-boot-tools-native"
DEPENDS:append:rock-pi-4 = " gnutls-native"
+DEPENDS:append:rk-u-boot-env = " u-boot-mkenvimage-native"
EXTRA_OEMAKE:append:px30 = " BL31=${DEPLOY_DIR_IMAGE}/bl31-px30.elf"
EXTRA_OEMAKE:append:rk3308 = " \
@@ -34,3 +40,37 @@ do_compile:append:rock2-square () {
cp ${B}/spl/${SPL_BINARY} ${B}
fi
}
+
+rk_generate_env() {
+ if [ ! -f "${B}/.config" ]; then
+ echo "U-Boot .config not found, can't determine environment size"
+ return 1
+ fi
+ cat ${B}/.config | grep "^CONFIG_ENV_SIZE=" > /dev/null
+ if [ $? -ne 0 ]; then
+ echo "can not find CONFIG_ENV_SIZE value in U-Boot .config"
+ return 1
+ fi
+
+ UBOOT_ENV_SIZE="$(cat ${B}/.config | grep "^CONFIG_ENV_SIZE=" | cut -d'=' -f2)"
+
+ # linux user-space U-Boot env config file
+ echo "/dev/disk/by-partlabel/uboot_env 0x0000 ${UBOOT_ENV_SIZE}" > ${WORKDIR}/fw_env.config
+
+ # convert text-based environment to binary suitable for image
+ if ${@bb.utils.to_boolean(d.getVar('RK_IMAGE_INCLUDES_UBOOT_ENV'), False)}; then
+ if [ ! -f ${B}/u-boot-initial-env ]; then
+ echo "initial, text-formatted U-Boot environment file \"${B}/u-boot-initial-env\" not found"
+ return 1
+ fi
+ mkenvimage -s ${UBOOT_ENV_SIZE} ${B}/u-boot-initial-env -o ${WORKDIR}/u-boot.env
+ fi
+}
+do_compile[postfuncs] += "${@bb.utils.contains('MACHINE_FEATURES', 'rk-u-boot-env', 'rk_generate_env', '', d)}"
+
+do_deploy:append:rk-u-boot-env() {
+ if [ -f ${WORKDIR}/u-boot.env ]; then
+ install -d ${DEPLOYDIR}
+ install -m 0644 ${WORKDIR}/u-boot.env ${DEPLOYDIR}
+ fi
+}
diff --git a/scripts/dump-uboot-env-from-yocto-image.sh b/scripts/dump-uboot-env-from-yocto-image.sh
new file mode 100755
index 000000000000..05b7048f1734
--- /dev/null
+++ b/scripts/dump-uboot-env-from-yocto-image.sh
@@ -0,0 +1,29 @@
+#/bin/bash
+# SPDX-License-Identifier: OSL-3.0
+#
+# a program that can take a wic file and dump out the contents
+# of the U-Boot environment in canonical hex+ascii format
+# (assuming the "rockchip" layout specified in this layer's wic file)
+
+# check for programs
+check_pgm() {
+ $1 --help > /dev/null 2>&1
+ if [ $? -ne 0 ]; then
+ echo "required program \"$1\" not found"
+ exit 1
+ fi
+}
+check_pgm dd
+check_pgm hexdump
+
+if [ $# -ne 1 ]; then
+ echo "required param missing: yocto wic image"
+ exit 1
+fi
+if [ ! -f "$1" ]; then
+ echo "specified file \"$1\" not found"
+ exit 1
+fi
+
+SKIP=$(( 8128 * 512 ))
+dd if="$1" ibs=1 skip=$SKIP count=32k 2> /dev/null | hexdump -C
diff --git a/wic/rockchip.wks b/wic/rockchip.wks
index 9ba3352b51bb..bc65f73b0cf3 100644
--- a/wic/rockchip.wks
+++ b/wic/rockchip.wks
@@ -22,7 +22,7 @@ part loader1 --offset 64s --fixed-size 3552K --fstype=none --part-name load
part v_storage --offset 7168s --fixed-size 256K --fstype=none --part-name v_storage
part reserved --offset 7680s --fixed-size 192K --fstype=none --part-name reserved
part reserved1 --offset 8064s --fixed-size 32K --fstype=none --part-name reserved1
-part uboot_env --offset 8128s --fixed-size 32K --fstype=none --part-name uboot_env
+part uboot_env --offset 8128s --fixed-size 32K --fstype=none --part-name uboot_env ${RK_UBOOT_ENV}
part reserved2 --offset 8192s --fixed-size 4096K --fstype=none --part-name reserved2
part loader2 --offset 16384s --fixed-size 4096K --fstype=none --part-name loader2 --source rawcopy --sourceparams="file=u-boot.${UBOOT_SUFFIX}"
part atf --offset 24576s --fixed-size 4096K --fstype=none --part-name atf
--
2.44.0.478.g7774cfed6261
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [yocto-patches] [meta-rockchip][PATCH v4] enable stored U-Boot environment
2024-04-30 20:23 [meta-rockchip][PATCH v4] enable stored U-Boot environment Trevor Woerner
@ 2024-05-03 13:22 ` Quentin Schulz
2024-05-04 12:49 ` Trevor Woerner
0 siblings, 1 reply; 3+ messages in thread
From: Quentin Schulz @ 2024-05-03 13:22 UTC (permalink / raw)
To: yocto-patches
Hi Trevor,
On 4/30/24 10:23 PM, Trevor Woerner via lists.yoctoproject.org wrote:
> U-Boot has the ability to store its environment variables to a permanent
> storage device. Whether or not it does so for any one specific device
> depends on whatever settings are enabled in that specific device's
> defconfig. In order to definitively configure U-Boot to be able to store
> its environment into the device from which it boots, for any device
> supported in this BSP, simply add the following to MACHINE_FEATURES:
>
> rk-u-boot-env
>
> If enabled, there is now a second choice to make: should the build also
> include the U-Boot environment in the image or not? The default environment,
> as generated by U-Boot, can be included in the generated wic image. If it
> is included, then flashing the image will also flash the default U-Boot
> environment variables and settings, wiping out anything that might have been
> there already. If it is not included then your device will either continue
> using whatever environment happens to be there (if valid), or will not use any
> stored environment if the stored environment has not been set or is invalid.
> The variable which governs this behaviour is:
>
> RK_IMAGE_INCLUDES_UBOOT_ENV
>
> By default this is set to "0", meaning that by default the image does not
> contain the U-Boot environment. To enable this behaviour, enable this
> variable. This variable only takes effect if rk-u-boot-env is listed in
> MACHINE_FEATURES, and has no effect otherwise.
>
> The script:
>
> scripts/dump-uboot-env-from-yocto-image.sh
>
> can be used on a rockchip wic image to see the contents of the U-Boot
> environment partition at build time.
>
> Tested by booting the same image on both eMMC and SDcard with the following
> devices, verifying the ability to read and write the U-Boot environment in
> both U-Boot and Linux user-space, and that changes made in one are seen in the
> other:
> rock-3a
> rock-5a
> rock-5b
> rock-pi-4b
> rock-pi-e
> rock64
>
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> ---
> v4 changes:
> - sort WICVARS alphabetically
> - tweak dump-uboot-env-from-yocto-image.sh script to not just check for
> the existence of the user-supplied parameter, but that it is a file
> specifically
> - u-boot bbappend:
> - use a do_compile[postfuncs] for generating the fw_env.config instead of
> a do_compile:append
> - move the conversion of the U-Boot text-based environment to the
> require binary representation from a do_deploy:append to the newly-created
> do_compile[postfuncs] created above, this will allow the user to inject
> any extra U-Boot environment variables they might wish as part of a
> do_compile:append and have that occur before the binary is generated
> so that those additional user-supplied values are included
> - add a lot of extra checking (instead of assuming various resources exist)
> before making use of said resources
>
> v3 changes:
> - switch from using a bbclass so the `rk-u-boot-env` override can be used
> directly
> - add SPDX header to script
> - drop `inherit deploy` in U-Boot bbappend
>
> v2 changes:
> - re-word the commit message and README for clarity
> - use bash's built-in math handling instead of depending on `bc`
> - in anticipation of the upcoming feature in U-Boot whereby rockchip
> devices can automatically select the environment storage device to be
> the same as the boot device, use a more recent SRCREV for builds that do
> environment handling, instead of hardcoding the environment storage device
> by SoC family
> - handle RK_IMAGE_INCLUDES_UBOOT_ENV as a boolean
> ---
> README | 20 ++++++++++
> conf/machine/include/rockchip-wic.inc | 11 +++++
> .../rockchip-enable-environment-mmc.cfg | 6 +++
> recipes-bsp/u-boot/u-boot_%.bbappend | 40 +++++++++++++++++++
> scripts/dump-uboot-env-from-yocto-image.sh | 29 ++++++++++++++
> wic/rockchip.wks | 2 +-
> 6 files changed, 107 insertions(+), 1 deletion(-)
> create mode 100644 recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg
> create mode 100755 scripts/dump-uboot-env-from-yocto-image.sh
>
> diff --git a/README b/README
> index c76fc9131276..605773d4ecd3 100644
> --- a/README
> +++ b/README
> @@ -67,6 +67,26 @@ Notes:
>
> in the configuration (e.g. conf/local.conf).
>
> +U-Boot Environment:
> +------------------
> + In order to configure U-Boot to be able to store its environment into the
> + device from which it was booted, for any device supported in this BSP,
> + simply add the following to MACHINE_FEATURES:
> +
> + rk-u-boot-env
> +
> + If enabled, to additionally have the U-Boot environment generated and
> + stored in the image, also enable the following variable (default: off):
> +
> + RK_IMAGE_INCLUDES_UBOOT_ENV
> +
> + The script:
> +
> + scripts/dump-uboot-env-from-yocto-image.sh
> +
> + can be used on a rockchip wic image to see the contents of the U-Boot
> + environment partition at build time.
> +
> Maintenance:
> -----------
> Please send pull requests, patches, comments, or questions to the
> diff --git a/conf/machine/include/rockchip-wic.inc b/conf/machine/include/rockchip-wic.inc
> index 147a36685d7d..92749b514499 100644
> --- a/conf/machine/include/rockchip-wic.inc
> +++ b/conf/machine/include/rockchip-wic.inc
> @@ -2,6 +2,11 @@
>
> require conf/machine/include/rockchip-extlinux.inc
>
> +# 'rk-u-boot-env' indicates the user wants to be able to save their U-Boot
> +# environment back to the drive from which the device was booted
> +MACHINEOVERRIDES .= "${@bb.utils.contains('MACHINE_FEATURES', 'rk-u-boot-env', ':rk-u-boot-env', '', d)}"
> +IMAGE_INSTALL:append:rk-u-boot-env = " u-boot-fw-utils u-boot-env"
> +
> SPL_BINARY ?= "idbloader.img"
>
> IMAGE_FSTYPES += "wic wic.bmap"
> @@ -11,7 +16,13 @@ WKS_FILE_DEPENDS ?= " \
> virtual/bootloader \
> "
>
> +RK_IMAGE_INCLUDES_UBOOT_ENV ?= "no"
> +RK_UBOOT_ENV = " "
> +RK_UBOOT_ENV:rk-u-boot-env = "${@ '--source rawcopy --sourceparams=file=u-boot.env' if \
> + bb.utils.to_boolean(d.getVar('RK_IMAGE_INCLUDES_UBOOT_ENV'), False) else ' '}"
> +
> WICVARS:append = " \
> + RK_UBOOT_ENV \
> SPL_BINARY \
> UBOOT_SUFFIX \
> "
> diff --git a/recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg b/recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg
> new file mode 100644
> index 000000000000..778772d27767
> --- /dev/null
> +++ b/recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg
> @@ -0,0 +1,6 @@
> +CONFIG_ENV_SIZE=0x8000
> +CONFIG_ENV_OFFSET=0x3f8000
> +# CONFIG_ENV_IS_NOWHERE is not set
> +CONFIG_ENV_IS_IN_MMC=y
> +CONFIG_DM_SEQ_ALIAS=y
> +CONFIG_SPL_DM_SEQ_ALIAS=y
> diff --git a/recipes-bsp/u-boot/u-boot_%.bbappend b/recipes-bsp/u-boot/u-boot_%.bbappend
> index a83179a9f007..3dd7f08732c5 100644
> --- a/recipes-bsp/u-boot/u-boot_%.bbappend
> +++ b/recipes-bsp/u-boot/u-boot_%.bbappend
> @@ -1,7 +1,13 @@
> +FILESEXTRAPATHS:prepend := "${THISDIR}/files:"
> +
> +SRC_URI:append:rk-u-boot-env = " file://rockchip-enable-environment-mmc.cfg"
> +SRCREV:rk-u-boot-env = "cdfcc37428e06f4730ab9a17cc084eeb7676ea1a"
> +
> # various machines require the pyelftools library for parsing dtb files
> DEPENDS:append = " python3-pyelftools-native"
> DEPENDS:append:rk3308 = " u-boot-tools-native"
> DEPENDS:append:rock-pi-4 = " gnutls-native"
> +DEPENDS:append:rk-u-boot-env = " u-boot-mkenvimage-native"
>
> EXTRA_OEMAKE:append:px30 = " BL31=${DEPLOY_DIR_IMAGE}/bl31-px30.elf"
> EXTRA_OEMAKE:append:rk3308 = " \
> @@ -34,3 +40,37 @@ do_compile:append:rock2-square () {
> cp ${B}/spl/${SPL_BINARY} ${B}
> fi
> }
> +
> +rk_generate_env() {
> + if [ ! -f "${B}/.config" ]; then
> + echo "U-Boot .config not found, can't determine environment size"
> + return 1
> + fi
> + cat ${B}/.config | grep "^CONFIG_ENV_SIZE=" > /dev/null
> + if [ $? -ne 0 ]; then
> + echo "can not find CONFIG_ENV_SIZE value in U-Boot .config"
> + return 1
> + fi
> +
> + UBOOT_ENV_SIZE="$(cat ${B}/.config | grep "^CONFIG_ENV_SIZE=" | cut -d'=' -f2)"
> +
> + # linux user-space U-Boot env config file
> + echo "/dev/disk/by-partlabel/uboot_env 0x0000 ${UBOOT_ENV_SIZE}" > ${WORKDIR}/fw_env.config
> +
> + # convert text-based environment to binary suitable for image
> + if ${@bb.utils.to_boolean(d.getVar('RK_IMAGE_INCLUDES_UBOOT_ENV'), False)}; then
> + if [ ! -f ${B}/u-boot-initial-env ]; then
> + echo "initial, text-formatted U-Boot environment file \"${B}/u-boot-initial-env\" not found"
> + return 1
> + fi
> + mkenvimage -s ${UBOOT_ENV_SIZE} ${B}/u-boot-initial-env -o ${WORKDIR}/u-boot.env
Could be put into ${B} instead of ${WORKDIR}, since we generate it? I
guess since other files we generate are put into WORKDIR, it'd be up to
you here.
> + fi
> +}
> +do_compile[postfuncs] += "${@bb.utils.contains('MACHINE_FEATURES', 'rk-u-boot-env', 'rk_generate_env', '', d)}"
I can suggest:
"""
rk_generate_env() {
:
}
rk_generate_env:rk-u-boot-env() {
[...]
}
do_compile[postfuncs] += "rk_generate_env;"
"""
Which is a bit more readable for the do_compile[postfuncs] part. Up to you.
> +
> +do_deploy:append:rk-u-boot-env() {
> + if [ -f ${WORKDIR}/u-boot.env ]; then
This is an issue. If one builds with RK_IMAGE_INCLUDES_UBOOT_ENV = 1 and
then RK_IMAGE_INCLUDES_UBOOT_ENV = 0, the file will still be in WORKDIR
(because it is never cleaned). I suggest to test for
RK_IMAGE_INCLUDES_UBOOT_ENV = true/1/yes/whatever instead (or in addition).
I am not entirely sure the migration to UNPACKDIR would fix this
(haven't read anything about the consequences of it, e.g. I'm not sure
it's cleaned by any task). I'm also a bit puzzled by master still having
some bits using WORKDIR in the u-boot recipe. Not sure what's going on here.
> + install -d ${DEPLOYDIR}
> + install -m 0644 ${WORKDIR}/u-boot.env ${DEPLOYDIR}
> + fi
> +}
We probably need to adapt this now that master uses UNPACKDIR instead of
WORKDIR. This will require us to fork a branch for scarthgap now, which
still uses WORKDIR but for the master branch, we need to use UNPACKDIR here.
Outside from the blocker in do_deploy, and the questions around
UNPACKDIR/WORKDIR, I think it's looking good right now?
Cheers,
Quentin
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [yocto-patches] [meta-rockchip][PATCH v4] enable stored U-Boot environment
2024-05-03 13:22 ` [yocto-patches] " Quentin Schulz
@ 2024-05-04 12:49 ` Trevor Woerner
0 siblings, 0 replies; 3+ messages in thread
From: Trevor Woerner @ 2024-05-04 12:49 UTC (permalink / raw)
To: yocto-patches
On Fri 2024-05-03 @ 03:22:54 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> Hi Trevor,
>
> On 4/30/24 10:23 PM, Trevor Woerner via lists.yoctoproject.org wrote:
> > U-Boot has the ability to store its environment variables to a permanent
> > storage device. Whether or not it does so for any one specific device
> > depends on whatever settings are enabled in that specific device's
> > defconfig. In order to definitively configure U-Boot to be able to store
> > its environment into the device from which it boots, for any device
> > supported in this BSP, simply add the following to MACHINE_FEATURES:
> >
> > rk-u-boot-env
> >
> > If enabled, there is now a second choice to make: should the build also
> > include the U-Boot environment in the image or not? The default environment,
> > as generated by U-Boot, can be included in the generated wic image. If it
> > is included, then flashing the image will also flash the default U-Boot
> > environment variables and settings, wiping out anything that might have been
> > there already. If it is not included then your device will either continue
> > using whatever environment happens to be there (if valid), or will not use any
> > stored environment if the stored environment has not been set or is invalid.
> > The variable which governs this behaviour is:
> >
> > RK_IMAGE_INCLUDES_UBOOT_ENV
> >
> > By default this is set to "0", meaning that by default the image does not
> > contain the U-Boot environment. To enable this behaviour, enable this
> > variable. This variable only takes effect if rk-u-boot-env is listed in
> > MACHINE_FEATURES, and has no effect otherwise.
> >
> > The script:
> >
> > scripts/dump-uboot-env-from-yocto-image.sh
> >
> > can be used on a rockchip wic image to see the contents of the U-Boot
> > environment partition at build time.
> >
> > Tested by booting the same image on both eMMC and SDcard with the following
> > devices, verifying the ability to read and write the U-Boot environment in
> > both U-Boot and Linux user-space, and that changes made in one are seen in the
> > other:
> > rock-3a
> > rock-5a
> > rock-5b
> > rock-pi-4b
> > rock-pi-e
> > rock64
> >
> > Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> > ---
> > v4 changes:
> > - sort WICVARS alphabetically
> > - tweak dump-uboot-env-from-yocto-image.sh script to not just check for
> > the existence of the user-supplied parameter, but that it is a file
> > specifically
> > - u-boot bbappend:
> > - use a do_compile[postfuncs] for generating the fw_env.config instead of
> > a do_compile:append
> > - move the conversion of the U-Boot text-based environment to the
> > require binary representation from a do_deploy:append to the newly-created
> > do_compile[postfuncs] created above, this will allow the user to inject
> > any extra U-Boot environment variables they might wish as part of a
> > do_compile:append and have that occur before the binary is generated
> > so that those additional user-supplied values are included
> > - add a lot of extra checking (instead of assuming various resources exist)
> > before making use of said resources
> >
> > v3 changes:
> > - switch from using a bbclass so the `rk-u-boot-env` override can be used
> > directly
> > - add SPDX header to script
> > - drop `inherit deploy` in U-Boot bbappend
> >
> > v2 changes:
> > - re-word the commit message and README for clarity
> > - use bash's built-in math handling instead of depending on `bc`
> > - in anticipation of the upcoming feature in U-Boot whereby rockchip
> > devices can automatically select the environment storage device to be
> > the same as the boot device, use a more recent SRCREV for builds that do
> > environment handling, instead of hardcoding the environment storage device
> > by SoC family
> > - handle RK_IMAGE_INCLUDES_UBOOT_ENV as a boolean
> > ---
> > README | 20 ++++++++++
> > conf/machine/include/rockchip-wic.inc | 11 +++++
> > .../rockchip-enable-environment-mmc.cfg | 6 +++
> > recipes-bsp/u-boot/u-boot_%.bbappend | 40 +++++++++++++++++++
> > scripts/dump-uboot-env-from-yocto-image.sh | 29 ++++++++++++++
> > wic/rockchip.wks | 2 +-
> > 6 files changed, 107 insertions(+), 1 deletion(-)
> > create mode 100644 recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg
> > create mode 100755 scripts/dump-uboot-env-from-yocto-image.sh
> >
> > diff --git a/README b/README
> > index c76fc9131276..605773d4ecd3 100644
> > --- a/README
> > +++ b/README
> > @@ -67,6 +67,26 @@ Notes:
> >
> > in the configuration (e.g. conf/local.conf).
> > +U-Boot Environment:
> > +------------------
> > + In order to configure U-Boot to be able to store its environment into the
> > + device from which it was booted, for any device supported in this BSP,
> > + simply add the following to MACHINE_FEATURES:
> > +
> > + rk-u-boot-env
> > +
> > + If enabled, to additionally have the U-Boot environment generated and
> > + stored in the image, also enable the following variable (default: off):
> > +
> > + RK_IMAGE_INCLUDES_UBOOT_ENV
> > +
> > + The script:
> > +
> > + scripts/dump-uboot-env-from-yocto-image.sh
> > +
> > + can be used on a rockchip wic image to see the contents of the U-Boot
> > + environment partition at build time.
> > +
> > Maintenance:
> > -----------
> > Please send pull requests, patches, comments, or questions to the
> > diff --git a/conf/machine/include/rockchip-wic.inc b/conf/machine/include/rockchip-wic.inc
> > index 147a36685d7d..92749b514499 100644
> > --- a/conf/machine/include/rockchip-wic.inc
> > +++ b/conf/machine/include/rockchip-wic.inc
> > @@ -2,6 +2,11 @@
> > require conf/machine/include/rockchip-extlinux.inc
> > +# 'rk-u-boot-env' indicates the user wants to be able to save their U-Boot
> > +# environment back to the drive from which the device was booted
> > +MACHINEOVERRIDES .= "${@bb.utils.contains('MACHINE_FEATURES', 'rk-u-boot-env', ':rk-u-boot-env', '', d)}"
> > +IMAGE_INSTALL:append:rk-u-boot-env = " u-boot-fw-utils u-boot-env"
> > +
> > SPL_BINARY ?= "idbloader.img"
> > IMAGE_FSTYPES += "wic wic.bmap"
> > @@ -11,7 +16,13 @@ WKS_FILE_DEPENDS ?= " \
> > virtual/bootloader \
> > "
> > +RK_IMAGE_INCLUDES_UBOOT_ENV ?= "no"
> > +RK_UBOOT_ENV = " "
> > +RK_UBOOT_ENV:rk-u-boot-env = "${@ '--source rawcopy --sourceparams=file=u-boot.env' if \
> > + bb.utils.to_boolean(d.getVar('RK_IMAGE_INCLUDES_UBOOT_ENV'), False) else ' '}"
> > +
> > WICVARS:append = " \
> > + RK_UBOOT_ENV \
> > SPL_BINARY \
> > UBOOT_SUFFIX \
> > "
> > diff --git a/recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg b/recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg
> > new file mode 100644
> > index 000000000000..778772d27767
> > --- /dev/null
> > +++ b/recipes-bsp/u-boot/files/rk-u-boot-env/rockchip-enable-environment-mmc.cfg
> > @@ -0,0 +1,6 @@
> > +CONFIG_ENV_SIZE=0x8000
> > +CONFIG_ENV_OFFSET=0x3f8000
> > +# CONFIG_ENV_IS_NOWHERE is not set
> > +CONFIG_ENV_IS_IN_MMC=y
> > +CONFIG_DM_SEQ_ALIAS=y
> > +CONFIG_SPL_DM_SEQ_ALIAS=y
> > diff --git a/recipes-bsp/u-boot/u-boot_%.bbappend b/recipes-bsp/u-boot/u-boot_%.bbappend
> > index a83179a9f007..3dd7f08732c5 100644
> > --- a/recipes-bsp/u-boot/u-boot_%.bbappend
> > +++ b/recipes-bsp/u-boot/u-boot_%.bbappend
> > @@ -1,7 +1,13 @@
> > +FILESEXTRAPATHS:prepend := "${THISDIR}/files:"
> > +
> > +SRC_URI:append:rk-u-boot-env = " file://rockchip-enable-environment-mmc.cfg"
> > +SRCREV:rk-u-boot-env = "cdfcc37428e06f4730ab9a17cc084eeb7676ea1a"
> > +
> > # various machines require the pyelftools library for parsing dtb files
> > DEPENDS:append = " python3-pyelftools-native"
> > DEPENDS:append:rk3308 = " u-boot-tools-native"
> > DEPENDS:append:rock-pi-4 = " gnutls-native"
> > +DEPENDS:append:rk-u-boot-env = " u-boot-mkenvimage-native"
> > EXTRA_OEMAKE:append:px30 = " BL31=${DEPLOY_DIR_IMAGE}/bl31-px30.elf"
> > EXTRA_OEMAKE:append:rk3308 = " \
> > @@ -34,3 +40,37 @@ do_compile:append:rock2-square () {
> > cp ${B}/spl/${SPL_BINARY} ${B}
> > fi
> > }
> > +
> > +rk_generate_env() {
> > + if [ ! -f "${B}/.config" ]; then
> > + echo "U-Boot .config not found, can't determine environment size"
> > + return 1
> > + fi
> > + cat ${B}/.config | grep "^CONFIG_ENV_SIZE=" > /dev/null
> > + if [ $? -ne 0 ]; then
> > + echo "can not find CONFIG_ENV_SIZE value in U-Boot .config"
> > + return 1
> > + fi
> > +
> > + UBOOT_ENV_SIZE="$(cat ${B}/.config | grep "^CONFIG_ENV_SIZE=" | cut -d'=' -f2)"
> > +
> > + # linux user-space U-Boot env config file
> > + echo "/dev/disk/by-partlabel/uboot_env 0x0000 ${UBOOT_ENV_SIZE}" > ${WORKDIR}/fw_env.config
> > +
> > + # convert text-based environment to binary suitable for image
> > + if ${@bb.utils.to_boolean(d.getVar('RK_IMAGE_INCLUDES_UBOOT_ENV'), False)}; then
> > + if [ ! -f ${B}/u-boot-initial-env ]; then
> > + echo "initial, text-formatted U-Boot environment file \"${B}/u-boot-initial-env\" not found"
> > + return 1
> > + fi
> > + mkenvimage -s ${UBOOT_ENV_SIZE} ${B}/u-boot-initial-env -o ${WORKDIR}/u-boot.env
>
> Could be put into ${B} instead of ${WORKDIR}, since we generate it? I guess
> since other files we generate are put into WORKDIR, it'd be up to you here.
>
> > + fi
> > +}
> > +do_compile[postfuncs] += "${@bb.utils.contains('MACHINE_FEATURES', 'rk-u-boot-env', 'rk_generate_env', '', d)}"
>
> I can suggest:
>
> """
> rk_generate_env() {
> :
> }
>
> rk_generate_env:rk-u-boot-env() {
> [...]
> }
>
> do_compile[postfuncs] += "rk_generate_env;"
> """
>
> Which is a bit more readable for the do_compile[postfuncs] part. Up to you.
I actually did try making this change and in this case I don't think it looks
more readable. Given that we only are handling 1 case, it looked messy to me
having 1 empty function and 1 function to cover the 1 case we're interested
in. If we had a set of cases then I'd agree that it would look much nicer to
always include the postfunc; for which in 1/N cases it's empty (where N is >
2).
> > +
> > +do_deploy:append:rk-u-boot-env() {
> > + if [ -f ${WORKDIR}/u-boot.env ]; then
>
> This is an issue. If one builds with RK_IMAGE_INCLUDES_UBOOT_ENV = 1 and
> then RK_IMAGE_INCLUDES_UBOOT_ENV = 0, the file will still be in WORKDIR
> (because it is never cleaned). I suggest to test for
> RK_IMAGE_INCLUDES_UBOOT_ENV = true/1/yes/whatever instead (or in addition).
Sounds good.
Also the handling of that variable in the postfuncs had a bug in it. I built
and built and built that code over and over with various configurations and it
always built fine on my build machine. For fun I tried that patch on a
completely different machine and it failed. I'll be sure to do more clean
builds. So fixing that bug and adding this check are similar.
> I am not entirely sure the migration to UNPACKDIR would fix this (haven't
> read anything about the consequences of it, e.g. I'm not sure it's cleaned
> by any task). I'm also a bit puzzled by master still having some bits using
> WORKDIR in the u-boot recipe. Not sure what's going on here.
>
> > + install -d ${DEPLOYDIR}
> > + install -m 0644 ${WORKDIR}/u-boot.env ${DEPLOYDIR}
> > + fi
> > +}
>
> We probably need to adapt this now that master uses UNPACKDIR instead of
> WORKDIR. This will require us to fork a branch for scarthgap now, which
> still uses WORKDIR but for the master branch, we need to use UNPACKDIR here.
Nice! I look forward to the UNPACKDIR functionality. My understanding is that
for the time-being UNPACKDIR is set to WORKDIR, so there are no visible
changes quite yet.
In any case I would prefer to get this patch in with WORKDIR, then perform a
WORKDIR → UNPACKDIR cleanup as a separate patch for the entire layer, rather
than half switching to UNPACKDIR for this patch then cleaning up the rest
sometime later.
> Outside from the blocker in do_deploy, and the questions around
> UNPACKDIR/WORKDIR, I think it's looking good right now?
>
> Cheers,
> Quentin
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#81): https://lists.yoctoproject.org/g/yocto-patches/message/81
> Mute This Topic: https://lists.yoctoproject.org/mt/105829012/900817
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13168745/900817/63955952/xyzzy [twoerner@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-04 12:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-30 20:23 [meta-rockchip][PATCH v4] enable stored U-Boot environment Trevor Woerner
2024-05-03 13:22 ` [yocto-patches] " Quentin Schulz
2024-05-04 12:49 ` Trevor Woerner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.