Re: [U-Boot] [PATCH V13 01/12] Add cmd_spl command
On 02/04/2012 11:22 AM, Stefano Babic wrote: From: Simon Schwarzsimonschwarz...@googlemail.com This adds a spl command to the u-boot. Related config: CONFIG_CMD_CPL activate/deactivate the command CONFIG_CMD_SPL_NAND_OFS Offset in NAND to use Signed-off-by: Simon Schwarzsimonschwarz...@gmail.com CC: Tom Rinitom.r...@gmail.com CC: Stefano Babicsba...@denx.de CC: Wolfgang Denkw...@denx.de --- common/Makefile |1 + common/cmd_spl.c| 229 +++ doc/README.commands.spl | 31 +++ include/cmd_spl.h | 30 ++ include/image.h |2 + 5 files changed, 293 insertions(+), 0 deletions(-) create mode 100644 common/cmd_spl.c create mode 100644 doc/README.commands.spl create mode 100644 include/cmd_spl.h diff --git a/common/Makefile b/common/Makefile index 2d9ae8c..910c056 100644 --- a/common/Makefile +++ b/common/Makefile @@ -162,6 +162,7 @@ COBJS-$(CONFIG_USB_STORAGE) += usb_storage.o endif COBJS-$(CONFIG_CMD_XIMG) += cmd_ximg.o COBJS-$(CONFIG_YAFFS2) += cmd_yaffs2.o +COBJS-$(CONFIG_CMD_SPL) += cmd_spl.o # others ifdef CONFIG_DDR_SPD diff --git a/common/cmd_spl.c b/common/cmd_spl.c new file mode 100644 index 000..deab8e9 --- /dev/null +++ b/common/cmd_spl.c @@ -0,0 +1,229 @@ +/* + * Copyright (C) 2011 + * Corscience GmbH Co. KG - Simon Schwarzschw...@corscience.de + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#includecommon.h +#includecommand.h +#includecmd_spl.h + +DECLARE_GLOBAL_DATA_PTR; + +/* Calls bootm with the parameters given */ +static int call_bootm(int argc, char * const argv[], char *subcommand[]) +{ + char *bootm_argv[5]; + char command[] = do_bootm; + + int i = 0; + int ret = 0; + + /* create paramter array */ + bootm_argv[0] = command; + switch (argc) { + case 3: + bootm_argv[4] = argv[2]; /* fdt addr */ + case 2: + bootm_argv[3] = argv[1]; /* initrd addr */ + case 1: + bootm_argv[2] = argv[0]; /* kernel addr */ + } + + + /* +* - do the work - +* exec subcommands of do_bootm to init the images +* data structure +*/ + while (subcommand[i] != NULL) { + bootm_argv[1] = subcommand[i]; + debug(args: %s, %s, %s, %s, %s, %d\n, bootm_argv[0], + bootm_argv[1], bootm_argv[2], bootm_argv[3], + bootm_argv[4], argc); + ret = do_bootm(find_cmd(do_bootm), 0, argc+2, + bootm_argv); + debug(Subcommand retcode: %d\n, ret); + i++; + } + + if (ret) { + printf(ERROR prep subcommand failed!\n); + return -1; + } + + return 0; +} + +/* assemble the bootm paramteres for fdt creation */ +static int spl_export_fdt(int argc, char * const argv[]) +{ +#ifdef CONFIG_OF_LIBFDT + /* Create subcommand string */ + char *subcommand[] = { + start, + loados, +#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH + ramdisk, +#endif + fdt, + cmdline, + bdt, + prep, + NULL}; + + /* inspect paramters and execute bootm */ + argc--; + argv++; + if (call_bootm(argc, argv, subcommand)) + return -1; + + printf(Argument image is now in RAM: 0x%p\n, + (void *)images.ft_addr); + return 0; +#else + printf(Das U-Boot was build without fdt support - aborting\n); + return -1; +#endif +} + +/* assemble the bootm patameters for atags creation */ +static int spl_export_atags(int argc, char * const argv[]) +{ +#if defined(CONFIG_SETUP_MEMORY_TAGS) || \ + defined(CONFIG_CMDLINE_TAG) || \ + defined(CONFIG_INITRD_TAG) || \ + defined(CONFIG_SERIAL_TAG) || \ + defined(CONFIG_REVISION_TAG) + /* Create subcommand string */ + char *subcommand[] = { + start, + loados, +#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH + ramdisk, +#endif + cmdline, + bdt, + prep, + NULL}; + + /* inspect parameters
Re: [U-Boot] [PATCH V13 01/12] Add cmd_spl command
On 13/02/2012 08:54, Wolfgang Denk wrote: Dear Stefano Babic, In message 1328350963-30989-2-git-send-email-sba...@denx.de you wrote: From: Simon Schwarz simonschwarz...@googlemail.com This adds a spl command to the u-boot. Related config: CONFIG_CMD_CPL activate/deactivate the command CONFIG_CMD_CPL??? Is this a typo, and you mean CONFIG_CMD_SPL ? Yes, this a type - must be fixed. CONFIG_CMD_SPL_NAND_OFS Offset in NAND to use What about other storage devices? Booting from other storage devices is not yet implemented, but it is surely desired. I can imagine that after this patchset will flow into mainline, there will be more use cases, and further storages will be added. At the moment, only NAND is implemented and for this reason only CONFIG_CMD_SPL_NAND_OFS is defined. This leaves the possibility to have different offsets from different boot devices for the kernel parameter area. ... +/* Calls bootm with the parameters given */ +static int call_bootm(int argc, char * const argv[], char *subcommand[]) +{ +char *bootm_argv[5]; +char command[] = do_bootm; ... +} Why is this needed? Why don't you run do_bootm() directly? Mmmhh... At least using find_cmd(do_bootm) in do_bootm should be not required, if this is what you mind. +static int spl_export_fdt(int argc, char * const argv[]) +{ ... +/* inspect paramters and execute bootm */ +argc--; +argv++; +if (call_bootm(argc, argv, subcommand)) +return -1; + +printf(Argument image is now in RAM: 0x%p\n, +(void *)images.ft_addr); +return 0; +#else +printf(Das U-Boot was build without fdt support - aborting\n); +return -1; +#endif +} + +/* assemble the bootm patameters for atags creation */ +static int spl_export_atags(int argc, char * const argv[]) +{ ... +/* inspect parameters and execute bootm */ +argc--; +argv++; +if (call_bootm(argc, argv, subcommand)) +return -1; + +printf(Argument image is now in RAM at: 0x%p\n, +(void *)gd-bd-bi_boot_params); +return 0; +#endif +printf(Das U-Boot was build without ATAGS support - aborting\n); +return -1; +} This is basicly identical code, only the data structures differ. Please use a common function, and pass (a pointer to) the data as argument. Clear, thanks, I'll fix it. +if (c) { +cmd = (int)c-cmd; +switch (cmd) { +case SPL_EXPORT_FDT: +argc--; +argv++; +return spl_export_fdt(argc, argv); +break; +case SPL_EXPORT_ATAGS: +argc--; +argv++; +return spl_export_atags(argc, argv); +break; +default: This will also eliminate this switch() - you just have to set the pointer to the respective data to pass. You're right ! +/* + * Arguments: + * 1: subcommand + * 2: image_type + * 3: nand_offset + * 4: kernel_addr + * 5: initrd_addr + * 6: fdt_adr + */ I think it is broken to restrict this by design to NAND booting only. It is not. The comment is wrong and does not correspond to the implemented command. The result of spl export is not written automatically to storage, and this let the possibility to write it into the correct storage device. spl export img=atags|fdt [kernel_addr] [initrd_addr] nand_offset is something obsolete - and the whole comment does not add more info as we can read after the U_BOOT_CMD. I will drop it. This is a command that attemtps to deal with SPL booting in general, so it should support all possible kinds of boot media - NOR, NAND, MMC, SPI flash, USB, ... Right. However, this restriction is not in this patch (apart the wrong comment). I think the command is already general and not limited to a storage device. But there is this limitation in another patch. The restriction about storage media is in board_init_r(), in [PATCH V13 03/12] omap-common: Add NAND SPL linux booting. Maybe you can take a look. Here the selection if booting the kernel or u-boot is taken inside spl_nand_load_image(). So at this point we have already decided which is the storage media, and this function starts the kernel. So maybe we should also rename it to a general name, such as spl_load_image(). Really this function has not a lot to do to do with NAND, except the fact it calls nand_spl_load_image() to copy from NAND to RAM. Your comment applies very well to this function. It should be made more general, maybe adding function pointers that must be called after selecting the right device, something like: device_media-load_image(addr, size); where device_media-load_image points to the specific devic media function to copy an image, for NAND it is nand_spl_load_image. What do you think about ? We have already discussed how we can proceed with this
Re: [U-Boot] [PATCH V13 01/12] Add cmd_spl command
Hello Tom, Tom Rini wrote: On Fri, Feb 10, 2012 at 11:53 AM, Tom Rini tom.r...@gmail.com wrote: On Fri, Feb 10, 2012 at 11:48 AM, Tom Rini tom.r...@gmail.com wrote: On Sat, Feb 4, 2012 at 3:22 AM, Stefano Babic sba...@denx.de wrote: From: Simon Schwarz simonschwarz...@googlemail.com This adds a spl command to the u-boot. Related config: CONFIG_CMD_CPL activate/deactivate the command CONFIG_CMD_SPL_NAND_OFS Offset in NAND to use Signed-off-by: Simon Schwarz simonschwarz...@gmail.com CC: Tom Rini tom.r...@gmail.com CC: Stefano Babic sba...@denx.de CC: Wolfgang Denk w...@denx.de --- common/Makefile |1 + common/cmd_spl.c| 229 +++ doc/README.commands.spl | 31 +++ include/cmd_spl.h | 30 ++ include/image.h |2 + 5 files changed, 293 insertions(+), 0 deletions(-) create mode 100644 common/cmd_spl.c create mode 100644 doc/README.commands.spl create mode 100644 include/cmd_spl.h The include/image.h part seems unnecessary and breaks cam_enc_4xx so I've dropped that from my tree. Whoops, spoke too soon, I'll resolve the cam_enc_4xx conflict and rename one of the variables. I've gone with renaming the cam_enc_4xx variable to 'imgs' and will post the patch shortly. Heiko, if the rename is OK with you can you Ack it so I can push the Linux SPL series soon? Thanks! Done. Thanks for the fix! bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V13 01/12] Add cmd_spl command
Dear Stefano Babic, In message 1328350963-30989-2-git-send-email-sba...@denx.de you wrote: From: Simon Schwarz simonschwarz...@googlemail.com This adds a spl command to the u-boot. Related config: CONFIG_CMD_CPL activate/deactivate the command CONFIG_CMD_CPL??? Is this a typo, and you mean CONFIG_CMD_SPL ? CONFIG_CMD_SPL_NAND_OFS Offset in NAND to use What about other storage devices? ... +/* Calls bootm with the parameters given */ +static int call_bootm(int argc, char * const argv[], char *subcommand[]) +{ + char *bootm_argv[5]; + char command[] = do_bootm; ... +} Why is this needed? Why don't you run do_bootm() directly? +static int spl_export_fdt(int argc, char * const argv[]) +{ ... + /* inspect paramters and execute bootm */ + argc--; + argv++; + if (call_bootm(argc, argv, subcommand)) + return -1; + + printf(Argument image is now in RAM: 0x%p\n, + (void *)images.ft_addr); + return 0; +#else + printf(Das U-Boot was build without fdt support - aborting\n); + return -1; +#endif +} + +/* assemble the bootm patameters for atags creation */ +static int spl_export_atags(int argc, char * const argv[]) +{ ... + /* inspect parameters and execute bootm */ + argc--; + argv++; + if (call_bootm(argc, argv, subcommand)) + return -1; + + printf(Argument image is now in RAM at: 0x%p\n, + (void *)gd-bd-bi_boot_params); + return 0; +#endif + printf(Das U-Boot was build without ATAGS support - aborting\n); + return -1; +} This is basicly identical code, only the data structures differ. Please use a common function, and pass (a pointer to) the data as argument. + if (c) { + cmd = (int)c-cmd; + switch (cmd) { + case SPL_EXPORT_FDT: + argc--; + argv++; + return spl_export_fdt(argc, argv); + break; + case SPL_EXPORT_ATAGS: + argc--; + argv++; + return spl_export_atags(argc, argv); + break; + default: This will also eliminate this switch() - you just have to set the pointer to the respective data to pass. +/* + * Arguments: + * 1: subcommand + * 2: image_type + * 3: nand_offset + * 4: kernel_addr + * 5: initrd_addr + * 6: fdt_adr + */ I think it is broken to restrict this by design to NAND booting only. This is a command that attemtps to deal with SPL booting in general, so it should support all possible kinds of boot media - NOR, NAND, MMC, SPI flash, USB, ... Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de That said, there may be good reasons for what you did beyond obsequi- ous sycophantic parody. Perhaps you might be so kind as to elucidate. -- Tom Christiansen in 5ldjbm$jtk$1...@csnews.cs.colorado.edu ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V13 01/12] Add cmd_spl command
On Sat, Feb 4, 2012 at 3:22 AM, Stefano Babic sba...@denx.de wrote: From: Simon Schwarz simonschwarz...@googlemail.com This adds a spl command to the u-boot. Related config: CONFIG_CMD_CPL activate/deactivate the command CONFIG_CMD_SPL_NAND_OFS Offset in NAND to use Signed-off-by: Simon Schwarz simonschwarz...@gmail.com CC: Tom Rini tom.r...@gmail.com CC: Stefano Babic sba...@denx.de CC: Wolfgang Denk w...@denx.de --- common/Makefile | 1 + common/cmd_spl.c | 229 +++ doc/README.commands.spl | 31 +++ include/cmd_spl.h | 30 ++ include/image.h | 2 + 5 files changed, 293 insertions(+), 0 deletions(-) create mode 100644 common/cmd_spl.c create mode 100644 doc/README.commands.spl create mode 100644 include/cmd_spl.h The include/image.h part seems unnecessary and breaks cam_enc_4xx so I've dropped that from my tree. -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V13 01/12] Add cmd_spl command
On Fri, Feb 10, 2012 at 11:48 AM, Tom Rini tom.r...@gmail.com wrote: On Sat, Feb 4, 2012 at 3:22 AM, Stefano Babic sba...@denx.de wrote: From: Simon Schwarz simonschwarz...@googlemail.com This adds a spl command to the u-boot. Related config: CONFIG_CMD_CPL activate/deactivate the command CONFIG_CMD_SPL_NAND_OFS Offset in NAND to use Signed-off-by: Simon Schwarz simonschwarz...@gmail.com CC: Tom Rini tom.r...@gmail.com CC: Stefano Babic sba...@denx.de CC: Wolfgang Denk w...@denx.de --- common/Makefile | 1 + common/cmd_spl.c | 229 +++ doc/README.commands.spl | 31 +++ include/cmd_spl.h | 30 ++ include/image.h | 2 + 5 files changed, 293 insertions(+), 0 deletions(-) create mode 100644 common/cmd_spl.c create mode 100644 doc/README.commands.spl create mode 100644 include/cmd_spl.h The include/image.h part seems unnecessary and breaks cam_enc_4xx so I've dropped that from my tree. Whoops, spoke too soon, I'll resolve the cam_enc_4xx conflict and rename one of the variables. -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V13 01/12] Add cmd_spl command
On Fri, Feb 10, 2012 at 11:53 AM, Tom Rini tom.r...@gmail.com wrote: On Fri, Feb 10, 2012 at 11:48 AM, Tom Rini tom.r...@gmail.com wrote: On Sat, Feb 4, 2012 at 3:22 AM, Stefano Babic sba...@denx.de wrote: From: Simon Schwarz simonschwarz...@googlemail.com This adds a spl command to the u-boot. Related config: CONFIG_CMD_CPL activate/deactivate the command CONFIG_CMD_SPL_NAND_OFS Offset in NAND to use Signed-off-by: Simon Schwarz simonschwarz...@gmail.com CC: Tom Rini tom.r...@gmail.com CC: Stefano Babic sba...@denx.de CC: Wolfgang Denk w...@denx.de --- common/Makefile | 1 + common/cmd_spl.c | 229 +++ doc/README.commands.spl | 31 +++ include/cmd_spl.h | 30 ++ include/image.h | 2 + 5 files changed, 293 insertions(+), 0 deletions(-) create mode 100644 common/cmd_spl.c create mode 100644 doc/README.commands.spl create mode 100644 include/cmd_spl.h The include/image.h part seems unnecessary and breaks cam_enc_4xx so I've dropped that from my tree. Whoops, spoke too soon, I'll resolve the cam_enc_4xx conflict and rename one of the variables. I've gone with renaming the cam_enc_4xx variable to 'imgs' and will post the patch shortly. Heiko, if the rename is OK with you can you Ack it so I can push the Linux SPL series soon? Thanks! -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH V13 01/12] Add cmd_spl command
From: Simon Schwarz simonschwarz...@googlemail.com This adds a spl command to the u-boot. Related config: CONFIG_CMD_CPL activate/deactivate the command CONFIG_CMD_SPL_NAND_OFS Offset in NAND to use Signed-off-by: Simon Schwarz simonschwarz...@gmail.com CC: Tom Rini tom.r...@gmail.com CC: Stefano Babic sba...@denx.de CC: Wolfgang Denk w...@denx.de --- common/Makefile |1 + common/cmd_spl.c| 229 +++ doc/README.commands.spl | 31 +++ include/cmd_spl.h | 30 ++ include/image.h |2 + 5 files changed, 293 insertions(+), 0 deletions(-) create mode 100644 common/cmd_spl.c create mode 100644 doc/README.commands.spl create mode 100644 include/cmd_spl.h diff --git a/common/Makefile b/common/Makefile index 2d9ae8c..910c056 100644 --- a/common/Makefile +++ b/common/Makefile @@ -162,6 +162,7 @@ COBJS-$(CONFIG_USB_STORAGE) += usb_storage.o endif COBJS-$(CONFIG_CMD_XIMG) += cmd_ximg.o COBJS-$(CONFIG_YAFFS2) += cmd_yaffs2.o +COBJS-$(CONFIG_CMD_SPL) += cmd_spl.o # others ifdef CONFIG_DDR_SPD diff --git a/common/cmd_spl.c b/common/cmd_spl.c new file mode 100644 index 000..deab8e9 --- /dev/null +++ b/common/cmd_spl.c @@ -0,0 +1,229 @@ +/* + * Copyright (C) 2011 + * Corscience GmbH Co. KG - Simon Schwarz schw...@corscience.de + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#include common.h +#include command.h +#include cmd_spl.h + +DECLARE_GLOBAL_DATA_PTR; + +/* Calls bootm with the parameters given */ +static int call_bootm(int argc, char * const argv[], char *subcommand[]) +{ + char *bootm_argv[5]; + char command[] = do_bootm; + + int i = 0; + int ret = 0; + + /* create paramter array */ + bootm_argv[0] = command; + switch (argc) { + case 3: + bootm_argv[4] = argv[2]; /* fdt addr */ + case 2: + bootm_argv[3] = argv[1]; /* initrd addr */ + case 1: + bootm_argv[2] = argv[0]; /* kernel addr */ + } + + + /* +* - do the work - +* exec subcommands of do_bootm to init the images +* data structure +*/ + while (subcommand[i] != NULL) { + bootm_argv[1] = subcommand[i]; + debug(args: %s, %s, %s, %s, %s, %d\n, bootm_argv[0], + bootm_argv[1], bootm_argv[2], bootm_argv[3], + bootm_argv[4], argc); + ret = do_bootm(find_cmd(do_bootm), 0, argc+2, + bootm_argv); + debug(Subcommand retcode: %d\n, ret); + i++; + } + + if (ret) { + printf(ERROR prep subcommand failed!\n); + return -1; + } + + return 0; +} + +/* assemble the bootm paramteres for fdt creation */ +static int spl_export_fdt(int argc, char * const argv[]) +{ +#ifdef CONFIG_OF_LIBFDT + /* Create subcommand string */ + char *subcommand[] = { + start, + loados, +#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH + ramdisk, +#endif + fdt, + cmdline, + bdt, + prep, + NULL}; + + /* inspect paramters and execute bootm */ + argc--; + argv++; + if (call_bootm(argc, argv, subcommand)) + return -1; + + printf(Argument image is now in RAM: 0x%p\n, + (void *)images.ft_addr); + return 0; +#else + printf(Das U-Boot was build without fdt support - aborting\n); + return -1; +#endif +} + +/* assemble the bootm patameters for atags creation */ +static int spl_export_atags(int argc, char * const argv[]) +{ +#if defined(CONFIG_SETUP_MEMORY_TAGS) || \ + defined(CONFIG_CMDLINE_TAG) || \ + defined(CONFIG_INITRD_TAG) || \ + defined(CONFIG_SERIAL_TAG) || \ + defined(CONFIG_REVISION_TAG) + /* Create subcommand string */ + char *subcommand[] = { + start, + loados, +#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH + ramdisk, +#endif + cmdline, + bdt, + prep, + NULL}; + + /* inspect parameters and execute bootm */ + argc--; + argv++;