| Message ID | 1309872039-15316-1-git-send-email-simonschwarzcor@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, 5 Jul 2011 15:20:39 +0200 Simon Schwarz <simonschwarzcor@googlemail.com> wrote: > +#ifdef CONFIG_SAVE_BOOT_ARGS > +/* This function writes given bootparams to NAND flash > + * adr: Start adress of Kernel parameter image (ATAGS, FDT) > + * length: length of the image in byte > + * > + * borrowd heavily from common/cmd_nand.c > + */ > +void boot_params_to_nand(u_char *adr, size_t length) { Brace on its own line for functions > + /* write */ > + if(nand_write_skip_bad(nand, off, &length, adr, 0)) > + printf("FAILED!\n"); Space after "if" > +#ifdef CONFIG_SAVE_BOOT_ARGS > + struct tag *t; > + size_t size=0; > +#endif Spaces around = > if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) > return 1; > > @@ -150,6 +198,17 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) > setup_end_tag(bd); > #endif > > +#ifdef CONFIG_SAVE_BOOT_ARGS > + printf("write ATAGS to NAND...\n"); > + > + /* get size of atags */ > + for_each_tag(t, (struct tag *)(bd->bi_boot_params)) > + size += t->hdr.size; > + size += 2; /* ATAG_NONE has size 0 */ > + size *= 4; /* words -> byte! */ > + boot_params_to_nand((u_char *)bd->bi_boot_params, size); > +#endif > + > announce_and_cleanup(); > > kernel_entry(0, machid, bd->bi_boot_params); > @@ -208,6 +267,11 @@ static int bootm_linux_fdt(int machid, bootm_headers_t *images) > > fdt_initrd(*of_flat_tree, *initrd_start, *initrd_end, 1); > > +#ifdef CONFIG_SAVE_BOOT_ARGS > + printf("write FDT to NAND...\n"); > + boot_params_to_nand((u_char *)(*of_flat_tree),of_size); > +#endif Why are you writing to NAND as part of bootm? If you just want access to the results of the FDT/ATAG preparation that bootm does, consider using the bootm subcommands to prepare them, then normal U-Boot commands to write to NAND. -Scott
Hi, thanks for your feedback! I will address the style problems - didn't run checkpatch for the RFC - sorry my fault. > Why are you writing to NAND as part of bootm? > > If you just want access to the results of the FDT/ATAG preparation that > bootm does, consider using the bootm subcommands to prepare them, then > normal U-Boot commands to write to NAND. Hm thats a point. I see this as a feature as it saves new settings without the need of special commands - you start the same configuration either with u-boot or the SPL. But you are right - an extra command would also be nice. I will add this. Regards Simon
> If you just want access to the results of the FDT/ATAG preparation that > bootm does, consider using the bootm subcommands to prepare them, then > normal U-Boot commands to write to NAND. Me again. I rethought and the problem with bootm is that the subfunctions (bootm_linux_fdt/do_bootm_linux) are directly boot the kernel. That means either I copy almost the whole function and delete the kernel_entry-call. Or I move the tag/fdt creation out of bootm-command and create a extra function. The first way I consider bad because of code duplication - therefore I would take the second path. Any Objections? Regards Simon
On Wed, 6 Jul 2011 09:42:52 +0200 Simon Schwarz <simonschwarzcor@googlemail.com> wrote: > Hi, > > thanks for your feedback! > > I will address the style problems - didn't run checkpatch for the RFC > - sorry my fault. > > > Why are you writing to NAND as part of bootm? > > > > If you just want access to the results of the FDT/ATAG preparation that > > bootm does, consider using the bootm subcommands to prepare them, then > > normal U-Boot commands to write to NAND. > > Hm thats a point. I see this as a feature as it saves new settings > without the need of special commands But that's not what bootm is for. If you're doing something new, it should be in a new command -- or in a script of existing commands. -Scott
Hi Simon,
Le 05/07/2011 15:20, Simon Schwarz a écrit :
> This is an RFC on how this can be done.
Then please tag the subject with [RFC] next time!
Amicalement,
> But that's not what bootm is for. If you're doing something new, it should > be in a new command -- or in a script of existing commands. True. The upper patch was the quickest way to do this. I implemented a first test for a command. This is a bit ugly because I have to rip apart bootm. For now I heavily used extern and removed static keywords from bootm - this is ok for the testing but not to get it into mainline... I'am not sure how i can handle this to get it into mainline. The main problem is in arch/arm/lib/bootm.c implementation: static void setup_start_tag (bd_t *bd); static void setup_memory_tags (bd_t *bd); static void setup_commandline_tag (bd_t *bd, char *commandline); static void setup_initrd_tag (bd_t *bd, ulong initrd_start, ulong initrd_end); static void setup_end_tag (bd_t *bd); static struct tag *params; static ulong get_sp(void); static int bootm_linux_fdt(int machid, bootm_headers_t *images); These function i need to build the atags. But they are defined static and not really meant to be used from the outside. What comes to my mind here is to create a new setup_atags.c in the same directory + a header in include. Is this the right place for this? Maybe another solution? Regards Simon
On Thu, 7 Jul 2011 17:50:12 +0200 Simon Schwarz <simonschwarzcor@googlemail.com> wrote: > > But that's not what bootm is for. If you're doing something new, it should > > be in a new command -- or in a script of existing commands. > > True. The upper patch was the quickest way to do this. > > I implemented a first test for a command. This is a bit ugly because I > have to rip apart bootm. For now I heavily used extern and removed > static keywords from bootm - this is ok for the testing but not to get > it into mainline... > > I'am not sure how i can handle this to get it into mainline. The main > problem is in arch/arm/lib/bootm.c implementation: > static void setup_start_tag (bd_t *bd); > static void setup_memory_tags (bd_t *bd); > static void setup_commandline_tag (bd_t *bd, char *commandline); > static void setup_initrd_tag (bd_t *bd, ulong initrd_start, > ulong initrd_end); > static void setup_end_tag (bd_t *bd); > static struct tag *params; > static ulong get_sp(void); > static int bootm_linux_fdt(int machid, bootm_headers_t *images); > > These function i need to build the atags. But they are defined static > and not really meant to be used from the outside. What comes to my > mind here is to create a new setup_atags.c in the same directory + a > header in include. Is this the right place for this? Maybe another > solution? Look at bootm subcommands to run just the portion of the existing bootm functionality that you need. Type "help bootm" for details. -Scott
> Look at bootm subcommands to run just the portion of the existing bootm > functionality that you need. Type "help bootm" for details. Of course I could implement a subcommand - actually this seems to be the best method. IMHO "prep" would be the right one to use? correct? Regards Simon
On Fri, 8 Jul 2011 09:53:46 +0200 Simon Schwarz <simonschwarzcor@googlemail.com> wrote: > > Look at bootm subcommands to run just the portion of the existing bootm > > functionality that you need. Type "help bootm" for details. > > Of course I could implement a subcommand - actually this seems to be > the best method. > > IMHO "prep" would be the right one to use? correct? Probably. -Scott
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 802e833..57e08d3 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -5,6 +5,10 @@ * * Copyright (C) 2001 Erik Mouw (J.A.K.Mouw@its.tudelft.nl) * + * Copyright (C) 2011 + * Corscience GmbH & Co. KG - Simon Schwarz <schwarz@corscience.de> + * - added saving of kernel-parameter-image to NAND flash + * * 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 @@ -30,6 +34,14 @@ #include <libfdt.h> #include <fdt_support.h> +#ifdef CONFIG_SAVE_BOOT_ARGS +#include <nand.h> +#include <asm/setup.h> +#ifdef CONFIG_OMAP34XX +#include <asm/arch/sys_proto.h> +#endif +#endif + DECLARE_GLOBAL_DATA_PTR; #if defined (CONFIG_SETUP_MEMORY_TAGS) || \ @@ -58,6 +70,38 @@ static ulong get_sp(void); static int bootm_linux_fdt(int machid, bootm_headers_t *images); #endif +#ifdef CONFIG_SAVE_BOOT_ARGS +/* This function writes given bootparams to NAND flash + * adr: Start adress of Kernel parameter image (ATAGS, FDT) + * length: length of the image in byte + * + * borrowd heavily from common/cmd_nand.c + */ +void boot_params_to_nand(u_char *adr, size_t length) { + nand_info_t *nand = &nand_info[nand_curr_device]; /* use current dev */ + loff_t off = CONFIG_BOOT_ARGS_NAND_OFS; + nand_erase_options_t opts; + +#ifdef CONFIG_OMAP34XX + omap_nand_switch_ecc(1); /* use hw ecc on omap for SPL compat */ +#endif + /* erase */ + memset(&opts, 0, sizeof(opts)); + opts.offset = off; + opts.length = length; + opts.quiet = 1; + opts.spread = 1; + nand_erase_opts(nand,&opts); + + /* write */ + if(nand_write_skip_bad(nand, off, &length, adr, 0)) + printf("FAILED!\n"); + + printf("Written to offset 0x%llX, size: %d bytes\n", + off, length); +} +#endif /* CONFIG_SAVE_BOOT_ARGS */ + void arch_lmb_reserve(struct lmb *lmb) { ulong sp; @@ -104,6 +148,10 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) char *commandline = getenv ("bootargs"); #endif +#ifdef CONFIG_SAVE_BOOT_ARGS + struct tag *t; + size_t size=0; +#endif if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1; @@ -150,6 +198,17 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) setup_end_tag(bd); #endif +#ifdef CONFIG_SAVE_BOOT_ARGS + printf("write ATAGS to NAND...\n"); + + /* get size of atags */ + for_each_tag(t, (struct tag *)(bd->bi_boot_params)) + size += t->hdr.size; + size += 2; /* ATAG_NONE has size 0 */ + size *= 4; /* words -> byte! */ + boot_params_to_nand((u_char *)bd->bi_boot_params, size); +#endif + announce_and_cleanup(); kernel_entry(0, machid, bd->bi_boot_params); @@ -208,6 +267,11 @@ static int bootm_linux_fdt(int machid, bootm_headers_t *images) fdt_initrd(*of_flat_tree, *initrd_start, *initrd_end, 1); +#ifdef CONFIG_SAVE_BOOT_ARGS + printf("write FDT to NAND...\n"); + boot_params_to_nand((u_char *)(*of_flat_tree),of_size); +#endif + announce_and_cleanup(); kernel_entry(0, machid, *of_flat_tree); diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h index 1bf6bea..3061fc2 100644 --- a/include/configs/devkit8000.h +++ b/include/configs/devkit8000.h @@ -345,4 +345,8 @@ CONFIG_SYS_INIT_RAM_SIZE - \ GENERATED_GBL_DATA_SIZE) +/* Direct OS boot options */ +#define CONFIG_SAVE_BOOT_ARGS +#define CONFIG_BOOT_ARGS_NAND_OFS 0x700000 + #endif /* __CONFIG_H */
Adds the saving of either ATAGS or FDT kernel argument image to NAND flash. This image then can be used in SPL boot. This adds two CONFIG_ paramter to board configuration (in this RFC as example added to devkit8000.h): CONFIG_SAVE_BOOT_ARGS makes the feature active CONFIG_BOOT_ARGS_NAND_OFS defines the offset in NAND flash where the image is saved For OMAP34XX the image is saved with hw-ecc. Signed-off-by: Simon Schwarz <simonschwarzcor@gmail.com> --- This is an RFC on how this can be done. What I don't like here is the omap specific hw ecc switch. But the nand-spl has just hw-ecc yet - at least afaik. Belonging discussion: http://marc.info/?t=130942998000003 Regards Simon arch/arm/lib/bootm.c | 64 ++++++++++++++++++++++++++++++++++++++++++ include/configs/devkit8000.h | 4 ++ 2 files changed, 68 insertions(+), 0 deletions(-)