[PATCH v6 0/8] Second part of bug-fixes series

This consists of some patches split ouf from an earlier series [1]. Apart from the fdt patch they are not needed for the upcoming release.
The FDT patch fixes a boot problem on several Chromebooks.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20240626155945.278640-15-sj...
Changes in v6: Take account of ROCKCHIP_EXTERNAL_TPL in phase_sdram_init()
Changes in v5: - Move setting of pmugrf into the probe() function
Changes in v4: - Drop Fixes tag
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: - Use 'phase' instead of 'stage' - Add new patch to correct memory size in SPL
Simon Glass (8): 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 fdt: Correct condition for bloblist existing rockchip: Ensure memory size is available in RK3399 SPL rockchip: Avoid #ifdefs in RK3399 SPL
doc/mkeficapsule.1 | 4 ++++ drivers/ram/rockchip/sdram_rk3399.c | 37 +++++++++++++++-------------- lib/fdtdec.c | 12 ++++++++-- tools/binman/btool/mkeficapsule.py | 3 ++- tools/binman/etype/efi_capsule.py | 5 +++- tools/mkeficapsule.c | 10 ++++++-- 6 files changed, 47 insertions(+), 24 deletions(-)

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 ---
(no changes since v4)
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

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);

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 ---
(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 ef9a1824b2b..bc79c034808 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; }

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.
- Drop the non-dcache optimisation, since the cache should normally be on
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v6: Take account of ROCKCHIP_EXTERNAL_TPL in phase_sdram_init()
Changes in v5: - Move setting of pmugrf into the probe() function
Changes in v3: - Split out the refactoring into a separate patch
drivers/ram/rockchip/sdram_rk3399.c | 35 +++++++++++++++-------------- 1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c index bc79c034808..55c0d1d2aed 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,19 @@ 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) && + !IS_ENABLED(CONFIG_ROCKCHIP_EXTERNAL_TPL) && + !spl_in_proper()); +} + static struct io_setting * lpddr4_get_io_settings(const struct rk3399_sdram_params *params, u32 mr5) { @@ -3024,7 +3032,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", @@ -3093,7 +3101,6 @@ static int rk3399_dmc_init(struct udevice *dev) priv->cic = syscon_get_first_range(ROCKCHIP_SYSCON_CIC); priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF); priv->pmu = syscon_get_first_range(ROCKCHIP_SYSCON_PMU); - priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF); priv->pmusgrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUSGRF); priv->pmucru = rockchip_get_pmucru(); priv->cru = rockchip_get_cru(); @@ -3138,19 +3145,16 @@ 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() && rk3399_dmc_init(dev)) + return 0; + priv->info.base = CFG_SYS_SDRAM_BASE; priv->info.size = rockchip_sdram_size((phys_addr_t)&priv->pmugrf->os_reg2); @@ -3181,10 +3185,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) || \
participants (1)
-
Simon Glass