Hi Simon,
On Sun, Mar 19, 2023 at 12:30 PM Simon Glass <s...@chromium.org> wrote:
>
> Hi Tony,
>
> On Sun, 19 Mar 2023 at 10:46, Tony Dinh <mibo...@gmail.com> wrote:
> >
> > - When Netconsole is running, stdin/stdout/stderr are set to nc. Reset
> > stdin/stdout/stderr to serial (a sane deffault) before booting kernel.
>
> spelling
OK.
>
> > - Enable net_timeout when netconsole starts will give a better user
> > experience if netconsole server is not running.
> >
> > Signed-off-by: Tony Dinh <mibo...@gmail.com>
> > ---
> >
> > boot/bootm.c | 16 +++++++++++++++-
> > drivers/net/netconsole.c | 2 +-
> > 2 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/boot/bootm.c b/boot/bootm.c
> > index 2eec60ec7b..c4a3aaf1bd 100644
> > --- a/boot/bootm.c
> > +++ b/boot/bootm.c
> > @@ -473,7 +473,21 @@ ulong bootm_disable_interrupts(void)
> > */
> > iflag = disable_interrupts();
> > #ifdef CONFIG_NETCONSOLE
>
> Can you convert this to 'if IS_ENABLED(...)' at the same time?
Sure I will.
>
> > - /* Stop the ethernet stack if NetConsole could have left it up */
> > + /*
> > + * Make sure that the starting kernel message printed out.
> > + * Reset stdin/out/err back to serial and stop the ethernet
> > + * stack if NetConsole could have left it up
> > + */
> > + char *s;
> > + int ret;
> > +
> > + s = env_get("stdout");
> > + if (strcmp(s, "nc") == 0) {
> > + printf("\n\nStarting kernel ...\n");
> > + ret = env_set("stdin", "serial");
> > + ret = env_set("stdout", "serial");
> > + ret = env_set("stderr", "serial");
> > + }
> > eth_halt();
> > #endif
> >
> > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> > index 151bc55e07..2091014918 100644
> > --- a/drivers/net/netconsole.c
> > +++ b/drivers/net/netconsole.c
> > @@ -20,7 +20,7 @@ static int input_size; /* char count in input buffer */
> > static int input_offset; /* offset to valid chars in input buffer */
> > static int input_recursion;
> > static int output_recursion;
> > -static int net_timeout;
> > +static int net_timeout = 1;
>
> What is this change?
I've been carrying this one-line patch out-of-tree for a few years.
Back when I saw that the netconsole started transitioning faster from
the serial console (I tested with 2 consoles running) . But I can no
longer prove that anymore! code change... I think it does not hurt to
set it to timeout initially anyway, since netconsole is polling for
chars. But I will remove this part of the patch if others think it is
not necessary.
>
> > static uchar nc_ether[6]; /* server enet address */
> > static struct in_addr nc_ip; /* server ip */
> > static short nc_out_port; /* target output port */
> > --
> > 2.30.2
> >
>
> Could you take a look at converting netconsole to driver model? It
> should be a child of the eth device probably. I think this patch is OK
> for, but it would be better if the driver could deregister itself from
> stdio in its remove() method, perhaps installing serial at that point.
Agreed. It would be nice to have netconsole as a driver model. It will
get more attention that it deserves. I'm a bit short for free time
atm, though. I will make it a task to work on it.
Thanks,
Tony
> Regards,
> SImon
