On Fri, 21 Jun 2024 at 15:14, Caleb Connolly <[email protected]> wrote: > > > > On 21/06/2024 10:07, Sumit Garg wrote: > > On Fri, 21 Jun 2024 at 13:19, Caleb Connolly <[email protected]> > > wrote: > >> > >> > >> > >> On 21/06/2024 09:35, Sumit Garg wrote: > >>> Hi Caleb, > >>> > >>> On Mon, 17 Jun 2024 at 14:26, Caleb Connolly <[email protected]> > >>> 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 <[email protected]> > >>>> --- > >>>> 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 <[email protected]> > > > > -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)
