From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Thu, 26 Apr 2018 08:25:42 +0200 Subject: [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time In-Reply-To: References: <8fad3265d924092e94aae75d4141a61872a61685.1524659886.git.michal.simek@xilinx.com> Message-ID: <400a89bc-2a36-fcdb-d75a-604d237a8b43@xilinx.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >> --- >> >> 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 >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -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