[PATCH] serial: 8250_bcm7271: UART errors after resuming from S2

[PATCH] serial: 8250_bcm7271: UART errors after resuming from S2

[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.

Fixes: 41a469482de2 ("serial: 8250: Add new 8250-core based Broadcom STB driver")
Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
 drivers/tty/serial/8250/8250_bcm7271.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c
index 7f656fac503f..5163d60756b7 100644
--- a/drivers/tty/serial/8250/8250_bcm7271.c
+++ b/drivers/tty/serial/8250/8250_bcm7271.c
@@ -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
-- 
2.17.1

Re: [PATCH] serial: 8250_bcm7271: UART errors after resuming from S2

[Florian Fainelli]

On 12/1/21 12:14 PM, Al Cooper wrote:
> 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 <alcooperx@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian