Re: [RFC][PATCH]: Adding support for omap-serail driver
From: Tony Lindgren <tony@atomide.com>
Date: 2009-08-28 15:41:24
Also in:
linux-omap, lkml
Hi, Some comments below. * vimal singh [off-list ref] [090828 06:52]:
From: Govindraj R <redacted>
This patch adds support for OMAP3430-HIGH SPEED UART Controller.
It currently adds support for the following features.
1. Supports Interrupt Mode for all three UARTs on SDP/ZOOM2.
2. Supports DMA Mode for all three UARTs on SDP/ZOOM2.
3. Supports Hardware flow control (CTS/RTS) on SDP/ZOOM2.
4. Supports 3.6Mbps baudrate on SDP/ZOOM2.
5. Debug Console support on all UARTs on SDP/ZOOM2.
6. Support for quad uart on ZOOM2 board.
The reason for adding this new driver alternative to 8250 is to avoid
polluting 8250 driver with too many omap specific data as 8250 already
supports more than 16 different uart controllers and may need too
many omap-platform checks to implement feature like DMA support.Just the DMA and PM features should be enough to justify having a custom driver for sure.
quoted hunk ↗ jump to hunk
diff --git a/arch/arm/plat-omap/include/mach/omap-serial.hb/arch/arm/plat-omap/include/mach/omap-serial.h new file mode 100644 index 0000000..d1b0bf2--- /dev/null +++ b/arch/arm/plat-omap/include/mach/omap-serial.h@@ -0,0 +1,84 @@ +/* + * arch/arm/plat-omap/include/mach/omap-serial.h + * + * Driver for OMAP3430 UART controller. + * + * Copyright (C) 2009 Texas Instruments. + * + * Authors: + * Thara Gopinath <thara@ti.com> + * Govindraj R <govindraj.raja@ti.com> + * + * 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> + +/* TI OMAP CONSOLE */ +#define PORT_OMAP 86 + +#define DRIVER_NAME "omap-hsuart" + +/* + * 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) + +#if defined(CONFIG_SERIAL_OMAP_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ) +#define SUPPORT_SYSRQ +#endif + +#ifdef CONFIG_ARCH_OMAP34XX +#define OMAP_MDR1_DISABLE 0x07 +#define OMAP_MDR1_MODE13X 0x03 +#define OMAP_MDR1_MODE16X 0x00 +#define OMAP_MODE13X_SPEED 230400 +#endif
Omap34xx specific things should be set dynamically during init rather than using ifdefs. Maybe do the low level init in mach-omap2/serial.c? That way the driver stays clean of any processor specific hacks.
+ +#define CONSOLE_NAME "console=" + +#define UART_CLK (48000000) +#define QUART_CLK (1843200) + +/* UART NUMBERS */ +#define UART1 (0x0) +#define UART2 (0x1) +#define UART3 (0x2) +#define QUART (0x3) + +#ifdef CONFIG_MACH_OMAP_ZOOM2 +#define MAX_UARTS QUART +#else +#define MAX_UARTS UART3 +#endif
This should be passed via platform_data.
+ +#define UART_BASE(uart_no) (uart_no == UART1) ? OMAP_UART1_BASE :\ + (uart_no == UART2) ? OMAP_UART2_BASE :\ + OMAP_UART3_BASE + +#define UART_MODULE_BASE(uart_no) (UART1 == uart_no ? \ + IO_ADDRESS(OMAP_UART1_BASE) :\ + (UART2 == uart_no ? \ + IO_ADDRESS(OMAP_UART2_BASE) :\ + IO_ADDRESS(OMAP_UART3_BASE)))
These too you can easily set in mach-omap2/serial.c so similar.
+extern unsigned int fcr[MAX_UARTS];
+extern char *saved_command_line;
+
+struct plat_serialomap_port {
+ void __iomem *membase; /* ioremap cookie or NULL */
+ resource_size_t mapbase; /* resource base */Extra space after tab. Please run through checkpatch.pl --strict.
quoted hunk ↗ jump to hunk
+ unsigned int irq; /* interrupt number */ + unsigned char regshift; /* register shift */ + upf_t flags; /* UPF_* flags */ + void *private_data; +}; + +#endif /* __OMAP_SERIAL_H__ */diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 037c1e0..906fb61 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig@@ -1359,6 +1359,98 @@ config SERIAL_OF_PLATFORM Currently, only 8250 compatible ports are supported, but others can easily be added. +config SERIAL_OMAP + bool "OMAP serial port support" + depends on ARM && ARCH_OMAP + 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=ttyS0". (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.) + +config SERIAL_OMAP_DMA_UART1 + bool "UART1 DMA support" + depends on SERIAL_OMAP + help + If you have enabled the serial port on the Texas Instruments OMAP + CPU you can enable the DMA transfer on UART 1 by answering + to this option. +
No need for Kconfig option. Please pass from board-*.c files in platform_data.
+config SERIAL_OMAP_UART1_RXDMA_TIMEOUT + int "Timeout value for RX DMA in microseconds" + depends on SERIAL_OMAP_DMA_UART1 + default "1" + help + Set the timeout value in which you want the data pulled by RX dma to + be passed to the tty framework.
Ditto.
+ +config SERIAL_OMAP_UART1_RXDMA_BUFSIZE + int "DMA buffer size for RX in bytes" + depends on SERIAL_OMAP_DMA_UART1 + default "4096" + help + Set the dma buffer size for UART Rx
Ditto for all other ports too. <snip>
quoted hunk ↗ jump to hunk
diff --git a/drivers/serial/omap-serial.c b/drivers/serial/omap-serial.c new file mode 100644 index 0000000..3b84740 --- /dev/null +++ b/drivers/serial/omap-serial.c@@ -0,0 +1,1431 @@ +/* + * drivers/serial/omap-serial.c + * + * Driver for OMAP3430 UART controller. + * + * Copyright (C) 2009 Texas Instruments. + * + * Authors: + * Thara Gopinath <thara@ti.com> + * Govindraj R <govindraj.raja@ti.com> + * + * 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. + */ + +#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 <asm/irq.h> +#include <mach/dma.h> +#include <mach/dmtimer.h> +#include <mach/omap-serial.h> + +#ifdef DEBUG +#define DPRINTK printk +#else +#define DPRINTK(x...) +#endif
Please get rid of custom debug functions. <snip>
+ if (*status & UART_LSR_BI) + flag = TTY_BREAK; + else if (*status & UART_LSR_PE) + flag = TTY_PARITY; + else if (*status & UART_LSR_FE) + flag = TTY_FRAME; + } + + if (uart_handle_sysrq_char(&up->port, ch)) + goto ignore_char; + + uart_insert_char(&up->port, *status, UART_LSR_OE, ch, flag); + +ignore_char: + *status = serial_in(up, UART_LSR); + } while ((*status & UART_LSR_DR) && (max_count-- > 0)); + tty_flip_buffer_push(tty); + + +}
Extras lines at the end of function, you might want to remove. <snip>
+static struct console serial_omap_console = {
+ .name = "ttyS",
+ .write = serial_omap_console_write,
+ .device = uart_console_device,
+ .setup = serial_omap_console_setup,
+ .flags = CON_PRINTBUFFER,
+ .index = -1,
+ .data = &serial_omap_reg,
+};I believe you'll need to use some other name than ttyS. Otherwise it will conflict with hotpluggable 8250 devices, such as bluetooth.
+static struct uart_driver serial_omap_reg = {
+ .owner = THIS_MODULE,
+ .driver_name = "OMAP-SERIAL",
+ .dev_name = "ttyS",
+ .major = TTY_MAJOR,
+ .minor = 64,
+ .nr = 4,
+ .cons = OMAP_CONSOLE,
+};Here too. Maybe ttyO for the name? Regards, Tony