Re: [PATCH] startup: add new AUTOBOOT_HALT state

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Marco,

On 22.10.24 08:18, Marco Felsch wrote:
> Hi Ahmad,
> 
> On 24-10-21, Ahmad Fatoum wrote:
>> To make the barebox shell more convenient to use, barebox executes some
>> actions only when dropping to an interactive shell:
>>
>>   - If dropping due to global.autoboot=abort, all watchdogs are
>>     automatically quiesced
>>   - In every case, network interfaces are brought up, so link
>>     negotiation happens in parallel to waiting for user input
>>
>> This is useful for development and debugging, but bad for automatic
>> tests as the system behaves differently when automatic test aborts
>> barebox to set e.g. a boot argument and when barebox is left to boot
>> uninterrupted.
>>
>> For use in such scenarios, let's introduce a new AUTOBOOT_HALT state.
>> This will not do any automatic interactive magic and can be entered by
>> either setting nv.autoboot=halt or by aborting the boot with ctrl+d.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> Thoughts?
>>
>> Alternatively, we could make only aborting the boot via e.g. enter
>> enable the network interfaces.
> 
> To have a more fine grained option we could use
> global.autoboot.skip_eth_auto_enable instead of making use of new
> autoboot state the user need to check first to know the differences
> between 'abort' and 'halt'. Of course this comes with the drawback of
> having a new parameter the user need to set but IMHO is more future
> proof e.g. if autoboot does more automagic in the future which can be
> controlled via global.autoboot.* params as well.

We still need a way for a test suite to abort boot without bringing
up network interfaces automatically - and without writing the environment.

Cheers,
Ahmad

> 
> Regards,
>   Marco
> 
>> ---
>>  common/startup.c  | 11 ++++++++---
>>  include/barebox.h |  3 +++
>>  2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/startup.c b/common/startup.c
>> index 47b70a7756cb..5d0ac116b98c 100644
>> --- a/common/startup.c
>> +++ b/common/startup.c
>> @@ -28,6 +28,7 @@
>>  #include <linux/stat.h>
>>  #include <envfs.h>
>>  #include <magicvar.h>
>> +#include <readkey.h>
>>  #include <linux/reboot-mode.h>
>>  #include <asm/sections.h>
>>  #include <uncompress.h>
>> @@ -103,6 +104,7 @@ static int global_autoboot_timeout = 3;
>>  static const char * const global_autoboot_states[] = {
>>  	[AUTOBOOT_COUNTDOWN] = "countdown",
>>  	[AUTOBOOT_ABORT] = "abort",
>> +	[AUTOBOOT_HALT] = "halt",
>>  	[AUTOBOOT_MENU] = "menu",
>>  	[AUTOBOOT_BOOT] = "boot",
>>  };
>> @@ -176,7 +178,8 @@ enum autoboot_state do_autoboot_countdown(void)
>>  		return autoboot_state;
>>  
>>  	if (!console_get_first_active() &&
>> -	    global_autoboot_state != AUTOBOOT_ABORT) {
>> +	    global_autoboot_state != AUTOBOOT_ABORT &&
>> +	    global_autoboot_state != AUTOBOOT_HALT) {
>>  		printf("\nNon-interactive console, booting system\n");
>>  		return autoboot_state = AUTOBOOT_BOOT;
>>  	}
>> @@ -215,6 +218,8 @@ enum autoboot_state do_autoboot_countdown(void)
>>  		autoboot_state = AUTOBOOT_BOOT;
>>  	else if (menu_exists && outkey == 'm')
>>  		autoboot_state = AUTOBOOT_MENU;
>> +	else if (outkey == CTL_CH('d'))
>> +		autoboot_state = AUTOBOOT_HALT;
>>  	else
>>  		autoboot_state = AUTOBOOT_ABORT;
>>  
>> @@ -310,7 +315,7 @@ static int run_init(void)
>>  	if (autoboot == AUTOBOOT_BOOT)
>>  		run_command("boot");
>>  
>> -	if (IS_ENABLED(CONFIG_NET))
>> +	if (IS_ENABLED(CONFIG_NET) && autoboot != AUTOBOOT_HALT)
>>  		eth_open_all();
>>  
>>  	if (autoboot == AUTOBOOT_MENU)
>> @@ -400,7 +405,7 @@ void shutdown_barebox(void)
>>  }
>>  
>>  BAREBOX_MAGICVAR(global.autoboot,
>> -                 "Autoboot state. Possible values: countdown (default), abort, menu, boot");
>> +                 "Autoboot state. Possible values: countdown (default), abort, halt, menu, boot");
>>  BAREBOX_MAGICVAR(global.autoboot_abort_key,
>>                   "Which key allows to interrupt autoboot. Possible values: any, ctrl-c");
>>  BAREBOX_MAGICVAR(global.autoboot_timeout,
>> diff --git a/include/barebox.h b/include/barebox.h
>> index 45e95f187462..b3d34b7b739b 100644
>> --- a/include/barebox.h
>> +++ b/include/barebox.h
>> @@ -22,6 +22,8 @@ extern int (*barebox_main)(void);
>>   * enum autoboot_state - autoboot action after init
>>   * @AUTOBOOT_COUNTDOWN: Count down to automatic boot action
>>   * @AUTOBOOT_ABORT: Abort boot and drop to barebox shell
>> + * @AUTOBOOT_HALT: Halt boot and drop to barebox shell, _without_ running
>> + *                 any interactive hooks (e.g. bringing up network)
>>   * AUTOBOOT_MENU: Show main menu directly
>>   * @AUTOBOOT_BOOT: Boot right away without an interruptible countdown
>>   * @AUTOBOOT_UNKNOWN: Boot right away without an interruptible countdown
>> @@ -32,6 +34,7 @@ extern int (*barebox_main)(void);
>>  enum autoboot_state {
>>  	AUTOBOOT_COUNTDOWN,
>>  	AUTOBOOT_ABORT,
>> +	AUTOBOOT_HALT,
>>  	AUTOBOOT_MENU,
>>  	AUTOBOOT_BOOT,
>>  	AUTOBOOT_UNKNOWN,
>> -- 
>> 2.39.5
>>
>>
>>
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |