[RESEND PATCH v2] netconsole: various improvements

[Simon Glass]

Hi Tony,

On Wed, 19 Apr 2023 at 21:55, Tony Dinh <mibodhi at gmail.com> wrote:
>
> On Wed, Apr 19, 2023 at 6:22 PM Tony Dinh <mibodhi at gmail.com> wrote:
> >
> > 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.

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