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

[Guillaume Gardet]

Le 09/04/2018 à 16:40, Gary Bisson a écrit :
> 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 at free.fr>
>> Cc: Troy Kisky <troy.kisky at boundarydevices.com>
>> Cc: Stefano Babic <sbabic at denx.de>
>> Cc: Fabio Estevam <fabio.estevam at 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.

Should we keep all the 6x scripts stuff or not?

>
>> +#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].

No, we just need to update the defconfig, see V2 of this patch sent earlier today.

>
>> +	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?

It was for backward compatibility, if people had some script using it.

>
>> -	"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.

Ok. Thanks.

Guillaume

>
> 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
>