Re: [PATCH v4 3/5] rockchip: px30: insert u-boot, spl-boot-device into U-Boot device tree

From: Kever Yang <kever.yang@rock-chips.com>
To: Quentin Schulz <foss+uboot@0leil.net>,
	Simon Glass <sjg@chromium.org>,
	Philipp Tomsich <philipp.tomsich@vrull.eu>,
	Heiko Stuebner <heiko@sntech.de>,
	Klaus Goger <klaus.goger@theobroma-systems.com>
Cc: Quentin Schulz <quentin.schulz@theobroma-systems.com>,
	u-boot@lists.denx.de
Subject: Re: [PATCH v4 3/5] rockchip: px30: insert u-boot, spl-boot-device into U-Boot device tree
Date: Mon, 19 Dec 2022 15:12:33 +0800	[thread overview]
Message-ID: <a84e2333-f10f-ab9a-e713-8736cf01213e@rock-chips.com> (raw)
In-Reply-To: <20221017-upstream-ringneck-v4-3-5e0a3f119047@theobroma-systems.com>

Hi Quentin,

     I got below error for this patch:

+arch/arm/mach-rockchip/px30/px30.c: In function 'spl_perform_fixups':
2782 
<https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/543635#L2782>+arch/arm/mach-rockchip/px30/px30.c:475:18: 
error: implicit declaration of function 'fdt_find_or_add_subnode'; did 
you mean 'fdt_for_each_subnode'? [-Werror=implicit-function-declaration]
2783 
<https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/543635#L2783>+ 
475 | chosen = fdt_find_or_add_subnode(blob, 0, "chosen");
2784 
<https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/543635#L2784>+ 
| ^~~~~~~~~~~~~~~~~~~~~~~
2785 
<https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/543635#L2785>+ 
| fdt_for_each_subnode

Thanks,
- Kever
On 2022/11/29 20:19, Quentin Schulz wrote:
> From: Quentin Schulz<quentin.schulz@theobroma-systems.com>
>
> It is possible to boot U-Boot proper from a different storage medium
> than the one used by the BOOTROM to load the SPL. This information is
> stored in the u-boot,spl-boot-device Device Tree property and is
> accessible from U-Boot proper so that it has knowledge at runtime where
> it was loaded from.
>
> Let's add support for this feature for px30.
>
> Cc: Quentin Schulz<foss+uboot@0leil.net>
> Signed-off-by: Quentin Schulz<quentin.schulz@theobroma-systems.com>
> ---
>   arch/arm/mach-rockchip/px30/px30.c | 50 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 50 insertions(+)
>
> diff --git a/arch/arm/mach-rockchip/px30/px30.c b/arch/arm/mach-rockchip/px30/px30.c
> index 481b50235e..5f26128d01 100644
> --- a/arch/arm/mach-rockchip/px30/px30.c
> +++ b/arch/arm/mach-rockchip/px30/px30.c
> @@ -6,6 +6,7 @@
>   #include <clk.h>
>   #include <dm.h>
>   #include <init.h>
> +#include <spl.h>
>   #include <asm/armv8/mmu.h>
>   #include <asm/io.h>
>   #include <asm/arch-rockchip/bootrom.h>
> @@ -427,3 +428,52 @@ void board_debug_uart_init(void)
>   #endif /* CONFIG_DEBUG_UART_BASE && CONFIG_DEBUG_UART_BASE == ... */
>   }
>   #endif /* CONFIG_DEBUG_UART_BOARD_INIT */
> +
> +#if defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD)
> +const char *spl_decode_boot_device(u32 boot_device)
> +{
> +	int i;
> +	static const struct {
> +		u32 boot_device;
> +		const char *ofpath;
> +	} spl_boot_devices_tbl[] = {
> +		{ BOOT_DEVICE_MMC2, "/mmc@ff370000" },
> +		{ BOOT_DEVICE_MMC1, "/mmc@ff390000" },
> +	};
> +
> +	for (i = 0; i < ARRAY_SIZE(spl_boot_devices_tbl); ++i)
> +		if (spl_boot_devices_tbl[i].boot_device == boot_device)
> +			return spl_boot_devices_tbl[i].ofpath;
> +
> +	return NULL;
> +}
> +
> +void spl_perform_fixups(struct spl_image_info *spl_image)
> +{
> +	void *blob = spl_image->fdt_addr;
> +	const char *boot_ofpath;
> +	int chosen;
> +
> +	/*
> +	 * Inject the ofpath of the device the full U-Boot (or Linux in
> +	 * Falcon-mode) was booted from into the FDT, if a FDT has been
> +	 * loaded at the same time.
> +	 */
> +	if (!blob)
> +		return;
> +
> +	boot_ofpath = spl_decode_boot_device(spl_image->boot_device);
> +	if (!boot_ofpath) {
> +		pr_err("%s: could not map boot_device to ofpath\n", __func__);
> +		return;
> +	}
> +
> +	chosen = fdt_find_or_add_subnode(blob, 0, "chosen");
> +	if (chosen < 0) {
> +		pr_err("%s: could not find/create '/chosen'\n", __func__);
> +		return;
> +	}
> +	fdt_setprop_string(blob, chosen,
> +			   "u-boot,spl-boot-device", boot_ofpath);
> +}
> +#endif
>