Skip to contentSkip to conversation

serial: 8250_bcm7271: UART errors after resuming from S2

PATCH In review Al Cooper 2 participants 2 messages Dec 1, 2021

Subsystems (2)
TTY layer and serial drivers Broadcom brcmstb UART driver
lore
Share & tools
lore

Al Cooper

author maintainer
#0 lore raw

PATCH 1/1 serial: 8250_bcm7271: UART errors after resuming from S2

There is a small window in time during resume where the hardware flow control signal RTS can be asserted (which allows a sender to resume sending data to the UART) but the baud rate has not yet been restored. This will cause corrupted data and FRAMING, OVERRUN and BREAK errors. This is happening because the MCTRL register is shadowed in uart_port struct and is later used during resume to set the MCTRL register during both serial8250_do_startup() and uart_resume_port(). Unfortunately, serial8250_do_startup() happens before the UART baud rate is restored. The fix is to clear the shadowed mctrl value at the end of suspend and restore it at the end of resume.

Fixes 41a469482de2 ("serial: 8250: Add new 8250-core based Broadcom STB driver")
Signed-off-by Al Cooper <[email protected]>
---
 drivers/tty/serial/8250/8250_bcm7271.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
drivers/tty/serial/8250/8250_bcm7271.c +13 -0
index 7f656fac503f..5163d60756b7 100644
@@ -237,6 +237,7 @@ struct brcmuart_priv {
 	u32		rx_err;
 	u32		rx_timeout;
 	u32		rx_abort;
+	u32		saved_mctrl;
 };
 
 static struct dentry *brcmuart_debugfs_root;

@@ -1133,16 +1134,27 @@ static int brcmuart_remove(struct platform_device *pdev)
 static int __maybe_unused brcmuart_suspend(struct device *dev)
 {
 	struct brcmuart_priv *priv = dev_get_drvdata(dev);
+	struct uart_8250_port *up = serial8250_get_port(priv->line);
+	struct uart_port *port = &up->port;
 
 	serial8250_suspend_port(priv->line);
 	clk_disable_unprepare(priv->baud_mux_clk);
 
+	/*
+	 * This will prevent resume from enabling RTS before the
+	 *  baud rate has been resored.
+	 */
+	priv->saved_mctrl = port->mctrl;
+	port->mctrl = 0;
+
 	return 0;
 }
 
 static int __maybe_unused brcmuart_resume(struct device *dev)
 {
 	struct brcmuart_priv *priv = dev_get_drvdata(dev);
+	struct uart_8250_port *up = serial8250_get_port(priv->line);
+	struct uart_port *port = &up->port;
 	int ret;
 
 	ret = clk_prepare_enable(priv->baud_mux_clk);

@@ -1165,6 +1177,7 @@ static int __maybe_unused brcmuart_resume(struct device *dev)
 		start_rx_dma(serial8250_get_port(priv->line));
 	}
 	serial8250_resume_port(priv->line);
+	port->mctrl = priv->saved_mctrl;
 	return 0;
 }

base-commit: 58e1100fdc5990b0cc0d4beaf2562a92e621ac7d

Signature · 1 line
2.17.1
Acked-by on v1 1/1
#1 lore raw
Quoted 12 lines · Al Cooper

There is a small window in time during resume where the hardware flow control signal RTS can be asserted (which allows a sender to resume sending data to the UART) but the baud rate has not yet been restored. This will cause corrupted data and FRAMING, OVERRUN and BREAK errors. This is happening because the MCTRL register is shadowed in uart_port struct and is later used during resume to set the MCTRL register during both serial8250_do_startup() and uart_resume_port(). Unfortunately, serial8250_do_startup() happens before the UART baud rate is restored. The fix is to clear the shadowed mctrl value at the end of suspend and restore it at the end of resume.

Quoted 2 lines
Fixes 41a469482de2 ("serial: 8250: Add new 8250-core based Broadcom STB driver")
Signed-off-by Al Cooper <[email protected]>
Acked-by Florian Fainelli <[email protected]>
Signature · 1 line
Florian