Hi,
Thanks for reposting this, it's been a while since the last review round
so I've taken a closer look at it.
Lots of comments below.
-Olof
On Wed, Jan 27, 2010 at 03:26:56PM +0530, Govindraj.R wrote:
> >From 869dde60756b2115c35d97f8e2daf014cb1af08e Mon Sep 17 00:00:00 2001
> From: Govindraj R <govindraj.raja@xxxxxx>
> Date: Wed, 27 Jan 2010 14:57:58 +0530
> Subject: [PATCH] OMAP UART: Add omap-serial driver support.
>
> This patch adds support for OMAP-HIGH SPEED UART Controller.
>
> It adds support for the following features:
> 1. It supports Interrupt mode and DMA mode of operation.
> 2. Supports Hardware flow control and software flow control.
> 3. Debug Console support on all UARTs.
>
> Cc: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Govindraj R <govindraj.raja@xxxxxx>
> ---
> arch/arm/plat-omap/include/plat/omap-serial.h | 128 +++
> drivers/serial/Kconfig | 23 +
> drivers/serial/Makefile | 1 +
> drivers/serial/omap-serial.c | 1349 +++++++++++++++++++++++++
> include/linux/serial_core.h | 3 +
> 5 files changed, 1504 insertions(+), 0 deletions(-)
> create mode 100755 arch/arm/plat-omap/include/plat/omap-serial.h
> create mode 100755 drivers/serial/omap-serial.c
>
> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h
> @@ -0,0 +1,128 @@
> +/*
> + * Driver for OMAP-UART controller.
> + * Based on drivers/serial/8250.c
> + *
> + * Copyright (C) 2009 Texas Instruments.
It's considered polite to duplicate the original copyright after the "based on" text.
Also, 8250.c is v2-or-later, and this is v2-only. You shouldn't change the license
without the original author's approval (i.e. it's more hassle than it is worth).
> + *
> + * Authors:
> + * Govindraj R <govindraj.raja@xxxxxx>
> + * Thara Gopinath <thara@xxxxxx>
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +#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"
> +
> +/*
> + * Use tty device name as ttyO, [O -> OMAP]
> + * in bootargs we specify as console=ttyO0 if uart1
> + * is used as console uart.
> + * Use Major 204 and minor 64.
> + * This is necessary if we should coexist with the 8250 driver,
> + * if we have an external TI-16C750 UART. Ex.ZOOM2/3 Boards.
> + */
Comments should document _why_ you do something, not _what_ you're
doing. As such, the "Use Major 204 and minor 64" is redundant, it's
obvious from looking at the lines below.
> +
> +#define OMAP_SERIAL_NAME "ttyO"
> +#define OMAP_SERIAL_MAJOR 204
> +#define OMAP_SERIAL_MINOR 64
Where did these numbers come from? 204/64 are listed as in use in
Documentation/devices.txt. 213 is the first available minor there. (You
should also update that file).
> +
> +/*
> + * We default to IRQ0 for the "no irq" hack. Some
> + * machine types want others as well - they're free
> + * to redefine this in their header file.
> + */
> +
> +#define is_real_interrupt(irq) ((irq) != 0)
You should compare to NO_IRQ instead.
> +
> +#if defined(CONFIG_SERIAL_OMAP_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
> +#define SUPPORT_SYSRQ
> +#endif
> +
> +#define OMAP_MDR1_DISABLE 0x07
> +#define OMAP_MDR1_MODE13X 0x03
> +#define OMAP_MDR1_MODE16X 0x00
> +#define OMAP_MODE13X_SPEED 230400
> +
> +#define CONSOLE_NAME "console="
> +
> +#define RX_TIMEOUT (3 * HZ)
> +
> +#ifdef CONFIG_ARCH_OMAP4
> +#define OMAP_MAX_HSUART_PORTS 4
> +#else
> +#define OMAP_MAX_HSUART_PORTS 3
> +#endif
> +
> +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;
> + /*
> + * Buffer for rx dma.It is not required for tx because the buffer
> + * comes from port structure
> + */
> + unsigned int uart_base;
> + unsigned char *rx_buf;
> + unsigned int prev_rx_dma_pos;
> + int tx_buf_size;
> + int tx_dma_state;
> + int rx_dma_state;
> + 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;
> + int is_buf_dma_alloced;
Alloced is bad english. :) "allocated" would be better.
> + /*
> + * 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;
> +#define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA
I know the 8250 code does the define in the structure, but it still seems
like a random thing to do. Please move it up with the other defines.
> + unsigned char msr_saved_flags;
> + char name[20];
> + spinlock_t uart_lock;
> + unsigned long port_activity;
> +};
> +
> +extern char *saved_command_line;
> +#endif /* __OMAP_SERIAL_H__ */
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 9ff47db..a5b1c71 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -1366,6 +1366,29 @@ 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_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.
> +
> +config SERIAL_OMAP_CONSOLE
> + bool "Console on OMAP serial port"
> + depends on SERIAL_OMAP
> + select SERIAL_CORE_CONSOLE
> + help
> + If you have enabled the serial port on the Texas Instruments OMAP
> + CPU you can make it the console by answering Y to this option.
> +
> + 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=tty0". (Try "man bootparam" or see the documentation of
> + your boot loader (lilo or loadlin) about how to pass options to the
> + kernel at boot time.)
ARM has neither lilo nor loadlin. The help text could do with a minor cleanup.
> +
> 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 5548fe7..42962b3 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -82,3 +82,4 @@ obj-$(CONFIG_KGDB_SERIAL_CONSOLE) += kgdboc.o
> obj-$(CONFIG_SERIAL_QE) += ucc_uart.o
> obj-$(CONFIG_SERIAL_TIMBERDALE) += timbuart.o
> obj-$(CONFIG_SERIAL_GRLIB_GAISLER_APBUART) += apbuart.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 100755
> index 0000000..72697ca
> --- /dev/null
> +++ b/drivers/serial/omap-serial.c
> @@ -0,0 +1,1349 @@
> +/*
> + * Driver for OMAP-UART controller.
> + * Based on drivers/serial/8250.c
> + *
> + * Copyright (C) 2009 Texas Instruments.
> + *
> + * Authors:
> + * Govindraj R <govindraj.raja@xxxxxx>
> + * Thara Gopinath <thara@xxxxxx>
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + *
> + * 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/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>
> +#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 void serial_omap_start_rxdma(struct uart_omap_port *up);
> +static int serial_omap_remove(struct platform_device *dev);
> +
> +int console_detect(char *str)
> +{
> + char *next, *start = NULL;
> + int i;
> +
> + i = strlen(CONSOLE_NAME);
> + next = saved_command_line;
> +
> + while ((next = strchr(next, 'c')) != NULL) {
> + if (!strncmp(next, CONSOLE_NAME, i)) {
> + start = next;
> + break;
> + } else {
> + next++;
> + }
> + }
> + if (!start)
> + return -EPERM;
> + i = 0;
> + start = strchr(start, '=') + 1;
> + while (*start != ',') {
> + str[i++] = *start++;
> + if (i > 6) {
> + pr_err("%s : Invalid Console Name\n", __func__);
> + return -EPERM;
> + }
> + }
> + str[i] = '\0';
> + return 0;
> +}
What's the purpose of this function? console_setup() will already select
the right console for you, right?
> +
> +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 *port)
> +{
> + struct uart_omap_port *up = (struct uart_omap_port *)port;
> +
> + 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
> + * @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.
> + */
Again, the why, not the how. Why do you need the 13x divisor? What's
magic about 3Mbaud?
> +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_state) {
> + del_timer_sync(&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 = 0xFF;
> + up->uart_dma.rx_dma_state = 0x0;
> + }
> +}
> +
> +static void serial_omap_enable_ms(struct uart_port *port)
> +{
> + struct uart_omap_port *up = (struct uart_omap_port *)port;
> +
> + 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;
> +
> + if (up->use_dma) {
> + if (up->uart_dma.tx_dma_channel != 0xFF) {
Could be nice to have a define for this magic value.
> + /*
> + * 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 = 0xFF;
> + }
> + }
> +
> + 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;
> +
> + 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++;
> + 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));
> + tty_flip_buffer_push(tty);
> +}
> +
> +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 void serial_omap_start_tx(struct uart_port *port)
> +{
> + struct uart_omap_port *up = (struct uart_omap_port *)port;
> +
> + if (up->use_dma && !(up->port.x_char)) {
If you turn this test around you can lose one level of indentation and
make the function more readable, by putting the 2-line else-case here
followed by the return, and not indent the rest.
> +
> + struct circ_buf *xmit = &up->port.state->xmit;
> + unsigned int start = up->uart_dma.tx_buf_dma_phys +
> + (xmit->tail & (UART_XMIT_SIZE - 1));
Might as well define these variables at the head of the function.
> + if (uart_circ_empty(xmit) || up->uart_dma.tx_dma_state)
> + return;
> + spin_lock(&(up->uart_dma.tx_lock));
> + up->uart_dma.tx_dma_state = 1;
What's state 1? Magic value.
> + spin_unlock(&(up->uart_dma.tx_lock));
> +
> + 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;
> +
> + if (up->uart_dma.tx_dma_channel == 0xFF)
> + omap_request_dma(up->uart_dma.uart_dma_tx,
> + "UART Tx DMA",
> + (void *)uart_tx_dma_callback, up,
> + &(up->uart_dma.tx_dma_channel));
> + 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);
> +
> + omap_start_dma(up->uart_dma.tx_dma_channel);
> +
> + } else if (!(up->ier & UART_IER_THRI)) {
> + up->ier |= UART_IER_THRI;
> + serial_out(up, UART_IER, up->ier);
> + }
> +}
> +
> +static unsigned int check_modem_status(struct uart_omap_port *up)
> +{
> + int status;
> + 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;
> + 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;
> +
> + iir = serial_in(up, UART_IIR);
> + if (iir & UART_IIR_NO_INT)
> + return IRQ_NONE;
> +
> + lsr = serial_in(up, UART_LSR);
> + if ((iir & 0x4) && up->use_dma) {
> + up->ier &= ~UART_IER_RDI;
> + serial_out(up, UART_IER, up->ier);
> + serial_omap_start_rxdma(up);
> + } else if (lsr & UART_LSR_DR) {
> + receive_chars(up, &lsr);
> + }
> + check_modem_status(up);
> + if ((lsr & UART_LSR_THRE) && (iir & 0x2))
> + transmit_chars(up);
> +
> + 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;
> +
> + 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;
> +
> + status = check_modem_status(up);
> + dev_dbg(up->port.dev, "serial_omap_get_mctrl+%d\n", up->pdev->id);
> +
> + ret = 0;
> + 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, 0x2);
> +
> + /*
> + * 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);
> +
> + /*
> + * Now, initialize the UART
> + */
> + serial_out(up, UART_LCR, UART_LCR_WLEN8);
> + spin_lock_irqsave(&up->port.lock, flags);
> + if (up->port.flags & UPF_FOURPORT) {
> + if (!is_real_interrupt(up->port.irq))
> + up->port.mctrl |= TIOCM_OUT1;
> + } else {
> + /*
> + * Most PC uarts need OUT2 raised to enable interrupts.
> + */
> + if (is_real_interrupt(up->port.irq))
> + up->port.mctrl |= TIOCM_OUT2;
> + }
The above if/else seems to come straight over from the 8250 driver. One
of the cases can likely be removed here. :)
> + 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) {
> + if (!up->is_buf_dma_alloced) {
Will you ever come here with use_dma = 1 and is_buf_dma_alloced = 0?
I.e. do yo need the separate variable at all?
> + free_page((unsigned long)up->port.state->xmit.buf);
> + up->port.state->xmit.buf = NULL;
> + up->port.state->xmit.buf = dma_alloc_coherent(NULL,
> + UART_XMIT_SIZE,
> + (dma_addr_t *)&(up->uart_dma.tx_buf_dma_phys),
> + 0);
Huh, why two assignments in a row?
> + up->is_buf_dma_alloced = 1;
> + }
> + 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);
> + if (up->port.flags & UPF_FOURPORT) {
> + /* reset interrupts on the AST Fourport board */
> + inb((up->port.iobase & 0xfe0) | 0x1f);
> + up->port.mctrl |= TIOCM_OUT1;
> + } else
> + up->port.mctrl &= ~TIOCM_OUT2;
Same comment here regarding fourport and 8250
> + 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) {
> + int tmp;
> + if (up->is_buf_dma_alloced) {
... same comment as above.
> + 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;
> + up->is_buf_dma_alloced = 0;
> + }
> + 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;
> + tmp = serial_in(up, UART_OMAP_SYSC) & 0x7;
> + serial_out(up, UART_OMAP_SYSC, tmp); /* force-idle */
> + }
> + 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, 0xbf);
> + 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 &= 0xf0;
> +
> + /*
> + * IXON Flag:
> + * Enable XON/XOFF flow control on output.
> + * Transmit XON1, XOFF1
> + */
> + if (termios->c_iflag & IXON)
> + efr |= 0x01 << 3;
> +
> + /*
> + * IXOFF Flag:
> + * Enable XON/XOFF flow control on input.
> + * Receiver compares XON1, XOFF1.
> + */
> + if (termios->c_iflag & IXOFF)
> + efr |= 0x01 << 1;
> +
> + serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
So, UART_EFR_ECB is documented, but it would be nice to have the other
bits documented as #defines too.
> + serial_out(up, UART_LCR, 0x80);
> +
> + 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 |= 1<<5;
> +
> + serial_out(up, UART_MCR, up->mcr | 1<<6);
> +
> + serial_out(up, UART_LCR, 0xbf);
> + serial_out(up, UART_TI752_TCR, 0x0f);
> + /* 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 | 1<<5);
> + serial_out(up, UART_LCR, 0x80);
> +
> + serial_out(up, UART_MCR, up->mcr & ~(1<<6));
> + 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);
> +
> + if (up->use_dma)
> + up->fcr = UART_FCR_ENABLE_FIFO
> + | 0x1 << 6 | 0x1 << 4
> + | UART_FCR_DMA_SELECT;
> + else
> + up->fcr = UART_FCR_ENABLE_FIFO
> + | 0x1 << 6 | 0x1 << 4;
More magic numbers.
> +
> + /*
> + * 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 -----------*/
Unstandard comment style
> +
> + /* 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, 0xbf);
> +
> + up->efr = serial_in(up, UART_EFR);
> + serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
> + serial_out(up, UART_LCR, 0x0); /* Access FCR */
Access FCR by writing LCR? Please explain. Same goes for the ones below.
> +
> + up->mcr = serial_in(up, UART_MCR);
> + serial_out(up, UART_MCR, up->mcr | 0x40); /* Access TLR*/
> + /* FIFO ENABLE, DMA MODE */
> + serial_out(up, UART_FCR, up->fcr);
> + serial_out(up, UART_LCR, 0xbf); /* Access EFR */
> +
> + if (up->use_dma) {
> + serial_out(up, UART_TI752_TLR, 0x00);
> + serial_out(up, UART_OMAP_SCR, ((1 << 6) | (1 << 7)));
> + }
> +
> + serial_out(up, UART_EFR, up->efr);
> + serial_out(up, UART_LCR, 0x80);
> + serial_out(up, UART_MCR, up->mcr); /* Restore TLR */
> +
> + /*-----Protocol, Baud Rate, and Interrupt Settings -- */
More unstandard comment format
> +
> + serial_out(up, UART_OMAP_MDR1, OMAP_MDR1_DISABLE);
> +
> + serial_out(up, UART_LCR, 0xbf);
> +
> + 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, 0xbf);
> +
> + 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, 0xbf);
> +
> + 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);
More 13-divider special case.
> +
> + /*---------- Hardware Flow Control Configuration------- */
> +
> + if (termios->c_cflag & CRTSCTS) {
> + efr |= (UART_EFR_CTS | UART_EFR_RTS);
> + serial_out(up, UART_LCR, 0x80);
> +
> + up->mcr = serial_in(up, UART_MCR);
> + serial_out(up, UART_MCR, up->mcr | 0x40);
> +
> + serial_out(up, UART_LCR, 0xbf); /* Access EFR */
EFR or LCR?!
> +
> + up->efr = serial_in(up, UART_EFR);
> + serial_out(up, UART_EFR, up->efr | UART_EFR_ECB);
> +
> + serial_out(up, UART_TI752_TCR, 0x0f);
> + serial_out(up, UART_EFR, efr); /* Enable AUTORTS and AUTOCTS */
> + serial_out(up, UART_LCR, 0x80);
> + serial_out(up, UART_MCR, up->mcr | 0x02);
> + 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);
> + efr = serial_in(up, UART_EFR);
> + serial_out(up, UART_LCR, 0xBF);
> + 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, 0xBF);
> + 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) ? 0x7f : 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;
> +}
> +
> +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);
> + 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 int ier;
> +
> + /*
> + * 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);
> +}
> +
> +static int __init
> +serial_omap_console_setup(struct console *co, char *options)
> +{
> + struct uart_omap_port *up;
> + int baud = 9600;
Nearly every OMAP setup seems to prefer 115200 as default baudrate, right?
> + 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;
> +}
> +
> +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) {}
Please do the empty body on a separate line.
> +
> +#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,
> + .major = OMAP_SERIAL_MAJOR,
> + .minor = OMAP_SERIAL_MINOR,
> + .nr = OMAP_MAX_HSUART_PORTS,
> + .cons = OMAP_CONSOLE,
> +};
> +
> +static
> +int serial_omap_suspend(struct platform_device *pdev, pm_message_t state)
No linewrap (checkpatch should have caught this)
> +{
> + 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_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;
> + serial_out(up, UART_IER, up->ier);
> + }
> + return;
> + } else {
Since you have a return above, there's no need to do the rest in the
else-clause. Saves one level of indentation.
> + unsigned int 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)
> + serial_omap_start_rxdma(up);
> + 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 void serial_omap_start_rxdma(struct uart_omap_port *up)
> +{
> + if (up->uart_dma.rx_dma_channel == 0xFF) {
> + omap_request_dma(up->uart_dma.uart_dma_rx, "UART Rx DMA",
> + (void *)uart_rx_dma_callback, up,
> + &(up->uart_dma.rx_dma_channel));
> +
> + 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;
> + if (cpu_is_omap44xx())
> + omap_writel(0, OMAP44XX_DMA4_BASE
> + + OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel));
> + else
> + omap_writel(0, OMAP34XX_DMA4_BASE
> + + OMAP_DMA4_CDAC(up->uart_dma.rx_dma_channel));
> + 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_state = 1;
> +}
> +
> +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);
Isn't there a dma_sync call missing somewhere around here? Same for
the similar RX setup (both before DMA is started, as well as once it is
completed on the RX side).
> +
> + 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_state = 0;
> + 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;
> + char str[7];
> +
> + 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;
> + }
> +
> + ret = (int) request_mem_region(mem->start, (mem->end - mem->start) + 1,
> + pdev->dev.driver->name);
> + if (!ret) {
> + dev_err(&pdev->dev, "memory region already claimed\n");
> + return -EBUSY;
> + }
> +
> + dma_tx = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> + if (!dma_tx) {
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + dma_rx = platform_get_resource(pdev, IORESOURCE_DMA, 1);
> + if (!dma_rx) {
> + 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;
> + 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 = 1;
> + }
> +
> + if (up->use_dma) {
> + spin_lock_init(&(up->uart_dma.tx_lock));
> + spin_lock_init(&(up->uart_dma.rx_lock));
> + up->uart_dma.tx_dma_channel = 0xFF;
> + up->uart_dma.rx_dma_channel = 0xFF;
> + }
> + if (console_detect(str)) {
> + pr_err("\n %s: Invalid console paramter...\n", __func__);
> + pr_err("\n %s: UART Driver Init Failed!\n", __func__);
> + return -EPERM;
> + }
See comments above about console_detect()
> + 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]\n",
> + pdev->id, __func__);
It would be nice to get the actual error (i.e. printing out err) here.
> +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 8c3dd36..e02b945 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -182,6 +182,9 @@
> /* Aeroflex Gaisler GRLIB APBUART */
> #define PORT_APBUART 90
>
> +/* TI OMAP-UART */
> +#define PORT_OMAP 91
> +
> #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
