From patchwork Mon Mar 23 02:46:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Serge Semin X-Patchwork-Id: 11452211 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 915616CA for ; Mon, 23 Mar 2020 02:53:58 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6F71420722 for ; Mon, 23 Mar 2020 02:53:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="BzPUZfpg" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6F71420722 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=baikalelectronics.ru Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=mr2Pu2BoigEPRHBAQ1yjIQW38eKObMNKt8aHe0sHBMA=; b=BzPUZfpg19PTCQ xXP4eqt8d4KNRBAksMcESEv9iUlJzPDxE3tptWLTBsthxFUJ5+EU8qRyHsiCKI7j16x6FVzxwM8oP M85g2bLVOJ9cDTAbLaIh7Cs2Vod74zdYuIHcXSmEw/slAY9K+KB8SGMW+wbx+Fb4PDTbiN5sEyYd3 vkJTmPoeYdtLQ6j3F/segPDN+spae2G6llKCwzHlFIS9TukZrBPGzx3QYbffs/sLDQut6ZVbeI63p 5Z6FYAD+8CPo49uveb59yNkqIcD8TIcXK9Z60EVg62Lb+OeCaM3WFyLBzlN5o0+AMgwa0R6Dyj2ke R8hkjvg1tvnme7io03Cg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jGDDv-0003Ay-28; Mon, 23 Mar 2020 02:53:55 +0000 Received: from mail.baikalelectronics.com ([87.245.175.226] helo=mail.baikalelectronics.ru) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jGDDr-0003AJ-Lk for linux-arm-kernel@lists.infradead.org; Mon, 23 Mar 2020 02:53:54 +0000 Received: from localhost (unknown [127.0.0.1]) by mail.baikalelectronics.ru (Postfix) with ESMTP id CB41180307CB; Mon, 23 Mar 2020 02:46:47 +0000 (UTC) X-Virus-Scanned: amavisd-new at baikalelectronics.ru Received: from mail.baikalelectronics.ru ([127.0.0.1]) by localhost (mail.baikalelectronics.ru [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UTjW9bYZDdnY; Mon, 23 Mar 2020 05:46:46 +0300 (MSK) From: To: Andy Shevchenko , Greg Kroah-Hartman , Jiri Slaby Subject: [PATCH v2] serial: 8250_dw: Fix common clocks usage race condition Date: Mon, 23 Mar 2020 05:46:09 +0300 Message-ID: <20200323024611.16039-1-Sergey.Semin@baikalelectronics.ru> In-Reply-To: <20200306130231.05BBC8030795@mail.baikalelectronics.ru> References: MIME-Version: 1.0 X-ClientProxiedBy: MAIL.baikal.int (192.168.51.25) To mail (192.168.51.25) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200322_195352_079752_4F707002 X-CRM114-Status: GOOD ( 18.53 ) X-Spam-Score: 0.0 (/) X-Spam-Report: SpamAssassin version 3.4.3 on bombadil.infradead.org summary: Content analysis details: (0.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrew Lunn , Kefeng Wang , Heikki Krogerus , Heiko Stuebner , Catalin Marinas , Michael Turquette , Serge Semin , Will Deacon , linux-clk@vger.kernel.org, Florian Fainelli , Maxim Kaurkin , Ramil Zaripov , Gregory Clement , Russell King , Wei Xu , Chen-Yu Tsai , Ekaterina Skachko , Sebastian Hesselbarth , Jason Cooper , Ray Jui , Maxime Ripard , Alexey Malahov , linux-serial@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Thomas Bogendoerfer , Vadim Vlasov , Paul Burton , Scott Branden , Stephen Boyd , linux-kernel@vger.kernel.org, Ralf Baechle , Serge Semin , Jisheng Zhang , Pavel Parkhomenko Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org From: Serge Semin 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 Cc: Alexey Malahov Cc: Maxim Kaurkin Cc: Pavel Parkhomenko Cc: Ramil Zaripov Cc: Ekaterina Skachko Cc: Vadim Vlasov Cc: Thomas Bogendoerfer Cc: Paul Burton Cc: Ralf Baechle Cc: Maxime Ripard Cc: Chen-Yu Tsai CC: Ray Jui Cc: Scott Branden Cc: Florian Fainelli Cc: Wei Xu Cc: Jason Cooper Cc: Andrew Lunn Cc: Gregory Clement Cc: Sebastian Hesselbarth Cc: Jisheng Zhang Cc: Heiko Stuebner Cc: Catalin Marinas Cc: Will Deacon Cc: Russell King Cc: linux-arm-kernel@lists.infradead.org Cc: Michael Turquette Cc: Stephen Boyd Cc: linux-clk@vger.kernel.org Signed-off-by: Serge Semin --- 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)