[PATCH v4 00/16] Bug-fixes for a few boards

This series includes fixes to get some rockchip and nvidia boards working again. It also drops the broken Beaglebone Black config and provides a devicetree fix for coral (x86).
Note that since this series fixes bugs, it is targeted at -master
Changes in v4: - Drop Fixes tag - Add new patch to set a board-size limit for chromebook_link - Drop the non-dcache optimisation, since the cache should normally be on - Move Binman size feature to a separate series
Changes in v3: - Use BLOBLIST instead of OF_BLOBLIST - Cut the patch down to bare bones - Split out the refactoring into a separate patch
Changes in v2: - Put the conditions under EFI_TCG2_PROTOCOL - Consider MEASURED_BOOT too - Remove the superfluous if() and drop the debug() as well - Use 'phase' instead of 'stage' - Add new patch to correct memory size in SPL - Drop patch "regulator: rk8xx: Fix incorrect parameter" - Rewrite boneblack patch to onstead drop the target and update docs
Simon Glass (16): mkeficapsule: Add a --version argument binman: Collect the version number for mkeficapsule binman: Deal with mkeficapsule being missing binman: Return failure when a usage() message is generated binman: Keep the efi_capsule input file nvidia: nyan-big: Disable debug UART tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL x86: Set a board-size limit for chromebook_link rockchip: veyron: Add logging for power init power: regulator: Handle autoset in regulators_enable_boot_on() fdt: Correct condition for bloblist existing spl: Allow ATF to work when dcache is disabled rockchip: Ensure memory size is available in RK3399 SPL rockchip: Avoid #ifdefs in RK3399 SPL rockchip: bob: kevin: Disable dcache in SPL Drop the special am335x_boneblack_vboot target
board/google/veyron/veyron.c | 30 +++---- board/ti/am335x/MAINTAINERS | 1 - boot/Kconfig | 4 + common/spl/spl_atf.c | 3 +- configs/am335x_boneblack_vboot_defconfig | 94 ---------------------- configs/am335x_evm_defconfig | 3 +- configs/chromebook_bob_defconfig | 1 + configs/chromebook_kevin_defconfig | 1 + configs/chromebook_link_defconfig | 2 + configs/nyan-big_defconfig | 1 - doc/mkeficapsule.1 | 4 + doc/usage/fit/beaglebone_vboot.rst | 21 +++-- drivers/power/regulator/regulator-uclass.c | 2 +- drivers/ram/rockchip/sdram_rk3399.c | 44 +++++----- lib/Kconfig | 4 - lib/fdtdec.c | 12 ++- tools/binman/btool/mkeficapsule.py | 3 +- tools/binman/etype/efi_capsule.py | 5 +- tools/mkeficapsule.c | 10 ++- 19 files changed, 86 insertions(+), 159 deletions(-) delete mode 100644 configs/am335x_boneblack_vboot_defconfig

Tools should have an option to obtain the version, so add this to the mkeficapsule tool.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
doc/mkeficapsule.1 | 4 ++++ tools/mkeficapsule.c | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1 index c4c2057d5c7..c3d0f21488a 100644 --- a/doc/mkeficapsule.1 +++ b/doc/mkeficapsule.1 @@ -87,6 +87,10 @@ Generate a firmware revert empty capsule .BI "-o\fR,\fB --capoemflag " Capsule OEM flag, value between 0x0000 to 0xffff
+.TP +.BR -V ", " --version +Print version information and exit. + .TP .BR -h ", " --help Print a help message diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c index 6a261ff549d..c112ae2de8d 100644 --- a/tools/mkeficapsule.c +++ b/tools/mkeficapsule.c @@ -21,6 +21,8 @@ #include <gnutls/pkcs7.h> #include <gnutls/abstract.h>
+#include <version.h> + #include "eficapsule.h"
static const char *tool_name = "mkeficapsule"; @@ -28,7 +30,7 @@ static const char *tool_name = "mkeficapsule"; efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
-static const char *opts_short = "g:i:I:v:p:c:m:o:dhARD"; +static const char *opts_short = "g:i:I:v:p:c:m:o:dhARDV";
enum { CAPSULE_NORMAL_BLOB = 0, @@ -70,6 +72,7 @@ static void print_usage(void) "\t-R, --fw-revert firmware revert capsule, takes no GUID, no image blob\n" "\t-o, --capoemflag Capsule OEM Flag, an integer between 0x0000 and 0xffff\n" "\t-D, --dump-capsule dump the contents of the capsule headers\n" + "\t-V, --version show version number\n" "\t-h, --help print a help message\n", tool_name); } @@ -969,6 +972,9 @@ int main(int argc, char **argv) case 'D': capsule_dump = true; break; + case 'V': + printf("mkeficapsule version %s\n", PLAIN_VERSION); + exit(EXIT_SUCCESS); default: print_usage(); exit(EXIT_SUCCESS);

Now that this tool has a version number, collect it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/btool/mkeficapsule.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/binman/btool/mkeficapsule.py b/tools/binman/btool/mkeficapsule.py index ef1da638df1..f7e5a886849 100644 --- a/tools/binman/btool/mkeficapsule.py +++ b/tools/binman/btool/mkeficapsule.py @@ -33,7 +33,8 @@ class Bintoolmkeficapsule(bintool.Bintool): commandline, or through a config file. """ def __init__(self, name): - super().__init__(name, 'mkeficapsule tool for generating capsules') + super().__init__(name, 'mkeficapsule tool for generating capsules', + r'mkeficapsule version (.*)')
def generate_capsule(self, image_index, image_guid, hardware_instance, payload, output_fname, priv_key, pub_key,

Tools cannot be assumed to be present. Add a check for this with the mkeficpasule tool.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: b617611b27a ("binman: capsule: Add support for generating...") ---
(no changes since v1)
tools/binman/etype/efi_capsule.py | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py index e3203717822..47da5da324b 100644 --- a/tools/binman/etype/efi_capsule.py +++ b/tools/binman/etype/efi_capsule.py @@ -150,6 +150,10 @@ class Entry_efi_capsule(Entry_section): if ret is not None: os.remove(payload) return tools.read_file(capsule_fname) + else: + # Bintool is missing; just use the input data as the output + self.record_missing_bintool(self.mkeficapsule) + return data
def AddBintools(self, btools): self.mkeficapsule = self.AddBintool(btools, 'mkeficapsule')

The tool must return an error code when invalid arguments are provided, otherwise binman has no way of knowing that anything went wrong.
Correct this.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: fab430be2f4 ("tools: add mkeficapsule command for UEFI...") ---
(no changes since v1)
tools/mkeficapsule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c index c112ae2de8d..f28008a0829 100644 --- a/tools/mkeficapsule.c +++ b/tools/mkeficapsule.c @@ -977,7 +977,7 @@ int main(int argc, char **argv) exit(EXIT_SUCCESS); default: print_usage(); - exit(EXIT_SUCCESS); + exit(EXIT_FAILURE); } }

There is no need to remove input files. It makes it harder to diagnose failures. Keep the payload file.
There is no test for this condition, but one could be added.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Sughosh Ganu sughosh.ganu@linaro.org ---
Changes in v4: - Drop Fixes tag
tools/binman/etype/efi_capsule.py | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py index 47da5da324b..03e55cbc4d9 100644 --- a/tools/binman/etype/efi_capsule.py +++ b/tools/binman/etype/efi_capsule.py @@ -148,7 +148,6 @@ class Entry_efi_capsule(Entry_section): self.fw_version, self.oem_flags) if ret is not None: - os.remove(payload) return tools.read_file(capsule_fname) else: # Bintool is missing; just use the input data as the output

This cannot be enabled early in boot since some other init is needed. At this point it is unclear exactly what init is needed, so disable the debug UART to avoid a hang.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
configs/nyan-big_defconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/configs/nyan-big_defconfig b/configs/nyan-big_defconfig index 1483d17d975..4dec710cf8d 100644 --- a/configs/nyan-big_defconfig +++ b/configs/nyan-big_defconfig @@ -17,7 +17,6 @@ CONFIG_TEGRA124=y CONFIG_TARGET_NYAN_BIG=y CONFIG_TEGRA_GPU=y CONFIG_SYS_LOAD_ADDR=0x82408000 -CONFIG_DEBUG_UART=y CONFIG_FIT=y CONFIG_FIT_BEST_MATCH=y CONFIG_BOOTSTAGE=y

It does not make sense to enable all SHA algorithms unless they are needed. It bloats the code and in this case, causes chromebook_link to fail to build. That board does use the TPM, but not with measured boot, nor EFI.
Since EFI_TCG2_PROTOCOL already selects these options, we just need to add them to MEASURED_BOOT as well.
Note that the original commit combines refactoring and new features, which makes it hard to see what is going on.
Fixes: 97707f12fda tpm: Support boot measurements Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Put the conditions under EFI_TCG2_PROTOCOL - Consider MEASURED_BOOT too
boot/Kconfig | 4 ++++ lib/Kconfig | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig index 6f3096c15a6..b061891e109 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT config MEASURED_BOOT bool "Measure boot images and configuration when booting without EFI" depends on HASH && TPM_V2 + select SHA1 + select SHA256 + select SHA384 + select SHA512 help This option enables measurement of the boot process when booting without UEFI . Measurement involves creating cryptographic hashes diff --git a/lib/Kconfig b/lib/Kconfig index 189e6eb31aa..568892fce44 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -438,10 +438,6 @@ config TPM bool "Trusted Platform Module (TPM) Support" depends on DM imply DM_RNG - select SHA1 - select SHA256 - select SHA384 - select SHA512 help This enables support for TPMs which can be used to provide security features for your board. The TPM can be connected via LPC or I2C

Set a size limit for this board so that we get a build error if it grows too much.
Note that the limit is approximately, since it does not include the FDT, microcode and fdtmap, which can change in size. However this board is fairly stable, so overflowing this limit will likely result in the image not fitting in the ROM space available for U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v4: - Add new patch to set a board-size limit for chromebook_link
configs/chromebook_link_defconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig index a9f91dd9b26..1a72fd178a8 100644 --- a/configs/chromebook_link_defconfig +++ b/configs/chromebook_link_defconfig @@ -15,6 +15,8 @@ CONFIG_DEBUG_UART=y CONFIG_HAVE_MRC=y CONFIG_SMP=y CONFIG_HAVE_VGA_BIOS=y +CONFIG_HAS_BOARD_SIZE_LIMIT=y +CONFIG_BOARD_SIZE_LIMIT=630000 CONFIG_FIT=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y

Add better logging for power init so that CONFIG_LOG_ERROR_RETURN can be enabled.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Quentin Schulz quentin.schulz@cherry.de ---
(no changes since v2)
Changes in v2: - Remove the superfluous if() and drop the debug() as well
board/google/veyron/veyron.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/board/google/veyron/veyron.c b/board/google/veyron/veyron.c index 32dbcdc4d10..6d4c9debdee 100644 --- a/board/google/veyron/veyron.c +++ b/board/google/veyron/veyron.c @@ -29,44 +29,38 @@ static int veyron_init(void) int ret;
ret = regulator_get_by_platname("vdd_arm", &dev); - if (ret) { - debug("Cannot set regulator name\n"); - return ret; - } + if (ret) + return log_msg_ret("vdd", ret);
/* Slowly raise to max CPU voltage to prevent overshoot */ ret = regulator_set_value(dev, 1200000); if (ret) - return ret; + return log_msg_ret("s12", ret); udelay(175); /* Must wait for voltage to stabilize, 2mV/us */ ret = regulator_set_value(dev, 1400000); if (ret) - return ret; + return log_msg_ret("s14", ret); udelay(100); /* Must wait for voltage to stabilize, 2mV/us */
ret = rockchip_get_clk(&clk.dev); if (ret) - return ret; + return log_msg_ret("clk", ret); clk.id = PLL_APLL; ret = clk_set_rate(&clk, 1800000000); if (IS_ERR_VALUE(ret)) - return ret; + return log_msg_ret("s18", ret);
ret = regulator_get_by_platname("vcc33_sd", &dev); - if (ret) { - debug("Cannot get regulator name\n"); - return ret; - } + if (ret) + return log_msg_ret("vcc", ret);
ret = regulator_set_value(dev, 3300000); if (ret) - return ret; + return log_msg_ret("s33", ret);
ret = regulators_enable_boot_on(false); - if (ret) { - debug("%s: Cannot enable boot on regulators\n", __func__); - return ret; - } + if (ret) + return log_msg_ret("boo", ret);
return 0; } @@ -81,7 +75,7 @@ int board_early_init_r(void) if (!fdt_node_check_compatible(gd->fdt_blob, 0, "google,veyron")) { ret = veyron_init(); if (ret) - return ret; + return log_msg_ret("vey", ret); } #endif /*

With a recent change, regulators_enable_boot_on() returns an error if a regulator is already set. Check for and handle this situation.
Fixes: d99fb64a98a power: regulator: Only run autoset once for each regulator Reviewed-by: Jonas Karlman jonas@kwiboo.se Reviewed-by: Quentin Schulz quentin.schulz@cherry.de
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
drivers/power/regulator/regulator-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 77d101f262e..d9e1fb68295 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -518,7 +518,7 @@ int regulators_enable_boot_on(bool verbose) dev; uclass_next_device(&dev)) { ret = regulator_autoset(dev); - if (ret == -EMEDIUMTYPE) { + if (ret == -EMEDIUMTYPE || ret == -EALREADY) { ret = 0; continue; }

On some boards, the bloblist is created in SPL once SDRAM is ready. It cannot be accessed until that point, so is not available early in SPL.
Add a condition to avoid a hang in this case.
This fixes a hang in chromebook_coral
Fixes: 70fe2385943 ("fdt: Allow the devicetree to come from a bloblist")
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v3)
Changes in v3: - Use BLOBLIST instead of OF_BLOBLIST
Changes in v2: - Use 'phase' instead of 'stage'
lib/fdtdec.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index b2c59ab3818..e16d1819449 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1669,8 +1669,16 @@ int fdtdec_setup(void) { int ret = -ENOENT;
- /* If allowing a bloblist, check that first */ - if (CONFIG_IS_ENABLED(BLOBLIST)) { + /* + * If allowing a bloblist, check that first. This would be better + * handled with an OF_BLOBLIST Kconfig, but that caused far too much + * argument, so add a hack here, used e.g. by chromebook_coral + * The necessary test is whether the previous phase passed a bloblist, + * not whether this phase creates one. + */ + if (CONFIG_IS_ENABLED(BLOBLIST) && + (spl_prev_phase() != PHASE_TPL || + !IS_ENABLED(CONFIG_TPL_BLOBLIST))) { ret = bloblist_maybe_init(); if (!ret) { gd->fdt_blob = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);

The dcache may not be enabled in SPL. Add a check to avoid trying to use an undefined function.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com ---
(no changes since v1)
common/spl/spl_atf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c index 3bdd013a35f..9afe6456bc4 100644 --- a/common/spl/spl_atf.c +++ b/common/spl/spl_atf.c @@ -204,7 +204,8 @@ static void __noreturn bl31_entry(uintptr_t bl31_entry, uintptr_t bl32_entry, fdt_addr);
raw_write_daif(SPSR_EXCEPTION_MASK); - dcache_disable(); + if (!CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) + dcache_disable();
atf_entry(bl31_params, (void *)fdt_addr); }

At present gd->ram_size is 0 in SPL, meaning that it is not possible to enable the cache. Correct this by always populating the RAM size correctly.
This increases code size by about 500 bytes in SPL, since it must call the rather large rockchip_sdram_size() function.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v3)
Changes in v3: - Cut the patch down to bare bones
Changes in v2: - Add new patch to correct memory size in SPL
drivers/ram/rockchip/sdram_rk3399.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index 02cc4a38cf0..3c4e20f4e80 100644 --- a/drivers/ram/rockchip/sdram_rk3399.c +++ b/drivers/ram/rockchip/sdram_rk3399.c @@ -3142,19 +3142,19 @@ static int rk3399_dmc_init(struct udevice *dev)
static int rk3399_dmc_probe(struct udevice *dev) { + struct dram_info *priv = dev_get_priv(dev); + #if defined(CONFIG_TPL_BUILD) || \ (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) if (rk3399_dmc_init(dev)) return 0; -#else - struct dram_info *priv = dev_get_priv(dev); - +#endif priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF); debug("%s: pmugrf = %p\n", __func__, priv->pmugrf); priv->info.base = CFG_SYS_SDRAM_BASE; priv->info.size = rockchip_sdram_size((phys_addr_t)&priv->pmugrf->os_reg2); -#endif + return 0; }

Hi Simon,
On 6/23/24 7:52 PM, Simon Glass wrote:
At present gd->ram_size is 0 in SPL, meaning that it is not possible to enable the cache. Correct this by always populating the RAM size correctly.
This increases code size by about 500 bytes in SPL, since it must call the rather large rockchip_sdram_size() function.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Quentin Schulz quentin.schulz@cherry.de
Thanks! Quentin

The code here is confusing due to large blocks which are #ifdefed out. Add a function phase_sdram_init() which returns whether SDRAM init should happen in the current phase, using that as needed to control the code flow.
This increases code size by about 500 bytes in SPL when the cache is on, since it must call the rather large rockchip_sdram_size() function.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v4: - Drop the non-dcache optimisation, since the cache should normally be on
Changes in v3: - Split out the refactoring into a separate patch
drivers/ram/rockchip/sdram_rk3399.c | 42 +++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index 3c4e20f4e80..949a082d00c 100644 --- a/drivers/ram/rockchip/sdram_rk3399.c +++ b/drivers/ram/rockchip/sdram_rk3399.c @@ -13,6 +13,7 @@ #include <log.h> #include <ram.h> #include <regmap.h> +#include <spl.h> #include <syscon.h> #include <asm/arch-rockchip/clock.h> #include <asm/arch-rockchip/cru.h> @@ -63,8 +64,6 @@ struct chan_info { };
struct dram_info { -#if defined(CONFIG_TPL_BUILD) || \ - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) u32 pwrup_srefresh_exit[2]; struct chan_info chan[2]; struct clk ddr_clk; @@ -75,7 +74,6 @@ struct dram_info { struct rk3399_pmusgrf_regs *pmusgrf; struct rk3399_ddr_cic_regs *cic; const struct sdram_rk3399_ops *ops; -#endif struct ram_info info; struct rk3399_pmugrf_regs *pmugrf; }; @@ -92,9 +90,6 @@ struct sdram_rk3399_ops { struct rk3399_sdram_params *params); };
-#if defined(CONFIG_TPL_BUILD) || \ - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) - struct rockchip_dmc_plat { #if CONFIG_IS_ENABLED(OF_PLATDATA) struct dtd_rockchip_rk3399_dmc dtplat; @@ -191,6 +186,17 @@ struct io_setting { }, };
+/** + * phase_sdram_init() - Check if this is the phase where SDRAM init happens + * + * Returns: true to do SDRAM init in this phase, false to not + */ +static bool phase_sdram_init(void) +{ + return spl_phase() == PHASE_TPL || + (!IS_ENABLED(CONFIG_TPL) && !spl_in_proper()); +} + static struct io_setting * lpddr4_get_io_settings(const struct rk3399_sdram_params *params, u32 mr5) { @@ -3024,7 +3030,7 @@ static int rk3399_dmc_of_to_plat(struct udevice *dev) struct rockchip_dmc_plat *plat = dev_get_plat(dev); int ret;
- if (!CONFIG_IS_ENABLED(OF_REAL)) + if (!CONFIG_IS_ENABLED(OF_REAL) || !phase_sdram_init()) return 0;
ret = dev_read_u32_array(dev, "rockchip,sdram-params", @@ -3138,22 +3144,21 @@ static int rk3399_dmc_init(struct udevice *dev)
return 0; } -#endif
static int rk3399_dmc_probe(struct udevice *dev) { struct dram_info *priv = dev_get_priv(dev);
-#if defined(CONFIG_TPL_BUILD) || \ - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) - if (rk3399_dmc_init(dev)) - return 0; -#endif - priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF); - debug("%s: pmugrf = %p\n", __func__, priv->pmugrf); + if (phase_sdram_init()) { + if (rk3399_dmc_init(dev)) + return 0; + } else { + priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF); + debug("%s: pmugrf = %p\n", __func__, priv->pmugrf); + } + priv->info.base = CFG_SYS_SDRAM_BASE; - priv->info.size = - rockchip_sdram_size((phys_addr_t)&priv->pmugrf->os_reg2); + priv->info.size = rockchip_sdram_size((ulong)&priv->pmugrf->os_reg2);
return 0; } @@ -3181,10 +3186,7 @@ U_BOOT_DRIVER(dmc_rk3399) = { .id = UCLASS_RAM, .of_match = rk3399_dmc_ids, .ops = &rk3399_dmc_ops, -#if defined(CONFIG_TPL_BUILD) || \ - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) .of_to_plat = rk3399_dmc_of_to_plat, -#endif .probe = rk3399_dmc_probe, .priv_auto = sizeof(struct dram_info), #if defined(CONFIG_TPL_BUILD) || \

Hi Simon,
On 6/23/24 7:53 PM, Simon Glass wrote:
The code here is confusing due to large blocks which are #ifdefed out. Add a function phase_sdram_init() which returns whether SDRAM init should happen in the current phase, using that as needed to control the code flow.
This increases code size by about 500 bytes in SPL when the cache is on, since it must call the rather large rockchip_sdram_size() function.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Drop the non-dcache optimisation, since the cache should normally be on
Changes in v3:
Split out the refactoring into a separate patch
drivers/ram/rockchip/sdram_rk3399.c | 42 +++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index 3c4e20f4e80..949a082d00c 100644 --- a/drivers/ram/rockchip/sdram_rk3399.c +++ b/drivers/ram/rockchip/sdram_rk3399.c @@ -13,6 +13,7 @@ #include <log.h> #include <ram.h> #include <regmap.h> +#include <spl.h> #include <syscon.h> #include <asm/arch-rockchip/clock.h> #include <asm/arch-rockchip/cru.h> @@ -63,8 +64,6 @@ struct chan_info { };
struct dram_info { -#if defined(CONFIG_TPL_BUILD) || \
- (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) u32 pwrup_srefresh_exit[2]; struct chan_info chan[2]; struct clk ddr_clk;
@@ -75,7 +74,6 @@ struct dram_info { struct rk3399_pmusgrf_regs *pmusgrf; struct rk3399_ddr_cic_regs *cic; const struct sdram_rk3399_ops *ops; -#endif struct ram_info info; struct rk3399_pmugrf_regs *pmugrf; }; @@ -92,9 +90,6 @@ struct sdram_rk3399_ops { struct rk3399_sdram_params *params); };
-#if defined(CONFIG_TPL_BUILD) || \
- (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
- struct rockchip_dmc_plat { #if CONFIG_IS_ENABLED(OF_PLATDATA) struct dtd_rockchip_rk3399_dmc dtplat;
@@ -191,6 +186,17 @@ struct io_setting { }, };
+/**
- phase_sdram_init() - Check if this is the phase where SDRAM init happens
- Returns: true to do SDRAM init in this phase, false to not
- */
+static bool phase_sdram_init(void) +{
- return spl_phase() == PHASE_TPL ||
(!IS_ENABLED(CONFIG_TPL) && !spl_in_proper());
+}
- static struct io_setting * lpddr4_get_io_settings(const struct rk3399_sdram_params *params, u32 mr5) {
@@ -3024,7 +3030,7 @@ static int rk3399_dmc_of_to_plat(struct udevice *dev) struct rockchip_dmc_plat *plat = dev_get_plat(dev); int ret;
- if (!CONFIG_IS_ENABLED(OF_REAL))
if (!CONFIG_IS_ENABLED(OF_REAL) || !phase_sdram_init()) return 0;
ret = dev_read_u32_array(dev, "rockchip,sdram-params",
@@ -3138,22 +3144,21 @@ static int rk3399_dmc_init(struct udevice *dev)
return 0; } -#endif
static int rk3399_dmc_probe(struct udevice *dev) { struct dram_info *priv = dev_get_priv(dev);
-#if defined(CONFIG_TPL_BUILD) || \
- (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
- if (rk3399_dmc_init(dev))
return 0;
-#endif
- priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
- debug("%s: pmugrf = %p\n", __func__, priv->pmugrf);
- if (phase_sdram_init()) {
if (rk3399_dmc_init(dev))
return 0;
- } else {
priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
debug("%s: pmugrf = %p\n", __func__, priv->pmugrf);
- }
I'm not sure we need to put the debug message into an else as this pmugrf is also used when dmc_init passes.
Additionally, we could also just set priv->pmugrf before the if condition and remove it from dmc_init if we really wanted to.
priv->info.base = CFG_SYS_SDRAM_BASE;
- priv->info.size =
rockchip_sdram_size((phys_addr_t)&priv->pmugrf->os_reg2);
- priv->info.size = rockchip_sdram_size((ulong)&priv->pmugrf->os_reg2);
Can you please say a few words about this change in the commit log? Why phys_addr_t->ulong here?
Otherwise looks ok to me, Cheers, Quentin

Hi Quentin,
On Mon, 24 Jun 2024 at 09:24, Quentin Schulz quentin.schulz@cherry.de wrote:
Hi Simon,
On 6/23/24 7:53 PM, Simon Glass wrote:
The code here is confusing due to large blocks which are #ifdefed out. Add a function phase_sdram_init() which returns whether SDRAM init should happen in the current phase, using that as needed to control the code flow.
This increases code size by about 500 bytes in SPL when the cache is on, since it must call the rather large rockchip_sdram_size() function.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Drop the non-dcache optimisation, since the cache should normally be on
Changes in v3:
Split out the refactoring into a separate patch
drivers/ram/rockchip/sdram_rk3399.c | 42 +++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index 3c4e20f4e80..949a082d00c 100644 --- a/drivers/ram/rockchip/sdram_rk3399.c +++ b/drivers/ram/rockchip/sdram_rk3399.c @@ -13,6 +13,7 @@ #include <log.h> #include <ram.h> #include <regmap.h> +#include <spl.h> #include <syscon.h> #include <asm/arch-rockchip/clock.h> #include <asm/arch-rockchip/cru.h> @@ -63,8 +64,6 @@ struct chan_info { };
struct dram_info { -#if defined(CONFIG_TPL_BUILD) || \
(!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD)) u32 pwrup_srefresh_exit[2]; struct chan_info chan[2]; struct clk ddr_clk;
@@ -75,7 +74,6 @@ struct dram_info { struct rk3399_pmusgrf_regs *pmusgrf; struct rk3399_ddr_cic_regs *cic; const struct sdram_rk3399_ops *ops; -#endif struct ram_info info; struct rk3399_pmugrf_regs *pmugrf; }; @@ -92,9 +90,6 @@ struct sdram_rk3399_ops { struct rk3399_sdram_params *params); };
-#if defined(CONFIG_TPL_BUILD) || \
(!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
- struct rockchip_dmc_plat { #if CONFIG_IS_ENABLED(OF_PLATDATA) struct dtd_rockchip_rk3399_dmc dtplat;
@@ -191,6 +186,17 @@ struct io_setting { }, };
+/**
- phase_sdram_init() - Check if this is the phase where SDRAM init happens
- Returns: true to do SDRAM init in this phase, false to not
- */
+static bool phase_sdram_init(void) +{
return spl_phase() == PHASE_TPL ||
(!IS_ENABLED(CONFIG_TPL) && !spl_in_proper());
+}
- static struct io_setting * lpddr4_get_io_settings(const struct rk3399_sdram_params *params, u32 mr5) {
@@ -3024,7 +3030,7 @@ static int rk3399_dmc_of_to_plat(struct udevice *dev) struct rockchip_dmc_plat *plat = dev_get_plat(dev); int ret;
if (!CONFIG_IS_ENABLED(OF_REAL))
if (!CONFIG_IS_ENABLED(OF_REAL) || !phase_sdram_init()) return 0; ret = dev_read_u32_array(dev, "rockchip,sdram-params",
@@ -3138,22 +3144,21 @@ static int rk3399_dmc_init(struct udevice *dev)
return 0;
} -#endif
static int rk3399_dmc_probe(struct udevice *dev) { struct dram_info *priv = dev_get_priv(dev);
-#if defined(CONFIG_TPL_BUILD) || \
(!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
if (rk3399_dmc_init(dev))
return 0;
-#endif
priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
debug("%s: pmugrf = %p\n", __func__, priv->pmugrf);
if (phase_sdram_init()) {
if (rk3399_dmc_init(dev))
return 0;
} else {
priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
debug("%s: pmugrf = %p\n", __func__, priv->pmugrf);
}
I'm not sure we need to put the debug message into an else as this pmugrf is also used when dmc_init passes.
Additionally, we could also just set priv->pmugrf before the if condition and remove it from dmc_init if we really wanted to.
priv->info.base = CFG_SYS_SDRAM_BASE;
priv->info.size =
rockchip_sdram_size((phys_addr_t)&priv->pmugrf->os_reg2);
priv->info.size = rockchip_sdram_size((ulong)&priv->pmugrf->os_reg2);
Can you please say a few words about this change in the commit log? Why phys_addr_t->ulong here?
Oh dear, it keeps coming back! Removed now.
Otherwise looks ok to me, Cheers, Quentin
Regards, Simon

Hi Simon,
On 2024-06-23 19:53, Simon Glass wrote:
The code here is confusing due to large blocks which are #ifdefed out. Add a function phase_sdram_init() which returns whether SDRAM init should happen in the current phase, using that as needed to control the code flow.
This increases code size by about 500 bytes in SPL when the cache is on, since it must call the rather large rockchip_sdram_size() function.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Drop the non-dcache optimisation, since the cache should normally be on
Changes in v3:
- Split out the refactoring into a separate patch
drivers/ram/rockchip/sdram_rk3399.c | 42 +++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index 3c4e20f4e80..949a082d00c 100644 --- a/drivers/ram/rockchip/sdram_rk3399.c +++ b/drivers/ram/rockchip/sdram_rk3399.c
[snip]
@@ -191,6 +186,17 @@ struct io_setting { }, };
+/**
- phase_sdram_init() - Check if this is the phase where SDRAM init happens
- Returns: true to do SDRAM init in this phase, false to not
- */
+static bool phase_sdram_init(void) +{
- return spl_phase() == PHASE_TPL ||
(!IS_ENABLED(CONFIG_TPL) && !spl_in_proper());
This also need to check for !IS_ENABLED(CONFIG_ROCKCHIP_EXTERNAL_TPL).
Regards, Jonas
+}
[snip]

Hi Jonas,
On Mon, 24 Jun 2024 at 09:53, Jonas Karlman jonas@kwiboo.se wrote:
Hi Simon,
On 2024-06-23 19:53, Simon Glass wrote:
The code here is confusing due to large blocks which are #ifdefed out. Add a function phase_sdram_init() which returns whether SDRAM init should happen in the current phase, using that as needed to control the code flow.
This increases code size by about 500 bytes in SPL when the cache is on, since it must call the rather large rockchip_sdram_size() function.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Drop the non-dcache optimisation, since the cache should normally be on
Changes in v3:
- Split out the refactoring into a separate patch
drivers/ram/rockchip/sdram_rk3399.c | 42 +++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index 3c4e20f4e80..949a082d00c 100644 --- a/drivers/ram/rockchip/sdram_rk3399.c +++ b/drivers/ram/rockchip/sdram_rk3399.c
[snip]
@@ -191,6 +186,17 @@ struct io_setting { }, };
+/**
- phase_sdram_init() - Check if this is the phase where SDRAM init happens
- Returns: true to do SDRAM init in this phase, false to not
- */
+static bool phase_sdram_init(void) +{
return spl_phase() == PHASE_TPL ||
(!IS_ENABLED(CONFIG_TPL) && !spl_in_proper());
This also need to check for !IS_ENABLED(CONFIG_ROCKCHIP_EXTERNAL_TPL).
I don't see this condition in the current code...I am wondering how this works today?
Regards, Simon

Hi Simon,
On 2024-06-26 16:54, Simon Glass wrote:
Hi Jonas,
On Mon, 24 Jun 2024 at 09:53, Jonas Karlman jonas@kwiboo.se wrote:
Hi Simon,
On 2024-06-23 19:53, Simon Glass wrote:
The code here is confusing due to large blocks which are #ifdefed out. Add a function phase_sdram_init() which returns whether SDRAM init should happen in the current phase, using that as needed to control the code flow.
This increases code size by about 500 bytes in SPL when the cache is on, since it must call the rather large rockchip_sdram_size() function.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Drop the non-dcache optimisation, since the cache should normally be on
Changes in v3:
- Split out the refactoring into a separate patch
drivers/ram/rockchip/sdram_rk3399.c | 42 +++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index 3c4e20f4e80..949a082d00c 100644 --- a/drivers/ram/rockchip/sdram_rk3399.c +++ b/drivers/ram/rockchip/sdram_rk3399.c
[snip]
@@ -191,6 +186,17 @@ struct io_setting { }, };
+/**
- phase_sdram_init() - Check if this is the phase where SDRAM init happens
- Returns: true to do SDRAM init in this phase, false to not
- */
+static bool phase_sdram_init(void) +{
return spl_phase() == PHASE_TPL ||
(!IS_ENABLED(CONFIG_TPL) && !spl_in_proper());
This also need to check for !IS_ENABLED(CONFIG_ROCKCHIP_EXTERNAL_TPL).
I don't see this condition in the current code...I am wondering how this works today?
Currently you have to build with CONFIG_TPL, then throw TPL away to use CONFIG_ROCKCHIP_EXTERNAL_TPL.
Regards, Jonas
Regards, Simon

This causes a hang, so disable it. Unfortunately the RAM-size fix does not resolve the problem and I am unsure what is wrong. As soon as the cache is enabled the board appears to hang.
Fixes: 6d8cdfd1536 ("rockchip: spl: Enable caches to speed up checksum validation")
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
configs/chromebook_bob_defconfig | 1 + configs/chromebook_kevin_defconfig | 1 + 2 files changed, 2 insertions(+)
diff --git a/configs/chromebook_bob_defconfig b/configs/chromebook_bob_defconfig index acfe3934104..b2ecfa6050c 100644 --- a/configs/chromebook_bob_defconfig +++ b/configs/chromebook_bob_defconfig @@ -1,5 +1,6 @@ CONFIG_ARM=y CONFIG_SKIP_LOWLEVEL_INIT=y +CONFIG_SPL_SYS_DCACHE_OFF=y CONFIG_COUNTER_FREQUENCY=24000000 CONFIG_ARCH_ROCKCHIP=y CONFIG_TEXT_BASE=0x00200000 diff --git a/configs/chromebook_kevin_defconfig b/configs/chromebook_kevin_defconfig index 95fdb418d82..da748e4f022 100644 --- a/configs/chromebook_kevin_defconfig +++ b/configs/chromebook_kevin_defconfig @@ -2,6 +2,7 @@ CONFIG_ARM=y CONFIG_SKIP_LOWLEVEL_INIT=y CONFIG_COUNTER_FREQUENCY=24000000 CONFIG_ARCH_ROCKCHIP=y +CONFIG_SPL_SYS_DCACHE_OFF=y CONFIG_TEXT_BASE=0x00200000 CONFIG_SPL_GPIO=y CONFIG_NR_DRAM_BANKS=1

Now that am335x_evm boots OK on the Beaglebone black, drop the latter and update the docs to cover the change.
Also add a few updates about 'make fit' and drop the note about the security review, as U-Boot's verified boot has had quite extensive review now.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com ---
Changes in v4: - Move Binman size feature to a separate series
Changes in v2: - Drop patch "regulator: rk8xx: Fix incorrect parameter" - Rewrite boneblack patch to onstead drop the target and update docs
board/ti/am335x/MAINTAINERS | 1 - configs/am335x_boneblack_vboot_defconfig | 94 ------------------------ configs/am335x_evm_defconfig | 3 +- doc/usage/fit/beaglebone_vboot.rst | 21 +++--- 4 files changed, 12 insertions(+), 107 deletions(-) delete mode 100644 configs/am335x_boneblack_vboot_defconfig
diff --git a/board/ti/am335x/MAINTAINERS b/board/ti/am335x/MAINTAINERS index 219c8715bf1..ed8800a2663 100644 --- a/board/ti/am335x/MAINTAINERS +++ b/board/ti/am335x/MAINTAINERS @@ -3,6 +3,5 @@ M: Tom Rini trini@konsulko.com S: Maintained F: board/ti/am335x/ F: include/configs/am335x_evm.h -F: configs/am335x_boneblack_vboot_defconfig F: configs/am335x_evm_defconfig F: configs/am335x_evm_spiboot_defconfig diff --git a/configs/am335x_boneblack_vboot_defconfig b/configs/am335x_boneblack_vboot_defconfig deleted file mode 100644 index d473a1a793b..00000000000 --- a/configs/am335x_boneblack_vboot_defconfig +++ /dev/null @@ -1,94 +0,0 @@ -CONFIG_ARM=y -CONFIG_ARCH_CPU_INIT=y -# CONFIG_SPL_USE_ARCH_MEMCPY is not set -# CONFIG_SPL_USE_ARCH_MEMSET is not set -CONFIG_ARCH_OMAP2PLUS=y -CONFIG_TI_COMMON_CMD_OPTIONS=y -CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y -CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x4030ff00 -CONFIG_SF_DEFAULT_SPEED=24000000 -CONFIG_DEFAULT_DEVICE_TREE="am335x-boneblack" -CONFIG_AM33XX=y -CONFIG_CLOCK_SYNTHESIZER=y -CONFIG_SPL=y -CONFIG_ENV_OFFSET_REDUND=0x280000 -CONFIG_TIMESTAMP=y -CONFIG_FIT_SIGNATURE=y -CONFIG_FIT_VERBOSE=y -CONFIG_SYS_BOOTM_LEN=0x1000000 -CONFIG_DISTRO_DEFAULTS=y -CONFIG_AUTOBOOT_KEYED=y -CONFIG_AUTOBOOT_PROMPT="Press SPACE to abort autoboot in %d seconds\n" -CONFIG_AUTOBOOT_DELAY_STR="d" -CONFIG_AUTOBOOT_STOP_STR=" " -CONFIG_BOOTCOMMAND="run findfdt; run init_console; run finduuid; run distro_bootcmd" -CONFIG_SYS_CONSOLE_INFO_QUIET=y -CONFIG_ARCH_MISC_INIT=y -CONFIG_SPL_SYS_MALLOC=y -CONFIG_SPL_SYS_MALLOC_SIZE=0x800000 -CONFIG_SPL_MUSB_NEW=y -# CONFIG_SPL_NAND_SUPPORT is not set -CONFIG_SPL_NET=y -CONFIG_SPL_NET_VCI_STRING="AM33xx U-Boot SPL" -CONFIG_SPL_OS_BOOT=y -CONFIG_SPL_FALCON_BOOT_MMCSD=y -CONFIG_SYS_MMCSD_RAW_MODE_KERNEL_SECTOR=0x1700 -CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR=0x1500 -CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS=0x200 -CONFIG_CMD_SPL=y -CONFIG_SYS_I2C_EEPROM_ADDR_LEN=2 -# CONFIG_CMD_SETEXPR is not set -CONFIG_BOOTP_DNS2=y -CONFIG_OF_CONTROL=y -CONFIG_SPL_OF_CONTROL=y -CONFIG_ENV_OVERWRITE=y -CONFIG_ENV_IS_IN_MMC=y -CONFIG_SYS_REDUNDAND_ENVIRONMENT=y -CONFIG_SYS_RELOC_GD_ENV_ADDR=y -CONFIG_SYS_MMC_ENV_DEV=1 -CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y -CONFIG_VERSION_VARIABLE=y -CONFIG_NET_RETRY_COUNT=10 -CONFIG_BOOTP_SEND_HOSTNAME=y -# CONFIG_SPL_BLK is not set -CONFIG_BOOTCOUNT_LIMIT=y -CONFIG_SYS_BOOTCOUNT_BE=y -CONFIG_DFU_MMC=y -CONFIG_DFU_RAM=y -CONFIG_USB_FUNCTION_FASTBOOT=y -CONFIG_DM_I2C=y -CONFIG_MISC=y -CONFIG_SYS_I2C_EEPROM_ADDR=0x50 -# CONFIG_SPL_DM_MMC is not set -CONFIG_MMC_OMAP_HS=y -CONFIG_MTD=y -CONFIG_DM_SPI_FLASH=y -CONFIG_SPI_FLASH_WINBOND=y -CONFIG_PHY_ATHEROS=y -CONFIG_PHY_SMSC=y -CONFIG_PHY_GIGE=y -CONFIG_MII=y -CONFIG_DRIVER_TI_CPSW=y -CONFIG_DM_PMIC=y -# CONFIG_SPL_DM_PMIC is not set -CONFIG_PMIC_TPS65217=y -CONFIG_SPL_POWER_TPS65910=y -CONFIG_SPI=y -CONFIG_DM_SPI=y -CONFIG_OMAP3_SPI=y -CONFIG_TIMER=y -CONFIG_OMAP_TIMER=y -CONFIG_USB=y -CONFIG_DM_USB_GADGET=y -CONFIG_SPL_DM_USB_GADGET=y -CONFIG_USB_MUSB_HOST=y -CONFIG_USB_MUSB_GADGET=y -CONFIG_USB_MUSB_TI=y -CONFIG_USB_GADGET=y -CONFIG_SPL_USB_GADGET=y -CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments" -CONFIG_USB_GADGET_VENDOR_NUM=0x0451 -CONFIG_USB_GADGET_PRODUCT_NUM=0xd022 -CONFIG_USB_ETHER=y -CONFIG_SPL_USB_ETHER=y -CONFIG_LZO=y diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig index d243cb16e72..cabc181460a 100644 --- a/configs/am335x_evm_defconfig +++ b/configs/am335x_evm_defconfig @@ -13,6 +13,8 @@ CONFIG_AM335X_USB0_PERIPHERAL=y CONFIG_AM335X_USB1=y CONFIG_SPL=y CONFIG_TIMESTAMP=y +CONFIG_FIT_SIGNATURE=y +CONFIG_FIT_VERBOSE=y CONFIG_SPL_LOAD_FIT=y CONFIG_SYS_BOOTM_LEN=0x1000000 CONFIG_DISTRO_DEFAULTS=y @@ -119,5 +121,4 @@ CONFIG_SPL_USB_ETHER=y CONFIG_WDT=y # CONFIG_SPL_WDT is not set CONFIG_DYNAMIC_CRC_TABLE=y -CONFIG_RSA=y CONFIG_LZO=y diff --git a/doc/usage/fit/beaglebone_vboot.rst b/doc/usage/fit/beaglebone_vboot.rst index cd6bb141910..1360c71803c 100644 --- a/doc/usage/fit/beaglebone_vboot.rst +++ b/doc/usage/fit/beaglebone_vboot.rst @@ -67,18 +67,20 @@ a. Set up the environment variable to point to your toolchain. You will need
export CROSS_COMPILE=arm-linux-gnueabi-
-b. Configure and build U-Boot with verified boot enabled:: +b. Configure and build U-Boot with verified boot enabled. Note that we use the +am335x_evm target since it covers all boards based on the AM335x evaluation +board::
export UBOOT=/path/to/u-boot cd $UBOOT # You can add -j10 if you have 10 CPUs to make it faster - make O=b/am335x_boneblack_vboot am335x_boneblack_vboot_config all - export UOUT=$UBOOT/b/am335x_boneblack_vboot + make O=b/am335x_evm am335x_evm_config all + export UOUT=$UBOOT/b/am335x_evm
c. You will now have a U-Boot image::
- file b/am335x_boneblack_vboot/u-boot-dtb.img - b/am335x_boneblack_vboot/u-boot-dtb.img: u-boot legacy uImage, + file b/am335x_evm/u-boot-dtb.img + b/am335x_evm/u-boot-dtb.img: u-boot legacy uImage, U-Boot 2014.07-rc2-00065-g2f69f8, Firmware/ARM, Firmware Image (Not compressed), 395375 bytes, Sat May 31 16:19:04 2014, Load Address: 0x80800000, Entry Point: 0x00000000, @@ -466,7 +468,7 @@ the private key that you signed with so that it can verify any kernels that you sign::
cd $UBOOT - make O=b/am335x_boneblack_vboot EXT_DTB=${WORK}/am335x-boneblack-pubkey.dtb + make O=b/am335x_evm EXT_DTB=${WORK}/am335x-boneblack-pubkey.dtb
Here we are overriding the normal device tree file with our one, which contains the public key. @@ -597,14 +599,11 @@ Further Improvements
Several of the steps here can be easily automated. In particular it would be capital if signing and packaging a kernel were easy, perhaps a simple make -target in the kernel. +target in the kernel. A stating point for this is the 'make image.fit' target +for ARM64 in Linux from v6.9 onwards.
Some mention of how to use multiple .dtb files in a FIT might be useful.
-U-Boot's verified boot mechanism has not had a robust and independent security -review. Such a review should look at the implementation and its resistance to -attacks. - Perhaps the verified boot feature could be integrated into the Amstrom distribution.

Hi Simon,
On 6/23/24 7:53 PM, Simon Glass wrote:
Now that am335x_evm boots OK on the Beaglebone black, drop the latter and update the docs to cover the change.
Also add a few updates about 'make fit' and drop the note about the security review, as U-Boot's verified boot has had quite extensive review now.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com
Changes in v4:
- Move Binman size feature to a separate series
Changes in v2:
Drop patch "regulator: rk8xx: Fix incorrect parameter"
Rewrite boneblack patch to onstead drop the target and update docs
board/ti/am335x/MAINTAINERS | 1 - configs/am335x_boneblack_vboot_defconfig | 94 ------------------------ configs/am335x_evm_defconfig | 3 +- doc/usage/fit/beaglebone_vboot.rst | 21 +++--- 4 files changed, 12 insertions(+), 107 deletions(-) delete mode 100644 configs/am335x_boneblack_vboot_defconfig
diff --git a/board/ti/am335x/MAINTAINERS b/board/ti/am335x/MAINTAINERS index 219c8715bf1..ed8800a2663 100644 --- a/board/ti/am335x/MAINTAINERS +++ b/board/ti/am335x/MAINTAINERS @@ -3,6 +3,5 @@ M: Tom Rini trini@konsulko.com S: Maintained F: board/ti/am335x/ F: include/configs/am335x_evm.h -F: configs/am335x_boneblack_vboot_defconfig F: configs/am335x_evm_defconfig F: configs/am335x_evm_spiboot_defconfig diff --git a/configs/am335x_boneblack_vboot_defconfig b/configs/am335x_boneblack_vboot_defconfig deleted file mode 100644 index d473a1a793b..00000000000 --- a/configs/am335x_boneblack_vboot_defconfig +++ /dev/null @@ -1,94 +0,0 @@ -CONFIG_ARM=y -CONFIG_ARCH_CPU_INIT=y -# CONFIG_SPL_USE_ARCH_MEMCPY is not set -# CONFIG_SPL_USE_ARCH_MEMSET is not set -CONFIG_ARCH_OMAP2PLUS=y -CONFIG_TI_COMMON_CMD_OPTIONS=y -CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y -CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x4030ff00 -CONFIG_SF_DEFAULT_SPEED=24000000 -CONFIG_DEFAULT_DEVICE_TREE="am335x-boneblack" -CONFIG_AM33XX=y -CONFIG_CLOCK_SYNTHESIZER=y -CONFIG_SPL=y -CONFIG_ENV_OFFSET_REDUND=0x280000 -CONFIG_TIMESTAMP=y -CONFIG_FIT_SIGNATURE=y -CONFIG_FIT_VERBOSE=y -CONFIG_SYS_BOOTM_LEN=0x1000000 -CONFIG_DISTRO_DEFAULTS=y -CONFIG_AUTOBOOT_KEYED=y -CONFIG_AUTOBOOT_PROMPT="Press SPACE to abort autoboot in %d seconds\n" -CONFIG_AUTOBOOT_DELAY_STR="d" -CONFIG_AUTOBOOT_STOP_STR=" " -CONFIG_BOOTCOMMAND="run findfdt; run init_console; run finduuid; run distro_bootcmd" -CONFIG_SYS_CONSOLE_INFO_QUIET=y -CONFIG_ARCH_MISC_INIT=y -CONFIG_SPL_SYS_MALLOC=y -CONFIG_SPL_SYS_MALLOC_SIZE=0x800000 -CONFIG_SPL_MUSB_NEW=y -# CONFIG_SPL_NAND_SUPPORT is not set -CONFIG_SPL_NET=y -CONFIG_SPL_NET_VCI_STRING="AM33xx U-Boot SPL" -CONFIG_SPL_OS_BOOT=y -CONFIG_SPL_FALCON_BOOT_MMCSD=y -CONFIG_SYS_MMCSD_RAW_MODE_KERNEL_SECTOR=0x1700 -CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR=0x1500 -CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS=0x200 -CONFIG_CMD_SPL=y -CONFIG_SYS_I2C_EEPROM_ADDR_LEN=2 -# CONFIG_CMD_SETEXPR is not set -CONFIG_BOOTP_DNS2=y -CONFIG_OF_CONTROL=y -CONFIG_SPL_OF_CONTROL=y -CONFIG_ENV_OVERWRITE=y -CONFIG_ENV_IS_IN_MMC=y -CONFIG_SYS_REDUNDAND_ENVIRONMENT=y -CONFIG_SYS_RELOC_GD_ENV_ADDR=y -CONFIG_SYS_MMC_ENV_DEV=1 -CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y -CONFIG_VERSION_VARIABLE=y -CONFIG_NET_RETRY_COUNT=10 -CONFIG_BOOTP_SEND_HOSTNAME=y -# CONFIG_SPL_BLK is not set -CONFIG_BOOTCOUNT_LIMIT=y -CONFIG_SYS_BOOTCOUNT_BE=y -CONFIG_DFU_MMC=y -CONFIG_DFU_RAM=y -CONFIG_USB_FUNCTION_FASTBOOT=y -CONFIG_DM_I2C=y -CONFIG_MISC=y -CONFIG_SYS_I2C_EEPROM_ADDR=0x50 -# CONFIG_SPL_DM_MMC is not set -CONFIG_MMC_OMAP_HS=y -CONFIG_MTD=y -CONFIG_DM_SPI_FLASH=y -CONFIG_SPI_FLASH_WINBOND=y -CONFIG_PHY_ATHEROS=y -CONFIG_PHY_SMSC=y -CONFIG_PHY_GIGE=y -CONFIG_MII=y -CONFIG_DRIVER_TI_CPSW=y -CONFIG_DM_PMIC=y -# CONFIG_SPL_DM_PMIC is not set -CONFIG_PMIC_TPS65217=y -CONFIG_SPL_POWER_TPS65910=y -CONFIG_SPI=y -CONFIG_DM_SPI=y -CONFIG_OMAP3_SPI=y -CONFIG_TIMER=y -CONFIG_OMAP_TIMER=y -CONFIG_USB=y -CONFIG_DM_USB_GADGET=y -CONFIG_SPL_DM_USB_GADGET=y -CONFIG_USB_MUSB_HOST=y -CONFIG_USB_MUSB_GADGET=y -CONFIG_USB_MUSB_TI=y -CONFIG_USB_GADGET=y -CONFIG_SPL_USB_GADGET=y -CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments" -CONFIG_USB_GADGET_VENDOR_NUM=0x0451 -CONFIG_USB_GADGET_PRODUCT_NUM=0xd022 -CONFIG_USB_ETHER=y -CONFIG_SPL_USB_ETHER=y -CONFIG_LZO=y diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig index d243cb16e72..cabc181460a 100644 --- a/configs/am335x_evm_defconfig +++ b/configs/am335x_evm_defconfig @@ -13,6 +13,8 @@ CONFIG_AM335X_USB0_PERIPHERAL=y CONFIG_AM335X_USB1=y CONFIG_SPL=y CONFIG_TIMESTAMP=y +CONFIG_FIT_SIGNATURE=y +CONFIG_FIT_VERBOSE=y CONFIG_SPL_LOAD_FIT=y CONFIG_SYS_BOOTM_LEN=0x1000000 CONFIG_DISTRO_DEFAULTS=y @@ -119,5 +121,4 @@ CONFIG_SPL_USB_ETHER=y CONFIG_WDT=y # CONFIG_SPL_WDT is not set CONFIG_DYNAMIC_CRC_TABLE=y -CONFIG_RSA=y CONFIG_LZO=y diff --git a/doc/usage/fit/beaglebone_vboot.rst b/doc/usage/fit/beaglebone_vboot.rst index cd6bb141910..1360c71803c 100644 --- a/doc/usage/fit/beaglebone_vboot.rst +++ b/doc/usage/fit/beaglebone_vboot.rst @@ -67,18 +67,20 @@ a. Set up the environment variable to point to your toolchain. You will need
export CROSS_COMPILE=arm-linux-gnueabi-
-b. Configure and build U-Boot with verified boot enabled:: +b. Configure and build U-Boot with verified boot enabled. Note that we use the +am335x_evm target since it covers all boards based on the AM335x evaluation +board::
export UBOOT=/path/to/u-boot cd $UBOOT # You can add -j10 if you have 10 CPUs to make it faster
- make O=b/am335x_boneblack_vboot am335x_boneblack_vboot_config all
- export UOUT=$UBOOT/b/am335x_boneblack_vboot
make O=b/am335x_evm am335x_evm_config all
export UOUT=$UBOOT/b/am335x_evm
c. You will now have a U-Boot image::
- file b/am335x_boneblack_vboot/u-boot-dtb.img
- b/am335x_boneblack_vboot/u-boot-dtb.img: u-boot legacy uImage,
- file b/am335x_evm/u-boot-dtb.img
- b/am335x_evm/u-boot-dtb.img: u-boot legacy uImage, U-Boot 2014.07-rc2-00065-g2f69f8, Firmware/ARM, Firmware Image (Not compressed), 395375 bytes, Sat May 31 16:19:04 2014, Load Address: 0x80800000, Entry Point: 0x00000000,
@@ -466,7 +468,7 @@ the private key that you signed with so that it can verify any kernels that you sign::
cd $UBOOT
- make O=b/am335x_boneblack_vboot EXT_DTB=${WORK}/am335x-boneblack-pubkey.dtb
make O=b/am335x_evm EXT_DTB=${WORK}/am335x-boneblack-pubkey.dtb
Here we are overriding the normal device tree file with our one, which contains the public key.
@@ -597,14 +599,11 @@ Further Improvements
Several of the steps here can be easily automated. In particular it would be capital if signing and packaging a kernel were easy, perhaps a simple make -target in the kernel. +target in the kernel. A stating point for this is the 'make image.fit' target
s/stating/starting/ I believe?
Cheers, Quentin
participants (3)
-
Jonas Karlman
-
Quentin Schulz
-
Simon Glass