[PATCH 0/3] Do not setup boot_targets if driver is not enabled

Fixing the boot_targets for versal-net, versal and zynqmp platforms.
Venkatesh Yadav Abbarapu (3): xilinx: versal-net: Do not setup boot_targets if driver is not enabled xilinx: versal: Do not setup boot_targets if driver is not enabled xilinx: zynqmp: Do not setup boot_targets if driver is not enabled
board/xilinx/versal-net/board.c | 76 ++++++++++++++++----------------- board/xilinx/versal/board.c | 67 +++++++++++++++-------------- board/xilinx/zynqmp/zynqmp.c | 71 +++++++++++++++--------------- 3 files changed, 108 insertions(+), 106 deletions(-)

SOC can boot in the device which is not accessible from APU and running this is detected as error which ends up in stopping boot process. Boot mode detection and logic around is present to setup priority on boot devices that SOC boot device is likely also used for booting OS. Change logic to detect this case with showing message about it but don't fail in boot process and don't prioritize boot device in this case.
Signed-off-by: Michal Simek michal.simek@amd.com Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com --- board/xilinx/versal-net/board.c | 76 ++++++++++++++++----------------- 1 file changed, 38 insertions(+), 38 deletions(-)
diff --git a/board/xilinx/versal-net/board.c b/board/xilinx/versal-net/board.c index f0d2224b33..7ad299f3a2 100644 --- a/board/xilinx/versal-net/board.c +++ b/board/xilinx/versal-net/board.c @@ -201,7 +201,7 @@ int board_late_init(void) int bootseq = -1; int bootseq_len = 0; int env_targets_len = 0; - const char *mode; + const char *mode = NULL; char *new_targets; char *env_targets;
@@ -229,8 +229,8 @@ int board_late_init(void) puts("QSPI_MODE_24\n"); if (uclass_get_device_by_name(UCLASS_SPI, "spi@f1030000", &dev)) { - puts("Boot from QSPI but without QSPI enabled!\n"); - return -1; + debug("QSPI driver for QSPI device is not present\n"); + break; } mode = "xspi"; bootseq = dev_seq(dev); @@ -239,8 +239,8 @@ int board_late_init(void) puts("QSPI_MODE_32\n"); if (uclass_get_device_by_name(UCLASS_SPI, "spi@f1030000", &dev)) { - puts("Boot from QSPI but without QSPI enabled!\n"); - return -1; + debug("QSPI driver for QSPI device is not present\n"); + break; } mode = "xspi"; bootseq = dev_seq(dev); @@ -249,8 +249,8 @@ int board_late_init(void) puts("OSPI_MODE\n"); if (uclass_get_device_by_name(UCLASS_SPI, "spi@f1010000", &dev)) { - puts("Boot from OSPI but without OSPI enabled!\n"); - return -1; + debug("OSPI driver for OSPI device is not present\n"); + break; } mode = "xspi"; bootseq = dev_seq(dev); @@ -264,8 +264,8 @@ int board_late_init(void) puts("SD_MODE\n"); if (uclass_get_device_by_name(UCLASS_MMC, "mmc@f1040000", &dev)) { - puts("Boot from SD0 but without SD0 enabled!\n"); - return -1; + debug("SD0 driver for SD0 device is not present\n"); + break; } debug("mmc0 device found at %p, seq %d\n", dev, dev_seq(dev));
@@ -279,8 +279,8 @@ int board_late_init(void) puts("SD_MODE1\n"); if (uclass_get_device_by_name(UCLASS_MMC, "mmc@f1050000", &dev)) { - puts("Boot from SD1 but without SD1 enabled!\n"); - return -1; + debug("SD1 driver for SD1 device is not present\n"); + break; } debug("mmc1 device found at %p, seq %d\n", dev, dev_seq(dev));
@@ -288,38 +288,38 @@ int board_late_init(void) bootseq = dev_seq(dev); break; default: - mode = ""; printf("Invalid Boot Mode:0x%x\n", bootmode); 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 - */ - env_targets = env_get("boot_targets"); - if (env_targets) - env_targets_len = strlen(env_targets); - - new_targets = calloc(1, strlen(mode) + env_targets_len + 2 + - bootseq_len); - if (!new_targets) - return -ENOMEM; - - 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); + if (mode) { + 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 + */ + env_targets = env_get("boot_targets"); + if (env_targets) + env_targets_len = strlen(env_targets); + + new_targets = calloc(1, strlen(mode) + env_targets_len + 2 + + bootseq_len); + if (!new_targets) + return -ENOMEM; + + 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); + } return board_late_init_xilinx(); }

Hi Venkatesh,
On Mon, 4 Sept 2023 at 07:58, Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com wrote:
SOC can boot in the device which is not accessible from APU and running this is detected as error which ends up in stopping boot process. Boot mode detection and logic around is present to setup priority on boot devices that SOC boot device is likely also used for booting OS. Change logic to detect this case with showing message about it but don't fail in boot process and don't prioritize boot device in this case.
Signed-off-by: Michal Simek michal.simek@amd.com Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com
board/xilinx/versal-net/board.c | 76 ++++++++++++++++----------------- 1 file changed, 38 insertions(+), 38 deletions(-)
If you switch to standard boot it won't try to boot on devices which don't exist.
Regards, Simon

On 9/4/23 16:43, Simon Glass wrote:
Hi Venkatesh,
On Mon, 4 Sept 2023 at 07:58, Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com wrote:
SOC can boot in the device which is not accessible from APU and running this is detected as error which ends up in stopping boot process. Boot mode detection and logic around is present to setup priority on boot devices that SOC boot device is likely also used for booting OS. Change logic to detect this case with showing message about it but don't fail in boot process and don't prioritize boot device in this case.
Signed-off-by: Michal Simek michal.simek@amd.com Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com
board/xilinx/versal-net/board.c | 76 ++++++++++++++++----------------- 1 file changed, 38 insertions(+), 38 deletions(-)
If you switch to standard boot it won't try to boot on devices which don't exist.
Actually I was looking at standard boot yesterday. We have enabled standard boot already but not BOOTSTD_FULL because there is a dependency on having video enabled. Actually this dependency should be covered in Kconfig too.
I don't think this is an issue with this series which is trying to address issue properly. But let me send a patch to guard this code only when CONFIG_DISTRO_DEFAULTS is enabled. And also that huge amount of variables don't need to be created too in this case.
Thanks, Michal

SOC can boot in the device which is not accessible from APU and running this is detected as error which ends up in stopping boot process. Boot mode detection and logic around is present to setup priority on boot devices that SOC boot device is likely also used for booting OS. Change logic to detect this case with showing message about it but don't fail in boot process and don't prioritize boot device in this case.
Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com --- board/xilinx/versal/board.c | 67 +++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 33 deletions(-)
diff --git a/board/xilinx/versal/board.c b/board/xilinx/versal/board.c index 60bf37d3c9..982a8b3779 100644 --- a/board/xilinx/versal/board.c +++ b/board/xilinx/versal/board.c @@ -133,7 +133,7 @@ int board_late_init(void) int bootseq = -1; int bootseq_len = 0; int env_targets_len = 0; - const char *mode; + const char *mode = NULL; char *new_targets; char *env_targets;
@@ -175,8 +175,8 @@ int board_late_init(void) "mmc@f1050000", &dev) && uclass_get_device_by_name(UCLASS_MMC, "sdhci@f1050000", &dev)) { - puts("Boot from EMMC but without SD1 enabled!\n"); - return -1; + debug("SD1 driver for SD1 device is not present\n"); + break; } debug("mmc1 device found at %p, seq %d\n", dev, dev_seq(dev)); mode = "mmc"; @@ -188,8 +188,8 @@ int board_late_init(void) "mmc@f1040000", &dev) && uclass_get_device_by_name(UCLASS_MMC, "sdhci@f1040000", &dev)) { - puts("Boot from SD0 but without SD0 enabled!\n"); - return -1; + debug("SD0 driver for SD0 device is not present\n"); + break; } debug("mmc0 device found at %p, seq %d\n", dev, dev_seq(dev));
@@ -205,8 +205,8 @@ int board_late_init(void) "mmc@f1050000", &dev) && uclass_get_device_by_name(UCLASS_MMC, "sdhci@f1050000", &dev)) { - puts("Boot from SD1 but without SD1 enabled!\n"); - return -1; + debug("SD1 driver for SD1 device is not present\n"); + break; } debug("mmc1 device found at %p, seq %d\n", dev, dev_seq(dev));
@@ -214,37 +214,38 @@ int board_late_init(void) bootseq = dev_seq(dev); break; default: - mode = ""; printf("Invalid Boot Mode:0x%x\n", bootmode); break; }
- if (bootseq >= 0) { - bootseq_len = snprintf(NULL, 0, "%i", bootseq); - debug("Bootseq len: %x\n", bootseq_len); - } + if (mode) { + 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 - */ - env_targets = env_get("boot_targets"); - if (env_targets) - env_targets_len = strlen(env_targets); - - new_targets = calloc(1, strlen(mode) + env_targets_len + 2 + - bootseq_len); - if (!new_targets) - return -ENOMEM; - - 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); + /* + * One terminating char + one byte for space between mode + * and default boot_targets + */ + env_targets = env_get("boot_targets"); + if (env_targets) + env_targets_len = strlen(env_targets); + + new_targets = calloc(1, strlen(mode) + env_targets_len + 2 + + bootseq_len); + if (!new_targets) + return -ENOMEM; + + 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); + }
return board_late_init_xilinx(); }

SOC can boot in the device which is not accessible from APU and running this is detected as error which ends up in stopping boot process. Boot mode detection and logic around is present to setup priority on boot devices that SOC boot device is likely also used for booting OS. Change logic to detect this case with showing message about it but don't fail in boot process and don't prioritize boot device in this case.
Signed-off-by: Venkatesh Yadav Abbarapu venkatesh.abbarapu@amd.com --- board/xilinx/zynqmp/zynqmp.c | 71 ++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 35 deletions(-)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 309f24a5f4..0b6d4e5707 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -391,7 +391,7 @@ int board_late_init(void) int bootseq = -1; int bootseq_len = 0; int env_targets_len = 0; - const char *mode; + const char *mode = NULL; char *new_targets; char *env_targets; int ret, multiboot; @@ -442,8 +442,8 @@ int board_late_init(void) "mmc@ff160000", &dev) && uclass_get_device_by_name(UCLASS_MMC, "sdhci@ff160000", &dev)) { - puts("Boot from EMMC but without SD0 enabled!\n"); - return -1; + debug("SD0 driver for SD0 device is not present\n"); + break; } debug("mmc0 device found at %p, seq %d\n", dev, dev_seq(dev));
@@ -457,8 +457,8 @@ int board_late_init(void) "mmc@ff160000", &dev) && uclass_get_device_by_name(UCLASS_MMC, "sdhci@ff160000", &dev)) { - puts("Boot from SD0 but without SD0 enabled!\n"); - return -1; + debug("SD0 driver for SD0 device is not present\n"); + break; } debug("mmc0 device found at %p, seq %d\n", dev, dev_seq(dev));
@@ -475,8 +475,8 @@ int board_late_init(void) "mmc@ff170000", &dev) && uclass_get_device_by_name(UCLASS_MMC, "sdhci@ff170000", &dev)) { - puts("Boot from SD1 but without SD1 enabled!\n"); - return -1; + debug("SD1 driver for SD1 device is not present\n"); + break; } debug("mmc1 device found at %p, seq %d\n", dev, dev_seq(dev));
@@ -490,39 +490,40 @@ int board_late_init(void) env_set("modeboot", "nandboot"); break; default: - mode = ""; printf("Invalid Boot Mode:0x%x\n", bootmode); break; }
- if (bootseq >= 0) { - bootseq_len = snprintf(NULL, 0, "%i", bootseq); - debug("Bootseq len: %x\n", bootseq_len); - env_set_hex("bootseq", bootseq); - } + if (mode) { + if (bootseq >= 0) { + bootseq_len = snprintf(NULL, 0, "%i", bootseq); + debug("Bootseq len: %x\n", bootseq_len); + env_set_hex("bootseq", bootseq); + }
- /* - * One terminating char + one byte for space between mode - * and default boot_targets - */ - env_targets = env_get("boot_targets"); - if (env_targets) - env_targets_len = strlen(env_targets); - - new_targets = calloc(1, strlen(mode) + env_targets_len + 2 + - bootseq_len); - if (!new_targets) - return -ENOMEM; - - 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); - free(new_targets); + /* + * One terminating char + one byte for space between mode + * and default boot_targets + */ + env_targets = env_get("boot_targets"); + if (env_targets) + env_targets_len = strlen(env_targets); + + new_targets = calloc(1, strlen(mode) + env_targets_len + 2 + + bootseq_len); + if (!new_targets) + return -ENOMEM; + + 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); + free(new_targets); + }
reset_reason();

On 9/4/23 05:20, Venkatesh Yadav Abbarapu wrote:
Fixing the boot_targets for versal-net, versal and zynqmp platforms.
Venkatesh Yadav Abbarapu (3): xilinx: versal-net: Do not setup boot_targets if driver is not enabled xilinx: versal: Do not setup boot_targets if driver is not enabled xilinx: zynqmp: Do not setup boot_targets if driver is not enabled
board/xilinx/versal-net/board.c | 76 ++++++++++++++++----------------- board/xilinx/versal/board.c | 67 +++++++++++++++-------------- board/xilinx/zynqmp/zynqmp.c | 71 +++++++++++++++--------------- 3 files changed, 108 insertions(+), 106 deletions(-)
Applied. M
participants (3)
-
Michal Simek
-
Simon Glass
-
Venkatesh Yadav Abbarapu