From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Thu, 26 Apr 2018 09:08:55 +0200 Subject: [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time In-Reply-To: <400a89bc-2a36-fcdb-d75a-604d237a8b43@xilinx.com> References: <8fad3265d924092e94aae75d4141a61872a61685.1524659886.git.michal.simek@xilinx.com> <400a89bc-2a36-fcdb-d75a-604d237a8b43@xilinx.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable To: u-boot@lists.denx.de > Am 26.04.2018 um 08:25 schrieb Michal Simek : >=20 >> On 26.4.2018 08:14, Alexander Graf wrote: >>=20 >>=20 >>> On 25.04.18 14:38, Michal Simek wrote: >>> Detect mmc alias at run time for setting up proper boot_targets sequenc= e. >>> The first target has to correspond with boot mode. >>>=20 >>> 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. >>>=20 >>> 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. >>>=20 >>> Signed-off-by: Michal Simek >>> --- >>>=20 >>> 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 >>>=20 >>> bootcmd_mmc0=3Dsetenv devnum 0; run mmc_boot >>> bootcmd_mmc1=3Dsetenv devnum 1; run mmc_boot >>>=20 >>> 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. >>=20 >> I don't understand this sentence. >=20 > Imagine case that you have mmc1000 =3D &sdhci0; > That 1000 is integer and u-boot is in hex that's why 1000 =3D 0x3e8. >=20 > It means boot_targets will start with "mmc3e8 +static one" >=20 > That's why > bootcmd_mmc3e8=3Dsetenv devnum 3e8; run mmc_boot >=20 > needs to be generated. But xilinx_zynqmp.h is only setting these lines > for mmc0 and mmc1. Ok, so what we really want is a distro.c file and corresponding boot_target= lines that generate boot target variables for every device enumerated in t= he system on boot. The reason we don=E2=80=98t have that yet I guess is because most targets t= arget one SoC/board that has a known set of devices. The more generic we ma= ke things, the more we have to enumerate at runtime. Alex >=20 >>=20 >>> --- >>> board/xilinx/zynqmp/zynqmp.c | 45 ++++++++++++++++++++++++++++-------- >>> 1 file changed, 35 insertions(+), 10 deletions(-) >>>=20 >>> 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 =3D 0; >>> u8 bootmode; >>> + struct udevice *dev; >>> + int bootseq =3D -1; >>> + int bootseq_len =3D 0; >>> int env_targets_len =3D 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 =3D "mmc0"; >>> + if (uclass_get_device_by_name(UCLASS_MMC, >>> + "sdhci at ff160000", &dev)) { >>=20 >> I think you need to check whether probing actually succeeded, no? >> Otherwise seq is invalid (-1). >>=20 >> So this should be if (uclass...() || (dev && dev->flags & >> DM_FLAG_ACTIVATED)) >=20 > tbh this needs to be tested by sw rewriting boot modes before this is > called. >=20 >=20 >>=20 >>> + puts("Boot from SD0 but without SD0 enabled!\n"); >>> + return -1; >>> + } >>> + debug("mmc0 device found at %p, seq %d\n", dev, dev->seq); >>> + >>> + mode =3D "mmc"; >>> + bootseq =3D 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 =3D "mmc1"; >>> - env_set("sdbootdev", "1"); >>> -#else >>> - mode =3D "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 =3D "mmc"; >>> + bootseq =3D dev->seq; >>> env_set("modeboot", "sdboot"); >>> break; >>> case NAND_MODE: >>> @@ -526,6 +541,11 @@ int board_late_init(void) >>> break; >>> } >>>=20 >>> + if (bootseq >=3D 0) { >>> + bootseq_len =3D snprintf(NULL, 0, "%i", bootseq); >>=20 >> This seems like quite a heavy way to figure out the number of digits of >> an integer ;). >=20 > Recursive function can be used too. But this is kind of nice even I > agree that it takes more time that for example this >=20 >=20 > static int intlen(u32 i) > { > if (i < 10) > return 1; >=20 > return 1 + intlen(i / 10); > } >=20 > Thanks, > Michal