[U-Boot] [PATCH] arm: omap3: Detect boot mode very early

[Siarhei Siamashka <siarhei.siamashka@gmail.com>]

On Fri, 14 Jul 2017 08:53:20 -0500
Adam Ford <aford173@gmail.com> wrote:

> Fixes 4bd754d8abef ("arm: omap: Detect boot mode very early") where
> the intent was to store the boot params information in a known
> location and pass it to SPL very early. Unfortunately it didn't
> account for OMAP3 boards.
> 
> This patch adds adds this functionality back into OMAP3 boards.
> 
> Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> 
> diff --git a/arch/arm/mach-omap2/omap3/board.c b/arch/arm/mach-omap2/omap3/board.c
> index cd8e302..a61b933 100644
> --- a/arch/arm/mach-omap2/omap3/board.c
> +++ b/arch/arm/mach-omap2/omap3/board.c
> @@ -212,6 +212,12 @@ void board_init_f(ulong dummy)
>  {
>  	early_system_init();
>  	mem_init();
> +	/*
> +	* Save the boot parameters passed from romcode.
> +	* We cannot delay the saving further than this,
> +	* to prevent overwrites.
> +	*/
> +	save_omap_boot_params();
>  }
>  #endif
>  

Hello,

Something seems to be fishy here.

The 4bd754d8abef patch from Lokesh Vutla basically replicated the same
chunk of code for every OMAP variant rather than keeping it in one
common location. The justification for this code duplication is that
the boot parameters may be overwritten. Can we have a bit more details
about how and when this overwrite actually happens in practice?

I don't quite like the fact that we still have the code, which relies
on OMAP_SRAM_SCRATCH_BOOT_PARAMS in the "jump_to_image_no_args"
function very late in the SPL:

void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
{
	typedef void __noreturn (*image_entry_noargs_t)(u32 *);
	image_entry_noargs_t image_entry =
			(image_entry_noargs_t) spl_image->entry_point;

	u32 boot_params = *((u32 *)OMAP_SRAM_SCRATCH_BOOT_PARAMS);

	debug("image entry point: 0x%lX\n", spl_image->entry_point);
	/* Pass the saved boot_params from rom code */
	image_entry((u32 *)boot_params);
}


This code had been introduced by Paul Kocialkowski in the old
patch 60c7c30aa084588ef9746 ("omap-common: Common boot code
OMAP3 support and cleanup") and still exists in U-Boot. And
the commit message from that patch implied that no overwrite of
this OMAP_SRAM_SCRATCH_BOOT_PARAMS data happens during the whole
SPL lifetime:
    http://git.denx.de/?p=u-boot.git;a=commitdiff;h=60c7c30aa084588ef974663be3d22a1de7f66cbb

So what's going on? Can Lokesh or Paul comment?

-- 
Best regards,
Siarhei Siamashka