From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 00929C7618A for ; Mon, 20 Mar 2023 08:00:55 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 896C685A83; Mon, 20 Mar 2023 09:00:53 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="eqf7485Z"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 46A5B85B2A; Mon, 20 Mar 2023 09:00:50 +0100 (CET) Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id EDCBE85A62 for ; Mon, 20 Mar 2023 09:00:46 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=pali@kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 6F5F1B80D6D; Mon, 20 Mar 2023 08:00:46 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C6353C4339B; Mon, 20 Mar 2023 08:00:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1679299245; bh=TNjGGzoWYLQyreKdc+AcE9naC0zgzTH8SmiB88PsUuA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=eqf7485ZDhQ2GvDyEA0DVafjOo3tRO6XwHtELuOeO8bG09lbi/cBzV8bmHdQ/iYS+ 80i+HxPJSfMpRDnH+DuN1MdfwAQpmHpvby4DLUTsVy0qfZ+8u/GxVfBucfG+P687ub Oq5q1D70tUvWbwyDXV70M6eU/rLm4zBnvpEjj3i6aOsSlAsFCsgdZniOIBK7cxoK1t xcch157/Ehx0r2OlmjwmClz8Bfi8US4NP4QZA51NVj122rc0clPApKIO99P8QiWUc1 CcEK/8cvkSu+roLJkam1Kwxh+wN4LubTjX+MNj7miXmF+Lf/NaUO6CWcvsJEj/SeNM JEom/YmkuL+Yw== Received: by pali.im (Postfix) id E169456B; Mon, 20 Mar 2023 09:00:41 +0100 (CET) Date: Mon, 20 Mar 2023 09:00:41 +0100 From: Pali =?utf-8?B?Um9ow6Fy?= To: Tony Dinh Cc: Joe Hershberger , Ramon Fried , U-Boot Mailing List , Tom Rini , Stefan Roese , Simon Glass , Daniel Golle , Philippe Reynes , Sean Anderson Subject: Re: [PATCH] netconsole: various improvements Message-ID: <20230320080041.x7qaajrbp2pmleqe@pali> References: <20230318214626.28655-1-mibodhi@gmail.com> <20230319193627.rw2dvwyrnsx7lyq6@pali> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20180716 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean On Sunday 19 March 2023 16:25:25 Tony Dinh wrote: > Hi Pali, > > On Sun, Mar 19, 2023 at 12:36 PM Pali Rohár wrote: > > > > On Saturday 18 March 2023 14:46:25 Tony Dinh wrote: > > > - When Netconsole is running, stdin/stdout/stderr are set to nc. Reset > > > stdin/stdout/stderr to serial (a sane deffault) before booting kernel. > > > > This can be a problematic. serial output does not have to be available > > for all devices. For example on Nokia N900 phone is available only > > lcd/vga display device. > > Here is a shortcoming of the current implementation of netconsole. > When it starts, the user sets the stdin/stdout/stderr envs to nc, and > we lose the previous state of the console. Especially with the > CONSOLE_MUX is not enabled. There is no way to set them back by > setting the envs in booting logic. Cannot user use console mux and set stdout to both serial and nc at the same time? > It sounds like we need to implement internal envs to save the previous > state, and then restore it before shutting down the network and > booting the kernel. > I recall in previous Linux kernel versions many years ago Linux still > booted OK, but recently it can no longer boot with the stdio set to nc > (I've tested this failure case). Perhaps nc should _not_ be set > explicitly by the user. We might need a u-boot command to start > netconsole, and that would include checking if the serverip is > running? > > > > > Also this can break CONSOLE_MUX support when more devices are specified > > in stdin/stdout/stderr env variables. With CONSOLE_MUX, output is send > > to more than one device. User may set it to both serial and nc or also > > to other output (e.g. to lcd like on Nokia N900). > > > > So in my opinion, if "nc" is is going to be turned off then just "nc" > > string should be removed from env variable and let all other devices > > stay in env variables. > > > > Maybe you can use something like this? (taken from common/usb_kbd.c) > > > > #if CONFIG_IS_ENABLED(CONSOLE_MUX) > > if (iomux_replace_device(stdin, "nc", "nulldev")) > > return 1; > > #endif > > Thanks for pointing out that fact about CONSOLE_MUX. Yes I will > incorporate that as part of the logic. > > All the best, > Tony > > > > > > - Enable net_timeout when netconsole starts will give a better user > > > experience if netconsole server is not running. > > > > > > Signed-off-by: Tony Dinh > > > --- > > > > > > 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 > > > - /* 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; > > > 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 > > >