Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753885AbbLERkN (ORCPT ); Sat, 5 Dec 2015 12:40:13 -0500 Received: from mail-ig0-f182.google.com ([209.85.213.182]:38104 "EHLO mail-ig0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752396AbbLERkL (ORCPT ); Sat, 5 Dec 2015 12:40:11 -0500 Subject: Re: [PATCH LINUX v3 08/10] tty: xuartps: Move RX path into helper function To: Soren Brinkmann , Greg Kroah-Hartman References: <1448161172-19717-1-git-send-email-soren.brinkmann@xilinx.com> <1448161172-19717-9-git-send-email-soren.brinkmann@xilinx.com> Cc: Jiri Slaby , Michal Simek , linux-serial@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org From: Peter Hurley Message-ID: <56632177.1090605@hurleysoftware.com> Date: Sat, 5 Dec 2015 12:40:07 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <1448161172-19717-9-git-send-email-soren.brinkmann@xilinx.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3647 Lines: 108 On 11/21/2015 09:59 PM, Soren Brinkmann wrote: > Move RX-related IRQ handling into a helper function. Fixes a problem where every char received after a parity or frame error in the current isr will also be tagged as a parity or frame error. Reviewed-by: Peter Hurley NB: the sysrq problem in cdns_uart_isr() with needing to drop the locks is because cdns_uart_console_write() tries to take the port->lock. The 8250 driver handles this problem by not trying to take the port->lock if port->sysrq is non-zero. See serial8250_console_write(). > Signed-off-by: Soren Brinkmann > --- > drivers/tty/serial/xilinx_uartps.c | 50 +++++++++++++++++++++----------------- > 1 file changed, 28 insertions(+), 22 deletions(-) > > diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c > index f3ac69387b0a..db9e23eaf300 100644 > --- a/drivers/tty/serial/xilinx_uartps.c > +++ b/drivers/tty/serial/xilinx_uartps.c > @@ -172,28 +172,8 @@ struct cdns_uart { > #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \ > clk_rate_change_nb); > > -/** > - * 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) > +static void cdns_uart_handle_rx(struct uart_port *port, unsigned int isrstatus) > { > - struct uart_port *port = (struct uart_port *)dev_id; > - unsigned long flags; > - unsigned int isrstatus, numbytes; > - unsigned int data; > - char status = TTY_NORMAL; See, the previous code never reset the status back to TTY_NORMAL after any other status. > - > - spin_lock_irqsave(&port->lock, flags); > - > - /* Read the interrupt status register to determine which > - * interrupt(s) is/are active. > - */ > - isrstatus = readl(port->membase + CDNS_UART_ISR_OFFSET); > - > /* > * There is no hardware break detection, so we interpret framing > * error with all-zeros data as a break sequence. Most of the time, > @@ -223,6 +203,9 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id) > /* Receive Timeout Interrupt */ > while (!(readl(port->membase + CDNS_UART_SR_OFFSET) & > CDNS_UART_SR_RXEMPTY)) { > + u32 data; > + char status = TTY_NORMAL; > + > data = readl(port->membase + CDNS_UART_FIFO_OFFSET); > > /* Non-NULL byte after BREAK is garbage (99%) */ > @@ -263,10 +246,33 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id) > } > > uart_insert_char(port, isrstatus, CDNS_UART_IXR_OVERRUN, > - data, status); > + data, status); > } > tty_flip_buffer_push(&port->state->port); > } > +} > + > +/** > + * 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 long flags; > + unsigned int isrstatus, numbytes; > + > + spin_lock_irqsave(&port->lock, flags); > + > + /* Read the interrupt status register to determine which > + * interrupt(s) is/are active. > + */ > + isrstatus = readl(port->membase + CDNS_UART_ISR_OFFSET); > + > + cdns_uart_handle_rx(port, isrstatus); > > /* Dispatch an appropriate handler */ > if ((isrstatus & CDNS_UART_IXR_TXEMPTY) == CDNS_UART_IXR_TXEMPTY) { > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/