From patchwork Wed Feb 1 14:26:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergey Organov X-Patchwork-Id: 13124457 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id B2575C636CD for ; Wed, 1 Feb 2023 14:41:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=A7/D9vQ3E0iEvfjbPtp6IFD+Gx5xQaUuLqeOPitXB1w=; b=TUTl5IGyhYhC9I eKn962xIyXQKBx9ITTU6mOjgYgWjMmRxYusUg+Ls/zku80EdR7og/WYqzLf20mIrXE3rRDiNHASQu opBf3e+rQH4LjEz+Y/YhmuB9/nzHpFcBmoVcW/4AwUpV1vFg7Of52ij/zkUVf+f7wnw0+H8vagkaX 2GjTiATkN5iGKvXQ4btUWY9OAmHANfY6UmLzNTsl8ZWHNqMWw1r2JZNTjkw/fj6/+wJZeGFVzMIXF sUr70hqk+kG9p4Z889z1Jbh7v4GFFuTSeDBWAMbNFKZi3ME/GPtTc9+MWjQg/Hs0+6UwRN4QHMuJR aRAiRuv79MZEH99S28Wg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pNEHL-00CR5n-Ay; Wed, 01 Feb 2023 14:40:03 +0000 Received: from mail-ed1-x532.google.com ([2a00:1450:4864:20::532]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pNE5Q-00CLm3-VK for linux-arm-kernel@lists.infradead.org; Wed, 01 Feb 2023 14:27:47 +0000 Received: by mail-ed1-x532.google.com with SMTP id z11so17876702ede.1 for ; Wed, 01 Feb 2023 06:27:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=vDEp4eKXdg/3wssI2Ou1QYWdbAeeL+58ez61t5CUM9E=; b=K1xsYdRkbxOW0gYgS8BKK9rscyZO9LYOWHC1AclbwYuUnU5VVnQNYiP9s8Qzv49dOt VZKhDGe92Pgad9UXE9+mTsO4tFSSB5C97yrnjv3d1BuEjq3dL8UZl4gRcVM/iNvjcmpe MaXYtmd1ceEMrb8Kl04RVgJ091/GwGf7bnMioZQx22Di8b9Zi65t1du7Vg3PpycUkoIb JUl+bHqsDwjTnsrAO7atQWMTon4HOkPOgshVxxRULQ3NaI4tHoVspB1SIjHSCUsfMgnV ou2nC65ChWg/s4wQWu1rqgcg/zp51B9RoQWyi51onq6BkTm0Ac8Znv6WVPX1WKi5uogY cQIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vDEp4eKXdg/3wssI2Ou1QYWdbAeeL+58ez61t5CUM9E=; b=qh4XyVEKVQAsMjWgG6YzMhLLkVaL5iTI8AJ5h87vnS0tf2EdqbvMrpQktctFUQht6d InbGVYfvGEzbusmN+Sci+geqKV/WgbQIAjqOsJW5JB1wR2wkOdAImW1XbGw8s4Z6ad5I SP5IvqVuxUtyBHRXm/+fAVUNrJZdfv5RWnRKN9sfl9EAk+On9BIzmITDiWRlOHoRr4AY MtBzXDh2hQUkj5mJXVADctZ0aVMw4E8CiJALUVlOD9DuLQnXmaynLSuxP24CgiDsiEZV HyvtXm1fOxeNoq5bHGT83SiNfSmaUeNRRMHajg6g34BNA+ZhmZ4uIThtWuEbxA1P6BWi A9vA== X-Gm-Message-State: AO0yUKWRpyJfzKVFdZ+bCpMgZVoG/IVJfE69aAF75ApshpOlsXIsmDO1 nPXKfVXcadMloHPjlEABG2s= X-Google-Smtp-Source: AK7set8MRay/4KmKUvccvdPoG83dTuDwt3q3ZbIm1GAwiPqy4nUgcodj++ljfSdIsLXuBfv8zZa4QQ== X-Received: by 2002:a05:6402:1950:b0:4a2:260f:3bbe with SMTP id f16-20020a056402195000b004a2260f3bbemr2606430edz.23.1675261660934; Wed, 01 Feb 2023 06:27:40 -0800 (PST) Received: from osv.localdomain ([89.175.180.246]) by smtp.gmail.com with ESMTPSA id cf8-20020a0564020b8800b004a18f2ffb86sm8975564edb.79.2023.02.01.06.27.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Feb 2023 06:27:40 -0800 (PST) From: Sergey Organov To: Greg Kroah-Hartman Cc: linux-serial@vger.kernel.org, Fabio Estevam , Jiri Slaby , Richard Genoud , Sascha Hauer , Shawn Guo , Tim Harvey , =?utf-8?q?Tomasz_Mo=C5=84?= , =?utf-8?q?Uwe_Kle?= =?utf-8?q?ine-K=C3=B6nig?= , Johan Hovold , =?utf-8?q?Ilpo_J=C3=A4rvinen?= , Sherry Sun , Stefan Wahren , linux-arm-kernel@lists.infradead.org, NXP Linux Team , Pengutronix Kernel Team , Sergey Organov Subject: [PATCH v1 2/7] serial: imx: work-around for hardware RX flood Date: Wed, 1 Feb 2023 17:26:55 +0300 Message-Id: <20230201142700.4346-3-sorganov@gmail.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230201142700.4346-1-sorganov@gmail.com> References: <87bko4e65y.fsf@osv.gnss.ru> <20230201142700.4346-1-sorganov@gmail.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230201_062745_059071_087F95A2 X-CRM114-Status: GOOD ( 32.33 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Check if hardware Rx flood is in progress, and issue soft reset to UART to stop the flood. A way to reproduce the flood (checked on iMX6SX) is: open iMX UART at 9600 8N1, and from external source send 0xf0 char at 115200 8N1. In about 90% of cases this starts a flood of "receiving" of 0xff characters by the iMX UART that is terminated by any activity on RxD line, or could be stopped by issuing soft reset to the UART (just stop/start of RX does not help). Note that in essence what we did here is sending isolated start bit about 2.4 times shorter than it is to be if issued on the UART configured baud rate. There was earlier attempt to fix similar issue in: 'commit b38cb7d25711 ("serial: imx: Disable new features of autobaud detection")', but apparently it only gets harder to reproduce the issue after that commit. Signed-off-by: Sergey Organov --- drivers/tty/serial/imx.c | 123 ++++++++++++++++++++++++++++++--------- 1 file changed, 95 insertions(+), 28 deletions(-) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index bf222d8568a9..e7fce31e460d 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -210,6 +210,9 @@ struct imx_port { struct mctrl_gpios *gpios; + /* counter to stop 0xff flood */ + int idle_counter; + /* shadow registers */ unsigned int ucr1; unsigned int ucr2; @@ -428,6 +431,8 @@ static void imx_uart_soft_reset(struct imx_port *sport) imx_uart_writel(sport, ubir, UBIR); imx_uart_writel(sport, ubmr, UBMR); imx_uart_writel(sport, uts, IMX21_UTS); + + sport->idle_counter = 0; } /* called with port.lock taken and irqs off */ @@ -834,15 +839,66 @@ static irqreturn_t imx_uart_txint(int irq, void *dev_id) return IRQ_HANDLED; } +/* Check if hardware Rx flood is in progress, and issue soft reset to stop it. + * This is to be called from Rx ISRs only when some bytes were actually + * received. + * + * A way to reproduce the flood (checked on iMX6SX) is: open iMX UART at 9600 + * 8N1, and from external source send 0xf0 char at 115200 8N1. In about 90% of + * cases this starts a flood of "receiving" of 0xff characters by the iMX6 UART + * that is terminated by any activity on RxD line, or could be stopped by + * issuing soft reset to the UART (just stop/start of RX does not help). Note + * that what we do here is sending isolated start bit about 2.4 times shorter + * than it is to be on UART configured baud rate. + */ +static void imx_uart_check_flood(struct imx_port *sport, u32 usr2) +{ + /* To detect hardware 0xff flood we monitor RxD line between RX + * interrupts to isolate "receiving" of char(s) with no activity + * on RxD line, that'd never happen on actual data transfers. + * + * We use USR2_WAKE bit to check for activity on RxD line, but we have a + * race here if we clear USR2_WAKE when receiving of a char is in + * progress, so we might get RX interrupt later with USR2_WAKE bit + * cleared. Note though that as we don't try to clear USR2_WAKE when we + * detected no activity, this race may hide actual activity only once. + * + * Yet another case where receive interrupt may occur without RxD + * activity is expiration of aging timer, so we consider this as well. + * + * We use 'idle_counter' to ensure that we got at least so many RX + * interrupts without any detected activity on RxD line. 2 cases + * described plus 1 to be on the safe side gives us a margin of 3, + * below. In practice I was not able to produce a false positive to + * induce soft reset at regular data transfers even using 1 as the + * margin, so 3 is actually very strong. + * + * We count interrupts, not chars in 'idle-counter' for simplicity. + */ + + if (usr2 & USR2_WAKE) { + imx_uart_writel(sport, USR2_WAKE, USR2); + sport->idle_counter = 0; + } else if (++sport->idle_counter > 3) { + dev_warn(sport->port.dev, "RX flood detected: soft reset."); + imx_uart_soft_reset(sport); /* also clears 'sport->idle_counter' */ + } +} + static irqreturn_t __imx_uart_rxint(int irq, void *dev_id) { struct imx_port *sport = dev_id; unsigned int rx, flg, ignored = 0; struct tty_port *port = &sport->port.state->port; + u32 usr2; - while (imx_uart_readl(sport, USR2) & USR2_RDR) { - u32 usr2; + usr2 = imx_uart_readl(sport, USR2); + /* If we received something, check for 0xff flood */ + if (usr2 & USR2_RDR) + imx_uart_check_flood(sport, usr2); + + for ( ; usr2 & USR2_RDR; usr2 = imx_uart_readl(sport, USR2)) { flg = TTY_NORMAL; sport->port.icount.rx++; @@ -1180,55 +1236,64 @@ static void imx_uart_dma_rx_callback(void *data) status = dmaengine_tx_status(chan, sport->rx_cookie, &state); if (status == DMA_ERROR) { + spin_lock(&sport->port.lock); imx_uart_clear_rx_errors(sport); + spin_unlock(&sport->port.lock); return; } - if (!(sport->port.ignore_status_mask & URXD_DUMMY_READ)) { + /* + * The state-residue variable represents the empty space + * relative to the entire buffer. Taking this in consideration + * the head is always calculated base on the buffer total + * length - DMA transaction residue. The UART script from the + * SDMA firmware will jump to the next buffer descriptor, + * once a DMA transaction if finalized (IMX53 RM - A.4.1.2.4). + * Taking this in consideration the tail is always at the + * beginning of the buffer descriptor that contains the head. + */ - /* - * The state-residue variable represents the empty space - * relative to the entire buffer. Taking this in consideration - * the head is always calculated base on the buffer total - * length - DMA transaction residue. The UART script from the - * SDMA firmware will jump to the next buffer descriptor, - * once a DMA transaction if finalized (IMX53 RM - A.4.1.2.4). - * Taking this in consideration the tail is always at the - * beginning of the buffer descriptor that contains the head. - */ + /* Calculate the head */ + rx_ring->head = sg_dma_len(sgl) - state.residue; - /* Calculate the head */ - rx_ring->head = sg_dma_len(sgl) - state.residue; + /* Calculate the tail. */ + bd_size = sg_dma_len(sgl) / sport->rx_periods; + rx_ring->tail = ((rx_ring->head-1) / bd_size) * bd_size; - /* Calculate the tail. */ - bd_size = sg_dma_len(sgl) / sport->rx_periods; - rx_ring->tail = ((rx_ring->head-1) / bd_size) * bd_size; + if (rx_ring->head <= sg_dma_len(sgl) && + rx_ring->head > rx_ring->tail) { - if (rx_ring->head <= sg_dma_len(sgl) && - rx_ring->head > rx_ring->tail) { + /* Move data from tail to head */ + r_bytes = rx_ring->head - rx_ring->tail; - /* Move data from tail to head */ - r_bytes = rx_ring->head - rx_ring->tail; + /* If we received something, check for 0xff flood */ + if (r_bytes > 0) { + spin_lock(&sport->port.lock); + imx_uart_check_flood(sport, imx_uart_readl(sport, USR2)); + spin_unlock(&sport->port.lock); + } + + if (!(sport->port.ignore_status_mask & URXD_DUMMY_READ)) { /* CPU claims ownership of RX DMA buffer */ dma_sync_sg_for_cpu(sport->port.dev, sgl, 1, - DMA_FROM_DEVICE); + DMA_FROM_DEVICE); w_bytes = tty_insert_flip_string(port, - sport->rx_buf + rx_ring->tail, r_bytes); + sport->rx_buf + rx_ring->tail, r_bytes); /* UART retrieves ownership of RX DMA buffer */ dma_sync_sg_for_device(sport->port.dev, sgl, 1, - DMA_FROM_DEVICE); + DMA_FROM_DEVICE); if (w_bytes != r_bytes) sport->port.icount.buf_overrun++; sport->port.icount.rx += w_bytes; - } else { - WARN_ON(rx_ring->head > sg_dma_len(sgl)); - WARN_ON(rx_ring->head <= rx_ring->tail); } + } else { + WARN_ON(rx_ring->head > sg_dma_len(sgl)); + WARN_ON(rx_ring->head <= rx_ring->tail); } if (w_bytes) { @@ -1304,6 +1369,8 @@ static void imx_uart_clear_rx_errors(struct imx_port *sport) imx_uart_writel(sport, USR2_ORE, USR2); } + sport->idle_counter = 0; + } #define TXTL_DEFAULT 2 /* reset default */