[RESEND PATCH v2] netconsole: various improvements
Tony Dinh
mibodhi at gmail.com
Thu Apr 20 03:22:45 CEST 2023
HI Simon,
On Tue, Apr 18, 2023 at 6:46 PM Simon Glass <sjg at chromium.org> wrote:
>
> 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.
I'd agree that converting to a driver model is the logical next step.
My motivation is to fix the booting problem for the Linux kernel first
(I'm having problems booting some kernels without this patch). And
then in the next go round, I'll convert netconsole to a driver. Most
of the code in this small patch will be needed whether it is an old
driver or DM anyway.
>
> >
> > 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.
Sure.
>
> > +}
> > +
> > 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.
Yes, but we still need the nc_stdio_stop() to do the cleanup, i.e.
removing nc from stdin/stdout/stderr and deregistering the nc device.
Except that it will be a uclass member like .pre_remove()?
It will be a bit of a learning curve for me to do the DM netconsole.
But I will give it a shot.
Thanks,
Tony
>
> > int drv_jtag_console_init(void);
> > int cbmemc_init(void);
>
> Regards,
> Simon
More information about the U-Boot
mailing list