diff mbox

[to,be,tested] serial: msm_serial: add missing sysrq handling

Message ID 53EC220D.5050908@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Frank Rowand Aug. 14, 2014, 2:42 a.m. UTC
On 8/13/2014 7:33 PM, Frank Rowand wrote:
> On 8/12/2014 5:23 PM, Stephen Boyd wrote:
>> On 08/06/14 17:16, Frank Rowand wrote:

< snip >
 
> The patches you sent are a little hard to read since they modify further code
> that my patch modified.  So I have redone your patches, as if my patch was
> not previously applied.  Hopefully I did not make any mistakes there.  I will
> reply to this email with each of your redone patches.

Stephen's patch alternative number 2:

---
 drivers/tty/serial/msm_serial.c |   41 +++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

Comments

Stephen Boyd Oct. 3, 2014, 9:34 p.m. UTC | #1
Hi Frank,

On 08/13/14 19:42, Frank Rowand wrote:
> On 8/13/2014 7:33 PM, Frank Rowand wrote:
>> On 8/12/2014 5:23 PM, Stephen Boyd wrote:
>>> On 08/06/14 17:16, Frank Rowand wrote:
> < snip >
>  
>> The patches you sent are a little hard to read since they modify further code
>> that my patch modified.  So I have redone your patches, as if my patch was
>> not previously applied.  Hopefully I did not make any mistakes there.  I will
>> reply to this email with each of your redone patches.
> Stephen's patch alternative number 2:

I had a discussion with the hardware engineer. Apparently the break bit
in the SR register is not "sticky" so it doesn't always stay set when
the handle_rx_dm() function runs and a break has entered the fifo. I
used your debug patches to confirm this (I never see the break status
bit when the fifo has multiple characters in it). It sounds like this
bit can't really be used reliably. The recommendation is to use either
the break start or break stop interrupt to detect when a break has
occurred and then search the fifo for the break character (an all zero
character). If there are two such characters then we can't be certain
when the break was, but the chances of this seem really slim considering
that the stale timeout probably triggers first before a human can type a
character after the break.

On 1.4 hardware we can change the mode to be single character and then
we can reliably detect the break character because only one character
enters the fifo and the higher bits of the fifo can be used to detect if
it is a break or not. Making that change is probably not that hard, I
believe we can reuse all the handle_rx_dm logic and force single
character mode on consoles, but 1.3 hardware doesn't benefit from this
change. I'll try to get something together soon that tries to make this
all work as best as it can.
Frank Rowand Oct. 3, 2014, 10:18 p.m. UTC | #2
On 10/3/2014 2:34 PM, Stephen Boyd wrote:
> Hi Frank,
> 
> On 08/13/14 19:42, Frank Rowand wrote:
>> On 8/13/2014 7:33 PM, Frank Rowand wrote:
>>> On 8/12/2014 5:23 PM, Stephen Boyd wrote:
>>>> On 08/06/14 17:16, Frank Rowand wrote:
>> < snip >
>>  
>>> The patches you sent are a little hard to read since they modify further code
>>> that my patch modified.  So I have redone your patches, as if my patch was
>>> not previously applied.  Hopefully I did not make any mistakes there.  I will
>>> reply to this email with each of your redone patches.
>> Stephen's patch alternative number 2:
> 
> I had a discussion with the hardware engineer. Apparently the break bit
> in the SR register is not "sticky" so it doesn't always stay set when
> the handle_rx_dm() function runs and a break has entered the fifo. I
> used your debug patches to confirm this (I never see the break status
> bit when the fifo has multiple characters in it). It sounds like this
> bit can't really be used reliably. The recommendation is to use either
> the break start or break stop interrupt to detect when a break has
> occurred and then search the fifo for the break character (an all zero
> character). If there are two such characters then we can't be certain
> when the break was, but the chances of this seem really slim considering
> that the stale timeout probably triggers first before a human can type a
> character after the break.

That sounds good to me.

> On 1.4 hardware we can change the mode to be single character and then
> we can reliably detect the break character because only one character
> enters the fifo and the higher bits of the fifo can be used to detect if
> it is a break or not. Making that change is probably not that hard, I
> believe we can reuse all the handle_rx_dm logic and force single
> character mode on consoles, but 1.3 hardware doesn't benefit from this
> change. I'll try to get something together soon that tries to make this
> all work as best as it can.

Thanks!  I'll be glad to review and test.  And whatever else I can do to
help.

-Frank
diff mbox

Patch

Index: b/drivers/tty/serial/msm_serial.c
===================================================================
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -125,25 +125,40 @@  static void handle_rx_dm(struct uart_por
 	port->icount.rx += count;
 
 	while (count > 0) {
-		unsigned int c;
+		unsigned char buf[4];
+		int sysrq, r_count, i;
 
 		sr = msm_read(port, UART_SR);
 		if ((sr & UART_SR_RX_READY) == 0) {
 			msm_port->old_snap_state -= count;
 			break;
 		}
-		c = msm_read(port, UARTDM_RF);
-		if (sr & UART_SR_RX_BREAK) {
-			port->icount.brk++;
-			if (uart_handle_break(port))
-				continue;
-		} else if (sr & UART_SR_PAR_FRAME_ERR)
-			port->icount.frame++;
-
-		/* TODO: handle sysrq */
-		tty_insert_flip_string(tport, (char *)&c,
-				       (count > 4) ? 4 : count);
-		count -= 4;
+		readsl(port->membase + UARTDM_RF, buf, 1);
+
+		r_count = min_t(int, count, sizeof(buf));
+
+		for (i = 0; i < r_count; i++) {
+			char flag = TTY_NORMAL;
+
+			if (sr & UART_SR_RX_BREAK) {
+				if (buf[i] == 0) {
+					port->icount.brk++;
+					flag = TTY_BREAK;
+					if (uart_handle_break(port))
+						continue;
+				}
+			}
+
+			if (!(port->read_status_mask & UART_SR_RX_BREAK))
+				flag = TTY_NORMAL;
+
+			spin_unlock(&port->lock);
+			sysrq = uart_handle_sysrq_char(port, buf[i]);
+			spin_lock(&port->lock);
+			if (!sysrq)
+				tty_insert_flip_char(tport, buf[i], flag);
+		}
+		count -= r_count;
 	}
 
 	spin_unlock(&port->lock);