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

[Michal Simek]

On 26.4.2018 08:14, Alexander Graf wrote:
> 
> 
> 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.

Imagine case that you have mmc1000 = &sdhci0;
That 1000 is integer and u-boot is in hex that's why 1000 = 0x3e8.

It means boot_targets will start with "mmc3e8 +static one"

That's why
bootcmd_mmc3e8=setenv devnum 3e8; run mmc_boot

needs to be generated. But xilinx_zynqmp.h is only setting these lines
for mmc0 and mmc1.

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

tbh this needs to be tested by sw rewriting boot modes before this is
called.


> 
>> +			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 ;).

Recursive function can be used too. But this is kind of nice even I
agree that it takes more time that for example this


static int intlen(u32 i)
{
	if (i < 10)
		return 1;

	return 1 + intlen(i / 10);
}

Thanks,
Michal