From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Slaby Subject: Re: [PATCH RESEND] tty: serial: men_z135_uart: Fix driver for changes in hardware Date: Wed, 10 Sep 2014 14:15:41 +0200 Message-ID: <541040ED.1020800@suse.cz> References: <1410329329-4327-1-git-send-email-johannes.thumshirn@men.de> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT Return-path: Received: from mail-wi0-f182.google.com ([209.85.212.182]:34547 "EHLO mail-wi0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750850AbaIJMPp (ORCPT ); Wed, 10 Sep 2014 08:15:45 -0400 In-Reply-To: <1410329329-4327-1-git-send-email-johannes.thumshirn@men.de> Sender: linux-serial-owner@vger.kernel.org List-Id: linux-serial@vger.kernel.org To: Johannes Thumshirn , Greg Kroah-Hartman Cc: linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org On 09/10/2014, 08:08 AM, Johannes Thumshirn wrote: > * Add module parameter to configure the (baud rate dependent) RX timeout. > Character timeout in seconds = (timeout_reg * baud_reg * 4)/freq_reg. Buried in the log. Instead, it would be nice to have this in MODULE_PARM_DESC. > --- a/drivers/tty/serial/men_z135_uart.c > +++ b/drivers/tty/serial/men_z135_uart.c ... > @@ -118,6 +117,10 @@ static int align; > module_param(align, int, S_IRUGO); > MODULE_PARM_DESC(align, "Keep hardware FIFO write pointer aligned, default 0"); > > +static int rx_timeout; > +module_param(rx_timeout, uint, S_IRUGO); int variable vs. uint handler. > @@ -373,43 +376,47 @@ out: > * @irq: The IRQ number > * @data: Pointer to UART port > * > - * Check IIR register to see which tasklet to start. > + * Check IIR register to find the cause of the interrupt and handle it. > + * It is possible that multiple interrupts reason bits are set and reading > + * the IIR is a destructive read, so we always need to check for all possible > + * interrupts and handle them. > */ > static irqreturn_t men_z135_intr(int irq, void *data) > { > struct men_z135_port *uart = (struct men_z135_port *)data; > struct uart_port *port = &uart->port; > + int handled = 0; This can be bool or use IRQ_NONE directly. ... > - return IRQ_HANDLED; > + if (irq_id & MEN_Z135_IRQ_ID_RDA || irq_id & MEN_Z135_IRQ_ID_CTI) { A matter of taste, but irq_id & (MEN_Z135_IRQ_ID_RDA | MEN_Z135_IRQ_ID_CTI) looks and works better. thanks, -- js suse labs