[U-Boot] [PATCH V3 2/8] Add savebp command
Andreas Bießmann
andreas.devel at googlemail.com
Thu Aug 25 12:37:33 CEST 2011
Dear Simon,
Am 25.08.2011 10:33, schrieb Simon Schwarz:
> This adds a savebp command to the u-boot.
>
> Related config:
> CONFIG_CMD_SAVEBP
> activate/deactivate the command
> CONFIG_CMD_SAVEBP_NAND_OFS
> Offset in NAND to use
> CONFIG_SYS_NAND_SPL_KERNEL_OFFS
> Offset in NAND of direct boot kernel image to use in SPL
This is not used in the code ... why defining it then (here)?
> CONFIG_SYS_SPL_ARGS_ADDR
> Address where the kernel boot arguments are expected - this is
> normally RAM-start + 0x100 (on ARM)
Well this is gd->bd->bi_boot_params (at least on ARM), so why don't you
use that parameter here?
>
> Signed-off-by: Simon Schwarz <simonschwarzcor at gmail.com>
> ---
>
> V2 changes:
> CHG corrected bootm call. Now bootm is called with five parameters including
> Address of FDT in RAM. This fixes the hang on savebp fdt call.
> ADD debug output of the actual bootm parameter call
> CHG help message
>
> V3 changes:
> FIX added missing brackets
> ---
> common/Makefile | 1 +
> common/cmd_savebp.c | 149 ++++++++++++++++++++++++++++++++++++++++++
> include/configs/devkit8000.h | 6 ++
where is the documentation?
> 3 files changed, 156 insertions(+), 0 deletions(-)
> create mode 100644 common/cmd_savebp.c
>
> diff --git a/common/Makefile b/common/Makefile
> index 124a427..0b42968 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -158,6 +158,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_SAVEBP) += cmd_savebp.o
>
> # others
> COBJS-$(CONFIG_DDR_SPD) += ddr_spd.o
> diff --git a/common/cmd_savebp.c b/common/cmd_savebp.c
> new file mode 100644
> index 0000000..4091ccb
> --- /dev/null
> +++ b/common/cmd_savebp.c
> @@ -0,0 +1,149 @@
> +/* Copyright (C) 2011
> + * Corscience GmbH & Co. KG - Simon Schwarz <schwarz at 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>
> +
> +#define TYPE_FDT 0
> +#define TYPE_ATAGS 1
should'nt this go into some header? How about enum here?
> +
> +static inline int str2off(const char *p, loff_t *num)
> +{
> + char *endptr;
> +
> + *num = simple_strtoull(p, &endptr, 16);
> + return *p != '\0' && *endptr == '\0';
> +}
could be merged with the one in cmd_nand.c. Maybe move the one from
cmd_nand.c to some header?
> +
> +int do_savebp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + loff_t offset;
> + int img_type = TYPE_ATAGS;
> + int ret = 0;
> + int bootm_argc = 5;
> + char *bootm_argsv[] = {"do_bootm", "xxxxxxx", "0x82000000", "-",
> + "0x80000100"};
> +
> + offset = (loff_t)CONFIG_CMD_SAVEBP_NAND_OFS;
> + bootm_argsv[2] = getenv("loadaddr");
> + /* - Validate args - */
> + switch (argc) {
> + case 6: /* 4. fdt addr */
^ wrong number?
> + if (strcmp(argv[5], "-"))
> + strcpy(bootm_argsv[4], argv[5]);
If one set '-' as fifth argument bootm_argsv will stay with your given
"0x80000100" ... shouldn't the default argument be pre-calculated at
compile time. I guess the 0x80000100 is CONFIG_SYS_SDRAM_BASE + 0x100 in
your case (or tell it CONFIG_SYS_SPL_ARGS_ADDR).
BTW: it is unsafe to use strcpy() here cause you may have some greater
string in 'src' than in 'dst' you should use strncpy() with strlen(dst)
as 'count' or even strdup() here.
In my opinion it would be best to use something like this here:
---8<---
char *bootm_argsv[5];
bootm_argsv[0] = "bootm";
...
bootm_argsv[4] = strdup(argv[5]);
...
--->8---
Or even
bootm_argsv[4] = argv[5];
But that have to be investigated, I don't know currently if it will work.
> + case 5: /* 5. initrd addr */
^ wrong number?
> + if (strcmp(argv[4], "-"))
> + strcpy(bootm_argsv[3], argv[4]);
> + case 4: /* 3. arg kernel addr */
> + if (strcmp(argv[3], "-"))
> + strcpy(bootm_argsv[2], argv[3]);
> + case 3: /* 2. arg offset */
> + if (strcmp(argv[2], "-")) {
> + if (!str2off(argv[2], &offset)) {
> + printf("'%s' is not a number\n", argv[2]);
> + return cmd_usage(cmdtp);
> + }
> + }
> + case 2: /* 1. arg atags or fdt */
> + if (!strcmp(argv[1], "fdt")) {
> + img_type = TYPE_FDT;
> + bootm_argc = 5;
> + } else if (!strcmp(argv[1], "atags")) {
> + img_type = TYPE_ATAGS;
> + bootm_argc = 4;
> + } else {
> + return cmd_usage(cmdtp);
> + }
> + /* using standard offset */
> + printf("using standard destination at: 0x%x\n",
> + (uint32_t)offset);
> + break;
> + default:
> + return cmd_usage(cmdtp);
> + }
> + debug("using as bootm arsgs: %s / %s / %s / %s / %s\n"
> + , bootm_argsv[0], bootm_argsv[1], bootm_argsv[2],
> + bootm_argsv[3], bootm_argsv[4]);
> +
> + /* - do the work - */
> + /* exec bootm_start as subcommand of do_bootm to init the images
> + * data structure */
> + debug("exec bootm subcommand start\n");
> + bootm_argsv[1] = "start";
> + ret = do_bootm(find_cmd("do_bootm"), 0, bootm_argc, bootm_argsv);
> + debug("Subcommand start bootm retcode: %d\n", ret);
> +
> + debug("exec bootm subcommand loados\n");
> + bootm_argsv[1] = "loados";
> + ret = do_bootm(find_cmd("do_bootm"), 0, bootm_argc, bootm_argsv);
> + debug("Subcommand loados bootm retcode: %d\n", ret);
> +
> +#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
> + debug("exec bootm subcommand ramdisk\n");
> + bootm_argsv[1] = "ramdisk";
> + ret = do_bootm(find_cmd("do_bootm"), 0, bootm_argc, bootm_argsv);
> + debug("Subcommand ramdisk bootm retcode: %d\n", ret);
> +#endif
> +
> +#ifdef CONFIG_OF_LIBFDT
> + if (img_type == TYPE_FDT) {
> + debug("exec bootm subcommand fdt\n");
> + bootm_argsv[1] = "fdt";
> + ret = do_bootm(find_cmd("do_bootm"), 0, bootm_argc,
> + bootm_argsv);
> + debug("Subcommand fdt bootm retcode: %d\n", ret);
> + }
> +#endif
> +
> + debug("exec bootm subcommand cmdline\n");
> + bootm_argsv[1] = "cmdline";
> + ret = do_bootm(find_cmd("do_bootm"), 0, bootm_argc, bootm_argsv);
> + debug("Subcommand cmdline bootm retcode: %d\n", ret);
> +
> + debug("exec bootm bdt cmdline\n");
> + bootm_argsv[1] = "bdt";
> + ret = do_bootm(find_cmd("do_bootm"), 0, bootm_argc, bootm_argsv);
> + debug("Subcommand bdt bootm retcode: %d\n", ret);
> +
> + debug("exec bootm subcommand prep\n");
> + bootm_argsv[1] = "prep";
> + ret = do_bootm(find_cmd("do_bootm"), 0, bootm_argc, bootm_argsv);
> + debug("Subcommand prep bootm retcode: %d\n", ret);
> + if (ret) {
> + printf("ERROR prep subcommand failed!\n");
> + return -1;
> + }
> + /* call arch specific handlers */
> + if (img_type == TYPE_FDT)
> + do_savebp_fdt(offset);
> + else
> + do_savebp_atags(offset);
where are these two do_savebp_x() functions?
> +
> + return 0;
> +}
> +
> +U_BOOT_CMD(
> + savebp, 6 , 1, do_savebp, "save boot params to NAND flash",
> + "[ftd|atags] [nand_offset] [kernel_addr] [initrd_addr] [fdt_addr] \
> + saves the parameter image to NAND. All but the first paramter \
> + can be omitted to use standard values. If nothing or a '-' is \
> + used standardvalues are used where necessary");
> diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
> index 9cbdb5d..80b441a 100644
> --- a/include/configs/devkit8000.h
> +++ b/include/configs/devkit8000.h
> @@ -353,4 +353,10 @@
> #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
> #define CONFIG_SYS_NAND_U_BOOT_SIZE 0x200000
>
> +/* SPL OS boot options */
> +#define CONFIG_CMD_SAVEBP
> +#define CONFIG_CMD_SAVEBP_NAND_OFS (CONFIG_SYS_NAND_SPL_KERNEL_OFFS+\
> + 0x400000)
> +#define CONFIG_SYS_NAND_SPL_KERNEL_OFFS 0x280000
> +#define CONFIG_SYS_SPL_ARGS_ADDR (PHYS_SDRAM_1 + 0x100)
> #endif /* __CONFIG_H */
regards
Andreas Bießmann
More information about the U-Boot
mailing list