mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Marco Felsch <m.felsch@pengutronix.de>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: barebox@lists.infradead.org, bst@pengutronix.de
Subject: Re: [PATCH] startup: add new AUTOBOOT_HALT state
Date: Tue, 22 Oct 2024 08:18:39 +0200	[thread overview]
Message-ID: <20241022061839.vxdlbf6yo6ct5xl3@pengutronix.de> (raw)
In-Reply-To: <20241021175507.3529332-1-a.fatoum@pengutronix.de>

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.

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
> 
> 
> 



  reply	other threads:[~2024-10-22  6:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-21 17:55 Ahmad Fatoum
2024-10-22  6:18 ` Marco Felsch [this message]
2024-10-22  6:19   ` Ahmad Fatoum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241022061839.vxdlbf6yo6ct5xl3@pengutronix.de \
    --to=m.felsch@pengutronix.de \
    --cc=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=bst@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox