[PATCH next v2 0/6] rockchip: puma-rk3399/ringneck-px30: fix check against value in default environment variable
For puma-rk3399 and ringneck-px30, U-Boot proper automatically modifies boot_targets to swap the order in which MMC storage media are used for standard boot based on which MMC storage medium was used to load U-Boot proper. This is however only done if the user has not changed it manually, therefore a check between the default and current value is done.
This used to work fine until the migration to standard boot where boot_targets value size in the default environment went above the 32 characters that env_get_default function can return, thus resulting in a truncated variable.
Therefore the check between default and current value would always fail.
By adding the env_get_default_into function, a buffer of the appropriate size can be allocated on the stack to get the whole value of boot_targets in the default environment and thus fixing the check.
While at it, simplify maintenance by putting this logic in a common file and stop using hardcoded DT paths for MMC controllers and SPI flash memory.
Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com --- Changes in v2: - added board/theobroma-systems/common putting all storage medium logic into a common file, - adapted the storage medium logic to be device-agnostic, - added Rbs - removed patches surrounding all code by an ifndef CONFIG_SPL_BUILD and do this at the Makefile level of each board, - Link to v1: https://lore.kernel.org/r/20231108-env_default_theobroma-v1-0-cb493bfdeffd@t...
--- Quentin Schulz (6): rockchip: ringneck-px30: do not hardcode MMC controller paths rockchip: puma-rk3399: do not hardcode MMC controller paths rockchip: ringneck-px30/puma-rk3399: factor out storage medium selection env: allow to copy value from default environment into a buffer env: migrate env_get_default to call env_get_default_into rockchip: theobroma-systems: fix modified boot_targets detection
board/theobroma-systems/common/common.c | 143 +++++++++++++++++++++ board/theobroma-systems/common/common.h | 19 +++ board/theobroma-systems/puma_rk3399/MAINTAINERS | 1 + board/theobroma-systems/puma_rk3399/Makefile | 3 + board/theobroma-systems/puma_rk3399/puma-rk3399.c | 143 +-------------------- board/theobroma-systems/ringneck_px30/MAINTAINERS | 1 + board/theobroma-systems/ringneck_px30/Makefile | 3 + .../ringneck_px30/ringneck-px30.c | 142 +------------------- env/common.c | 16 ++- include/env.h | 10 ++ 10 files changed, 195 insertions(+), 286 deletions(-) --- base-commit: 9e53e45292ee2f1d9d2ccc59914b161bef9b10d7 change-id: 20231108-env_default_theobroma-33774a337fe6
Best regards,
From: Quentin Schulz quentin.schulz@theobroma-systems.com
To prepare to put the similar logic around storage medium selection for Ringneck PX30 and Puma RK3399 in common, let's not use hardcoded paths but use uclass functions instead to find udevice based on their DT node full path.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com --- .../ringneck_px30/ringneck-px30.c | 29 +++++++++++++--------- 1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/board/theobroma-systems/ringneck_px30/ringneck-px30.c b/board/theobroma-systems/ringneck_px30/ringneck-px30.c index 537ce0d1d11..fb5570767db 100644 --- a/board/theobroma-systems/ringneck_px30/ringneck-px30.c +++ b/board/theobroma-systems/ringneck_px30/ringneck-px30.c @@ -68,11 +68,16 @@ static int setup_boottargets(void) * Make the default boot medium between SD Card and eMMC, the one that * was used to load U-Boot proper. */ - bool sd_booted = !strcmp(boot_device, "/mmc@ff370000"); + struct udevice *devp; + + if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) { + debug("%s: not reordering boot_targets, bootdev %s != MMC\n", + __func__, boot_device); + return 0; + } + char *mmc0, *mmc1;
- debug("%s: booted from %s\n", __func__, - sd_booted ? "SD-Card" : "eMMC"); mmc0 = strstr(env, "mmc0"); mmc1 = strstr(env, "mmc1");
@@ -87,8 +92,8 @@ static int setup_boottargets(void) * If mmc1 comes first in the boot order and U-Boot proper was * loaded from mmc0, swap mmc0 and mmc1 in the list. */ - if ((mmc0 < mmc1 && sd_booted) || - (mmc0 > mmc1 && !sd_booted)) { + if ((mmc0 < mmc1 && devp->seq_ == 1) || + (mmc0 > mmc1 && devp->seq_ == 0)) { mmc0[3] = '1'; mmc1[3] = '0'; debug("%s: set boot_targets to: %s\n", __func__, env); @@ -102,6 +107,7 @@ int mmc_get_env_dev(void) { const char *boot_device = ofnode_read_chosen_string("u-boot,spl-boot-device"); + struct udevice *devp;
if (!boot_device) { debug("%s: /chosen/u-boot,spl-boot-device not set\n", @@ -111,19 +117,19 @@ int mmc_get_env_dev(void)
debug("%s: booted from %s\n", __func__, boot_device);
- if (!strcmp(boot_device, "/mmc@ff370000")) - return 1; + if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) + return CONFIG_SYS_MMC_ENV_DEV;
- if (!strcmp(boot_device, "/mmc@ff390000")) - return 0; + debug("%s: get MMC ENV from mmc%d\n", __func__, devp->seq_);
- return CONFIG_SYS_MMC_ENV_DEV; + return devp->seq_; }
enum env_location arch_env_get_location(enum env_operation op, int prio) { const char *boot_device = ofnode_read_chosen_string("u-boot,spl-boot-device"); + struct udevice *devp;
if (prio > 0) return ENVL_UNKNOWN; @@ -137,8 +143,7 @@ enum env_location arch_env_get_location(enum env_operation op, int prio) debug("%s: booted from %s\n", __func__, boot_device);
if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC) && - (!strcmp(boot_device, "/mmc@ff370000") || - !strcmp(boot_device, "/mmc@ff390000"))) + !uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) return ENVL_MMC;
printf("%s: No environment available: booted from %s but U-Boot "
From: Quentin Schulz quentin.schulz@theobroma-systems.com
To prepare to put the similar logic around storage medium selection for Ringneck PX30 and Puma RK3399 in common, let's not use hardcoded paths but use uclass functions instead to find udevice based on their DT node full path.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com --- board/theobroma-systems/puma_rk3399/puma-rk3399.c | 73 ++++++++++++----------- 1 file changed, 38 insertions(+), 35 deletions(-)
diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c index 614a60ee8f9..ac8ceedc7c9 100644 --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c @@ -82,33 +82,36 @@ static int setup_boottargets(void) * was used to load U-Boot proper. If SPI-NOR flash was used, keep * original default order. */ - if (strcmp(boot_device, "/spi@ff1d0000/flash@0")) { - bool sd_booted = !strcmp(boot_device, "/mmc@fe320000"); - char *mmc0, *mmc1; - - debug("%s: booted from %s\n", __func__, - sd_booted ? "SD-Card" : "eMMC"); - mmc0 = strstr(env, "mmc0"); - mmc1 = strstr(env, "mmc1"); - - if (!mmc0 || !mmc1) { - debug("%s: only one mmc boot_target found\n", __func__); - return -1; - } - - /* - * If mmc0 comes first in the boot order and U-Boot proper was - * loaded from mmc1, swap mmc0 and mmc1 in the list. - * If mmc1 comes first in the boot order and U-Boot proper was - * loaded from mmc0, swap mmc0 and mmc1 in the list. - */ - if ((mmc0 < mmc1 && sd_booted) || - (mmc0 > mmc1 && !sd_booted)) { - mmc0[3] = '1'; - mmc1[3] = '0'; - debug("%s: set boot_targets to: %s\n", __func__, env); - env_set("boot_targets", env); - } + struct udevice *devp; + + if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) { + debug("%s: not reordering boot_targets, bootdev %s != MMC\n", + __func__, boot_device); + return 0; + } + + char *mmc0, *mmc1; + + mmc0 = strstr(env, "mmc0"); + mmc1 = strstr(env, "mmc1"); + + if (!mmc0 || !mmc1) { + debug("%s: only one mmc boot_target found\n", __func__); + return -1; + } + + /* + * If mmc0 comes first in the boot order and U-Boot proper was + * loaded from mmc1, swap mmc0 and mmc1 in the list. + * If mmc1 comes first in the boot order and U-Boot proper was + * loaded from mmc0, swap mmc0 and mmc1 in the list. + */ + if ((mmc0 < mmc1 && devp->seq_ == 1) || + (mmc0 > mmc1 && devp->seq_ == 0)) { + mmc0[3] = '1'; + mmc1[3] = '0'; + debug("%s: set boot_targets to: %s\n", __func__, env); + env_set("boot_targets", env); }
return 0; @@ -118,6 +121,7 @@ int mmc_get_env_dev(void) { const char *boot_device = ofnode_read_chosen_string("u-boot,spl-boot-device"); + struct udevice *devp;
if (!boot_device) { debug("%s: /chosen/u-boot,spl-boot-device not set\n", @@ -127,19 +131,19 @@ int mmc_get_env_dev(void)
debug("%s: booted from %s\n", __func__, boot_device);
- if (!strcmp(boot_device, "/mmc@fe320000")) - return 1; + if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) + return CONFIG_SYS_MMC_ENV_DEV;
- if (!strcmp(boot_device, "/mmc@fe330000")) - return 0; + debug("%s: get MMC ENV from mmc%d\n", __func__, devp->seq_);
- return CONFIG_SYS_MMC_ENV_DEV; + return devp->seq_; }
enum env_location arch_env_get_location(enum env_operation op, int prio) { const char *boot_device = ofnode_read_chosen_string("u-boot,spl-boot-device"); + struct udevice *devp;
if (prio > 0) return ENVL_UNKNOWN; @@ -153,12 +157,11 @@ enum env_location arch_env_get_location(enum env_operation op, int prio) debug("%s: booted from %s\n", __func__, boot_device);
if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH) && - !strcmp(boot_device, "/spi@ff1d0000/flash@0")) + !uclass_find_device_by_ofnode(UCLASS_SPI_FLASH, ofnode_path(boot_device), &devp)) return ENVL_SPI_FLASH;
if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC) && - (!strcmp(boot_device, "/mmc@fe320000") || - !strcmp(boot_device, "/mmc@fe330000"))) + !uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) return ENVL_MMC;
printf("%s: No environment available: booted from %s but U-Boot "
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Ringneck PX30 and Puma RK3399 both have the same expectation with regard to bootstd device order and U-Boot environment storage device, except that Puma RK3399 also supports SPI Flash.
Let's move all of this into a common file where common logic can be put.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com --- board/theobroma-systems/common/common.c | 139 +++++++++++++++++++ board/theobroma-systems/common/common.h | 19 +++ board/theobroma-systems/puma_rk3399/MAINTAINERS | 1 + board/theobroma-systems/puma_rk3399/Makefile | 3 + board/theobroma-systems/puma_rk3399/puma-rk3399.c | 146 +------------------- board/theobroma-systems/ringneck_px30/MAINTAINERS | 1 + board/theobroma-systems/ringneck_px30/Makefile | 3 + .../ringneck_px30/ringneck-px30.c | 147 +-------------------- 8 files changed, 168 insertions(+), 291 deletions(-)
diff --git a/board/theobroma-systems/common/common.c b/board/theobroma-systems/common/common.c new file mode 100644 index 00000000000..f4b854554ea --- /dev/null +++ b/board/theobroma-systems/common/common.c @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH + */ + +#include <dm.h> +#include <env.h> +#include <env_internal.h> +#include <dm/uclass-internal.h> + +/* + * Swap mmc0 and mmc1 in boot_targets if booted from SD-Card. + * + * If bootsource is uSD-card we can assume that we want to use the + * SD-Card instead of the eMMC as first boot_target for distroboot. + * We only want to swap the defaults and not any custom environment a + * user has set. We exit early if a changed boot_targets environment + * is detected. + */ +int setup_boottargets(void) +{ + const char *boot_device = + ofnode_read_chosen_string("u-boot,spl-boot-device"); + char *env_default, *env; + + if (!boot_device) { + debug("%s: /chosen/u-boot,spl-boot-device not set\n", + __func__); + return -1; + } + debug("%s: booted from %s\n", __func__, boot_device); + + env_default = env_get_default("boot_targets"); + env = env_get("boot_targets"); + if (!env) { + debug("%s: boot_targets does not exist\n", __func__); + return -1; + } + debug("%s: boot_targets current: %s - default: %s\n", + __func__, env, env_default); + + if (strcmp(env_default, env) != 0) { + debug("%s: boot_targets not default, don't change it\n", + __func__); + return 0; + } + + /* + * Make the default boot medium between SD Card and eMMC, the one that + * was used to load U-Boot proper. If SPI-NOR flash was used, keep + * original default order. + */ + struct udevice *devp; + + if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) { + debug("%s: not reordering boot_targets, bootdev %s != MMC\n", + __func__, boot_device); + return 0; + } + + char *mmc0, *mmc1; + + mmc0 = strstr(env, "mmc0"); + mmc1 = strstr(env, "mmc1"); + + if (!mmc0 || !mmc1) { + debug("%s: only one mmc boot_target found\n", __func__); + return -1; + } + + /* + * If mmc0 comes first in the boot order and U-Boot proper was + * loaded from mmc1, swap mmc0 and mmc1 in the list. + * If mmc1 comes first in the boot order and U-Boot proper was + * loaded from mmc0, swap mmc0 and mmc1 in the list. + */ + if ((mmc0 < mmc1 && devp->seq_ == 1) || + (mmc0 > mmc1 && devp->seq_ == 0)) { + mmc0[3] = '1'; + mmc1[3] = '0'; + debug("%s: set boot_targets to: %s\n", __func__, env); + env_set("boot_targets", env); + } + + return 0; +} + +int mmc_get_env_dev(void) +{ + const char *boot_device = + ofnode_read_chosen_string("u-boot,spl-boot-device"); + struct udevice *devp; + + if (!boot_device) { + debug("%s: /chosen/u-boot,spl-boot-device not set\n", + __func__); + return CONFIG_SYS_MMC_ENV_DEV; + } + + debug("%s: booted from %s\n", __func__, boot_device); + + if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) + return CONFIG_SYS_MMC_ENV_DEV; + + debug("%s: get MMC ENV from mmc%d\n", __func__, devp->seq_); + + return devp->seq_; +} + +enum env_location arch_env_get_location(enum env_operation op, int prio) +{ + const char *boot_device = + ofnode_read_chosen_string("u-boot,spl-boot-device"); + struct udevice *devp; + + if (prio > 0) + return ENVL_UNKNOWN; + + if (!boot_device) { + debug("%s: /chosen/u-boot,spl-boot-device not set\n", + __func__); + return ENVL_NOWHERE; + } + + debug("%s: booted from %s\n", __func__, boot_device); + + if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH) && + !uclass_find_device_by_ofnode(UCLASS_SPI_FLASH, ofnode_path(boot_device), &devp)) + return ENVL_SPI_FLASH; + + if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC) && + !uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) + return ENVL_MMC; + + printf("%s: No environment available: booted from %s but U-Boot config does not allow loading environment from it.", + __func__, boot_device); + + return ENVL_NOWHERE; +} diff --git a/board/theobroma-systems/common/common.h b/board/theobroma-systems/common/common.h new file mode 100644 index 00000000000..488313a2d86 --- /dev/null +++ b/board/theobroma-systems/common/common.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * (C) Copyright 2023 Theobroma Systems Design und Consulting GmbH + */ + +/* + * setup_boottargets() - Swap mmc0 and mmc1 in boot_targets depending on U-Boot + * proper load medium. + * + * If bootsource is uSD-card we can assume that we want to use the + * SD-Card instead of the eMMC as first boot_target for distroboot. + * We only want to swap the defaults and not any custom environment a + * user has set. We exit early if a changed boot_targets environment + * is detected. + * + * Return: + * 0 if OK, -1 otherwise + */ +int setup_boottargets(void); diff --git a/board/theobroma-systems/puma_rk3399/MAINTAINERS b/board/theobroma-systems/puma_rk3399/MAINTAINERS index 1ec2dd72d6c..93f570fc4f9 100644 --- a/board/theobroma-systems/puma_rk3399/MAINTAINERS +++ b/board/theobroma-systems/puma_rk3399/MAINTAINERS @@ -3,6 +3,7 @@ M: Quentin Schulz quentin.schulz@theobroma-systems.com M: Klaus Goger klaus.goger@theobroma-systems.com S: Maintained F: board/theobroma-systems/puma_rk3399 +F: board/theobroma-systems/common F: include/configs/puma_rk3399.h F: arch/arm/dts/rk3399-puma.dts F: configs/puma-rk3399_defconfig diff --git a/board/theobroma-systems/puma_rk3399/Makefile b/board/theobroma-systems/puma_rk3399/Makefile index d962b56f111..edd61a35c1e 100644 --- a/board/theobroma-systems/puma_rk3399/Makefile +++ b/board/theobroma-systems/puma_rk3399/Makefile @@ -5,3 +5,6 @@ #
obj-y += puma-rk3399.o +ifneq ($(CONFIG_SPL_BUILD),y) +obj-y += ../common/common.o +endif diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c index ac8ceedc7c9..a82f97b2d54 100644 --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c @@ -3,28 +3,15 @@ * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH */
-#include <common.h> #include <dm.h> -#include <env.h> -#include <env_internal.h> -#include <init.h> -#include <log.h> -#include <misc.h> -#include <spl.h> #include <syscon.h> -#include <u-boot/crc.h> -#include <usb.h> #include <dm/pinctrl.h> -#include <dm/uclass-internal.h> #include <asm/io.h> -#include <asm/setup.h> #include <asm/arch-rockchip/clock.h> #include <asm/arch-rockchip/hardware.h> #include <asm/arch-rockchip/grf_rk3399.h> -#include <asm/arch-rockchip/periph.h> #include <asm/arch-rockchip/misc.h> -#include <power/regulator.h> -#include <u-boot/sha256.h> +#include "../common/common.h"
static void setup_iodomain(void) { @@ -40,137 +27,6 @@ static void setup_iodomain(void) rk_setreg(&grf->io_vsel, 1 << GRF_IO_VSEL_GPIO4CD_SHIFT); }
-/* - * Swap mmc0 and mmc1 in boot_targets if booted from SD-Card. - * - * If bootsource is uSD-card we can assume that we want to use the - * SD-Card instead of the eMMC as first boot_target for distroboot. - * We only want to swap the defaults and not any custom environment a - * user has set. We exit early if a changed boot_targets environment - * is detected. - */ -static int setup_boottargets(void) -{ - const char *boot_device = - ofnode_read_chosen_string("u-boot,spl-boot-device"); - char *env_default, *env; - - if (!boot_device) { - debug("%s: /chosen/u-boot,spl-boot-device not set\n", - __func__); - return -1; - } - debug("%s: booted from %s\n", __func__, boot_device); - - env_default = env_get_default("boot_targets"); - env = env_get("boot_targets"); - if (!env) { - debug("%s: boot_targets does not exist\n", __func__); - return -1; - } - debug("%s: boot_targets current: %s - default: %s\n", - __func__, env, env_default); - - if (strcmp(env_default, env) != 0) { - debug("%s: boot_targets not default, don't change it\n", - __func__); - return 0; - } - - /* - * Make the default boot medium between SD Card and eMMC, the one that - * was used to load U-Boot proper. If SPI-NOR flash was used, keep - * original default order. - */ - struct udevice *devp; - - if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) { - debug("%s: not reordering boot_targets, bootdev %s != MMC\n", - __func__, boot_device); - return 0; - } - - char *mmc0, *mmc1; - - mmc0 = strstr(env, "mmc0"); - mmc1 = strstr(env, "mmc1"); - - if (!mmc0 || !mmc1) { - debug("%s: only one mmc boot_target found\n", __func__); - return -1; - } - - /* - * If mmc0 comes first in the boot order and U-Boot proper was - * loaded from mmc1, swap mmc0 and mmc1 in the list. - * If mmc1 comes first in the boot order and U-Boot proper was - * loaded from mmc0, swap mmc0 and mmc1 in the list. - */ - if ((mmc0 < mmc1 && devp->seq_ == 1) || - (mmc0 > mmc1 && devp->seq_ == 0)) { - mmc0[3] = '1'; - mmc1[3] = '0'; - debug("%s: set boot_targets to: %s\n", __func__, env); - env_set("boot_targets", env); - } - - return 0; -} - -int mmc_get_env_dev(void) -{ - const char *boot_device = - ofnode_read_chosen_string("u-boot,spl-boot-device"); - struct udevice *devp; - - if (!boot_device) { - debug("%s: /chosen/u-boot,spl-boot-device not set\n", - __func__); - return CONFIG_SYS_MMC_ENV_DEV; - } - - debug("%s: booted from %s\n", __func__, boot_device); - - if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) - return CONFIG_SYS_MMC_ENV_DEV; - - debug("%s: get MMC ENV from mmc%d\n", __func__, devp->seq_); - - return devp->seq_; -} - -enum env_location arch_env_get_location(enum env_operation op, int prio) -{ - const char *boot_device = - ofnode_read_chosen_string("u-boot,spl-boot-device"); - struct udevice *devp; - - if (prio > 0) - return ENVL_UNKNOWN; - - if (!boot_device) { - debug("%s: /chosen/u-boot,spl-boot-device not set\n", - __func__); - return ENVL_NOWHERE; - } - - debug("%s: booted from %s\n", __func__, boot_device); - - if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH) && - !uclass_find_device_by_ofnode(UCLASS_SPI_FLASH, ofnode_path(boot_device), &devp)) - return ENVL_SPI_FLASH; - - if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC) && - !uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) - return ENVL_MMC; - - printf("%s: No environment available: booted from %s but U-Boot " - "config does not allow loading environment from it.", - __func__, boot_device); - - return ENVL_NOWHERE; -} - int misc_init_r(void) { const u32 cpuid_offset = 0x7; diff --git a/board/theobroma-systems/ringneck_px30/MAINTAINERS b/board/theobroma-systems/ringneck_px30/MAINTAINERS index d764e26066c..06e1beaab14 100644 --- a/board/theobroma-systems/ringneck_px30/MAINTAINERS +++ b/board/theobroma-systems/ringneck_px30/MAINTAINERS @@ -3,6 +3,7 @@ M: Quentin Schulz quentin.schulz@theobroma-systems.com M: Klaus Goger klaus.goger@theobroma-systems.com S: Maintained F: board/theobroma-systems/ringneck_px30 +F: board/theobroma-systems/common F: include/configs/ringneck_px30.h F: arch/arm/dts/px30-ringneck* F: configs/ringneck-px30_defconfig diff --git a/board/theobroma-systems/ringneck_px30/Makefile b/board/theobroma-systems/ringneck_px30/Makefile index 31ada1a6942..45cc65baf9a 100644 --- a/board/theobroma-systems/ringneck_px30/Makefile +++ b/board/theobroma-systems/ringneck_px30/Makefile @@ -5,3 +5,6 @@ #
obj-y += ringneck-px30.o +ifneq ($(CONFIG_SPL_BUILD),y) +obj-y += ../common/common.o +endif diff --git a/board/theobroma-systems/ringneck_px30/ringneck-px30.c b/board/theobroma-systems/ringneck_px30/ringneck-px30.c index fb5570767db..ff7e414303d 100644 --- a/board/theobroma-systems/ringneck_px30/ringneck-px30.c +++ b/board/theobroma-systems/ringneck_px30/ringneck-px30.c @@ -3,155 +3,10 @@ * (C) Copyright 2022 Theobroma Systems Design und Consulting GmbH */
-#include <common.h> -#include <dm.h> -#include <env.h> -#include <env_internal.h> -#include <init.h> -#include <log.h> -#include <misc.h> -#include <spl.h> -#include <syscon.h> -#include <u-boot/crc.h> -#include <usb.h> -#include <dm/pinctrl.h> -#include <dm/uclass-internal.h> #include <asm/gpio.h> -#include <asm/io.h> -#include <asm/setup.h> -#include <asm/arch-rockchip/clock.h> -#include <asm/arch-rockchip/hardware.h> -#include <asm/arch-rockchip/periph.h> #include <asm/arch-rockchip/misc.h> #include <linux/delay.h> -#include <power/regulator.h> -#include <u-boot/sha256.h> - -/* - * Swap mmc0 and mmc1 in boot_targets if booted from SD-Card. - * - * If bootsource is uSD-card we can assume that we want to use the - * SD-Card instead of the eMMC as first boot_target for distroboot. - * We only want to swap the defaults and not any custom environment a - * user has set. We exit early if a changed boot_targets environment - * is detected. - */ -static int setup_boottargets(void) -{ - const char *boot_device = - ofnode_read_chosen_string("u-boot,spl-boot-device"); - char *env_default, *env; - - if (!boot_device) { - debug("%s: /chosen/u-boot,spl-boot-device not set\n", - __func__); - return -1; - } - debug("%s: booted from %s\n", __func__, boot_device); - - env_default = env_get_default("boot_targets"); - env = env_get("boot_targets"); - if (!env) { - debug("%s: boot_targets does not exist\n", __func__); - return -1; - } - debug("%s: boot_targets current: %s - default: %s\n", - __func__, env, env_default); - - if (strcmp(env_default, env) != 0) { - debug("%s: boot_targets not default, don't change it\n", - __func__); - return 0; - } - - /* - * Make the default boot medium between SD Card and eMMC, the one that - * was used to load U-Boot proper. - */ - struct udevice *devp; - - if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) { - debug("%s: not reordering boot_targets, bootdev %s != MMC\n", - __func__, boot_device); - return 0; - } - - char *mmc0, *mmc1; - - mmc0 = strstr(env, "mmc0"); - mmc1 = strstr(env, "mmc1"); - - if (!mmc0 || !mmc1) { - debug("%s: only one mmc boot_target found\n", __func__); - return -1; - } - - /* - * If mmc0 comes first in the boot order and U-Boot proper was - * loaded from mmc1, swap mmc0 and mmc1 in the list. - * If mmc1 comes first in the boot order and U-Boot proper was - * loaded from mmc0, swap mmc0 and mmc1 in the list. - */ - if ((mmc0 < mmc1 && devp->seq_ == 1) || - (mmc0 > mmc1 && devp->seq_ == 0)) { - mmc0[3] = '1'; - mmc1[3] = '0'; - debug("%s: set boot_targets to: %s\n", __func__, env); - env_set("boot_targets", env); - } - - return 0; -} - -int mmc_get_env_dev(void) -{ - const char *boot_device = - ofnode_read_chosen_string("u-boot,spl-boot-device"); - struct udevice *devp; - - if (!boot_device) { - debug("%s: /chosen/u-boot,spl-boot-device not set\n", - __func__); - return CONFIG_SYS_MMC_ENV_DEV; - } - - debug("%s: booted from %s\n", __func__, boot_device); - - if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) - return CONFIG_SYS_MMC_ENV_DEV; - - debug("%s: get MMC ENV from mmc%d\n", __func__, devp->seq_); - - return devp->seq_; -} - -enum env_location arch_env_get_location(enum env_operation op, int prio) -{ - const char *boot_device = - ofnode_read_chosen_string("u-boot,spl-boot-device"); - struct udevice *devp; - - if (prio > 0) - return ENVL_UNKNOWN; - - if (!boot_device) { - debug("%s: /chosen/u-boot,spl-boot-device not set\n", - __func__); - return ENVL_NOWHERE; - } - - debug("%s: booted from %s\n", __func__, boot_device); - - if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC) && - !uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), &devp)) - return ENVL_MMC; - - printf("%s: No environment available: booted from %s but U-Boot " - "config does not allow loading environment from it.", - __func__, boot_device); - - return ENVL_NOWHERE; -} +#include "../common/common.h"
int misc_init_r(void) {
From: Quentin Schulz quentin.schulz@theobroma-systems.com
env_get_default suffers from a particular issue int that it can only return a value truncated to gd->env_buf (32) characters. This may be enough for most variables but it isn't for others, so let's allow users to provide a preallocated buffer to copy the value into instead, allowing for more control, though it'll still be truncated if the value size is bigger than the preallocated buffer.
Cc: Quentin Schulz foss+uboot@0leil.net Reviewed-by: Tom Rini trini@konsulko.com Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com --- env/common.c | 8 ++++++++ include/env.h | 10 ++++++++++ 2 files changed, 18 insertions(+)
diff --git a/env/common.c b/env/common.c index eb1a9137953..307003099dd 100644 --- a/env/common.c +++ b/env/common.c @@ -254,6 +254,14 @@ char *env_get_default(const char *name) return NULL; }
+/* + * Look up the variable from the default environment and store its value in buf + */ +int env_get_default_into(const char *name, char *buf, unsigned int len) +{ + return env_get_from_linear(default_environment, name, buf, len); +} + void env_set_default(const char *s, int flags) { if (s) { diff --git a/include/env.h b/include/env.h index 430c4fa94a4..9a406de3781 100644 --- a/include/env.h +++ b/include/env.h @@ -348,6 +348,16 @@ int env_import_redund(const char *buf1, int buf1_read_fail, */ char *env_get_default(const char *name);
+/** + * env_get_default_into() - Look up a variable from the default environment and + * copy its value in buf. + * + * @name: Variable to look up + * Return: actual length of the variable value excluding the terminating + * NULL-byte, or -1 if the variable is not found + */ +int env_get_default_into(const char *name, char *buf, unsigned int len); + /* [re]set to the default environment */ void env_set_default(const char *s, int flags);
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Since both functions share a similar goal and env_get_default_into can do what env_get_default wants to do with specific arguments, let's make env_get_default call env_get_default_into so as to avoid code duplication.
Cc: Quentin Schulz foss+uboot@0leil.net Reviewed-by: Tom Rini trini@konsulko.com Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com --- env/common.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/env/common.c b/env/common.c index 307003099dd..930bdef2f58 100644 --- a/env/common.c +++ b/env/common.c @@ -246,9 +246,11 @@ bool env_get_autostart(void) */ char *env_get_default(const char *name) { - if (env_get_from_linear(default_environment, name, - (char *)(gd->env_buf), - sizeof(gd->env_buf)) >= 0) + int ret; + + ret = env_get_default_into(name, (char *)(gd->env_buf), + sizeof(gd->env_buf)); + if (ret >= 0) return (char *)(gd->env_buf);
return NULL;
From: Quentin Schulz quentin.schulz@theobroma-systems.com
U-Boot proper automatically modifies boot_targets to swap the order in which MMC storage media are used for standard boot based on which MMC storage medium was used to load U-Boot proper. This is however only done if the user has not changed it manually, therefore a check between the default and current value is done.
This used to work fine until the migration to standard boot where boot_targets value size in the default environment went above the 32 characters that env_get_default function can return, thus resulting in a truncated variable.
Therefore the check between default and current value would always fail.
By using the newly added env_get_default_into function, a buffer of the appropriate size can be allocated on the stack to get the whole value of boot_targets in the default environment and thus fixing the check.
Cc: Quentin Schulz foss+uboot@0leil.net Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com --- board/theobroma-systems/common/common.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/board/theobroma-systems/common/common.c b/board/theobroma-systems/common/common.c index f4b854554ea..3ddc2351eee 100644 --- a/board/theobroma-systems/common/common.c +++ b/board/theobroma-systems/common/common.c @@ -21,7 +21,9 @@ int setup_boottargets(void) { const char *boot_device = ofnode_read_chosen_string("u-boot,spl-boot-device"); - char *env_default, *env; + char env_default[sizeof(BOOT_TARGETS)]; + char *env; + int ret;
if (!boot_device) { debug("%s: /chosen/u-boot,spl-boot-device not set\n", @@ -30,7 +32,9 @@ int setup_boottargets(void) } debug("%s: booted from %s\n", __func__, boot_device);
- env_default = env_get_default("boot_targets"); + ret = env_get_default_into("boot_targets", env_default, sizeof(env_default)); + if (ret < 0) + env_default[0] = '\0'; env = env_get("boot_targets"); if (!env) { debug("%s: boot_targets does not exist\n", __func__);
participants (1)
-
Quentin Schulz