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 8D960C43334 for ; Mon, 4 Jul 2022 08:41:49 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 4B979844F7; Mon, 4 Jul 2022 10:41:47 +0200 (CEST) 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="aXr7VnrY"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id D0CDA84512; Mon, 4 Jul 2022 10:41:45 +0200 (CEST) Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) (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 89DAB844F7 for ; Mon, 4 Jul 2022 10:41:42 +0200 (CEST) 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 dfw.source.kernel.org (Postfix) with ESMTPS id B22066125B; Mon, 4 Jul 2022 08:41:40 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id ECAB4C3411E; Mon, 4 Jul 2022 08:41:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656924100; bh=uZc9YYcPGW/+b/qYKwaRmdjsyU98c/Jl1H6Z7y4NwY8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=aXr7VnrYCdfbOvI4CqeBGjgcCEp3lWeLOvmPgVVphqpPAffJOf8PWOp/XWqjkKd81 uIVw2TJId0wMuKW0unIqx3oSIfToeDqf1g9ywIJzFUg0Ihf+g+aLQEfEx6G7G3MHIT vqrRwTYJ8+pRPjxIXdltbcHQKEncjDx8b7pqKeoadK4x/X6akGNG9dZo/sh21ndEye ha1aszJ/03IbOWhgCjSLbVjmCeKLs9dch1NGXePGAqJ+GDC3kMI0iefQgdE+g4HvO9 hP3mArdsuuvZ0eiS9Gtt79RH7zIIRRaRr8kGrOaX1LxUaWmfN4KVChvmXxNkR8Gjpt GBIaG4Am333GQ== Received: by pali.im (Postfix) id 160AB6E8; Mon, 4 Jul 2022 10:41:37 +0200 (CEST) Date: Mon, 4 Jul 2022 10:41:36 +0200 From: Pali =?utf-8?B?Um9ow6Fy?= To: Stefan Roese Cc: Simon Glass , u-boot@lists.denx.de, Marek =?utf-8?B?QmVow7pu?= Subject: Re: [PATCH] serial: ns16550: Wait in debug_uart_init until tx buffer is empty Message-ID: <20220704084136.pgxsddnvshsx7a6p@pali> References: <20220623121356.4149-1-pali@kernel.org> <32ac71aa-2d33-6b8b-a0aa-122a71973867@denx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <32ac71aa-2d33-6b8b-a0aa-122a71973867@denx.de> 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.6 at phobos.denx.de X-Virus-Status: Clean On Friday 01 July 2022 09:21:34 Stefan Roese wrote: > On 23.06.22 14:13, Pali Rohár wrote: > > Commit d293759d55cc ("serial: ns16550: Add support for > > SPL_DEBUG_UART_BASE") fixed support for setting correct early debug UART > > base address in SPL. > > > > But after this commit, output from Marvell A385 BootROM is truncated or > > lost and not fully present on serial console. > > > > Debugging this issue showed that BootROM just put bytes into UART HW output > > buffer and does not wait until UART HW transmit all characters. U-Boot > > ns16550 early debug is initialized very early and during its initialization > > is resetting UART HW and flushing remaining transmit buffer (which still > > contains BootROM output). > > > > Fix this issue by waiting in init function prior resetting UART HW until > > TxEmpty bit in UART Line Status Register is set. TxEmpty is set when all > > remaining bytes from HW buffer are transmitted. > > > > Signed-off-by: Pali Rohár > > Just checking, do you need a "Fixes" tag as well here? Well, ns16550_init() function already contains that waiting for UART_LSR_TEMT bit. This was introduced in following commit years ago: https://source.denx.de/u-boot/u-boot/-/commit/cb55b3320014b7f6014416c556fe506efbf0a84b So probably the "fixes" tag could contain commit which introduced _debug_uart_init() function. It was in this commit years ago: https://source.denx.de/u-boot/u-boot/-/commit/21d004368fc8a4da07147c58dfe9a4e16d4ab761 Support for SPL_DEBUG_UART_BASE in my recent commit just caused to hit issue that in _debug_uart_init() is missing waiting until TxEmpty. > > --- > > drivers/serial/ns16550.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c > > index 78bfe6281ce3..13418004e2b0 100644 > > --- a/drivers/serial/ns16550.c > > +++ b/drivers/serial/ns16550.c > > @@ -328,6 +328,8 @@ static inline void _debug_uart_init(void) > > struct ns16550 *com_port = (struct ns16550 *)CONFIG_VAL(DEBUG_UART_BASE); > > int baud_divisor; > > + while (!(serial_din(&com_port->lsr) & UART_LSR_TEMT)); > > I personally prefer the ";" in a separate line instead. Not sure what > the official coding style mentions to such constructs. I have no problem with this change. If you prefer it, feel free to move ";" to next separate line. > Anyways: > > Reviewed-by: Stefan Roese > > Thanks, > Stefan