mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@suse.de>
To: linux-kernel@vger.kernel.org
Cc: Ondrej Puzman <puzman@gmail.com>, Greg Kroah-Hartman <gregkh@suse.de>
Subject: [PATCH 29/36] 8250: fix uninitialized FIFOs
Date: Thu,  6 Jan 2011 14:23:18 -0800	[thread overview]
Message-ID: <1294352605-31906-29-git-send-email-gregkh@suse.de> (raw)
In-Reply-To: <20110106215404.GA30624@kroah.com>

From: Ondrej Puzman <puzman@gmail.com>

I have found a bug in 8250.c driver which causes that 16550A uart FIFOs
are not turned on during initialization if they are manually configured
by setserial. UART is then working only as plain 16450 without FIFOs. On
systems with higher interrupt latency this causes buffer overruns and
loss of received data when using higher communication speeds.

I'm working for a company which produces industrial computers. These
devices typically contain high number (8 or more) of traditional 16550A
uarts - we use TL16C554A chips, but that is not much relevant. UARTs are
connected to the CPU by ISA bus (Celeron based devices) or LPC bus (Atom
based devices).

In the Linux the UARTs are using standard 8250.c driver and are
initialized using setserial command:
setserial /dev/ttyS4 uart 16550A port 0x3E0 irq 10 baud_base 115200

This executes the UART initialization through serial8250_startup()
function. At the beginning of the function up->capabilities is
initialized from uart_config:
 up->capabilities = uart_config[up->port.type].flags;
Please note that neither up->port.fifosize nor up->tx_loadsz is
initialized here!!

Later in the same function serial8250_clear_fifos() is called and
disables FIFOs. The above comment says that they will be reenabled in
set_termios (they won't ...)

After serial8250_startup() the serial8250_set_termios() is called. In
this function the following check fails because up->port.fifosize is
zero because it is not initialized correctly.

        if (up->capabilities & UART_CAP_FIFO && up->port.fifosize > 1) {
                if (baud < 2400)
                        fcr = UART_FCR_ENABLE_FIFO | UART_FCR_TRIGGER_1;
                else
                        fcr = uart_config[up->port.type].fcr;
        }

fcr variable remains zero and in the end the FCR register is set to zero
which results in disabled FIFOs even if the UART type is 16550A. This is
also true for other types of UARTs with FIFOs.

If the UART is autoconfigured via 'setserial /dev/ttySx autoconfig' then
port.fifosize and tx_loadsz are initialized correctly in the
autoconfig() function and the UART is working correctly then.

I checked the source codes and I can say that this bug is present in
2.6.x series of kernels for a couple of years. Namely I can confirm its
presence in 2.6.16.57, 2.6.32.24 and 2.6.36.1 (tested all of them on our
hardware).

I think it was not noticed before because not many people use manually
configured non PNP UARTs on ISA/LPC bus these days. Also the data loss
caused by buffer overruns occures only if  IRQ latency is higher then
time needed to receive one character on given communication speed.
For example our hardware looses received characters only if the UARTs
are connected throught LPC bus with SERIRQ (serial IRQ transport) and
not if they are connected to ISA bus because LPC SERIRQ has higher
interrupt latency then parallel ISA interupt lines.

Here is the patch to correct the bug created against 2.6.36.1:

Signed-off-by: Ondrej Puzman <puzman@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/serial/8250.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 9839199..8747215 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1981,6 +1981,8 @@ static int serial8250_startup(struct uart_port *port)
 	unsigned char lsr, iir;
 	int retval;
 
+	up->port.fifosize = uart_config[up->port.type].fifo_size;
+	up->tx_loadsz = uart_config[up->port.type].tx_loadsz;
 	up->capabilities = uart_config[up->port.type].flags;
 	up->mcr = 0;
 
-- 
1.7.3.2


  parent reply	other threads:[~2011-01-06 22:24 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-06 21:54 [GIT PATCH] TTY/serial merge for .38 Greg KH
2011-01-06 22:00 ` Alan Cox
2011-01-06 22:24   ` Greg KH
2011-01-06 23:54     ` Kay Sievers
2011-01-07  3:40       ` Greg KH
2011-01-06 22:22 ` [PATCH 01/36] ifx6x60: SPI protocol driver for Infineon 6x60 modem Greg Kroah-Hartman
2011-01-06 22:22 ` [PATCH 02/36] serial: Add support for UART on VIA VT8500 and compatibles Greg Kroah-Hartman
2011-01-06 22:22 ` [PATCH 03/36] n_gsm: Fix support for legacy encoding Greg Kroah-Hartman
2011-01-06 22:22 ` [PATCH 04/36] n_gsm: clean up printks Greg Kroah-Hartman
2011-01-06 22:22 ` [PATCH 05/36] serial: add Documentation about RS485 serial communications Greg Kroah-Hartman
2011-01-06 22:22 ` [PATCH 06/36] serial: cpm_uat: reducing CPM serial latency Greg Kroah-Hartman
2011-01-06 22:22 ` [PATCH 07/36] serial: cpu_uart: Remove unused uart_cpm_port fields Greg Kroah-Hartman
2011-01-06 22:22 ` [PATCH 08/36] console: move for_each_console to linux/console.h Greg Kroah-Hartman
2011-01-06 22:22 ` [PATCH 09/36] TTY: include termios.h in tty_driver.h Greg Kroah-Hartman
2011-01-06 22:22 ` [PATCH 10/36] VIDEO: xen-fb, switch to for_each_console Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 11/36] console: add /proc/consoles Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 12/36] parisc: cleanup console handling Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 13/36] serial: mpc52xx: make printout for type more generic Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 14/36] serial: ifx6x60: The IFX requires SPI Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 15/36] serial: ifx6x60: Fix missing include for msleep Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 16/36] Serial: ce4100: Add PCI UART support for the ce4100 Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 17/36] Serial: EG20T: add PCH_UART driver Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 18/36] serial: ifx6x60: free IRQ on error Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 19/36] serial: ifx6x60: fix memory leak Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 20/36] RS485 documentation: add 16C950 UART description Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 21/36] drivers: char: hvc: add arm JTAG DCC console support Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 22/36] serial: fix pch_uart kconfig & build Greg Kroah-Hartman
2011-01-07  0:21   ` Tomoya MORINAGA
2011-01-07  0:31     ` Greg KH
2011-01-06 22:23 ` [PATCH 23/36] serial: omap-serial: Add support for kernel debugger Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 24/36] 8250: use container_of() instead of casting Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 25/36] 8250: add a UPIO_DWAPB32 for 32 bit accesses Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 26/36] rocket: fix compiler warning on rocket_pci_ids Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 27/36] specialix: fix compiler warning on specialix_pci_tbl Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 28/36] ip2: fix compiler warning on ip2main_pci_tbl Greg Kroah-Hartman
2011-01-06 22:23 ` Greg Kroah-Hartman [this message]
2011-01-06 22:23 ` [PATCH 30/36] pch_uart : fix warnings for 64bit compile Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 31/36] tty: fix typos/errors in tty_driver.h comments Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 32/36] Serial: Avoid unbalanced IRQ wake disable during resume Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 33/36] drivers: serial: apbuart: Handle OF failures gracefully Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 34/36] tty: add 'active' sysfs attribute to tty0 and console device Greg Kroah-Hartman
2011-01-07  9:09   ` Jiri Slaby
2011-01-07 13:20     ` Kay Sievers
2011-01-07 15:12       ` Jiri Slaby
2011-01-06 22:23 ` [PATCH 35/36] TTY: Add tty ioctl to figure device node of the system console Greg Kroah-Hartman
2011-01-06 22:23 ` [PATCH 36/36] serial: apbuart: Fixup apbuart_console_init() Greg Kroah-Hartman
2011-01-07 22:44 ` [GIT PATCH] TTY/serial merge for .38 Linus Torvalds
2011-01-07 23:41   ` Greg KH
2011-01-11 14:00     ` Daniel Hellstrom
2011-01-11 14:56       ` Thomas Gleixner
2011-01-11 15:08         ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1294352605-31906-29-git-send-email-gregkh@suse.de \
    --to=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=puzman@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome