From patchwork Tue Apr 5 10:32:53 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yegor Yefremov X-Patchwork-Id: 8749471 Return-Path: X-Original-To: patchwork-linux-omap@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1AA3A9F336 for ; Tue, 5 Apr 2016 10:33:20 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C8C5920389 for ; Tue, 5 Apr 2016 10:33:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 80C8E2034E for ; Tue, 5 Apr 2016 10:33:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757581AbcDEKdQ (ORCPT ); Tue, 5 Apr 2016 06:33:16 -0400 Received: from mail-vk0-f67.google.com ([209.85.213.67]:36334 "EHLO mail-vk0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757305AbcDEKdO (ORCPT ); Tue, 5 Apr 2016 06:33:14 -0400 Received: by mail-vk0-f67.google.com with SMTP id x190so1275373vka.3; Tue, 05 Apr 2016 03:33:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=HW7nslABrx1OECSoX6UvMyB1SInxvez5zsHqjnEm6C4=; b=qwkoE2FQ6vl7v8+Aue98TQq8DupQtmq1oU4nqC70gjtZ/APM285KBJ/lDIcvEQLhfp A7Rke0k+vBS0QbHjoPJDKrDnoqM5Wq9HlekGeAepuP2patmF8lVQV+DvdWQF5KWcvDbi U/1ZNZ4th6K1e5O6uwBf5CkdQfcF/5EaFUgKUzeIwVkRu3Rn8zo8gZgGpmTkNB9dF4U8 IT20JKNTvHrNjCwEw0ho7QFAXQN6+idkxYhN+kEpaWpU16Iw7UTl8ptxAbaXgktZEe0L zkXnu1zWCSohOCv6QB86aMdPxHW1wrX7ltt0bWdxvnbVd1ycG7tUw0YLkY/YbNbTc6Jy okZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=HW7nslABrx1OECSoX6UvMyB1SInxvez5zsHqjnEm6C4=; b=C9l1lpP7Do1sE8N4UsG0KUNWgOi/5K/eNQ002x9tYn9f4qL6xMOdX0bN8lqHBHwT70 QoxUukpg70h5+Ysnaa07GKam5FKEfXM+1xPU/r2VWCWqx8jXtzeMj4Y5Of8ohkU1qoDX R4O0WNImJMc1DJkz4E2X0tR8WubncBFjK5l2Hw98WdMS2T8E0nEK6FIe2Kxc/ZDvhAjY /rJL+GZtMh4DL91LBX/dGfIgh7Gs3c+FK77M9ZO1h6UgdtkfT9T8yzsX2kjpHxQEtv3E Fi2Xv94xgp/Xqd1ajIgiL+YY6kvrZfFT6I8cFGh5epHKGqkDXIf5O/LlwKKQ/Xa3xpsh i1QQ== X-Gm-Message-State: AD7BkJIyJzHLxcY9sIjiN1TsPizI64jG2NFHEnoFJBDw31dPDSIubc+3BvviDEKH+S2SyYPJoaE3QN8CH9Vs+w== X-Received: by 10.176.2.49 with SMTP id 46mr8007858uas.39.1459852393509; Tue, 05 Apr 2016 03:33:13 -0700 (PDT) MIME-Version: 1.0 Received: by 10.31.190.196 with HTTP; Tue, 5 Apr 2016 03:32:53 -0700 (PDT) In-Reply-To: <1458652329-10138-6-git-send-email-yegorslists@googlemail.com> References: <1458652329-10138-1-git-send-email-yegorslists@googlemail.com> <1458652329-10138-6-git-send-email-yegorslists@googlemail.com> From: Yegor Yefremov Date: Tue, 5 Apr 2016 12:32:53 +0200 Message-ID: Subject: Re: [PATCH v2 5/5] tty/serial/8250: use mctrl_gpio helpers To: "linux-serial@vger.kernel.org" Cc: "linux-omap@vger.kernel.org" , Greg KH , Nishanth Menon , Sebastian Andrzej Siewior , Tony Lindgren , "Nori, Sekhar" , Yegor Yefremov , Peter Hurley , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham 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 On Tue, Mar 22, 2016 at 2:12 PM, wrote: > From: Yegor Yefremov > > This patch permits the usage fo GPIOs to control the CTS/RTS/DTR/DSR/DCD/RI > signals. > > Signed-off-by: Yegor Yefremov > --- > hanges: > v2: - use newly introduced serial8250_in_MCR/serial8250_out_MCR > - don't use autoRTS/CTS, if RTS pin is made via GPIO > > Documentation/devicetree/bindings/serial/8250.txt | 19 ++++++++++ > drivers/tty/serial/8250/8250.h | 43 ++++++++++++++++++++++- > drivers/tty/serial/8250/8250_core.c | 4 +++ > drivers/tty/serial/8250/8250_omap.c | 31 +++++++++------- > drivers/tty/serial/8250/8250_port.c | 7 +++- > drivers/tty/serial/8250/Kconfig | 1 + > include/linux/serial_8250.h | 1 + > 7 files changed, 91 insertions(+), 15 deletions(-) > > diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt > index 936ab5b..f5561ac 100644 > --- a/Documentation/devicetree/bindings/serial/8250.txt > +++ b/Documentation/devicetree/bindings/serial/8250.txt > @@ -42,6 +42,9 @@ Optional properties: > - auto-flow-control: one way to enable automatic flow control support. The > driver is allowed to detect support for the capability even without this > property. > +- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD > + line respectively. It will use specified GPIO instead of the peripheral > + function pin for the UART feature. If unsure, don't specify this property. > > Note: > * fsl,ns16550: > @@ -63,3 +66,19 @@ Example: > interrupts = <10>; > reg-shift = <2>; > }; > + > +Example for OMAP UART using GPIO-based modem control signals: > + > + uart4: serial@49042000 { > + compatible = "ti,omap3-uart"; > + reg = <0x49042000 0x400>; > + interrupts = <80>; > + ti,hwmods = "uart4"; > + clock-frequency = <48000000>; > + cts-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>; > + rts-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>; > + dtr-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>; > + dsr-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>; > + dcd-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>; > + rng-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>; > + }; > diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h > index 73bf57d..8e92baf 100644 > --- a/drivers/tty/serial/8250/8250.h > +++ b/drivers/tty/serial/8250/8250.h > @@ -15,6 +15,8 @@ > #include > #include > > +#include "../serial_mctrl_gpio.h" > + > struct uart_8250_dma { > int (*tx_dma)(struct uart_8250_port *p); > int (*rx_dma)(struct uart_8250_port *p, unsigned int iir); > @@ -134,12 +136,51 @@ void serial8250_em485_destroy(struct uart_8250_port *p); > > static inline void serial8250_out_MCR(struct uart_8250_port *up, int value) > { > + int mctrl_gpio = 0; > + > serial_out(up, UART_MCR, value); > + > + if (value & UART_MCR_RTS) > + mctrl_gpio |= TIOCM_RTS; > + if (value & UART_MCR_DTR) > + mctrl_gpio |= TIOCM_DTR; > + if (value & UART_MCR_OUT1) > + mctrl_gpio |= TIOCM_OUT1; > + if (value & UART_MCR_OUT2) > + mctrl_gpio |= TIOCM_OUT2; > + > + mctrl_gpio_set(up->gpios, mctrl_gpio); I've got a kernel crash from kernel robot. If we use UART before general initialization (earlyprintk), then any call to mctrl API would result in NULL pointer dereference. One solution would be to check, if gpios IS_ERR_OR_NULL(). See below: desc_array[count] = gpios->gpio[i]; What do you think? > } > > static inline int serial8250_in_MCR(struct uart_8250_port *up) > { > - return serial_in(up, UART_MCR); > + int mctrl, mctrl_gpio = 0; > + > + mctrl = serial_in(up, UART_MCR); > + > + mctrl_gpio = mctrl_gpio_get_outputs(up->gpios, &mctrl_gpio); > + > + if (mctrl_gpio & TIOCM_RTS) > + mctrl |= UART_MCR_RTS; > + else > + mctrl &= ~UART_MCR_RTS; > + > + if (mctrl_gpio & TIOCM_DTR) > + mctrl |= UART_MCR_DTR; > + else > + mctrl &= ~UART_MCR_DTR; > + > + if (mctrl_gpio & TIOCM_OUT1) > + mctrl |= UART_MCR_OUT1; > + else > + mctrl &= ~UART_MCR_OUT1; > + > + if (mctrl_gpio & TIOCM_OUT2) > + mctrl |= UART_MCR_OUT2; > + else > + mctrl &= ~UART_MCR_OUT2; Should we perhaps check, if particular signal is really implemented as GPIO? > + return mctrl; > } > > #if defined(__alpha__) && !defined(CONFIG_PCI) > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c > index 2f4f5ee..bfde589 100644 > --- a/drivers/tty/serial/8250/8250_core.c > +++ b/drivers/tty/serial/8250/8250_core.c > @@ -1010,6 +1010,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up) > if (up->port.flags & UPF_FIXED_TYPE) > uart->port.type = up->port.type; > > + uart->gpios = mctrl_gpio_init(&uart->port, 0); > + if (IS_ERR(uart->gpios)) > + dev_dbg(uart->port.dev, "Failed to init mctrl_gpio\n"); > + > serial8250_set_defaults(uart); > > /* Possibly override default I/O functions. */ > diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c > index d8bd8ce..d4e7663 100644 > --- a/drivers/tty/serial/8250/8250_omap.c > +++ b/drivers/tty/serial/8250/8250_omap.c > @@ -128,18 +128,21 @@ static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl) > > serial8250_do_set_mctrl(port, mctrl); > > - /* > - * Turn off autoRTS if RTS is lowered and restore autoRTS setting > - * if RTS is raised > - */ > - lcr = serial_in(up, UART_LCR); > - serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); > - if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS)) > - priv->efr |= UART_EFR_RTS; > - else > - priv->efr &= ~UART_EFR_RTS; > - serial_out(up, UART_EFR, priv->efr); > - serial_out(up, UART_LCR, lcr); > + if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios, > + UART_GPIO_RTS))) { > + /* > + * Turn off autoRTS if RTS is lowered and restore autoRTS > + * setting if RTS is raised > + */ > + lcr = serial_in(up, UART_LCR); > + serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); > + if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS)) > + priv->efr |= UART_EFR_RTS; > + else > + priv->efr &= ~UART_EFR_RTS; > + serial_out(up, UART_EFR, priv->efr); > + serial_out(up, UART_LCR, lcr); > + } > } > > /* > @@ -440,7 +443,9 @@ static void omap_8250_set_termios(struct uart_port *port, > priv->efr = 0; > up->port.status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS | UPSTAT_AUTOXOFF); > > - if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) { > + if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW > + && IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios, > + UART_GPIO_RTS))) { > /* Enable AUTOCTS (autoRTS is enabled when RTS is raised) */ > up->port.status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS; > priv->efr |= UART_EFR_CTS; > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > index 094763b..bdf2dc4 100644 > --- a/drivers/tty/serial/8250/8250_port.c > +++ b/drivers/tty/serial/8250/8250_port.c > @@ -1611,6 +1611,8 @@ static void serial8250_disable_ms(struct uart_port *port) > if (up->bugs & UART_BUG_NOMSR) > return; > > + mctrl_gpio_disable_ms(up->gpios); > + > up->ier &= ~UART_IER_MSI; > serial_port_out(port, UART_IER, up->ier); > } > @@ -1623,6 +1625,8 @@ static void serial8250_enable_ms(struct uart_port *port) > if (up->bugs & UART_BUG_NOMSR) > return; > > + mctrl_gpio_enable_ms(up->gpios); > + > up->ier |= UART_IER_MSI; > > serial8250_rpm_get(up); > @@ -1900,7 +1904,8 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port) > ret |= TIOCM_DSR; > if (status & UART_MSR_CTS) > ret |= TIOCM_CTS; > - return ret; > + > + return mctrl_gpio_get(up->gpios, &ret); > } > > void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl) > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig > index 64742a0..74cf227 100644 > --- a/drivers/tty/serial/8250/Kconfig > +++ b/drivers/tty/serial/8250/Kconfig > @@ -6,6 +6,7 @@ > config SERIAL_8250 > tristate "8250/16550 and compatible serial support" > select SERIAL_CORE > + select SERIAL_MCTRL_GPIO if GPIOLIB > ---help--- > This selects whether you want to include the driver for the standard > serial ports. The standard answer is Y. People who might say N > diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h > index 4348797..3148d6d 100644 > --- a/include/linux/serial_8250.h > +++ b/include/linux/serial_8250.h > @@ -110,6 +110,7 @@ struct uart_8250_port { > * if no_console_suspend > */ > unsigned char probe; > + struct mctrl_gpios *gpios; > #define UART_PROBE_RSA (1 << 0) > > /* > -- > 2.1.4 > --- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/drivers/tty/serial/serial_mctrl_gpio.c +++ b/drivers/tty/serial/serial_mctrl_gpio.c @@ -54,6 +54,9 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl) int value_array[UART_GPIO_MAX]; unsigned int count = 0; + if (IS_ERR_OR_NULL(gpios)) + return; + for (i = 0; i < UART_GPIO_MAX; i++) if (gpios->gpio[i] && mctrl_gpios_desc[i].dir_out) {