Re: [PATCH v2 3/4] sunxi: Support SPL in both eGON and TOC0 images

[Andre Przywara <andre.przywara@arm.com>]

On Sat, 21 Aug 2021 23:46:47 -0500
Samuel Holland <samuel@sholland.org> wrote:

Hi,

> SPL uses the image header to detect the boot device and to find the
> offset of the next U-Boot stage. Since this information is stored
> differently in the eGON and TOC0 image headers, add code to find the
> correct value based on the image type currently in use.

many thanks for determining this at runtime!

> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v2:
>  - Moved SPL header signature checks out of sunxi_image.h
>  - Refactored SPL header signature checks to use fewer casts
> 
>  arch/arm/include/asm/arch-sunxi/spl.h |  2 --
>  arch/arm/mach-sunxi/board.c           | 50 +++++++++++++++++++++++----
>  2 files changed, 43 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h
> index 58cdf806d9a..157b11e4897 100644
> --- a/arch/arm/include/asm/arch-sunxi/spl.h
> +++ b/arch/arm/include/asm/arch-sunxi/spl.h
> @@ -19,8 +19,6 @@
>  #define SUNXI_BOOTED_FROM_MMC0_HIGH	0x10
>  #define SUNXI_BOOTED_FROM_MMC2_HIGH	0x12
>  
> -#define is_boot0_magic(addr)	(memcmp((void *)(addr), BOOT0_MAGIC, 8) == 0)
> -
>  uint32_t sunxi_get_boot_device(void);
>  
>  #endif
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index d9b04f75fc4..b6f92bdc5e7 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -244,12 +244,40 @@ void s_init(void)
>  
>  #define SUNXI_INVALID_BOOT_SOURCE	-1
>  
> -static int sunxi_get_boot_source(void)
> +static struct boot_file_head *sunxi_egon_get_head(void)
>  {
> -	if (!is_boot0_magic(SPL_ADDR + 4)) /* eGON.BT0 */
> -		return SUNXI_INVALID_BOOT_SOURCE;
> +	struct boot_file_head *egon_head = (void *)SPL_ADDR;
> +
> +	if (memcmp(egon_head, BOOT0_MAGIC, 8)) /* eGON.BT0 */

For eGON the magic is not at the beginning of the struct, so you need:
	memcmp(&egon_head->magic, BOOT0_MAGIC, 8)

Otherwise 99.9% of all Allwinner users will be very disappointed ;-)

And there is another problem: For 32-bit SoCs the SPL address is
literally 0 (SRAM A1), so the return value in the successful case is
NULL as well :-(

Maybe have a function to return an enum (EGON, TOC0, NONE/FEL) instead?
After all the address will always be SPL_ADDR. 

Cheers,
Andre

> +		return NULL;
> +
> +	return egon_head;
> +}
> +
> +static struct toc0_main_info *sunxi_toc0_get_info(void)
> +{
> +	struct toc0_main_info *toc0_info = (void *)SPL_ADDR;
> +
> +	if (memcmp(toc0_info->name, TOC0_MAIN_INFO_NAME, 8)) /* TOC0.GLH */
> +		return NULL;
>  
> -	return readb(SPL_ADDR + 0x28);
> +	return toc0_info;
> +}
> +
> +static int sunxi_get_boot_source(void)
> +{
> +	struct boot_file_head *egon_head;
> +	struct toc0_main_info *toc0_info;
> +
> +	egon_head = sunxi_egon_get_head();
> +	if (egon_head)
> +		return readb(&egon_head->boot_media);
> +	toc0_info = sunxi_toc0_get_info();
> +	if (toc0_info)
> +		return readb(&toc0_info->platform[0]);
> +
> +	/* Not a valid image, so we must have been booted via FEL. */
> +	return SUNXI_INVALID_BOOT_SOURCE;
>  }
>  
>  /* The sunxi internal brom will try to loader external bootloader
> @@ -297,10 +325,18 @@ uint32_t sunxi_get_boot_device(void)
>  #ifdef CONFIG_SPL_BUILD
>  static u32 sunxi_get_spl_size(void)
>  {
> -	if (!is_boot0_magic(SPL_ADDR + 4)) /* eGON.BT0 */
> -		return 0;
> +	struct boot_file_head *egon_head;
> +	struct toc0_main_info *toc0_info;
> +
> +	egon_head = sunxi_egon_get_head();
> +	if (egon_head)
> +		return readl(&egon_head->length);
> +	toc0_info = sunxi_toc0_get_info();
> +	if (toc0_info)
> +		return readl(&toc0_info->length);
>  
> -	return readl(SPL_ADDR + 0x10);
> +	/* Not a valid image, so use the default U-Boot offset. */
> +	return 0;
>  }
>  
>  /*