On 21/06/2024 10:07, Sumit Garg wrote:
On Fri, 21 Jun 2024 at 13:19, Caleb Connolly <caleb.conno...@linaro.org> wrote:



On 21/06/2024 09:35, Sumit Garg wrote:
Hi Caleb,

On Mon, 17 Jun 2024 at 14:26, Caleb Connolly <caleb.conno...@linaro.org> wrote:

Make use of CONFIG_DEFAULT_ENV_FILE and move the default qcom
environment to a file under board/qualcomm.

This is much cleaner and means we don't need to recompile on changing
the environment.

Additionally, extend the environment to include a boot menu and
auto-boot using EFI instead of bootm. Since we now support MMC and USB
on most platforms, these are much more useful defaults.

Signed-off-by: Caleb Connolly <caleb.conno...@linaro.org>
---
   board/qualcomm/default.env | 12 ++++++++++++
   configs/qcom_defconfig     |  2 ++
   include/configs/qcom.h     |  7 -------
   3 files changed, 14 insertions(+), 7 deletions(-)
   create mode 100644 board/qualcomm/default.env

diff --git a/board/qualcomm/default.env b/board/qualcomm/default.env
new file mode 100644
index 000000000000..243aede77be7
--- /dev/null
+++ b/board/qualcomm/default.env
@@ -0,0 +1,12 @@
+stdin=serial,button-kbd
+stdout=serial,vidconsole
+stderr=serial,vidconsole
+bootfile=/extlinux/extlinux.conf

Is this file used to perform EFI boot or is it redundantly copied from
somewhere?

This is here because there's a bunch of codepaths in U-Boot that
implicitly expect the bootfile environment variable to have something
useful in. It stops a whole class of null pointer exceptions.

extlinux isn't used for EFI booting, but it's supported, and having a
sensible default here is nice-to-have regardless.

extlinux is a different boot method as compared to EFI, so it looks
confusing to use extlinux as default when we want EFI to be the
default. AFAICS, bootfile simply means the file to load and boot which
makes it more suitable for Linux image ("Image") to be the default
option.

Image? Why not Image.gz, vmlinuz, or vmlinuz.efi?

The value is meaningless, since all this does is stop U-Boot crashing if you run some commands without arguments.

The only times I've ever encountered bootfile has been in pxe or something related, where extlinux.conf was expected. Obviously this is a heavily overloaded variable...

Maybe I should just leave it unset to encourage folks to fix more of those unchecked accesses...

I don't think changing this to Image would be any more sensible than its current value.

With that change, feel free to add

Reviewed-by: Sumit Garg <sumit.g...@linaro.org>

-Sumit


Kind regards,

-Sumit

+preboot=scsi scan; usb start
+fastboot=fastboot -l $fastboot_addr_r usb 0
+do_boot=bootefi bootmgr
+bootmenu_0=Boot first available device=run do_boot
+bootmenu_1=Enable fastboot mode=run fastboot
+bootmenu_2=Reset device=reset
+menucmd=bootmenu
+bootcmd=run do_boot
diff --git a/configs/qcom_defconfig b/configs/qcom_defconfig
index 80ad3b32e139..a9e3797bb39a 100644
--- a/configs/qcom_defconfig
+++ b/configs/qcom_defconfig
@@ -35,8 +35,10 @@ CONFIG_CMD_BMP=y
   CONFIG_CMD_LOG=y
   CONFIG_OF_LIVE=y
   CONFIG_OF_BOARD_SETUP=y
   CONFIG_BUTTON_QCOM_PMIC=y
+CONFIG_USE_DEFAULT_ENV_FILE=y
+CONFIG_DEFAULT_ENV_FILE="board/qualcomm/default.env"
   CONFIG_CLK=y
   CONFIG_CLK_QCOM_QCM2290=y
   CONFIG_CLK_QCOM_QCS404=y
   CONFIG_CLK_QCOM_SDM845=y
diff --git a/include/configs/qcom.h b/include/configs/qcom.h
index e50b3bce5cdd..5b5ebbd844df 100644
--- a/include/configs/qcom.h
+++ b/include/configs/qcom.h
@@ -10,12 +10,5 @@
   #define __CONFIGS_SNAPDRAGON_H

   #define CFG_SYS_BAUDRATE_TABLE { 115200, 230400, 460800, 921600 }

-/* Load addressed are calculated during board_late_init(). See 
arm/mach-snapdragon/board.c */
-#define CFG_EXTRA_ENV_SETTINGS \
-       "stdin=serial,button-kbd\0"     \
-       "stdout=serial,vidconsole\0"    \
-       "stderr=serial,vidconsole\0" \
-       "bootcmd=bootm $prevbl_initrd_start_addr\0"
-
   #endif
--
2.45.0


--
// Caleb (they/them)

--
// Caleb (they/them)

Reply via email to