public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Lukas Wunner <lukas@wunner.de>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	Su Bao Cheng <baocheng.su@siemens.com>,
	Matthias Schiffer <matthias.schiffer@ew.tq-group.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Sasha Levin <sashal@kernel.org>,
	jirislaby@kernel.org, ilpo.jarvinen@linux.intel.com,
	tony@atomide.com, andriy.shevchenko@linux.intel.com,
	linux-serial@vger.kernel.org
Subject: [PATCH AUTOSEL 5.4 04/15] serial: 8250: 8250_omap: Avoid RS485 RTS glitch on ->set_termios()
Date: Wed, 23 Nov 2022 07:44:14 -0500	[thread overview]
Message-ID: <20221123124427.266286-4-sashal@kernel.org> (raw)
In-Reply-To: <20221123124427.266286-1-sashal@kernel.org>

From: Lukas Wunner <lukas@wunner.de>

[ Upstream commit 038ee49fef18710bedd38b531d173ccd746b2d8d ]

RS485-enabled UART ports on TI Sitara SoCs with active-low polarity
exhibit a Transmit Enable glitch on ->set_termios():

omap8250_restore_regs(), which is called from omap_8250_set_termios(),
sets the TCRTLR bit in the MCR register and clears all other bits,
including RTS.  If RTS uses active-low polarity, it is now asserted
for no reason.

The TCRTLR bit is subsequently cleared by writing up->mcr to the MCR
register.  That variable is always zero, so the RTS bit is still cleared
(incorrectly so if RTS is active-high).

(up->mcr is not, as one might think, a cache of the MCR register's
current value.  Rather, it only caches a single bit of that register,
the AFE bit.  And it only does so if the UART supports the AFE bit,
which OMAP does not.  For details see serial8250_do_set_termios() and
serial8250_do_set_mctrl().)

Finally at the end of omap8250_restore_regs(), the MCR register is
restored (and RTS deasserted) by a call to up->port.ops->set_mctrl()
(which equals serial8250_set_mctrl()) and serial8250_em485_stop_tx().

So there's an RTS glitch between setting TCRTLR and calling
serial8250_em485_stop_tx().  Avoid by using a read-modify-write
when setting TCRTLR.

While at it, drop a redundant initialization of up->mcr.  As explained
above, the variable isn't used by the driver and it is already
initialized to zero because it is part of the static struct
serial8250_ports[] declared in 8250_core.c.  (Static structs are
initialized to zero per section 6.7.8 nr. 10 of the C99 standard.)

Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Su Bao Cheng <baocheng.su@siemens.com>
Tested-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Link: https://lore.kernel.org/r/6554b0241a2c7fd50f32576fdbafed96709e11e8.1664278942.git.lukas@wunner.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/tty/serial/8250/8250_omap.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index efe793a2fc65..f2a0ae49b143 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -268,6 +268,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
 {
 	struct omap8250_priv *priv = up->port.private_data;
 	struct uart_8250_dma	*dma = up->dma;
+	u8 mcr = serial8250_in_MCR(up);
 
 	if (dma && dma->tx_running) {
 		/*
@@ -284,7 +285,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
 	serial_out(up, UART_EFR, UART_EFR_ECB);
 
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
-	serial8250_out_MCR(up, UART_MCR_TCRTLR);
+	serial8250_out_MCR(up, mcr | UART_MCR_TCRTLR);
 	serial_out(up, UART_FCR, up->fcr);
 
 	omap8250_update_scr(up, priv);
@@ -300,7 +301,8 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
 	serial_out(up, UART_LCR, 0);
 
 	/* drop TCR + TLR access, we setup XON/XOFF later */
-	serial8250_out_MCR(up, up->mcr);
+	serial8250_out_MCR(up, mcr);
+
 	serial_out(up, UART_IER, up->ier);
 
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
@@ -611,7 +613,6 @@ static int omap_8250_startup(struct uart_port *port)
 
 	pm_runtime_get_sync(port->dev);
 
-	up->mcr = 0;
 	serial_out(up, UART_FCR, UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
 
 	serial_out(up, UART_LCR, UART_LCR_WLEN8);
-- 
2.35.1


  parent reply	other threads:[~2022-11-23 12:54 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-23 12:44 [PATCH AUTOSEL 5.4 01/15] Input: synaptics - switch touchpad on HP Laptop 15-da3001TU to RMI mode Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 02/15] ASoC: Intel: bytcht_es8316: Add quirk for the Nanote UMPC-01 Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 03/15] ASoC: Intel: soc-acpi: add ES83x6 support to IceLake Sasha Levin
2022-11-23 12:44 ` Sasha Levin [this message]
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 05/15] Input: goodix - try resetting the controller when no config is set Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 06/15] xen/platform-pci: add missing free_irq() in error path Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 07/15] platform/x86: asus-wmi: add missing pci_dev_put() in asus_wmi_set_xusb2pr() Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 08/15] platform/x86: acer-wmi: Enable SW_TABLET_MODE on Switch V 10 (SW5-017) Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 09/15] platform/x86: hp-wmi: Ignore Smart Experience App event Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 10/15] tcp: configurable source port perturb table size Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 11/15] block: make blk_set_default_limits() private Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 12/15] dm-integrity: set dma_alignment limit in io_hints Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 13/15] dm-log-writes: " Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 14/15] net: usb: qmi_wwan: add Telit 0x103a composition Sasha Levin
2022-11-23 12:44 ` [PATCH AUTOSEL 5.4 15/15] dm integrity: flush the journal on suspend Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221123124427.266286-4-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=baocheng.su@siemens.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=matthias.schiffer@ew.tq-group.com \
    --cc=stable@vger.kernel.org \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox