From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 10 Sep 2015 20:36:14 +0200
Subject: [U-Boot] [RFC PATCH 2/2] sunxi: add "fel" boot target
In-Reply-To: <1441289520-22749-3-git-send-email-bernhard.nortmann@web.de>
References: <1441289520-22749-1-git-send-email-bernhard.nortmann@web.de>
 <1441289520-22749-3-git-send-email-bernhard.nortmann@web.de>
Message-ID: <55F1CD9E.6080303@redhat.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi,

On 03-09-15 16:12, Bernhard Nortmann wrote:
> This patch makes use of the previous changes to add a new "fel" boot
> target for sunxi boards.
>
> When booting via FEL, it's often desirable to work around the absence
> of other (usable) boot devices - or to be able to override them,
> deviating from the standard boot sequence. To achieve this, the "fel"
> boot target gets the highest priority, but won't actually do anything
> unless certain criteria are met.
>
> The "bootcmd_fel" implementation proposed here first tests if an actual
> FEL boot takes place (using the "fel_booted" env var), and secondly
> checks that "fel_data_addr" was set (originating from the 'loader',
> i.e. the sunxi-tools fel utility). If both checks pass, then it will
> try to execute a boot script (boot.scr) at the given address. In case
> of an error (e.g. an invalid image), the source command might return
> "false", causing "distro_bootcmd" to proceed with the next boot target.
>
> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
>
> ---
>
>   include/configs/sunxi-common.h | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 48cc4ed..750355d 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -423,7 +423,20 @@ extern int soft_i2c_gpio_scl;
>   #define BOOT_TARGET_DEVICES_USB(func)
>   #endif
>
> +/* FEL boot support, auto-execute boot.scr if a script address was provided */
> +#define BOOTENV_DEV_FEL(devtypeu, devtypel, instance) \
> +	"bootcmd_fel=" \
> +		"if test -n ${fel_booted}; then " \
> +			"echo '(FEL boot)';" \
> +			"if test -n ${fel_data_addr}; then " \
> +				"source ${fel_data_addr}; " \
> +			"fi; " \
> +		"fi\0"

I would prefer to have this like this:

	"bootcmd_fel=" \
		"if test -n ${fel_booted} && test -n ${fel_data_addr}; then " \
			"echo '(FEL boot)';" \
			"source ${fel_data_addr}; " \
		"fi\0"

Also if we are not using fel_data_size, then why do we even
have it ?

Regards,

Hans



> +#define BOOTENV_DEV_NAME_FEL(devtypeu, devtypel, instance) \
> +	"fel "
> +
>   #define BOOT_TARGET_DEVICES(func) \
> +	func(FEL, fel, na) \
>   	BOOT_TARGET_DEVICES_MMC(func) \
>   	BOOT_TARGET_DEVICES_SCSI(func) \
>   	BOOT_TARGET_DEVICES_USB(func) \
>