Message ID | 20160112221721.GA23583@localhost.localdomain |
---|---|
State | Superseded |
Headers | show |
Hello Ladislav, Am 12.01.2016 um 23:17 schrieb Ladislav Michl: > 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 :) Its a warning only ... Yes, I am also unhappy to adding blanks in such cases ... maybe this should be discussed in a seperate thread? > Anyway, there is similar "unaligned case" few lines bellow. Perhaps send > additional patch to fix them both? If you fix it, then please in a v4 patch, thanks! >>> 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. Yes indeed ... so let it as it is. >>> 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 :) Please post it as a RFC patch seperate, with for example Marek in Cc, so he can speak up here too. > > commit c092b3c0627dd8d4b3f3d756c58b53fcf205587f > Author: Ladislav Michl <ladis@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@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@linux-mips.org> > + * > + * bootz code: > + * Copyright (C) 2012 Marek Vasut <marek.vasut@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@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@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@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. Thanks! >>> + 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? Ah, correct ... I should try your patches on a board ... also it is in board specific code, so Ok for me. Just if it is only for pretty print .. drop it and save some bytes? So I wait for your v4 round, thanks for your work! bye, Heiko
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@linux-mips.org> + * + * bootz code: + * Copyright (C) 2012 Marek Vasut <marek.vasut@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);