diff mbox

[U-Boot,V9,1/4] Add cmd_spl command

Message ID 1323196478-14254-2-git-send-email-simonschwarzcor@gmail.com
State Superseded, archived
Delegated to: Tom Rini
Headers show

Commit Message

Simon Schwarz Dec. 6, 2011, 6:34 p.m. UTC
From: Simon Schwarz <simonschwarzcor@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 <simonschwarzcor@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

V4 changes:
CHG Corrected argument number in comments
CHG added check for CONFIG_OF_LIBFDT
CHG squashed the README to this commit
DEL define description from commit message - unused in this patch
CHG renamed to spl now with subcommand export, very different now
ADD New call style with subcommands.
CHG added printf where the image is located
CHG Patched README to reflect changes
CHG parameter count
CHG usage message

V5 changes:
nothing

V6 changes:
nothing

V7 changes:
FIX multiline comment style, cosmetic changes
	(http://article.gmane.org/gmane.comp.boot-loaders.u-boot/113499)
REBASE on u-boot
---
 common/Makefile              |    1 +
 common/cmd_spl.c             |  224 ++++++++++++++++++++++++++++++++++++++++++
 doc/README.commands.spl      |   31 ++++++
 include/cmd_spl.h            |   30 ++++++
 include/configs/devkit8000.h |    7 ++
 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

Comments

Mike Frysinger Dec. 8, 2011, 12:48 a.m. UTC | #1
On Tuesday 06 December 2011 13:34:35 Simon Schwarz wrote:
> --- /dev/null
> +++ b/common/cmd_spl.c
>
> +int call_bootm(int argc, char * const argv[], char *subcommand[])

static

> +int spl_export_fdt(int argc, char * const argv[])

static


> +#ifdef CONFIG_OF_LIBFDT
> +	/* Create subcommand string */
> +	char *subcommand[] = {"start",

that start needs to be on a new line

> +	'\0'};

if this were NULL (and the call_bootm() checked for that) would be more 
natural to argv[] processing

> +int spl_export_atags(int argc, char * const argv[])

static

> +	char *subcommand[] = {"start", "loados",
> +#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
> +	"ramdisk",
> +#endif
> +	"cmdline", "bdt", "prep", '\0'};

char *subcommand[] = {
	... some strings ...
	... some more strings ...
};

> +int spl_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

static

> +{
> +

delete that newline

> +	cmd_tbl_t *c;

i think you can const this ...

> +int do_spl(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

static

> +	cmd_tbl_t *c;

i think you can const this ...

> +		cmd = (int)c->cmd;

you're casting a pointer to an integer ?  wtf is going on ?

> --- /dev/null
> +++ b/include/cmd_spl.h

needs #ifdef protection against multiple inclusion

> +extern bootm_headers_t images;

i get the feeling this isn't the right place for this and it should be in 
include/image.h instead ...

> +enum image_type {FDT, ATAGS};

these names are too short, and the "image" namespace is taken by image.h 
already ... but leading on to the following defines ...

> +#define SPL_EXPORT	(0x00000001)
> +
> +#define SPL_EXPORT_FDT		(0x00000001)
> +#define SPL_EXPORT_ATAGS	(0x00000002)

why do these need to be defines ?
enum spl_export_type {
	SPL_EXPORT_FDT = 1,
	SPL_EXPORT_ATAGS = 2,
};

also, drop the paren here

> --- a/include/configs/devkit8000.h
> +++ b/include/configs/devkit8000.h

generally board updates should be a sep commit
-mike
Simon Schwarz Dec. 12, 2011, 5:55 p.m. UTC | #2
Hi Mike,

2011/12/8 Mike Frysinger <vapier@gentoo.org>:
> On Tuesday 06 December 2011 13:34:35 Simon Schwarz wrote:
>> --- /dev/null
>> +++ b/common/cmd_spl.c
>>
>> +int call_bootm(int argc, char * const argv[], char *subcommand[])
>
> static
done.
>
>> +int spl_export_fdt(int argc, char * const argv[])
>
> static
>
done.
>
>> +#ifdef CONFIG_OF_LIBFDT
>> +     /* Create subcommand string */
>> +     char *subcommand[] = {"start",
>
> that start needs to be on a new line
>
>> +     '\0'};

ok. Will change this to NULL.
>
> if this were NULL (and the call_bootm() checked for that) would be more
> natural to argv[] processing
>
>> +int spl_export_atags(int argc, char * const argv[])
>
> static

done
>
>> +     char *subcommand[] = {"start", "loados",
>> +#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
>> +     "ramdisk",
>> +#endif
>> +     "cmdline", "bdt", "prep", '\0'};
>
> char *subcommand[] = {
>        ... some strings ...
>        ... some more strings ...
> };

done
>
>> +int spl_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>
> static
>
done
>> +{
>> +
>
> delete that newline
>
done
>> +     cmd_tbl_t *c;
>
> i think you can const this ...
>
right should work -> done.
>> +int do_spl(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>
> static
>
done.
>> +     cmd_tbl_t *c;
>
> i think you can const this ...
>
done.
>> +             cmd = (int)c->cmd;
>
> you're casting a pointer to an integer ?  wtf is going on ?
>
This is the same as in bootm.c. cmd is overloaded with the state of
the statemachine. It is used as an integer.
>> --- /dev/null
>> +++ b/include/cmd_spl.h
>
> needs #ifdef protection against multiple inclusion
>
right. done.
>> +extern bootm_headers_t images;
>
> i get the feeling this isn't the right place for this and it should be in
> include/image.h instead ...
>

I trust you with this ;)

>> +enum image_type {FDT, ATAGS};
>
> these names are too short, and the "image" namespace is taken by image.h
> already ... but leading on to the following defines ...

Hm - it seems that they are not used anyway. deleted.

>
>> +#define SPL_EXPORT   (0x00000001)
>> +
>> +#define SPL_EXPORT_FDT               (0x00000001)
>> +#define SPL_EXPORT_ATAGS     (0x00000002)
>
> why do these need to be defines ?
> enum spl_export_type {
>        SPL_EXPORT_FDT = 1,
>        SPL_EXPORT_ATAGS = 2,
> };
>
> also, drop the paren here
>

I used bootm.c as reference - it's the same there.

>> --- a/include/configs/devkit8000.h
>> +++ b/include/configs/devkit8000.h
>
> generally board updates should be a sep commit

will do.
> -mike

Thanks!
Simon
diff mbox

Patch

diff --git a/common/Makefile b/common/Makefile
index 1b672ad..4056e41 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -164,6 +164,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 0000000..eb498c6
--- /dev/null
+++ b/common/cmd_spl.c
@@ -0,0 +1,224 @@ 
+/*
+ * Copyright (C) 2011
+ * Corscience GmbH & Co. KG - Simon Schwarz <schwarz@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 */
+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] != '\0') {
+		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 */
+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",
+	'\0'};
+
+	/* 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 */
+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", '\0'};
+
+	/* 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;
+}
+
+static cmd_tbl_t cmd_spl_export_sub[] = {
+	U_BOOT_CMD_MKENT(fdt, 0, 1, (void *)SPL_EXPORT_FDT, "", ""),
+	U_BOOT_CMD_MKENT(atags, 0, 1, (void *)SPL_EXPORT_ATAGS, "", ""),
+};
+
+int spl_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+
+	cmd_tbl_t *c;
+	int cmd;
+
+	if (argc < 2) /* no subcommand */
+		return cmd_usage(cmdtp);
+
+	c = find_cmd_tbl(argv[1], &cmd_spl_export_sub[0],
+		ARRAY_SIZE(cmd_spl_export_sub));
+	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:
+			/* unrecognized command */
+			return cmd_usage(cmdtp);
+		}
+	} else {
+		/* Unrecognized command */
+		return cmd_usage(cmdtp);
+	}
+
+	return 0;
+}
+
+static cmd_tbl_t cmd_spl_sub[] = {
+	U_BOOT_CMD_MKENT(export, 0, 1, (void *)SPL_EXPORT, "", ""),
+};
+
+int do_spl(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	cmd_tbl_t *c;
+	int cmd;
+
+	if (argc < 2) /* no subcommand */
+		return cmd_usage(cmdtp);
+
+	c = find_cmd_tbl(argv[1], &cmd_spl_sub[0], ARRAY_SIZE(cmd_spl_sub));
+	if (c) {
+		cmd = (int)c->cmd;
+		switch (cmd) {
+		case SPL_EXPORT:
+			argc--;
+			argv++;
+			if (spl_export(cmdtp, flag, argc, argv))
+				printf("Subcommand failed\n");
+			break;
+		default:
+			/* unrecognized command */
+			return cmd_usage(cmdtp);
+		}
+	} else {
+		/* Unrecognized command */
+		return cmd_usage(cmdtp);
+	}
+	return 0;
+}
+
+/*
+ * Arguments:
+ * 1: subcommand
+ * 2: image_type
+ * 3: nand_offset
+ * 4: kernel_addr
+ * 5: initrd_addr
+ * 6: fdt_adr
+ */
+
+U_BOOT_CMD(
+	spl, 6 , 1, do_spl, "SPL configuration",
+	"export <img=atags|fdt> [kernel_addr] [initrd_addr] "
+	"[fdt_addr if <img> = fdt] - export a kernel parameter image\n"
+	"\t initrd_img can be set to \"-\" if fdt_addr without initrd img is"
+	"used");
diff --git a/doc/README.commands.spl b/doc/README.commands.spl
new file mode 100644
index 0000000..818dd53
--- /dev/null
+++ b/doc/README.commands.spl
@@ -0,0 +1,31 @@ 
+The spl command is used to export a boot parameter image to RAM. Later
+it may implement more functions connected to the SPL.
+
+SUBCOMMAND EXPORT
+To execute the command everything has to be in place as if bootm should be
+used. (kernel image, initrd-image, fdt-image etc.)
+
+export has to subcommands:
+	atags: exports the ATAGS
+	fdt: exports the FDT
+
+Call is:
+spl export <ftd|atags> [kernel_addr] [initrd_addr] [fdt_addr if fdt]
+
+
+TYPICAL CALL
+
+on OMAP3:
+nandecc hw
+nand read 0x82000000 0x280000 0x400000 	/* Read kernel image from NAND*/
+spl export atags 			/* export ATAGS */
+nand erase 0x680000 0x20000		/* erase - one page */
+nand write 0x80000100 0x680000 0x20000	/* write the image - one page */
+
+call with FDT:
+nandecc hw
+nand read 0x82000000 0x280000 0x400000 	/* Read kernel image from NAND*/
+tftpboot 0x80000100 devkit8000.dtb /* Read fdt */
+spl export fdt 0x82000000 - 0x80000100	/* export FDT */
+nand erase 0x680000 0x20000		/* erase - one page */
+nand write <adress shown by spl export> 0x680000 0x20000
diff --git a/include/cmd_spl.h b/include/cmd_spl.h
new file mode 100644
index 0000000..dd3431d
--- /dev/null
+++ b/include/cmd_spl.h
@@ -0,0 +1,30 @@ 
+/* Copyright (C) 2011
+ * Corscience GmbH & Co. KG - Simon Schwarz <schwarz@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
+ */
+
+extern bootm_headers_t images;
+
+enum image_type {FDT, ATAGS};
+
+#define SPL_EXPORT	(0x00000001)
+
+#define SPL_EXPORT_FDT		(0x00000001)
+#define SPL_EXPORT_ATAGS	(0x00000002)
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
index a2e1d2d..bb7509b 100644
--- a/include/configs/devkit8000.h
+++ b/include/configs/devkit8000.h
@@ -368,4 +368,11 @@ 
 #define CONFIG_SYS_SPL_MALLOC_START	0x80208000
 #define CONFIG_SYS_SPL_MALLOC_SIZE	0x100000	/* 1 MB */
 
+/* SPL OS boot options */
+#define CONFIG_CMD_SPL
+#define CONFIG_CMD_SPL_WRITE_SIZE	0x400 /* 1024 byte */
+#define CONFIG_CMD_SPL_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 */