Date:   Mon, 23 Mar 2020 05:46:09 +0300
From:   <Sergey.Semin@...kalelectronics.ru>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>
CC:     Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        Serge Semin <fancer.lancer@...il.com>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Maxim Kaurkin <Maxim.Kaurkin@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        Ramil Zaripov <Ramil.Zaripov@...kalelectronics.ru>,
        Ekaterina Skachko <Ekaterina.Skachko@...kalelectronics.ru>,
        Vadim Vlasov <V.Vlasov@...kalelectronics.ru>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Paul Burton <paulburton@...nel.org>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Maxime Ripard <mripard@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>, Ray Jui <rjui@...adcom.com>,
        Scott Branden <sbranden@...adcom.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Wei Xu <xuwei5@...ilicon.com>,
        Jason Cooper <jason@...edaemon.net>,
        Andrew Lunn <andrew@...n.ch>,
        Gregory Clement <gregory.clement@...tlin.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Jisheng Zhang <Jisheng.Zhang@...aptics.com>,
        Heiko Stuebner <heiko@...ech.de>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        <linux-arm-kernel@...ts.infradead.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, <linux-clk@...r.kernel.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Kefeng Wang <wangkefeng.wang@...wei.com>,
        <linux-serial@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: [PATCH v2] serial: 8250_dw: Fix common clocks usage race condition

From: Serge Semin <Sergey.Semin@...kalelectronics.ru>

There are races possible in the dw8250_set_termios() callback method
and while the device is in PM suspend state. A race condition may
happen if the baudrate clock source device is shared with some other
device (in our machine it's another DW UART port). In this case if that
device changes the clock rate while serial console is using it the
DW 8250 UART port might not only end up with an invalid uartclk value
saved, but may also experience a distorted output data since baud-clock
could have been changed. In order to fix this lets enable an exclusive
reference clock rate access in case if "baudclk" device is specified.

So if some other device also acquires the rate exclusivity during the
time of a DW UART 8250 port being opened, then DW UART 8250 driver
won't be able to alter the baud-clock. It shall just use the available
clock rate. Similarly another device also won't manage to change the
rate at that time. If nothing else have the exclusive rate access
acquired except DW UART 8250 driver, then the driver will be able to
alter the rate as much as it needs to in accordance with the currently
implemented logic.

Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@...kalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@...kalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@...kalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@...kalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>
Cc: Paul Burton <paulburton@...nel.org>
Cc: Ralf Baechle <ralf@...ux-mips.org>
Cc: Maxime Ripard <mripard@...nel.org>
Cc: Chen-Yu Tsai <wens@...e.org>
CC: Ray Jui <rjui@...adcom.com>
Cc: Scott Branden <sbranden@...adcom.com>
Cc: Florian Fainelli <f.fainelli@...il.com>
Cc: Wei Xu <xuwei5@...ilicon.com>
Cc: Jason Cooper <jason@...edaemon.net>
Cc: Andrew Lunn <andrew@...n.ch>
Cc: Gregory Clement <gregory.clement@...tlin.com>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Cc: Jisheng Zhang <Jisheng.Zhang@...aptics.com>
Cc: Heiko Stuebner <heiko@...ech.de>
Cc: Catalin Marinas <catalin.marinas@....com>
Cc: Will Deacon <will@...nel.org>
Cc: Russell King <linux@...linux.org.uk>
Cc: linux-arm-kernel@...ts.infradead.org
Cc: Michael Turquette <mturquette@...libre.com>
Cc: Stephen Boyd <sboyd@...nel.org>
Cc: linux-clk@...r.kernel.org

---

Changelog v2:
- Move exclusive ref clock lock/unlock precudures to the 8250 port
  startup/shutdown methods.
- The changelog message has also been slightly modified due to the
  alteration.
- Remove Alexey' SoB tag.
- Cc someone from ARM who might be concerned regarding this change.
- Cc someone from Clocks Framework to get their comments on this patch.
---
 drivers/tty/serial/8250/8250_dw.c | 36 +++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index aab3cccc6789..08f3f745ed54 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -319,6 +319,40 @@ static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
 	serial8250_do_set_ldisc(p, termios);
 }
 
+static int dw8250_startup(struct uart_port *p)
+{
+	struct dw8250_data *d = to_dw8250_data(p->private_data);
+
+	/*
+	 * Some platforms may provide a reference clock shared between several
+	 * devices. In this case before using the serial port first we have to
+	 * make sure nothing will change the rate behind our back and second
+	 * the tty/serial subsystem knows the actual reference clock rate of
+	 * the port.
+	 */
+	if (clk_rate_exclusive_get(d->clk)) {
+		dev_warn(p->dev, "Couldn't lock the clock rate\n");
+	} else if (d->clk) {
+		p->uartclk = clk_get_rate(d->clk);
+		if (!p->uartclk) {
+			clk_rate_exclusive_put(d->clk);
+			dev_err(p->dev, "Clock rate not defined\n");
+			return -EINVAL;
+		}
+	}
+
+	return serial8250_do_startup(p);
+}
+
+static void dw8250_shutdown(struct uart_port *p)
+{
+	struct dw8250_data *d = to_dw8250_data(p->private_data);
+
+	serial8250_do_shutdown(p);
+
+	clk_rate_exclusive_put(d->clk);
+}
+
 /*
  * dw8250_fallback_dma_filter will prevent the UART from getting just any free
  * channel on platforms that have DMA engines, but don't have any channels
@@ -414,6 +448,8 @@ static int dw8250_probe(struct platform_device *pdev)
 	p->serial_out	= dw8250_serial_out;
 	p->set_ldisc	= dw8250_set_ldisc;
 	p->set_termios	= dw8250_set_termios;
+	p->startup	= dw8250_startup;
+	p->shutdown	= dw8250_shutdown;
 
 	p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
 	if (!p->membase)
-- 
2.25.1

Powered by blists - more mailing lists