From 49d5741be27aa90301b89bf254972b355ed9c8ee Mon Sep 17 00:00:00 2001 From: Jamie Iles Date: Wed, 1 Dec 2010 23:39:35 +0000 Subject: 8250: use container_of() instead of casting The 8250 driver structure uart_8250_port took advantage of the fact that the struct uart_port was the first member of its structure and used an explicit cast to convert to the derived class. Replace the explicit casts with container_of() for safety and clarity. Signed-off-by: Jamie Iles Signed-off-by: Greg Kroah-Hartman --- drivers/serial/8250.c | 60 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 20 deletions(-) (limited to 'drivers/serial/8250.c') diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c index 4d8e14b7aa9..0b4ce47d987 100644 --- a/drivers/serial/8250.c +++ b/drivers/serial/8250.c @@ -461,7 +461,8 @@ static void dwapb_serial_out(struct uart_port *p, int offset, int value) /* Save the LCR value so it can be re-written when a * Busy Detect interrupt occurs. */ if (save_offset == UART_LCR) { - struct uart_8250_port *up = (struct uart_8250_port *)p; + struct uart_8250_port *up = + container_of(p, struct uart_8250_port, port); up->lcr = value; } writeb(value, p->membase + offset); @@ -485,7 +486,8 @@ static void io_serial_out(struct uart_port *p, int offset, int value) static void set_io_from_upio(struct uart_port *p) { - struct uart_8250_port *up = (struct uart_8250_port *)p; + struct uart_8250_port *up = + container_of(p, struct uart_8250_port, port); switch (p->iotype) { case UPIO_HUB6: p->serial_in = hub6_serial_in; @@ -1319,7 +1321,8 @@ static inline void __stop_tx(struct uart_8250_port *p) static void serial8250_stop_tx(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); __stop_tx(up); @@ -1336,7 +1339,8 @@ static void transmit_chars(struct uart_8250_port *up); static void serial8250_start_tx(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); if (!(up->ier & UART_IER_THRI)) { up->ier |= UART_IER_THRI; @@ -1364,7 +1368,8 @@ static void serial8250_start_tx(struct uart_port *port) static void serial8250_stop_rx(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); up->ier &= ~UART_IER_RLSI; up->port.read_status_mask &= ~UART_LSR_DR; @@ -1373,7 +1378,8 @@ static void serial8250_stop_rx(struct uart_port *port) static void serial8250_enable_ms(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); /* no MSR capabilities */ if (up->bugs & UART_BUG_NOMSR) @@ -1781,7 +1787,8 @@ static void serial8250_backup_timeout(unsigned long data) static unsigned int serial8250_tx_empty(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); unsigned long flags; unsigned int lsr; @@ -1795,7 +1802,8 @@ static unsigned int serial8250_tx_empty(struct uart_port *port) static unsigned int serial8250_get_mctrl(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); unsigned int status; unsigned int ret; @@ -1815,7 +1823,8 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port) static void serial8250_set_mctrl(struct uart_port *port, unsigned int mctrl) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); unsigned char mcr = 0; if (mctrl & TIOCM_RTS) @@ -1836,7 +1845,8 @@ static void serial8250_set_mctrl(struct uart_port *port, unsigned int mctrl) static void serial8250_break_ctl(struct uart_port *port, int break_state) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); unsigned long flags; spin_lock_irqsave(&up->port.lock, flags); @@ -1890,7 +1900,8 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits) static int serial8250_get_poll_char(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); unsigned char lsr = serial_inp(up, UART_LSR); if (!(lsr & UART_LSR_DR)) @@ -1904,7 +1915,8 @@ static void serial8250_put_poll_char(struct uart_port *port, unsigned char c) { unsigned int ier; - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); /* * First save the IER then disable the interrupts @@ -1938,7 +1950,8 @@ static void serial8250_put_poll_char(struct uart_port *port, static int serial8250_startup(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); unsigned long flags; unsigned char lsr, iir; int retval; @@ -2166,7 +2179,8 @@ dont_test_tx_en: static void serial8250_shutdown(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); unsigned long flags; /* @@ -2235,7 +2249,8 @@ void serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios, struct ktermios *old) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); unsigned char cval, fcr = 0; unsigned long flags; unsigned int baud, quot; @@ -2435,7 +2450,8 @@ serial8250_set_ldisc(struct uart_port *port, int new) void serial8250_do_pm(struct uart_port *port, unsigned int state, unsigned int oldstate) { - struct uart_8250_port *p = (struct uart_8250_port *)port; + struct uart_8250_port *p = + container_of(port, struct uart_8250_port, port); serial8250_set_sleep(p, state != 0); } @@ -2566,7 +2582,8 @@ static void serial8250_release_rsa_resource(struct uart_8250_port *up) static void serial8250_release_port(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); serial8250_release_std_resource(up); if (up->port.type == PORT_RSA) @@ -2575,7 +2592,8 @@ static void serial8250_release_port(struct uart_port *port) static int serial8250_request_port(struct uart_port *port) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); int ret = 0; ret = serial8250_request_std_resource(up); @@ -2590,7 +2608,8 @@ static int serial8250_request_port(struct uart_port *port) static void serial8250_config_port(struct uart_port *port, int flags) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); int probeflags = PROBE_ANY; int ret; @@ -2771,7 +2790,8 @@ serial8250_register_ports(struct uart_driver *drv, struct device *dev) static void serial8250_console_putchar(struct uart_port *port, int ch) { - struct uart_8250_port *up = (struct uart_8250_port *)port; + struct uart_8250_port *up = + container_of(port, struct uart_8250_port, port); wait_for_xmitr(up, UART_LSR_THRE); serial_out(up, UART_TX, ch); -- cgit v1.2.3 From a3ae0fc34f58e7163b7724feb3d77aa4603f0dc3 Mon Sep 17 00:00:00 2001 From: Jamie Iles Date: Wed, 1 Dec 2010 23:39:36 +0000 Subject: 8250: add a UPIO_DWAPB32 for 32 bit accesses Some platforms contain a Synopsys DesignWare APB UART that is attached to a 32-bit APB bus where sub-word accesses are not allowed. Add a new IO type (UPIO_DWAPB32) that performs 32 bit acccesses to the UART. v2: - don't test for 32 bit in the output fast path, provide a separate dwabp32_serial_out() function. Refactor dwabp_serial_out() so that we can reuse the LCR saving code. v3: - rebased on top of "8250: use container_of() instead of casting" Signed-off-by: Jamie Iles Signed-off-by: Greg Kroah-Hartman --- drivers/serial/8250.c | 51 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 12 deletions(-) (limited to 'drivers/serial/8250.c') diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c index 0b4ce47d987..9839199c1d1 100644 --- a/drivers/serial/8250.c +++ b/drivers/serial/8250.c @@ -454,22 +454,40 @@ static void tsi_serial_out(struct uart_port *p, int offset, int value) writeb(value, p->membase + offset); } +/* Save the LCR value so it can be re-written when a Busy Detect IRQ occurs. */ +static inline void dwapb_save_out_value(struct uart_port *p, int offset, + int value) +{ + struct uart_8250_port *up = + container_of(p, struct uart_8250_port, port); + + if (offset == UART_LCR) + up->lcr = value; +} + +/* Read the IER to ensure any interrupt is cleared before returning from ISR. */ +static inline void dwapb_check_clear_ier(struct uart_port *p, int offset) +{ + if (offset == UART_TX || offset == UART_IER) + p->serial_in(p, UART_IER); +} + static void dwapb_serial_out(struct uart_port *p, int offset, int value) { int save_offset = offset; offset = map_8250_out_reg(p, offset) << p->regshift; - /* Save the LCR value so it can be re-written when a - * Busy Detect interrupt occurs. */ - if (save_offset == UART_LCR) { - struct uart_8250_port *up = - container_of(p, struct uart_8250_port, port); - up->lcr = value; - } + dwapb_save_out_value(p, save_offset, value); writeb(value, p->membase + offset); - /* Read the IER to ensure any interrupt is cleared before - * returning from ISR. */ - if (save_offset == UART_TX || save_offset == UART_IER) - value = p->serial_in(p, UART_IER); + dwapb_check_clear_ier(p, save_offset); +} + +static void dwapb32_serial_out(struct uart_port *p, int offset, int value) +{ + int save_offset = offset; + offset = map_8250_out_reg(p, offset) << p->regshift; + dwapb_save_out_value(p, save_offset, value); + writel(value, p->membase + offset); + dwapb_check_clear_ier(p, save_offset); } static unsigned int io_serial_in(struct uart_port *p, int offset) @@ -520,6 +538,11 @@ static void set_io_from_upio(struct uart_port *p) p->serial_out = dwapb_serial_out; break; + case UPIO_DWAPB32: + p->serial_in = mem32_serial_in; + p->serial_out = dwapb32_serial_out; + break; + default: p->serial_in = io_serial_in; p->serial_out = io_serial_out; @@ -538,6 +561,7 @@ serial_out_sync(struct uart_8250_port *up, int offset, int value) case UPIO_MEM32: case UPIO_AU: case UPIO_DWAPB: + case UPIO_DWAPB32: p->serial_out(p, offset, value); p->serial_in(p, UART_LCR); /* safe, no side-effects */ break; @@ -1587,7 +1611,8 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id) handled = 1; end = NULL; - } else if (up->port.iotype == UPIO_DWAPB && + } else if ((up->port.iotype == UPIO_DWAPB || + up->port.iotype == UPIO_DWAPB32) && (iir & UART_IIR_BUSY) == UART_IIR_BUSY) { /* The DesignWare APB UART has an Busy Detect (0x07) * interrupt meaning an LCR write attempt occured while the @@ -2492,6 +2517,7 @@ static int serial8250_request_std_resource(struct uart_8250_port *up) case UPIO_MEM32: case UPIO_MEM: case UPIO_DWAPB: + case UPIO_DWAPB32: if (!up->port.mapbase) break; @@ -2529,6 +2555,7 @@ static void serial8250_release_std_resource(struct uart_8250_port *up) case UPIO_MEM32: case UPIO_MEM: case UPIO_DWAPB: + case UPIO_DWAPB32: if (!up->port.mapbase) break; -- cgit v1.2.3 From e4f05af136016958f52455da3070ca6622439b10 Mon Sep 17 00:00:00 2001 From: Ondrej Puzman Date: Sat, 4 Dec 2010 21:17:38 +0100 Subject: 8250: fix uninitialized FIFOs 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 Signed-off-by: Greg Kroah-Hartman --- drivers/serial/8250.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/serial/8250.c') diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c index 9839199c1d1..8747215dc97 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; -- cgit v1.2.3