[U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time

[Alexander Graf]

On 25.04.18 14:38, Michal Simek wrote:
> Detect mmc alias at run time for setting up proper boot_targets sequence.
> The first target has to correspond with boot mode.
> 
> The purpose of this patch is to get rid of CONFIG_ZYNQ_SDHCI0/1
> parameters in full U-Boot.
> Unfortunately this patch can't remove it because there is missing
> mmc implementation for SPL_DM_SEQ_ALIAS.
> 
> Also xilinx_zynqmp.h only setup boot commands for mmc0 and mmc1.
> It means using aliases with higher number won't work. But switching
> between mmc0 and mmc1 should work properly.
> 
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
> 
> Not sure how exactly to tune BOOT_TARGET_DEVICES_MMC to have functions
> for different aliases ID. I can simply setup devnum based on dev->seq
> and also generate the whole bootcmd_mmc0
> 
> bootcmd_mmc0=setenv devnum 0; run mmc_boot
> bootcmd_mmc1=setenv devnum 1; run mmc_boot
> 
> The second patch is doing that.
> But still if you setup alias to higher number mmc core is not mmc dev
> command is not able to work with it.

I don't understand this sentence.

> ---
>  board/xilinx/zynqmp/zynqmp.c | 45 ++++++++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 10 deletions(-)
> 
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index f7fe1fdfff7b..96ea0f578d30 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -16,6 +16,7 @@
>  #include <asm/arch/sys_proto.h>
>  #include <asm/arch/psu_init_gpl.h>
>  #include <asm/io.h>
> +#include <dm/device.h>
>  #include <dm/uclass.h>
>  #include <usb.h>
>  #include <dwc3-uboot.h>
> @@ -454,6 +455,9 @@ int board_late_init(void)
>  {
>  	u32 reg = 0;
>  	u8 bootmode;
> +	struct udevice *dev;
> +	int bootseq = -1;
> +	int bootseq_len = 0;
>  	int env_targets_len = 0;
>  	const char *mode;
>  	char *new_targets;
> @@ -499,7 +503,15 @@ int board_late_init(void)
>  		break;
>  	case SD_MODE:
>  		puts("SD_MODE\n");
> -		mode = "mmc0";
> +		if (uclass_get_device_by_name(UCLASS_MMC,
> +					      "sdhci at ff160000", &dev)) {

I think you need to check whether probing actually succeeded, no?
Otherwise seq is invalid (-1).

So this should be if (uclass...() || (dev && dev->flags &
DM_FLAG_ACTIVATED))

> +			puts("Boot from SD0 but without SD0 enabled!\n");
> +			return -1;
> +		}
> +		debug("mmc0 device found at %p, seq %d\n", dev, dev->seq);
> +
> +		mode = "mmc";
> +		bootseq = dev->seq;
>  		env_set("modeboot", "sdboot");
>  		break;
>  	case SD1_LSHFT_MODE:
> @@ -507,12 +519,15 @@ int board_late_init(void)
>  		/* fall through */
>  	case SD_MODE1:
>  		puts("SD_MODE1\n");
> -#if defined(CONFIG_ZYNQ_SDHCI0) && defined(CONFIG_ZYNQ_SDHCI1)
> -		mode = "mmc1";
> -		env_set("sdbootdev", "1");
> -#else
> -		mode = "mmc0";
> -#endif
> +		if (uclass_get_device_by_name(UCLASS_MMC,
> +					      "sdhci at ff170000", &dev)) {
> +			puts("Boot from SD1 but without SD1 enabled!\n");
> +			return -1;
> +		}
> +		debug("mmc1 device found at %p, seq %d\n", dev, dev->seq);
> +
> +		mode = "mmc";
> +		bootseq = dev->seq;
>  		env_set("modeboot", "sdboot");
>  		break;
>  	case NAND_MODE:
> @@ -526,6 +541,11 @@ int board_late_init(void)
>  		break;
>  	}
>  
> +	if (bootseq >= 0) {
> +		bootseq_len = snprintf(NULL, 0, "%i", bootseq);

This seems like quite a heavy way to figure out the number of digits of
an integer ;).


Alex

> +		debug("Bootseq len: %x\n", bootseq_len);
> +	}
> +
>  	/*
>  	 * One terminating char + one byte for space between mode
>  	 * and default boot_targets
> @@ -534,10 +554,15 @@ int board_late_init(void)
>  	if (env_targets)
>  		env_targets_len = strlen(env_targets);
>  
> -	new_targets = calloc(1, strlen(mode) + env_targets_len + 2);
> +	new_targets = calloc(1, strlen(mode) + env_targets_len + 2 +
> +			     bootseq_len);
>  
> -	sprintf(new_targets, "%s %s", mode,
> -		env_targets ? env_targets : "");
> +	if (bootseq >= 0)
> +		sprintf(new_targets, "%s%x %s", mode, bootseq,
> +			env_targets ? env_targets : "");
> +	else
> +		sprintf(new_targets, "%s %s", mode,
> +			env_targets ? env_targets : "");
>  
>  	env_set("boot_targets", new_targets);
>  
>