[PATCH v3] serial: mxs-auart: gpios as modem signals (dirty)

[PATCH v3] serial: mxs-auart: gpios as modem signals (dirty)

[Janusz Uzycki]

The patchset
"Re: [PATCH 1/4] serial: mxs-auart: use mctrl_gpio helpers for handling modem signals (v2.2c)"
is changed and resent here.

Please comment.

v3 changelog:
* own patches reordered to apply mainline
* outsize of the patchset as independent:
   serial: mxs-auart: add sysrq support

new [PATCH 1/4] serial: mxs-auart: ctrl removed from mxs_auart_port
* the ctrl variable duplicated mctrl, member of uart_port structure
  in serial_core.h
* the code duplicated uart_update_mctrl() and uart_tiocmget()
  in serial_core.c
* mxs_auart_get_mctrl() reads back RTS line. It could be removed too
  but not sure.

[PATCH 2/4] serial: mxs-auart: use mctrl_gpio helpers for handling
 modem signals
* mctrl_gpio_free() removed to simplify:
  mctrl_gpio_free() is not necessary in mxs_auart_probe() and
  mxs_auart_remove() because mctrl_gpio_init() does all
  allocations with devm_* functions.
  (see Documentation/serial/driver since kernel 3.16)
* DMA on HW flow control comment updated, still not sure about the comment
* mxs_auart_modem_status() removed from mxs_auart_get_mctrl():
  mctrl_gpio_get() does not clear gpio interrupt pendings like
  8250_core.c does with MSR.
* mxs_auart_modem_status() moved to [3/4]
  If enable_ms() is not called, uart_handle_cts_change()
  shouldn't be called.

[PATCH 3/4] serial: mxs-auart: add interrupts for modem control lines
* introduces mctrl_prev instead of removed ctrl
* mxs_auart_modem_status() moved from [3/4]
* mxs_auart_modem_status() interrupt_enabled meant s->ms_irq_enabled

[PATCH 4/4] serial: mxs-auart: enable PPS support
* no changes

 .../devicetree/bindings/serial/fsl-mxs-auart.txt   |  10 +-
 drivers/tty/serial/Kconfig                         |   1 +
 drivers/tty/serial/mxs-auart.c                     | 222 ++++++++++++++++++++-
 3 files changed, 224 insertions(+), 9 deletions(-)

[PATCH 1/4] serial: mxs-auart: ctrl removed from mxs_auart_port

[Janusz Uzycki]

The ctrl variable duplicated mctrl, member of uart_port structure
in serial_core.h.
The code duplicated uart_update_mctrl() and uart_tiocmget()
in serial_core.c.
mxs_auart_get_mctrl() reads back RTS line. It could be removed too
but not sure.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---
 drivers/tty/serial/mxs-auart.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index e838e84..c712108 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -143,7 +143,6 @@ struct mxs_auart_port {
 #define MXS_AUART_DMA_RX_READY	3  /* bit 3 */
 #define MXS_AUART_RTSCTS	4  /* bit 4 */
 	unsigned long flags;
-	unsigned int ctrl;
 	enum mxs_auart_type devtype;
 
 	unsigned int irq;
@@ -406,8 +405,6 @@ static void mxs_auart_release_port(struct uart_port *u)
 
 static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 {
-	struct mxs_auart_port *s = to_auart_port(u);
-
 	u32 ctrl = readl(u->membase + AUART_CTRL2);
 
 	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
@@ -418,16 +415,14 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 			ctrl |= AUART_CTRL2_RTS;
 	}
 
-	s->ctrl = mctrl;
 	writel(ctrl, u->membase + AUART_CTRL2);
 }
 
 static u32 mxs_auart_get_mctrl(struct uart_port *u)
 {
-	struct mxs_auart_port *s = to_auart_port(u);
 	u32 stat = readl(u->membase + AUART_STAT);
 	int ctrl2 = readl(u->membase + AUART_CTRL2);
-	u32 mctrl = s->ctrl;
+	u32 mctrl = 0;
 
 	mctrl &= ~TIOCM_CTS;
 	if (stat & AUART_STAT_CTS)
@@ -1071,8 +1066,6 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	s->port.type = PORT_IMX;
 	s->port.dev = s->dev = &pdev->dev;
 
-	s->ctrl = 0;
-
 	s->irq = platform_get_irq(pdev, 0);
 	s->port.irq = s->irq;
 	ret = request_irq(s->irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s);
-- 
1.7.11.3

[PATCH 2/4] serial: mxs-auart: use mctrl_gpio helpers for handling modem signals

[Janusz Uzycki]

Dedicated CTS and RTS pins are unusable together with a lot of other
peripherals because they share the same line. Pinctrl is limited.

Moreover, the AUART controller doesn't handle DTR/DSR/DCD/RI signals,
so we have to control them via GPIO.

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

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---
 .../devicetree/bindings/serial/fsl-mxs-auart.txt   | 10 +++++-
 drivers/tty/serial/Kconfig           |  1 +
 drivers/tty/serial/mxs-auart.c       | 37 ++++++++++++++++++++--
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt b/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt
index 59a40f1..7c408c8 100644
--- a/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt
+++ b/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt
@@ -11,8 +11,13 @@ Required properties:
 - dma-names: "rx" for RX channel, "tx" for TX channel.
 
 Optional properties:
-- fsl,uart-has-rtscts : Indicate the UART has RTS and CTS lines,
+- fsl,uart-has-rtscts : Indicate the UART has RTS and CTS lines
+  for hardware flow control,
 	it also means you enable the DMA support for this UART.
+- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
+  line respectively. It will use specified PIO instead of the peripheral
+  function pin for the USART feature.
+  If unsure, don't specify this property.
 
 Example:
 auart0: serial@8006a000 {
@@ -21,6 +26,9 @@ auart0: serial@8006a000 {
 	interrupts = <112>;
 	dmas = <&dma_apbx 8>, <&dma_apbx 9>;
 	dma-names = "rx", "tx";
+	cts-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
+	dsr-gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
+	dcd-gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
 };
 
 Note: Each auart port should have an alias correctly numbered in "aliases"
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 4fe8ca1..90e8516 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1357,6 +1357,7 @@ config SERIAL_MXS_AUART
 	depends on ARCH_MXS
 	tristate "MXS AUART support"
 	select SERIAL_CORE
+	select SERIAL_MCTRL_GPIO if GPIOLIB
 	help
 	  This driver supports the MXS Application UART (AUART) port.
 
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index c712108..fdfa8a9 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -42,6 +42,9 @@
 
 #include <asm/cacheflush.h>
 
+#include <linux/err.h>
+#include "serial_mctrl_gpio.h"
+
 #define MXS_AUART_PORTS 5
 #define MXS_AUART_FIFO_SIZE		16
 
@@ -158,6 +161,8 @@ struct mxs_auart_port {
 	struct scatterlist rx_sgl;
 	struct dma_chan	*rx_dma_chan;
 	void *rx_dma_buf;
+
+	struct mctrl_gpios	*gpios;
 };
 
 static struct platform_device_id mxs_auart_devtype[] = {
@@ -405,6 +410,8 @@ static void mxs_auart_release_port(struct uart_port *u)
 
 static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 {
+	struct mxs_auart_port *s = to_auart_port(u);
+
 	u32 ctrl = readl(u->membase + AUART_CTRL2);
 
 	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
@@ -416,10 +423,13 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 	}
 
 	writel(ctrl, u->membase + AUART_CTRL2);
+
+	mctrl_gpio_set(s->gpios, mctrl);
 }
 
 static u32 mxs_auart_get_mctrl(struct uart_port *u)
 {
+	struct mxs_auart_port *s = to_auart_port(u);
 	u32 stat = readl(u->membase + AUART_STAT);
 	int ctrl2 = readl(u->membase + AUART_CTRL2);
 	u32 mctrl = 0;
@@ -431,7 +441,7 @@ static u32 mxs_auart_get_mctrl(struct uart_port *u)
 	if (ctrl2 & AUART_CTRL2_RTS)
 		mctrl |= TIOCM_RTS;
 
-	return mctrl;
+	return mctrl_gpio_get(s->gpios, &mctrl);
 }
 
 static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
@@ -635,7 +645,12 @@ static void mxs_auart_settermios(struct uart_port *u,
 		ctrl |= AUART_LINECTRL_STP2;
 
 	/* figure out the hardware flow control settings */
-	if (cflag & CRTSCTS) {
+	/* FIXME: Likely DMA could be enabled not only for HW flow control */
+	if (cflag & CRTSCTS &&
+			(IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
+					 UART_GPIO_RTS)) ||
+			 IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
+					 UART_GPIO_CTS)))) {
 		/*
 		 * The DMA has a bug(see errata:2836) in mx23.
 		 * So we can not implement the DMA for auart in mx23,
@@ -695,7 +710,10 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
 			s->port.membase + AUART_INTR_CLR);
 
 	if (istat & AUART_INTR_CTSMIS) {
-		uart_handle_cts_change(&s->port, stat & AUART_STAT_CTS);
+		if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
+						UART_GPIO_CTS)))
+			uart_handle_cts_change(&s->port,
+					stat & AUART_STAT_CTS);
 		writel(AUART_INTR_CTSMIS,
 				s->port.membase + AUART_INTR_CLR);
 		istat &= ~AUART_INTR_CTSMIS;
@@ -1019,6 +1037,14 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
 	return 0;
 }
 
+static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
+{
+	s->gpios = mctrl_gpio_init(dev, 0);
+	if (IS_ERR_OR_NULL(s->gpios))
+		return -1;
+	return 0;
+}
+
 static int mxs_auart_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id =
@@ -1074,6 +1100,11 @@ static int mxs_auart_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, s);
 
+	ret = mxs_auart_init_gpios(s, &pdev->dev);
+	if (ret < 0)
+		dev_err(&pdev->dev, "%s",
+			"Failed to initialize GPIOs. The serial port may not work as expected");
+
 	auart_port[s->port.line] = s;
 
 	mxs_auart_reset(&s->port);
-- 
1.7.11.3

[PATCH 3/4] serial: mxs-auart: add interrupts for modem control lines

[Janusz Uzycki]

Handle CTS/DSR/RI/DCD GPIO interrupts in mxs-auart.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---
 drivers/tty/serial/mxs-auart.c | 176 ++++++++++++++++++++++++++-
 1 file changed, 174 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index fdfa8a9..ab127a2 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -42,7 +42,10 @@
 
 #include <asm/cacheflush.h>
 
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/err.h>
+#include <linux/irq.h>
 #include "serial_mctrl_gpio.h"
 
 #define MXS_AUART_PORTS 5
@@ -146,6 +149,7 @@ struct mxs_auart_port {
 #define MXS_AUART_DMA_RX_READY	3  /* bit 3 */
 #define MXS_AUART_RTSCTS	4  /* bit 4 */
 	unsigned long flags;
+	unsigned int mctrl_prev;
 	enum mxs_auart_type devtype;
 
 	unsigned int irq;
@@ -163,6 +167,8 @@ struct mxs_auart_port {
 	void *rx_dma_buf;
 
 	struct mctrl_gpios	*gpios;
+	int			gpio_irq[UART_GPIO_MAX];
+	bool			ms_irq_enabled;
 };
 
 static struct platform_device_id mxs_auart_devtype[] = {
@@ -427,6 +433,29 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 	mctrl_gpio_set(s->gpios, mctrl);
 }
 
+#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
+static u32 mxs_auart_modem_status(struct mxs_auart_port *s, u32 mctrl)
+{
+	u32 mctrl_diff;
+
+	mctrl_diff = mctrl ^ s->mctrl_prev;
+	s->mctrl_prev = mctrl;
+	if (mctrl_diff & MCTRL_ANY_DELTA && s->ms_irq_enabled &&
+						s->port.state != NULL) {
+		if (mctrl_diff & TIOCM_RI)
+			s->port.icount.rng++;
+		if (mctrl_diff & TIOCM_DSR)
+			s->port.icount.dsr++;
+		if (mctrl_diff & TIOCM_CD)
+			uart_handle_dcd_change(&s->port, mctrl & TIOCM_CD);
+		if (mctrl_diff & TIOCM_CTS)
+			uart_handle_cts_change(&s->port, mctrl & TIOCM_CTS);
+
+		wake_up_interruptible(&s->port.state->port.delta_msr_wait);
+	}
+	return mctrl;
+}
+
 static u32 mxs_auart_get_mctrl(struct uart_port *u)
 {
 	struct mxs_auart_port *s = to_auart_port(u);
@@ -444,6 +473,64 @@ static u32 mxs_auart_get_mctrl(struct uart_port *u)
 	return mctrl_gpio_get(s->gpios, &mctrl);
 }
 
+/*
+ * Enable modem status interrupts
+ */
+static void mxs_auart_enable_ms(struct uart_port *port)
+{
+	struct mxs_auart_port *s = to_auart_port(port);
+
+	/*
+	 * Interrupt should not be enabled twice
+	 */
+	if (s->ms_irq_enabled)
+		return;
+
+	s->ms_irq_enabled = true;
+
+	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
+		enable_irq(s->gpio_irq[UART_GPIO_CTS]);
+	/* TODO: enable AUART_INTR_CTSMIEN otherwise */
+
+	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
+		enable_irq(s->gpio_irq[UART_GPIO_DSR]);
+
+	if (s->gpio_irq[UART_GPIO_RI] >= 0)
+		enable_irq(s->gpio_irq[UART_GPIO_RI]);
+
+	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
+		enable_irq(s->gpio_irq[UART_GPIO_DCD]);
+}
+
+/*
+ * Disable modem status interrupts
+ */
+static void mxs_auart_disable_ms(struct uart_port *port)
+{
+	struct mxs_auart_port *s = to_auart_port(port);
+
+	/*
+	 * Interrupt should not be disabled twice
+	 */
+	if (!s->ms_irq_enabled)
+		return;
+
+	s->ms_irq_enabled = false;
+
+	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
+		disable_irq(s->gpio_irq[UART_GPIO_CTS]);
+	/* TODO: disable AUART_INTR_CTSMIEN otherwise */
+
+	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
+		disable_irq(s->gpio_irq[UART_GPIO_DSR]);
+
+	if (s->gpio_irq[UART_GPIO_RI] >= 0)
+		disable_irq(s->gpio_irq[UART_GPIO_RI]);
+
+	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
+		disable_irq(s->gpio_irq[UART_GPIO_DCD]);
+}
+
 static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
 static void dma_rx_callback(void *arg)
 {
@@ -692,6 +779,12 @@ static void mxs_auart_settermios(struct uart_port *u,
 			dev_err(s->dev, "We can not start up the DMA.\n");
 		}
 	}
+
+	/* CTS flow-control and modem-status interrupts */
+	if (UART_ENABLE_MS(u, termios->c_cflag))
+		mxs_auart_enable_ms(u);
+	else
+		mxs_auart_disable_ms(u);
 }
 
 static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
@@ -709,9 +802,20 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
 		| AUART_INTR_CTSMIS),
 			s->port.membase + AUART_INTR_CLR);
 
+	/*
+	 * Dealing with GPIO interrupt
+	 */
+	if (irq == s->gpio_irq[UART_GPIO_CTS] ||
+	    irq == s->gpio_irq[UART_GPIO_DCD] ||
+	    irq == s->gpio_irq[UART_GPIO_DSR] ||
+	    irq == s->gpio_irq[UART_GPIO_RI])
+		mxs_auart_modem_status(s,
+				mctrl_gpio_get(s->gpios, &s->mctrl_prev));
+
 	if (istat & AUART_INTR_CTSMIS) {
 		if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
-						UART_GPIO_CTS)))
+							UART_GPIO_CTS))
+						&& s->ms_irq_enabled)
 			uart_handle_cts_change(&s->port,
 					stat & AUART_STAT_CTS);
 		writel(AUART_INTR_CTSMIS,
@@ -774,6 +878,10 @@ static int mxs_auart_startup(struct uart_port *u)
 	 */
 	writel(AUART_LINECTRL_FEN, u->membase + AUART_LINECTRL_SET);
 
+	/* get initial status of modem lines */
+	mctrl_gpio_get(s->gpios, &s->mctrl_prev);
+
+	s->ms_irq_enabled = false;
 	return 0;
 }
 
@@ -781,6 +889,8 @@ static void mxs_auart_shutdown(struct uart_port *u)
 {
 	struct mxs_auart_port *s = to_auart_port(u);
 
+	mxs_auart_disable_ms(u);
+
 	if (auart_dma_enabled(s))
 		mxs_auart_dma_exit(s);
 
@@ -837,6 +947,7 @@ static struct uart_ops mxs_auart_ops = {
 	.start_tx       = mxs_auart_start_tx,
 	.stop_tx	= mxs_auart_stop_tx,
 	.stop_rx	= mxs_auart_stop_rx,
+	.enable_ms      = mxs_auart_enable_ms,
 	.break_ctl      = mxs_auart_break_ctl,
 	.set_mctrl	= mxs_auart_set_mctrl,
 	.get_mctrl      = mxs_auart_get_mctrl,
@@ -1039,12 +1150,61 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
 
 static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
 {
+	enum mctrl_gpio_idx i;
+	struct gpio_desc *gpiod;
+
 	s->gpios = mctrl_gpio_init(dev, 0);
 	if (IS_ERR_OR_NULL(s->gpios))
 		return -1;
+
+	for (i = 0; i < UART_GPIO_MAX; i++) {
+		gpiod = mctrl_gpio_to_gpiod(s->gpios, i);
+		if (gpiod && (gpiod_get_direction(gpiod) == GPIOF_DIR_IN))
+			s->gpio_irq[i] = gpiod_to_irq(gpiod);
+		else
+			s->gpio_irq[i] = -EINVAL;
+	}
+
 	return 0;
 }
 
+static void mxs_auart_free_gpio_irq(struct mxs_auart_port *s)
+{
+	enum mctrl_gpio_idx i;
+
+	for (i = 0; i < UART_GPIO_MAX; i++)
+		if (s->gpio_irq[i] >= 0)
+			free_irq(s->gpio_irq[i], s);
+}
+
+static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
+{
+	int *irq = s->gpio_irq;
+	enum mctrl_gpio_idx i;
+	int err = 0;
+
+	for (i = 0; (i < UART_GPIO_MAX) && !err; i++) {
+		if (irq[i] < 0)
+			continue;
+
+		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
+		err = request_irq(irq[i], mxs_auart_irq_handle,
+				IRQ_TYPE_EDGE_BOTH, dev_name(s->dev), s);
+		if (err)
+			dev_err(s->dev, "%s - Can't get %d irq\n",
+				__func__, irq[i]);
+	}
+
+	/*
+	 * If something went wrong, rollback.
+	 */
+	while (err && (--i >= 0))
+		if (irq[i] >= 0)
+			free_irq(irq[i], s);
+
+	return err;
+}
+
 static int mxs_auart_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id =
@@ -1092,6 +1252,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	s->port.type = PORT_IMX;
 	s->port.dev = s->dev = &pdev->dev;
 
+	s->mctrl_prev = 0;
+
 	s->irq = platform_get_irq(pdev, 0);
 	s->port.irq = s->irq;
 	ret = request_irq(s->irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s);
@@ -1105,13 +1267,20 @@ static int mxs_auart_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "%s",
 			"Failed to initialize GPIOs. The serial port may not work as expected");
 
+	/*
+	 * Get the GPIO lines IRQ
+	 */
+	ret = mxs_auart_request_gpio_irq(s);
+	if (ret)
+		goto out_free_irq;
+
 	auart_port[s->port.line] = s;
 
 	mxs_auart_reset(&s->port);
 
 	ret = uart_add_one_port(&auart_driver, &s->port);
 	if (ret)
-		goto out_free_irq;
+		goto out_free_gpio_irq;
 
 	version = readl(s->port.membase + AUART_VERSION);
 	dev_info(&pdev->dev, "Found APPUART %d.%d.%d\n",
@@ -1120,6 +1289,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
 
 	return 0;
 
+out_free_gpio_irq:
+	mxs_auart_free_gpio_irq(s);
 out_free_irq:
 	auart_port[pdev->id] = NULL;
 	free_irq(s->irq, s);
@@ -1139,6 +1310,7 @@ static int mxs_auart_remove(struct platform_device *pdev)
 
 	auart_port[pdev->id] = NULL;
 
+	mxs_auart_free_gpio_irq(s);
 	clk_put(s->clk);
 	free_irq(s->irq, s);
 	kfree(s);
-- 
1.7.11.3

[PATCH 4/4] serial: mxs-auart: enable PPS support

[Janusz Uzycki]

Enables PPS support in mxs-auart serial driver to make PPS API working.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---
 drivers/tty/serial/mxs-auart.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index ab127a2..4f1ab98 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -787,6 +787,15 @@ static void mxs_auart_settermios(struct uart_port *u,
 		mxs_auart_disable_ms(u);
 }
 
+static void mxs_auart_set_ldisc(struct uart_port *port, int new)
+{
+	if (new == N_PPS) {
+		port->flags |= UPF_HARDPPS_CD;
+		mxs_auart_enable_ms(port);
+	} else
+		port->flags &= ~UPF_HARDPPS_CD;
+}
+
 static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
 {
 	u32 istat;
@@ -954,6 +963,7 @@ static struct uart_ops mxs_auart_ops = {
 	.startup	= mxs_auart_startup,
 	.shutdown       = mxs_auart_shutdown,
 	.set_termios    = mxs_auart_settermios,
+	.set_ldisc      = mxs_auart_set_ldisc,
 	.type	   	= mxs_auart_type,
 	.release_port   = mxs_auart_release_port,
 	.request_port   = mxs_auart_request_port,
-- 
1.7.11.3

Re: [PATCH 1/4] serial: mxs-auart: ctrl removed from mxs_auart_port

[Russell King - ARM Linux]

On Sat, Sep 27, 2014 at 11:46:34AM +0200, Janusz Uzycki wrote:
> The ctrl variable duplicated mctrl, member of uart_port structure
> in serial_core.h.
> The code duplicated uart_update_mctrl() and uart_tiocmget()
> in serial_core.c.
> mxs_auart_get_mctrl() reads back RTS line. It could be removed too
> but not sure.

The only thing which the .get_mctrl method is supposed to do is to return
the state of the /input/ lines, which are CTS, DCD, DSR, RI.  The output
line state is stored in port->mctrl, and is added to the returned value
by serial_core when it's required.  RTS output state should not be
returned from the .get_mctrl method.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/4] serial: mxs-auart: use mctrl_gpio helpers for handling modem signals

[Russell King - ARM Linux]

On Sat, Sep 27, 2014 at 11:46:35AM +0200, Janusz Uzycki wrote:
> @@ -635,7 +645,12 @@ static void mxs_auart_settermios(struct uart_port *u,
>  		ctrl |= AUART_LINECTRL_STP2;
>  
>  	/* figure out the hardware flow control settings */
> -	if (cflag & CRTSCTS) {
> +	/* FIXME: Likely DMA could be enabled not only for HW flow control */
> +	if (cflag & CRTSCTS &&
> +			(IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
> +					 UART_GPIO_RTS)) ||
> +			 IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
> +					 UART_GPIO_CTS)))) {

There has to be a better way to do this.

> +static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
> +{
> +	s->gpios = mctrl_gpio_init(dev, 0);
> +	if (IS_ERR_OR_NULL(s->gpios))
> +		return -1;
> +	return 0;

I personally hate the practise of returning -1 - it ends up getting returned
to userspace as an error code, and it means EPERM.  If you want to indicate
success/failure, use the bool return type, and return true/false.

> @@ -1074,6 +1100,11 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, s);
>  
> +	ret = mxs_auart_init_gpios(s, &pdev->dev);
> +	if (ret < 0)
> +		dev_err(&pdev->dev, "%s",
> +			"Failed to initialize GPIOs. The serial port may not work as expected");

Does the "%s" do anything for this message?  More importantly though, it's
lacking a newline.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/4] serial: mxs-auart: use mctrl_gpio helpers for handling modem signals

[Janusz Użycki]

Could somebody comment if DMA could be enabled also
if CTS/RTS is supported by GPIOs? Is it safe?
It would allow me to avoid ugly code in mxs_auart_settermios(),
as commented Russel King.
Otherwise I have to keep backward compatibility on DMA
and HW flow control, and fix the ugly code.

Marek Vasut:
"Why do you think DMA would do any good on long transfers without 
flowcontrol."

In past: "serial: mxs: enable the DMA only when the RTS/CTS is valid":
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/drivers/tty/serial/mxs-auart.c?id=8418e67d95235c3449df6f2e5b33863343fa72f9 

"The original DMA support works only when RTS/CTS is enabled.
(see the "e800163 serial: mxs-auart: add the DMA support for mx28")
But after several patches, DMA support has lost this limit.
(see the "bcc20f9 serial: mxs-auart: move to use generic DMA helper")

So an UART without the RTS/CTS lines may also enables the DMA support,
in which case the UART may gets unpredictable results.

This patch adds an optional property for the UART DT node
which indicates the UART has RTS and CTS lines, and it also means you
enable the DMA support for this UART.

This patch also adds a macro MXS_AUART_RTSCTS, and uses it to check
RTS/CTS before we enable the DMA for the UART. "

best regards
Janusz

W dniu 2014-09-27 11:46, Janusz Uzycki pisze:
> Dedicated CTS and RTS pins are unusable together with a lot of other
> peripherals because they share the same line. Pinctrl is limited.
>
> Moreover, the AUART controller doesn't handle DTR/DSR/DCD/RI signals,
> so we have to control them via GPIO.
>
> This patch permits to use GPIOs to control the CTS/RTS/DTR/DSR/DCD/RI
> signals.
>
> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
> ---
>   .../devicetree/bindings/serial/fsl-mxs-auart.txt   | 10 +++++-
>   drivers/tty/serial/Kconfig           |  1 +
>   drivers/tty/serial/mxs-auart.c       | 37 ++++++++++++++++++++--
>   3 files changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt b/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt
> index 59a40f1..7c408c8 100644
> --- a/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt
> +++ b/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt
> @@ -11,8 +11,13 @@ Required properties:
>   - dma-names: "rx" for RX channel, "tx" for TX channel.
>   
>   Optional properties:
> -- fsl,uart-has-rtscts : Indicate the UART has RTS and CTS lines,
> +- fsl,uart-has-rtscts : Indicate the UART has RTS and CTS lines
> +  for hardware flow control,
>   	it also means you enable the DMA support for this UART.
> +- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
> +  line respectively. It will use specified PIO instead of the peripheral
> +  function pin for the USART feature.
> +  If unsure, don't specify this property.
>   
>   Example:
>   auart0: serial@8006a000 {
> @@ -21,6 +26,9 @@ auart0: serial@8006a000 {
>   	interrupts = <112>;
>   	dmas = <&dma_apbx 8>, <&dma_apbx 9>;
>   	dma-names = "rx", "tx";
> +	cts-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
> +	dsr-gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
> +	dcd-gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
>   };
>   
>   Note: Each auart port should have an alias correctly numbered in "aliases"
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 4fe8ca1..90e8516 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1357,6 +1357,7 @@ config SERIAL_MXS_AUART
>   	depends on ARCH_MXS
>   	tristate "MXS AUART support"
>   	select SERIAL_CORE
> +	select SERIAL_MCTRL_GPIO if GPIOLIB
>   	help
>   	  This driver supports the MXS Application UART (AUART) port.
>   
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index c712108..fdfa8a9 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -42,6 +42,9 @@
>   
>   #include <asm/cacheflush.h>
>   
> +#include <linux/err.h>
> +#include "serial_mctrl_gpio.h"
> +
>   #define MXS_AUART_PORTS 5
>   #define MXS_AUART_FIFO_SIZE		16
>   
> @@ -158,6 +161,8 @@ struct mxs_auart_port {
>   	struct scatterlist rx_sgl;
>   	struct dma_chan	*rx_dma_chan;
>   	void *rx_dma_buf;
> +
> +	struct mctrl_gpios	*gpios;
>   };
>   
>   static struct platform_device_id mxs_auart_devtype[] = {
> @@ -405,6 +410,8 @@ static void mxs_auart_release_port(struct uart_port *u)
>   
>   static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
>   {
> +	struct mxs_auart_port *s = to_auart_port(u);
> +
>   	u32 ctrl = readl(u->membase + AUART_CTRL2);
>   
>   	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
> @@ -416,10 +423,13 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
>   	}
>   
>   	writel(ctrl, u->membase + AUART_CTRL2);
> +
> +	mctrl_gpio_set(s->gpios, mctrl);
>   }
>   
>   static u32 mxs_auart_get_mctrl(struct uart_port *u)
>   {
> +	struct mxs_auart_port *s = to_auart_port(u);
>   	u32 stat = readl(u->membase + AUART_STAT);
>   	int ctrl2 = readl(u->membase + AUART_CTRL2);
>   	u32 mctrl = 0;
> @@ -431,7 +441,7 @@ static u32 mxs_auart_get_mctrl(struct uart_port *u)
>   	if (ctrl2 & AUART_CTRL2_RTS)
>   		mctrl |= TIOCM_RTS;
>   
> -	return mctrl;
> +	return mctrl_gpio_get(s->gpios, &mctrl);
>   }
>   
>   static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
> @@ -635,7 +645,12 @@ static void mxs_auart_settermios(struct uart_port *u,
>   		ctrl |= AUART_LINECTRL_STP2;
>   
>   	/* figure out the hardware flow control settings */
> -	if (cflag & CRTSCTS) {
> +	/* FIXME: Likely DMA could be enabled not only for HW flow control */
> +	if (cflag & CRTSCTS &&
> +			(IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
> +					 UART_GPIO_RTS)) ||
> +			 IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
> +					 UART_GPIO_CTS)))) {
>   		/*
>   		 * The DMA has a bug(see errata:2836) in mx23.
>   		 * So we can not implement the DMA for auart in mx23,
> @@ -695,7 +710,10 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
>   			s->port.membase + AUART_INTR_CLR);
>   
>   	if (istat & AUART_INTR_CTSMIS) {
> -		uart_handle_cts_change(&s->port, stat & AUART_STAT_CTS);
> +		if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
> +						UART_GPIO_CTS)))
> +			uart_handle_cts_change(&s->port,
> +					stat & AUART_STAT_CTS);
>   		writel(AUART_INTR_CTSMIS,
>   				s->port.membase + AUART_INTR_CLR);
>   		istat &= ~AUART_INTR_CTSMIS;
> @@ -1019,6 +1037,14 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
>   	return 0;
>   }
>   
> +static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
> +{
> +	s->gpios = mctrl_gpio_init(dev, 0);
> +	if (IS_ERR_OR_NULL(s->gpios))
> +		return -1;
> +	return 0;
> +}
> +
>   static int mxs_auart_probe(struct platform_device *pdev)
>   {
>   	const struct of_device_id *of_id =
> @@ -1074,6 +1100,11 @@ static int mxs_auart_probe(struct platform_device *pdev)
>   
>   	platform_set_drvdata(pdev, s);
>   
> +	ret = mxs_auart_init_gpios(s, &pdev->dev);
> +	if (ret < 0)
> +		dev_err(&pdev->dev, "%s",
> +			"Failed to initialize GPIOs. The serial port may not work as expected");
> +
>   	auart_port[s->port.line] = s;
>   
>   	mxs_auart_reset(&s->port);

Re: [PATCH 2/4] serial: mxs-auart: use mctrl_gpio helpers for handling modem signals

[Russell King - ARM Linux]

On Sat, Sep 27, 2014 at 12:33:16PM +0200, Janusz Użycki wrote:
> Could somebody comment if DMA could be enabled also
> if CTS/RTS is supported by GPIOs? Is it safe?
> It would allow me to avoid ugly code in mxs_auart_settermios(),
> as commented Russel King.

Russel*L* please, otherwise you are giving me implicit permission to
mis-spell your name.

> Otherwise I have to keep backward compatibility on DMA
> and HW flow control, and fix the ugly code.
>
> Marek Vasut:
> "Why do you think DMA would do any good on long transfers without  
> flowcontrol."

The problem with DMA without hardware flow control is that the UART
transmission will run at full speed, with characters back to back.

You will have a variable amount of latency when dealing with the CTS
signal - interrupt handling latency in Linux can be quite variable.
(I'm not sure whether this has improved recently, USB used to be
particularly bad, causing 3+ ms latencies.)

When CTS is deasserted, you have to pause the transmission as soon as
possible to avoid overflowing the remote end.  At 115200 baud, each
character takes around 90us to be transmitted.  If you are delayed in
handling the CTS interrupt for 3ms, then you will have transmitted
around 35 characters in that time, which could be enough to cause an
overflow at the remote end.

Since the idea of flow control is to prevent overruns/overflow at the
receiver, this is far from ideal.

If Linux had a way to handle flow control interrupts as a higher
priority than other interrupts (especially interrupting an already
in-progress interrupt handler) then having GPIOs as flow control
signals would be a much saner proposition.

Unfortunately, lockdep completely gets in the way of that.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] serial: mxs-auart: use mctrl_gpio helpers for handling modem signals

[Janusz Użycki]

W dniu 2014-09-27 12:54, Russell King - ARM Linux pisze:
> On Sat, Sep 27, 2014 at 12:33:16PM +0200, Janusz Użycki wrote:
>> Could somebody comment if DMA could be enabled also
>> if CTS/RTS is supported by GPIOs? Is it safe?
>> It would allow me to avoid ugly code in mxs_auart_settermios(),
>> as commented Russel King.
> Russel*L* please, otherwise you are giving me implicit permission to
> mis-spell your name.

Sorry. I will care about.
Many people change my name also from Janusz to Januzs.

>> Otherwise I have to keep backward compatibility on DMA
>> and HW flow control, and fix the ugly code.
>>
>> Marek Vasut:
>> "Why do you think DMA would do any good on long transfers without
>> flowcontrol."
> The problem with DMA without hardware flow control is that the UART
> transmission will run at full speed, with characters back to back.
>
> You will have a variable amount of latency when dealing with the CTS
> signal - interrupt handling latency in Linux can be quite variable.
> (I'm not sure whether this has improved recently, USB used to be
> particularly bad, causing 3+ ms latencies.)
>
> When CTS is deasserted, you have to pause the transmission as soon as
> possible to avoid overflowing the remote end.  At 115200 baud, each
> character takes around 90us to be transmitted.  If you are delayed in
> handling the CTS interrupt for 3ms, then you will have transmitted
> around 35 characters in that time, which could be enough to cause an
> overflow at the remote end.
>
> Since the idea of flow control is to prevent overruns/overflow at the
> receiver, this is far from ideal.
>
> If Linux had a way to handle flow control interrupts as a higher
> priority than other interrupts (especially interrupting an already
> in-progress interrupt handler) then having GPIOs as flow control
> signals would be a much saner proposition.
>
> Unfortunately, lockdep completely gets in the way of that.
It means it is possible but not in Linux today?
I think the problem is not interrupt delay, which is often much below 100us
(we get ~10us delay on DCD).
USB or PCIe are exceptions  - interrupt is packed to frame by serialized
and even polled by USB controller.
Isn't the problem rather how to break DMA transmission fast?
Hardware flow control not always requires to stop transmission
immediately. Usually delay of some chars is acceptable if remote receiver
implements low/high water mark (overflow trigger lower than RX FIFO size).

best regards
Janusz

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

[PATCH] serial: mxs-auart: clean get_mctrl and set_mctrl

[Janusz Uzycki]

Russell King:
The only thing which the .get_mctrl method is supposed to do is
to return the state of the /input/ lines, which are CTS, DCD, DSR, RI.
The output line state is stored in port->mctrl, and is added to
the returned value by serial_core when it's required.
RTS output state should not be returned from the .get_mctrl method.

This patch removes ctrl variable from mxs_auart_port
and removes useless reading back RTS line.

The ctrl variable in mxs_auart_port duplicated mctrl,
member of uart_port structure in serial_core.h.
The removed code from mxs_auart_set_mctrl() and mxs_auart_get_mctrl
duplicated uart_update_mctrl() and uart_tiocmget()
in serial_core.c.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---
 drivers/tty/serial/mxs-auart.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index e838e84..2d49901 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -143,7 +143,6 @@ struct mxs_auart_port {
 #define MXS_AUART_DMA_RX_READY	3  /* bit 3 */
 #define MXS_AUART_RTSCTS	4  /* bit 4 */
 	unsigned long flags;
-	unsigned int ctrl;
 	enum mxs_auart_type devtype;
 
 	unsigned int irq;
@@ -406,8 +405,6 @@ static void mxs_auart_release_port(struct uart_port *u)
 
 static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 {
-	struct mxs_auart_port *s = to_auart_port(u);
-
 	u32 ctrl = readl(u->membase + AUART_CTRL2);
 
 	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
@@ -418,24 +415,17 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 			ctrl |= AUART_CTRL2_RTS;
 	}
 
-	s->ctrl = mctrl;
 	writel(ctrl, u->membase + AUART_CTRL2);
 }
 
 static u32 mxs_auart_get_mctrl(struct uart_port *u)
 {
-	struct mxs_auart_port *s = to_auart_port(u);
 	u32 stat = readl(u->membase + AUART_STAT);
-	int ctrl2 = readl(u->membase + AUART_CTRL2);
-	u32 mctrl = s->ctrl;
+	u32 mctrl = 0;
 
-	mctrl &= ~TIOCM_CTS;
 	if (stat & AUART_STAT_CTS)
 		mctrl |= TIOCM_CTS;
 
-	if (ctrl2 & AUART_CTRL2_RTS)
-		mctrl |= TIOCM_RTS;
-
 	return mctrl;
 }
 
@@ -1071,8 +1061,6 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	s->port.type = PORT_IMX;
 	s->port.dev = s->dev = &pdev->dev;
 
-	s->ctrl = 0;
-
 	s->irq = platform_get_irq(pdev, 0);
 	s->port.irq = s->irq;
 	ret = request_irq(s->irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s);
-- 
1.7.11.3

Re: [PATCH 2/4] serial: mxs-auart: use mctrl_gpio helpers for handling modem signals

[Russell King - ARM Linux]

On Sat, Sep 27, 2014 at 01:15:00PM +0200, Janusz Użycki wrote:
> It means it is possible but not in Linux today?

It depends on your maximum interrupt latency.

> I think the problem is not interrupt delay, which is often much below 100us
> (we get ~10us delay on DCD).

http://archive.arm.linux.org.uk/lurker/message/20130724.145238.44ad37b5.en.html

   I can believe why this all happens, when you see USB interrupts taking
   upwards of 3ms to complete:

   Longest time: 3247506ns
   Longest irq: 24
   Longest handler: usb_hcd_irq+0x0/0x68

This no longer seems to be the case - the maximum interrupt time I'm seeing
on recent kernels is:

   Longest time: 382236ns
   Longest irq: 62
   Longest handler: mv_interrupt+0x0/0x948

My Dove kernel runs permanently with my own maximum interrupt latency
tracking enabled.

> Isn't the problem rather how to break DMA transmission fast?

There are two factors there.  The first is being able to tell the DMA
engine to stop, the second is the UART hardware FIFO draining.

> Hardware flow control not always requires to stop transmission
> immediately.

No.  Hardware flow control is normally implemented at the UART, and
CTS is normally implemented to prevent the transmitter starting a new
character.  Any in-progress character is completed before the transmitter
stops, so that there are no errors.

At least, this is the behaviour found on the UARTs which I've seen
implementing hardware flow control, and it is the only sane way to do
it.

> Usually delay of some chars is acceptable if remote receiver
> implements low/high water mark (overflow trigger lower than RX FIFO size).

The receiver you may be communicating with may have a receive FIFO of
only 16 characters.  It may deassert CTS when it reaches half-full state.
That leaves you with only 8 characters to send before it overflows.  At
115200 baud, you will have started to send the 9th characters by 700us.

However, that isn't the full picture.  The full picture is that CTS is
normally controlled by software as well, and depends on the availability
of buffers to store characters.  There's latency at the remote end to
deassert CTS in software when the software buffers reach their high
watermark, and there's the hope that there is sufficient room in those
buffers to take the excess characters that the remote end may continue
to send.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] serial: mxs-auart: enable PPS support

[Sergei Shtylyov]

Hello.

On 9/27/2014 1:46 PM, Janusz Uzycki wrote:

> Enables PPS support in mxs-auart serial driver to make PPS API working.

> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
> ---
>   drivers/tty/serial/mxs-auart.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)

> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index ab127a2..4f1ab98 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -787,6 +787,15 @@ static void mxs_auart_settermios(struct uart_port *u,
>   		mxs_auart_disable_ms(u);
>   }
>
> +static void mxs_auart_set_ldisc(struct uart_port *port, int new)
> +{
> +	if (new == N_PPS) {
> +		port->flags |= UPF_HARDPPS_CD;
> +		mxs_auart_enable_ms(port);
> +	} else
> +		port->flags &= ~UPF_HARDPPS_CD;

    The kernel coding style dictates that {} should be in both arms of the 
*if* statement.

[...]

WBR, Sergei

[PATCH] serial: mxs-auart: enable PPS support

[Janusz Uzycki]

Enables PPS support in mxs-auart serial driver to make PPS API working.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---
 drivers/tty/serial/mxs-auart.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 4c238bf..fc262ba 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -782,6 +782,16 @@ static void mxs_auart_settermios(struct uart_port *u,
 		mxs_auart_disable_ms(u);
 }
 
+static void mxs_auart_set_ldisc(struct uart_port *port, int new)
+{
+	if (new == N_PPS) {
+		port->flags |= UPF_HARDPPS_CD;
+		mxs_auart_enable_ms(port);
+	} else {
+		port->flags &= ~UPF_HARDPPS_CD;
+	}
+}
+
 static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
 {
 	u32 istat;
@@ -949,6 +959,7 @@ static struct uart_ops mxs_auart_ops = {
 	.startup	= mxs_auart_startup,
 	.shutdown       = mxs_auart_shutdown,
 	.set_termios    = mxs_auart_settermios,
+	.set_ldisc      = mxs_auart_set_ldisc,
 	.type	   	= mxs_auart_type,
 	.release_port   = mxs_auart_release_port,
 	.request_port   = mxs_auart_request_port,
-- 
1.7.11.3

Re: serial: mxs-auart: gpios as modem signals (dirty)

[Janusz Uzycki]

Is it better in mxs_auart_init_gpios():
a) to block DMA by disabling MXS_AUART_RTSCTS (misleading name problem)
b) or just warn (depends on DT) if MXS_AUART_RTSCTS is set and CTS or RTS
   is defined as GPIO line? Then MXS_AUART_RTSCTS could be used
   instead of CTS_AT_AUART() in the interrupt and mxs_auart_settermios().

What do you think about RTSEN? It is set in mxs_auart_settermios()
and mxs_auart_set_mctrl().

[PATCH 1/2] serial: mxs-auart: use mctrl_gpio helpers for handling
* RTS_AT_AUART() and CTS_AT_AUART() macro defined
* DMA engine disabled if RTS or CTS is GPIO line
* CTSEN can't be enabled for hardware flow control block
  if CTS is defined as GPIO line
* RTSEN can be enabled for hardware flow control block
  even if RTS is defined as GPIO line.
  RTS pin depends on pinctrl configuration which
  selects RTS output from hardware flow control block or GPIO line.
* mxs_auart_settermios(): RTS_AT_AUART() and CTS_AT_AUART() used
* mxs_auart_irq_handle(): CTS_AT_AUART() used
* mxs_auart_init_gpios() returns true(success)/false(failure)
* dev_err() message fixed in mxs_auart_probe()

[PATCH 2/2] serial: mxs-auart: add interrupts for modem control
* rebased

[PATCH 1/2] serial: mxs-auart: use mctrl_gpio helpers for handling modem signals

[Janusz Uzycki]

Dedicated CTS and RTS pins are unusable together with a lot of other
peripherals because they share the same line. Pinctrl is limited.

Moreover, the AUART controller doesn't handle DTR/DSR/DCD/RI signals,
so we have to control them via GPIO.

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

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---
 .../devicetree/bindings/serial/fsl-mxs-auart.txt   | 10 ++++-
 drivers/tty/serial/Kconfig           |  1 +
 drivers/tty/serial/mxs-auart.c       | 46 +++++++++++++++++++---
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt b/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt
index 59a40f1..7c408c8 100644
--- a/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt
+++ b/Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt
@@ -11,8 +11,13 @@ Required properties:
 - dma-names: "rx" for RX channel, "tx" for TX channel.
 
 Optional properties:
-- fsl,uart-has-rtscts : Indicate the UART has RTS and CTS lines,
+- fsl,uart-has-rtscts : Indicate the UART has RTS and CTS lines
+  for hardware flow control,
 	it also means you enable the DMA support for this UART.
+- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
+  line respectively. It will use specified PIO instead of the peripheral
+  function pin for the USART feature.
+  If unsure, don't specify this property.
 
 Example:
 auart0: serial@8006a000 {
@@ -21,6 +26,9 @@ auart0: serial@8006a000 {
 	interrupts = <112>;
 	dmas = <&dma_apbx 8>, <&dma_apbx 9>;
 	dma-names = "rx", "tx";
+	cts-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
+	dsr-gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
+	dcd-gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
 };
 
 Note: Each auart port should have an alias correctly numbered in "aliases"
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 4fe8ca1..90e8516 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1357,6 +1357,7 @@ config SERIAL_MXS_AUART
 	depends on ARCH_MXS
 	tristate "MXS AUART support"
 	select SERIAL_CORE
+	select SERIAL_MCTRL_GPIO if GPIOLIB
 	help
 	  This driver supports the MXS Application UART (AUART) port.
 
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 2d49901..46fb8a6 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -42,6 +42,9 @@
 
 #include <asm/cacheflush.h>
 
+#include <linux/err.h>
+#include "serial_mctrl_gpio.h"
+
 #define MXS_AUART_PORTS 5
 #define MXS_AUART_FIFO_SIZE		16
 
@@ -158,6 +161,8 @@ struct mxs_auart_port {
 	struct scatterlist rx_sgl;
 	struct dma_chan	*rx_dma_chan;
 	void *rx_dma_buf;
+
+	struct mctrl_gpios	*gpios;
 };
 
 static struct platform_device_id mxs_auart_devtype[] = {
@@ -405,6 +410,8 @@ static void mxs_auart_release_port(struct uart_port *u)
 
 static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 {
+	struct mxs_auart_port *s = to_auart_port(u);
+
 	u32 ctrl = readl(u->membase + AUART_CTRL2);
 
 	ctrl &= ~(AUART_CTRL2_RTSEN | AUART_CTRL2_RTS);
@@ -416,17 +423,20 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 	}
 
 	writel(ctrl, u->membase + AUART_CTRL2);
+
+	mctrl_gpio_set(s->gpios, mctrl);
 }
 
 static u32 mxs_auart_get_mctrl(struct uart_port *u)
 {
+	struct mxs_auart_port *s = to_auart_port(u);
 	u32 stat = readl(u->membase + AUART_STAT);
 	u32 mctrl = 0;
 
 	if (stat & AUART_STAT_CTS)
 		mctrl |= TIOCM_CTS;
 
-	return mctrl;
+	return mctrl_gpio_get(s->gpios, &mctrl);
 }
 
 static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
@@ -554,6 +564,10 @@ err_out:
 
 }
 
+#define RTS_AT_AUART()	IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,	\
+							UART_GPIO_RTS))
+#define CTS_AT_AUART()	IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,	\
+							UART_GPIO_CTS))
 static void mxs_auart_settermios(struct uart_port *u,
 				 struct ktermios *termios,
 				 struct ktermios *old)
@@ -630,6 +644,7 @@ static void mxs_auart_settermios(struct uart_port *u,
 		ctrl |= AUART_LINECTRL_STP2;
 
 	/* figure out the hardware flow control settings */
+	ctrl2 &= ~(AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN);
 	if (cflag & CRTSCTS) {
 		/*
 		 * The DMA has a bug(see errata:2836) in mx23.
@@ -644,9 +659,11 @@ static void mxs_auart_settermios(struct uart_port *u,
 				ctrl2 |= AUART_CTRL2_TXDMAE | AUART_CTRL2_RXDMAE
 				       | AUART_CTRL2_DMAONERR;
 		}
-		ctrl2 |= AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN;
-	} else {
-		ctrl2 &= ~(AUART_CTRL2_CTSEN | AUART_CTRL2_RTSEN);
+		/* Even if RTS is GPIO line RTSEN can be enabled because
+		 * the pinctrl configuration decides about RTS pin function */
+		ctrl2 |= AUART_CTRL2_RTSEN;
+		if (CTS_AT_AUART())
+			ctrl2 |= AUART_CTRL2_CTSEN;
 	}
 
 	/* set baud rate */
@@ -690,7 +707,9 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
 			s->port.membase + AUART_INTR_CLR);
 
 	if (istat & AUART_INTR_CTSMIS) {
-		uart_handle_cts_change(&s->port, stat & AUART_STAT_CTS);
+		if (CTS_AT_AUART())
+			uart_handle_cts_change(&s->port,
+					stat & AUART_STAT_CTS);
 		writel(AUART_INTR_CTSMIS,
 				s->port.membase + AUART_INTR_CLR);
 		istat &= ~AUART_INTR_CTSMIS;
@@ -1014,6 +1033,19 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
 	return 0;
 }
 
+static bool mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
+{
+	s->gpios = mctrl_gpio_init(dev, 0);
+	if (IS_ERR_OR_NULL(s->gpios))
+		return false;
+
+	/* Block (enabled before) DMA option if RTS or CTS is GPIO line */
+	if (!RTS_AT_AUART() || !CTS_AT_AUART())
+		clear_bit(MXS_AUART_RTSCTS, &s->flags);
+
+	return true;
+}
+
 static int mxs_auart_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id =
@@ -1069,6 +1101,10 @@ static int mxs_auart_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, s);
 
+	if (!mxs_auart_init_gpios(s, &pdev->dev))
+		dev_err(&pdev->dev,
+			"Failed to initialize GPIOs. The serial port may not work as expected\n");
+
 	auart_port[s->port.line] = s;
 
 	mxs_auart_reset(&s->port);
-- 
1.7.11.3

[PATCH 2/2] serial: mxs-auart: add interrupts for modem control lines

[Janusz Uzycki]

Handle CTS/DSR/RI/DCD GPIO interrupts in mxs-auart.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---
 drivers/tty/serial/mxs-auart.c | 174 ++++++++++++++++++++++++++-
 1 file changed, 172 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 46fb8a6..3977bd3 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -42,7 +42,10 @@
 
 #include <asm/cacheflush.h>
 
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/err.h>
+#include <linux/irq.h>
 #include "serial_mctrl_gpio.h"
 
 #define MXS_AUART_PORTS 5
@@ -146,6 +149,7 @@ struct mxs_auart_port {
 #define MXS_AUART_DMA_RX_READY	3  /* bit 3 */
 #define MXS_AUART_RTSCTS	4  /* bit 4 */
 	unsigned long flags;
+	unsigned int mctrl_prev;
 	enum mxs_auart_type devtype;
 
 	unsigned int irq;
@@ -163,6 +167,8 @@ struct mxs_auart_port {
 	void *rx_dma_buf;
 
 	struct mctrl_gpios	*gpios;
+	int			gpio_irq[UART_GPIO_MAX];
+	bool			ms_irq_enabled;
 };
 
 static struct platform_device_id mxs_auart_devtype[] = {
@@ -427,6 +433,29 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 	mctrl_gpio_set(s->gpios, mctrl);
 }
 
+#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
+static u32 mxs_auart_modem_status(struct mxs_auart_port *s, u32 mctrl)
+{
+	u32 mctrl_diff;
+
+	mctrl_diff = mctrl ^ s->mctrl_prev;
+	s->mctrl_prev = mctrl;
+	if (mctrl_diff & MCTRL_ANY_DELTA && s->ms_irq_enabled &&
+						s->port.state != NULL) {
+		if (mctrl_diff & TIOCM_RI)
+			s->port.icount.rng++;
+		if (mctrl_diff & TIOCM_DSR)
+			s->port.icount.dsr++;
+		if (mctrl_diff & TIOCM_CD)
+			uart_handle_dcd_change(&s->port, mctrl & TIOCM_CD);
+		if (mctrl_diff & TIOCM_CTS)
+			uart_handle_cts_change(&s->port, mctrl & TIOCM_CTS);
+
+		wake_up_interruptible(&s->port.state->port.delta_msr_wait);
+	}
+	return mctrl;
+}
+
 static u32 mxs_auart_get_mctrl(struct uart_port *u)
 {
 	struct mxs_auart_port *s = to_auart_port(u);
@@ -439,6 +468,64 @@ static u32 mxs_auart_get_mctrl(struct uart_port *u)
 	return mctrl_gpio_get(s->gpios, &mctrl);
 }
 
+/*
+ * Enable modem status interrupts
+ */
+static void mxs_auart_enable_ms(struct uart_port *port)
+{
+	struct mxs_auart_port *s = to_auart_port(port);
+
+	/*
+	 * Interrupt should not be enabled twice
+	 */
+	if (s->ms_irq_enabled)
+		return;
+
+	s->ms_irq_enabled = true;
+
+	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
+		enable_irq(s->gpio_irq[UART_GPIO_CTS]);
+	/* TODO: enable AUART_INTR_CTSMIEN otherwise */
+
+	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
+		enable_irq(s->gpio_irq[UART_GPIO_DSR]);
+
+	if (s->gpio_irq[UART_GPIO_RI] >= 0)
+		enable_irq(s->gpio_irq[UART_GPIO_RI]);
+
+	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
+		enable_irq(s->gpio_irq[UART_GPIO_DCD]);
+}
+
+/*
+ * Disable modem status interrupts
+ */
+static void mxs_auart_disable_ms(struct uart_port *port)
+{
+	struct mxs_auart_port *s = to_auart_port(port);
+
+	/*
+	 * Interrupt should not be disabled twice
+	 */
+	if (!s->ms_irq_enabled)
+		return;
+
+	s->ms_irq_enabled = false;
+
+	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
+		disable_irq(s->gpio_irq[UART_GPIO_CTS]);
+	/* TODO: disable AUART_INTR_CTSMIEN otherwise */
+
+	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
+		disable_irq(s->gpio_irq[UART_GPIO_DSR]);
+
+	if (s->gpio_irq[UART_GPIO_RI] >= 0)
+		disable_irq(s->gpio_irq[UART_GPIO_RI]);
+
+	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
+		disable_irq(s->gpio_irq[UART_GPIO_DCD]);
+}
+
 static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
 static void dma_rx_callback(void *arg)
 {
@@ -689,6 +776,12 @@ static void mxs_auart_settermios(struct uart_port *u,
 			dev_err(s->dev, "We can not start up the DMA.\n");
 		}
 	}
+
+	/* CTS flow-control and modem-status interrupts */
+	if (UART_ENABLE_MS(u, termios->c_cflag))
+		mxs_auart_enable_ms(u);
+	else
+		mxs_auart_disable_ms(u);
 }
 
 static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
@@ -706,8 +799,18 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
 		| AUART_INTR_CTSMIS),
 			s->port.membase + AUART_INTR_CLR);
 
+	/*
+	 * Dealing with GPIO interrupt
+	 */
+	if (irq == s->gpio_irq[UART_GPIO_CTS] ||
+	    irq == s->gpio_irq[UART_GPIO_DCD] ||
+	    irq == s->gpio_irq[UART_GPIO_DSR] ||
+	    irq == s->gpio_irq[UART_GPIO_RI])
+		mxs_auart_modem_status(s,
+				mctrl_gpio_get(s->gpios, &s->mctrl_prev));
+
 	if (istat & AUART_INTR_CTSMIS) {
-		if (CTS_AT_AUART())
+		if (CTS_AT_AUART() && s->ms_irq_enabled)
 			uart_handle_cts_change(&s->port,
 					stat & AUART_STAT_CTS);
 		writel(AUART_INTR_CTSMIS,
@@ -770,6 +873,10 @@ static int mxs_auart_startup(struct uart_port *u)
 	 */
 	writel(AUART_LINECTRL_FEN, u->membase + AUART_LINECTRL_SET);
 
+	/* get initial status of modem lines */
+	mctrl_gpio_get(s->gpios, &s->mctrl_prev);
+
+	s->ms_irq_enabled = false;
 	return 0;
 }
 
@@ -777,6 +884,8 @@ static void mxs_auart_shutdown(struct uart_port *u)
 {
 	struct mxs_auart_port *s = to_auart_port(u);
 
+	mxs_auart_disable_ms(u);
+
 	if (auart_dma_enabled(s))
 		mxs_auart_dma_exit(s);
 
@@ -833,6 +942,7 @@ static struct uart_ops mxs_auart_ops = {
 	.start_tx       = mxs_auart_start_tx,
 	.stop_tx	= mxs_auart_stop_tx,
 	.stop_rx	= mxs_auart_stop_rx,
+	.enable_ms      = mxs_auart_enable_ms,
 	.break_ctl      = mxs_auart_break_ctl,
 	.set_mctrl	= mxs_auart_set_mctrl,
 	.get_mctrl      = mxs_auart_get_mctrl,
@@ -1035,6 +1145,9 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
 
 static bool mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
 {
+	enum mctrl_gpio_idx i;
+	struct gpio_desc *gpiod;
+
 	s->gpios = mctrl_gpio_init(dev, 0);
 	if (IS_ERR_OR_NULL(s->gpios))
 		return false;
@@ -1043,9 +1156,54 @@ static bool mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
 	if (!RTS_AT_AUART() || !CTS_AT_AUART())
 		clear_bit(MXS_AUART_RTSCTS, &s->flags);
 
+	for (i = 0; i < UART_GPIO_MAX; i++) {
+		gpiod = mctrl_gpio_to_gpiod(s->gpios, i);
+		if (gpiod && (gpiod_get_direction(gpiod) == GPIOF_DIR_IN))
+			s->gpio_irq[i] = gpiod_to_irq(gpiod);
+		else
+			s->gpio_irq[i] = -EINVAL;
+	}
+
 	return true;
 }
 
+static void mxs_auart_free_gpio_irq(struct mxs_auart_port *s)
+{
+	enum mctrl_gpio_idx i;
+
+	for (i = 0; i < UART_GPIO_MAX; i++)
+		if (s->gpio_irq[i] >= 0)
+			free_irq(s->gpio_irq[i], s);
+}
+
+static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
+{
+	int *irq = s->gpio_irq;
+	enum mctrl_gpio_idx i;
+	int err = 0;
+
+	for (i = 0; (i < UART_GPIO_MAX) && !err; i++) {
+		if (irq[i] < 0)
+			continue;
+
+		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
+		err = request_irq(irq[i], mxs_auart_irq_handle,
+				IRQ_TYPE_EDGE_BOTH, dev_name(s->dev), s);
+		if (err)
+			dev_err(s->dev, "%s - Can't get %d irq\n",
+				__func__, irq[i]);
+	}
+
+	/*
+	 * If something went wrong, rollback.
+	 */
+	while (err && (--i >= 0))
+		if (irq[i] >= 0)
+			free_irq(irq[i], s);
+
+	return err;
+}
+
 static int mxs_auart_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id =
@@ -1093,6 +1251,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	s->port.type = PORT_IMX;
 	s->port.dev = s->dev = &pdev->dev;
 
+	s->mctrl_prev = 0;
+
 	s->irq = platform_get_irq(pdev, 0);
 	s->port.irq = s->irq;
 	ret = request_irq(s->irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s);
@@ -1105,13 +1265,20 @@ static int mxs_auart_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev,
 			"Failed to initialize GPIOs. The serial port may not work as expected\n");
 
+	/*
+	 * Get the GPIO lines IRQ
+	 */
+	ret = mxs_auart_request_gpio_irq(s);
+	if (ret)
+		goto out_free_irq;
+
 	auart_port[s->port.line] = s;
 
 	mxs_auart_reset(&s->port);
 
 	ret = uart_add_one_port(&auart_driver, &s->port);
 	if (ret)
-		goto out_free_irq;
+		goto out_free_gpio_irq;
 
 	version = readl(s->port.membase + AUART_VERSION);
 	dev_info(&pdev->dev, "Found APPUART %d.%d.%d\n",
@@ -1120,6 +1287,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
 
 	return 0;
 
+out_free_gpio_irq:
+	mxs_auart_free_gpio_irq(s);
 out_free_irq:
 	auart_port[pdev->id] = NULL;
 	free_irq(s->irq, s);
@@ -1139,6 +1308,7 @@ static int mxs_auart_remove(struct platform_device *pdev)
 
 	auart_port[pdev->id] = NULL;
 
+	mxs_auart_free_gpio_irq(s);
 	clk_put(s->clk);
 	free_irq(s->irq, s);
 	kfree(s);
-- 
1.7.11.3

Re: [PATCH 3/4] serial: mxs-auart: add interrupts for modem control lines

[Richard Genoud]

On 27/09/2014 11:46, Janusz Uzycki wrote:
> Handle CTS/DSR/RI/DCD GPIO interrupts in mxs-auart.
> 
> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
> ---
>  drivers/tty/serial/mxs-auart.c | 176 ++++++++++++++++++++++++++-
>  1 file changed, 174 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index fdfa8a9..ab127a2 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -42,7 +42,10 @@
>  
>  #include <asm/cacheflush.h>
>  
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/err.h>
> +#include <linux/irq.h>
>  #include "serial_mctrl_gpio.h"
>  
>  #define MXS_AUART_PORTS 5
> @@ -146,6 +149,7 @@ struct mxs_auart_port {
>  #define MXS_AUART_DMA_RX_READY	3  /* bit 3 */
>  #define MXS_AUART_RTSCTS	4  /* bit 4 */
>  	unsigned long flags;
> +	unsigned int mctrl_prev;
>  	enum mxs_auart_type devtype;
>  
>  	unsigned int irq;
> @@ -163,6 +167,8 @@ struct mxs_auart_port {
>  	void *rx_dma_buf;
>  
>  	struct mctrl_gpios	*gpios;
> +	int			gpio_irq[UART_GPIO_MAX];
> +	bool			ms_irq_enabled;
>  };
>  
>  static struct platform_device_id mxs_auart_devtype[] = {
> @@ -427,6 +433,29 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
>  	mctrl_gpio_set(s->gpios, mctrl);
>  }
>  
> +#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
> +static u32 mxs_auart_modem_status(struct mxs_auart_port *s, u32 mctrl)
> +{
> +	u32 mctrl_diff;
> +
> +	mctrl_diff = mctrl ^ s->mctrl_prev;
> +	s->mctrl_prev = mctrl;
> +	if (mctrl_diff & MCTRL_ANY_DELTA && s->ms_irq_enabled &&
> +						s->port.state != NULL) {
> +		if (mctrl_diff & TIOCM_RI)
> +			s->port.icount.rng++;
> +		if (mctrl_diff & TIOCM_DSR)
> +			s->port.icount.dsr++;
> +		if (mctrl_diff & TIOCM_CD)
> +			uart_handle_dcd_change(&s->port, mctrl & TIOCM_CD);
> +		if (mctrl_diff & TIOCM_CTS)
> +			uart_handle_cts_change(&s->port, mctrl & TIOCM_CTS);
> +
> +		wake_up_interruptible(&s->port.state->port.delta_msr_wait);
> +	}
> +	return mctrl;
> +}
> +
>  static u32 mxs_auart_get_mctrl(struct uart_port *u)
>  {
>  	struct mxs_auart_port *s = to_auart_port(u);
> @@ -444,6 +473,64 @@ static u32 mxs_auart_get_mctrl(struct uart_port *u)
>  	return mctrl_gpio_get(s->gpios, &mctrl);
>  }
>  
> +/*
> + * Enable modem status interrupts
> + */
> +static void mxs_auart_enable_ms(struct uart_port *port)
> +{
> +	struct mxs_auart_port *s = to_auart_port(port);
> +
> +	/*
> +	 * Interrupt should not be enabled twice
> +	 */
> +	if (s->ms_irq_enabled)
> +		return;
> +
> +	s->ms_irq_enabled = true;
> +
> +	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
> +		enable_irq(s->gpio_irq[UART_GPIO_CTS]);
> +	/* TODO: enable AUART_INTR_CTSMIEN otherwise */
> +
> +	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
> +		enable_irq(s->gpio_irq[UART_GPIO_DSR]);
> +
> +	if (s->gpio_irq[UART_GPIO_RI] >= 0)
> +		enable_irq(s->gpio_irq[UART_GPIO_RI]);
> +
> +	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
> +		enable_irq(s->gpio_irq[UART_GPIO_DCD]);
> +}
> +
> +/*
> + * Disable modem status interrupts
> + */
> +static void mxs_auart_disable_ms(struct uart_port *port)
> +{
> +	struct mxs_auart_port *s = to_auart_port(port);
> +
> +	/*
> +	 * Interrupt should not be disabled twice
> +	 */
> +	if (!s->ms_irq_enabled)
> +		return;
> +
> +	s->ms_irq_enabled = false;
> +
> +	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
> +		disable_irq(s->gpio_irq[UART_GPIO_CTS]);
> +	/* TODO: disable AUART_INTR_CTSMIEN otherwise */
> +
> +	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
> +		disable_irq(s->gpio_irq[UART_GPIO_DSR]);
> +
> +	if (s->gpio_irq[UART_GPIO_RI] >= 0)
> +		disable_irq(s->gpio_irq[UART_GPIO_RI]);
> +
> +	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
> +		disable_irq(s->gpio_irq[UART_GPIO_DCD]);
> +}
> +
>  static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
>  static void dma_rx_callback(void *arg)
>  {
> @@ -692,6 +779,12 @@ static void mxs_auart_settermios(struct uart_port *u,
>  			dev_err(s->dev, "We can not start up the DMA.\n");
>  		}
>  	}
> +
> +	/* CTS flow-control and modem-status interrupts */
> +	if (UART_ENABLE_MS(u, termios->c_cflag))
> +		mxs_auart_enable_ms(u);
> +	else
> +		mxs_auart_disable_ms(u);
>  }
>  
>  static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
> @@ -709,9 +802,20 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
>  		| AUART_INTR_CTSMIS),
>  			s->port.membase + AUART_INTR_CLR);
>  
> +	/*
> +	 * Dealing with GPIO interrupt
> +	 */
> +	if (irq == s->gpio_irq[UART_GPIO_CTS] ||
> +	    irq == s->gpio_irq[UART_GPIO_DCD] ||
> +	    irq == s->gpio_irq[UART_GPIO_DSR] ||
> +	    irq == s->gpio_irq[UART_GPIO_RI])
> +		mxs_auart_modem_status(s,
> +				mctrl_gpio_get(s->gpios, &s->mctrl_prev));
> +
>  	if (istat & AUART_INTR_CTSMIS) {
>  		if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
> -						UART_GPIO_CTS)))
> +							UART_GPIO_CTS))
> +						&& s->ms_irq_enabled)
>  			uart_handle_cts_change(&s->port,
>  					stat & AUART_STAT_CTS);
>  		writel(AUART_INTR_CTSMIS,
> @@ -774,6 +878,10 @@ static int mxs_auart_startup(struct uart_port *u)
>  	 */
>  	writel(AUART_LINECTRL_FEN, u->membase + AUART_LINECTRL_SET);
>  
> +	/* get initial status of modem lines */
> +	mctrl_gpio_get(s->gpios, &s->mctrl_prev);
> +
> +	s->ms_irq_enabled = false;
>  	return 0;
>  }
>  
> @@ -781,6 +889,8 @@ static void mxs_auart_shutdown(struct uart_port *u)
>  {
>  	struct mxs_auart_port *s = to_auart_port(u);
>  
> +	mxs_auart_disable_ms(u);
> +
>  	if (auart_dma_enabled(s))
>  		mxs_auart_dma_exit(s);
>  
> @@ -837,6 +947,7 @@ static struct uart_ops mxs_auart_ops = {
>  	.start_tx       = mxs_auart_start_tx,
>  	.stop_tx	= mxs_auart_stop_tx,
>  	.stop_rx	= mxs_auart_stop_rx,
> +	.enable_ms      = mxs_auart_enable_ms,
>  	.break_ctl      = mxs_auart_break_ctl,
>  	.set_mctrl	= mxs_auart_set_mctrl,
>  	.get_mctrl      = mxs_auart_get_mctrl,
> @@ -1039,12 +1150,61 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
>  
>  static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
>  {
> +	enum mctrl_gpio_idx i;
> +	struct gpio_desc *gpiod;
> +
>  	s->gpios = mctrl_gpio_init(dev, 0);
>  	if (IS_ERR_OR_NULL(s->gpios))
>  		return -1;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++) {
> +		gpiod = mctrl_gpio_to_gpiod(s->gpios, i);
> +		if (gpiod && (gpiod_get_direction(gpiod) == GPIOF_DIR_IN))
> +			s->gpio_irq[i] = gpiod_to_irq(gpiod);
> +		else
> +			s->gpio_irq[i] = -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  
> +static void mxs_auart_free_gpio_irq(struct mxs_auart_port *s)
> +{
> +	enum mctrl_gpio_idx i;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++)
> +		if (s->gpio_irq[i] >= 0)
> +			free_irq(s->gpio_irq[i], s);
> +}
> +
> +static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
> +{
> +	int *irq = s->gpio_irq;
> +	enum mctrl_gpio_idx i;
> +	int err = 0;
> +
> +	for (i = 0; (i < UART_GPIO_MAX) && !err; i++) {
> +		if (irq[i] < 0)
> +			continue;
> +
> +		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
> +		err = request_irq(irq[i], mxs_auart_irq_handle,
> +				IRQ_TYPE_EDGE_BOTH, dev_name(s->dev), s);
> +		if (err)
> +			dev_err(s->dev, "%s - Can't get %d irq\n",
> +				__func__, irq[i]);
> +	}
> +
> +	/*
> +	 * If something went wrong, rollback.
> +	 */
> +	while (err && (--i >= 0))
> +		if (irq[i] >= 0)
> +			free_irq(irq[i], s);
> +
> +	return err;
> +}
> +
>  static int mxs_auart_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *of_id =
> @@ -1092,6 +1252,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	s->port.type = PORT_IMX;
>  	s->port.dev = s->dev = &pdev->dev;
>  
> +	s->mctrl_prev = 0;
> +
>  	s->irq = platform_get_irq(pdev, 0);
>  	s->port.irq = s->irq;
>  	ret = request_irq(s->irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s);
> @@ -1105,13 +1267,20 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "%s",
>  			"Failed to initialize GPIOs. The serial port may not work as expected");
>  
> +	/*
> +	 * Get the GPIO lines IRQ
> +	 */
> +	ret = mxs_auart_request_gpio_irq(s);
> +	if (ret)
> +		goto out_free_irq;
> +
>  	auart_port[s->port.line] = s;
>  
>  	mxs_auart_reset(&s->port);
>  
>  	ret = uart_add_one_port(&auart_driver, &s->port);
>  	if (ret)
> -		goto out_free_irq;
> +		goto out_free_gpio_irq;
>  
>  	version = readl(s->port.membase + AUART_VERSION);
>  	dev_info(&pdev->dev, "Found APPUART %d.%d.%d\n",
> @@ -1120,6 +1289,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> +out_free_gpio_irq:
> +	mxs_auart_free_gpio_irq(s);
>  out_free_irq:
>  	auart_port[pdev->id] = NULL;
>  	free_irq(s->irq, s);
> @@ -1139,6 +1310,7 @@ static int mxs_auart_remove(struct platform_device *pdev)
>  
>  	auart_port[pdev->id] = NULL;
>  
> +	mxs_auart_free_gpio_irq(s);
>  	clk_put(s->clk);
>  	free_irq(s->irq, s);
>  	kfree(s);
> 
Seems also ok.
Janusz, will you provide a patch to complete {en,dis}able_ms functions
for non-gpio ?

Reviewed-by: Richard Genoud <richard.genoud@gmail.com>

Re: [PATCH 3/4] serial: mxs-auart: add interrupts for modem control lines

[Janusz Użycki]

W dniu 2014-10-24 o 17:32, Richard Genoud pisze:
> On 27/09/2014 11:46, Janusz Uzycki wrote:
>> Handle CTS/DSR/RI/DCD GPIO interrupts in mxs-auart.
>>
>> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
>> ---
>>   drivers/tty/serial/mxs-auart.c | 176 ++++++++++++++++++++++++++-
>>   1 file changed, 174 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
>> index fdfa8a9..ab127a2 100644
>> --- a/drivers/tty/serial/mxs-auart.c
>> +++ b/drivers/tty/serial/mxs-auart.c
>> @@ -42,7 +42,10 @@
>>   
>>   #include <asm/cacheflush.h>
>>   
>> +#include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>>   #include <linux/err.h>
>> +#include <linux/irq.h>
>>   #include "serial_mctrl_gpio.h"
>>   
>>   #define MXS_AUART_PORTS 5
>> @@ -146,6 +149,7 @@ struct mxs_auart_port {
>>   #define MXS_AUART_DMA_RX_READY	3  /* bit 3 */
>>   #define MXS_AUART_RTSCTS	4  /* bit 4 */
>>   	unsigned long flags;
>> +	unsigned int mctrl_prev;
>>   	enum mxs_auart_type devtype;
>>   
>>   	unsigned int irq;
>> @@ -163,6 +167,8 @@ struct mxs_auart_port {
>>   	void *rx_dma_buf;
>>   
>>   	struct mctrl_gpios	*gpios;
>> +	int			gpio_irq[UART_GPIO_MAX];
>> +	bool			ms_irq_enabled;
>>   };
>>   
>>   static struct platform_device_id mxs_auart_devtype[] = {
>> @@ -427,6 +433,29 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
>>   	mctrl_gpio_set(s->gpios, mctrl);
>>   }
>>   
>> +#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
>> +static u32 mxs_auart_modem_status(struct mxs_auart_port *s, u32 mctrl)
>> +{
>> +	u32 mctrl_diff;
>> +
>> +	mctrl_diff = mctrl ^ s->mctrl_prev;
>> +	s->mctrl_prev = mctrl;
>> +	if (mctrl_diff & MCTRL_ANY_DELTA && s->ms_irq_enabled &&
>> +						s->port.state != NULL) {
>> +		if (mctrl_diff & TIOCM_RI)
>> +			s->port.icount.rng++;
>> +		if (mctrl_diff & TIOCM_DSR)
>> +			s->port.icount.dsr++;
>> +		if (mctrl_diff & TIOCM_CD)
>> +			uart_handle_dcd_change(&s->port, mctrl & TIOCM_CD);
>> +		if (mctrl_diff & TIOCM_CTS)
>> +			uart_handle_cts_change(&s->port, mctrl & TIOCM_CTS);
>> +
>> +		wake_up_interruptible(&s->port.state->port.delta_msr_wait);
>> +	}
>> +	return mctrl;
>> +}
>> +
>>   static u32 mxs_auart_get_mctrl(struct uart_port *u)
>>   {
>>   	struct mxs_auart_port *s = to_auart_port(u);
>> @@ -444,6 +473,64 @@ static u32 mxs_auart_get_mctrl(struct uart_port *u)
>>   	return mctrl_gpio_get(s->gpios, &mctrl);
>>   }
>>   
>> +/*
>> + * Enable modem status interrupts
>> + */
>> +static void mxs_auart_enable_ms(struct uart_port *port)
>> +{
>> +	struct mxs_auart_port *s = to_auart_port(port);
>> +
>> +	/*
>> +	 * Interrupt should not be enabled twice
>> +	 */
>> +	if (s->ms_irq_enabled)
>> +		return;
>> +
>> +	s->ms_irq_enabled = true;
>> +
>> +	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
>> +		enable_irq(s->gpio_irq[UART_GPIO_CTS]);
>> +	/* TODO: enable AUART_INTR_CTSMIEN otherwise */
>> +
>> +	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
>> +		enable_irq(s->gpio_irq[UART_GPIO_DSR]);
>> +
>> +	if (s->gpio_irq[UART_GPIO_RI] >= 0)
>> +		enable_irq(s->gpio_irq[UART_GPIO_RI]);
>> +
>> +	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
>> +		enable_irq(s->gpio_irq[UART_GPIO_DCD]);
>> +}
>> +
>> +/*
>> + * Disable modem status interrupts
>> + */
>> +static void mxs_auart_disable_ms(struct uart_port *port)
>> +{
>> +	struct mxs_auart_port *s = to_auart_port(port);
>> +
>> +	/*
>> +	 * Interrupt should not be disabled twice
>> +	 */
>> +	if (!s->ms_irq_enabled)
>> +		return;
>> +
>> +	s->ms_irq_enabled = false;
>> +
>> +	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
>> +		disable_irq(s->gpio_irq[UART_GPIO_CTS]);
>> +	/* TODO: disable AUART_INTR_CTSMIEN otherwise */
>> +
>> +	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
>> +		disable_irq(s->gpio_irq[UART_GPIO_DSR]);
>> +
>> +	if (s->gpio_irq[UART_GPIO_RI] >= 0)
>> +		disable_irq(s->gpio_irq[UART_GPIO_RI]);
>> +
>> +	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
>> +		disable_irq(s->gpio_irq[UART_GPIO_DCD]);
>> +}
>> +
>>   static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
>>   static void dma_rx_callback(void *arg)
>>   {
>> @@ -692,6 +779,12 @@ static void mxs_auart_settermios(struct uart_port *u,
>>   			dev_err(s->dev, "We can not start up the DMA.\n");
>>   		}
>>   	}
>> +
>> +	/* CTS flow-control and modem-status interrupts */
>> +	if (UART_ENABLE_MS(u, termios->c_cflag))
>> +		mxs_auart_enable_ms(u);
>> +	else
>> +		mxs_auart_disable_ms(u);
>>   }
>>   
>>   static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
>> @@ -709,9 +802,20 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
>>   		| AUART_INTR_CTSMIS),
>>   			s->port.membase + AUART_INTR_CLR);
>>   
>> +	/*
>> +	 * Dealing with GPIO interrupt
>> +	 */
>> +	if (irq == s->gpio_irq[UART_GPIO_CTS] ||
>> +	    irq == s->gpio_irq[UART_GPIO_DCD] ||
>> +	    irq == s->gpio_irq[UART_GPIO_DSR] ||
>> +	    irq == s->gpio_irq[UART_GPIO_RI])
>> +		mxs_auart_modem_status(s,
>> +				mctrl_gpio_get(s->gpios, &s->mctrl_prev));
>> +
>>   	if (istat & AUART_INTR_CTSMIS) {
>>   		if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,
>> -						UART_GPIO_CTS)))
>> +							UART_GPIO_CTS))
>> +						&& s->ms_irq_enabled)
>>   			uart_handle_cts_change(&s->port,
>>   					stat & AUART_STAT_CTS);
>>   		writel(AUART_INTR_CTSMIS,
>> @@ -774,6 +878,10 @@ static int mxs_auart_startup(struct uart_port *u)
>>   	 */
>>   	writel(AUART_LINECTRL_FEN, u->membase + AUART_LINECTRL_SET);
>>   
>> +	/* get initial status of modem lines */
>> +	mctrl_gpio_get(s->gpios, &s->mctrl_prev);
>> +
>> +	s->ms_irq_enabled = false;
>>   	return 0;
>>   }
>>   
>> @@ -781,6 +889,8 @@ static void mxs_auart_shutdown(struct uart_port *u)
>>   {
>>   	struct mxs_auart_port *s = to_auart_port(u);
>>   
>> +	mxs_auart_disable_ms(u);
>> +
>>   	if (auart_dma_enabled(s))
>>   		mxs_auart_dma_exit(s);
>>   
>> @@ -837,6 +947,7 @@ static struct uart_ops mxs_auart_ops = {
>>   	.start_tx       = mxs_auart_start_tx,
>>   	.stop_tx	= mxs_auart_stop_tx,
>>   	.stop_rx	= mxs_auart_stop_rx,
>> +	.enable_ms      = mxs_auart_enable_ms,
>>   	.break_ctl      = mxs_auart_break_ctl,
>>   	.set_mctrl	= mxs_auart_set_mctrl,
>>   	.get_mctrl      = mxs_auart_get_mctrl,
>> @@ -1039,12 +1150,61 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
>>   
>>   static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
>>   {
>> +	enum mctrl_gpio_idx i;
>> +	struct gpio_desc *gpiod;
>> +
>>   	s->gpios = mctrl_gpio_init(dev, 0);
>>   	if (IS_ERR_OR_NULL(s->gpios))
>>   		return -1;
>> +
>> +	for (i = 0; i < UART_GPIO_MAX; i++) {
>> +		gpiod = mctrl_gpio_to_gpiod(s->gpios, i);
>> +		if (gpiod && (gpiod_get_direction(gpiod) == GPIOF_DIR_IN))
>> +			s->gpio_irq[i] = gpiod_to_irq(gpiod);
>> +		else
>> +			s->gpio_irq[i] = -EINVAL;
>> +	}
>> +
>>   	return 0;
>>   }
>>   
>> +static void mxs_auart_free_gpio_irq(struct mxs_auart_port *s)
>> +{
>> +	enum mctrl_gpio_idx i;
>> +
>> +	for (i = 0; i < UART_GPIO_MAX; i++)
>> +		if (s->gpio_irq[i] >= 0)
>> +			free_irq(s->gpio_irq[i], s);
>> +}
>> +
>> +static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
>> +{
>> +	int *irq = s->gpio_irq;
>> +	enum mctrl_gpio_idx i;
>> +	int err = 0;
>> +
>> +	for (i = 0; (i < UART_GPIO_MAX) && !err; i++) {
>> +		if (irq[i] < 0)
>> +			continue;
>> +
>> +		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
>> +		err = request_irq(irq[i], mxs_auart_irq_handle,
>> +				IRQ_TYPE_EDGE_BOTH, dev_name(s->dev), s);
>> +		if (err)
>> +			dev_err(s->dev, "%s - Can't get %d irq\n",
>> +				__func__, irq[i]);
>> +	}
>> +
>> +	/*
>> +	 * If something went wrong, rollback.
>> +	 */
>> +	while (err && (--i >= 0))
>> +		if (irq[i] >= 0)
>> +			free_irq(irq[i], s);
>> +
>> +	return err;
>> +}
>> +
>>   static int mxs_auart_probe(struct platform_device *pdev)
>>   {
>>   	const struct of_device_id *of_id =
>> @@ -1092,6 +1252,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
>>   	s->port.type = PORT_IMX;
>>   	s->port.dev = s->dev = &pdev->dev;
>>   
>> +	s->mctrl_prev = 0;
>> +
>>   	s->irq = platform_get_irq(pdev, 0);
>>   	s->port.irq = s->irq;
>>   	ret = request_irq(s->irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s);
>> @@ -1105,13 +1267,20 @@ static int mxs_auart_probe(struct platform_device *pdev)
>>   		dev_err(&pdev->dev, "%s",
>>   			"Failed to initialize GPIOs. The serial port may not work as expected");
>>   
>> +	/*
>> +	 * Get the GPIO lines IRQ
>> +	 */
>> +	ret = mxs_auart_request_gpio_irq(s);
>> +	if (ret)
>> +		goto out_free_irq;
>> +
>>   	auart_port[s->port.line] = s;
>>   
>>   	mxs_auart_reset(&s->port);
>>   
>>   	ret = uart_add_one_port(&auart_driver, &s->port);
>>   	if (ret)
>> -		goto out_free_irq;
>> +		goto out_free_gpio_irq;
>>   
>>   	version = readl(s->port.membase + AUART_VERSION);
>>   	dev_info(&pdev->dev, "Found APPUART %d.%d.%d\n",
>> @@ -1120,6 +1289,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
>>   
>>   	return 0;
>>   
>> +out_free_gpio_irq:
>> +	mxs_auart_free_gpio_irq(s);
>>   out_free_irq:
>>   	auart_port[pdev->id] = NULL;
>>   	free_irq(s->irq, s);
>> @@ -1139,6 +1310,7 @@ static int mxs_auart_remove(struct platform_device *pdev)
>>   
>>   	auart_port[pdev->id] = NULL;
>>   
>> +	mxs_auart_free_gpio_irq(s);
>>   	clk_put(s->clk);
>>   	free_irq(s->irq, s);
>>   	kfree(s);
>>
> Seems also ok.
> Janusz, will you provide a patch to complete {en,dis}able_ms functions
> for non-gpio ?
>
> Reviewed-by: Richard Genoud <richard.genoud@gmail.com>

Thanks for comments.
I added "TODO" to not forget
but I can't add non-gpio crt/rts on the moment (no my case).

best regards
Janusz