[RESEND PATCH v2] netconsole: various improvements

Simon Glass sjg at chromium.org
Wed Apr 19 03:46:29 CEST 2023


Hi Tony,

On Mon, 3 Apr 2023 at 15:42, Tony Dinh <mibodhi at gmail.com> wrote:
>
> Use CONFIG_CONSOLE_MUX for netconsole. When netconsole is running,
> stdin/stdout/stder must be set to some primary console, in addtion to nc.
> For example, stdin=serial,nc. Some recent Linux kernels will not boot with
> only nc on the stdout list, ie. stdout=nc. When netconsole exits, remove
> nc from the list of devices in stdin/stdout/stderr.
>
> Signed-off-by: Tony Dinh <mibodhi at gmail.com>
> ---
>
> Changes in v2:
> - Select CONFIG_CONSOLE_MUX if CONFIG_NETCONSOLE is enabled
> - Add new functions in netconsole driver to support CONSOLE_MUX
> - Add new function to encapsulate the process of stopping netconsole
> from bootm
> - Remove unecessary net_timeout initial value = 1
> - Resend to correct missing <stdio_dev.h> include in bootm.c
>
>  boot/bootm.c             | 14 +++++++---
>  drivers/net/Kconfig      | 10 +++++++
>  drivers/net/netconsole.c | 60 ++++++++++++++++++++++++++++++++++++++++
>  include/stdio_dev.h      |  1 +
>  4 files changed, 81 insertions(+), 4 deletions(-)

This seems OK to me but for a few thoughts below.

But can we use this opportunity to move this to driver model? The
netconsole driver could be bound in eth_post_bind() and the code in
netconsole.c converted to be a driver.

I think that would be better than building on an out-of-date driver.

>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 2eec60ec7b..1b2542b570 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -18,6 +18,7 @@
>  #include <malloc.h>
>  #include <mapmem.h>
>  #include <net.h>
> +#include <stdio_dev.h>
>  #include <asm/cache.h>
>  #include <asm/global_data.h>
>  #include <asm/io.h>
> @@ -472,11 +473,16 @@ ulong bootm_disable_interrupts(void)
>          * recover from any failures any more...
>          */
>         iflag = disable_interrupts();
> -#ifdef CONFIG_NETCONSOLE
> -       /* Stop the ethernet stack if NetConsole could have left it up */
> -       eth_halt();
> -#endif
>
> +       if (IS_ENABLED(CONFIG_NETCONSOLE)) {
> +               /*
> +                * Make sure that the starting kernel message printed out,
> +                * stop netconsole and the ethernet stack
> +                */
> +               printf("\n\nStarting kernel ...\n\n");
> +               drv_nc_stop();
> +               eth_halt();
> +       }
>  #if defined(CONFIG_CMD_USB)
>         /*
>          * turn off USB to prevent the host controller from writing to the
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index ceadee98a1..0661059dfa 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -945,4 +945,14 @@ config MDIO_MUX_MESON_G12A
>           This driver is used for the MDIO mux found on the Amlogic G12A & compatible
>           SoCs.
>
> +config NETCONSOLE
> +       bool "Enable netconsole"
> +       select CONSOLE_MUX
> +       help
> +         NetConsole requires CONSOLE_MUX, i.e. at least one default console such
> +         must be specified in addition to nc console. For example, for boards that
> +         the main console is serial, set each of the envs stdin/stdout/stderr to serial,nc.
> +         See CONFIG_CONSOLE_MUX and CONFIG_SYS_CONSOLE_IS_IN_ENV help for detailed
> +         description of usage.
> +
>  endif # NETDEVICES
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 151bc55e07..bb92d2e130 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -7,6 +7,7 @@
>  #include <common.h>
>  #include <command.h>
>  #include <env.h>
> +#include <iomux.h>
>  #include <log.h>
>  #include <stdio_dev.h>
>  #include <net.h>
> @@ -33,6 +34,12 @@ static int output_packet_len;
>   */
>  enum proto_t net_loop_last_protocol = BOOTP;
>
> +/*
> + * Net console helpers
> + */
> +static void usage(void);
> +static void remove_nc_from(const int console);
> +
>  static void nc_wait_arp_handler(uchar *pkt, unsigned dest,
>                                  struct in_addr sip, unsigned src,
>                                  unsigned len)
> @@ -111,6 +118,21 @@ static int refresh_settings_from_env(void)
>         return 0;
>  }
>
> +static void remove_nc_from(const int console)
> +{
> +       int ret;
> +
> +       ret = iomux_replace_device(console, "nc", "nulldev");
> +       if (ret) {
> +               printf("\n*** Warning: Cannot remove nc from %s Error=%d\n",
> +                       stdio_names[console], ret);
> +               printf("%s=", stdio_names[console]);
> +               iomux_printdevs(console);
> +               usage();
> +               flush();
> +       }
> +}
> +
>  /**
>   * Called from net_loop in net/net.c before each packet
>   */
> @@ -241,6 +263,29 @@ static int nc_stdio_start(struct stdio_dev *dev)
>         return 0;
>  }
>
> +void nc_stdio_stop(void)
> +{
> +       if (IS_ENABLED(CONFIG_CONSOLE_MUX)) {
> +               int ret;
> +               struct stdio_dev *sdev;
> +
> +               /* Remove nc from each stdio file  */
> +               remove_nc_from(stdin);
> +               remove_nc_from(stderr);
> +               remove_nc_from(stdout);
> +
> +               /* Deregister nc device */
> +               sdev = stdio_get_by_name("nc");
> +               ret = stdio_deregister_dev(sdev, true);
> +               if (ret)
> +                       printf("\nWarning: Cannot deregister nc console Error=%d\n", ret);
> +       } else {
> +               printf("\n*** Warning: CONFIG_CONSOLE_MUX must be enabled for netconsole\n"
> +                       "to work properly. The nc console will be removed when\n"
> +                       "netconsole driver stops\n");
> +       }
> +}
> +
>  static void nc_stdio_putc(struct stdio_dev *dev, char c)
>  {
>         if (output_recursion)
> @@ -316,6 +361,16 @@ static int nc_stdio_tstc(struct stdio_dev *dev)
>         return input_size != 0;
>  }
>
> +static void usage(void)
> +{
> +       printf("USAGE:\n"
> +               "NetConsole requires CONFIG_CONSOLE_MUX. At least one default console\n"
> +               "must be specified in addition to nc console. For example, for boards\n"
> +               "that the main console is serial, set the each of envs stdin/stdout/stderr\n"
> +               "to serial,nc. Some kernel might fail to boot when nc is the only device in\n"
> +               "stdout list\n");

How about creating a doc/ file with info about net console? This is a
bit too much text to put in a user message.

> +}
> +
>  int drv_nc_init(void)
>  {
>         struct stdio_dev dev;
> @@ -335,3 +390,8 @@ int drv_nc_init(void)
>
>         return (rc == 0) ? 1 : rc;
>  }
> +
> +void drv_nc_stop(void)
> +{
> +       nc_stdio_stop();
> +}
> diff --git a/include/stdio_dev.h b/include/stdio_dev.h
> index 3105928970..9d2375a67e 100644
> --- a/include/stdio_dev.h
> +++ b/include/stdio_dev.h
> @@ -112,6 +112,7 @@ int drv_keyboard_init(void);
>  int drv_usbtty_init(void);
>  int drv_usbacm_init(void);
>  int drv_nc_init(void);
> +void drv_nc_stop(void);

Once this is a driver we won't need this.

>  int drv_jtag_console_init(void);
>  int cbmemc_init(void);

Regards,
Simon


More information about the U-Boot mailing list