[U-Boot] [PATCHv3 4/4] igep00x0: UBIize
Ladislav Michl
ladis at linux-mips.org
Tue Jan 12 23:17:21 CET 2016
Hi Heiko,
On Tue, Jan 12, 2016 at 10:08:21AM +0100, Heiko Schocher wrote:
> Am 11.01.2016 um 13:58 schrieb Ladislav Michl:
> >On Mon, Jan 11, 2016 at 07:20:06AM +0100, Heiko Schocher wrote:
> >>Beside of that, this patch does not apply ...
> >
> >Ah, igep00x0 part is based on top of this:
> >http://lists.denx.de/pipermail/u-boot/2016-January/240013.html
> >I silently hoped to be applied for 2016.01 release, but never mind :)
>
> ;-)
>
> Ah, I added them to my automated build, and now it works again :-D
>
> BTW: patch "[U-Boot,PATCHv2,2/5] igep00x0: Cleanup ethernet support"
> has a checkpatch warning, search in
>
> http://xeidos.ddns.net/buildbot/builders/smartweb_dfu/builds/43/steps/shell/logs/tbotlog
>
> for "2016-01-12 07:47:08,369"
Hmm, I do not agree with warning as I consider code pretty readable :)
Anyway, there is similar "unaligned case" few lines bellow. Perhaps send
additional patch to fix them both?
> >Now assumption is that once board switches to UBI, loading u-boot or kernel
> >from bare flash does not make a sense anymore, so with CONFIG_SPL_UBI
> >all that code in spl_nor.c (reevaluate?!), spl_nand.c and spl_onenand.c
> >is not used in favour of spl_ubi.c. As ubispl can load volumes by volume id
>
> I thought about this change too, and I think your assumption is OK
> here. Let us bring in this change, and if someone has other needs,
> we have to look again at this place .. but I think, if switching to
> use UBI, than it makes no sense to read in raw mode ...
>
> Other opinions?
>
> >and not by name, it is bringing some inconsistencies with for example ubi
> >environment code, which is using volume names. Is it worth fixing?
>
> It would be nice to have ... yes, if it is easy to do? Also we must
> have a look at the codesize.
It seems Thomas had a good reason to use volume ids, see ubi_scan_vid_hdr
called from ipl_scan.
> >All that ubispl_info structure is board specific and there is not much left
> >besides initializing it. Also volumes can differ per board basis, so
> >providing common function is somewhat questionable. However here it is,
> >just to show how does it look like. Suggestions are very welcome as silence
> >around this part of patch is a bit suspicious ;-)
>
> Questions are coming if there are users ;-)
>
> I vote for bringing this in, and we will see, where we have to make
> things more configurable ... some nitpicks below ...
Okay, I need to be able to load also bare zImage. This change is independent
and possibly controversial, so I'll put it here for disscusion :)
commit c092b3c0627dd8d4b3f3d756c58b53fcf205587f
Author: Ladislav Michl <ladis at linux-mips.org>
Date: Tue Jan 12 16:37:04 2016 +0100
spl: zImage support in Falcon mode
Other payload than uImage is currently considered to be raw U-Boot
image. Check also for zImage in Falcon mode.
Signed-off-by: Ladislav Michl <ladis at linux-mips.org>
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index f3db7b5..07a9019 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -26,11 +26,13 @@ endif
obj-$(CONFIG_CPU_V7M) += cmd_boot.o
obj-$(CONFIG_OF_LIBFDT) += bootm-fdt.o
obj-$(CONFIG_CMD_BOOTM) += bootm.o
+obj-$(CONFIG_CMD_BOOTM) += zimage.o
obj-$(CONFIG_SYS_L2_PL310) += cache-pl310.o
obj-$(CONFIG_USE_ARCH_MEMSET) += memset.o
obj-$(CONFIG_USE_ARCH_MEMCPY) += memcpy.o
else
obj-$(CONFIG_SPL_FRAMEWORK) += spl.o
+obj-$(CONFIG_SPL_FRAMEWORK) += zimage.o
endif
obj-$(CONFIG_SEMIHOSTING) += semihosting.o
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index a477cae..fbfc0ad 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -348,38 +348,6 @@ int do_bootm_linux(int flag, int argc, char * const argv[],
return 0;
}
-#ifdef CONFIG_CMD_BOOTZ
-
-struct zimage_header {
- uint32_t code[9];
- uint32_t zi_magic;
- uint32_t zi_start;
- uint32_t zi_end;
-};
-
-#define LINUX_ARM_ZIMAGE_MAGIC 0x016f2818
-
-int bootz_setup(ulong image, ulong *start, ulong *end)
-{
- struct zimage_header *zi;
-
- zi = (struct zimage_header *)map_sysmem(image, 0);
- if (zi->zi_magic != LINUX_ARM_ZIMAGE_MAGIC) {
- puts("Bad Linux ARM zImage magic!\n");
- return 1;
- }
-
- *start = zi->zi_start;
- *end = zi->zi_end;
-
- printf("Kernel image @ %#08lx [ %#08lx - %#08lx ]\n", image, *start,
- *end);
-
- return 0;
-}
-
-#endif /* CONFIG_CMD_BOOTZ */
-
#if defined(CONFIG_BOOTM_VXWORKS)
void boot_prep_vxworks(bootm_headers_t *images)
{
diff --git a/arch/arm/lib/zimage.c b/arch/arm/lib/zimage.c
new file mode 100644
index 0000000..f870d72
--- /dev/null
+++ b/arch/arm/lib/zimage.c
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2016
+ * Ladislav Michl <ladis at linux-mips.org>
+ *
+ * bootz code:
+ * Copyright (C) 2012 Marek Vasut <marek.vasut at gmail.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+#include <common.h>
+
+#define LINUX_ARM_ZIMAGE_MAGIC 0x016f2818
+
+struct arm_z_header {
+ uint32_t code[9];
+ uint32_t zi_magic;
+ uint32_t zi_start;
+ uint32_t zi_end;
+} __attribute__ ((__packed__));
+
+int bootz_setup(ulong image, ulong *start, ulong *end)
+{
+ struct arm_z_header *zi = (struct arm_z_header *) image;
+
+ if (zi->zi_magic != LINUX_ARM_ZIMAGE_MAGIC) {
+#ifndef CONFIG_SPL_FRAMEWORK
+ puts("Bad Linux ARM zImage magic!\n");
+#endif
+ return 1;
+ }
+
+ *start = zi->zi_start;
+ *end = zi->zi_end;
+#ifndef CONFIG_SPL_FRAMEWORK
+ printf("Kernel image @ %#08lx [ %#08lx - %#08lx ]\n", image, *start,
+ *end);
+#endif
+
+ return 0;
+}
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 7665105..342c3b8 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -52,6 +52,15 @@ __weak int spl_start_uboot(void)
puts("SPL: Direct Linux boot not active!\n");
return 1;
}
+
+/*
+ * Weak default function for arch specific zImage check. Return zero
+ * and fill start and end address if image is recognized.
+ */
+int __weak bootz_setup(ulong image, ulong *start, ulong *end)
+{
+ return 1;
+}
#endif
/*
@@ -112,6 +121,20 @@ void spl_parse_image_header(const struct image_header *header)
*/
panic("** no mkimage signature but raw image not supported");
#else
+#ifdef CONFIG_SPL_OS_BOOT
+ ulong start, end;
+
+ if (!bootz_setup((ulong)header, &start, &end)) {
+ spl_image.name = "Linux";
+ spl_image.os = IH_OS_LINUX;
+ spl_image.load_addr = (u32)header;
+ spl_image.entry_point = (u32)header;
+ spl_image.size = end - start;
+ debug("spl: payload zImage, load addr: 0x%x size: %d\n",
+ spl_image.load_addr, spl_image.size);
+ return;
+ }
+#endif
/* Signature not found - assume u-boot.bin */
debug("mkimage signature not found - ih_magic = %x\n",
header->ih_magic);
> >commit f68d017a4d35bfac3cccd7c7f19ab1c2fe76d908
> >Author: Ladislav Michl <ladis at linux-mips.org>
> >Date: Mon Jan 11 13:08:10 2016 +0100
> >
> > spl: support loading from UBI volumes
> >
> > Add simple support for loading from UBI volumes.
> > This is just a test and needs to be made configurable
> >
> > Signed-off-by: Ladislav Michl <ladis at linux-mips.org>
> >
> >diff --git a/common/spl/Makefile b/common/spl/Makefile
> >index 10a4589..e4535c4 100644
> >--- a/common/spl/Makefile
> >+++ b/common/spl/Makefile
> >@@ -10,10 +10,13 @@
> >
> > ifdef CONFIG_SPL_BUILD
> > obj-$(CONFIG_SPL_FRAMEWORK) += spl.o
> >-obj-$(CONFIG_SPL_NOR_SUPPORT) += spl_nor.o
> > obj-$(CONFIG_SPL_YMODEM_SUPPORT) += spl_ymodem.o
> >+ifndef CONFIG_SPL_UBI
> >+obj-$(CONFIG_SPL_NOR_SUPPORT) += spl_nor.o
> > obj-$(CONFIG_SPL_NAND_SUPPORT) += spl_nand.o
> > obj-$(CONFIG_SPL_ONENAND_SUPPORT) += spl_onenand.o
> >+endif
> >+obj-$(CONFIG_SPL_UBI) += spl_ubi.o
> > obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o
> > obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o
> > obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o
> >diff --git a/common/spl/spl.c b/common/spl/spl.c
> >index 6e6dee7..048a325 100644
> >--- a/common/spl/spl.c
> >+++ b/common/spl/spl.c
> >@@ -286,6 +286,18 @@ static int spl_load_image(u32 boot_device)
> > case BOOT_DEVICE_MMC2_2:
> > return spl_mmc_load_image(boot_device);
> > #endif
> >+#ifdef CONFIG_SPL_UBI
> >+#ifdef CONFIG_SPL_NAND_SUPPORT
> >+ case BOOT_DEVICE_NAND:
> >+#endif
> >+#ifdef CONFIG_SPL_ONENAND_SUPPORT
> >+ case BOOT_DEVICE_ONENAND:
> >+#endif
> >+#ifdef CONFIG_SPL_NOR_SUPPORT
> >+ case BOOT_DEVICE_NOR:
> >+#endif
> >+ return spl_ubi_load_image(boot_device);
> >+#else
> > #ifdef CONFIG_SPL_NAND_SUPPORT
> > case BOOT_DEVICE_NAND:
> > return spl_nand_load_image();
> >@@ -298,6 +310,7 @@ static int spl_load_image(u32 boot_device)
> > case BOOT_DEVICE_NOR:
> > return spl_nor_load_image();
> > #endif
> >+#endif /* CONFIG_SPL_UBI */
> > #ifdef CONFIG_SPL_YMODEM_SUPPORT
> > case BOOT_DEVICE_UART:
> > return spl_ymodem_load_image();
> >diff --git a/common/spl/spl_ubi.c b/common/spl/spl_ubi.c
> >new file mode 100644
> >index 0000000..38ddb57
> >--- /dev/null
> >+++ b/common/spl/spl_ubi.c
> >@@ -0,0 +1,73 @@
> >+/*
> >+ * Copyright (C) 2016
> >+ * Ladislav Michl <ladis at linux-mips.org>
> >+ *
> >+ * SPDX-License-Identifier: GPL-2.0+
> >+ */
> >+
> >+#include <common.h>
> >+#include <config.h>
> >+#include <nand.h>
> >+#include <ubispl.h>
> >+#include <spl.h>
> >+
> >+int spl_ubi_load_image(u32 boot_device)
> >+{
> >+ int ret;
> >+ struct image_header *header;
> >+ struct ubispl_info info;
> >+ struct ubispl_load volumes[2];
> >+
> >+#ifdef CONFIG_SPL_NAND_SUPPORT
> >+ if (boot_device == BOOT_DEVICE_NAND)
> >+ nand_init();
> >+#endif
> >+
> >+ /* TODO: Make it decently configurable */
> >+ info.ubi = (struct ubi_scan_info *)
> >+ (CONFIG_SYS_SPL_MALLOC_START + CONFIG_SYS_SPL_MALLOC_SIZE);
> >+ info.fastmap = 1;
> >+ info.read = nand_spl_read_block;
> >+
> >+ info.peb_offset = 4;
> >+ info.peb_size = CONFIG_SYS_NAND_BLOCK_SIZE;
> >+ info.vid_offset = 512;
> >+ info.leb_start = 2048;
>
> this three values should be configurable!
Sure and it is done in next version.
> >+ info.peb_count = CONFIG_SPL_UBI_MAX_PEBS - info.peb_offset;
> >+
> >+#ifdef CONFIG_SPL_OS_BOOT
> >+ if (!spl_start_uboot()) {
> >+ volumes[0].name = "kernel";
>
> Also we should have the names configurable ... not for all boards
> the kernel is stored in "kernel" ...
It is not needed here to make them configurable, because current code
loads volumes by id not by name. Current code loads volumes by id for
a sake of simplicity. Perhaps we could drop name field as it is used
only for pretty print?
[...]
Best regards,
ladis
More information about the U-Boot
mailing list