Hi Greg
This issue is seen easily in automated testing on our switch with
16550 UART at 9600 baud. For UARTs with smaller FIFOs, this issue may
be seen even easily. I don't think we'll run into this problem with
manual typing through UART.
Peter Hurley is another reviewer who looked at this problem/code area
when I first sent the patch. Peter, could you take a look at patch and
see if there are other ways (best way) to solve this.
It seems right to pick data from recv FIFO while we wait for
transmitter to be empty. Another option would be to do something like
this in __wait_for_xmitr():
+
+ if (rx && status & (UART_LSR_DR | UART_LSR_BI) &&
+ !emergency_situation(&up->port))
+ serial8250_rx_chars(up, status, true);
+ else
- udelay(1);
+ udelay(1);
}
If recv FIFO is not empty, instead of sleeping with udelay(1), pick
data from FIFO. Otherwise just do udelay(1) as before.
Pl comment.
thank you.
On Fri, Jan 30, 2015 at 3:21 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, Jan 14, 2015 at 11:47:35AM -0800, prasad.koya@xxxxxxxxx wrote:
>> From: Prasad Koya <prasad@xxxxxxxxxx>
>>
>> At slow baud rates, say 9600, it is easy to run into tty buffer overrun
>> as interrupts could get disabled for a long time. To avoid that, if
>> UART_LSR_DR is set, pick up bytes from rx fifo while waiting for tx
>> buffer to be empty. Stop buffering if port->sysrq is true or
>> oops_in_progress is set.
>>
>> George Spelvin reworked original patch quite a bit.
>>
>> Tested on 16550 UART with 16byte FIFO
>>
>> Signed-off-by: Prasad Koya <prasad@xxxxxxxxxx>
>> Signed-off-by: George Spelvin <linux@xxxxxxxxxxx>
>> ---
>> drivers/tty/serial/8250/8250_core.c | 65 +++++++++++++++++++++++++++++++++----
>> drivers/tty/serial/8250/8250_fsl.c | 2 +-
>> include/linux/serial_8250.h | 3 +-
>> include/linux/serial_core.h | 2 +-
>> 4 files changed, 63 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>> index 11c6685..a9adf9d 100644
>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c
>> @@ -1434,9 +1434,12 @@ static void serial8250_enable_ms(struct uart_port *port)
>> * serial8250_rx_chars: processes according to the passed in LSR
>> * value, and returns the remaining LSR bits not handled
>> * by this Rx routine.
>> + * no_sysrq is normally false, but when set it indicates we should stop
>> + * processing input after receiving a break, rather than passing the
>> + * next character to uart_handle_sysrq_char.
>> */
>> unsigned char
>> -serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr)
>> +serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr, bool no_sysrq)
>
> I hate functions that have "bool" variables like this, it's a mess and
> can cause them to do different things so you always have to look up the
> details.
>
> Why isn't this a problem with other uarts and slow baud rates? Why
> hasn't this problem ever been reported in the past for other uarts? Is
> this really the best/only way to solve this?
>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
