serial: mxs-auart: gpios as modem signals

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 <[email protected]>

---
v3 -> v4 changelog:
[PATCH 1/4] serial: mxs-auart: clean get_mctrl and set_mctrl
* renamed from "serial: mxs-auart: ctrl removed from mxs_auart_port"
* mxs_auart_get_mctrl() read back RTS line. It is removed too.

v2 -> v3 changelog:
The patch was introduced:
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.

---
 drivers/tty/serial/mxs-auart.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

drivers/tty/serial/mxs-auart.c +1 -11

index e838e84..2d49901 100644
@@ -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;
 }
 

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 <[email protected]>

---
v3 -> v4 changelog:
* RTS_AT_AUART() and CTS_AT_AUART() macro defined
* DMA engine disabled if RTS or CTS is GPIO line
* warn if fsl,uart-has-rtscts and RTS/CTS via GPIO are mixed
* 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()

v2 -> v3 changelog:
* 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.

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

Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt +9 -1

index 59a40f1..7c408c8 100644
@@ -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 at 8006a000 {

@@ -21,6 +26,9 @@ auart0: serial at 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"

drivers/tty/serial/Kconfig +1 -0

index 4fe8ca1..90e8516 100644
@@ -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.
 

drivers/tty/serial/mxs-auart.c +24 -5

index 2d49901..8bbcfd1 100644
@@ -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;

drivers/tty/serial/mxs-auart.c +10 -0

+	if (!RTS_AT_AUART() || !CTS_AT_AUART()) {
+		if (test_bit(MXS_AUART_RTSCTS, &s->flags))
+			dev_warn(dev,
+				 "DMA and flow control via gpio may cause some problems. DMA disabled!\n");
+		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 =

Quoted 187 lines

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 <[email protected]>

---
v3 -> v4 changelog:
* RTS_AT_AUART() and CTS_AT_AUART() macro defined
* DMA engine disabled if RTS or CTS is GPIO line
* warn if fsl,uart-has-rtscts and RTS/CTS via GPIO are mixed
* 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()

v2 -> v3 changelog:
* 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.

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

Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt +9 -1

index 59a40f1..7c408c8 100644
@@ -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"

drivers/tty/serial/Kconfig +1 -0

index 4fe8ca1..90e8516 100644
@@ -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.
 

drivers/tty/serial/mxs-auart.c +24 -5

index 2d49901..8bbcfd1 100644
@@ -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;

drivers/tty/serial/mxs-auart.c static int serial_mxs_probe_dt(struct mxs_auart_port *s, · line 1042

Quoted 10 lines

drivers/tty/serial/mxs-auart.c +7 -0

@@ -1014,6 +1033,23 @@ 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 */

This is confusing, so I think some more comments won't be too much. Something like:
/*
* The DMA only work if the lines CTS/RTS are handled by the controller

* cf. 8418e67d9523 serial: mxs: enable the DMA only when the RTS/CTS is
valid
* So, If the CTS/RTS lines are controlled via GPIO the MXS_AUART_RTSCTS
bit should be
* cleaned to indicate that the RTS/CTS lines are not handled by the
controller.
*/
(If I understood correctly what is done here.)

Quoted 15 lines

drivers/tty/serial/mxs-auart.c +10 -0

+	if (!RTS_AT_AUART() || !CTS_AT_AUART()) {
+		if (test_bit(MXS_AUART_RTSCTS, &s->flags))
+			dev_warn(dev,
+				 "DMA and flow control via gpio may cause some problems. DMA disabled!\n");
+		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 =

drivers/tty/serial/mxs-auart.c static int mxs_auart_probe(struct platform_device *pdev) · line 1114

Quoted 10 lines

drivers/tty/serial/mxs-auart.c +4 -0

@@ -1069,6 +1105,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);

Reviewed-by Richard Genoud <[email protected]>

W dniu 2014-10-24 o 17:32, Richard Genoud pisze:

Quoted 198 lines · Janusz Uzycki

Quoted 182 lines

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 <[email protected]>

---
v3 -> v4 changelog:
* RTS_AT_AUART() and CTS_AT_AUART() macro defined
* DMA engine disabled if RTS or CTS is GPIO line
* warn if fsl,uart-has-rtscts and RTS/CTS via GPIO are mixed
* 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()

v2 -> v3 changelog:
* 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.

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

Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt +3 -1

index 59a40f1..7c408c8 100644
@@ -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

Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt +6 -0

+  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 at 8006a000 {

@@ -21,6 +26,9 @@ auart0: serial at 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"

drivers/tty/serial/Kconfig +1 -0

index 4fe8ca1..90e8516 100644
@@ -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..8bbcfd1 100644

drivers/tty/serial/mxs-auart.c +14 -0

@@ -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;
                }

drivers/tty/serial/mxs-auart.c +2 -3

-               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 */

drivers/tty/serial/mxs-auart.c +6 -1

+               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,23 @@ static int serial_mxs_probe_dt(struct
 mxs_auart_port *s,
        return 0;

drivers/tty/serial/mxs-auart.c static int serial_mxs_probe_dt(struct · line 1044

Quoted 10 lines

drivers/tty/serial/mxs-auart.c +6 -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
 */

This is confusing, so I think some more comments won't be too much. Something like:
/*
* The DMA only work if the lines CTS/RTS are handled by the controller

Quoted 8 lines

* cf. 8418e67d9523 serial: mxs: enable the DMA only when the RTS/CTS is
valid
* So, If the CTS/RTS lines are controlled via GPIO the MXS_AUART_RTSCTS
bit should be
* cleaned to indicate that the RTS/CTS lines are not handled by the
controller.
*/
(If I understood correctly what is done here.)

This is not clear in the driver. If hardware RTS/CTS (non-gpio) is not defined in DT
only DMA is not used. It is still possible to use hardware RTS/CTS lines if you set hw-flowcontrol in termios. So it is also possible to mix hardware RTS/CTS
and gpio RTS/CTS. In mixed or gpio case the code blocks/disables DMA. I know my comment is not clear but the reason is in the earlier code. Can you propose better solution?

After review and small fixes requested new version is required for a patchset
or rather patch resend?

kind regards
Janusz

Quoted 26 lines

Quoted 16 lines

drivers/tty/serial/mxs-auart.c +4 -0

+       if (!RTS_AT_AUART() || !CTS_AT_AUART()) {
+               if (test_bit(MXS_AUART_RTSCTS, &s->flags))
+                       dev_warn(dev,
+                                "DMA and flow control via gpio may cause

some problems. DMA disabled!\n");

drivers/tty/serial/mxs-auart.c +6 -0

+               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 =

drivers/tty/serial/mxs-auart.c static int mxs_auart_probe(struct platform_device · line 1113

Quoted 9 lines

drivers/tty/serial/mxs-auart.c +3 -0

@@ -1069,6 +1105,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);

Reviewed-by Richard Genoud <[email protected]>

2014-10-24 17:51 GMT+02:00 Janusz U?ycki <[email protected]>:

Quoted 207 lines

W dniu 2014-10-24 o 17:32, Richard Genoud pisze:

Quoted 196 lines · Janusz Uzycki

Quoted 181 lines

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 <[email protected]>

---
v3 -> v4 changelog:
* RTS_AT_AUART() and CTS_AT_AUART() macro defined
* DMA engine disabled if RTS or CTS is GPIO line
* warn if fsl,uart-has-rtscts and RTS/CTS via GPIO are mixed
* 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()

v2 -> v3 changelog:
* 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.

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

Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt +3 -1

index 59a40f1..7c408c8 100644
@@ -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

Documentation/devicetree/bindings/serial/fsl-mxs-auart.txt +6 -0

+  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 at 8006a000 {

@@ -21,6 +26,9 @@ auart0: serial at 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"

drivers/tty/serial/Kconfig +1 -0

index 4fe8ca1..90e8516 100644
@@ -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..8bbcfd1 100644

drivers/tty/serial/mxs-auart.c +14 -0

@@ -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;
                 }

drivers/tty/serial/mxs-auart.c +2 -3

-               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 */

drivers/tty/serial/mxs-auart.c +6 -1

+               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,23 @@ static int serial_mxs_probe_dt(struct
 mxs_auart_port *s,
         return 0;

drivers/tty/serial/mxs-auart.c static int serial_mxs_probe_dt(struct · line 1044

Quoted 10 lines

drivers/tty/serial/mxs-auart.c +6 -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
 */

This is confusing, so I think some more comments won't be too much. Something like:
/*
* The DMA only work if the lines CTS/RTS are handled by the controller

Quoted 8 lines

* cf. 8418e67d9523 serial: mxs: enable the DMA only when the RTS/CTS is
valid
* So, If the CTS/RTS lines are controlled via GPIO the MXS_AUART_RTSCTS
bit should be
* cleaned to indicate that the RTS/CTS lines are not handled by the
controller.
*/
(If I understood correctly what is done here.)

Quoted 8 lines

This is not clear in the driver. If hardware RTS/CTS (non-gpio) is not defined in DT
only DMA is not used. It is still possible to use hardware RTS/CTS lines if you set hw-flowcontrol in termios. So it is also possible to mix hardware RTS/CTS
and gpio RTS/CTS. In mixed or gpio case the code blocks/disables DMA. I know my comment is not clear but the reason is in the earlier code. Can you propose better solution?

Maybe Huang Shijie knows more on this DMA vs RTS/CTS incompatibility. [added Huang Shijie in Cc ]

Quoted 3 lines

After review and small fixes requested new version is required for a patchset
or rather patch resend?

It's better to resend the whole patchset (after giving some time for Huang Shijie to give its thoughts )

Quoted 31 lines

kind regards
Janusz

Quoted 26 lines

Quoted 16 lines

drivers/tty/serial/mxs-auart.c +4 -0

+       if (!RTS_AT_AUART() || !CTS_AT_AUART()) {
+               if (test_bit(MXS_AUART_RTSCTS, &s->flags))
+                       dev_warn(dev,
+                                "DMA and flow control via gpio may cause

some problems. DMA disabled!\n");

drivers/tty/serial/mxs-auart.c +6 -0

+               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 =

drivers/tty/serial/mxs-auart.c static int mxs_auart_probe(struct platform_device · line 1113

Quoted 9 lines

drivers/tty/serial/mxs-auart.c +3 -0

@@ -1069,6 +1105,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);

Reviewed-by Richard Genoud <[email protected]>

why change them to gpio? If we change them to gpio. Could the DMA still works fine?
did you test the DMA with this patch?

Add Marek for this patch too.

Quoted 1 line

why change them to gpio?

Quoted 5 lines

If we change them to gpio. Could the DMA still
works fine?
did you test the DMA with this patch?

Add Marek for this patch too.

I didn't look too deep into the patch, so here's just my experience:

1) The AUART block signals and GPIO block signals are not sychronised using the same clock. Therefore, the latency between toggling of the AUART lines and the GPIO-driven pins will not be deterministic and will vary. There might be a way to approximate that, but that's definitelly not a reliable solution.

This is very bad for example if you drive RS485 DIR line with the RTS pin as a GPIO ; the RTS pin will toggle at non-deterministic time compared to the end of UART transmission. This will trigger bit-loss on the RS485 line and you just don't want that.

2) Speaking of RS485, there's [1] and [2]. which I believe apply to any combo of UART+GPIO toggling.

So I hate to bring the bad news , but UART+GPIO combo toggling is really a bad bad idea.

HTH

[1] http://comments.gmane.org/gmane.linux.serial/6770
[2] http://article.gmane.org/gmane.linux.serial/3619/

Best regards,
Marek Vasut

W dniu 2014-11-07 o 09:03, Marek Vasut pisze:

Quoted 2 lines · Huang Shijie

Quoted 1 line

why change them to gpio?

Hardware RTS/CTS lines can be occupied by RX/TX of other AUART port in order to obtain as much uarts as possible using i.mx283. Therefore gpios can be used for "hardware" flow control.

Quoted 25 lines

Quoted 6 lines

If we change them to gpio. Could the DMA still

works fine?
did you test the DMA with this patch?

Add Marek for this patch too.

I didn't look too deep into the patch, so here's just my experience:

1) The AUART block signals and GPIO block signals are not sychronised using the

same clock. Therefore, the latency between toggling of the AUART lines and the GPIO-driven pins will not be deterministic and will vary. There might be a way to approximate that, but that's definitelly not a reliable solution.

This is very bad for example if you drive RS485 DIR line with the RTS pin as a GPIO ; the RTS pin will toggle at non-deterministic time compared to the end of UART transmission. This will trigger bit-loss on the RS485 line and you just don't want that.

2) Speaking of RS485, there's [1] and [2]. which I believe apply to any combo

Quoted 4 lines

of UART+GPIO toggling.

So I hate to bring the bad news , but UART+GPIO combo toggling is really a bad bad idea.

Unfortunately if hardware is limited there is no choice and UART+GPIO is necessary.

Your experience confirms the discussion [3] with Russell King. DMA should be disabled and
the patch disables DMA support in mxs_auart_init_gpios() if RTS or CTS line is set as gpio.

So I didn't test the patch with the DMA - I've just disabled it.

The question is different. The driver supports the cases for RTS/CTS:
1) both RTS/CTS assigned to hardware AUART (pinmux configured by DT, DT sets fsl,uart-has-rtscts)
a) settermios() sets CRTSCTS: DMA enabled, auto RTS/CTS is enabled
b) settermios() doesn't set CRTSCTS: DMA disabled, RTS/CTS
controlled by get/set_mctrl()
2) both RTS/CTS assigned to hardware AUART (pinmux configured by DT, DT doesn't set fsl,uart-has-rtscts)
a) settermios() sets CRTSCTS: DMA disabled, auto RTS/CTS is enabled
b) settermios() doesn't set CRTSCTS: DMA disabled, RTS/CTS
controlled by get/set_mctrl()

and after the patch it is more complex (because of mixed cases):
3) both RTS/CTS assigned to gpios (DT sets or not fsl,uart-has-rtscts):
the patch cancels fsl,uart-has-rtscts flag to disable DMA support, settermios() sets or not CRTSCTS: DMA disabled, RTS/CTS controlled
by get/set_mctrl(),
both lines by gpios. In addition:
a) settermios() doesn't set CRTSCTS: RTS of hardware AUART is also
controlled but
the case assumes it is not selected by pinmux in DT
b) settermios() sets CRTSCTS: auto RTS of hardware AUART is also
enabled but
the case assumes it is not selected by pinmux in DT
4) RTS assigned to hardware AUART (pinmux configured by DT, fsl,uart-has-rtscts set or not),
CTS assigned to gpio: the patch cancels fsl,uart-has-rtscts flag to
disable DMA support,
a) settermios() doesn't set CRTSCTS: DMA disabled, CTS (as gpio)
read by get_mctrl(),
RTS of hardware AUART controlled by set_mctrl()
b) settermios() sets CRTSCTS: DMA disabled, CTS (as gpio) read by
get_mctrl(),
auto RTS is enabled
So case 4) is exactly 3) case with just different pinmux
configuration and the patch
threats 3) and 4) as the same case.
5) CTS assigned to hardware AUART (pinmux configured by DT, fsl,uart-has-rtscts set or not),
RTS assigned to gpio: the patch cancels fsl,uart-has-rtscts flag to
disable DMA support,
a) settermios() doesn't set CRTSCTS: DMA disabled, CTS of hardware
AUART read by
get_mctrl(), RTS (as gpio) controlled by set_mctrl(), in
addition RTS of hardware AUART
is also controlled but the case assumes it is not selected by
pinmux in DT
b) settermios() sets CRTSCTS: DMA disabled, auto RTS/CTS is enabled
but RTS is controlled by set_mctrl() as gpio because the case assumes it is not selected
by pinmux in DT

It is not nice description but I hadn't idea how to write it more clear. The mixed cases 4) and 5) are likely not so useful but possible and not so expensive
to support.
Now the question: "fsl,uart-has-rtscts" name seems to be misleading now, do you agree? It rather should include "dma" word in the name. Any suggestion?

[3] http://thread.gmane.org/gmane.linux.serial/16069/focus=16077

Quoted 3 lines

W dniu 2014-11-07 o 09:03, Marek Vasut pisze:

Quoted 2 lines · Huang Shijie

Quoted 1 line

why change them to gpio?

+CC Alexandre, since he might be interested :-)

Quoted 4 lines

Hardware RTS/CTS lines can be occupied by RX/TX of other AUART port in order to obtain as much uarts as possible using i.mx283. Therefore gpios can be used for "hardware" flow control.

Your logic is outright flawed here, the first sentence doesn't implicate the second sentence. In fact, those two are completely unrelated.

Quoted 24 lines

Quoted 24 lines

Quoted 6 lines

If we change them to gpio. Could the DMA still

works fine?
did you test the DMA with this patch?

Add Marek for this patch too.

I didn't look too deep into the patch, so here's just my experience:

1) The AUART block signals and GPIO block signals are not sychronised using the

same clock. Therefore, the latency between toggling of the AUART lines and the GPIO-driven pins will not be deterministic and will vary. There might be a way to approximate that, but that's definitelly not a reliable solution.

This is very bad for example if you drive RS485 DIR line with the RTS pin as a GPIO ; the RTS pin will toggle at non-deterministic time compared to the end of UART transmission. This will trigger bit-loss on the RS485 line and you just don't want that.

2) Speaking of RS485, there's [1] and [2]. which I believe apply to any combo

Quoted 6 lines

Quoted 4 lines

of UART+GPIO toggling.

So I hate to bring the bad news , but UART+GPIO combo toggling is really a bad bad idea.

Unfortunately if hardware is limited there is no choice and UART+GPIO is necessary.

You will run into timing problems (see above).

What you're proposing here is a workaround for broken hardware, which was proven to be a bad idea and NAK'd already multiple times in the past (please see the links I posted in my last email).

Quoted 4 lines

Your experience confirms the discussion [3] with Russell King. DMA should be disabled and
the patch disables DMA support in mxs_auart_init_gpios() if RTS or CTS line is set as gpio.

DMA has nothing to do with those problems here. DMA can be safely ignored for the purpose of the discussion altogether.

Like I explained already -- the problem is with the GPIO and AUART block not being synchronised by the same clock. It is therefore impossible to predict and control when the GPIO signals and the AUART signals will toggle in relation to one another.

Quoted 1 line

So I didn't test the patch with the DMA - I've just disabled it.

This would break the driver for pretty much everyone using higher baud rates.

Quoted 61 lines

The question is different. The driver supports the cases for RTS/CTS:
1) both RTS/CTS assigned to hardware AUART (pinmux configured by DT, DT sets fsl,uart-has-rtscts)
a) settermios() sets CRTSCTS: DMA enabled, auto RTS/CTS is enabled
b) settermios() doesn't set CRTSCTS: DMA disabled, RTS/CTS
controlled by get/set_mctrl()
2) both RTS/CTS assigned to hardware AUART (pinmux configured by DT, DT doesn't set fsl,uart-has-rtscts)
a) settermios() sets CRTSCTS: DMA disabled, auto RTS/CTS is enabled
b) settermios() doesn't set CRTSCTS: DMA disabled, RTS/CTS
controlled by get/set_mctrl()

and after the patch it is more complex (because of mixed cases):
3) both RTS/CTS assigned to gpios (DT sets or not fsl,uart-has-rtscts):
the patch cancels fsl,uart-has-rtscts flag to disable DMA support, settermios() sets or not CRTSCTS: DMA disabled, RTS/CTS controlled
by get/set_mctrl(),
both lines by gpios. In addition:
a) settermios() doesn't set CRTSCTS: RTS of hardware AUART is also
controlled but
the case assumes it is not selected by pinmux in DT
b) settermios() sets CRTSCTS: auto RTS of hardware AUART is also
enabled but
the case assumes it is not selected by pinmux in DT
4) RTS assigned to hardware AUART (pinmux configured by DT, fsl,uart-has-rtscts set or not),
CTS assigned to gpio: the patch cancels fsl,uart-has-rtscts flag to
disable DMA support,
a) settermios() doesn't set CRTSCTS: DMA disabled, CTS (as gpio)
read by get_mctrl(),
RTS of hardware AUART controlled by set_mctrl()
b) settermios() sets CRTSCTS: DMA disabled, CTS (as gpio) read by
get_mctrl(),
auto RTS is enabled
So case 4) is exactly 3) case with just different pinmux
configuration and the patch
threats 3) and 4) as the same case.
5) CTS assigned to hardware AUART (pinmux configured by DT, fsl,uart-has-rtscts set or not),
RTS assigned to gpio: the patch cancels fsl,uart-has-rtscts flag to
disable DMA support,
a) settermios() doesn't set CRTSCTS: DMA disabled, CTS of hardware
AUART read by
get_mctrl(), RTS (as gpio) controlled by set_mctrl(), in
addition RTS of hardware AUART
is also controlled but the case assumes it is not selected by
pinmux in DT
b) settermios() sets CRTSCTS: DMA disabled, auto RTS/CTS is enabled
but RTS is controlled by set_mctrl() as gpio because the case assumes it is not selected
by pinmux in DT

It is not nice description but I hadn't idea how to write it more clear. The mixed cases 4) and 5) are likely not so useful but possible and not so expensive
to support.
Now the question: "fsl,uart-has-rtscts" name seems to be misleading now, do you agree? It rather should include "dma" word in the name. Any suggestion?

Quoted 1 line

[3] http://thread.gmane.org/gmane.linux.serial/16069/focus=16077

The best suggestion I can give you is to fix your hardware early, before you run into nasty deep s.....tuff. These workarounds do not work and they will bit you later on, when it's hard to fix the hardware anymore.

Quoted 6 lines

Quoted 3 lines

Hardware RTS/CTS lines can be occupied by RX/TX of other AUART port in order to obtain as much uarts as possible using i.mx283. Therefore gpios can be used for "hardware" flow control.

Your logic is outright flawed here, the first sentence doesn't implicate the second sentence. In fact, those two are completely unrelated.

I didn't write MUST but CAN. There is a choice. Is flexibility of the driver disadvantage?

Quoted 33 lines

Quoted 32 lines

Quoted 22 lines

Quoted 6 lines

If we change them to gpio. Could the DMA still

works fine?
did you test the DMA with this patch?

Add Marek for this patch too.

I didn't look too deep into the patch, so here's just my experience:

1) The AUART block signals and GPIO block signals are not sychronised using the

same clock. Therefore, the latency between toggling of the AUART lines and the GPIO-driven pins will not be deterministic and will vary. There might be a way to approximate that, but that's definitelly not a reliable solution.

This is very bad for example if you drive RS485 DIR line with the RTS pin as a GPIO ; the RTS pin will toggle at non-deterministic time compared to the end of UART transmission. This will trigger bit-loss on the RS485 line and you just don't want that.

Quoted 7 lines

2) Speaking of RS485, there's [1] and [2]. which I believe apply to any combo

of UART+GPIO toggling.

So I hate to bring the bad news , but UART+GPIO combo toggling is really a bad bad idea.

Unfortunately if hardware is limited there is no choice and UART+GPIO is necessary.

Quoted 1 line

You will run into timing problems (see above).

A lot of 8250-compatible devices has no hardware flow control and in most cases
they works and it is enough even for 115200 speed if CTS is handled by irq. So it depends on your needs.

Quoted 3 lines

What you're proposing here is a workaround for broken hardware, which was proven to be a bad idea and NAK'd already multiple times in the past (please see the links I posted in my last email).

It is not broken hardware - rather limited to lower speeds but still very useful solution.

Quoted 5 lines

Quoted 4 lines

Your experience confirms the discussion [3] with Russell King. DMA should be disabled and
the patch disables DMA support in mxs_auart_init_gpios() if RTS or CTS line is set as gpio.

Quoted 2 lines

DMA has nothing to do with those problems here. DMA can be safely ignored for the purpose of the discussion altogether.

When gpios are used for RTS/CTS DMA is not used. However DMA is related due to the driver
and "fsl,uart-has-rtscts". If you look into code of the driver you should agree.

Quoted 9 lines

Quoted 5 lines

Now the question: "fsl,uart-has-rtscts" name seems to be misleading now, do you agree? It rather should include "dma" word in the name. Any suggestion?

[3] http://thread.gmane.org/gmane.linux.serial/16069/focus=16077

The best suggestion I can give you is to fix your hardware early, before you run into nasty deep s.....tuff. These workarounds do not work and they will bit you later on, when it's hard to fix the hardware anymore.

The speed is limited but why don't you accept SW-HW mixed solutions?

Exactly the same method is accepted for 8250.

It is good to have choice than not. We can comment that speed is limited for gpio-based hardware flow control. So please, rethink again.

[...]

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Hardware RTS/CTS lines can be occupied by RX/TX of other AUART port in order to obtain as much uarts as possible using i.mx283. Therefore gpios can be used for "hardware" flow control.

Quoted 4 lines

Quoted 2 lines

Your logic is outright flawed here, the first sentence doesn't implicate the second sentence. In fact, those two are completely unrelated.

I didn't write MUST but CAN. There is a choice. Is flexibility of the driver disadvantage?

If the flexibility brings in known problems, then yes, it is a problem. Not because of the flexibility, but because it brings in bugs.

Quoted 31 lines

Quoted 31 lines

Quoted 31 lines

Quoted 22 lines

Quoted 7 lines

If we change them to gpio. Could the DMA still

works fine?
did you test the DMA with this patch?

Add Marek for this patch too.

I didn't look too deep into the patch, so here's just my experience:

1) The AUART block signals and GPIO block signals are not sychronised using the

same clock. Therefore, the latency between toggling of the AUART lines and the GPIO-driven pins will not be deterministic and will vary. There might be a way to approximate that, but that's definitelly not a reliable solution.

This is very bad for example if you drive RS485 DIR line with the RTS pin as a GPIO ; the RTS pin will toggle at non-deterministic time compared to the end of UART transmission. This will trigger bit-loss on the RS485 line and you just don't want that.

Quoted 7 lines

2) Speaking of RS485, there's [1] and [2]. which I believe apply to any combo

of UART+GPIO toggling.

So I hate to bring the bad news , but UART+GPIO combo toggling is really a bad bad idea.

Unfortunately if hardware is limited there is no choice and UART+GPIO is necessary.

Quoted 5 lines

Quoted 1 line

You will run into timing problems (see above).

A lot of 8250-compatible devices has no hardware flow control and in most cases
they works and it is enough even for 115200 speed if CTS is handled by irq. So it depends on your needs.

I presume that in such a case , the 8250 still handles the CTS line, not some external GPIO block, yes ?

Quoted 6 lines

Quoted 4 lines

What you're proposing here is a workaround for broken hardware, which was proven to be a bad idea and NAK'd already multiple times in the past (please see the links I posted in my last email).

It is not broken hardware - rather limited to lower speeds but still very useful solution.

What exact "lower speed" are you talking about here and why ?

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

Your experience confirms the discussion [3] with Russell King. DMA should be disabled and
the patch disables DMA support in mxs_auart_init_gpios() if RTS or CTS line is set as gpio.

DMA has nothing to do with those problems here. DMA can be safely ignored for the purpose of the discussion altogether.

Quoted 4 lines

When gpios are used for RTS/CTS DMA is not used. However DMA is related due to the driver
and "fsl,uart-has-rtscts". If you look into code of the driver you should agree.

This makes me believe that the DMA introduces too many timing fluctuations, so it's really not possible for you to keep toggling the GPIOs such that the bus would work. Is that the case ?

[...]

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Now the question: "fsl,uart-has-rtscts" name seems to be misleading now, do you agree? It rather should include "dma" word in the name. Any suggestion?

Quoted 5 lines

Quoted 4 lines

Quoted 1 line

[3] http://thread.gmane.org/gmane.linux.serial/16069/focus=16077

The best suggestion I can give you is to fix your hardware early, before you run into nasty deep s.....tuff. These workarounds do not work and they will bit you later on, when it's hard to fix the hardware anymore.

The speed is limited but why don't you accept SW-HW mixed solutions?

Did you read up on the RS485 timing problems and why that solution was never accepted for any driver ? I believe the threads explained that quite clearly.

Quoted 2 lines

Exactly the same method is accepted for 8250.

Can you point out this code please ?

Quoted 3 lines

It is good to have choice than not. We can comment that speed is limited for gpio-based hardware flow control. So please, rethink again.

[...]

W dniu 2014-11-07 o 15:48, Marek Vasut pisze:

Quoted 11 lines · Janusz U?ycki

[...]

Quoted 8 lines

Quoted 7 lines

Quoted 3 lines

Hardware RTS/CTS lines can be occupied by RX/TX of other AUART port in order to obtain as much uarts as possible using i.mx283. Therefore gpios can be used for "hardware" flow control.

Your logic is outright flawed here, the first sentence doesn't implicate the second sentence. In fact, those two are completely unrelated.

Quoted 5 lines

Quoted 2 lines

I didn't write MUST but CAN. There is a choice. Is flexibility of the driver disadvantage?

If the flexibility brings in known problems, then yes, it is a problem. Not because of the flexibility, but because it brings in bugs.

New features new bugs :) Does it mean to stop development?

Quoted 41 lines

Quoted 40 lines

Quoted 28 lines

Quoted 28 lines

Quoted 28 lines

Quoted 6 lines

If we change them to gpio. Could the DMA still

works fine?
did you test the DMA with this patch?

Add Marek for this patch too.

I didn't look too deep into the patch, so here's just my experience:

1) The AUART block signals and GPIO block signals are not sychronised using the

same clock. Therefore, the latency between toggling of the AUART lines and the GPIO-driven pins will not be deterministic and will vary. There might be a way to approximate that, but that's definitelly not a reliable solution.

This is very bad for example if you drive RS485 DIR line with the RTS pin as a GPIO ; the RTS pin will toggle at non-deterministic time compared to the end of UART transmission. This will trigger bit-loss on the RS485 line and you just don't want that.

2) Speaking of RS485, there's [1] and [2]. which I believe apply to any combo

of UART+GPIO toggling.

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

So I hate to bring the bad news , but UART+GPIO combo toggling is really a bad bad idea.

Unfortunately if hardware is limited there is no choice and UART+GPIO is necessary.

You will run into timing problems (see above).

A lot of 8250-compatible devices has no hardware flow control and in most cases
they works and it is enough even for 115200 speed if CTS is handled by irq. So it depends on your needs.

Quoted 2 lines

I presume that in such a case , the 8250 still handles the CTS line, not some external GPIO block, yes ?

Yes. However mxs includes both GPIO and AUART. Clocks differs but it is still the same silicon.
External GPIO block is extreme example highly not recommended here.

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

What you're proposing here is a workaround for broken hardware, which was proven to be a bad idea and NAK'd already multiple times in the past (please see the links I posted in my last email).

It is not broken hardware - rather limited to lower speeds but still very useful solution.

Quoted 1 line

What exact "lower speed" are you talking about here and why ?

For example not more than 115200 but it depends on CPU load of course, FIFO size
and device on the opposite site. RTS/CTS via GPIO require to know the limit in an application.

I googled even so exotic thing like:
"8250: add support for DTR/DSR hardware flow control"

Quoted 13 lines

Quoted 12 lines

Quoted 7 lines

Quoted 4 lines

Your experience confirms the discussion [3] with Russell King. DMA should be disabled and
the patch disables DMA support in mxs_auart_init_gpios() if RTS or CTS line is set as gpio.

DMA has nothing to do with those problems here. DMA can be safely ignored for the purpose of the discussion altogether.

When gpios are used for RTS/CTS DMA is not used. However DMA is related due to the driver
and "fsl,uart-has-rtscts". If you look into code of the driver you should agree.

Quoted 3 lines

This makes me believe that the DMA introduces too many timing fluctuations, so it's really not possible for you to keep toggling the GPIOs such that the bus would work. Is that the case ?

Yes, for RTS/CTS based on gpio DMA is not used. They are just toggled. So you probably misunderstood me.

Quoted 13 lines

[...]

Quoted 11 lines

Quoted 10 lines

Quoted 5 lines

Now the question: "fsl,uart-has-rtscts" name seems to be misleading now, do you agree? It rather should include "dma" word in the name. Any suggestion?

[3] http://thread.gmane.org/gmane.linux.serial/16069/focus=16077

The best suggestion I can give you is to fix your hardware early, before you run into nasty deep s.....tuff. These workarounds do not work and they will bit you later on, when it's hard to fix the hardware anymore.

Quoted 5 lines

Quoted 1 line

The speed is limited but why don't you accept SW-HW mixed solutions?

Did you read up on the RS485 timing problems and why that solution was never accepted for any driver ? I believe the threads explained that quite clearly.

Example of RS485 implementation where RTS is toggled by software:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/dri
vers/tty/serial/omap-serial.c?id=4a0ac0f55b18dc297a87a85417fcf068658bf103
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/
drivers/tty/serial/omap-serial.c

Quoted 3 lines

Quoted 1 line

Exactly the same method is accepted for 8250.

Can you point out this code please ?

If 8250 doesn't support auto flow control RTS/CTS are also toggled by software,
uart_trottle(), uart_untrothle(), uart_handle_cts_change():
http://lxr.free-electrons.com/source/drivers/tty/serial/serial_core.c#L635
http://lxr.free-electrons.com/source/drivers/tty/serial/serial_core.c#L2796
Detected by irq (ms enabled when CRTSCTS) CTS change stops tx because UPF_HARD_FLOW flag is not set by the mxs-auart driver.

Of course timing problem exists but in many cases it is not critical - the toggle method was implemented many years ago and it seems to work.

Quoted 4 lines

Quoted 2 lines

It is good to have choice than not. We can comment that speed is limited for gpio-based hardware flow control. So please, rethink again.

[...]

The only problem for me is misleading "fsl,uart-has-rtscts" name because the flag
only enables DMA if CRTSCTS is set and hardware flow control of AUART block is used.

best regards
Janusz

Quoted 10 lines

W dniu 2014-11-07 o 15:48, Marek Vasut pisze:

Quoted 9 lines · Janusz U?ycki

[...]

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

Hardware RTS/CTS lines can be occupied by RX/TX of other AUART port in order to obtain as much uarts as possible using i.mx283. Therefore gpios can be used for "hardware" flow control.

Your logic is outright flawed here, the first sentence doesn't implicate the second sentence. In fact, those two are completely unrelated.

Quoted 5 lines

Quoted 4 lines

Quoted 2 lines

I didn't write MUST but CAN. There is a choice. Is flexibility of the driver disadvantage?

If the flexibility brings in known problems, then yes, it is a problem. Not because of the flexibility, but because it brings in bugs.

New features new bugs :) Does it mean to stop development?

You shouldn't push code which is known to be defective by design into mainline.

Quoted 36 lines

Quoted 36 lines

Quoted 36 lines

Quoted 25 lines

Quoted 25 lines

Quoted 25 lines

Quoted 6 lines

If we change them to gpio. Could the DMA still

works fine?
did you test the DMA with this patch?

Add Marek for this patch too.

I didn't look too deep into the patch, so here's just my experience:

1) The AUART block signals and GPIO block signals are not sychronised using the

same clock. Therefore, the latency between toggling of the AUART lines and the GPIO-driven pins will not be deterministic and will vary. There might be a way to approximate that, but that's definitelly not a reliable solution.

This is very bad for example if you drive RS485 DIR line with the RTS pin as a GPIO ; the RTS pin will toggle at non-deterministic time compared to the end of UART transmission. This will trigger bit-loss on the RS485 line and you just don't want that.

2) Speaking of RS485, there's [1] and [2]. which I believe apply to any combo

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

of UART+GPIO toggling.

So I hate to bring the bad news , but UART+GPIO combo toggling is really a bad bad idea.

Unfortunately if hardware is limited there is no choice and UART+GPIO is necessary.

You will run into timing problems (see above).

A lot of 8250-compatible devices has no hardware flow control and in most cases
they works and it is enough even for 115200 speed if CTS is handled by irq. So it depends on your needs.

Quoted 5 lines

Quoted 2 lines

I presume that in such a case , the 8250 still handles the CTS line, not some external GPIO block, yes ?

Yes. However mxs includes both GPIO and AUART. Clocks differs but it is still the same silicon.
External GPIO block is extreme example highly not recommended here.

The way you implemented this particular change, it is possible to use arbitrary GPIO pin. There is no way you can guarantee anything about the latency of the GPIO toggling (I am repeating myself).

Quoted 11 lines

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

What you're proposing here is a workaround for broken hardware, which was proven to be a bad idea and NAK'd already multiple times in the past (please see the links I posted in my last email).

It is not broken hardware - rather limited to lower speeds but still very useful solution.

What exact "lower speed" are you talking about here and why ?

For example not more than 115200 but it depends on CPU load of course, FIFO size
and device on the opposite site. RTS/CTS via GPIO require to know the limit in an application.

OK, this is completely unreliable solution, which works just by sheer luck.

Quoted 2 lines

I googled even so exotic thing like:
"8250: add support for DTR/DSR hardware flow control"

The fact that those perversions exists doesn't make them right. It doesn't even make them mainlinable.

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Quoted 7 lines

Quoted 5 lines

Your experience confirms the discussion [3] with Russell King. DMA should be disabled and
the patch disables DMA support in mxs_auart_init_gpios() if RTS or CTS line is set as gpio.

DMA has nothing to do with those problems here. DMA can be safely ignored for the purpose of the discussion altogether.

When gpios are used for RTS/CTS DMA is not used. However DMA is related due to the driver
and "fsl,uart-has-rtscts". If you look into code of the driver you should agree.

Quoted 5 lines

Quoted 3 lines

This makes me believe that the DMA introduces too many timing fluctuations, so it's really not possible for you to keep toggling the GPIOs such that the bus would work. Is that the case ?

Yes, for RTS/CTS based on gpio DMA is not used. They are just toggled. So you probably misunderstood me.

I understand you -- in case DMA is enabled on the AUART block, your hack is no longer capable to working correctly, so everything falls apart.

Quoted 13 lines

Quoted 13 lines

[...]

Quoted 10 lines

Quoted 9 lines

Quoted 5 lines

Now the question: "fsl,uart-has-rtscts" name seems to be misleading now, do you agree? It rather should include "dma" word in the name. Any suggestion?

[3] http://thread.gmane.org/gmane.linux.serial/16069/focus=16077

The best suggestion I can give you is to fix your hardware early, before you run into nasty deep s.....tuff. These workarounds do not work and they will bit you later on, when it's hard to fix the hardware anymore.

The speed is limited but why don't you accept SW-HW mixed solutions?

Quoted 8 lines

Quoted 3 lines

Did you read up on the RS485 timing problems and why that solution was never accepted for any driver ? I believe the threads explained that quite clearly.

Example of RS485 implementation where RTS is toggled by software:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/dri
vers/tty/serial/omap-serial.c?id=4a0ac0f55b18dc297a87a85417fcf068658bf103
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/
drivers/tty/serial/omap-serial.c

This is a question for Greg. I checked the discussion about this patch [1] and I see this timing issue was brought up, but the patch was applied anyway. I cannot tell you why , I just know that this GPIO approach has problems and I wrestled those a couple of days ago (without success, it's not possible to get correct timing).

[1] http://www.spinics.net/lists/linux-serial/msg10574.html

Quoted 9 lines

Quoted 2 lines

Quoted 1 line

Exactly the same method is accepted for 8250.

Can you point out this code please ?

If 8250 doesn't support auto flow control RTS/CTS are also toggled by software,
uart_trottle(), uart_untrothle(), uart_handle_cts_change():
http://lxr.free-electrons.com/source/drivers/tty/serial/serial_core.c#L635
http://lxr.free-electrons.com/source/drivers/tty/serial/serial_core.c#L2796
Detected by irq (ms enabled when CRTSCTS) CTS change stops tx because UPF_HARD_FLOW flag is not set by the mxs-auart driver.

This doesn't use GPIO to toggle the RTS/CTS pins, it does toggle then via the 8250 IP block registers, right ?

Quoted 2 lines

Of course timing problem exists but in many cases it is not critical - the toggle method was implemented many years ago and it seems to work.

Yes, it does seem to work initially, that's why so many hardware people implement it, thinking the software people can fix those flubs. Problem is, this is one of those nasty problems, which cannot be fixed in software.

Quoted 7 lines

The only problem for me is misleading "fsl,uart-has-rtscts" name because the flag
only enables DMA if CRTSCTS is set and hardware flow control of AUART block is used.

best regards
Janusz