[PATCH] mach-snapdragon: move default environment to a file
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.connolly@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 +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
Hi Caleb,
On Mon, 17 Jun 2024 at 14:26, Caleb Connolly caleb.connolly@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.connolly@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?
-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
On 21/06/2024 09:35, Sumit Garg wrote:
Hi Caleb,
On Mon, 17 Jun 2024 at 14:26, Caleb Connolly caleb.connolly@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.connolly@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.
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
On Fri, 21 Jun 2024 at 13:19, Caleb Connolly caleb.connolly@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.connolly@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.connolly@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.
With that change, feel free to add
Reviewed-by: Sumit Garg sumit.garg@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)
On 21/06/2024 10:07, Sumit Garg wrote:
On Fri, 21 Jun 2024 at 13:19, Caleb Connolly caleb.connolly@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.connolly@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.connolly@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.garg@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)
On Fri, 21 Jun 2024 at 15:14, Caleb Connolly caleb.connolly@linaro.org wrote:
On 21/06/2024 10:07, Sumit Garg wrote:
On Fri, 21 Jun 2024 at 13:19, Caleb Connolly caleb.connolly@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.connolly@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.connolly@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...
That was my original suggestion too, drop it and feel free to keep a review tag with that.
-Sumit
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.garg@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)
participants (2)
-
Caleb Connolly -
Sumit Garg