Hi Simon, On Mon, Apr 24, 2023 at 12:43 PM Simon Glass <s...@chromium.org> wrote: > > Hi Tony, > > On Wed, 19 Apr 2023 at 21:55, Tony Dinh <mibo...@gmail.com> wrote: > > > > On Wed, Apr 19, 2023 at 6:22 PM Tony Dinh <mibo...@gmail.com> wrote: > > > > > > HI Simon, > > > > > > On Tue, Apr 18, 2023 at 6:46 PM Simon Glass <s...@chromium.org> wrote: > > > > > > > > Hi Tony, > > > > > > > > On Mon, 3 Apr 2023 at 15:42, Tony Dinh <mibo...@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 <mibo...@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. > > 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
