Re: [RESEND PATCH v2] netconsole: various improvements
On Mon, May 8, 2023 at 3:53 PM Tony Dinh wrote: > > Hi Simon, > > On Mon, May 8, 2023 at 2:23 PM Simon Glass wrote: > > > > Hi Tony, > > > > On Sun, 7 May 2023 at 22:25, Tony Dinh wrote: > > > > > > Hi Tom, > > > > > > On Fri, May 5, 2023 at 11:34 AM Tom Rini wrote: > > > > > > > > On Mon, Apr 03, 2023 at 02:41:52PM -0700, Tony Dinh 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 > > > > > > > > This breaks the ut_bootstd_bootflow_cmd_boot test on sandbox. I'm not > > > > sure if it's a test issue or something else. > > > > > > Thanks for trying, I'm having some difficulty building a sandbox > > > u-boot, so I can't get to the point where I can see what's the problem > > > yet. > > > > What is going wrong? Does this help? > > > > https://u-boot.readthedocs.io/en/latest/build/gcc.html > > Yes, I've done that when I realized my ARM development box setup > missed several Debian packages building sandbox (I only build Linux > kernels and U-Boot boards natively on ARM). However, after installing > all those packages it is still not possible to build a sandbox on ARM. > So currently I am switching to cross compiling on my Intel laptop. > Later, I'll come back to the ARM box to see whether we need some > documentation update for doc/build/gcc.rst. FYI, what's missing in the doc/build/gcc.rst is the SDL option. I could not build without NO_SDL=1. make sandbox_defconfig all NO_SDL=1 All the best, Tony
Re: [RESEND PATCH v2] netconsole: various improvements
Hi Simon, On Mon, May 8, 2023 at 2:23 PM Simon Glass wrote: > > Hi Tony, > > On Sun, 7 May 2023 at 22:25, Tony Dinh wrote: > > > > Hi Tom, > > > > On Fri, May 5, 2023 at 11:34 AM Tom Rini wrote: > > > > > > On Mon, Apr 03, 2023 at 02:41:52PM -0700, Tony Dinh 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 > > > > > > This breaks the ut_bootstd_bootflow_cmd_boot test on sandbox. I'm not > > > sure if it's a test issue or something else. > > > > Thanks for trying, I'm having some difficulty building a sandbox > > u-boot, so I can't get to the point where I can see what's the problem > > yet. > > What is going wrong? Does this help? > > https://u-boot.readthedocs.io/en/latest/build/gcc.html Yes, I've done that when I realized my ARM development box setup missed several Debian packages building sandbox (I only build Linux kernels and U-Boot boards natively on ARM). However, after installing all those packages it is still not possible to build a sandbox on ARM. So currently I am switching to cross compiling on my Intel laptop. Later, I'll come back to the ARM box to see whether we need some documentation update for doc/build/gcc.rst. Thanks, Tony > > Regards, > Simon
Re: [RESEND PATCH v2] netconsole: various improvements
Hi Tony, On Sun, 7 May 2023 at 22:25, Tony Dinh wrote: > > Hi Tom, > > On Fri, May 5, 2023 at 11:34 AM Tom Rini wrote: > > > > On Mon, Apr 03, 2023 at 02:41:52PM -0700, Tony Dinh 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 > > > > This breaks the ut_bootstd_bootflow_cmd_boot test on sandbox. I'm not > > sure if it's a test issue or something else. > > Thanks for trying, I'm having some difficulty building a sandbox > u-boot, so I can't get to the point where I can see what's the problem > yet. What is going wrong? Does this help? https://u-boot.readthedocs.io/en/latest/build/gcc.html Regards, Simon
Re: [RESEND PATCH v2] netconsole: various improvements
Hi Tom, On Fri, May 5, 2023 at 11:34 AM Tom Rini wrote: > > On Mon, Apr 03, 2023 at 02:41:52PM -0700, Tony Dinh 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 > > This breaks the ut_bootstd_bootflow_cmd_boot test on sandbox. I'm not > sure if it's a test issue or something else. Thanks for trying, I'm having some difficulty building a sandbox u-boot, so I can't get to the point where I can see what's the problem yet. All the best, Tony > > -- > Tom
Re: [RESEND PATCH v2] netconsole: various improvements
On Mon, Apr 03, 2023 at 02:41:52PM -0700, Tony Dinh 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 This breaks the ut_bootstd_bootflow_cmd_boot test on sandbox. I'm not sure if it's a test issue or something else. -- Tom signature.asc Description: PGP signature
Re: [RESEND PATCH v2] netconsole: various improvements
On Mon, Apr 24, 2023 at 07:06:08PM -0700, Tony Dinh wrote: > Hi Simon, > > On Mon, Apr 24, 2023 at 12:43 PM Simon Glass wrote: > > > > Hi Tony, > > > > On Wed, 19 Apr 2023 at 21:55, Tony Dinh wrote: > > > > > > On Wed, Apr 19, 2023 at 6:22 PM Tony Dinh wrote: > > > > > > > > HI Simon, > > > > > > > > On Tue, Apr 18, 2023 at 6:46 PM Simon Glass wrote: > > > > > > > > > > Hi Tony, > > > > > > > > > > On Mon, 3 Apr 2023 at 15:42, Tony Dinh 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 > > > > > > --- > > > > > > > > > > > > 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 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. > > > > This crosses the line to a feature, IMO... > > My initial patch would be a pure bug fix. But it also breaks other > boards like Nokia, as Pali has pointed out. So in the v2 patch, > Console Mux is introduced to cover all boards. I don't think that we > are adding any features here. > > It's just that I'm using netconsole often so I noticed the booting > problem with recent Linux kernels. It's kind of a catch-22, without a > serial console I cannot debug Linux early booting problems, and with > serial console active there is no booting problem. Yes, we should fix functionality then work on clean-ups. -- Tom signature.asc Description: PGP signature
Re: [RESEND PATCH v2] netconsole: various improvements
Hi Simon, On Mon, Apr 24, 2023 at 12:43 PM Simon Glass wrote: > > Hi Tony, > > On Wed, 19 Apr 2023 at 21:55, Tony Dinh wrote: > > > > On Wed, Apr 19, 2023 at 6:22 PM Tony Dinh wrote: > > > > > > HI Simon, > > > > > > On Tue, Apr 18, 2023 at 6:46 PM Simon Glass wrote: > > > > > > > > Hi Tony, > > > > > > > > On Mon, 3 Apr 2023 at 15:42, Tony Dinh 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 > > > > > --- > > > > > > > > > > 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 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. > > This crosses the line to a feature, IMO... My initial patch would be a pure bug fix. But it also breaks other boards like Nokia, as Pali has pointed out. So in the v2 patch, Console Mux is introduced to cover all boards. I don't think that we are adding any features here. It's just that I'm using netconsole often so I noticed the booting problem with recent Linux kernels. It's kind of a catch-22, without a serial console I cannot debug Linux early booting problems, and with serial console active there is no booting problem. All the best, Tony > > [..] > > > > > > 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()? > > Yes, or perhaps the driver's remove() method. > > > > > > > It will be a bit of a learning curve for me to do the DM netconsole. > > > But I will give it a shot. > > > > In the meantime, can this patch get merged on its own merit as a bug fix? > > I suppose so, so long as you can figure out the conversion. Let me > know if you have any questions. > > Regards, > Simon
Re: [RESEND PATCH v2] netconsole: various improvements
Hi Tony, On Wed, 19 Apr 2023 at 21:55, Tony Dinh wrote: > > On Wed, Apr 19, 2023 at 6:22 PM Tony Dinh wrote: > > > > HI Simon, > > > > On Tue, Apr 18, 2023 at 6:46 PM Simon Glass wrote: > > > > > > Hi Tony, > > > > > > On Mon, 3 Apr 2023 at 15:42, Tony Dinh 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 > > > > --- > > > > > > > > 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 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. This crosses the line to a feature, IMO... [..] > > > > 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()? Yes, or perhaps the driver's remove() method. > > > > It will be a bit of a learning curve for me to do the DM netconsole. > > But I will give it a shot. > > In the meantime, can this patch get merged on its own merit as a bug fix? I suppose so, so long as you can figure out the conversion. Let me know if you have any questions. Regards, Simon
Re: [RESEND PATCH v2] netconsole: various improvements
On Wed, Apr 19, 2023 at 6:22 PM Tony Dinh wrote: > > HI Simon, > > On Tue, Apr 18, 2023 at 6:46 PM Simon Glass wrote: > > > > Hi Tony, > > > > On Mon, 3 Apr 2023 at 15:42, Tony Dinh 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 > > > --- > > > > > > 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 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 > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -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 > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -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 conso
Re: [RESEND PATCH v2] netconsole: various improvements
HI Simon, On Tue, Apr 18, 2023 at 6:46 PM Simon Glass wrote: > > Hi Tony, > > On Mon, 3 Apr 2023 at 15:42, Tony Dinh 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 > > --- > > > > 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 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 > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -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 > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -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])
Re: [RESEND PATCH v2] netconsole: various improvements
Hi Tony, On Mon, 3 Apr 2023 at 15:42, Tony Dinh 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 > --- > > 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 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 > #include > #include > +#include > #include > #include > #include > @@ -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 > #include > #include > +#include > #include > #include > #include > @@ -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 */ > +
Re: [RESEND PATCH v2] netconsole: various improvements
Hi Ramon, Hi Joe, Any comments on this patch? Thanks, Tony On Mon, Apr 3, 2023 at 2:42 PM Tony Dinh 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 > --- > > 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 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(-) > > 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 > #include > #include > +#include > #include > #include > #include > @@ -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 > #include > #include > +#include > #include > #include > #include > @@ -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 { > +