Received: by 2002:a05:7412:d8a:b0:e2:908c:2ebd with SMTP id b10csp2453190rdg; Mon, 16 Oct 2023 05:14:09 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGLmBkUTLnxt76ZK7oM8IortOUIpUnGv1gHhse60UqjBlJda6XCMsp/CdgodyxQUgfwnw+b X-Received: by 2002:a05:6830:1d3:b0:6bb:1049:6919 with SMTP id r19-20020a05683001d300b006bb10496919mr39244121ota.12.1697458448770; Mon, 16 Oct 2023 05:14:08 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697458448; cv=none; d=google.com; s=arc-20160816; b=aGLn+4M6LjzOPXvp/p+zKvXbuzZ65zdIvhyUhIQ+JNF6eoVRiLcrHFfVsQCb7BGlvg NYeHPWN18kCTkFEBPoJrNeRYP337r67/Uw9sLdXj4L7nf+If+I10WOc9CNu+1vBHZ0EV flLZYXBx2Zj6vVfBDtutSvnST1sz//VHTImdVvlyuoyiQ55k12WYm8Gpmmwj415w5sEs Y93CXpeQQ5kuQZI3S1v4Ns9mhcv9K4BCSnIKkLaXn2qfbXQh6Rm0LHKVgET6VKIXf7I2 xcO6Yl5Uw4VGjamhuHxJ9fiXzHAKomeK7sLM1T4pfDTcYf2hM7rG2LUJ5W6DPwI/tEbH jcbg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:references:message-id:in-reply-to :subject:cc:to:from:date:dkim-signature; bh=TblkvUx1DRW0VXGVwr6jfmRZlydVb2tIOWt2HYo2cDc=; fh=35ZdEhtSaqQUPgaeU/Yy7h23dRtnyABHgQQLEueQkGw=; b=S2MsitZnnRoAOHhgHw78r0FQEuLZ6HcVbO/mxCBfVUzZzumzlver2F4TJhrUpC15uD Sl+y2gDdQZ1Z4yaXLdke8kcLQiHGc/RXKW7WMo+j1fa6GYISRhv4Ucf+LChI4awY9YBv 0Pf68uai5WCDBDjzwr0Wu3PUdiIuzLIHvtCWR+/HST7uh/xHVZGnSpEO0wiRxJnIHSxj c/m8fAuEQ17u0JUb9rNn2L7pESv9FJSXTNcqUUhV3bX4e4uQueFg2n9mC/Q3MZaPAgA1 OIjmTPP344rVquMNUCoZnQHbCR9A5cCCLXl6XGM1k4nD/6nSzaOHq5iUt7BzjIxNfe6P a2qA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=YJIgkJki; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Return-Path: Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id d19-20020a056a0024d300b006b47a8c2a67si7180614pfv.359.2023.10.16.05.14.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 Oct 2023 05:14:08 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=YJIgkJki; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id E3BC7807593C; Mon, 16 Oct 2023 05:14:07 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231868AbjJPMOF (ORCPT + 99 others); Mon, 16 Oct 2023 08:14:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34044 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229621AbjJPMOE (ORCPT ); Mon, 16 Oct 2023 08:14:04 -0400 Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.31]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DD5718E; Mon, 16 Oct 2023 05:14:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1697458442; x=1728994442; h=date:from:to:cc:subject:in-reply-to:message-id: references:mime-version; bh=fYX09gVTs+rdDQq31Aoyype6rTbwF7iR1knuJuk7e3c=; b=YJIgkJkiK06uaWWlR8PBXUYghOok8tWbm6xqg/Gp+UrIm9/teAqypPSw XblyLHWYKcS4rlVfvmM7j4jYLckJzWY0100mIwISAO0DwH5jLtJ5jDeK4 IQDwrBX3rZGy3Y6cop36fusWR3WZZX04dfUSQXYr3Tj9u17s2ZTOGJoyv xIHHEqoY3XlztNI0a3wFGLaUIpiHiOYrVVjliyK6RhygxHcQXz0PhferE gOwpVBxSJIhsIbwmRr8anteOrjdWGWjHl9NgS/taEIgStVhwyyY7/2N80 jGzNad//S4EJFuYrPPAqW/QHYuvrFWlircBFxHWDMtB5tMblDY6uXpqiI w==; X-IronPort-AV: E=McAfee;i="6600,9927,10863"; a="449720154" X-IronPort-AV: E=Sophos;i="6.03,229,1694761200"; d="scan'208";a="449720154" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Oct 2023 05:14:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10863"; a="929322173" X-IronPort-AV: E=Sophos;i="6.03,229,1694761200"; d="scan'208";a="929322173" Received: from rhaeussl-mobl.ger.corp.intel.com ([10.252.59.103]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Oct 2023 05:13:59 -0700 Date: Mon, 16 Oct 2023 15:13:56 +0300 (EEST) From: =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= To: Richard Laing cc: Greg Kroah-Hartman , Jiri Slaby , robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, ilpo.jarvinen@linux.intel.com, Andy Shevchenko , LKML , linux-serial , devicetree@vger.kernel.org Subject: Re: [PATCH] serial: 8250_dw: Allow TX FIFO to drain before writing to UART_LCR An issue has been observed on the Broadcom BCM56160 serial port which appears closely related to a similar issue on the Marvell Armada 38x serial port. In-Reply-To: <20231016013207.2249946-2-richard.laing@alliedtelesis.co.nz> Message-ID: References: <20231016013207.2249946-1-richard.laing@alliedtelesis.co.nz> <20231016013207.2249946-2-richard.laing@alliedtelesis.co.nz> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Mon, 16 Oct 2023 05:14:08 -0700 (PDT) On Mon, 16 Oct 2023, Richard Laing wrote: Your subject line is way too long. If you refer to some other issue, please link to it properly with commit id and/or with Link: tags. > Writes to UART_LCR can result in characters that are currently held in the > TX FIFO being lost rather than sent, even if the userspace process has > attempted to flush them. > > This is most visible when using the "resize" command (tested on Busybox), > where we have observed the escape code for restoring cursor position > becoming mangled. > > Since this appears to be a more common problem add a new driver option > to flush the TX FIFO before writing to the UART_LCR. This looks like a problem we already have solution for, the userspace can use TCSADRAIN/FLUSH to indicate what kind of flushing it wants for Tx when it makes the tcsetattr() call. Thus, userspace can avoid the Tx side corruption as long as its behavior is sane and doesn't e.g. try to race writes with tcsetattr() call as mentioned in commit 094fb49a2d0d ("tty: Prevent writing chars during tcsetattr TCSADRAIN/FLUSH"). Have you tried to use the userspace solution? Isn't it working for some reason? -- i. > > Signed-off-by: Richard Laing > --- > drivers/tty/serial/8250/8250_dw.c | 18 ++++++++++++++++++ > drivers/tty/serial/8250/8250_dwlib.h | 1 + > 2 files changed, 19 insertions(+) > > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c > index f4cafca1a7da..17ee824294c7 100644 > --- a/drivers/tty/serial/8250/8250_dw.c > +++ b/drivers/tty/serial/8250/8250_dw.c > @@ -161,6 +161,10 @@ static void dw8250_serial_out(struct uart_port *p, int offset, int value) > { > struct dw8250_data *d = to_dw8250_data(p->private_data); > > + /* Allow the TX to drain before we reconfigure */ > + if (offset == UART_LCR && d->drain_before_lcr_change) > + dw8250_tx_wait_empty(p); > + > writeb(value, p->membase + (offset << p->regshift)); > > if (offset == UART_LCR && !d->uart_16550_compatible) > @@ -197,6 +201,10 @@ static void dw8250_serial_outq(struct uart_port *p, int offset, int value) > { > struct dw8250_data *d = to_dw8250_data(p->private_data); > > + /* Allow the TX to drain before we reconfigure */ > + if (offset == UART_LCR && d->drain_before_lcr_change) > + dw8250_tx_wait_empty(p); > + > value &= 0xff; > __raw_writeq(value, p->membase + (offset << p->regshift)); > /* Read back to ensure register write ordering. */ > @@ -211,6 +219,10 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value) > { > struct dw8250_data *d = to_dw8250_data(p->private_data); > > + /* Allow the TX to drain before we reconfigure */ > + if (offset == UART_LCR && d->drain_before_lcr_change) > + dw8250_tx_wait_empty(p); > + > writel(value, p->membase + (offset << p->regshift)); > > if (offset == UART_LCR && !d->uart_16550_compatible) > @@ -228,6 +240,10 @@ static void dw8250_serial_out32be(struct uart_port *p, int offset, int value) > { > struct dw8250_data *d = to_dw8250_data(p->private_data); > > + /* Allow the TX to drain before we reconfigure */ > + if (offset == UART_LCR && d->drain_before_lcr_change) > + dw8250_tx_wait_empty(p); > + > iowrite32be(value, p->membase + (offset << p->regshift)); > > if (offset == UART_LCR && !d->uart_16550_compatible) > @@ -597,6 +613,8 @@ static int dw8250_probe(struct platform_device *pdev) > /* Always ask for fixed clock rate from a property. */ > device_property_read_u32(dev, "clock-frequency", &p->uartclk); > > + data->drain_before_lcr_change = device_property_read_bool(dev, "drain-before-lcr-change"); > + > /* If there is separate baudclk, get the rate from it. */ > data->clk = devm_clk_get_optional(dev, "baudclk"); > if (data->clk == NULL) > diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h > index f13e91f2cace..f7d88fa8f058 100644 > --- a/drivers/tty/serial/8250/8250_dwlib.h > +++ b/drivers/tty/serial/8250/8250_dwlib.h > @@ -45,6 +45,7 @@ struct dw8250_data { > > unsigned int skip_autocfg:1; > unsigned int uart_16550_compatible:1; > + unsigned int drain_before_lcr_change:1; > }; > > void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, const struct ktermios *old);