RE: [PATCH 11/13 v2] serial: Add OMAP high-speed UART driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Govindraj,

> -----Original Message-----
> From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Govindraj.R
> Sent: Wednesday, September 22, 2010 10:15 AM
> To: linux-omap@xxxxxxxxxxxxxxx
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-serial@xxxxxxxxxxxxxxx;
> Kevin Hilman; Tony Lindgren
> Subject: [PATCH 11/13 v2] serial: Add OMAP high-speed UART driver
>
> This patch adds driver support for OMAP2/3/4 high speed UART.
>
> The driver is made separate from 8250 driver as we cannot
> over load 8250 driver with omap platform specific configuration for
> features like DMA, it makes easier to implement features like DMA and
> hardware flow control and software flow control configuration with
> this driver as required for the omap-platform.
> This patch involves only the core driver and its dependent.
>
> Cc: Tony Lindgren <tony@xxxxxxxxxxx>
> Tested-by: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Govindraj.R <govindraj.raja@xxxxxx>
> ---
>  arch/arm/plat-omap/include/plat/omap-serial.h |  129 +++
>  drivers/serial/Kconfig                        |   27 +
>  drivers/serial/Makefile                       |    1 +
>  drivers/serial/omap-serial.c                  | 1332
> +++++++++++++++++++++++++
>  include/linux/serial_core.h                   |    3 +
>  5 files changed, 1492 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/plat-omap/include/plat/omap-serial.h
>  create mode 100644 drivers/serial/omap-serial.c
>
> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h
> b/arch/arm/plat-omap/include/plat/omap-serial.h
> new file mode 100644
> index 0000000..0d6f076
> --- /dev/null
> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
> @@ -0,0 +1,129 @@
> +/*
> + * Driver for OMAP-UART controller.
> + * Based on drivers/serial/8250.c
> + *
> + * Copyright (C) 2010 Texas Instruments.
> + *
> + * Authors:
> + *   Govindraj R     <govindraj.raja@xxxxxx>
> + *   Thara Gopinath  <thara@xxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __OMAP_SERIAL_H__
> +#define __OMAP_SERIAL_H__
> +
> +#include <linux/serial_core.h>
> +#include <linux/platform_device.h>
> +
> +#include <plat/control.h>
> +#include <plat/mux.h>
> +
> +#define DRIVER_NAME  "omap-hsuart"

Is there a reason why you keep 2 names in the driver?

You use this for the platform_driver struct "serial_omap_driver",
Meanwhile you send a driver name of "OMAP-SERIAL" to uart_driver struct...

Also IMHO, if you're just going to use this once, then it makes no sense to
have this macro.

> +
> +/*
> + * Use tty device name as ttyO, [O -> OMAP]
> + * in bootargs we specify as console=ttyO0 if uart1
> + * is used as console uart.
> + */
> +#define OMAP_SERIAL_NAME     "ttyO"
> +
> +#define OMAP_MDR1_DISABLE    0x07
> +#define OMAP_MDR1_MODE13X    0x03
> +#define OMAP_MDR1_MODE16X    0x00
> +#define OMAP_MODE13X_SPEED   230400
> +
> +/*
> + * LCR = 0XBF: Switch to Configuration Mode B.
> + * In configuration mode b allow access
> + * to EFR,DLL,DLH.
> + * Reference OMAP TRM Chapter 17
> + * Section: 1.4.3 Mode Selection
> + */
> +#define OMAP_UART_LCR_CONF_MDB       0XBF
> +
> +/* WER = 0x7F
> + * Enable module level wakeup in WER reg
> + */
> +#define OMAP_UART_WER_MOD_WKUP       0X7F
> +
> +/* Enable XON/XOFF flow control on output */
> +#define OMAP_UART_SW_TX              0x04
> +
> +/* Enable XON/XOFF flow control on input */
> +#define OMAP_UART_SW_RX              0x04
> +
> +#define OMAP_UART_SYSC_RESET 0X07
> +#define OMAP_UART_TCR_TRIG   0X0F
> +#define OMAP_UART_SW_CLR     0XF0
> +#define OMAP_UART_FIFO_CLR   0X06
> +
> +#define OMAP_UART_DMA_CH_FREE        -1
> +
> +#define RX_TIMEOUT           (3 * HZ)
> +#define OMAP_MAX_HSUART_PORTS        4
> +
> +#define MSR_SAVE_FLAGS               UART_MSR_ANY_DELTA
> +
> +struct omap_uart_port_info {
> +     bool                    dma_enabled;    /* To specify DMA Mode */
> +     unsigned int            uartclk;        /* UART clock rate */
> +     void __iomem            *membase;       /* ioremap cookie or NULL */
> +     resource_size_t         mapbase;        /* resource base */
> +     unsigned long           irqflags;       /* request_irq flags */
> +     upf_t                   flags;          /* UPF_* flags */
> +};
> +
> +struct uart_omap_dma {
> +     u8                      uart_dma_tx;
> +     u8                      uart_dma_rx;
> +     int                     rx_dma_channel;
> +     int                     tx_dma_channel;
> +     dma_addr_t              rx_buf_dma_phys;
> +     dma_addr_t              tx_buf_dma_phys;
> +     unsigned int            uart_base;
> +     /*
> +      * Buffer for rx dma.It is not required for tx because the buffer
> +      * comes from port structure.
> +      */
> +     unsigned char           *rx_buf;
> +     unsigned int            prev_rx_dma_pos;
> +     int                     tx_buf_size;
> +     int                     tx_dma_used;
> +     int                     rx_dma_used;
> +     spinlock_t              tx_lock;
> +     spinlock_t              rx_lock;
> +     /* timer to poll activity on rx dma */
> +     struct timer_list       rx_timer;
> +     int                     rx_buf_size;
> +     int                     rx_timeout;
> +};
> +
> +struct uart_omap_port {
> +     struct uart_port        port;
> +     struct uart_omap_dma    uart_dma;
> +     struct platform_device  *pdev;
> +
> +     unsigned char           ier;
> +     unsigned char           lcr;
> +     unsigned char           mcr;
> +     unsigned char           fcr;
> +     unsigned char           efr;
> +
> +     int                     use_dma;
> +     /*
> +      * Some bits in registers are cleared on a read, so they must
> +      * be saved whenever the register is read but the bits will not
> +      * be immediately processed.
> +      */
> +     unsigned int            lsr_break_flag;
> +     unsigned char           msr_saved_flags;
> +     char                    name[20];
> +     unsigned long           port_activity;
> +};
> +
> +#endif /* __OMAP_SERIAL_H__ */
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 12900f7..8d8b975 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -1416,6 +1416,33 @@ config SERIAL_OF_PLATFORM
>         Currently, only 8250 compatible ports are supported, but
>         others can easily be added.
>
> +config SERIAL_OMAP
> +     tristate "OMAP serial port support"
> +     depends on ARCH_OMAP2 || ARCH_OMAP3 || ARCH_OMAP4
> +     select SERIAL_CORE
> +     help
> +       If you have a machine based on an Texas Instruments OMAP CPU you
> +       can enable its onboard serial ports by enabling this option.
> +
> +       By enabling this option you take advantage of dma feature
> available
> +       with the omap-serial driver. DMA support can be enabled from
> platform
> +       data.
> +
> +config SERIAL_OMAP_CONSOLE
> +     bool "Console on OMAP serial port"
> +     depends on SERIAL_OMAP
> +     select SERIAL_CORE_CONSOLE
> +     help
> +       Select this option if you would like to use omap serial port as
> +       console.
> +
> +       Even if you say Y here, the currently visible virtual console
> +       (/dev/tty0) will still be used as the system console by default,
> but
> +       you can alter that using a kernel command line option such as
> +       "console=ttyOx". (Try "man bootparam" or see the documentation of
> +       your boot loader about how to pass options to the kernel at
> +       boot time.)
> +
>  config SERIAL_OF_PLATFORM_NWPSERIAL
>       tristate "NWP serial port driver"
>       depends on PPC_OF && PPC_DCR
> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> index 1ca4fd5..c570576 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -88,3 +88,4 @@ obj-$(CONFIG_SERIAL_ALTERA_JTAGUART) +=
> altera_jtaguart.o
>  obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
>  obj-$(CONFIG_SERIAL_MRST_MAX3110)    += mrst_max3110.o
>  obj-$(CONFIG_SERIAL_MFD_HSU) += mfd.o
> +obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
> diff --git a/drivers/serial/omap-serial.c b/drivers/serial/omap-serial.c
> new file mode 100644
> index 0000000..7b8474e
> --- /dev/null
> +++ b/drivers/serial/omap-serial.c
> @@ -0,0 +1,1332 @@
> +/*
> + * Driver for OMAP-UART controller.
> + * Based on drivers/serial/8250.c
> + *
> + * Copyright (C) 2010 Texas Instruments.
> + *
> + * Authors:
> + *   Govindraj R     <govindraj.raja@xxxxxx>
> + *   Thara Gopinath  <thara@xxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * Note: This driver is made seperate from 8250 driver as we cannot
> + * over load 8250 driver with omap platform specific configuration for
> + * features like DMA, it makes easier to implement features like DMA and
> + * hardware flow control and software flow control configuration with
> + * this driver as required for the omap-platform.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/console.h>
> +#include <linux/serial_reg.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include <linux/io.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/clk.h>
> +#include <linux/serial_core.h>
> +
> +#include <asm/irq.h>

Shouldn't this be:

#include <linux/irq.h>

?

> +#include <plat/dma.h>
> +#include <plat/dmtimer.h>
> +#include <plat/omap-serial.h>
> +
> +static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
> +
> +/* Forward declaration of functions */
> +static void uart_tx_dma_callback(int lch, u16 ch_status, void *data);
> +static void serial_omap_rx_timeout(unsigned long uart_no);
> +static int serial_omap_start_rxdma(struct uart_omap_port *up);
> +
> +static inline unsigned int serial_in(struct uart_omap_port *up, int
> offset)
> +{
> +     offset <<= up->port.regshift;
> +     return readw(up->port.membase + offset);
> +}
> +
> +static inline void serial_out(struct uart_omap_port *up, int offset, int
> value)
> +{
> +     offset <<= up->port.regshift;
> +     writew(value, up->port.membase + offset);
> +}
> +
> +static inline void serial_omap_clear_fifos(struct uart_omap_port *up)
> +{
> +     serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO);
> +     serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO |
> +                    UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
> +     serial_out(up, UART_FCR, 0);
> +}
> +
> +/**
> + * serial_omap_get_divisor() - calculate divisor value

I heard sometime back a suggestion that kernel doc type comments aren't
required for static private functions, as nobody is supposed to care about
this driver internals.

Also, the parenthesis after the function name shouldn't be there.

> + * @port: uart port info
> + * @baud: baudrate for which divisor needs to be calculated.
> + *
> + * We have written our own function to get the divisor so as to support
> + * 13x mode. 3Mbps Baudrate as an different divisor.
> + * Reference OMAP TRM Chapter 17:
> + * Table 17-1. UART Mode Baud Rates, Divisor Values, and Error Rates
> + * referring to oversampling - divisor value
> + * baudrate 460,800 to 3,686,400 all have divisor 13
> + * except 3,000,000 which has divisor value 16
> + */
> +static unsigned int
> +serial_omap_get_divisor(struct uart_port *port, unsigned int baud)
> +{
> +     unsigned int divisor;
> +
> +     if (baud > OMAP_MODE13X_SPEED && baud != 3000000)
> +             divisor = 13;
> +     else
> +             divisor = 16;
> +     return port->uartclk/(baud * divisor);
> +}
> +
> +static void serial_omap_stop_rxdma(struct uart_omap_port *up)
> +{
> +     if (up->uart_dma.rx_dma_used) {
> +             del_timer(&up->uart_dma.rx_timer);
> +             omap_stop_dma(up->uart_dma.rx_dma_channel);
> +             omap_free_dma(up->uart_dma.rx_dma_channel);
> +             up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
> +             up->uart_dma.rx_dma_used = false;
> +     }
> +}
> +
> +static void serial_omap_enable_ms(struct uart_port *port)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;

Isn't this a bit error prone?

I mean, How would you ensure that the uart_port and uart_omap_port structs
Will remain compatible?

> +
> +     dev_dbg(up->port.dev, "serial_omap_enable_ms+%d\n", up->pdev->id);
> +     up->ier |= UART_IER_MSI;
> +     serial_out(up, UART_IER, up->ier);
> +}
> +
> +static void serial_omap_stop_tx(struct uart_port *port)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;

Same here and below functions that does the same.

> +
> +     if (up->use_dma &&
> +             up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {
> +             /*
> +              * Check if dma is still active. If yes do nothing,
> +              * return. Else stop dma
> +              */
> +             if (omap_get_dma_active_status(up->uart_dma.tx_dma_channel))
> +                     return;
> +             omap_stop_dma(up->uart_dma.tx_dma_channel);
> +             omap_free_dma(up->uart_dma.tx_dma_channel);
> +             up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
> +     }
> +
> +     if (up->ier & UART_IER_THRI) {
> +             up->ier &= ~UART_IER_THRI;
> +             serial_out(up, UART_IER, up->ier);
> +     }
> +}
> +
> +static void serial_omap_stop_rx(struct uart_port *port)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +
> +     if (up->use_dma)
> +             serial_omap_stop_rxdma(up);
> +     up->ier &= ~UART_IER_RLSI;
> +     up->port.read_status_mask &= ~UART_LSR_DR;
> +     serial_out(up, UART_IER, up->ier);
> +}
> +
> +static inline void receive_chars(struct uart_omap_port *up, int *status)
> +{
> +     struct tty_struct *tty = up->port.state->port.tty;
> +     unsigned int flag;
> +     unsigned char ch, lsr = *status;
> +     int max_count = 256;
> +
> +     do {
> +             if (likely(lsr & UART_LSR_DR))
> +                     ch = serial_in(up, UART_RX);
> +             flag = TTY_NORMAL;
> +             up->port.icount.rx++;
> +
> +             if (unlikely(lsr & UART_LSR_BRK_ERROR_BITS)) {
> +                     /*
> +                      * For statistics only
> +                      */
> +                     if (lsr & UART_LSR_BI) {
> +                             lsr &= ~(UART_LSR_FE | UART_LSR_PE);
> +                             up->port.icount.brk++;
> +                             /*
> +                              * We do the SysRQ and SAK checking
> +                              * here because otherwise the break
> +                              * may get masked by ignore_status_mask
> +                              * or read_status_mask.
> +                              */
> +                             if (uart_handle_break(&up->port))
> +                                     goto ignore_char;
> +                     } else if (lsr & UART_LSR_PE)
> +                             up->port.icount.parity++;
> +                     else if (lsr & UART_LSR_FE)
> +                             up->port.icount.frame++;

AFAIK, Above 2 if conditions require brackets aswell, to keep consistency
And ease of reading the whole 3 cases as part of a single chain of
comparisons.

Also, add a space here, as below if condition doesn't seem to be related.

> +                     if (lsr & UART_LSR_OE)
> +                             up->port.icount.overrun++;
> +
> +                     /*
> +                      * Mask off conditions which should be ignored.
> +                      */
> +                     lsr &= up->port.read_status_mask;
> +
> +#ifdef CONFIG_SERIAL_OMAP_CONSOLE
> +                     if (up->port.line == up->port.cons->index) {
> +                             /* Recover the break flag from console xmit */
> +                             lsr |= up->lsr_break_flag;
> +                             up->lsr_break_flag = 0;
> +                     }
> +#endif
> +                     if (lsr & UART_LSR_BI)
> +                             flag = TTY_BREAK;
> +                     else if (lsr & UART_LSR_PE)
> +                             flag = TTY_PARITY;
> +                     else if (lsr & UART_LSR_FE)
> +                             flag = TTY_FRAME;
> +             }
> +
> +             if (uart_handle_sysrq_char(&up->port, ch))
> +                     goto ignore_char;
> +             uart_insert_char(&up->port, lsr, UART_LSR_OE, ch, flag);
> +ignore_char:
> +             lsr = serial_in(up, UART_LSR);
> +     } while ((lsr & (UART_LSR_DR | UART_LSR_BI)) && (max_count-- > 0));
> +     spin_unlock(&up->port.lock);
> +     tty_flip_buffer_push(tty);
> +     spin_lock(&up->port.lock);

This is very odd... So you unlock a spinlock, and then lock it back?

Have you activated spinlock debug in menuconfig to test locking?

> +}
> +
> +static void transmit_chars(struct uart_omap_port *up)
> +{
> +     struct circ_buf *xmit = &up->port.state->xmit;
> +     int count;
> +
> +     if (up->port.x_char) {
> +             serial_out(up, UART_TX, up->port.x_char);
> +             up->port.icount.tx++;
> +             up->port.x_char = 0;
> +             return;
> +     }
> +     if (uart_circ_empty(xmit) || uart_tx_stopped(&up->port)) {
> +             serial_omap_stop_tx(&up->port);
> +             return;
> +     }
> +     count = up->port.fifosize / 4;
> +     do {
> +             serial_out(up, UART_TX, xmit->buf[xmit->tail]);
> +             xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> +             up->port.icount.tx++;
> +             if (uart_circ_empty(xmit))
> +                     break;
> +     } while (--count > 0);
> +
> +     if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> +             uart_write_wakeup(&up->port);
> +
> +     if (uart_circ_empty(xmit))
> +             serial_omap_stop_tx(&up->port);
> +}
> +
> +static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
> +{
> +     if (!(up->ier & UART_IER_THRI)) {
> +             up->ier |= UART_IER_THRI;
> +             serial_out(up, UART_IER, up->ier);
> +     }
> +}
> +
> +static void serial_omap_start_tx(struct uart_port *port)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +     struct circ_buf *xmit;
> +     unsigned int start;
> +     int ret = 0;

This var is useless.

> +
> +     if (!up->use_dma) {
> +             serial_omap_enable_ier_thri(up);
> +             return;
> +     }
> +
> +     if (up->uart_dma.tx_dma_used)
> +             return;
> +
> +     xmit = &up->port.state->xmit;
> +
> +     if (up->uart_dma.tx_dma_channel == OMAP_UART_DMA_CH_FREE) {
> +             ret = omap_request_dma(up->uart_dma.uart_dma_tx,
> +                             "UART Tx DMA",
> +                             (void *)uart_tx_dma_callback, up,
> +                             &(up->uart_dma.tx_dma_channel));
> +
> +             if (ret < 0) {

As you're not using the return value anywhere, why not doing this instead?

                if (omap_request_dma(up->uart_dma.uart_dma_tx,
                                "UART Tx DMA",
                                (void *)uart_tx_dma_callback, up,
                                &(up->uart_dma.tx_dma_channel)) < 0) {

> +                     serial_omap_enable_ier_thri(up);
> +                     return;
> +             }
> +     }
> +     spin_lock(&(up->uart_dma.tx_lock));
> +     up->uart_dma.tx_dma_used = true;
> +     spin_unlock(&(up->uart_dma.tx_lock));
> +
> +     start = up->uart_dma.tx_buf_dma_phys +
> +                             (xmit->tail & (UART_XMIT_SIZE - 1));
> +
> +     up->uart_dma.tx_buf_size = uart_circ_chars_pending(xmit);
> +     /*
> +      * It is a circular buffer. See if the buffer has wounded back.
> +      * If yes it will have to be transferred in two separate dma
> +      * transfers
> +      */
> +     if (start + up->uart_dma.tx_buf_size >=
> +                     up->uart_dma.tx_buf_dma_phys + UART_XMIT_SIZE)
> +             up->uart_dma.tx_buf_size =
> +                     (up->uart_dma.tx_buf_dma_phys +
> +                     UART_XMIT_SIZE) - start;
> +
> +     omap_set_dma_dest_params(up->uart_dma.tx_dma_channel, 0,
> +                             OMAP_DMA_AMODE_CONSTANT,
> +                             up->uart_dma.uart_base, 0, 0);
> +     omap_set_dma_src_params(up->uart_dma.tx_dma_channel, 0,
> +                             OMAP_DMA_AMODE_POST_INC, start, 0, 0);
> +     omap_set_dma_transfer_params(up->uart_dma.tx_dma_channel,
> +                             OMAP_DMA_DATA_TYPE_S8,
> +                             up->uart_dma.tx_buf_size, 1,
> +                             OMAP_DMA_SYNC_ELEMENT,
> +                             up->uart_dma.uart_dma_tx, 0);
> +     /* FIXME: Cache maintenance needed here? */
> +     omap_start_dma(up->uart_dma.tx_dma_channel);
> +}
> +
> +static unsigned int check_modem_status(struct uart_omap_port *up)
> +{
> +     int status;

This should be unsigned int.

Also, space here.

> +     status = serial_in(up, UART_MSR);
> +
> +     status |= up->msr_saved_flags;
> +     up->msr_saved_flags = 0;
> +
> +     if ((status & UART_MSR_ANY_DELTA) == 0)
> +             return status;

Space here too.

> +     if (status & UART_MSR_ANY_DELTA && up->ier & UART_IER_MSI &&
> +         up->port.state != NULL) {
> +             if (status & UART_MSR_TERI)
> +                     up->port.icount.rng++;
> +             if (status & UART_MSR_DDSR)
> +                     up->port.icount.dsr++;
> +             if (status & UART_MSR_DDCD)
> +                     uart_handle_dcd_change
> +                             (&up->port, status & UART_MSR_DCD);
> +             if (status & UART_MSR_DCTS)
> +                     uart_handle_cts_change
> +                             (&up->port, status & UART_MSR_CTS);
> +             wake_up_interruptible(&up->port.state->port.delta_msr_wait);
> +     }
> +
> +     return status;
> +}
> +
> +/**
> + * serial_omap_irq() - This handles the interrupt from one port
> + * @irq: uart port irq number
> + * @dev_id: uart port info
> + */
> +static inline irqreturn_t serial_omap_irq(int irq, void *dev_id)
> +{
> +     struct uart_omap_port *up = dev_id;
> +     unsigned int iir, lsr;
> +     unsigned long flags;
> +
> +     iir = serial_in(up, UART_IIR);
> +     if (iir & UART_IIR_NO_INT)
> +             return IRQ_NONE;
> +
> +     spin_lock_irqsave(&up->port.lock, flags);

You're in interrupt context already, why do you need to disable interrupts?

> +     lsr = serial_in(up, UART_LSR);
> +     if (iir & UART_IIR_RLSI) {
> +             if (!up->use_dma) {
> +                     if (lsr & UART_LSR_DR)
> +                             receive_chars(up, &lsr);
> +             } else {
> +                     up->ier &= ~(UART_IER_RDI | UART_IER_RLSI);
> +                     serial_out(up, UART_IER, up->ier);
> +                     if (serial_omap_start_rxdma(up) != 0)
> +                             if (lsr & UART_LSR_DR)
> +                                     receive_chars(up, &lsr);

Shouldn't be simpler to just do:

                        if ((serial_omap_start_rxdma(up) != 0) &&
                                        (lsr & UART_LSR_DR))
                                receive_chars(up, &lsr);

?

> +             }
> +     }
> +
> +     check_modem_status(up);
> +     if ((lsr & UART_LSR_THRE) && (iir & UART_IIR_THRI))
> +             transmit_chars(up);
> +
> +     spin_unlock_irqrestore(&up->port.lock, flags);
> +     up->port_activity = jiffies;
> +     return IRQ_HANDLED;
> +}
> +
> +static unsigned int serial_omap_tx_empty(struct uart_port *port)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +     unsigned long flags = 0;
> +     unsigned int ret = 0;
> +
> +     dev_dbg(up->port.dev, "serial_omap_tx_empty+%d\n", up->pdev->id);
> +     spin_lock_irqsave(&up->port.lock, flags);
> +     ret = serial_in(up, UART_LSR) & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
> +     spin_unlock_irqrestore(&up->port.lock, flags);
> +
> +     return ret;
> +}
> +
> +static unsigned int serial_omap_get_mctrl(struct uart_port *port)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +     unsigned char status;
> +     unsigned int ret = 0;
> +
> +     status = check_modem_status(up);
> +     dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->pdev->id);
> +
> +     if (status & UART_MSR_DCD)
> +             ret |= TIOCM_CAR;
> +     if (status & UART_MSR_RI)
> +             ret |= TIOCM_RNG;
> +     if (status & UART_MSR_DSR)
> +             ret |= TIOCM_DSR;
> +     if (status & UART_MSR_CTS)
> +             ret |= TIOCM_CTS;
> +     return ret;
> +}
> +
> +static void serial_omap_set_mctrl(struct uart_port *port, unsigned int
> mctrl)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +     unsigned char mcr = 0;
> +
> +     dev_dbg(up->port.dev, "serial_omap_set_mctrl+%d\n", up->pdev->id);
> +     if (mctrl & TIOCM_RTS)
> +             mcr |= UART_MCR_RTS;
> +     if (mctrl & TIOCM_DTR)
> +             mcr |= UART_MCR_DTR;
> +     if (mctrl & TIOCM_OUT1)
> +             mcr |= UART_MCR_OUT1;
> +     if (mctrl & TIOCM_OUT2)
> +             mcr |= UART_MCR_OUT2;
> +     if (mctrl & TIOCM_LOOP)
> +             mcr |= UART_MCR_LOOP;
> +
> +     mcr |= up->mcr;
> +     serial_out(up, UART_MCR, mcr);
> +}
> +
> +static void serial_omap_break_ctl(struct uart_port *port, int
> break_state)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +     unsigned long flags = 0;
> +
> +     dev_dbg(up->port.dev, "serial_omap_break_ctl+%d\n", up->pdev->id);
> +     spin_lock_irqsave(&up->port.lock, flags);
> +     if (break_state == -1)
> +             up->lcr |= UART_LCR_SBC;
> +     else
> +             up->lcr &= ~UART_LCR_SBC;
> +     serial_out(up, UART_LCR, up->lcr);
> +     spin_unlock_irqrestore(&up->port.lock, flags);
> +}
> +
> +static int serial_omap_startup(struct uart_port *port)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +     unsigned long flags = 0;
> +     int retval;
> +
> +     /*
> +      * Allocate the IRQ
> +      */
> +     retval = request_irq(up->port.irq, serial_omap_irq, up-
> >port.irqflags,
> +                             up->name, up);
> +     if (retval)
> +             return retval;
> +
> +     dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->pdev->id);
> +
> +     /*
> +      * Clear the FIFO buffers and disable them.
> +      * (they will be reenabled in set_termios())
> +      */
> +     serial_omap_clear_fifos(up);
> +     /* For Hardware flow control */
> +     serial_out(up, UART_MCR, UART_MCR_RTS);
> +
> +     /*
> +      * Clear the interrupt registers.
> +      */
> +     (void) serial_in(up, UART_LSR);
> +     if (serial_in(up, UART_LSR) & UART_LSR_DR)
> +             (void) serial_in(up, UART_RX);
> +     (void) serial_in(up, UART_IIR);
> +     (void) serial_in(up, UART_MSR);

Why the void typecasting?

> +
> +     /*
> +      * Now, initialize the UART
> +      */
> +     serial_out(up, UART_LCR, UART_LCR_WLEN8);
> +     spin_lock_irqsave(&up->port.lock, flags);
> +     /*
> +      * Most PC uarts need OUT2 raised to enable interrupts.
> +      */
> +     up->port.mctrl |= TIOCM_OUT2;
> +     serial_omap_set_mctrl(&up->port, up->port.mctrl);
> +     spin_unlock_irqrestore(&up->port.lock, flags);
> +
> +     up->msr_saved_flags = 0;
> +     if (up->use_dma) {
> +             free_page((unsigned long)up->port.state->xmit.buf);
> +             up->port.state->xmit.buf = dma_alloc_coherent(NULL,
> +                     UART_XMIT_SIZE,
> +                     (dma_addr_t *)&(up->uart_dma.tx_buf_dma_phys),
> +                     0);
> +             init_timer(&(up->uart_dma.rx_timer));
> +             up->uart_dma.rx_timer.function = serial_omap_rx_timeout;
> +             up->uart_dma.rx_timer.data = up->pdev->id;
> +             /* Currently the buffer size is 4KB. Can increase it */
> +             up->uart_dma.rx_buf = dma_alloc_coherent(NULL,
> +                     up->uart_dma.rx_buf_size,
> +                     (dma_addr_t *)&(up->uart_dma.rx_buf_dma_phys), 0);
> +     }
> +     /*
> +      * Finally, enable interrupts. Note: Modem status interrupts
> +      * are set via set_termios(), which will be occurring imminently
> +      * anyway, so we don't enable them here.
> +      */
> +     up->ier = UART_IER_RLSI | UART_IER_RDI;
> +     serial_out(up, UART_IER, up->ier);
> +
> +     up->port_activity = jiffies;
> +     return 0;
> +}
> +
> +static void serial_omap_shutdown(struct uart_port *port)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +     unsigned long flags = 0;
> +
> +     dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->pdev->id);
> +     /*
> +      * Disable interrupts from this port
> +      */
> +     up->ier = 0;
> +     serial_out(up, UART_IER, 0);
> +
> +     spin_lock_irqsave(&up->port.lock, flags);
> +     up->port.mctrl &= ~TIOCM_OUT2;
> +     serial_omap_set_mctrl(&up->port, up->port.mctrl);
> +     spin_unlock_irqrestore(&up->port.lock, flags);
> +
> +     /*
> +      * Disable break condition and FIFOs
> +      */
> +     serial_out(up, UART_LCR, serial_in(up, UART_LCR) & ~UART_LCR_SBC);
> +     serial_omap_clear_fifos(up);
> +
> +     /*
> +      * Read data port to reset things, and then free the irq
> +      */
> +     if (serial_in(up, UART_LSR) & UART_LSR_DR)
> +             (void) serial_in(up, UART_RX);
> +     if (up->use_dma) {
> +             dma_free_coherent(up->port.dev,
> +                     UART_XMIT_SIZE, up->port.state->xmit.buf,
> +                     up->uart_dma.tx_buf_dma_phys);
> +             up->port.state->xmit.buf = NULL;
> +             serial_omap_stop_rx(port);
> +             dma_free_coherent(up->port.dev,
> +                     up->uart_dma.rx_buf_size, up->uart_dma.rx_buf,
> +                     up->uart_dma.rx_buf_dma_phys);
> +             up->uart_dma.rx_buf = NULL;
> +     }
> +     free_irq(up->port.irq, up);
> +}
> +
> +static inline void
> +serial_omap_configure_xonxoff
> +             (struct uart_omap_port *up, struct ktermios *termios)
> +{
> +     unsigned char efr = 0;
> +
> +     up->lcr = serial_in(up, UART_LCR);
> +     serial_out(up, UART_LCR, OMAP_UART_LCR_CONF_MDB);
> +     up->efr = serial_in(up, UART_EFR);
> +     serial_out(up, UART_EFR, up->efr & ~UART_EFR_ECB);
> +
> +     serial_out(up, UART_XON1, termios->c_cc[VSTART]);
> +     serial_out(up, UART_XOFF1, termios->c_cc[VSTOP]);
> +
> +     /* clear SW control mode bits */
> +     efr = up->efr;
> +     efr &= OMAP_UART_SW_CLR;
> +
> +     /*
> +      * IXON Flag:
> +      * Enable XON/XOFF flow control on output.
> +      * Transmit XON1, XOFF1
> +      */
> +     if (termios->c_iflag & IXON)
> +             efr |= OMAP_UART_SW_TX;
> +
> +     /*
> +      * IXOFF Flag:
> +      * Enable XON/XOFF flow control on input.
> +      * Receiver compares XON1, XOFF1.
> +      */
> +     if (termios->c_iflag & IXOFF)
> +             efr |= OMAP_UART_SW_RX;
> +
> +     serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
> +     serial_out(up, UART_LCR, UART_LCR_DLAB);
> +
> +     up->mcr = serial_in(up, UART_MCR);
> +
> +     /*
> +      * IXANY Flag:
> +      * Enable any character to restart output.
> +      * Operation resumes after receiving any
> +      * character after recognition of the XOFF character
> +      */
> +     if (termios->c_iflag & IXANY)
> +             up->mcr |= UART_MCR_XONANY;
> +
> +     serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
> +     serial_out(up, UART_LCR, OMAP_UART_LCR_CONF_MDB);
> +     serial_out(up, UART_TI752_TCR, OMAP_UART_TCR_TRIG);
> +     /* Enable special char function UARTi.EFR_REG[5] and
> +      * load the new software flow control mode IXON or IXOFF
> +      * and restore the UARTi.EFR_REG[4] ENHANCED_EN value.
> +      */
> +     serial_out(up, UART_EFR, efr | UART_EFR_SCD);
> +     serial_out(up, UART_LCR, UART_LCR_DLAB);
> +
> +     serial_out(up, UART_MCR, up->mcr & ~UART_MCR_TCRTLR);
> +     serial_out(up, UART_LCR, up->lcr);
> +}
> +
> +static void
> +serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
> +                     struct ktermios *old)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +     unsigned char cval = 0;
> +     unsigned char efr = 0;
> +     unsigned long flags = 0;
> +     unsigned int baud, quot;
> +
> +     switch (termios->c_cflag & CSIZE) {
> +     case CS5:
> +             cval = UART_LCR_WLEN5;
> +             break;
> +     case CS6:
> +             cval = UART_LCR_WLEN6;
> +             break;
> +     case CS7:
> +             cval = UART_LCR_WLEN7;
> +             break;
> +     default:
> +     case CS8:
> +             cval = UART_LCR_WLEN8;
> +             break;
> +     }
> +
> +     if (termios->c_cflag & CSTOPB)
> +             cval |= UART_LCR_STOP;
> +     if (termios->c_cflag & PARENB)
> +             cval |= UART_LCR_PARITY;
> +     if (!(termios->c_cflag & PARODD))
> +             cval |= UART_LCR_EPAR;
> +
> +     /*
> +      * Ask the core to calculate the divisor for us.
> +      */
> +
> +     baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/13);
> +     quot = serial_omap_get_divisor(port, baud);
> +
> +     up->fcr = UART_FCR_R_TRIG_01 | UART_FCR_T_TRIG_01 |
> +                     UART_FCR_ENABLE_FIFO;
> +     if (up->use_dma)
> +             up->fcr |= UART_FCR_DMA_SELECT;
> +
> +     /*
> +      * Ok, we're now changing the port state. Do it with
> +      * interrupts disabled.
> +      */
> +     spin_lock_irqsave(&up->port.lock, flags);
> +
> +     /*
> +      * Update the per-port timeout.
> +      */
> +     uart_update_timeout(port, termios->c_cflag, baud);
> +
> +     up->port.read_status_mask = UART_LSR_OE | UART_LSR_THRE |
> UART_LSR_DR;
> +     if (termios->c_iflag & INPCK)
> +             up->port.read_status_mask |= UART_LSR_FE | UART_LSR_PE;
> +     if (termios->c_iflag & (BRKINT | PARMRK))
> +             up->port.read_status_mask |= UART_LSR_BI;
> +
> +     /*
> +      * Characters to ignore
> +      */
> +     up->port.ignore_status_mask = 0;
> +     if (termios->c_iflag & IGNPAR)
> +             up->port.ignore_status_mask |= UART_LSR_PE | UART_LSR_FE;
> +     if (termios->c_iflag & IGNBRK) {
> +             up->port.ignore_status_mask |= UART_LSR_BI;
> +             /*
> +              * If we're ignoring parity and break indicators,
> +              * ignore overruns too (for real raw support).
> +              */
> +             if (termios->c_iflag & IGNPAR)
> +                     up->port.ignore_status_mask |= UART_LSR_OE;
> +     }
> +
> +     /*
> +      * ignore all characters if CREAD is not set
> +      */
> +     if ((termios->c_cflag & CREAD) == 0)
> +             up->port.ignore_status_mask |= UART_LSR_DR;
> +
> +     /*
> +      * Modem status interrupts
> +      */
> +     up->ier &= ~UART_IER_MSI;
> +     if (UART_ENABLE_MS(&up->port, termios->c_cflag))
> +             up->ier |= UART_IER_MSI;
> +     serial_out(up, UART_IER, up->ier);
> +     serial_out(up, UART_LCR, cval);         /* reset DLAB */
> +
> +     /* FIFOs and DMA Settings */
> +
> +     /* FCR can be changed only when the
> +      * baud clock is not running
> +      * DLL_REG and DLH_REG set to 0.
> +      */
> +     serial_out(up, UART_LCR, UART_LCR_DLAB);
> +     serial_out(up, UART_DLL, 0);
> +     serial_out(up, UART_DLM, 0);
> +     serial_out(up, UART_LCR, 0);
> +
> +     serial_out(up, UART_LCR, OMAP_UART_LCR_CONF_MDB);
> +
> +     up->efr = serial_in(up, UART_EFR);
> +     serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
> +
> +     serial_out(up, UART_LCR, UART_LCR_DLAB);
> +     up->mcr = serial_in(up, UART_MCR);
> +     serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
> +     /* FIFO ENABLE, DMA MODE */
> +     serial_out(up, UART_FCR, up->fcr);
> +     serial_out(up, UART_LCR, OMAP_UART_LCR_CONF_MDB);
> +
> +     if (up->use_dma) {
> +             serial_out(up, UART_TI752_TLR, 0);
> +             serial_out(up, UART_OMAP_SCR,
> +                     (UART_FCR_TRIGGER_4 | UART_FCR_TRIGGER_8));
> +     }
> +
> +     serial_out(up, UART_EFR, up->efr);
> +     serial_out(up, UART_LCR, UART_LCR_DLAB);
> +     serial_out(up, UART_MCR, up->mcr);
> +
> +     /* Protocol, Baud Rate, and Interrupt Settings */
> +
> +     serial_out(up, UART_OMAP_MDR1, OMAP_MDR1_DISABLE);
> +     serial_out(up, UART_LCR, OMAP_UART_LCR_CONF_MDB);
> +
> +     up->efr = serial_in(up, UART_EFR);
> +     serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
> +
> +     serial_out(up, UART_LCR, 0);
> +     serial_out(up, UART_IER, 0);
> +     serial_out(up, UART_LCR, OMAP_UART_LCR_CONF_MDB);
> +
> +     serial_out(up, UART_DLL, quot & 0xff);          /* LS of divisor */
> +     serial_out(up, UART_DLM, quot >> 8);            /* MS of divisor */
> +
> +     serial_out(up, UART_LCR, 0);
> +     serial_out(up, UART_IER, up->ier);
> +     serial_out(up, UART_LCR, OMAP_UART_LCR_CONF_MDB);
> +
> +     serial_out(up, UART_EFR, up->efr);
> +     serial_out(up, UART_LCR, cval);
> +
> +     if (baud > 230400 && baud != 3000000)
> +             serial_out(up, UART_OMAP_MDR1, OMAP_MDR1_MODE13X);
> +     else
> +             serial_out(up, UART_OMAP_MDR1, OMAP_MDR1_MODE16X);
> +
> +     /* Hardware Flow Control Configuration */
> +
> +     if (termios->c_cflag & CRTSCTS) {
> +             efr |= (UART_EFR_CTS | UART_EFR_RTS);
> +             serial_out(up, UART_LCR, UART_LCR_DLAB);
> +
> +             up->mcr = serial_in(up, UART_MCR);
> +             serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
> +
> +             serial_out(up, UART_LCR, OMAP_UART_LCR_CONF_MDB);
> +             up->efr = serial_in(up, UART_EFR);
> +             serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
> +
> +             serial_out(up, UART_TI752_TCR, OMAP_UART_TCR_TRIG);
> +             serial_out(up, UART_EFR, efr); /* Enable AUTORTS and AUTOCTS
> */
> +             serial_out(up, UART_LCR, UART_LCR_DLAB);
> +             serial_out(up, UART_MCR, up->mcr | UART_MCR_RTS);
> +             serial_out(up, UART_LCR, cval);
> +     }
> +
> +     serial_omap_set_mctrl(&up->port, up->port.mctrl);
> +     /* Software Flow Control Configuration */
> +     if (termios->c_iflag & (IXON | IXOFF))
> +             serial_omap_configure_xonxoff(up, termios);
> +
> +     spin_unlock_irqrestore(&up->port.lock, flags);
> +     dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->pdev->id);
> +}
> +
> +static void
> +serial_omap_pm(struct uart_port *port, unsigned int state,
> +            unsigned int oldstate)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +     unsigned char efr;
> +
> +     dev_dbg(up->port.dev, "serial_omap_pm+%d\n", up->pdev->id);
> +     serial_out(up, UART_LCR, OMAP_UART_LCR_CONF_MDB);
> +     efr = serial_in(up, UART_EFR);
> +     serial_out(up, UART_EFR, efr | UART_EFR_ECB);
> +     serial_out(up, UART_LCR, 0);
> +
> +     serial_out(up, UART_IER, (state != 0) ? UART_IERX_SLEEP : 0);
> +     serial_out(up, UART_LCR, OMAP_UART_LCR_CONF_MDB);
> +     serial_out(up, UART_EFR, efr);
> +     serial_out(up, UART_LCR, 0);
> +     /* Enable module level wake up */
> +     serial_out(up, UART_OMAP_WER,
> +             (state != 0) ? OMAP_UART_WER_MOD_WKUP : 0);
> +}
> +
> +static void serial_omap_release_port(struct uart_port *port)
> +{
> +     dev_dbg(port->dev, "serial_omap_release_port+\n");
> +}
> +
> +static int serial_omap_request_port(struct uart_port *port)
> +{
> +     dev_dbg(port->dev, "serial_omap_request_port+\n");
> +     return 0;
> +}
> +
> +static void serial_omap_config_port(struct uart_port *port, int flags)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +
> +     dev_dbg(up->port.dev, "serial_omap_config_port+%d\n",
> +                                                     up->pdev->id);
> +     up->port.type = PORT_OMAP;
> +}
> +
> +static int
> +serial_omap_verify_port(struct uart_port *port, struct serial_struct
> *ser)
> +{
> +     /* we don't want the core code to modify any port params */
> +     dev_dbg(port->dev, "serial_omap_verify_port+\n");
> +     return -EINVAL;

Is this some sort of hack?

If so, it should be marked as such.

> +}
> +
> +static const char *
> +serial_omap_type(struct uart_port *port)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +
> +     dev_dbg(up->port.dev, "serial_omap_type+%d\n", up->pdev->id);
> +     return up->name;
> +}
> +
> +#ifdef CONFIG_SERIAL_OMAP_CONSOLE
> +
> +static struct uart_omap_port *serial_omap_console_ports[4];
> +
> +static struct uart_driver serial_omap_reg;
> +
> +#define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
> +
> +static inline void wait_for_xmitr(struct uart_omap_port *up)
> +{
> +     unsigned int status, tmout = 10000;
> +
> +     /* Wait up to 10ms for the character(s) to be sent. */
> +     do {
> +             status = serial_in(up, UART_LSR);
> +
> +             if (status & UART_LSR_BI)
> +                     up->lsr_break_flag = UART_LSR_BI;
> +
> +             if (--tmout == 0)
> +                     break;
> +             udelay(1);
> +     } while ((status & BOTH_EMPTY) != BOTH_EMPTY);
> +
> +     /* Wait up to 1s for flow control if necessary */
> +     if (up->port.flags & UPF_CONS_FLOW) {
> +             tmout = 1000000;
> +             for (tmout = 1000000; tmout; tmout--) {
> +                     unsigned int msr = serial_in(up, UART_MSR);

Space here.

> +                     up->msr_saved_flags |= msr & MSR_SAVE_FLAGS;
> +                     if (msr & UART_MSR_CTS)
> +                             break;
> +                     udelay(1);
> +             }
> +     }
> +}
> +
> +static void serial_omap_console_putchar(struct uart_port *port, int ch)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)port;
> +
> +     wait_for_xmitr(up);
> +     serial_out(up, UART_TX, ch);
> +}
> +
> +static void
> +serial_omap_console_write(struct console *co, const char *s,
> +             unsigned int count)
> +{
> +     struct uart_omap_port *up = serial_omap_console_ports[co->index];
> +     unsigned long flags;
> +     unsigned int ier;
> +     int locked = 1;
> +
> +     local_irq_save(flags);
> +     if (up->port.sysrq)
> +             locked = 0;
> +     else if (oops_in_progress)
> +             locked = spin_trylock(&up->port.lock);
> +     else
> +             spin_lock(&up->port.lock);
> +
> +     /*
> +      * First save the IER then disable the interrupts
> +      */
> +     ier = serial_in(up, UART_IER);
> +     serial_out(up, UART_IER, 0);
> +
> +     uart_console_write(&up->port, s, count,
> serial_omap_console_putchar);
> +
> +     /*
> +      * Finally, wait for transmitter to become empty
> +      * and restore the IER
> +      */
> +     wait_for_xmitr(up);
> +     serial_out(up, UART_IER, ier);
> +     /*
> +      * The receive handling will happen properly because the
> +      * receive ready bit will still be set; it is not cleared
> +      * on read.  However, modem control will not, we must
> +      * call it if we have saved something in the saved flags
> +      * while processing with interrupts off.
> +      */
> +     if (up->msr_saved_flags)
> +             check_modem_status(up);
> +
> +     if (locked)
> +             spin_unlock(&up->port.lock);
> +     local_irq_restore(flags);
> +}
> +
> +static int __init
> +serial_omap_console_setup(struct console *co, char *options)
> +{
> +     struct uart_omap_port *up;
> +     int baud = 115200;
> +     int bits = 8;
> +     int parity = 'n';
> +     int flow = 'n';
> +     int r;
> +
> +     if (serial_omap_console_ports[co->index] == NULL)
> +             return -ENODEV;
> +     up = serial_omap_console_ports[co->index];
> +
> +     if (options)
> +             uart_parse_options(options, &baud, &parity, &bits, &flow);
> +
> +     r = uart_set_options(&up->port, co, baud, parity, bits, flow);
> +
> +     return r;


You could just do:

        return uart_set_options(&up->port, co, baud, parity, bits, flow);

And get rid of var "r".

> +}
> +
> +static struct console serial_omap_console = {
> +     .name           = OMAP_SERIAL_NAME,
> +     .write          = serial_omap_console_write,
> +     .device         = uart_console_device,
> +     .setup          = serial_omap_console_setup,
> +     .flags          = CON_PRINTBUFFER,
> +     .index          = -1,
> +     .data           = &serial_omap_reg,
> +};
> +
> +static void serial_omap_add_console_port(struct uart_omap_port *up)
> +{
> +     serial_omap_console_ports[up->pdev->id] = up;
> +}
> +
> +#define OMAP_CONSOLE (&serial_omap_console)
> +
> +#else
> +
> +#define OMAP_CONSOLE NULL
> +
> +static inline void serial_omap_add_console_port(struct uart_omap_port
> *up)
> +{}
> +
> +#endif
> +
> +struct uart_ops serial_omap_pops = {
> +     .tx_empty       = serial_omap_tx_empty,
> +     .set_mctrl      = serial_omap_set_mctrl,
> +     .get_mctrl      = serial_omap_get_mctrl,
> +     .stop_tx        = serial_omap_stop_tx,
> +     .start_tx       = serial_omap_start_tx,
> +     .stop_rx        = serial_omap_stop_rx,
> +     .enable_ms      = serial_omap_enable_ms,
> +     .break_ctl      = serial_omap_break_ctl,
> +     .startup        = serial_omap_startup,
> +     .shutdown       = serial_omap_shutdown,
> +     .set_termios    = serial_omap_set_termios,
> +     .pm             = serial_omap_pm,
> +     .type           = serial_omap_type,
> +     .release_port   = serial_omap_release_port,
> +     .request_port   = serial_omap_request_port,
> +     .config_port    = serial_omap_config_port,
> +     .verify_port    = serial_omap_verify_port,
> +};
> +
> +static struct uart_driver serial_omap_reg = {
> +     .owner          = THIS_MODULE,
> +     .driver_name    = "OMAP-SERIAL",
> +     .dev_name       = OMAP_SERIAL_NAME,
> +     .nr             = OMAP_MAX_HSUART_PORTS,
> +     .cons           = OMAP_CONSOLE,
> +};
> +
> +static int
> +serial_omap_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +     struct uart_omap_port *up = platform_get_drvdata(pdev);
> +
> +     if (up)
> +             uart_suspend_port(&serial_omap_reg, &up->port);
> +     return 0;
> +}
> +
> +static int serial_omap_resume(struct platform_device *dev)
> +{
> +     struct uart_omap_port *up = platform_get_drvdata(dev);
> +
> +     if (up)
> +             uart_resume_port(&serial_omap_reg, &up->port);
> +     return 0;
> +}
> +
> +static void serial_omap_rx_timeout(unsigned long uart_no)
> +{
> +     struct uart_omap_port *up = ui[uart_no];
> +     unsigned int curr_dma_pos, curr_transmitted_size;
> +     unsigned int ret = 0;
> +
> +     curr_dma_pos = omap_get_dma_dst_pos(up->uart_dma.rx_dma_channel);
> +     if ((curr_dma_pos == up->uart_dma.prev_rx_dma_pos) ||
> +                          (curr_dma_pos == 0)) {
> +             if (jiffies_to_msecs(jiffies - up->port_activity) <
> +                                                     RX_TIMEOUT) {
> +                     mod_timer(&up->uart_dma.rx_timer, jiffies +
> +                             usecs_to_jiffies(up->uart_dma.rx_timeout));
> +             } else {
> +                     serial_omap_stop_rxdma(up);
> +                     up->ier |= (UART_IER_RDI | UART_IER_RLSI);
> +                     serial_out(up, UART_IER, up->ier);
> +             }
> +             return;
> +     }
> +
> +     curr_transmitted_size = curr_dma_pos -
> +                                     up->uart_dma.prev_rx_dma_pos;
> +     up->port.icount.rx += curr_transmitted_size;
> +     tty_insert_flip_string(up->port.state->port.tty,
> +                     up->uart_dma.rx_buf +
> +                     (up->uart_dma.prev_rx_dma_pos -
> +                     up->uart_dma.rx_buf_dma_phys),
> +                     curr_transmitted_size);
> +     tty_flip_buffer_push(up->port.state->port.tty);
> +     up->uart_dma.prev_rx_dma_pos = curr_dma_pos;
> +     if (up->uart_dma.rx_buf_size +
> +                     up->uart_dma.rx_buf_dma_phys == curr_dma_pos) {
> +             ret = serial_omap_start_rxdma(up);
> +             if (ret < 0) {
> +                     serial_omap_stop_rxdma(up);
> +                     up->ier |= (UART_IER_RDI | UART_IER_RLSI);
> +                     serial_out(up, UART_IER, up->ier);
> +             }
> +     } else  {
> +             mod_timer(&up->uart_dma.rx_timer, jiffies +
> +                     usecs_to_jiffies(up->uart_dma.rx_timeout));
> +     }
> +     up->port_activity = jiffies;
> +}
> +
> +static void uart_rx_dma_callback(int lch, u16 ch_status, void *data)
> +{
> +     return;
> +}
> +
> +static int serial_omap_start_rxdma(struct uart_omap_port *up)
> +{
> +     int ret = 0;
> +
> +     if (up->uart_dma.rx_dma_channel == -1) {
> +             ret = omap_request_dma(up->uart_dma.uart_dma_rx,
> +                             "UART Rx DMA",
> +                             (void *)uart_rx_dma_callback, up,
> +                             &(up->uart_dma.rx_dma_channel));
> +             if (ret < 0)
> +                     return ret;
> +
> +             omap_set_dma_src_params(up->uart_dma.rx_dma_channel, 0,
> +                             OMAP_DMA_AMODE_CONSTANT,
> +                             up->uart_dma.uart_base, 0, 0);
> +             omap_set_dma_dest_params(up->uart_dma.rx_dma_channel, 0,
> +                             OMAP_DMA_AMODE_POST_INC,
> +                             up->uart_dma.rx_buf_dma_phys, 0, 0);
> +             omap_set_dma_transfer_params(up->uart_dma.rx_dma_channel,
> +                             OMAP_DMA_DATA_TYPE_S8,
> +                             up->uart_dma.rx_buf_size, 1,
> +                             OMAP_DMA_SYNC_ELEMENT,
> +                             up->uart_dma.uart_dma_rx, 0);
> +     }
> +     up->uart_dma.prev_rx_dma_pos = up->uart_dma.rx_buf_dma_phys;
> +     /* FIXME: Cache maintenance needed here? */
> +     omap_start_dma(up->uart_dma.rx_dma_channel);
> +     mod_timer(&up->uart_dma.rx_timer, jiffies +
> +                             usecs_to_jiffies(up->uart_dma.rx_timeout));
> +     up->uart_dma.rx_dma_used = true;
> +     return ret;
> +}
> +
> +static void serial_omap_continue_tx(struct uart_omap_port *up)
> +{
> +     struct circ_buf *xmit = &up->port.state->xmit;
> +     int start = up->uart_dma.tx_buf_dma_phys
> +                     + (xmit->tail & (UART_XMIT_SIZE - 1));
> +
> +     if (uart_circ_empty(xmit))
> +             return;
> +
> +     up->uart_dma.tx_buf_size = uart_circ_chars_pending(xmit);
> +     /*
> +      * It is a circular buffer. See if the buffer has wounded back.
> +      * If yes it will have to be transferred in two separate dma
> +      * transfers
> +      */
> +     if (start + up->uart_dma.tx_buf_size >=
> +                     up->uart_dma.tx_buf_dma_phys + UART_XMIT_SIZE)
> +             up->uart_dma.tx_buf_size =
> +                     (up->uart_dma.tx_buf_dma_phys + UART_XMIT_SIZE) - start;
> +     omap_set_dma_dest_params(up->uart_dma.tx_dma_channel, 0,
> +                             OMAP_DMA_AMODE_CONSTANT,
> +                             up->uart_dma.uart_base, 0, 0);
> +     omap_set_dma_src_params(up->uart_dma.tx_dma_channel, 0,
> +                             OMAP_DMA_AMODE_POST_INC, start, 0, 0);
> +     omap_set_dma_transfer_params(up->uart_dma.tx_dma_channel,
> +                             OMAP_DMA_DATA_TYPE_S8,
> +                             up->uart_dma.tx_buf_size, 1,
> +                             OMAP_DMA_SYNC_ELEMENT,
> +                             up->uart_dma.uart_dma_tx, 0);
> +     /* FIXME: Cache maintenance needed here? */
> +     omap_start_dma(up->uart_dma.tx_dma_channel);
> +}
> +
> +static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
> +{
> +     struct uart_omap_port *up = (struct uart_omap_port *)data;
> +     struct circ_buf *xmit = &up->port.state->xmit;
> +
> +     xmit->tail = (xmit->tail + up->uart_dma.tx_buf_size) & \
> +                     (UART_XMIT_SIZE - 1);
> +     up->port.icount.tx += up->uart_dma.tx_buf_size;
> +
> +     if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> +             uart_write_wakeup(&up->port);
> +
> +     if (uart_circ_empty(xmit)) {
> +             spin_lock(&(up->uart_dma.tx_lock));
> +             serial_omap_stop_tx(&up->port);
> +             up->uart_dma.tx_dma_used = false;
> +             spin_unlock(&(up->uart_dma.tx_lock));
> +     } else {
> +             omap_stop_dma(up->uart_dma.tx_dma_channel);
> +             serial_omap_continue_tx(up);
> +     }
> +     up->port_activity = jiffies;
> +     return;
> +}
> +
> +static int serial_omap_probe(struct platform_device *pdev)
> +{
> +     struct uart_omap_port   *up;
> +     struct resource         *mem, *irq, *dma_tx, *dma_rx;
> +     struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
> +     int ret = -ENOSPC;
> +
> +     mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +     if (!mem) {
> +             dev_err(&pdev->dev, "no mem resource?\n");
> +             return -ENODEV;
> +     }
> +
> +     irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +     if (!irq) {
> +             dev_err(&pdev->dev, "no irq resource?\n");
> +             return -ENODEV;
> +     }
> +
> +     if (!request_mem_region(mem->start, (mem->end - mem->start) + 1,
> +                                  pdev->dev.driver->name)) {
> +             dev_err(&pdev->dev, "memory region already claimed\n");
> +             return -EBUSY;
> +     }
> +
> +     dma_rx = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx");
> +     if (!dma_rx) {
> +             ret = -EINVAL;
> +             goto err;
> +     }
> +
> +     dma_tx = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx");
> +     if (!dma_tx) {
> +             ret = -EINVAL;
> +             goto err;
> +     }
> +
> +     up = kzalloc(sizeof(*up), GFP_KERNEL);
> +     if (up == NULL) {
> +             ret = -ENOMEM;
> +             goto do_release_region;
> +     }
> +     sprintf(up->name, "OMAP UART%d", pdev->id);
> +     up->pdev = pdev;
> +     up->port.dev = &pdev->dev;
> +     up->port.type = PORT_OMAP;
> +     up->port.iotype = UPIO_MEM;
> +     up->port.irq = irq->start;
> +
> +     up->port.regshift = 2;
> +     up->port.fifosize = 64;
> +     up->port.ops = &serial_omap_pops;
> +     up->port.line = pdev->id;
> +
> +     up->port.membase = omap_up_info->membase;
> +     up->port.mapbase = omap_up_info->mapbase;
> +     up->port.flags = omap_up_info->flags;
> +     up->port.irqflags = omap_up_info->irqflags;
> +     up->port.uartclk = omap_up_info->uartclk;

Where is up->port.lock being initialized?

Regards,
Sergio

> +     up->uart_dma.uart_base = mem->start;
> +
> +     if (omap_up_info->dma_enabled) {
> +             up->uart_dma.uart_dma_tx = dma_tx->start;
> +             up->uart_dma.uart_dma_rx = dma_rx->start;
> +             up->use_dma = 1;
> +             up->uart_dma.rx_buf_size = 4096;
> +             up->uart_dma.rx_timeout = 2;
> +             spin_lock_init(&(up->uart_dma.tx_lock));
> +             spin_lock_init(&(up->uart_dma.rx_lock));
> +             up->uart_dma.tx_dma_channel = OMAP_UART_DMA_CH_FREE;
> +             up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
> +     }
> +
> +     ui[pdev->id] = up;
> +     serial_omap_add_console_port(up);
> +
> +     ret = uart_add_one_port(&serial_omap_reg, &up->port);
> +     if (ret != 0)
> +             goto do_release_region;
> +
> +     platform_set_drvdata(pdev, up);
> +     return 0;
> +err:
> +     dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n",
> +                             pdev->id, __func__, ret);
> +do_release_region:
> +     release_mem_region(mem->start, (mem->end - mem->start) + 1);
> +     return ret;
> +}
> +
> +static int serial_omap_remove(struct platform_device *dev)
> +{
> +     struct uart_omap_port *up = platform_get_drvdata(dev);
> +
> +     platform_set_drvdata(dev, NULL);
> +     if (up) {
> +             uart_remove_one_port(&serial_omap_reg, &up->port);
> +             kfree(up);
> +     }
> +     return 0;
> +}
> +
> +static struct platform_driver serial_omap_driver = {
> +     .probe          = serial_omap_probe,
> +     .remove         = serial_omap_remove,
> +
> +     .suspend        = serial_omap_suspend,
> +     .resume         = serial_omap_resume,
> +     .driver         = {
> +             .name   = DRIVER_NAME,
> +     },
> +};
> +
> +int __init serial_omap_init(void)
> +{
> +     int ret;
> +
> +     ret = uart_register_driver(&serial_omap_reg);
> +     if (ret != 0)
> +             return ret;
> +     ret = platform_driver_register(&serial_omap_driver);
> +     if (ret != 0)
> +             uart_unregister_driver(&serial_omap_reg);
> +     return ret;
> +}
> +
> +void __exit serial_omap_exit(void)
> +{
> +     platform_driver_unregister(&serial_omap_driver);
> +     uart_unregister_driver(&serial_omap_reg);
> +}
> +
> +module_init(serial_omap_init);
> +module_exit(serial_omap_exit);
> +
> +MODULE_DESCRIPTION("OMAP High Speed UART driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Texas Instruments Inc");
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 563e234..295e898 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -196,6 +196,9 @@
>  /* High Speed UART for Medfield */
>  #define PORT_MFD     95
>
> +/* TI OMAP-UART */
> +#define PORT_OMAP    96
> +
>  #ifdef __KERNEL__
>
>  #include <linux/compiler.h>
> --
> 1.6.3.3
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux