[RESEND PATCH v2] netconsole: various improvements

[Tony Dinh]

Hi Simon,

On Mon, Apr 24, 2023 at 12:43 PM Simon Glass <sjg at chromium.org> wrote:
>
> 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...

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