Dear Andreas,
On 08/25/2011 12:37 PM, Andreas Bießmann wrote:
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)?
Will move the description to the proper patch.
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?
Because this is used in SPL where the bd struct is not fully initialized.
Signed-off-by: Simon Schwarzsimonschwarzcor@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?
Last patch - will squash it.
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 Schwarzschwarz@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?
Will change.
+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?
Hm, what would be the best place for that? It's at least useful for mmc and nand. common.h? mtd_common.h?
+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?
Changed.
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).
Will change.
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.
will do.
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.
Will think about...
- case 5: /* 5. initrd addr */
^ wrong number?
changed.
<snip>
- }
- /* 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?
later patch will change sequence.
Regards, thanks for the feedback! Simon