From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Hurley Subject: Re: [PATCH RESEND 5/5] tty/serial: at91: add interrupts for modem control lines Date: Tue, 02 Sep 2014 07:40:18 -0400 Message-ID: <5405ACA2.5010308@hurleysoftware.com> References: <1400005245-11943-1-git-send-email-richard.genoud@gmail.com> <1400005245-11943-6-git-send-email-richard.genoud@gmail.com> <53E7C8F8.1080507@hurleysoftware.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: Received: from mailout32.mail01.mtsvc.net ([216.70.64.70]:53410 "EHLO n23.mail01.mtsvc.net" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752461AbaIBLk3 (ORCPT ); Tue, 2 Sep 2014 07:40:29 -0400 In-Reply-To: Sender: linux-serial-owner@vger.kernel.org List-Id: linux-serial@vger.kernel.org To: Richard Genoud Cc: Greg Kroah-Hartman , Yegor Yefremov , Nicolas Ferre , Linus Walleij , Alexander Shiyan , "linux-serial@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" On 09/01/2014 05:26 AM, Richard Genoud wrote: > 2014-08-10 21:33 GMT+02:00 Peter Hurley : >> Hi Richard, > > Hi ! > Sorry for the delay ! > >> On 05/13/2014 02:20 PM, Richard Genoud wrote: >>> Handle CTS/DSR/RI/DCD GPIO interrupts in atmel_serial. >>> >>> Signed-off-by: Richard Genoud >>> Acked-by: Greg Kroah-Hartman >>> Acked-by: Nicolas Ferre >>> --- >>> drivers/tty/serial/atmel_serial.c | 125 ++++++++++++++++++++++++++++++++++++- >>> 1 file changed, 122 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c >>> index 43ca659..3fceae0 100644 >>> --- a/drivers/tty/serial/atmel_serial.c >>> +++ b/drivers/tty/serial/atmel_serial.c >>> @@ -45,6 +45,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #include >>> #include >>> @@ -167,7 +168,9 @@ struct atmel_uart_port { >>> >>> struct serial_rs485 rs485; /* rs485 settings */ >>> struct mctrl_gpios *gpios; >>> + int gpio_irq[UART_GPIO_MAX]; >>> unsigned int tx_done_mask; >>> + bool ms_irq_enabled; >>> bool is_usart; /* usart or uart */ >>> struct timer_list uart_timer; /* uart timer */ >>> int (*prepare_rx)(struct uart_port *port); >>> @@ -489,8 +492,38 @@ static void atmel_stop_rx(struct uart_port *port) >>> */ >>> static void atmel_enable_ms(struct uart_port *port) >>> { >>> - UART_PUT_IER(port, ATMEL_US_RIIC | ATMEL_US_DSRIC >>> - | ATMEL_US_DCDIC | ATMEL_US_CTSIC); >>> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); >>> + uint32_t ier = 0; >>> + >>> + /* >>> + * Interrupt should not be enabled twice >>> + */ >>> + if (atmel_port->ms_irq_enabled) >>> + return; >>> + >>> + atmel_port->ms_irq_enabled = true; >>> + >>> + if (atmel_port->gpio_irq[UART_GPIO_CTS] >= 0) >>> + enable_irq(atmel_port->gpio_irq[UART_GPIO_CTS]); >>> + else >>> + ier |= ATMEL_US_CTSIC; >>> + >>> + if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0) >>> + enable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]); >>> + else >>> + ier |= ATMEL_US_DSRIC; >>> + >>> + if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0) >>> + enable_irq(atmel_port->gpio_irq[UART_GPIO_RI]); >>> + else >>> + ier |= ATMEL_US_RIIC; >>> + >>> + if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0) >>> + enable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]); >>> + else >>> + ier |= ATMEL_US_DCDIC; >> >> Where are these gpio irqs disabled if !UART_ENABLE_MS()? > They are disabled in atmel_shutdown()/atmel_free_gpio_irq(). > Like the doc says in Documentation/serial/driver > enable_ms(port) > Enable the modem status interrupts. > > This method may be called multiple times. Modem status > interrupts should be disabled when the shutdown method is > called. Thanks for replying. The point of the UART_ENABLE_MS() macro is to allow the UART driver to determine if modem status interrupts need to be enabled when handling the .set_termios() method. When handling set_termios(), if UART_ENABLE_MS is false (ie., the serial core does not require modem status interrupts), the UART driver can and should turn modem status interrupts off. It is _also_ true that "modem status interrupts should be disabled when the shutdown method is called"; in fact, _all_ UART-sourced interrupts should be disabled when the shutdown method is called. Regards, Peter Hurley