* [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time
@ 2018-04-25 12:38 Michal Simek
2018-04-25 12:38 ` [U-Boot] [RFC PATCH 2/2] arm64: zynqmp: Try to create bootcm_mmcX " Michal Simek
2018-04-26 6:14 ` [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target " Alexander Graf
0 siblings, 2 replies; 9+ messages in thread
From: Michal Simek @ 2018-04-25 12:38 UTC (permalink / raw)
To: u-boot
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@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.
---
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)) {
+ 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);
+ 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);
--
2.17.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [RFC PATCH 2/2] arm64: zynqmp: Try to create bootcm_mmcX at run time
2018-04-25 12:38 [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time Michal Simek
@ 2018-04-25 12:38 ` Michal Simek
2018-04-26 6:23 ` Alexander Graf
2018-04-26 6:14 ` [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target " Alexander Graf
1 sibling, 1 reply; 9+ messages in thread
From: Michal Simek @ 2018-04-25 12:38 UTC (permalink / raw)
To: u-boot
Just an attept to create boot commands for mmcs above 2 which is not
generated.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Probably some ifdefs around are missing.
---
board/xilinx/zynqmp/zynqmp.c | 28 ++++++++++++++++++++++++++++
include/configs/xilinx_zynqmp.h | 7 -------
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 96ea0f578d30..37fa8f4f4d3f 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -451,6 +451,32 @@ void reset_cpu(ulong addr)
{
}
+static int create_mmc_boot_commands(void)
+{
+/* Size of strings below + one terminating char \0 + 3 possitions for seq */
+#define MAX_COMMAND_LEN 15
+#define MAX_BODY_LEN 32
+
+ int ret;
+ struct uclass *uc;
+ struct udevice *dev;
+ char body[MAX_BODY_LEN];
+ char command[MAX_COMMAND_LEN];
+
+ ret = uclass_get(UCLASS_MMC, &uc);
+ if (ret)
+ return ret;
+
+ uclass_foreach_dev(dev, uc) {
+ snprintf(body, MAX_BODY_LEN, "setenv devnum %x; run mmc_boot",
+ dev->seq);
+ snprintf(command, MAX_COMMAND_LEN, "bootcmd_mmc%x", dev->seq);
+ env_set(command, body);
+ }
+
+ return 0;
+}
+
int board_late_init(void)
{
u32 reg = 0;
@@ -546,6 +572,8 @@ int board_late_init(void)
debug("Bootseq len: %x\n", bootseq_len);
}
+ create_mmc_boot_commands();
+
/*
* One terminating char + one byte for space between mode
* and default boot_targets
diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h
index c8a0dbb7e3b5..a2590998429e 100644
--- a/include/configs/xilinx_zynqmp.h
+++ b/include/configs/xilinx_zynqmp.h
@@ -155,12 +155,6 @@
"scriptaddr=0x02000000\0" \
"ramdisk_addr_r=0x02100000\0" \
-#if defined(CONFIG_MMC_SDHCI_ZYNQ)
-# define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0) func(MMC, mmc, 1)
-#else
-# define BOOT_TARGET_DEVICES_MMC(func)
-#endif
-
#if defined(CONFIG_SATA_CEVA)
# define BOOT_TARGET_DEVICES_SCSI(func) func(SCSI, scsi, 0)
#else
@@ -186,7 +180,6 @@
#endif
#define BOOT_TARGET_DEVICES(func) \
- BOOT_TARGET_DEVICES_MMC(func) \
BOOT_TARGET_DEVICES_USB(func) \
BOOT_TARGET_DEVICES_SCSI(func) \
BOOT_TARGET_DEVICES_PXE(func) \
--
2.17.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time
2018-04-25 12:38 [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time Michal Simek
2018-04-25 12:38 ` [U-Boot] [RFC PATCH 2/2] arm64: zynqmp: Try to create bootcm_mmcX " Michal Simek
@ 2018-04-26 6:14 ` Alexander Graf
2018-04-26 6:25 ` Michal Simek
1 sibling, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2018-04-26 6:14 UTC (permalink / raw)
To: u-boot
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@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);
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC PATCH 2/2] arm64: zynqmp: Try to create bootcm_mmcX at run time
2018-04-25 12:38 ` [U-Boot] [RFC PATCH 2/2] arm64: zynqmp: Try to create bootcm_mmcX " Michal Simek
@ 2018-04-26 6:23 ` Alexander Graf
2018-04-26 6:27 ` Michal Simek
0 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2018-04-26 6:23 UTC (permalink / raw)
To: u-boot
On 25.04.18 14:38, Michal Simek wrote:
> Just an attept to create boot commands for mmcs above 2 which is not
> generated.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> Probably some ifdefs around are missing.
> ---
> board/xilinx/zynqmp/zynqmp.c | 28 ++++++++++++++++++++++++++++
> include/configs/xilinx_zynqmp.h | 7 -------
> 2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 96ea0f578d30..37fa8f4f4d3f 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -451,6 +451,32 @@ void reset_cpu(ulong addr)
> {
> }
>
> +static int create_mmc_boot_commands(void)
> +{
> +/* Size of strings below + one terminating char \0 + 3 possitions for seq */
> +#define MAX_COMMAND_LEN 15
> +#define MAX_BODY_LEN 32
> +
> + int ret;
> + struct uclass *uc;
> + struct udevice *dev;
> + char body[MAX_BODY_LEN];
> + char command[MAX_COMMAND_LEN];
> +
> + ret = uclass_get(UCLASS_MMC, &uc);
> + if (ret)
> + return ret;
> +
> + uclass_foreach_dev(dev, uc) {
> + snprintf(body, MAX_BODY_LEN, "setenv devnum %x; run mmc_boot",
> + dev->seq);
> + snprintf(command, MAX_COMMAND_LEN, "bootcmd_mmc%x", dev->seq);
> + env_set(command, body);
> + }
Why not just remove the ones you don't need? You could leave the
template in and just remove bootcmd_mmc0 if you don't see an mmc0 device.
Alex
> +
> + return 0;
> +}
> +
> int board_late_init(void)
> {
> u32 reg = 0;
> @@ -546,6 +572,8 @@ int board_late_init(void)
> debug("Bootseq len: %x\n", bootseq_len);
> }
>
> + create_mmc_boot_commands();
> +
> /*
> * One terminating char + one byte for space between mode
> * and default boot_targets
> diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h
> index c8a0dbb7e3b5..a2590998429e 100644
> --- a/include/configs/xilinx_zynqmp.h
> +++ b/include/configs/xilinx_zynqmp.h
> @@ -155,12 +155,6 @@
> "scriptaddr=0x02000000\0" \
> "ramdisk_addr_r=0x02100000\0" \
>
> -#if defined(CONFIG_MMC_SDHCI_ZYNQ)
> -# define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0) func(MMC, mmc, 1)
> -#else
> -# define BOOT_TARGET_DEVICES_MMC(func)
> -#endif
> -
> #if defined(CONFIG_SATA_CEVA)
> # define BOOT_TARGET_DEVICES_SCSI(func) func(SCSI, scsi, 0)
> #else
> @@ -186,7 +180,6 @@
> #endif
>
> #define BOOT_TARGET_DEVICES(func) \
> - BOOT_TARGET_DEVICES_MMC(func) \
> BOOT_TARGET_DEVICES_USB(func) \
> BOOT_TARGET_DEVICES_SCSI(func) \
> BOOT_TARGET_DEVICES_PXE(func) \
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time
2018-04-26 6:14 ` [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target " Alexander Graf
@ 2018-04-26 6:25 ` Michal Simek
2018-04-26 7:08 ` Alexander Graf
0 siblings, 1 reply; 9+ messages in thread
From: Michal Simek @ 2018-04-26 6:25 UTC (permalink / raw)
To: u-boot
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@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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC PATCH 2/2] arm64: zynqmp: Try to create bootcm_mmcX at run time
2018-04-26 6:23 ` Alexander Graf
@ 2018-04-26 6:27 ` Michal Simek
0 siblings, 0 replies; 9+ messages in thread
From: Michal Simek @ 2018-04-26 6:27 UTC (permalink / raw)
To: u-boot
On 26.4.2018 08:23, Alexander Graf wrote:
>
>
> On 25.04.18 14:38, Michal Simek wrote:
>> Just an attept to create boot commands for mmcs above 2 which is not
>> generated.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> Probably some ifdefs around are missing.
>> ---
>> board/xilinx/zynqmp/zynqmp.c | 28 ++++++++++++++++++++++++++++
>> include/configs/xilinx_zynqmp.h | 7 -------
>> 2 files changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>> index 96ea0f578d30..37fa8f4f4d3f 100644
>> --- a/board/xilinx/zynqmp/zynqmp.c
>> +++ b/board/xilinx/zynqmp/zynqmp.c
>> @@ -451,6 +451,32 @@ void reset_cpu(ulong addr)
>> {
>> }
>>
>> +static int create_mmc_boot_commands(void)
>> +{
>> +/* Size of strings below + one terminating char \0 + 3 possitions for seq */
>> +#define MAX_COMMAND_LEN 15
>> +#define MAX_BODY_LEN 32
>> +
>> + int ret;
>> + struct uclass *uc;
>> + struct udevice *dev;
>> + char body[MAX_BODY_LEN];
>> + char command[MAX_COMMAND_LEN];
>> +
>> + ret = uclass_get(UCLASS_MMC, &uc);
>> + if (ret)
>> + return ret;
>> +
>> + uclass_foreach_dev(dev, uc) {
>> + snprintf(body, MAX_BODY_LEN, "setenv devnum %x; run mmc_boot",
>> + dev->seq);
>> + snprintf(command, MAX_COMMAND_LEN, "bootcmd_mmc%x", dev->seq);
>> + env_set(command, body);
>> + }
>
> Why not just remove the ones you don't need? You could leave the
> template in and just remove bootcmd_mmc0 if you don't see an mmc0 device.
It is not a problem if that alias is mmc0 or mmc1 but if that alias is
mmc3 because none is generating that distro default boot command line.
Look at my reply on 1/2
M
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time
2018-04-26 6:25 ` Michal Simek
@ 2018-04-26 7:08 ` Alexander Graf
2018-04-26 8:20 ` Michal Simek
0 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2018-04-26 7:08 UTC (permalink / raw)
To: u-boot
> Am 26.04.2018 um 08:25 schrieb Michal Simek <michal.simek@xilinx.com>:
>
>> 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@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.
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 the system on boot.
The reason we don‘t have that yet I guess is because most targets target one SoC/board that has a known set of devices. The more generic we make things, the more we have to enumerate at runtime.
Alex
>
>>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time
2018-04-26 7:08 ` Alexander Graf
@ 2018-04-26 8:20 ` Michal Simek
2018-04-26 8:46 ` Alexander Graf
0 siblings, 1 reply; 9+ messages in thread
From: Michal Simek @ 2018-04-26 8:20 UTC (permalink / raw)
To: u-boot
On 26.4.2018 09:08, Alexander Graf wrote:
>
>
>> Am 26.04.2018 um 08:25 schrieb Michal Simek <michal.simek@xilinx.com>:
>>
>>> 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@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.
>
> 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 the system on boot.
>
> The reason we don‘t have that yet I guess is because most targets target one SoC/board that has a known set of devices. The more generic we make things, the more we have to enumerate at runtime.
>
yes, and do this only in case of when variables are not restored. It
means only when default one are used.
And it is really a question if make sense to change current distro
default or just wait for EBBR.
For zynqmp device itself using mmc0/mmc1 should be enough.
Thanks,
Michal
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time
2018-04-26 8:20 ` Michal Simek
@ 2018-04-26 8:46 ` Alexander Graf
0 siblings, 0 replies; 9+ messages in thread
From: Alexander Graf @ 2018-04-26 8:46 UTC (permalink / raw)
To: u-boot
On 04/26/2018 10:20 AM, Michal Simek wrote:
> On 26.4.2018 09:08, Alexander Graf wrote:
>>
>>> Am 26.04.2018 um 08:25 schrieb Michal Simek <michal.simek@xilinx.com>:
>>>
>>>> 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@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.
>> 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 the system on boot.
>>
>> The reason we don‘t have that yet I guess is because most targets target one SoC/board that has a known set of devices. The more generic we make things, the more we have to enumerate at runtime.
>>
> yes, and do this only in case of when variables are not restored. It
> means only when default one are used.
Well, instead of the current "mmc0 mmc1" boot targets you would
introduce an "mmc" boot target that probably needs to call C code to do
the device walking. That way the environment would be unchanged
regardless of the target system.
> And it is really a question if make sense to change current distro
> default or just wait for EBBR.
I'm not sure EBBR is going to help here.
> For zynqmp device itself using mmc0/mmc1 should be enough.
Yes, I think as a first approximation it's fine to have mmc0/mmc1
entries in the static boot target list and just blank/remove the
bootcmd_mmc{0,1} ones that don't exist on boot.
Alex
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-04-26 8:46 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-25 12:38 [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target at run time Michal Simek
2018-04-25 12:38 ` [U-Boot] [RFC PATCH 2/2] arm64: zynqmp: Try to create bootcm_mmcX " Michal Simek
2018-04-26 6:23 ` Alexander Graf
2018-04-26 6:27 ` Michal Simek
2018-04-26 6:14 ` [U-Boot] [RFC PATCH 1/2] arm64: zynqmp: Setup the first boot_target " Alexander Graf
2018-04-26 6:25 ` Michal Simek
2018-04-26 7:08 ` Alexander Graf
2018-04-26 8:20 ` Michal Simek
2018-04-26 8:46 ` Alexander Graf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.