[U-Boot] [PATCH 3/6] SPL: Add XIP booting support

[Vikas MANOCHA]

Hi Alex,

> -----Original Message-----
> From: Alexandru Gagniuc [mailto:alex.g at adaptrum.com]
> Sent: Monday, May 22, 2017 10:37 AM
> To: Vikas MANOCHA <vikas.manocha at st.com>; u-boot at lists.denx.de
> Cc: Patrick DELAUNAY <patrick.delaunay at st.com>; Patrice CHOTARD <patrice.chotard at st.com>; Christophe KERELLO
> <christophe.kerello at st.com>; Christophe PRIOUZEAU <christophe.priouzeau at st.com>; Alexandre TORGUE
> <alexandre.torgue at st.com>; Albert Aribaud <albert.u.boot at aribaud.net>; Andrew F. Davis <afd at ti.com>; Bin Meng
> <bmeng.cn at gmail.com>; B, Ravi <ravibabu at ti.com>; Heiko Schocher <hs at denx.de>; Ladislav Michl <ladis at linux-mips.org>;
> Masahiro Yamada <yamada.masahiro at socionext.com>; Michal Simek <michal.simek at xilinx.com>; Simon Glass
> <sjg at chromium.org>; Stefan Agner <stefan.agner at toradex.com>
> Subject: Re: [PATCH 3/6] SPL: Add XIP booting support
> 
> 
> On 05/22/2017 09:55 AM, Vikas MANOCHA wrote:
> > Hi Alex,
> Hi
> 
> [snip]
> 
> >>
> >>> diff --git a/common/spl/spl_xip.c b/common/spl/spl_xip.c new file
> >>> mode 100644 index 0000000..50e2f34
> >>> --- /dev/null
> >>> +++ b/common/spl/spl_xip.c
> >>> @@ -0,0 +1,31 @@
> >>> +/*
> >>> + * Copyright (C) 2017 Vikas Manocha <vikas.manocha at st.com>
> >>> + *
> >>> + * SPDX-License-Identifier:	GPL-2.0+
> >>> + */
> >>> +
> >>> +#include <common.h>
> >>> +#include <spl.h>
> >>> +
> >>> +static int spl_xip(struct spl_image_info *spl_image,
> >>> +		   struct spl_boot_device *bootdev) { #ifdef CONFIG_SPL_OS_BOOT
> >>> +	if (!spl_start_uboot()) {
> >>> +		spl_image->arg = (void *)CONFIG_SYS_FDT_BASE;
> >>> +		spl_image->name = "Linux";
> >>> +		spl_image->os = IH_OS_LINUX;
> >>> +		spl_image->load_addr = CONFIG_SYS_LOAD_ADDR;
> >>> +		spl_image->entry_point = CONFIG_SYS_LOAD_ADDR; #ifdef
> >>> +CONFIG_CPU_V7M
> >>
> >> This looks like it should be handled by spl_set_header_raw_uboot(). I
> >> don't see other SPL loaders do this.
> >
> > We might not want to boot kernel if header is not present in every situation. With spl_xip config option, we enable if we need it.
> 
> I'm talkVing about the '#ifdef CONFIG_CPU_7M' part. A lot of the spl loaders are mostly boilerplate, but it should be consistent
> boilerplate.
> If there is a good reason to have a different boilerplate, then at the very least a comment should explain "why" it is done different.

#ifdef CONFIG_CPU_V7M part is to keep v7m cpu in thumb mode as it does not support arm mode. The same is the reason for it to
be in spl_set_header_raw_uboot().

On a second thought, I think it will be good to move this part just before booting next image. In that case it would be required just once.
I will send a separate patch for it.

Cheers,
Vikas

> 
> Alex