[PATCH 0/3] tty: serial: 8250 introduce mctrl_gpio helpers

[PATCH 0/3] tty: serial: 8250 introduce mctrl_gpio helpers

[yegorslists-gM/Ye1E23mwN+BqQ9rBEUg]

From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

This patch series aims to reintroduce the  mctrl_gpio helpers for 8250
UARTs.

There are some UARTs that use GPIO signals as a wakeup-sourse.
The first patch addresses this issue and tries to destinguish GPIO usage
via searching for "wakeup-sourse" property. Though it must be decided whether
this property is secure to use for this purpose.

Yegor Yefremov (3):
  serial: mctrl_gpio: restrict MCTRL initialization
  tty/serial/8250: use mctrl_gpio helpers
  tty/serial/8250: add DT bindings description for mctrl_gpio helpers

 Documentation/devicetree/bindings/serial/8250.txt | 26 +++++++++++++++++
 drivers/tty/serial/8250/8250.h                    | 35 ++++++++++++++++++++++-
 drivers/tty/serial/8250/8250_core.c               |  9 ++++++
 drivers/tty/serial/8250/8250_omap.c               | 31 +++++++++++---------
 drivers/tty/serial/8250/8250_port.c               |  7 ++++-
 drivers/tty/serial/8250/Kconfig                   |  1 +
 drivers/tty/serial/serial_mctrl_gpio.c            |  4 +++
 include/linux/serial_8250.h                       |  1 +
 8 files changed, 99 insertions(+), 15 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] serial: mctrl_gpio: restrict MCTRL initialization

[yegorslists-gM/Ye1E23mwN+BqQ9rBEUg]

From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

GPIOs specified for serial port can be used either as GPIO driven MCTRL
or as a wakeup-source. Use the latter property to abort the MCTRL
initialization.

Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 drivers/tty/serial/serial_mctrl_gpio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index d2da6aa..dce661c 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -19,6 +19,7 @@
 #include <linux/irq.h>
 #include <linux/gpio/consumer.h>
 #include <linux/termios.h>
+#include <linux/property.h>
 #include <linux/serial_core.h>
 #include <linux/module.h>
 
@@ -118,6 +119,9 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
 	struct mctrl_gpios *gpios;
 	enum mctrl_gpio_idx i;
 
+	if (device_property_present(dev, "wakeup-source"))
+		return ERR_PTR(-ENOSYS);
+
 	gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
 	if (!gpios)
 		return ERR_PTR(-ENOMEM);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] serial: mctrl_gpio: restrict MCTRL initialization

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 329 bytes --]

Hello,

On Thu, Jul 27, 2017 at 03:17:34PM +0200, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org wrote:
> GPIOs specified for serial port can be used either as GPIO driven MCTRL
> or as a wakeup-source. Use the latter property to abort the MCTRL
> initialization.

Why is that an either-or and not an and?

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] serial: mctrl_gpio: restrict MCTRL initialization

[Andy Shevchenko]

On Thu, 2017-07-27 at 16:00 +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Thu, Jul 27, 2017 at 03:17:34PM +0200, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org
> wrote:
> > GPIOs specified for serial port can be used either as GPIO driven
> > MCTRL
> > or as a wakeup-source. Use the latter property to abort the MCTRL
> > initialization.
> 
> Why is that an either-or and not an and?

That is how mctrl is designed right now.

Note marking the pin as a wakeup source inside mctrl driver will not
help since there is much bigger issue, i.e. pinctrl is not ready for a
such.

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] serial: mctrl_gpio: restrict MCTRL initialization

[Andy Shevchenko]

On Thu, 2017-07-27 at 15:17 +0200, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org wrote:
> From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> 
> GPIOs specified for serial port can be used either as GPIO driven
> MCTRL
> or as a wakeup-source. Use the latter property to abort the MCTRL
> initialization.

Please, elaborate on a choice of error code for this (-ENOSYS).

> 
> Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Suggested-by: I suppose.

I dunno about documentation part for this property, I leave it for DT
people to decide. Below part looks good to me.

> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  drivers/tty/serial/serial_mctrl_gpio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c
> b/drivers/tty/serial/serial_mctrl_gpio.c
> index d2da6aa..dce661c 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -19,6 +19,7 @@
>  #include <linux/irq.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/termios.h>
> +#include <linux/property.h>
>  #include <linux/serial_core.h>
>  #include <linux/module.h>
>  
> @@ -118,6 +119,9 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct
> device *dev, unsigned int idx)
>  	struct mctrl_gpios *gpios;
>  	enum mctrl_gpio_idx i;
>  
> +	if (device_property_present(dev, "wakeup-source"))
> +		return ERR_PTR(-ENOSYS);
> +
>  	gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
>  	if (!gpios)
>  		return ERR_PTR(-ENOMEM);

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] serial: mctrl_gpio: restrict MCTRL initialization

[Yegor Yefremov]

On Thu, Jul 27, 2017 at 4:01 PM, Andy Shevchenko
<andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On Thu, 2017-07-27 at 15:17 +0200, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org wrote:
>> From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>
>> GPIOs specified for serial port can be used either as GPIO driven
>> MCTRL
>> or as a wakeup-source. Use the latter property to abort the MCTRL
>> initialization.
>
> Please, elaborate on a choice of error code for this (-ENOSYS).

Something like this:

Return -ENOSYS as this would just skip mctrl initialization in the
case of wakeup source usage and not disable the port completely due to
failed serial8250_register_8250_port().

Yegor

>>
>> Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>
> Suggested-by: I suppose.
>
> I dunno about documentation part for this property, I leave it for DT
> people to decide. Below part looks good to me.
>
>> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>>  drivers/tty/serial/serial_mctrl_gpio.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c
>> b/drivers/tty/serial/serial_mctrl_gpio.c
>> index d2da6aa..dce661c 100644
>> --- a/drivers/tty/serial/serial_mctrl_gpio.c
>> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/irq.h>
>>  #include <linux/gpio/consumer.h>
>>  #include <linux/termios.h>
>> +#include <linux/property.h>
>>  #include <linux/serial_core.h>
>>  #include <linux/module.h>
>>
>> @@ -118,6 +119,9 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct
>> device *dev, unsigned int idx)
>>       struct mctrl_gpios *gpios;
>>       enum mctrl_gpio_idx i;
>>
>> +     if (device_property_present(dev, "wakeup-source"))
>> +             return ERR_PTR(-ENOSYS);
>> +
>>       gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
>>       if (!gpios)
>>               return ERR_PTR(-ENOMEM);
>
> --
> Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] tty/serial/8250: use mctrl_gpio helpers

[yegorslists-gM/Ye1E23mwN+BqQ9rBEUg]

From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

This patch permits the usage for GPIOs to control
the CTS/RTS/DTR/DSR/DCD/RI signals.

Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 drivers/tty/serial/8250/8250.h      | 35 ++++++++++++++++++++++++++++++++++-
 drivers/tty/serial/8250/8250_core.c |  9 +++++++++
 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 +
 6 files changed, 69 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index b2bdc35..c47ef69 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -15,6 +15,8 @@
 #include <linux/serial_reg.h>
 #include <linux/dmaengine.h>
 
+#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);
@@ -145,12 +147,43 @@ 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;
+
+	mctrl_gpio_set(up->gpios, mctrl_gpio);
 }
 
 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);
+
+	/* save current MCR values */
+	if (mctrl & UART_MCR_RTS)
+		mctrl_gpio |= TIOCM_RTS;
+	if (mctrl & UART_MCR_DTR)
+		mctrl_gpio |= TIOCM_DTR;
+
+	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;
+
+	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 b5def35..7a3772e 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -975,6 +975,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 
 	uart = serial8250_find_match_or_unused(&up->port);
 	if (uart && uart->port.type != PORT_8250_CIR) {
+		struct mctrl_gpios *gpios;
+
 		if (uart->port.dev)
 			uart_remove_one_port(&serial8250_reg, &uart->port);
 
@@ -1012,6 +1014,13 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		if (up->port.flags & UPF_FIXED_TYPE)
 			uart->port.type = up->port.type;
 
+		gpios = mctrl_gpio_init(&uart->port, 0);
+		if (IS_ERR(gpios)) {
+			if (PTR_ERR(gpios) != -ENOSYS)
+				return PTR_ERR(gpios);
+		} else
+			uart->gpios = gpios;
+
 		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 833771b..93fe5b9 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -134,18 +134,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);
+	}
 }
 
 /*
@@ -446,7 +449,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 a5fe0e6..f173cda 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1631,6 +1631,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);
 }
@@ -1643,6 +1645,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);
@@ -1946,7 +1950,8 @@ unsigned int serial8250_do_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);
 }
 EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
 
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index a1161ec..c0e51b0 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 61fbb44..23b6ab1 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -113,6 +113,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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] tty/serial/8250: add DT bindings description for mctrl_gpio helpers

[yegorslists-gM/Ye1E23mwN+BqQ9rBEUg]

From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

Describe {rts,cts,dtr,dsr,rng,dcd}-gpios usage.

Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 Documentation/devicetree/bindings/serial/8250.txt | 26 +++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 419ff6c..1c09db2 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -48,6 +48,16 @@ Optional properties:
 - tx-threshold: Specify the TX FIFO low water indication for parts with
   programmable TX FIFO thresholds.
 - resets : phandle + reset specifier pairs
+- rts-gpios:
+- cts-gpios:
+- dtr-gpios:
+- dsr-gpios:
+- rng-gpios:
+- 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.
+  If "wakeup-source" property is specified for the device this GPIO
+  initialization will be aborted.
 
 Note:
 * fsl,ns16550:
@@ -69,3 +79,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>;
+	};
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] tty/serial/8250: add DT bindings description for mctrl_gpio helpers

[Rob Herring]

On Thu, Jul 27, 2017 at 03:17:36PM +0200, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org wrote:
> From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> 
> Describe {rts,cts,dtr,dsr,rng,dcd}-gpios usage.
> 
> Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/serial/8250.txt | 26 +++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index 419ff6c..1c09db2 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -48,6 +48,16 @@ Optional properties:
>  - tx-threshold: Specify the TX FIFO low water indication for parts with
>    programmable TX FIFO thresholds.
>  - resets : phandle + reset specifier pairs
> +- rts-gpios:
> +- cts-gpios:
> +- dtr-gpios:
> +- dsr-gpios:
> +- rng-gpios:
> +- 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.

These are already described in serial.txt.

> +  If "wakeup-source" property is specified for the device this GPIO
> +  initialization will be aborted.

Why? Many GPIOs can function as wake-up sources. You would need get the 
IRQ for the GPIO. Also, some UARTs wake-up from the RX line. Not sure 
about any 8250 flavors, but is probably implementation specific what is 
the trigger for wake-up.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] tty: serial: 8250 introduce mctrl_gpio helpers

[Tony Lindgren]

* yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170727 06:18]:
> From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> 
> This patch series aims to reintroduce the  mctrl_gpio helpers for 8250
> UARTs.
> 
> There are some UARTs that use GPIO signals as a wakeup-sourse.
> The first patch addresses this issue and tries to destinguish GPIO usage
> via searching for "wakeup-sourse" property. Though it must be decided whether
> this property is secure to use for this purpose.

The wakeup-source part you should be able to handle pretty much
out of box with Linux generic wakeirqs if configured. See for example
drivers/i2c/i2c-core.c for the dev_pm_set_dedicated_wake_irq() part.
As long as the 8250 driver has runtime PM implemented it will wake
up the 8250 device.

This should work just fine also with am335x gpios, just configure the
secondary wakeup gpio interrupt using interrupts-extended in device
tree. Typically the interrupts are named "irq" and "wakeup". And if the
pin is used as gpio, you can just dev_pm_clear_wake_irq() during
runtime.

If having issues, we're still missing the wakeirq level configuration,
the patch below should do the trick there.

Regards,

Tony

8< -----------
diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
--- a/drivers/base/power/wakeirq.c
+++ b/drivers/base/power/wakeirq.c
@@ -198,7 +198,8 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
 	 * so we use a threaded irq.
 	 */
 	err = request_threaded_irq(irq, NULL, handle_threaded_wake_irq,
-				   IRQF_ONESHOT, dev_name(dev), wirq);
+				   irq_get_trigger_type(irq) | IRQF_ONESHOT,
+				   dev_name(dev), wirq);
 	if (err)
 		goto err_free;
 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] tty: serial: 8250 introduce mctrl_gpio helpers

[Andy Shevchenko]

On Thu, 2017-07-27 at 23:40 -0700, Tony Lindgren wrote:
> * yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170727
> 06:18]:
> > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > 
> > This patch series aims to reintroduce the  mctrl_gpio helpers for
> > 8250
> > UARTs.
> > 
> > There are some UARTs that use GPIO signals as a wakeup-sourse.
> > The first patch addresses this issue and tries to destinguish GPIO
> > usage
> > via searching for "wakeup-sourse" property. Though it must be
> > decided whether
> > this property is secure to use for this purpose.
> 
> The wakeup-source part you should be able to handle pretty much
> out of box with Linux generic wakeirqs if configured. See for example
> drivers/i2c/i2c-core.c for the dev_pm_set_dedicated_wake_irq() part.

Here are 3 cases:
1) out-of-band interrupt (usually GPIO) which can be wake source;
2) in-band interrupt (have not seen this for UART, though it's supported
by tty framework for a long time AFAIU);
3) special case when Out-of-band interrupt uses the pin in UART pool of
pins.

We are talking here about case 3).

> As long as the 8250 driver has runtime PM implemented

As you remember it has not (the ugly hack which is used right now is not
correct implementation of RPM).

>  it will wake
> up the 8250 device.

See above, in case 3) we would be able to achieve that when pin mode is
switched to GPIO and back. I don't remember if OMAP uses this approach.

> This should work just fine also with am335x gpios, just configure the
> secondary wakeup gpio interrupt using interrupts-extended in device
> tree. Typically the interrupts are named "irq" and "wakeup". And if
> the
> pin is used as gpio, you can just dev_pm_clear_wake_irq() during
> runtime.

I think this one is not related to case 3).

> If having issues, we're still missing the wakeirq level configuration,
> the patch below should do the trick there.

>  	err = request_threaded_irq(irq, NULL,
> handle_threaded_wake_irq,
> -				   IRQF_ONESHOT, dev_name(dev),
> wirq);
> +				   irq_get_trigger_type(irq) |
> IRQF_ONESHOT,

This is not needed if you use DT or ACPI and framework (serial in this
case) checks for interrupt correctly.

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] tty: serial: 8250 introduce mctrl_gpio helpers

[Tony Lindgren]

* Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> [170728 03:37]:
> On Thu, 2017-07-27 at 23:40 -0700, Tony Lindgren wrote:
> > * yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170727
> > 06:18]:
> > > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > > 
> > > This patch series aims to reintroduce the  mctrl_gpio helpers for
> > > 8250
> > > UARTs.
> > > 
> > > There are some UARTs that use GPIO signals as a wakeup-sourse.
> > > The first patch addresses this issue and tries to destinguish GPIO
> > > usage
> > > via searching for "wakeup-sourse" property. Though it must be
> > > decided whether
> > > this property is secure to use for this purpose.
> > 
> > The wakeup-source part you should be able to handle pretty much
> > out of box with Linux generic wakeirqs if configured. See for example
> > drivers/i2c/i2c-core.c for the dev_pm_set_dedicated_wake_irq() part.
> 
> Here are 3 cases:
> 1) out-of-band interrupt (usually GPIO) which can be wake source;
> 2) in-band interrupt (have not seen this for UART, though it's supported
> by tty framework for a long time AFAIU);

And case 2) is somewhat useless as it requires the uart to be
clocked.

> 3) special case when Out-of-band interrupt uses the pin in UART pool of
> pins.
> 
> We are talking here about case 3).

OK at least for am335x/am437x, changing it's mode between gpio and
uart use can be done with pinctrl framework with named pinctrl
named modes.

> > As long as the 8250 driver has runtime PM implemented
> 
> As you remember it has not (the ugly hack which is used right now is not
> correct implementation of RPM).
> 
> >  it will wake
> > up the 8250 device.
> 
> See above, in case 3) we would be able to achieve that when pin mode is
> switched to GPIO and back. I don't remember if OMAP uses this approach.

Yes that's needed for am335x and am437x to get a wake-up event from
rx pin for example. Not needed for omap3/4/5.

> > This should work just fine also with am335x gpios, just configure the
> > secondary wakeup gpio interrupt using interrupts-extended in device
> > tree. Typically the interrupts are named "irq" and "wakeup". And if
> > the
> > pin is used as gpio, you can just dev_pm_clear_wake_irq() during
> > runtime.
> 
> I think this one is not related to case 3).

Hmm but it seems we already have that working for 8250_omap for
omap3/4/5 where rx pin provides wakeup events when 8250_omap is
in runtime suspend state. See the dev_pm_set_dedicated_wake_irq()
parts in 8250_omap.c. For am335x/am437x I think the only thing
missing is toggling between uart and gpio mode for the rx pin
using pinctrl named modes. There's an example for am335x in
omap_hsmmc.c with pinctrl_pm_select_idle_state() if that helps.

But maybe you have something in case 3) that I don't follow?

> > If having issues, we're still missing the wakeirq level configuration,
> > the patch below should do the trick there.
> 
> >  	err = request_threaded_irq(irq, NULL,
> > handle_threaded_wake_irq,
> > -				   IRQF_ONESHOT, dev_name(dev),
> > wirq);
> > +				   irq_get_trigger_type(irq) |
> > IRQF_ONESHOT,
> 
> This is not needed if you use DT or ACPI and framework (serial in this
> case) checks for interrupt correctly.

I think it's needed in the DT gpio interrupt case as some
gpio controllers need to configure edge vs level.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html