From patchwork Mon Aug 17 07:22:34 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Simek X-Patchwork-Id: 7023771 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 75579C05AC for ; Mon, 17 Aug 2015 07:25:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 44F0720614 for ; Mon, 17 Aug 2015 07:25:15 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 269D2205DC for ; Mon, 17 Aug 2015 07:25:14 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZRElg-0008NV-8h; Mon, 17 Aug 2015 07:23:40 +0000 Received: from mail-wi0-f178.google.com ([209.85.212.178]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZRElE-0007uG-Vm for linux-arm-kernel@lists.infradead.org; Mon, 17 Aug 2015 07:23:14 +0000 Received: by wibhh20 with SMTP id hh20so72525968wib.0 for ; Mon, 17 Aug 2015 00:22:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references:in-reply-to:references; bh=RnbxW8DL0x9y4msjXoOxuLFNmjlzYbCEJ4ZNGR11t1Q=; b=iUVUHRBL9tQdGU1jC0ot126IlZ4+p+v0046huP1JMbDFhqdxfmudmQ5yj0cS1Sk+XK lVo9RM9xP4bRNpp6zlPcRWGi/LDQrzWjY2mvGHiX/1HdDNwvZYG/Ru6nNd8BRzvV1Bam 3vIBoeTDfsqLAlAqIICPNqAIgkRpGirflXWAGwYyDCVmTjfLbcczHyb0SVZ5prFokhYe 3GvSA7/dTIr0KmJlWDNbr6a04PQyTg+FsjpwAxoj6uN1qGeZA6MlhmZtq6mzfmMI0nZ8 kin4Qqk/7KYYnvttxS4NBenvCd3yLy0sTqdDenamAmyAi31ENvsLGtyObb9b/lgwWNGY MJAQ== X-Gm-Message-State: ALoCoQkVa1GphsjzzwYh1FJNJkcogFmgHZxlvq1/BxIECIHt3KceBhkfm8rG7tELVUNVGuFbKObF X-Received: by 10.180.87.230 with SMTP id bb6mr29976164wib.36.1439796171223; Mon, 17 Aug 2015 00:22:51 -0700 (PDT) Received: from localhost (nat-35.starnet.cz. [178.255.168.35]) by smtp.gmail.com with ESMTPSA id ub19sm15384922wib.8.2015.08.17.00.22.50 (version=TLSv1.1 cipher=RC4-SHA bits=128/128); Mon, 17 Aug 2015 00:22:50 -0700 (PDT) From: Michal Simek To: linux-kernel@vger.kernel.org, monstr@monstr.eu Subject: [PATCH 4/4] serial: xuartps: Rewrite the interrupt handling logic Date: Mon, 17 Aug 2015 09:22:34 +0200 Message-Id: X-Mailer: git-send-email 2.3.5 In-Reply-To: <5b154ba97bcd79e49e2131152eb2fc1761594a6c.1439796149.git.michal.simek@xilinx.com> References: <5b154ba97bcd79e49e2131152eb2fc1761594a6c.1439796149.git.michal.simek@xilinx.com> In-Reply-To: <5b154ba97bcd79e49e2131152eb2fc1761594a6c.1439796149.git.michal.simek@xilinx.com> References: <5b154ba97bcd79e49e2131152eb2fc1761594a6c.1439796149.git.michal.simek@xilinx.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150817_002313_194815_8639E6C2 X-CRM114-Status: GOOD ( 19.93 ) X-Spam-Score: -2.6 (--) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Anirudha Sarangi , Jiri Slaby , =?UTF-8?q?S=C3=B6ren=20Brinkmann?= , linux-serial@vger.kernel.org, Greg Kroah-Hartman , linux-arm-kernel@lists.infradead.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Anirudha Sarangi The existing interrupt handling logic has followins issues. - Upon a parity error with default configuration, the control never comes out of the ISR thereby hanging Linux. - The error handling logic around framing and parity error are buggy. There are chances that the errors will never be captured. - The existing ISR is just too long. This patch fixes all these concerns. It separates out the Tx and Rx hanling logic into separate functions. It ensures that the status registers are cleared on all cases so that a hang situation never arises. Signed-off-by: Anirudha Sarangi Signed-off-by: Michal Simek --- drivers/tty/serial/xilinx_uartps.c | 194 ++++++++++++++++++++----------------- 1 file changed, 104 insertions(+), 90 deletions(-) diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index 2dc26e5f1384..c771dbbf6161 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -173,61 +173,86 @@ struct cdns_uart { clk_rate_change_nb); /** - * cdns_uart_isr - Interrupt handler - * @irq: Irq number - * @dev_id: Id of the port - * - * Return: IRQHANDLED + * cdns_uart_handle_tx - Handle the bytes to be Txed. + * @dev_id: Id of the UART port + * Return: None */ -static irqreturn_t cdns_uart_isr(int irq, void *dev_id) +static void cdns_uart_handle_tx(void *dev_id) { struct uart_port *port = (struct uart_port *)dev_id; - unsigned long flags; - unsigned int isrstatus, numbytes; - unsigned int data; - char status = TTY_NORMAL; + unsigned int numbytes; - spin_lock_irqsave(&port->lock, flags); + if (uart_circ_empty(&port->state->xmit)) { + writel(CDNS_UART_IXR_TXEMPTY, port->membase + + CDNS_UART_IDR_OFFSET); + } else { + numbytes = port->fifosize; + /* Break if no more data available in the UART buffer */ + while (numbytes--) { + if (uart_circ_empty(&port->state->xmit)) + break; + /* + * Get the data from the UART circular buffer + * and write it to the cdns_uart's TX_FIFO + * register. + */ + writel(port->state->xmit.buf[port->state->xmit.tail], + port->membase + CDNS_UART_FIFO_OFFSET); - /* Read the interrupt status register to determine which - * interrupt(s) is/are active. - */ - isrstatus = readl(port->membase + CDNS_UART_ISR_OFFSET); + port->icount.tx++; - /* - * There is no hardware break detection, so we interpret framing - * error with all-zeros data as a break sequence. Most of the time, - * there's another non-zero byte at the end of the sequence. - */ - if (isrstatus & CDNS_UART_IXR_FRAMING) { - while (!(readl(port->membase + CDNS_UART_SR_OFFSET) & - CDNS_UART_SR_RXEMPTY)) { - if (!readl(port->membase + CDNS_UART_FIFO_OFFSET)) { - port->read_status_mask |= CDNS_UART_IXR_BRK; - isrstatus &= ~CDNS_UART_IXR_FRAMING; - } + /* + * Adjust the tail of the UART buffer and wrap + * the buffer if it reaches limit. + */ + port->state->xmit.tail = + (port->state->xmit.tail + 1) & + (UART_XMIT_SIZE - 1); } - writel(CDNS_UART_IXR_FRAMING, - port->membase + CDNS_UART_ISR_OFFSET); - } - /* drop byte with parity error if IGNPAR specified */ - if (isrstatus & port->ignore_status_mask & CDNS_UART_IXR_PARITY) - isrstatus &= ~(CDNS_UART_IXR_RXTRIG | CDNS_UART_IXR_TOUT); + if (uart_circ_chars_pending( + &port->state->xmit) < WAKEUP_CHARS) + uart_write_wakeup(port); + } +} - isrstatus &= port->read_status_mask; - isrstatus &= ~port->ignore_status_mask; +/** + * cdns_uart_handle_rx - Handle the received bytes along with Rx errors. + * @dev_id: Id of the UART port + * @isrstatus: The interrupt status register value as read + * Return: None + */ +static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus) +{ + struct uart_port *port = (struct uart_port *)dev_id; + unsigned int data; + unsigned int framerrprocessed = 0; + char status = TTY_NORMAL; - if ((isrstatus & CDNS_UART_IXR_TOUT) || - (isrstatus & CDNS_UART_IXR_RXTRIG)) { - /* Receive Timeout Interrupt */ - while (!(readl(port->membase + CDNS_UART_SR_OFFSET) & - CDNS_UART_SR_RXEMPTY)) { - data = readl(port->membase + CDNS_UART_FIFO_OFFSET); + while ((readl(port->membase + CDNS_UART_SR_OFFSET) & + CDNS_UART_SR_RXEMPTY) != CDNS_UART_SR_RXEMPTY) { + data = readl(port->membase + CDNS_UART_FIFO_OFFSET); + port->icount.rx++; + /* + * There is no hardware break detection, so we interpret + * framing error with all-zeros data as a break sequence. + * Most of the time, there's another non-zero byte at the + * end of the sequence. + */ + if (isrstatus & CDNS_UART_IXR_FRAMING) { + if (!data) { + port->read_status_mask |= CDNS_UART_IXR_BRK; + framerrprocessed = 1; + continue; + } + } + isrstatus &= port->read_status_mask; + isrstatus &= ~port->ignore_status_mask; - /* Non-NULL byte after BREAK is garbage (99%) */ - if (data && (port->read_status_mask & - CDNS_UART_IXR_BRK)) { + if ((isrstatus & CDNS_UART_IXR_TOUT) || + (isrstatus & CDNS_UART_IXR_RXTRIG)) { + if (data && + (port->read_status_mask & CDNS_UART_IXR_BRK)) { port->read_status_mask &= ~CDNS_UART_IXR_BRK; port->icount.brk++; if (uart_handle_break(port)) @@ -249,67 +274,56 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id) spin_lock(&port->lock); } #endif - - port->icount.rx++; - if (isrstatus & CDNS_UART_IXR_PARITY) { port->icount.parity++; status = TTY_PARITY; - } else if (isrstatus & CDNS_UART_IXR_FRAMING) { + } + if ((isrstatus & CDNS_UART_IXR_FRAMING) && + !framerrprocessed) { port->icount.frame++; status = TTY_FRAME; - } else if (isrstatus & CDNS_UART_IXR_OVERRUN) { + } + if (isrstatus & CDNS_UART_IXR_OVERRUN) { port->icount.overrun++; + tty_insert_flip_char(&port->state->port, 0, + TTY_OVERRUN); } - - uart_insert_char(port, isrstatus, CDNS_UART_IXR_OVERRUN, - data, status); + tty_insert_flip_char(&port->state->port, data, status); } - spin_unlock(&port->lock); - tty_flip_buffer_push(&port->state->port); - spin_lock(&port->lock); } + spin_unlock(&port->lock); + tty_flip_buffer_push(&port->state->port); + spin_lock(&port->lock); +} - /* Dispatch an appropriate handler */ - if ((isrstatus & CDNS_UART_IXR_TXEMPTY) == CDNS_UART_IXR_TXEMPTY) { - if (uart_circ_empty(&port->state->xmit)) { - writel(CDNS_UART_IXR_TXEMPTY, - port->membase + CDNS_UART_IDR_OFFSET); - } else { - numbytes = port->fifosize; - /* Break if no more data available in the UART buffer */ - while (numbytes--) { - if (uart_circ_empty(&port->state->xmit)) - break; - /* Get the data from the UART circular buffer - * and write it to the cdns_uart's TX_FIFO - * register. - */ - writel(port->state->xmit.buf[ - port->state->xmit.tail], - port->membase + CDNS_UART_FIFO_OFFSET); - - port->icount.tx++; - - /* Adjust the tail of the UART buffer and wrap - * the buffer if it reaches limit. - */ - port->state->xmit.tail = - (port->state->xmit.tail + 1) & - (UART_XMIT_SIZE - 1); - } +/** + * cdns_uart_isr - Interrupt handler + * @irq: Irq number + * @dev_id: Id of the port + * + * Return: IRQHANDLED + */ +static irqreturn_t cdns_uart_isr(int irq, void *dev_id) +{ + struct uart_port *port = (struct uart_port *)dev_id; + unsigned int isrstatus; - if (uart_circ_chars_pending( - &port->state->xmit) < WAKEUP_CHARS) - uart_write_wakeup(port); - } - } + spin_lock(&port->lock); + /* Read the interrupt status register to determine which + * interrupt(s) is/are active and clear them. + */ + isrstatus = readl(port->membase + CDNS_UART_ISR_OFFSET); writel(isrstatus, port->membase + CDNS_UART_ISR_OFFSET); - /* be sure to release the lock and tty before leaving */ - spin_unlock_irqrestore(&port->lock, flags); + if (isrstatus & CDNS_UART_IXR_TXEMPTY) { + cdns_uart_handle_tx(dev_id); + isrstatus &= ~CDNS_UART_IXR_TXEMPTY; + } + if (isrstatus & CDNS_UART_IXR_MASK) + cdns_uart_handle_rx(dev_id, isrstatus); + spin_unlock(&port->lock); return IRQ_HANDLED; }