Hi Alex,
-----Original Message----- From: Alexandru Gagniuc [mailto:alex.g@adaptrum.com] Sent: Monday, May 22, 2017 10:37 AM To: Vikas MANOCHA vikas.manocha@st.com; u-boot@lists.denx.de Cc: Patrick DELAUNAY patrick.delaunay@st.com; Patrice CHOTARD patrice.chotard@st.com; Christophe KERELLO christophe.kerello@st.com; Christophe PRIOUZEAU christophe.priouzeau@st.com; Alexandre TORGUE alexandre.torgue@st.com; Albert Aribaud albert.u.boot@aribaud.net; Andrew F. Davis afd@ti.com; Bin Meng bmeng.cn@gmail.com; B, Ravi ravibabu@ti.com; Heiko Schocher hs@denx.de; Ladislav Michl ladis@linux-mips.org; Masahiro Yamada yamada.masahiro@socionext.com; Michal Simek michal.simek@xilinx.com; Simon Glass sjg@chromium.org; Stefan Agner stefan.agner@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@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