From patchwork Wed Sep 23 16:19:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Serge Semin X-Patchwork-Id: 11795231 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 98C9792C for ; Wed, 23 Sep 2020 16:20:35 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 5ED412065E for ; Wed, 23 Sep 2020 16:20:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="0VmbXgsP" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5ED412065E 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=merlin.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=1dWuWh+Kcel8UQE+MdBdTI2nShpidAKcvaY0gcxeabM=; b=0VmbXgsPJB8WlO3/hoF3SYRV5 Akvpjsp0H2/SmTWa4knxacB4KmMn9aMwydvhlwIGHfvZ23xMiALVKLXrFEnU1hVQsaQfhH2cBj18n 5e1Caq2kNFIJNnVGh1tpFuTDQOD6G9LN5hPsQHyqTZNwpj1/JkClHgPliUqVxMBm3iMtMcpqtH5Mk QJovauZN51LE9AHy1E9atjZ+gkofESvVIueUnZTJtOEgtF7PWPG+bHADBm6iPnP7a2dlMjZRagfSE EbZNneHCtEYgA/BzpAEKd3HxUynuTu5oz9hz2GUPY8/0gcI3BpBR8rbRMFhExMe1ckvu6CxMXC0g1 D6Z5fZHwg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kL7VG-0003Kb-3z; Wed, 23 Sep 2020 16:20:22 +0000 Received: from mail.baikalelectronics.com ([87.245.175.226] helo=mail.baikalelectronics.ru) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kL7Ux-0003EK-Ii for linux-arm-kernel@lists.infradead.org; Wed, 23 Sep 2020 16:20:06 +0000 Received: from localhost (unknown [127.0.0.1]) by mail.baikalelectronics.ru (Postfix) with ESMTP id C0796803086A; Wed, 23 Sep 2020 16:19:57 +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 M6bFa9JfLHjy; Wed, 23 Sep 2020 19:19:55 +0300 (MSK) From: Serge Semin To: Greg Kroah-Hartman , Jiri Slaby , Andy Shevchenko Subject: [PATCH 3/3] serial: 8250_dw: Fix clk-notifier/port suspend deadlock Date: Wed, 23 Sep 2020 19:19:50 +0300 Message-ID: <20200923161950.6237-4-Sergey.Semin@baikalelectronics.ru> In-Reply-To: <20200923161950.6237-1-Sergey.Semin@baikalelectronics.ru> References: <20200923161950.6237-1-Sergey.Semin@baikalelectronics.ru> 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-20200923_122003_923509_DEC13766 X-CRM114-Status: GOOD ( 15.40 ) X-Spam-Score: 0.0 (/) X-Spam-Report: SpamAssassin version 3.4.4 on merlin.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: Maxime Ripard , linux-kernel@vger.kernel.org, Russell King , Serge Semin , Alexey Malahov , Serge Semin , Hans de Goede , Andy Shevchenko , Pavel Parkhomenko , linux-serial@vger.kernel.org, Will Deacon , linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org It has been discovered that there is a potential deadlock between the clock-change-notifier thread and the UART port suspending one: CPU0 (suspend CPU/UART) CPU1 (update clock) ---- ---- lock(&port->mutex); lock((work_completion)(&data->clk_work)); lock(&port->mutex); lock((work_completion)(&data->clk_work)); *** DEADLOCK *** The best way to fix this is to eliminate the CPU0 port->mutex/work-completion scenario. So we suggest to register and unregister the clock-notifier during the DW APB UART port probe/remove procedures, instead of doing that at the points of the port startup/shutdown. Link: https://lore.kernel.org/linux-serial/f1cd5c75-9cda-6896-a4e2-42c5bfc3f5c3@redhat.com Fixes: cc816969d7b5 ("serial: 8250_dw: Fix common clocks usage race condition") Reported-by: Hans de Goede Signed-off-by: Serge Semin Tested-by: Jonathan Liu --- drivers/tty/serial/8250/8250_dw.c | 54 +++++++++++-------------------- 1 file changed, 19 insertions(+), 35 deletions(-) diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index 87f450b7c177..9e204f9b799a 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -373,39 +373,6 @@ 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); - int ret; - - /* - * 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 that any clock state change is known to the UART port at - * least post factum. - */ - if (d->clk) { - ret = clk_notifier_register(d->clk, &d->clk_notifier); - if (ret) - dev_warn(p->dev, "Failed to set the clock notifier\n"); - } - - 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); - - if (d->clk) { - clk_notifier_unregister(d->clk, &d->clk_notifier); - - flush_work(&d->clk_work); - } -} - /* * 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 @@ -501,8 +468,6 @@ 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) @@ -622,6 +587,19 @@ static int dw8250_probe(struct platform_device *pdev) goto err_reset; } + /* + * Some platforms may provide a reference clock shared between several + * devices. In this case any clock state change must be known to the + * UART port at least post factum. + */ + if (data->clk) { + err = clk_notifier_register(data->clk, &data->clk_notifier); + if (err) + dev_warn(p->dev, "Failed to set the clock notifier\n"); + else + queue_work(system_unbound_wq, &data->clk_work); + } + platform_set_drvdata(pdev, data); pm_runtime_set_active(dev); @@ -648,6 +626,12 @@ static int dw8250_remove(struct platform_device *pdev) pm_runtime_get_sync(dev); + if (data->clk) { + clk_notifier_unregister(data->clk, &data->clk_notifier); + + flush_work(&data->clk_work); + } + serial8250_unregister_port(data->data.line); reset_control_assert(data->rst);