diff mbox

[U-Boot,2/6] add header with a generic set of boot commands defined.

Message ID 1395353581-5839-3-git-send-email-dennis@ausil.us
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Dennis Gilmore March 20, 2014, 10:12 p.m. UTC
As the next step in a generic config we are introducing a set of generic boot
paramaters. Depending on the hardwares configuration, booting from supported
hardware will be enabled, mmc, usb, sata, scsi, ide, pxe and dhcp.

There is nothing to stop this being extended to support nand and any other
type of storage that comes along. An ideal future enhancement will be to
allow the user to dynamically reorder the boot devices, and allow one off
boots. for example simply be able to pxe boot to reinstall

Signed-off-by: Dennis Gilmore <dennis@ausil.us>
---
 include/config_distro_bootcmd.h | 208 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 208 insertions(+)
 create mode 100644 include/config_distro_bootcmd.h

Comments

Marek Vasut March 21, 2014, 6:37 p.m. UTC | #1
On Thursday, March 20, 2014 at 11:12:57 PM, Dennis Gilmore wrote:
> As the next step in a generic config we are introducing a set of generic
> boot paramaters. Depending on the hardwares configuration, booting from
> supported hardware will be enabled, mmc, usb, sata, scsi, ide, pxe and
> dhcp.
> 
> There is nothing to stop this being extended to support nand and any other
> type of storage that comes along. An ideal future enhancement will be to
> allow the user to dynamically reorder the boot devices, and allow one off
> boots. for example simply be able to pxe boot to reinstall
> 
> Signed-off-by: Dennis Gilmore <dennis@ausil.us>
> ---
>  include/config_distro_bootcmd.h | 208
> ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 208 insertions(+)
>  create mode 100644 include/config_distro_bootcmd.h
> 
> diff --git a/include/config_distro_bootcmd.h
> b/include/config_distro_bootcmd.h new file mode 100644
> index 0000000..0fe94be
> --- /dev/null
> +++ b/include/config_distro_bootcmd.h
> @@ -0,0 +1,208 @@
> +/*
> + * (C) Copyright 2014
> + * NVIDIA Corporation <www.nvidia.com>
> + *
> + * Copyright 2014 Red Hat, Inc.
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#ifndef _CONFIG_CMD_DISTRO_BOOTCMD_H
> +#define _CONFIG_CMD_DISTRO_BOOTCMD_H
> +
> +
> +#ifdef CONFIG_CMD_MMC
> +#define BOOTCMDS_MMC \
> +	"mmc_boot=" \
> +		"setenv devtype mmc; " \

Please use 'env set ...'

> +		"if mmc dev ${devnum}; then " \
> +			"run scan_boot; " \
> +		"fi\0" \
> +	"bootcmd_mmc0=setenv devnum 0; run mmc_boot;\0" \
> +	"bootcmd_mmc1=setenv devnum 1; run mmc_boot;\0"
> +#define BOOT_TARGETS_MMC "mmc1 mmc0"

This will not work on a boot with three MMC cards ... this does not scale and 
needs re-thinking.

> +#else
> +#define BOOTCMDS_MMC ""
> +#define BOOT_TARGETS_MMC ""

You need to #undef those after you assembled the env, otherwise you will 
propagate those throughout the rest of the U-Boot at build time, which is not 
nice.
[...]
Tom Rini March 21, 2014, 6:48 p.m. UTC | #2
On Thu, Mar 20, 2014 at 05:12:57PM -0500, Dennis Gilmore wrote:

> As the next step in a generic config we are introducing a set of generic boot
> paramaters. Depending on the hardwares configuration, booting from supported
> hardware will be enabled, mmc, usb, sata, scsi, ide, pxe and dhcp.
> 
> There is nothing to stop this being extended to support nand and any other
> type of storage that comes along. An ideal future enhancement will be to
> allow the user to dynamically reorder the boot devices, and allow one off
> boots. for example simply be able to pxe boot to reinstall
[snip]
> +	"bootcmd_mmc0=setenv devnum 0; run mmc_boot;\0" \
> +	"bootcmd_mmc1=setenv devnum 1; run mmc_boot;\0"
> +#define BOOT_TARGETS_MMC "mmc1 mmc0"

This is because we want to prefer eMMC to SD card?  Or is mmc1=SD,
mmc0=eMMC on Tegra?  That's opposite of TI parts so we might need to
#ifndef/define/endif that combo at least.

Otherwise:

Reviewed-by: Tom Rini <trini@ti.com>
Tom Rini March 21, 2014, 6:53 p.m. UTC | #3
On Fri, Mar 21, 2014 at 07:37:52PM +0100, Marek Vasut wrote:
> On Thursday, March 20, 2014 at 11:12:57 PM, Dennis Gilmore wrote:
> > As the next step in a generic config we are introducing a set of generic
> > boot paramaters. Depending on the hardwares configuration, booting from
> > supported hardware will be enabled, mmc, usb, sata, scsi, ide, pxe and
> > dhcp.
> > 
> > There is nothing to stop this being extended to support nand and any other
> > type of storage that comes along. An ideal future enhancement will be to
> > allow the user to dynamically reorder the boot devices, and allow one off
> > boots. for example simply be able to pxe boot to reinstall
> > 
> > Signed-off-by: Dennis Gilmore <dennis@ausil.us>
> > ---
> >  include/config_distro_bootcmd.h | 208
> > ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 208 insertions(+)
> >  create mode 100644 include/config_distro_bootcmd.h
> > 
> > diff --git a/include/config_distro_bootcmd.h
> > b/include/config_distro_bootcmd.h new file mode 100644
> > index 0000000..0fe94be
> > --- /dev/null
> > +++ b/include/config_distro_bootcmd.h
> > @@ -0,0 +1,208 @@
> > +/*
> > + * (C) Copyright 2014
> > + * NVIDIA Corporation <www.nvidia.com>
> > + *
> > + * Copyright 2014 Red Hat, Inc.
> > + *
> > + * SPDX-License-Identifier:     GPL-2.0+
> > + */
> > +
> > +#ifndef _CONFIG_CMD_DISTRO_BOOTCMD_H
> > +#define _CONFIG_CMD_DISTRO_BOOTCMD_H
> > +
> > +
> > +#ifdef CONFIG_CMD_MMC
> > +#define BOOTCMDS_MMC \
> > +	"mmc_boot=" \
> > +		"setenv devtype mmc; " \
> 
> Please use 'env set ...'

Why?  Almost nothing uses that syntax..

> > +		"if mmc dev ${devnum}; then " \
> > +			"run scan_boot; " \
> > +		"fi\0" \
> > +	"bootcmd_mmc0=setenv devnum 0; run mmc_boot;\0" \
> > +	"bootcmd_mmc1=setenv devnum 1; run mmc_boot;\0"
> > +#define BOOT_TARGETS_MMC "mmc1 mmc0"
> 
> This will not work on a boot with three MMC cards ... this does not scale and 
> needs re-thinking.

Maybe once we have device model and can see how many MMCs have been
probed?  I'm not worried about a 3 mmc card system (those are
"expensive" to add, I'd be surprised about a system with some
combination of 3 or more setup for SD/eMMC, rather than saying #3+ is for
SDIO type things and more on-board SD slots being via USB where that's
cheap).
Marek Vasut March 21, 2014, 9 p.m. UTC | #4
On Friday, March 21, 2014 at 07:53:58 PM, Tom Rini wrote:
> On Fri, Mar 21, 2014 at 07:37:52PM +0100, Marek Vasut wrote:
> > On Thursday, March 20, 2014 at 11:12:57 PM, Dennis Gilmore wrote:
> > > As the next step in a generic config we are introducing a set of
> > > generic boot paramaters. Depending on the hardwares configuration,
> > > booting from supported hardware will be enabled, mmc, usb, sata, scsi,
> > > ide, pxe and dhcp.
> > > 
> > > There is nothing to stop this being extended to support nand and any
> > > other type of storage that comes along. An ideal future enhancement
> > > will be to allow the user to dynamically reorder the boot devices, and
> > > allow one off boots. for example simply be able to pxe boot to
> > > reinstall
> > > 
> > > Signed-off-by: Dennis Gilmore <dennis@ausil.us>
> > > ---
> > > 
> > >  include/config_distro_bootcmd.h | 208
> > > 
> > > ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 208
> > > insertions(+)
> > > 
> > >  create mode 100644 include/config_distro_bootcmd.h
> > > 
> > > diff --git a/include/config_distro_bootcmd.h
> > > b/include/config_distro_bootcmd.h new file mode 100644
> > > index 0000000..0fe94be
> > > --- /dev/null
> > > +++ b/include/config_distro_bootcmd.h
> > > @@ -0,0 +1,208 @@
> > > +/*
> > > + * (C) Copyright 2014
> > > + * NVIDIA Corporation <www.nvidia.com>
> > > + *
> > > + * Copyright 2014 Red Hat, Inc.
> > > + *
> > > + * SPDX-License-Identifier:     GPL-2.0+
> > > + */
> > > +
> > > +#ifndef _CONFIG_CMD_DISTRO_BOOTCMD_H
> > > +#define _CONFIG_CMD_DISTRO_BOOTCMD_H
> > > +
> > > +
> > > +#ifdef CONFIG_CMD_MMC
> > > +#define BOOTCMDS_MMC \
> > > +	"mmc_boot=" \
> > > +		"setenv devtype mmc; " \
> > 
> > Please use 'env set ...'
> 
> Why?  Almost nothing uses that syntax..

Because we want to weed out the old/legacy syntax and use the new one . The 
'env' command is also more powerful and setenv/saveenv/... should just go away.

> > > +		"if mmc dev ${devnum}; then " \
> > > +			"run scan_boot; " \
> > > +		"fi\0" \
> > > +	"bootcmd_mmc0=setenv devnum 0; run mmc_boot;\0" \
> > > +	"bootcmd_mmc1=setenv devnum 1; run mmc_boot;\0"
> > > +#define BOOT_TARGETS_MMC "mmc1 mmc0"
> > 
> > This will not work on a boot with three MMC cards ... this does not scale
> > and needs re-thinking.
> 
> Maybe once we have device model and can see how many MMCs have been
> probed?  I'm not worried about a 3 mmc card system (those are
> "expensive" to add, I'd be surprised about a system with some
> combination of 3 or more setup for SD/eMMC

I have one on my table right now right here. I also have a mutilated M28EVK with 
three SD cards , so these systems do exist. Oh, but hey, we can of course say 
"systems with three SD cards are not supported", I saw "someone" from RH take 
that kind of attitude already ...

> , rather than saying #3+ is for
> SDIO type things and more on-board SD slots being via USB where that's
> cheap).

Best regards,
Marek Vasut
Stephen Warren March 25, 2014, 8:36 p.m. UTC | #5
On 03/20/2014 04:12 PM, Dennis Gilmore wrote:
> As the next step in a generic config we are introducing a set of generic boot
> paramaters. Depending on the hardwares configuration, booting from supported
> hardware will be enabled, mmc, usb, sata, scsi, ide, pxe and dhcp.
> 
> There is nothing to stop this being extended to support nand and any other
> type of storage that comes along. An ideal future enhancement will be to
> allow the user to dynamically reorder the boot devices, and allow one off
> boots. for example simply be able to pxe boot to reinstall

One-off boots can already be performed: "run bootcmd_mmc1" or "run
bootcmd_dhcp" work for me.

> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h

> +#ifndef _CONFIG_CMD_DISTRO_BOOTCMD_H
> +#define _CONFIG_CMD_DISTRO_BOOTCMD_H
> +
> +

Nit: double blank line there.

> +#ifdef CONFIG_CMD_MMC
> +#define BOOTCMDS_MMC \
> +	"mmc_boot=" \
> +		"setenv devtype mmc; " \
> +		"if mmc dev ${devnum}; then " \
> +			"run scan_boot; " \
> +		"fi\0" \
> +	"bootcmd_mmc0=setenv devnum 0; run mmc_boot;\0" \
> +	"bootcmd_mmc1=setenv devnum 1; run mmc_boot;\0"
> +#define BOOT_TARGETS_MMC "mmc1 mmc0"

It would be nice if this required the file that includes this file to
define how many MMC devices there are (some only have 1...) and the best
default boot order.

Still, that's probably challenging using the C pre-processor. Perhaps a
Python script to auto-generate these command could be more flexible.
Probably a job for a follow-on patch though.

> +#define BOOTCMDS_COMMON \
> +	"rootpart=1\0" \
> +	\
> +	"do_envimport="                                                   \
> +		"load ${devtype} ${devnum}:${rootpart} ${loadaddr} "      \
> +			"${environment}\0"                                \
> +		"env import -t ${loadaddr} $filesize\0"                   \

I think that should be:

if load ....; then
    env import ...;
fi

So the script doesn't accidentally import something stale at $loadaddr
that wasn't actually loaded.

> +	\
> +	"envimport="                                                      \
> +		"for environment in ${boot_envs}; do "                    \
> +			"if test -e ${devtype} ${devnum}:${rootpart} "    \
> +					"${prefix}${environment}; then "  \
> +				"echo Found U-Boot environment "          \
> +					"${prefix}${environment}; "       \
> +				"run do_envimport;"                       \
> +				"echo Import FAILED; continuing...; "     \
> +			"fi; "                                            \
> +		"done\0"                                                  \
> +	\

envimport doesn't actually seem to be used anywhere at least in this
patch. Is it really needed? It shouldn't be needed for generic distro
support, but more for boards without any non-filesystem-based
environment storage, and in those cases, shouldn't they use
CONFIG_PREBOOT like rpi_b.h does, so it's well-defined where the
environment comes from, before ${boot_targets} is evaluated?

> +	"do_script_boot="                                                 \
> +		"load ${devtype} ${devnum}:${rootpart} "                  \
> +			"${scriptaddr} ${prefix}${script}; "              \
> +		"source ${scriptaddr}\0"                                  \

That should be "if load ; ...; fi" too. Sorry, the Tegra/RPI scripts
give bad examples for this.
Stephen Warren March 25, 2014, 8:38 p.m. UTC | #6
On 03/21/2014 12:48 PM, Tom Rini wrote:
> On Thu, Mar 20, 2014 at 05:12:57PM -0500, Dennis Gilmore wrote:
> 
>> As the next step in a generic config we are introducing a set of generic boot
>> paramaters. Depending on the hardwares configuration, booting from supported
>> hardware will be enabled, mmc, usb, sata, scsi, ide, pxe and dhcp.
>>
>> There is nothing to stop this being extended to support nand and any other
>> type of storage that comes along. An ideal future enhancement will be to
>> allow the user to dynamically reorder the boot devices, and allow one off
>> boots. for example simply be able to pxe boot to reinstall
> [snip]
>> +	"bootcmd_mmc0=setenv devnum 0; run mmc_boot;\0" \
>> +	"bootcmd_mmc1=setenv devnum 1; run mmc_boot;\0"
>> +#define BOOT_TARGETS_MMC "mmc1 mmc0"
> 
> This is because we want to prefer eMMC to SD card?  Or is mmc1=SD,
> mmc0=eMMC on Tegra?  That's opposite of TI parts so we might need to
> #ifndef/define/endif that combo at least.

On most (all?) Tegra devices, mmc1 is SD and mmc0 is eMMC. We prefer to
try to boot from SD first so that the user can just plug in an SD card
to boot from it if they want, but otherwise boot from eMMC (just like
floppy vs HDD on a PC) without having to muck with boot order manually.

Well, that and I almost exclusively boot from SD, so it's marginally
faster and less noisy:-)
diff mbox

Patch

diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
new file mode 100644
index 0000000..0fe94be
--- /dev/null
+++ b/include/config_distro_bootcmd.h
@@ -0,0 +1,208 @@ 
+/*
+ * (C) Copyright 2014
+ * NVIDIA Corporation <www.nvidia.com>
+ *  
+ * Copyright 2014 Red Hat, Inc.
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#ifndef _CONFIG_CMD_DISTRO_BOOTCMD_H
+#define _CONFIG_CMD_DISTRO_BOOTCMD_H
+
+
+#ifdef CONFIG_CMD_MMC
+#define BOOTCMDS_MMC \
+	"mmc_boot=" \
+		"setenv devtype mmc; " \
+		"if mmc dev ${devnum}; then " \
+			"run scan_boot; " \
+		"fi\0" \
+	"bootcmd_mmc0=setenv devnum 0; run mmc_boot;\0" \
+	"bootcmd_mmc1=setenv devnum 1; run mmc_boot;\0"
+#define BOOT_TARGETS_MMC "mmc1 mmc0"
+#else
+#define BOOTCMDS_MMC ""
+#define BOOT_TARGETS_MMC ""
+#endif
+
+#ifdef CONFIG_CMD_USB
+#define BOOTCMD_INIT_USB "run usb_init; "
+#define BOOTCMDS_USB \
+	"usb_init=" \
+		"if ${usb_need_init}; then " \
+			"set usb_need_init false; " \
+			"usb start 0; " \
+		"fi\0" \
+	\
+	"usb_boot=" \
+		"setenv devtype usb; " \
+		BOOTCMD_INIT_USB \
+		"if usb dev ${devnum}; then " \
+			"run scan_boot; " \
+		"fi\0" \
+	\
+	"bootcmd_usb0=setenv devnum 0; run usb_boot;\0" \
+	"bootcmd_usb1=setenv devnum 1; run usb_boot;\0"
+#define BOOT_TARGETS_USB "usb0 usb1"
+#else
+#define BOOTCMD_INIT_USB ""
+#define BOOTCMDS_USB ""
+#define BOOT_TARGETS_USB ""
+#endif
+
+#ifdef CONFIG_CMD_SATA
+#define BOOTCMDS_SATA \
+	"sata_boot=" \
+		"setenv devtype sata; " \
+		"if sata dev ${devnum}; then " \
+			"run scan_boot; " \
+		"fi\0" \
+	\
+	"bootcmd_sata0=setenv devnum 0; run sata_boot;\0" \
+	"bootcmd_sata1=setenv devnum 1; run sata_boot;\0"
+#define BOOT_TARGETS_SATA "sata0 sata1"
+#else
+#define BOOTCMDS_SATA ""
+#define BOOT_TARGETS_SATA ""
+#endif
+
+#ifdef CONFIG_CMD_SCSI
+#define BOOTCMDS_SCSI \
+        "scsi_boot=" \
+                "setenv devtype scsi; " \
+                "if scsi dev ${devnum}; then " \
+                        "run scan_boot; " \
+                "fi\0" \
+        \
+        "bootcmd_scsi0=setenv devnum 0; run scsi_boot;\0" \
+        "bootcmd_scsi1=setenv devnum 1; run scsi_boot;\0"
+#define BOOT_TARGETS_SCSI "scsi0 scsi1"
+#else
+#define BOOTCMDS_SCSI ""
+#define BOOT_TARGETS_SCSI ""
+#endif
+
+#ifdef CONFIG_CMD_IDE
+#define BOOTCMDS_IDE \
+        "ide_boot=" \
+                "setenv devtype ide; " \
+                "if ide dev ${devnum}; then " \
+                        "run scan_boot; " \
+                "fi\0" \
+        \
+        "bootcmd_ide0=setenv devnum 0; run ide_boot;\0" \
+        "bootcmd_ide1=setenv devnum 1; run ide_boot;\0"
+#define BOOT_TARGETS_IDE "ide0 ide1"
+#else
+#define BOOTCMDS_IDE ""
+#define BOOT_TARGETS_IDE ""
+#endif
+
+#ifdef CONFIG_CMD_DHCP
+#define BOOTCMDS_DHCP \
+	"bootcmd_dhcp=" \
+		BOOTCMD_INIT_USB \
+		"if dhcp ${scriptaddr} boot.scr.uimg; then "\
+			"source ${scriptaddr}; " \
+		"fi\0"
+#define BOOT_TARGETS_DHCP "dhcp"
+#else
+#define BOOTCMDS_DHCP ""
+#define BOOT_TARGETS_DHCP ""
+#endif
+
+#if defined(CONFIG_CMD_DHCP) && defined(CONFIG_CMD_PXE)
+#define BOOTCMDS_PXE \
+	"bootcmd_pxe=" \
+		BOOTCMD_INIT_USB \
+		"dhcp; " \
+		"if pxe get; then " \
+			"pxe boot; " \
+		"fi\0"
+#define BOOT_TARGETS_PXE "pxe"
+#else
+#define BOOTCMDS_PXE ""
+#define BOOT_TARGETS_PXE ""
+#endif
+
+#define BOOTCMDS_COMMON \
+	"rootpart=1\0" \
+	\
+	"do_envimport="                                                   \
+		"load ${devtype} ${devnum}:${rootpart} ${loadaddr} "      \
+			"${environment}\0"                                \
+		"env import -t ${loadaddr} $filesize\0"                   \
+	\
+	"envimport="                                                      \
+		"for environment in ${boot_envs}; do "                    \
+			"if test -e ${devtype} ${devnum}:${rootpart} "    \
+					"${prefix}${environment}; then "  \
+				"echo Found U-Boot environment "          \
+					"${prefix}${environment}; "       \
+				"run do_envimport;"                       \
+				"echo Import FAILED; continuing...; "     \
+			"fi; "                                            \
+		"done\0"                                                  \
+	\
+	"do_script_boot="                                                 \
+		"load ${devtype} ${devnum}:${rootpart} "                  \
+			"${scriptaddr} ${prefix}${script}; "              \
+		"source ${scriptaddr}\0"                                  \
+	\
+	"script_boot="                                                    \
+		"for script in ${boot_scripts}; do "                      \
+			"if test -e ${devtype} ${devnum}:${rootpart} "    \
+					"${prefix}${script}; then "       \
+				"echo Found U-Boot script "               \
+					"${prefix}${script}; "            \
+				"run do_script_boot;"                     \
+				"echo SCRIPT FAILED; continuing...; "     \
+			"fi; "                                            \
+		"done\0"                                                  \
+	\
+	"do_sysboot_boot="                                                \
+		"sysboot ${devtype} ${devnum}:${rootpart} any "           \
+			"${scriptaddr} ${prefix}extlinux/extlinux.conf\0" \
+	\
+	"sysboot_boot="                                                   \
+		"if test -e ${devtype} ${devnum}:${rootpart} "            \
+				"${prefix}extlinux/extlinux.conf; then "  \
+			"echo Found extlinux config "                     \
+				"${prefix}extlinux/extlinux.conf; "       \
+			"run do_sysboot_boot;"                            \
+			"echo SYSBOOT FAILED; continuing...; "            \
+		"fi\0"                                                    \
+	\
+	"scan_boot="                                                      \
+		"echo Scanning ${devtype} ${devnum}...; "                 \
+		"for prefix in ${boot_prefixes}; do "                     \
+			"run sysboot_boot; "                              \
+			"run script_boot; "                               \
+		"done\0"                                                  \
+	\
+	"boot_targets=" \
+		BOOT_TARGETS_MMC " "  \
+		BOOT_TARGETS_USB " "  \
+		BOOT_TARGETS_SATA " " \
+		BOOT_TARGETS_SCSI " " \
+		BOOT_TARGETS_IDE " "  \
+		BOOT_TARGETS_PXE " "  \
+		BOOT_TARGETS_DHCP " " \
+		"\0" \
+	\
+	"boot_prefixes=/ /boot/\0" \
+	\
+	"boot_scripts=boot.scr.uimg boot.scr\0" \
+	\
+	"boot_envs=uEnv.txt\0" \
+	\
+	BOOTCMDS_MMC \
+	BOOTCMDS_USB \
+	BOOTCMDS_SATA \
+	BOOTCMDS_SCSI \
+	BOOTCMDS_IDE \
+	BOOTCMDS_DHCP \
+	BOOTCMDS_PXE
+
+#endif  /* _CONFIG_CMD_DISTRO_BOOTCMD_H */