[U-Boot] [PATCH 1/1] colibri_imx6: use UUID for rootfs

[Marcel Ziswiler]

Hi Igor

Thanks for cleaning that up.

On Fri, 2019-04-05 at 15:51 +0200, Igor Opaniuk wrote:
> Replace usage of "/dev/mmcblk*p*" with a proper UUID of rootfs
> partition.
> This fixes the issue, when MMC controllers are probed in a different
> order
> in U-boot and Linux kernel.

Basically a dream come true (;-p). Looks fine and works with latest
2.8b6 as well as today's linux-next. Just a few minor comments below.

> Signed-off-by: Igor Opaniuk <igor.opaniuk at toradex.com>

With that addressed you may add my

Tested-by: Marcel Ziswiler <marcel.ziswiler at toradex.com>

And tackle all the other boards as well.

> ---
>  configs/colibri_imx6_defconfig |  1 +
>  include/configs/colibri_imx6.h | 55 +++++++++++++++++++++++---------
> --
>  2 files changed, 38 insertions(+), 18 deletions(-)
> 
> diff --git a/configs/colibri_imx6_defconfig
> b/configs/colibri_imx6_defconfig
> index bf05c68807..083588e02d 100644
> --- a/configs/colibri_imx6_defconfig
> +++ b/configs/colibri_imx6_defconfig
> @@ -40,6 +40,7 @@ CONFIG_CMD_GPIO=y
>  CONFIG_CMD_GPT=y
>  CONFIG_CMD_I2C=y
>  CONFIG_CMD_MMC=y
> +CONFIG_CMD_UUID=y
>  CONFIG_CMD_USB=y
>  CONFIG_CMD_USB_SDP=y
>  CONFIG_CMD_USB_MASS_STORAGE=y
> diff --git a/include/configs/colibri_imx6.h
> b/include/configs/colibri_imx6.h
> index 803c9be064..6603698a66 100644
> --- a/include/configs/colibri_imx6.h
> +++ b/include/configs/colibri_imx6.h
> @@ -115,16 +115,22 @@
>  	"imx6dl-colibri-cam-eval-v3.dtb fat 0 1"
>  
>  #define EMMC_BOOTCMD \
> -	"emmcargs=ip=off root=/dev/mmcblk0p2 rw,noatime rootfstype=ext4
> " \
> +	"emmcdev=0\0" \
> +	"emmcbootpart=1\0" \
> +	"emmcrootpart=2\0" \
> +	"emmcfinduuid=part uuid mmc ${mmcdev}:${emmcrootpart} uuid\0" \
> +	"set_emmcargs=setenv emmcargs ip=off root=PARTUUID=${uuid} "\
> +		"rw,noatime rootfstype=ext4 " \
>  		"rootwait\0" \
> -	"emmcboot=run setup; " \
> +	"emmcboot=run setup; run emmcfinduuid; run set_emmcargs; " \
>  		"setenv bootargs ${defargs} ${emmcargs} ${setupargs} "
> \
>  		"${vidargs}; echo Booting from internal eMMC chip...; "
> 	\
> -		"run emmcdtbload; load mmc 0:1 ${kernel_addr_r} " \
> -		"${boot_file} && run fdt_fixup && " \
> +		"run emmcdtbload; load mmc ${emmcdev}:${emmcbootpart} "
> \
> +		"${kernel_addr_r} ${boot_file} && run fdt_fixup && " \
>  		"bootz ${kernel_addr_r} ${dtbparam}\0" \
> -	"emmcdtbload=setenv dtbparam; load mmc 0:1 ${fdt_addr_r} " \
> -		"${fdt_file} && setenv dtbparam \" - ${fdt_addr_r}\" &&
> true\0"
> +	"emmcdtbload=setenv dtbparam; load mmc
> ${emmcdev}:${emmcbootpart} " \
> +		"${fdt_addr_r} ${fdt_file} && " \
> +		"setenv dtbparam \" - ${fdt_addr_r}\" && true\0"
>  
>  #define MEM_LAYOUT_ENV_SETTINGS \
>  	"bootm_size=0x10000000\0" \

Looks great, except I would sort that stuff alphabetically again
(applies to sdboot and usbboot as well). Just makes comparing things so
much easier.

> @@ -147,26 +153,38 @@
>  		"&& setenv dtbparam \" - ${fdt_addr_r}\" && true\0"
>  
>  #define SD_BOOTCMD \
> -	"sdargs=ip=off root=/dev/mmcblk1p2 rw,noatime rootfstype=ext4 "
> \
> -		"rootwait\0" \
> -	"sdboot=run setup; " \
> +	"sddev=1\0" \
> +	"sdbootpart=1\0" \
> +	"sdrootpart=2\0" \
> +	"sdfinduuid=part uuid mmc ${sddev}:${sdrootpart} uuid\0" \
> +	"set_sdargs=setenv sdargs ip=off root=PARTUUID=${uuid}
> rw,noatime " \
> +		"rootfstype=ext4 rootwait\0" \
> +	"sdboot=run setup; run sdfinduuid; run set_sdargs; " \
>  		"setenv bootargs ${defargs} ${sdargs} ${setupargs} " \
>  		"${vidargs}; echo Booting from SD card; " \
> -		"run sddtbload; load mmc 1:1 ${kernel_addr_r} " \
> -		"${boot_file} && run fdt_fixup && " \
> +		"run sddtbload; load mmc ${sddev}:${sdbootpart} "\
> +		"${kernel_addr_r} ${boot_file} && run fdt_fixup && " \
>  		"bootz ${kernel_addr_r} ${dtbparam}\0" \
> -	"sddtbload=setenv dtbparam; load mmc 1:1 ${fdt_addr_r} " \
> -		"${fdt_file} && setenv dtbparam \" - ${fdt_addr_r}\" &&
> true\0"
> +	"sddtbload=setenv dtbparam; load mmc ${sddev}:${sdbootpart} " \
> +		"${fdt_addr_r} ${fdt_file} && setenv dtbparam \" - " \
> +		"${fdt_addr_r}\" && true\0"
>  
>  #define USB_BOOTCMD \
> -	"usbargs=ip=off root=/dev/sda2 rw,noatime rootfstype=ext4 " \
> -		"rootwait\0" \
> -	"usbboot=run setup; setenv bootargs ${defargs} ${setupargs} " \
> +	"usbdev=0\0" \
> +	"usbbootpart=1\0" \
> +	"usbrootpart=2\0" \
> +	"usbfinduuid=part uuid usb ${usbdev}:${usbrootpart} uuid\0" \
> +	"set_usbargs=setenv usbargs ip=off root=PARTUUID=${uuid}
> rw,noatime " \
> +		"rootfstype=ext4 rootwait\0" \
> +	"usbboot=run setup; usb start; run usbfinduuid; run
> set_usbargs; " \
> +		"setenv bootargs ${defargs} ${setupargs} " \
>  		"${usbargs} ${vidargs}; echo Booting from USB stick...;
> " \
> -		"usb start && run usbdtbload; load usb 0:1
> ${kernel_addr_r} " \
> +		"run usbdtbload; " \
> +		"load usb ${usbdev}:${usbbootpart} ${kernel_addr_r} " \
>  		"${boot_file} && run fdt_fixup && " \
>  		"bootz ${kernel_addr_r} ${dtbparam}\0" \
> -	"usbdtbload=setenv dtbparam; load usb 0:1 ${fdt_addr_r} " \
> +	"usbdtbload=setenv dtbparam; load usb ${usbdev}:${usbbootpart}
> " \
> +		"${fdt_addr_r} " \
>  		"${fdt_file} && setenv dtbparam \" - ${fdt_addr_r}\" &&
> true\0"
>  
>  #define FDT_FILE "imx6dl-colibri-eval-v3.dtb"
> @@ -186,6 +204,7 @@
>  	MEM_LAYOUT_ENV_SETTINGS \
>  	NFS_BOOTCMD \
>  	SD_BOOTCMD \
> +	USB_BOOTCMD \

Yeah, that is a good one (;-p). Maybe you could add a note to the
commit message that this indeed also fixes the legacy usbboot command.
Actually, the commit message probably should mention sdboot and usbboot
as well anyway.

>  	"setethupdate=if env exists ethaddr; then; else setenv ethaddr
> " \
>  		"00:14:2d:00:00:00; fi; tftpboot ${loadaddr} " \
>  		"flash_eth.img && source ${loadaddr}\0" \

Cheers

Marcel