[PATCH 00/11] Minimum set of omap serial patches to fix merge window breakage

[PATCH 00/11] Minimum set of omap serial patches to fix merge window breakage

[Russell King - ARM Linux]

Hi,

During the merge window, a series of patches from various people went in,
allegedly fixing various problems with the OMAP serial driver.

Unfortunately, there was not a full understanding of the issues I brought
up here back in April, and so the "fixes", while being individually
correct, result in a worse situation with the driver than before.

Specifically, this patch:

commit 957ee7270d632245b43f6feb0e70d9a5e9ea6cf6
Author: Vikram Pandita <vikram.pandita@ti.com>
Date:   Thu Sep 6 15:45:37 2012 +0300

    serial: omap: fix software flow control

    Software flow control register bits were not defined correctly.

    Also clarify the IXON and IXOFF logic to reflect what userspace wants.

does what it says on the tin - it fixes the register definitions so that
we do end up enabling the right two software flow control bits.

The down side is that there are other bugs in this area which have been
exposed.  For example:

1. the XON/XOFF registers aren't written to; their address is, but we will
   not be writing to the right registers because their access rules are not
   respected by the driver.  These are by default zero.

   This means that with hardware assisted software flow control enabled,
   the port will now transmit an 0x00 byte for XON and XOFF events.

2. the driver set_termios function is not called for changes in software
   flow control settings if not accompanied by some other change.

3. the there isn't actually a way for the hardware assisted flow control
   to be used other than by increasing interrupt latency to cause the
   receiver hardware FIFO to fill.  This will cause 0x00 bytes to be
   transmitted.

There are two options to resolve this.  The first one is to revert this
patch to bring the driver back down to the pre-merge window state.  The
other is to apply this series of patches, which frankly I don't think
is -rc material, even with the above regressions.

Given that the above regressions were caused by a lack of due care and
correct process (I had declared to TI that I had investigated these
issues back in April), I believe that the right answer is to revert at
least commit 957ee7270d, which should re-hide these other bugs in the
driver.

[PATCH 01/11] SERIAL: core: use local variable uport in uart_set_termios()

[Russell King]

This is to make the following change more clear.

Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/serial_core.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0fcfd98..bc2065d 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1210,6 +1210,7 @@ static void uart_set_termios(struct tty_struct *tty,
 						struct ktermios *old_termios)
 {
 	struct uart_state *state = tty->driver_data;
+	struct uart_port *uport = state->uart_port;
 	unsigned long flags;
 	unsigned int cflag = tty->termios.c_cflag;
 
@@ -1232,31 +1233,31 @@ static void uart_set_termios(struct tty_struct *tty,
 
 	/* Handle transition to B0 status */
 	if ((old_termios->c_cflag & CBAUD) && !(cflag & CBAUD))
-		uart_clear_mctrl(state->uart_port, TIOCM_RTS | TIOCM_DTR);
+		uart_clear_mctrl(uport, TIOCM_RTS | TIOCM_DTR);
 	/* Handle transition away from B0 status */
 	else if (!(old_termios->c_cflag & CBAUD) && (cflag & CBAUD)) {
 		unsigned int mask = TIOCM_DTR;
 		if (!(cflag & CRTSCTS) ||
 		    !test_bit(TTY_THROTTLED, &tty->flags))
 			mask |= TIOCM_RTS;
-		uart_set_mctrl(state->uart_port, mask);
+		uart_set_mctrl(uport, mask);
 	}
 
 	/* Handle turning off CRTSCTS */
 	if ((old_termios->c_cflag & CRTSCTS) && !(cflag & CRTSCTS)) {
-		spin_lock_irqsave(&state->uart_port->lock, flags);
+		spin_lock_irqsave(&uport->lock, flags);
 		tty->hw_stopped = 0;
 		__uart_start(tty);
-		spin_unlock_irqrestore(&state->uart_port->lock, flags);
+		spin_unlock_irqrestore(&uport->lock, flags);
 	}
 	/* Handle turning on CRTSCTS */
 	else if (!(old_termios->c_cflag & CRTSCTS) && (cflag & CRTSCTS)) {
-		spin_lock_irqsave(&state->uart_port->lock, flags);
-		if (!(state->uart_port->ops->get_mctrl(state->uart_port) & TIOCM_CTS)) {
+		spin_lock_irqsave(&uport->lock, flags);
+		if (!(uport->ops->get_mctrl(uport) & TIOCM_CTS)) {
 			tty->hw_stopped = 1;
-			state->uart_port->ops->stop_tx(state->uart_port);
+			uport->ops->stop_tx(uport);
 		}
-		spin_unlock_irqrestore(&state->uart_port->lock, flags);
+		spin_unlock_irqrestore(&uport->lock, flags);
 	}
 }
 
-- 
1.7.4.4

[PATCH 02/11] SERIAL: core: add hardware assisted s/w flow control support

[Russell King]

Ports which are capable of handling s/w flow control in hardware to
know when the s/w flow control termios settings are changed.  Add a
flag to allow the low level serial drivers to indicate that they
support this, and these changes should be propagated to them.

Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/serial_core.c |   16 ++++++++++++++--
 include/linux/serial_core.h      |    2 ++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index bc2065d..bd10bbd 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1213,7 +1213,19 @@ static void uart_set_termios(struct tty_struct *tty,
 	struct uart_port *uport = state->uart_port;
 	unsigned long flags;
 	unsigned int cflag = tty->termios.c_cflag;
+	unsigned int iflag_mask = IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK;
+	bool sw_changed = false;
 
+	/*
+	 * Drivers doing software flow control also need to know
+	 * about changes to these input settings.
+	 */
+	if (uport->flags & UPF_SOFT_FLOW) {
+		iflag_mask |= IXANY|IXON|IXOFF;
+		sw_changed =
+		   tty->termios.c_cc[VSTART] != old_termios->c_cc[VSTART] ||
+		   tty->termios.c_cc[VSTOP] != old_termios->c_cc[VSTOP];
+	}
 
 	/*
 	 * These are the bits that are used to setup various
@@ -1221,11 +1233,11 @@ static void uart_set_termios(struct tty_struct *tty,
 	 * bits in c_cflag; c_[io]speed will always be set
 	 * appropriately by set_termios() in tty_ioctl.c
 	 */
-#define RELEVANT_IFLAG(iflag)	((iflag) & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK))
 	if ((cflag ^ old_termios->c_cflag) == 0 &&
 	    tty->termios.c_ospeed == old_termios->c_ospeed &&
 	    tty->termios.c_ispeed == old_termios->c_ispeed &&
-	    RELEVANT_IFLAG(tty->termios.c_iflag ^ old_termios->c_iflag) == 0) {
+	    ((tty->termios.c_iflag ^ old_termios->c_iflag) & iflag_mask) == 0 &&
+	    !sw_changed) {
 		return;
 	}
 
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 3c43022..0005138 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -163,6 +163,8 @@ struct uart_port {
 #define UPF_BUGGY_UART		((__force upf_t) (1 << 14))
 #define UPF_NO_TXEN_TEST	((__force upf_t) (1 << 15))
 #define UPF_MAGIC_MULTIPLIER	((__force upf_t) (1 << 16))
+/* Port has hardware-assisted s/w flow control */
+#define UPF_SOFT_FLOW		((__force upf_t) (1 << 22))
 #define UPF_CONS_FLOW		((__force upf_t) (1 << 23))
 #define UPF_SHARE_IRQ		((__force upf_t) (1 << 24))
 #define UPF_EXAR_EFR		((__force upf_t) (1 << 25))
-- 
1.7.4.4

[PATCH 03/11] SERIAL: core: add hardware assisted h/w flow control support

[Russell King]

Ports which are handling h/w flow control in hardware must not have
their RTS state altered depending on the tty's hardware-stopped state.
Avoid this additional logic when setting the termios state.

Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/serial_core.c |    7 +++++++
 include/linux/serial_core.h      |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index bd10bbd..9d8796e 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1255,6 +1255,13 @@ static void uart_set_termios(struct tty_struct *tty,
 		uart_set_mctrl(uport, mask);
 	}
 
+	/*
+	 * If the port is doing h/w assisted flow control, do nothing.
+	 * We assume that tty->hw_stopped has never been set.
+	 */
+	if (uport->flags & UPF_HARD_FLOW)
+		return;
+
 	/* Handle turning off CRTSCTS */
 	if ((old_termios->c_cflag & CRTSCTS) && !(cflag & CRTSCTS)) {
 		spin_lock_irqsave(&uport->lock, flags);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 0005138..e2cda5d 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -163,6 +163,8 @@ struct uart_port {
 #define UPF_BUGGY_UART		((__force upf_t) (1 << 14))
 #define UPF_NO_TXEN_TEST	((__force upf_t) (1 << 15))
 #define UPF_MAGIC_MULTIPLIER	((__force upf_t) (1 << 16))
+/* Port has hardware-assisted h/w flow control (iow, auto-RTS *not* auto-CTS) */
+#define UPF_HARD_FLOW		((__force upf_t) (1 << 21))
 /* Port has hardware-assisted s/w flow control */
 #define UPF_SOFT_FLOW		((__force upf_t) (1 << 22))
 #define UPF_CONS_FLOW		((__force upf_t) (1 << 23))
-- 
1.7.4.4

[PATCH 04/11] SERIAL: core: add throttle/unthrottle callbacks for hardware assisted flow control

[Russell King]

Add two callbacks for hardware assisted flow control; we need to know
when the tty layers want us to stop and restart due to their buffer
levels.

Call a driver specific throttle/unthrottle function if and only if the
driver indicates that it is using an enabled hardware assisted flow
control method, otherwise fall back to the non-hardware assisted
methods.

Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/serial_core.c |   31 +++++++++++++++++++++++++++----
 include/linux/serial_core.h      |    2 ++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9d8796e..098bb99 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -610,27 +610,50 @@ static void uart_send_xchar(struct tty_struct *tty, char ch)
 static void uart_throttle(struct tty_struct *tty)
 {
 	struct uart_state *state = tty->driver_data;
+	struct uart_port *port = state->uart_port;
+	uint32_t mask = 0;
 
 	if (I_IXOFF(tty))
+		mask |= UPF_SOFT_FLOW;
+	if (tty->termios.c_cflag & CRTSCTS)
+		mask |= UPF_HARD_FLOW;
+
+	if (port->flags & mask) {
+		port->ops->throttle(port);
+		mask &= ~port->flags;
+	}
+
+	if (mask & UPF_SOFT_FLOW)
 		uart_send_xchar(tty, STOP_CHAR(tty));
 
-	if (tty->termios.c_cflag & CRTSCTS)
-		uart_clear_mctrl(state->uart_port, TIOCM_RTS);
+	if (mask & UPF_HARD_FLOW)
+		uart_clear_mctrl(port, TIOCM_RTS);
 }
 
 static void uart_unthrottle(struct tty_struct *tty)
 {
 	struct uart_state *state = tty->driver_data;
 	struct uart_port *port = state->uart_port;
+	uint32_t mask = 0;
 
-	if (I_IXOFF(tty)) {
+	if (I_IXOFF(tty))
+		mask |= UPF_SOFT_FLOW;
+	if (tty->termios.c_cflag & CRTSCTS)
+		mask |= UPF_HARD_FLOW;
+
+	if (port->flags & mask) {
+		port->ops->unthrottle(port);
+		mask &= ~port->flags;
+	}
+
+	if (mask & UPF_SOFT_FLOW) {
 		if (port->x_char)
 			port->x_char = 0;
 		else
 			uart_send_xchar(tty, START_CHAR(tty));
 	}
 
-	if (tty->termios.c_cflag & CRTSCTS)
+	if (mask & UPF_HARD_FLOW)
 		uart_set_mctrl(port, TIOCM_RTS);
 }
 
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index e2cda5d..c6690a2 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -46,6 +46,8 @@ struct uart_ops {
 	unsigned int	(*get_mctrl)(struct uart_port *);
 	void		(*stop_tx)(struct uart_port *);
 	void		(*start_tx)(struct uart_port *);
+	void		(*throttle)(struct uart_port *);
+	void		(*unthrottle)(struct uart_port *);
 	void		(*send_xchar)(struct uart_port *, char ch);
 	void		(*stop_rx)(struct uart_port *);
 	void		(*enable_ms)(struct uart_port *);
-- 
1.7.4.4

[PATCH 05/11] SERIAL: omap: allow hardware assisted rts/cts modes to be disabled

[Russell King]

There is nothing which clears the auto RTS/CTS bits, so once hardware
flow control gets enabled, there's no possibility to disable it.
So, clear these bits when CRTSCTS is cleared.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/omap-serial.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6ede6fd..c55af63 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -921,6 +921,13 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
 		serial_out(up, UART_MCR, up->mcr | UART_MCR_RTS);
 		serial_out(up, UART_LCR, cval);
+	} else {
+		/* Disable AUTORTS and AUTOCTS */
+		up->efr &= ~(UART_EFR_CTS | UART_EFR_RTS);
+
+		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+		serial_out(up, UART_EFR, up->efr);
+		serial_out(up, UART_LCR, cval);
 	}
 
 	serial_omap_set_mctrl(&up->port, up->port.mctrl);
-- 
1.7.4.4

[PATCH 06/11] SERIAL: omap: allow hardware assisted IXANY mode to be disabled

[Russell King]

Nothing was clearing the UART_MCR_XONANY bit, so once the ixany
mode gets set, there's no possibility to disable it.  Clear this
bit when IXANY mode is cleared.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/omap-serial.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index c55af63..7c313c2 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -698,6 +698,8 @@ serial_omap_configure_xonxoff
 	 */
 	if (termios->c_iflag & IXANY)
 		up->mcr |= UART_MCR_XONANY;
+	else
+		up->mcr &= ~UART_MCR_XONANY;
 
 	serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
-- 
1.7.4.4

[PATCH 07/11] SERIAL: omap: remove setting of EFR SCD bit

[Russell King]

The SCD (special character detect) bit enables comparisons with XOFF2,
which we do not program.  As the XOFF2 character remains unprogrammed,
there's little point enabling this feature along with its associated
interrupt.  Remove this, and ensure that the SCD bit is cleared.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/omap-serial.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 7c313c2..71db4e6 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -704,13 +704,8 @@ serial_omap_configure_xonxoff
 	serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 	serial_out(up, UART_TI752_TCR, OMAP_UART_TCR_TRIG);
-	/* Enable special char function UARTi.EFR_REG[5] and
-	 * load the new software flow control mode IXON or IXOFF
-	 * and restore the UARTi.EFR_REG[4] ENHANCED_EN value.
-	 */
-	serial_out(up, UART_EFR, up->efr | UART_EFR_SCD);
+	serial_out(up, UART_EFR, up->efr);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
-
 	serial_out(up, UART_MCR, up->mcr & ~UART_MCR_TCRTLR);
 	serial_out(up, UART_LCR, up->lcr);
 }
@@ -843,6 +838,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 
 	up->efr = serial_in(up, UART_EFR);
+	up->efr &= ~UART_EFR_SCD;
 	serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
 
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
-- 
1.7.4.4

[PATCH 08/11] SERIAL: omap: no need to re-read EFR

[Russell King]

There's no need to re-read EFR after we've recently written it; the
register is a configuration register which doesn't change its value
without us writing to it.  The last value which was written to this
register was up->efr.

Removing this re-reading avoids the possibility that we end up with
up->efr having unintended bits set, which should only be temporarily
set when accessing the enhanced features.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/omap-serial.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 71db4e6..c8a0425 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -873,8 +873,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 		serial_out(up, UART_OMAP_MDR1, up->mdr1);
 
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
-
-	up->efr = serial_in(up, UART_EFR);
 	serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
 
 	serial_out(up, UART_LCR, 0);
-- 
1.7.4.4

[PATCH 09/11] SERIAL: omap: fix set_mctrl() breakage

[Russell King]

c538d20c7f (and maybe previous commits) broke set_mctrl() by making
it only capable of setting bits in the MCR register.  This prevents
software controlled flow control and modem control line manipulation
via TIOCMSET/TIOCMBIC from working correctly.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/omap-serial.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index c8a0425..537829f 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -504,7 +504,7 @@ static unsigned int serial_omap_get_mctrl(struct uart_port *port)
 static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
 	struct uart_omap_port *up = to_uart_omap_port(port);
-	unsigned char mcr = 0;
+	unsigned char mcr = 0, old_mcr;
 
 	dev_dbg(up->port.dev, "serial_omap_set_mctrl+%d\n", up->port.line);
 	if (mctrl & TIOCM_RTS)
@@ -519,8 +519,10 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
 		mcr |= UART_MCR_LOOP;
 
 	pm_runtime_get_sync(up->dev);
-	up->mcr = serial_in(up, UART_MCR);
-	up->mcr |= mcr;
+	old_mcr = serial_in(up, UART_MCR);
+	old_mcr &= ~(UART_MCR_LOOP | UART_MCR_OUT2 | UART_MCR_OUT1 |
+		     UART_MCR_DTR | UART_MCR_RTS);
+	up->mcr = old_mcr | mcr;
 	serial_out(up, UART_MCR, up->mcr);
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
-- 
1.7.4.4

[PATCH 10/11] SERIAL: omap: fix MCR TCRTLR bit handling

[Russell King]

The MCR TCRTLR bit can only be changed when ECB is set in the EFR.
Unfortunately, several places were trying to alter this bit while ECB
was clear:

- serial_omap_configure_xonxoff() was attempting to clear the bit after
  explicitly clearing the ECB bit.
- serial_omap_set_termios() was trying the same trick after setting the
  SCR, and when trying to change the TCR register when hardware flow
  control was enabled.

Fix this by ensuring that we always have ECB set whenever the TCRTLR bit
is changed.

Moreover, we start out by reading the EFR and MCR registers, which may
have indeterminent bit settings for the ECB and TCRTLR bits.  Ensure
that these bits always start off in a known state.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/omap-serial.c |   32 ++++++++++++++++++--------------
 1 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 537829f..7cc151c 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -706,9 +706,10 @@ serial_omap_configure_xonxoff
 	serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 	serial_out(up, UART_TI752_TCR, OMAP_UART_TCR_TRIG);
-	serial_out(up, UART_EFR, up->efr);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
 	serial_out(up, UART_MCR, up->mcr & ~UART_MCR_TCRTLR);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+	serial_out(up, UART_EFR, up->efr);
 	serial_out(up, UART_LCR, up->lcr);
 }
 
@@ -729,7 +730,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 {
 	struct uart_omap_port *up = to_uart_omap_port(port);
 	unsigned char cval = 0;
-	unsigned char efr = 0;
 	unsigned long flags = 0;
 	unsigned int baud, quot;
 
@@ -839,12 +839,12 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 
-	up->efr = serial_in(up, UART_EFR);
+	up->efr = serial_in(up, UART_EFR) & ~UART_EFR_ECB;
 	up->efr &= ~UART_EFR_SCD;
 	serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
 
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
-	up->mcr = serial_in(up, UART_MCR);
+	up->mcr = serial_in(up, UART_MCR) & ~UART_MCR_TCRTLR;
 	serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
 	/* FIFO ENABLE, DMA MODE */
 
@@ -863,9 +863,12 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	serial_out(up, UART_OMAP_SCR, up->scr);
 
-	serial_out(up, UART_EFR, up->efr);
+	/* Reset UART_MCR_TCRTLR: this must be done with the EFR_ECB bit set */
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
 	serial_out(up, UART_MCR, up->mcr);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+	serial_out(up, UART_EFR, up->efr);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
 
 	/* Protocol, Baud Rate, and Interrupt Settings */
 
@@ -904,20 +907,21 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	/* Hardware Flow Control Configuration */
 
 	if (termios->c_cflag & CRTSCTS) {
-		efr |= (UART_EFR_CTS | UART_EFR_RTS);
-		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
-
-		up->mcr = serial_in(up, UART_MCR);
-		serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
-
+		/* Enable access to TCR/TLR */
 		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
-		up->efr = serial_in(up, UART_EFR);
 		serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
+		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
+		serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
 
 		serial_out(up, UART_TI752_TCR, OMAP_UART_TCR_TRIG);
-		serial_out(up, UART_EFR, efr); /* Enable AUTORTS and AUTOCTS */
-		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
+
+		/* Enable AUTORTS and AUTOCTS */
+		up->efr |= UART_EFR_CTS | UART_EFR_RTS;
+
+		/* Disable access to TCR/TLR */
 		serial_out(up, UART_MCR, up->mcr | UART_MCR_RTS);
+		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+		serial_out(up, UART_EFR, up->efr);
 		serial_out(up, UART_LCR, cval);
 	} else {
 		/* Disable AUTORTS and AUTOCTS */
-- 
1.7.4.4

[PATCH 10/11] SERIAL: omap: fix MCR TCRTLR bit handling

[Russell King - ARM Linux]

On Tue, Oct 16, 2012 at 12:01:06PM +0100, Russell King wrote:
> The MCR TCRTLR bit can only be changed when ECB is set in the EFR.
> Unfortunately, several places were trying to alter this bit while ECB
> was clear:
> 
> - serial_omap_configure_xonxoff() was attempting to clear the bit after
>   explicitly clearing the ECB bit.
> - serial_omap_set_termios() was trying the same trick after setting the
>   SCR, and when trying to change the TCR register when hardware flow
>   control was enabled.
> 
> Fix this by ensuring that we always have ECB set whenever the TCRTLR bit
> is changed.
> 
> Moreover, we start out by reading the EFR and MCR registers, which may
> have indeterminent bit settings for the ECB and TCRTLR bits.  Ensure
> that these bits always start off in a known state.

Note - this patch unfortunately exposes some of the bugs in their full
glory as we now write to the correct registers and actually end up
programming the hardware correctly.

I've reduced the window as far as possible by putting this one next to
patch 11 which fixes it, but no amount of reordering can hide some kind
of breakage along the way.

> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  drivers/tty/serial/omap-serial.c |   32 ++++++++++++++++++--------------
>  1 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index 537829f..7cc151c 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -706,9 +706,10 @@ serial_omap_configure_xonxoff
>  	serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
>  	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
>  	serial_out(up, UART_TI752_TCR, OMAP_UART_TCR_TRIG);
> -	serial_out(up, UART_EFR, up->efr);
>  	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
>  	serial_out(up, UART_MCR, up->mcr & ~UART_MCR_TCRTLR);
> +	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> +	serial_out(up, UART_EFR, up->efr);
>  	serial_out(up, UART_LCR, up->lcr);
>  }
>  
> @@ -729,7 +730,6 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>  {
>  	struct uart_omap_port *up = to_uart_omap_port(port);
>  	unsigned char cval = 0;
> -	unsigned char efr = 0;
>  	unsigned long flags = 0;
>  	unsigned int baud, quot;
>  
> @@ -839,12 +839,12 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>  
>  	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
>  
> -	up->efr = serial_in(up, UART_EFR);
> +	up->efr = serial_in(up, UART_EFR) & ~UART_EFR_ECB;
>  	up->efr &= ~UART_EFR_SCD;
>  	serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
>  
>  	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
> -	up->mcr = serial_in(up, UART_MCR);
> +	up->mcr = serial_in(up, UART_MCR) & ~UART_MCR_TCRTLR;
>  	serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
>  	/* FIFO ENABLE, DMA MODE */
>  
> @@ -863,9 +863,12 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>  
>  	serial_out(up, UART_OMAP_SCR, up->scr);
>  
> -	serial_out(up, UART_EFR, up->efr);
> +	/* Reset UART_MCR_TCRTLR: this must be done with the EFR_ECB bit set */
>  	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
>  	serial_out(up, UART_MCR, up->mcr);
> +	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> +	serial_out(up, UART_EFR, up->efr);
> +	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
>  
>  	/* Protocol, Baud Rate, and Interrupt Settings */
>  
> @@ -904,20 +907,21 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
>  	/* Hardware Flow Control Configuration */
>  
>  	if (termios->c_cflag & CRTSCTS) {
> -		efr |= (UART_EFR_CTS | UART_EFR_RTS);
> -		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
> -
> -		up->mcr = serial_in(up, UART_MCR);
> -		serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
> -
> +		/* Enable access to TCR/TLR */
>  		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> -		up->efr = serial_in(up, UART_EFR);
>  		serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
> +		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
> +		serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
>  
>  		serial_out(up, UART_TI752_TCR, OMAP_UART_TCR_TRIG);
> -		serial_out(up, UART_EFR, efr); /* Enable AUTORTS and AUTOCTS */
> -		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
> +
> +		/* Enable AUTORTS and AUTOCTS */
> +		up->efr |= UART_EFR_CTS | UART_EFR_RTS;
> +
> +		/* Disable access to TCR/TLR */
>  		serial_out(up, UART_MCR, up->mcr | UART_MCR_RTS);
> +		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> +		serial_out(up, UART_EFR, up->efr);
>  		serial_out(up, UART_LCR, cval);
>  	} else {
>  		/* Disable AUTORTS and AUTOCTS */
> -- 
> 1.7.4.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 11/11] SERIAL: omap: fix hardware assisted flow control

[Russell King]

When the UART device has hardware flow control enabled, it ignores the
MCR RTS bit in the MCR register, and keeps RTS asserted as long as we
continue to read characters from the UART receiver FIFO.  This means
that when the TTY buffers become full, the UART doesn't tell the remote
end to stop sending, which causes the TTY layer to start dropping
characters.

A similar problem exists with software flow control.  We need the FIFO
register to fill when software flow control is enabled to provoke the
UART to send the XOFF character.

Fix this by implementing the throttle/unthrottle callbacks, and use
these to disable receiver interrupts.  This in turn means that the UART
FIFO will fill, which will then cause the UART's hardware to deassert
the RTS signal and/or send the XOFF character, stopping the remote end.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/tty/serial/omap-serial.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 7cc151c..aabc94d 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -303,6 +303,34 @@ static void serial_omap_start_tx(struct uart_port *port)
 	pm_runtime_put_autosuspend(up->dev);
 }
 
+static void serial_omap_throttle(struct uart_port *port)
+{
+	struct uart_omap_port *up = to_uart_omap_port(port);
+	unsigned long flags;
+
+	pm_runtime_get_sync(up->dev);
+	spin_lock_irqsave(&up->port.lock, flags);
+	up->ier &= ~(UART_IER_RLSI | UART_IER_RDI);
+	serial_out(up, UART_IER, up->ier);
+	spin_unlock_irqrestore(&up->port.lock, flags);
+	pm_runtime_mark_last_busy(up->dev);
+	pm_runtime_put_autosuspend(up->dev);
+}
+
+static void serial_omap_unthrottle(struct uart_port *port)
+{
+	struct uart_omap_port *up = to_uart_omap_port(port);
+	unsigned long flags;
+
+	pm_runtime_get_sync(up->dev);
+	spin_lock_irqsave(&up->port.lock, flags);
+	up->ier |= UART_IER_RLSI | UART_IER_RDI;
+	serial_out(up, UART_IER, up->ier);
+	spin_unlock_irqrestore(&up->port.lock, flags);
+	pm_runtime_mark_last_busy(up->dev);
+	pm_runtime_put_autosuspend(up->dev);
+}
+
 static unsigned int check_modem_status(struct uart_omap_port *up)
 {
 	unsigned int status;
@@ -1000,6 +1028,7 @@ static void serial_omap_config_port(struct uart_port *port, int flags)
 	dev_dbg(up->port.dev, "serial_omap_config_port+%d\n",
 							up->port.line);
 	up->port.type = PORT_OMAP;
+	up->port.flags |= UPF_SOFT_FLOW | UPF_HARD_FLOW;
 }
 
 static int
@@ -1203,6 +1232,8 @@ static struct uart_ops serial_omap_pops = {
 	.get_mctrl	= serial_omap_get_mctrl,
 	.stop_tx	= serial_omap_stop_tx,
 	.start_tx	= serial_omap_start_tx,
+	.throttle	= serial_omap_throttle,
+	.unthrottle	= serial_omap_unthrottle,
 	.stop_rx	= serial_omap_stop_rx,
 	.enable_ms	= serial_omap_enable_ms,
 	.break_ctl	= serial_omap_break_ctl,
-- 
1.7.4.4

[PATCH 00/11] Minimum set of omap serial patches to fix merge window breakage

[Russell King - ARM Linux]

On Tue, Oct 16, 2012 at 11:14:58AM +0100, Russell King - ARM Linux wrote:
> During the merge window, a series of patches from various people went in,
> allegedly fixing various problems with the OMAP serial driver.
> 
> Unfortunately, there was not a full understanding of the issues I brought
> up here back in April, and so the "fixes", while being individually
> correct, result in a worse situation with the driver than before.
> 
> Specifically, this patch:
> 
> commit 957ee7270d632245b43f6feb0e70d9a5e9ea6cf6
> Author: Vikram Pandita <vikram.pandita@ti.com>
> Date:   Thu Sep 6 15:45:37 2012 +0300
> 
>     serial: omap: fix software flow control
> 
>     Software flow control register bits were not defined correctly.
> 
>     Also clarify the IXON and IXOFF logic to reflect what userspace wants.
> 
> does what it says on the tin - it fixes the register definitions so that
> we do end up enabling the right two software flow control bits.
> 
> The down side is that there are other bugs in this area which have been
> exposed.  For example:
> 
> 1. the XON/XOFF registers aren't written to; their address is, but we will
>    not be writing to the right registers because their access rules are not
>    respected by the driver.  These are by default zero.
> 
>    This means that with hardware assisted software flow control enabled,
>    the port will now transmit an 0x00 byte for XON and XOFF events.
> 
> 2. the driver set_termios function is not called for changes in software
>    flow control settings if not accompanied by some other change.
> 
> 3. the there isn't actually a way for the hardware assisted flow control
>    to be used other than by increasing interrupt latency to cause the
>    receiver hardware FIFO to fill.  This will cause 0x00 bytes to be
>    transmitted.
> 
> There are two options to resolve this.  The first one is to revert this
> patch to bring the driver back down to the pre-merge window state.  The
> other is to apply this series of patches, which frankly I don't think
> is -rc material, even with the above regressions.
> 
> Given that the above regressions were caused by a lack of due care and
> correct process (I had declared to TI that I had investigated these
> issues back in April), I believe that the right answer is to revert at
> least commit 957ee7270d, which should re-hide these other bugs in the
> driver.

Note: these patches are not as well validated as my previous set; I
haven't even build-tested just this set, but only my full set.  YMMV.

[PATCH 00/11] Minimum set of omap serial patches to fix merge window breakage

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [121016 03:16]:
> 
> There are two options to resolve this.  The first one is to revert this
> patch to bring the driver back down to the pre-merge window state.  The
> other is to apply this series of patches, which frankly I don't think
> is -rc material, even with the above regressions.

Yes reverting it is the way to go as we discussed in some off-list
emails. I think everybody agrees that this is too intrusive for the
-rc series :)

Regards,

Tony