| Message ID | 53EC220D.5050908@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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
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);