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

[Stephen Warren]

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.