[U-Boot] [PATCH] imx: nitrogen6x: Convert Sabrelite to distro boot support

[Gary Bisson <gary.bisson@boundarydevices.com>]

Hi Guillaume,

Thanks for your patch! Switching the upstream nitrogen6x configuration
to distro bootcmd has been in our todo list for some time since we also
use that in our own git repo.

On Fri, Apr 06, 2018 at 12:05:48PM +0200, Guillaume GARDET wrote:
> Boot tested with boot.scr script and EFI/Grub2 on mmc0 and mmc1 slots.
> 
> Signed-off-by: Guillaume GARDET <guillaume.gardet@free.fr>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> ---
>  include/configs/nitrogen6x.h | 86 ++++++++++----------------------------------
>  1 file changed, 18 insertions(+), 68 deletions(-)
> 
> diff --git a/include/configs/nitrogen6x.h b/include/configs/nitrogen6x.h
> index c73cfb7f7e..2e2c499cb6 100644
> --- a/include/configs/nitrogen6x.h
> +++ b/include/configs/nitrogen6x.h
> @@ -102,82 +102,32 @@
>  #define CONFIG_UMSDEVS CONFIG_DRIVE_SATA CONFIG_DRIVE_MMC
>  
>  #if defined(CONFIG_SABRELITE)

Please also switch nitrogen6x to it, not only sabrelite.

> +#define BOOT_TARGET_DEVICES(func) \
> +	func(MMC, mmc, 0) \
> +	func(MMC, mmc, 1) \
> +	func(SATA, sata, 0) \
> +	func(USB, usb, 0) \
> +	func(PXE, pxe, na) \

That currently doesn't build because CMD_PXE isn't selected in the
sabrelite defconfig.
"include/config_distro_bootcmd.h:319:2: error: expected ‘}’ before
‘BOOT_TARGET_DEVICES_references_PXE_without_CONFIG_CMD_DHCP_or_PXE’"

Which brings a good point, all the above should depend on the storage
command being present, like done here [1].

> +	func(DHCP, dhcp, na)
> +
> +#include <config_distro_bootcmd.h>
> +
>  #define CONFIG_EXTRA_ENV_SETTINGS \
> -	"script=boot.scr\0" \
> -	"uimage=uImage\0" \
>  	"console=ttymxc1\0" \
>  	"fdt_high=0xffffffff\0" \
>  	"initrd_high=0xffffffff\0" \
> -	"fdt_file=imx6q-sabrelite.dtb\0" \
> +	"fdtfile=imx6q-sabrelite.dtb\0" \

I wish the default efi_fdtfile would work, Fabio is there any plan to
populate the ${soc} variable like it is done for i.MX7? [2]
Then a small patch in the nitrogen6x.c would set the proper board env
variable.

That way there would be no difference in this header between sabrelite
vs. nitrogen6x.

>  	"fdt_addr=0x18000000\0" \

Do we need to keep fdt_addr since we define fdt_addr_r below?

> -	"boot_fdt=try\0" \
> +	"fdt_addr_r=0x18000000\0" \
> +	"kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0"  \
> +	"pxefile_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \
> +	"scriptaddr=" __stringify(CONFIG_LOADADDR) "\0" \
> +	"ramdisk_addr_r=0x13000000\0" \
> +	"ramdiskaddr=0x13000000\0" \
>  	"ip_dyn=yes\0" \
>  	"usb_pgood_delay=2000\0" \
> -	"mmcdevs=0 1\0" \
> -	"mmcpart=1\0" \
> -	"mmcroot=/dev/mmcblk0p2 rootwait rw\0" \
> -	"mmcargs=setenv bootargs console=${console},${baudrate} " \
> -		"root=${mmcroot}\0" \
> -	"loadbootscript=" \
> -		"load mmc ${mmcdev}:${mmcpart} ${loadaddr} ${script};\0" \
> -	"bootscript=echo Running bootscript from mmc ...; " \
> -		"source\0" \
> -	"loaduimage=load mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \
> -	"loadfdt=load mmc ${mmcdev}:${mmcpart} ${fdt_addr} ${fdt_file}\0" \
> -	"mmcboot=echo Booting from mmc ...; " \
> -		"run mmcargs; " \
> -		"if test ${boot_fdt} = yes || test ${boot_fdt} = try; then " \
> -			"if run loadfdt; then " \
> -				"bootm ${loadaddr} - ${fdt_addr}; " \
> -			"else " \
> -				"if test ${boot_fdt} = try; then " \
> -					"bootm; " \
> -				"else " \
> -					"echo WARN: Cannot load the DT; " \
> -				"fi; " \
> -			"fi; " \
> -		"else " \
> -			"bootm; " \
> -		"fi;\0" \
> -	"netargs=setenv bootargs console=${console},${baudrate} " \
> -		"root=/dev/nfs " \
> -	"ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \
> -		"netboot=echo Booting from net ...; " \
> -		"run netargs; " \
> -		"if test ${ip_dyn} = yes; then " \
> -			"setenv get_cmd dhcp; " \
> -		"else " \
> -			"setenv get_cmd tftp; " \
> -		"fi; " \
> -		"${get_cmd} ${uimage}; " \
> -		"if test ${boot_fdt} = yes || test ${boot_fdt} = try; then " \
> -			"if ${get_cmd} ${fdt_addr} ${fdt_file}; then " \
> -				"bootm ${loadaddr} - ${fdt_addr}; " \
> -			"else " \
> -				"if test ${boot_fdt} = try; then " \
> -					"bootm; " \
> -				"else " \
> -					"echo WARN: Cannot load the DT; " \
> -				"fi; " \
> -			"fi; " \
> -		"else " \
> -			"bootm; " \
> -		"fi;\0"
> +	BOOTENV
>  
> -#define CONFIG_BOOTCOMMAND \
> -	"for mmcdev in ${mmcdevs}; do " \
> -		"mmc dev ${mmcdev}; " \
> -		"if mmc rescan; then " \
> -			"if run loadbootscript; then " \
> -				"run bootscript; " \
> -			"else " \
> -				"if run loaduimage; then " \
> -					"run mmcboot; " \
> -				"fi; " \
> -			"fi; " \
> -		"fi; " \
> -	"done; " \
> -	"run netboot; "
>  #else
>  #define CONFIG_EXTRA_ENV_SETTINGS \
>  	"bootdevs=" CONFIG_DRIVE_TYPES "\0" \
> -- 
> 2.13.6

The rest looks good to me.

Regards,
Gary

[1] https://github.com/boundarydevices/u-boot-imx6/blob/boundary-v2017.07/include/configs/boundary.h#L209
[2] http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/mach-imx/mx7/soc.c;hb=HEAD#l188