From patchwork Mon Jul 13 07:32:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Helmut Grohne X-Patchwork-Id: 11658943 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 9E3E0138C for ; Mon, 13 Jul 2020 07:33:48 +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 7747E20702 for ; Mon, 13 Jul 2020 07:33:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="2DkRfNOy"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=intenta.de header.i=@intenta.de header.b="O2mUO1JG" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7747E20702 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intenta.de 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:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=w8+G856/Ov1qrQnHoxTzfNECV6jEpHvvVh+prl90l+Q=; b=2DkRfNOyGevyX6rdS8ggbis4E COviR24Ij7e5cRMNVOXoJ2i5y24pyagl5Ar6NGQMXYm0tDmS0N6b5p5Vj6IaiwEKL/MiRdduNjhpu 9STD6LpquLmUX/3ZhBqS5cdJh4K5zk/H/L4zakg84q04lrpuwcXp7G6ISL14JD5UJcvo8AUFoT17T hz9htMpF1Mx5CNqUsissTi7yDN4JLv+z1ylWjDhkVHLEJrVn7AbWt8e+tj+9T+SNiMRczQFfOm+w8 iw+RUlX5jElVrX9gxFrRt6RhmtcEnDyK5doTR5X/w1f7ClsKxiaFbOTyUKLRziWZQLtFLtZCFKjXS tqeukGJ9A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jusx1-0005dL-RH; Mon, 13 Jul 2020 07:32:35 +0000 Received: from mail.intenta.de ([178.249.25.132]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1juswz-0005cM-2D for linux-arm-kernel@lists.infradead.org; Mon, 13 Jul 2020 07:32:34 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=intenta.de; s=dkim1; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:CC:To:From:Date; bh=FSVKKh1K3meJ2NLE68XU6nTptr/hNpfqhkvhx31taXA=; b=O2mUO1JGdL4/AbtHEfauBDNtzEgysjWjoTAEjYR9au4rNCkW7R8lB9Q/eObE95p1qFy4Nvlt11xS//A49bE25U9v84DKexywB7ZALe8rW42MRY12IDYLSEtw2vN7OS+r5aJ41Rqo71NGjJCSd0R7XZo1mFAHRhi8ELY94jbhH6Dsyn9YFruncKOs1dKOqr33BCNQSCPNfJkM79OcUyQrpJe+KVmQIpokuYL5LbEOTIialSpFkjoJUA/tZotFzzeMIOo0Nn2c/oMM+41E7Kbe28JguRa41a56o0R9jm9ErYslM66EpjkJTAXfDJHr0XKryqZvQpYDt0LOdPo5CImSUg==; Date: Mon, 13 Jul 2020 09:32:28 +0200 From: Helmut Grohne To: Greg Kroah-Hartman , Jiri Slaby , Michal Simek , Shubhrajyoti Datta , Jan Kiszka Subject: [PATCH v2] tty: xilinx_uartps: Really fix id assignment Message-ID: <20200713073227.GA3805@laureti-dev> References: <20200710124535.GA1584780@kroah.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200710124535.GA1584780@kroah.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-ClientProxiedBy: ICSMA002.intenta.de (10.10.16.48) To ICSMA002.intenta.de (10.10.16.48) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200713_033233_259553_588F608C X-CRM114-Status: GOOD ( 18.82 ) X-Spam-Score: -2.5 (--) X-Spam-Report: SpamAssassin version 3.4.4 on merlin.infradead.org summary: Content analysis details: (-2.5 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at https://www.dnswl.org/, medium trust [178.249.25.132 listed in list.dnswl.org] -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 SPF_PASS SPF: sender matches SPF record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 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: linux-arm-kernel@lists.infradead.org, linux-serial@vger.kernel.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org The problems started with the revert (18cc7ac8a28e28). The cdns_uart_console.index is statically assigned -1. When the port is registered, Linux assigns consecutive numbers to it. It turned out that when using ttyPS1 as console, the index is not updated as we are reusing the same cdns_uart_console instance for multiple ports. When registering ttyPS0, it gets updated from -1 to 0, but when registering ttyPS1, it already is 0 and not updated. That led to 2ae11c46d5fdc4. It assigns the index prior to registering the uart_driver once. Unfortunately, that ended up breaking the situation where the probe order does not match the id order. When using the same device tree for both uboot and linux, it is important that the serial0 alias points to the console. So some boards reverse those aliases. This was reported by Jan Kiszka. The proposed fix was reverting the index assignment and going back to the previous iteration. However such a reversed assignement (serial0 -> uart1, serial1 -> uart0) was already partially broken by the revert (18cc7ac8a28e28). While the ttyPS device works, the kmsg connection is already broken and kernel messages go missing. Reverting the id assignment does not fix this. From the xilinx_uartps driver pov (after reverting the refactoring commits), there can be only one console. This manifests in static variables console_pprt and cdns_uart_console. These variables are not properly linked and can go out of sync. The cdns_uart_console.index is important for uart_add_one_port. We call that function for each port - one of which hopefully is the console. If it isn't, the CON_ENABLED flag is not set and console_port is cleared. The next cdns_uart_probe call then tries to register the next port using that same cdns_uart_console. It is important that console_port and cdns_uart_console (and its index in particular) stay in sync. The index assignment implemented by Shubhrajyoti Datta is correct in principle. It just may have to happen a second time if the first cdns_uart_probe call didn't encounter the console device. And we shouldn't change the index once the console uart is registered. Reported-by: Shubhrajyoti Datta Reported-by: Jan Kiszka Link: https://lore.kernel.org/linux-serial/f4092727-d8f5-5f91-2c9f-76643aace993@siemens.com/ Fixes: 18cc7ac8a28e28 ("Revert "serial: uartps: Register own uart console and driver structures"") Fixes: 2ae11c46d5fdc4 ("tty: xilinx_uartps: Fix missing id assignment to the console") Fixes: 76ed2e10579671 ("Revert "tty: xilinx_uartps: Fix missing id assignment to the console"") Signed-off-by: Helmut Grohne --- drivers/tty/serial/xilinx_uartps.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) Changes since v1: * This patch was meant to replace 76ed2e10579671. Since it is part of gregkh/tty-linus already, rebase this patch onto it. diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index 672cfa075e28..2833f1418d6d 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -1580,8 +1580,10 @@ static int cdns_uart_probe(struct platform_device *pdev) * If register_console() don't assign value, then console_port pointer * is cleanup. */ - if (!console_port) + if (!console_port) { + cdns_uart_console.index = id; console_port = port; + } #endif rc = uart_add_one_port(&cdns_uart_uart_driver, port); @@ -1594,8 +1596,10 @@ static int cdns_uart_probe(struct platform_device *pdev) #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE /* This is not port which is used for console that's why clean it up */ if (console_port == port && - !(cdns_uart_uart_driver.cons->flags & CON_ENABLED)) + !(cdns_uart_uart_driver.cons->flags & CON_ENABLED)) { console_port = NULL; + cdns_uart_console.index = -1; + } #endif cdns_uart_data->cts_override = of_property_read_bool(pdev->dev.of_node,