[PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[Haavard Skinnemoen]

The following patchset cleans up the atmel_serial driver a bit,
moves a significant portion of the interrupt handler into a tasklet,
and adds DMA support. This is the result of a combined effort by Chip
Coldwell, Remy Bohmer and me. The patches should apply cleanly onto
Linus' latest git tree.

With DMA, I see transfer rates around 92 kbps when transferring a big
file using ZModem (both directions are roughly the same.)

Note that break and error handling doesn't work too well with DMA
enabled. This is a common problem with all the efforts I've seen
adding DMA support to this driver (including my own). The PDC error
handling also accesses icount without locking. I'm tempted to just
ignore the problem for now and hopefully come up with a solution
later.

Everyone, please give it a try and/or review the code.

Chip Coldwell (1):
      atmel_serial: Add DMA support

Haavard Skinnemoen (3):
      atmel_serial: Use cpu_relax() when busy-waiting
      atmel_serial: Use existing console options only if BRG is running
      atmel_serial: Fix bugs in probe() error path and remove()

Remy Bohmer (2):
      atmel_serial: Clean up the code
      atmel_serial: Split the interrupt handler

 drivers/serial/atmel_serial.c |  938 ++++++++++++++++++++++++++++++++---------
 1 files changed, 738 insertions(+), 200 deletions(-)

[PATCH v2 1/6] atmel_serial: Clean up the code

[Haavard Skinnemoen]

From: Remy Bohmer <linux@bohmer.net>

This patch cleans up the atmel_serial driver to conform the coding rules.
It contains no functional change.

Signed-off-by: Remy Bohmer <linux@bohmer.net>
Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 drivers/serial/atmel_serial.c |  327 ++++++++++++++++++++++++-----------------
 1 files changed, 189 insertions(+), 138 deletions(-)

diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
index 111da57..bd41529 100644
--- a/drivers/serial/atmel_serial.c
+++ b/drivers/serial/atmel_serial.c
@@ -74,38 +74,42 @@
 
 #define ATMEL_ISR_PASS_LIMIT	256
 
-#define UART_PUT_CR(port,v)	__raw_writel(v, (port)->membase + ATMEL_US_CR)
-#define UART_GET_MR(port)	__raw_readl((port)->membase + ATMEL_US_MR)
-#define UART_PUT_MR(port,v)	__raw_writel(v, (port)->membase + ATMEL_US_MR)
-#define UART_PUT_IER(port,v)	__raw_writel(v, (port)->membase + ATMEL_US_IER)
-#define UART_PUT_IDR(port,v)	__raw_writel(v, (port)->membase + ATMEL_US_IDR)
-#define UART_GET_IMR(port)	__raw_readl((port)->membase + ATMEL_US_IMR)
-#define UART_GET_CSR(port)	__raw_readl((port)->membase + ATMEL_US_CSR)
-#define UART_GET_CHAR(port)	__raw_readl((port)->membase + ATMEL_US_RHR)
-#define UART_PUT_CHAR(port,v)	__raw_writel(v, (port)->membase + ATMEL_US_THR)
-#define UART_GET_BRGR(port)	__raw_readl((port)->membase + ATMEL_US_BRGR)
-#define UART_PUT_BRGR(port,v)	__raw_writel(v, (port)->membase + ATMEL_US_BRGR)
-#define UART_PUT_RTOR(port,v)	__raw_writel(v, (port)->membase + ATMEL_US_RTOR)
-
-// #define UART_GET_CR(port)	__raw_readl((port)->membase + ATMEL_US_CR)		// is write-only
+#define at_readl(port, off)	__raw_readl((port)->membase + (off))
+#define at_writel(v, port, off)	__raw_writel(v, (port)->membase + (off))
+
+#define UART_PUT_CR(port, v)	at_writel(v, port, ATMEL_US_CR)
+#define UART_PUT_MR(port, v)	at_writel(v, port, ATMEL_US_MR)
+#define UART_PUT_IER(port, v)	at_writel(v, port, ATMEL_US_IER)
+#define UART_PUT_IDR(port, v)	at_writel(v, port, ATMEL_US_IDR)
+#define UART_PUT_CHAR(port, v)	at_writel(v, port, ATMEL_US_THR)
+#define UART_PUT_BRGR(port, v)	at_writel(v, port, ATMEL_US_BRGR)
+#define UART_PUT_RTOR(port, v)	at_writel(v, port, ATMEL_US_RTOR)
+
+#define UART_GET_MR(port)	at_readl(port, ATMEL_US_MR)
+#define UART_GET_IMR(port)	at_readl(port, ATMEL_US_IMR)
+#define UART_GET_CSR(port)	at_readl(port, ATMEL_US_CSR)
+#define UART_GET_CHAR(port)	at_readl(port, ATMEL_US_RHR)
+#define UART_GET_BRGR(port)	at_readl(port, ATMEL_US_BRGR)
+
+/* is write-only */
+/* #define UART_GET_CR(port)	at_readl(port, ATMEL_US_CR) */
 
  /* PDC registers */
-#define UART_PUT_PTCR(port,v)	__raw_writel(v, (port)->membase + ATMEL_PDC_PTCR)
-#define UART_GET_PTSR(port)	__raw_readl((port)->membase + ATMEL_PDC_PTSR)
-
-#define UART_PUT_RPR(port,v)	__raw_writel(v, (port)->membase + ATMEL_PDC_RPR)
-#define UART_GET_RPR(port)	__raw_readl((port)->membase + ATMEL_PDC_RPR)
-#define UART_PUT_RCR(port,v)	__raw_writel(v, (port)->membase + ATMEL_PDC_RCR)
-#define UART_PUT_RNPR(port,v)	__raw_writel(v, (port)->membase + ATMEL_PDC_RNPR)
-#define UART_PUT_RNCR(port,v)	__raw_writel(v, (port)->membase + ATMEL_PDC_RNCR)
-
-#define UART_PUT_TPR(port,v)	__raw_writel(v, (port)->membase + ATMEL_PDC_TPR)
-#define UART_PUT_TCR(port,v)	__raw_writel(v, (port)->membase + ATMEL_PDC_TCR)
-//#define UART_PUT_TNPR(port,v)	__raw_writel(v, (port)->membase + ATMEL_PDC_TNPR)
-//#define UART_PUT_TNCR(port,v)	__raw_writel(v, (port)->membase + ATMEL_PDC_TNCR)
-
-static int (*atmel_open_hook)(struct uart_port *);
-static void (*atmel_close_hook)(struct uart_port *);
+#define UART_PUT_PTCR(port, v)	at_writel(v, port, ATMEL_PDC_PTCR)
+#define UART_PUT_RPR(port, v)	at_writel(v, port, ATMEL_PDC_RPR)
+#define UART_PUT_RCR(port, v)	at_writel(v, port, ATMEL_PDC_RCR)
+#define UART_PUT_RNPR(port, v)	at_writel(v, port, ATMEL_PDC_RNPR)
+#define UART_PUT_RNCR(port, v)	at_writel(v, port, ATMEL_PDC_RNCR)
+#define UART_PUT_TPR(port, v)	at_writel(v, port, ATMEL_PDC_TPR)
+#define UART_PUT_TCR(port, v)	at_writel(v, port, ATMEL_PDC_TCR)
+/*#define UART_PUT_TNPR(port, v) at_writel(v, port, ATMEL_PDC_TNPR) */
+/*#define UART_PUT_TNCR(port, v) at_writel(v, port, ATMEL_PDC_TNCR) */
+
+#define UART_GET_PTSR(port)	at_readl(port, ATMEL_PDC_PTSR)
+#define UART_GET_RPR(port)	at_readl(port, ATMEL_PDC_RPR)
+
+static int (*atmel_open_hook) (struct uart_port *);
+static void (*atmel_close_hook) (struct uart_port *);
 
 /*
  * We wrap our port structure around the generic uart_port.
@@ -142,8 +146,8 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
 #ifdef CONFIG_ARCH_AT91RM9200
 	if (cpu_is_at91rm9200()) {
 		/*
-		 * AT91RM9200 Errata #39: RTS0 is not internally connected to PA21.
-		 *  We need to drive the pin manually.
+		 * AT91RM9200 Errata #39: RTS0 is not internally connected
+		 * to PA21. We need to drive the pin manually.
 		 */
 		if (port->mapbase == AT91RM9200_BASE_US0) {
 			if (mctrl & TIOCM_RTS)
@@ -228,7 +232,8 @@ static void atmel_stop_rx(struct uart_port *port)
  */
 static void atmel_enable_ms(struct uart_port *port)
 {
-	UART_PUT_IER(port, ATMEL_US_RIIC | ATMEL_US_DSRIC | ATMEL_US_DCDIC | ATMEL_US_CTSIC);
+	UART_PUT_IER(port, ATMEL_US_RIIC | ATMEL_US_DSRIC
+			| ATMEL_US_DCDIC | ATMEL_US_CTSIC);
 }
 
 /*
@@ -247,7 +252,7 @@ static void atmel_break_ctl(struct uart_port *port, int break_state)
  */
 static void atmel_rx_chars(struct uart_port *port)
 {
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 	struct tty_struct *tty = port->info->tty;
 	unsigned int status, ch, flg;
 
@@ -266,10 +271,12 @@ static void atmel_rx_chars(struct uart_port *port)
 		if (unlikely(status & (ATMEL_US_PARE | ATMEL_US_FRAME
 				       | ATMEL_US_OVRE | ATMEL_US_RXBRK)
 			     || atmel_port->break_active)) {
-			UART_PUT_CR(port, ATMEL_US_RSTSTA);	/* clear error */
+			/* clear error */
+			UART_PUT_CR(port, ATMEL_US_RSTSTA);
 			if (status & ATMEL_US_RXBRK
 			    && !atmel_port->break_active) {
-				status &= ~(ATMEL_US_PARE | ATMEL_US_FRAME);	/* ignore side-effect */
+				/* ignore side-effect */
+				status &= ~(ATMEL_US_PARE | ATMEL_US_FRAME);
 				port->icount.brk++;
 				atmel_port->break_active = 1;
 				UART_PUT_IER(port, ATMEL_US_RXBRK);
@@ -309,7 +316,7 @@ static void atmel_rx_chars(struct uart_port *port)
 
 		uart_insert_char(port, status, ATMEL_US_OVRE, ch, flg);
 
-	ignore_char:
+ignore_char:
 		status = UART_GET_CSR(port);
 	}
 
@@ -350,44 +357,73 @@ static void atmel_tx_chars(struct uart_port *port)
 }
 
 /*
+ * receive interrupt handler.
+ */
+static void
+atmel_handle_receive(struct uart_port *port, unsigned int pending)
+{
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+
+	/* Interrupt receive */
+	if (pending & ATMEL_US_RXRDY)
+		atmel_rx_chars(port);
+	else if (pending & ATMEL_US_RXBRK) {
+		/*
+		 * End of break detected. If it came along with a
+		 * character, atmel_rx_chars will handle it.
+		 */
+		UART_PUT_CR(port, ATMEL_US_RSTSTA);
+		UART_PUT_IDR(port, ATMEL_US_RXBRK);
+		atmel_port->break_active = 0;
+	}
+}
+
+/*
+ * transmit interrupt handler.
+ */
+static void
+atmel_handle_transmit(struct uart_port *port, unsigned int pending)
+{
+	/* Interrupt transmit */
+	if (pending & ATMEL_US_TXRDY)
+		atmel_tx_chars(port);
+}
+
+/*
+ * status flags interrupt handler.
+ */
+static void
+atmel_handle_status(struct uart_port *port, unsigned int pending,
+		    unsigned int status)
+{
+	/* TODO: All reads to CSR will clear these interrupts! */
+	if (pending & ATMEL_US_RIIC)
+		port->icount.rng++;
+	if (pending & ATMEL_US_DSRIC)
+		port->icount.dsr++;
+	if (pending & ATMEL_US_DCDIC)
+		uart_handle_dcd_change(port, !(status & ATMEL_US_DCD));
+	if (pending & ATMEL_US_CTSIC)
+		uart_handle_cts_change(port, !(status & ATMEL_US_CTS));
+	if (pending & (ATMEL_US_RIIC | ATMEL_US_DSRIC | ATMEL_US_DCDIC
+				| ATMEL_US_CTSIC))
+		wake_up_interruptible(&port->info->delta_msr_wait);
+}
+
+/*
  * Interrupt handler
  */
 static irqreturn_t atmel_interrupt(int irq, void *dev_id)
 {
 	struct uart_port *port = dev_id;
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
 	unsigned int status, pending, pass_counter = 0;
 
 	status = UART_GET_CSR(port);
 	pending = status & UART_GET_IMR(port);
 	while (pending) {
-		/* Interrupt receive */
-		if (pending & ATMEL_US_RXRDY)
-			atmel_rx_chars(port);
-		else if (pending & ATMEL_US_RXBRK) {
-			/*
-			 * End of break detected. If it came along
-			 * with a character, atmel_rx_chars will
-			 * handle it.
-			 */
-			UART_PUT_CR(port, ATMEL_US_RSTSTA);
-			UART_PUT_IDR(port, ATMEL_US_RXBRK);
-			atmel_port->break_active = 0;
-		}
-
-		// TODO: All reads to CSR will clear these interrupts!
-		if (pending & ATMEL_US_RIIC) port->icount.rng++;
-		if (pending & ATMEL_US_DSRIC) port->icount.dsr++;
-		if (pending & ATMEL_US_DCDIC)
-			uart_handle_dcd_change(port, !(status & ATMEL_US_DCD));
-		if (pending & ATMEL_US_CTSIC)
-			uart_handle_cts_change(port, !(status & ATMEL_US_CTS));
-		if (pending & (ATMEL_US_RIIC | ATMEL_US_DSRIC | ATMEL_US_DCDIC | ATMEL_US_CTSIC))
-			wake_up_interruptible(&port->info->delta_msr_wait);
-
-		/* Interrupt transmit */
-		if (pending & ATMEL_US_TXRDY)
-			atmel_tx_chars(port);
+		atmel_handle_receive(port, pending);
+		atmel_handle_status(port, pending, status);
+		atmel_handle_transmit(port, pending);
 
 		if (pass_counter++ > ATMEL_ISR_PASS_LIMIT)
 			break;
@@ -415,7 +451,8 @@ static int atmel_startup(struct uart_port *port)
 	/*
 	 * Allocate the IRQ
 	 */
-	retval = request_irq(port->irq, atmel_interrupt, IRQF_SHARED, "atmel_serial", port);
+	retval = request_irq(port->irq, atmel_interrupt, IRQF_SHARED,
+			"atmel_serial", port);
 	if (retval) {
 		printk("atmel_serial: atmel_startup - Can't get irq\n");
 		return retval;
@@ -437,9 +474,11 @@ static int atmel_startup(struct uart_port *port)
 	 * Finally, enable the serial port
 	 */
 	UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
-	UART_PUT_CR(port, ATMEL_US_TXEN | ATMEL_US_RXEN);		/* enable xmit & rcvr */
+	/* enable xmit & rcvr */
+	UART_PUT_CR(port, ATMEL_US_TXEN | ATMEL_US_RXEN);
 
-	UART_PUT_IER(port, ATMEL_US_RXRDY);		/* enable receive only */
+	/* enable receive only */
+	UART_PUT_IER(port, ATMEL_US_RXRDY);
 
 	return 0;
 }
@@ -471,45 +510,48 @@ static void atmel_shutdown(struct uart_port *port)
 /*
  * Power / Clock management.
  */
-static void atmel_serial_pm(struct uart_port *port, unsigned int state, unsigned int oldstate)
+static void atmel_serial_pm(struct uart_port *port, unsigned int state,
+			    unsigned int oldstate)
 {
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
 	switch (state) {
-		case 0:
-			/*
-			 * Enable the peripheral clock for this serial port.
-			 * This is called on uart_open() or a resume event.
-			 */
-			clk_enable(atmel_port->clk);
-			break;
-		case 3:
-			/*
-			 * Disable the peripheral clock for this serial port.
-			 * This is called on uart_close() or a suspend event.
-			 */
-			clk_disable(atmel_port->clk);
-			break;
-		default:
-			printk(KERN_ERR "atmel_serial: unknown pm %d\n", state);
+	case 0:
+		/*
+		 * Enable the peripheral clock for this serial port.
+		 * This is called on uart_open() or a resume event.
+		 */
+		clk_enable(atmel_port->clk);
+		break;
+	case 3:
+		/*
+		 * Disable the peripheral clock for this serial port.
+		 * This is called on uart_close() or a suspend event.
+		 */
+		clk_disable(atmel_port->clk);
+		break;
+	default:
+		printk(KERN_ERR "atmel_serial: unknown pm %d\n", state);
 	}
 }
 
 /*
  * Change the port parameters
  */
-static void atmel_set_termios(struct uart_port *port, struct ktermios * termios, struct ktermios * old)
+static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
+			      struct ktermios *old)
 {
 	unsigned long flags;
 	unsigned int mode, imr, quot, baud;
 
 	/* Get current mode register */
-	mode = UART_GET_MR(port) & ~(ATMEL_US_USCLKS | ATMEL_US_CHRL | ATMEL_US_NBSTOP | ATMEL_US_PAR);
+	mode = UART_GET_MR(port) & ~(ATMEL_US_USCLKS | ATMEL_US_CHRL
+					| ATMEL_US_NBSTOP | ATMEL_US_PAR);
 
-	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/16);
+	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
 	quot = uart_get_divisor(port, baud);
 
-	if (quot > 65535) {		/* BRGR is 16-bit, so switch to slower clock */
+	if (quot > 65535) {	/* BRGR is 16-bit, so switch to slower clock */
 		quot /= 8;
 		mode |= ATMEL_US_USCLKS_MCK_DIV8;
 	}
@@ -536,18 +578,17 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios * termios,
 
 	/* parity */
 	if (termios->c_cflag & PARENB) {
-		if (termios->c_cflag & CMSPAR) {			/* Mark or Space parity */
+		/* Mark or Space parity */
+		if (termios->c_cflag & CMSPAR) {
 			if (termios->c_cflag & PARODD)
 				mode |= ATMEL_US_PAR_MARK;
 			else
 				mode |= ATMEL_US_PAR_SPACE;
-		}
-		else if (termios->c_cflag & PARODD)
+		} else if (termios->c_cflag & PARODD)
 			mode |= ATMEL_US_PAR_ODD;
 		else
 			mode |= ATMEL_US_PAR_EVEN;
-	}
-	else
+	} else
 		mode |= ATMEL_US_PAR_NONE;
 
 	spin_lock_irqsave(&port->lock, flags);
@@ -573,16 +614,16 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios * termios,
 		if (termios->c_iflag & IGNPAR)
 			port->ignore_status_mask |= ATMEL_US_OVRE;
 	}
-
-	// TODO: Ignore all characters if CREAD is set.
+	/* TODO: Ignore all characters if CREAD is set.*/
 
 	/* update the per-port timeout */
 	uart_update_timeout(port, termios->c_cflag, baud);
 
-	/* disable interrupts and drain transmitter */
-	imr = UART_GET_IMR(port);	/* get interrupt mask */
-	UART_PUT_IDR(port, -1);		/* disable all interrupts */
-	while (!(UART_GET_CSR(port) & ATMEL_US_TXEMPTY)) { barrier(); }
+	/* save/disable interrupts and drain transmitter */
+	imr = UART_GET_IMR(port);
+	UART_PUT_IDR(port, -1);
+	while (!(UART_GET_CSR(port) & ATMEL_US_TXEMPTY))
+		barrier();
 
 	/* disable receiver and transmitter */
 	UART_PUT_CR(port, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
@@ -708,7 +749,8 @@ static struct uart_ops atmel_pops = {
 /*
  * Configure the port from the platform device resource info.
  */
-static void __devinit atmel_init_port(struct atmel_uart_port *atmel_port, struct platform_device *pdev)
+static void __devinit atmel_init_port(struct atmel_uart_port *atmel_port,
+				      struct platform_device *pdev)
 {
 	struct uart_port *port = &atmel_port->uart;
 	struct atmel_uart_data *data = pdev->dev.platform_data;
@@ -731,7 +773,8 @@ static void __devinit atmel_init_port(struct atmel_uart_port *atmel_port, struct
 		port->membase	= NULL;
 	}
 
-	if (!atmel_port->clk) {		/* for console, the clock could already be configured */
+	/* for console, the clock could already be configured */
+	if (!atmel_port->clk) {
 		atmel_port->clk = clk_get(&pdev->dev, "usart");
 		clk_enable(atmel_port->clk);
 		port->uartclk = clk_get_rate(atmel_port->clk);
@@ -755,7 +798,6 @@ void __init atmel_register_uart_fns(struct atmel_port_fns *fns)
 	atmel_pops.set_wake	= fns->set_wake;
 }
 
-
 #ifdef CONFIG_SERIAL_ATMEL_CONSOLE
 static void atmel_console_putchar(struct uart_port *port, int ch)
 {
@@ -773,28 +815,30 @@ static void atmel_console_write(struct console *co, const char *s, u_int count)
 	unsigned int status, imr;
 
 	/*
-	 *	First, save IMR and then disable interrupts
+	 * First, save IMR and then disable interrupts
 	 */
-	imr = UART_GET_IMR(port);	/* get interrupt mask */
+	imr = UART_GET_IMR(port);
 	UART_PUT_IDR(port, ATMEL_US_RXRDY | ATMEL_US_TXRDY);
 
 	uart_console_write(port, s, count, atmel_console_putchar);
 
 	/*
-	 *	Finally, wait for transmitter to become empty
-	 *	and restore IMR
+	 * Finally, wait for transmitter to become empty
+	 * and restore IMR
 	 */
 	do {
 		status = UART_GET_CSR(port);
 	} while (!(status & ATMEL_US_TXRDY));
-	UART_PUT_IER(port, imr);	/* set interrupts back the way they were */
+	/* set interrupts back the way they were */
+	UART_PUT_IER(port, imr);
 }
 
 /*
- * If the port was already initialised (eg, by a boot loader), try to determine
- * the current setup.
+ * If the port was already initialised (eg, by a boot loader),
+ * try to determine the current setup.
  */
-static void __init atmel_console_get_options(struct uart_port *port, int *baud, int *parity, int *bits)
+static void __init atmel_console_get_options(struct uart_port *port, int *baud,
+					     int *parity, int *bits)
 {
 	unsigned int mr, quot;
 
@@ -836,10 +880,11 @@ static int __init atmel_console_setup(struct console *co, char *options)
 	int parity = 'n';
 	int flow = 'n';
 
-	if (port->membase == 0)		/* Port not initialized yet - delay setup */
+	if (port->membase == 0)
+		/* Port not initialized yet - delay setup */
 		return -ENODEV;
 
-	UART_PUT_IDR(port, -1);				/* disable interrupts */
+	UART_PUT_IDR(port, -1);
 	UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
 	UART_PUT_CR(port, ATMEL_US_TXEN | ATMEL_US_RXEN);
 
@@ -854,13 +899,13 @@ static int __init atmel_console_setup(struct console *co, char *options)
 static struct uart_driver atmel_uart;
 
 static struct console atmel_console = {
-	.name		= ATMEL_DEVICENAME,
-	.write		= atmel_console_write,
-	.device		= uart_console_device,
-	.setup		= atmel_console_setup,
-	.flags		= CON_PRINTBUFFER,
-	.index		= -1,
-	.data		= &atmel_uart,
+	.name	= ATMEL_DEVICENAME,
+	.write	= atmel_console_write,
+	.device	= uart_console_device,
+	.setup	= atmel_console_setup,
+	.flags	= CON_PRINTBUFFER,
+	.index	= -1,
+	.data	= &atmel_uart,
 };
 
 #define ATMEL_CONSOLE_DEVICE	&atmel_console
@@ -871,13 +916,16 @@ static struct console atmel_console = {
 static int __init atmel_console_init(void)
 {
 	if (atmel_default_console_device) {
-		add_preferred_console(ATMEL_DEVICENAME, atmel_default_console_device->id, NULL);
-		atmel_init_port(&(atmel_ports[atmel_default_console_device->id]), atmel_default_console_device);
+		add_preferred_console(ATMEL_DEVICENAME,
+				      atmel_default_console_device->id, NULL);
+		atmel_init_port(&atmel_ports[atmel_default_console_device->id],
+				atmel_default_console_device);
 		register_console(&atmel_console);
 	}
 
 	return 0;
 }
+
 console_initcall(atmel_console_init);
 
 /*
@@ -885,11 +933,13 @@ console_initcall(atmel_console_init);
  */
 static int __init atmel_late_console_init(void)
 {
-	if (atmel_default_console_device && !(atmel_console.flags & CON_ENABLED))
+	if (atmel_default_console_device
+	    && !(atmel_console.flags & CON_ENABLED))
 		register_console(&atmel_console);
 
 	return 0;
 }
+
 core_initcall(atmel_late_console_init);
 
 #else
@@ -897,22 +947,24 @@ core_initcall(atmel_late_console_init);
 #endif
 
 static struct uart_driver atmel_uart = {
-	.owner			= THIS_MODULE,
-	.driver_name		= "atmel_serial",
-	.dev_name		= ATMEL_DEVICENAME,
-	.major			= SERIAL_ATMEL_MAJOR,
-	.minor			= MINOR_START,
-	.nr			= ATMEL_MAX_UART,
-	.cons			= ATMEL_CONSOLE_DEVICE,
+	.owner		= THIS_MODULE,
+	.driver_name	= "atmel_serial",
+	.dev_name	= ATMEL_DEVICENAME,
+	.major		= SERIAL_ATMEL_MAJOR,
+	.minor		= MINOR_START,
+	.nr		= ATMEL_MAX_UART,
+	.cons		= ATMEL_CONSOLE_DEVICE,
 };
 
 #ifdef CONFIG_PM
-static int atmel_serial_suspend(struct platform_device *pdev, pm_message_t state)
+static int atmel_serial_suspend(struct platform_device *pdev,
+				pm_message_t state)
 {
 	struct uart_port *port = platform_get_drvdata(pdev);
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
-	if (device_may_wakeup(&pdev->dev) && !at91_suspend_entering_slow_clock())
+	if (device_may_wakeup(&pdev->dev)
+	    && !at91_suspend_entering_slow_clock())
 		enable_irq_wake(port->irq);
 	else {
 		uart_suspend_port(&atmel_uart, port);
@@ -925,13 +977,12 @@ static int atmel_serial_suspend(struct platform_device *pdev, pm_message_t state
 static int atmel_serial_resume(struct platform_device *pdev)
 {
 	struct uart_port *port = platform_get_drvdata(pdev);
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
 	if (atmel_port->suspended) {
 		uart_resume_port(&atmel_uart, port);
 		atmel_port->suspended = 0;
-	}
-	else
+	} else
 		disable_irq_wake(port->irq);
 
 	return 0;
@@ -961,7 +1012,7 @@ static int __devinit atmel_serial_probe(struct platform_device *pdev)
 static int __devexit atmel_serial_remove(struct platform_device *pdev)
 {
 	struct uart_port *port = platform_get_drvdata(pdev);
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 	int ret = 0;
 
 	clk_disable(atmel_port->clk);
-- 
1.5.3.4

[PATCH v2 2/6] atmel_serial: Use cpu_relax() when busy-waiting

[Haavard Skinnemoen]

Replace two instances of barrier() with cpu_relax() since that's the
right thing to do when busy-waiting. This does not actually change
anything since cpu_relax() is defined as barrier() on both ARM and
AVR32.

Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 drivers/serial/atmel_serial.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
index bd41529..61d71b0 100644
--- a/drivers/serial/atmel_serial.c
+++ b/drivers/serial/atmel_serial.c
@@ -623,7 +623,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 	imr = UART_GET_IMR(port);
 	UART_PUT_IDR(port, -1);
 	while (!(UART_GET_CSR(port) & ATMEL_US_TXEMPTY))
-		barrier();
+		cpu_relax();
 
 	/* disable receiver and transmitter */
 	UART_PUT_CR(port, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
@@ -802,7 +802,7 @@ void __init atmel_register_uart_fns(struct atmel_port_fns *fns)
 static void atmel_console_putchar(struct uart_port *port, int ch)
 {
 	while (!(UART_GET_CSR(port) & ATMEL_US_TXRDY))
-		barrier();
+		cpu_relax();
 	UART_PUT_CHAR(port, ch);
 }
 
-- 
1.5.3.4

[PATCH v2 3/6] atmel_serial: Use existing console options only if BRG is running

[Haavard Skinnemoen]

If BRGR is zero, the baud rate generator isn't running, so the boot
loader can't have initialized the port.

Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 drivers/serial/atmel_serial.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
index 61d71b0..a6b3828 100644
--- a/drivers/serial/atmel_serial.c
+++ b/drivers/serial/atmel_serial.c
@@ -842,13 +842,13 @@ static void __init atmel_console_get_options(struct uart_port *port, int *baud,
 {
 	unsigned int mr, quot;
 
-// TODO: CR is a write-only register
-//	unsigned int cr;
-//
-//	cr = UART_GET_CR(port) & (ATMEL_US_RXEN | ATMEL_US_TXEN);
-//	if (cr == (ATMEL_US_RXEN | ATMEL_US_TXEN)) {
-//		/* ok, the port was enabled */
-//	}
+	/*
+	 * If the baud rate generator isn't running, the port wasn't
+	 * initialized by the boot loader.
+	 */
+	quot = UART_GET_BRGR(port);
+	if (!quot)
+		return;
 
 	mr = UART_GET_MR(port) & ATMEL_US_CHRL;
 	if (mr == ATMEL_US_CHRL_8)
@@ -868,7 +868,6 @@ static void __init atmel_console_get_options(struct uart_port *port, int *baud,
 	 * lower than one of those, as it would make us fall through
 	 * to a much lower baud rate than we really want.
 	 */
-	quot = UART_GET_BRGR(port);
 	*baud = port->uartclk / (16 * (quot - 1));
 }
 
-- 
1.5.3.4

[PATCH v2 4/6] atmel_serial: Fix bugs in probe() error path and remove()

[Haavard Skinnemoen]

When an error happens in probe(), the clocks should be disabled, but
only if the port isn't already used as a console.

In remove(), the port struct shouldn't be freed because it's defined
statically.

Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 drivers/serial/atmel_serial.c |   39 +++++++++++++++++++++++++++++----------
 1 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
index a6b3828..d2b4b0a 100644
--- a/drivers/serial/atmel_serial.c
+++ b/drivers/serial/atmel_serial.c
@@ -941,8 +941,18 @@ static int __init atmel_late_console_init(void)
 
 core_initcall(atmel_late_console_init);
 
+static inline bool atmel_is_console_port(struct uart_port *port)
+{
+	return port->cons && port->cons->index == port->line;
+}
+
 #else
 #define ATMEL_CONSOLE_DEVICE	NULL
+
+static inline bool atmel_is_console_port(struct uart_port *port)
+{
+	return false;
+}
 #endif
 
 static struct uart_driver atmel_uart = {
@@ -1000,9 +1010,19 @@ static int __devinit atmel_serial_probe(struct platform_device *pdev)
 	atmel_init_port(port, pdev);
 
 	ret = uart_add_one_port(&atmel_uart, &port->uart);
-	if (!ret) {
-		device_init_wakeup(&pdev->dev, 1);
-		platform_set_drvdata(pdev, port);
+	if (ret)
+		goto err_add_port;
+
+	device_init_wakeup(&pdev->dev, 1);
+	platform_set_drvdata(pdev, port);
+
+	return 0;
+
+err_add_port:
+	if (!atmel_is_console_port(&port->uart)) {
+		clk_disable(port->clk);
+		clk_put(port->clk);
+		port->clk = NULL;
 	}
 
 	return ret;
@@ -1014,16 +1034,15 @@ static int __devexit atmel_serial_remove(struct platform_device *pdev)
 	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 	int ret = 0;
 
-	clk_disable(atmel_port->clk);
-	clk_put(atmel_port->clk);
-
 	device_init_wakeup(&pdev->dev, 0);
 	platform_set_drvdata(pdev, NULL);
 
-	if (port) {
-		ret = uart_remove_one_port(&atmel_uart, port);
-		kfree(port);
-	}
+	ret = uart_remove_one_port(&atmel_uart, port);
+
+	/* "port" is allocated statically, so we shouldn't free it */
+
+	clk_disable(atmel_port->clk);
+	clk_put(atmel_port->clk);
 
 	return ret;
 }
-- 
1.5.3.4

[PATCH v2 5/6] atmel_serial: Split the interrupt handler

[Haavard Skinnemoen]

From: Remy Bohmer <linux@bohmer.net>

This patch splits up the interrupt handler of the serial port
into a interrupt top-half and a tasklet.

The goal is to get the interrupt top-half as short as possible to
minimize latencies on interrupts. But the old code also does some
calls in the interrupt handler that are not allowed on preempt-RT
in IRQF_NODELAY context. This handler is executed in this context
because of the interrupt sharing with the timer interrupt.
The timer interrupt on Preempt-RT runs in IRQF_NODELAY context.

The tasklet takes care of handling control status changes, pushing
incoming characters to the tty layer, handling break and other errors.
It also handles pushing TX data into the data register.

Reading the complete receive queue is still done in the top-half
because we never want to miss any incoming character.

[hskinnemoen@atmel.com: misc cleanups and simplifications]
Signed-off-by: Remy Bohmer <linux@bohmer.net>
Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 drivers/serial/atmel_serial.c |  239 +++++++++++++++++++++++++++++++----------
 1 files changed, 184 insertions(+), 55 deletions(-)

diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
index d2b4b0a..0013c6a 100644
--- a/drivers/serial/atmel_serial.c
+++ b/drivers/serial/atmel_serial.c
@@ -111,6 +111,13 @@
 static int (*atmel_open_hook) (struct uart_port *);
 static void (*atmel_close_hook) (struct uart_port *);
 
+struct atmel_uart_char {
+	u16		status;
+	u16		ch;
+};
+
+#define ATMEL_SERIAL_RINGSIZE 1024
+
 /*
  * We wrap our port structure around the generic uart_port.
  */
@@ -119,6 +126,12 @@ struct atmel_uart_port {
 	struct clk		*clk;		/* uart clock */
 	unsigned short		suspended;	/* is port suspended? */
 	int			break_active;	/* break being received */
+
+	struct tasklet_struct	tasklet;
+	unsigned int		irq_status;
+	unsigned int		irq_status_prev;
+
+	struct circ_buf		rx_ring;
 };
 
 static struct atmel_uart_port atmel_ports[ATMEL_MAX_UART];
@@ -248,22 +261,42 @@ static void atmel_break_ctl(struct uart_port *port, int break_state)
 }
 
 /*
+ * Stores the incoming character in the ring buffer
+ */
+static void
+atmel_buffer_rx_char(struct uart_port *port, unsigned int status,
+		     unsigned int ch)
+{
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+	struct circ_buf *ring = &atmel_port->rx_ring;
+	struct atmel_uart_char *c;
+
+	if (!CIRC_SPACE(ring->head, ring->tail, ATMEL_SERIAL_RINGSIZE))
+		/* Buffer overflow, ignore char */
+		return;
+
+	c = &((struct atmel_uart_char *)ring->buf)[ring->head];
+	c->status	= status;
+	c->ch		= ch;
+
+	/* Make sure the character is stored before we update head. */
+	smp_wmb();
+
+	ring->head = (ring->head + 1) & (ATMEL_SERIAL_RINGSIZE - 1);
+}
+
+/*
  * Characters received (called from interrupt handler)
  */
 static void atmel_rx_chars(struct uart_port *port)
 {
 	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
-	struct tty_struct *tty = port->info->tty;
-	unsigned int status, ch, flg;
+	unsigned int status, ch;
 
 	status = UART_GET_CSR(port);
 	while (status & ATMEL_US_RXRDY) {
 		ch = UART_GET_CHAR(port);
 
-		port->icount.rx++;
-
-		flg = TTY_NORMAL;
-
 		/*
 		 * note that the error handling code is
 		 * out of the main execution path
@@ -271,17 +304,14 @@ static void atmel_rx_chars(struct uart_port *port)
 		if (unlikely(status & (ATMEL_US_PARE | ATMEL_US_FRAME
 				       | ATMEL_US_OVRE | ATMEL_US_RXBRK)
 			     || atmel_port->break_active)) {
+
 			/* clear error */
 			UART_PUT_CR(port, ATMEL_US_RSTSTA);
+
 			if (status & ATMEL_US_RXBRK
 			    && !atmel_port->break_active) {
-				/* ignore side-effect */
-				status &= ~(ATMEL_US_PARE | ATMEL_US_FRAME);
-				port->icount.brk++;
 				atmel_port->break_active = 1;
 				UART_PUT_IER(port, ATMEL_US_RXBRK);
-				if (uart_handle_break(port))
-					goto ignore_char;
 			} else {
 				/*
 				 * This is either the end-of-break
@@ -294,52 +324,30 @@ static void atmel_rx_chars(struct uart_port *port)
 				status &= ~ATMEL_US_RXBRK;
 				atmel_port->break_active = 0;
 			}
-			if (status & ATMEL_US_PARE)
-				port->icount.parity++;
-			if (status & ATMEL_US_FRAME)
-				port->icount.frame++;
-			if (status & ATMEL_US_OVRE)
-				port->icount.overrun++;
-
-			status &= port->read_status_mask;
-
-			if (status & ATMEL_US_RXBRK)
-				flg = TTY_BREAK;
-			else if (status & ATMEL_US_PARE)
-				flg = TTY_PARITY;
-			else if (status & ATMEL_US_FRAME)
-				flg = TTY_FRAME;
 		}
 
-		if (uart_handle_sysrq_char(port, ch))
-			goto ignore_char;
-
-		uart_insert_char(port, status, ATMEL_US_OVRE, ch, flg);
-
-ignore_char:
+		atmel_buffer_rx_char(port, status, ch);
 		status = UART_GET_CSR(port);
 	}
 
-	tty_flip_buffer_push(tty);
+	tasklet_schedule(&atmel_port->tasklet);
 }
 
 /*
- * Transmit characters (called from interrupt handler)
+ * Transmit characters (called from tasklet with TXRDY interrupt
+ * disabled)
  */
 static void atmel_tx_chars(struct uart_port *port)
 {
 	struct circ_buf *xmit = &port->info->xmit;
 
-	if (port->x_char) {
+	if (port->x_char && UART_GET_CSR(port) & ATMEL_US_TXRDY) {
 		UART_PUT_CHAR(port, port->x_char);
 		port->icount.tx++;
 		port->x_char = 0;
-		return;
 	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		atmel_stop_tx(port);
+	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
 		return;
-	}
 
 	while (UART_GET_CSR(port) & ATMEL_US_TXRDY) {
 		UART_PUT_CHAR(port, xmit->buf[xmit->tail]);
@@ -352,8 +360,8 @@ static void atmel_tx_chars(struct uart_port *port)
 	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
 		uart_write_wakeup(port);
 
-	if (uart_circ_empty(xmit))
-		atmel_stop_tx(port);
+	if (!uart_circ_empty(xmit))
+		UART_PUT_IER(port, ATMEL_US_TXRDY);
 }
 
 /*
@@ -379,14 +387,18 @@ atmel_handle_receive(struct uart_port *port, unsigned int pending)
 }
 
 /*
- * transmit interrupt handler.
+ * transmit interrupt handler. (Transmit is IRQF_NODELAY safe)
  */
 static void
 atmel_handle_transmit(struct uart_port *port, unsigned int pending)
 {
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+
 	/* Interrupt transmit */
-	if (pending & ATMEL_US_TXRDY)
-		atmel_tx_chars(port);
+	if (pending & ATMEL_US_TXRDY) {
+		UART_PUT_IDR(port, ATMEL_US_TXRDY);
+		tasklet_schedule(&atmel_port->tasklet);
+	}
 }
 
 /*
@@ -396,18 +408,13 @@ static void
 atmel_handle_status(struct uart_port *port, unsigned int pending,
 		    unsigned int status)
 {
-	/* TODO: All reads to CSR will clear these interrupts! */
-	if (pending & ATMEL_US_RIIC)
-		port->icount.rng++;
-	if (pending & ATMEL_US_DSRIC)
-		port->icount.dsr++;
-	if (pending & ATMEL_US_DCDIC)
-		uart_handle_dcd_change(port, !(status & ATMEL_US_DCD));
-	if (pending & ATMEL_US_CTSIC)
-		uart_handle_cts_change(port, !(status & ATMEL_US_CTS));
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+
 	if (pending & (ATMEL_US_RIIC | ATMEL_US_DSRIC | ATMEL_US_DCDIC
-				| ATMEL_US_CTSIC))
-		wake_up_interruptible(&port->info->delta_msr_wait);
+				| ATMEL_US_CTSIC)) {
+		atmel_port->irq_status = status;
+		tasklet_schedule(&atmel_port->tasklet);
+	}
 }
 
 /*
@@ -434,6 +441,108 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void atmel_rx_from_ring(struct uart_port *port)
+{
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+	struct circ_buf *ring = &atmel_port->rx_ring;
+	unsigned int flg;
+	unsigned int status;
+
+	while (ring->head != ring->tail) {
+		struct atmel_uart_char c;
+
+		/* Make sure c is loaded after head. */
+		smp_rmb();
+
+		c = ((struct atmel_uart_char *)ring->buf)[ring->tail];
+
+		ring->tail = (ring->tail + 1) & (ATMEL_SERIAL_RINGSIZE - 1);
+
+		port->icount.rx++;
+		status = c.status;
+		flg = TTY_NORMAL;
+
+		/*
+		 * note that the error handling code is
+		 * out of the main execution path
+		 */
+		if (unlikely(status & (ATMEL_US_PARE | ATMEL_US_FRAME
+				       | ATMEL_US_OVRE | ATMEL_US_RXBRK))) {
+			if (status & ATMEL_US_RXBRK) {
+				/* ignore side-effect */
+				status &= ~(ATMEL_US_PARE | ATMEL_US_FRAME);
+
+				port->icount.brk++;
+				if (uart_handle_break(port))
+					continue;
+			}
+			if (status & ATMEL_US_PARE)
+				port->icount.parity++;
+			if (status & ATMEL_US_FRAME)
+				port->icount.frame++;
+			if (status & ATMEL_US_OVRE)
+				port->icount.overrun++;
+
+			status &= port->read_status_mask;
+
+			if (status & ATMEL_US_RXBRK)
+				flg = TTY_BREAK;
+			else if (status & ATMEL_US_PARE)
+				flg = TTY_PARITY;
+			else if (status & ATMEL_US_FRAME)
+				flg = TTY_FRAME;
+		}
+
+
+		if (uart_handle_sysrq_char(port, c.ch))
+			continue;
+
+		uart_insert_char(port, status, ATMEL_US_OVRE, c.ch, flg);
+	}
+
+	tty_flip_buffer_push(port->info->tty);
+}
+
+/*
+ * tasklet handling tty stuff outside the interrupt handler.
+ */
+static void atmel_tasklet_func(unsigned long data)
+{
+	struct uart_port *port = (struct uart_port *)data;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+	unsigned int status;
+	unsigned int status_change;
+
+	/* The interrupt handler does not take the lock */
+	spin_lock(&port->lock);
+
+	atmel_tx_chars(port);
+
+	status = atmel_port->irq_status;
+	status_change = status ^ atmel_port->irq_status_prev;
+
+	if (status_change & (ATMEL_US_RI | ATMEL_US_DSR
+				| ATMEL_US_DCD | ATMEL_US_CTS)) {
+		/* TODO: All reads to CSR will clear these interrupts! */
+		if (status_change & ATMEL_US_RI)
+			port->icount.rng++;
+		if (status_change & ATMEL_US_DSR)
+			port->icount.dsr++;
+		if (status_change & ATMEL_US_DCD)
+			uart_handle_dcd_change(port, !(status & ATMEL_US_DCD));
+		if (status_change & ATMEL_US_CTS)
+			uart_handle_cts_change(port, !(status & ATMEL_US_CTS));
+
+		wake_up_interruptible(&port->info->delta_msr_wait);
+
+		atmel_port->irq_status_prev = status;
+	}
+
+	atmel_rx_from_ring(port);
+
+	spin_unlock(&port->lock);
+}
+
 /*
  * Perform initialization and enable port for reception
  */
@@ -765,6 +874,11 @@ static void __devinit atmel_init_port(struct atmel_uart_port *atmel_port,
 	port->mapbase	= pdev->resource[0].start;
 	port->irq	= pdev->resource[1].start;
 
+	tasklet_init(&atmel_port->tasklet, atmel_tasklet_func,
+			(unsigned long)port);
+
+	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
+
 	if (data->regs)
 		/* Already mapped by setup code */
 		port->membase = data->regs;
@@ -1004,11 +1118,20 @@ static int atmel_serial_resume(struct platform_device *pdev)
 static int __devinit atmel_serial_probe(struct platform_device *pdev)
 {
 	struct atmel_uart_port *port;
+	void *data;
 	int ret;
 
+	BUILD_BUG_ON(!is_power_of_2(ATMEL_SERIAL_RINGSIZE));
+
 	port = &atmel_ports[pdev->id];
 	atmel_init_port(port, pdev);
 
+	ret = -ENOMEM;
+	data = kmalloc(ATMEL_SERIAL_RINGSIZE, GFP_KERNEL);
+	if (!data)
+		goto err_alloc_ring;
+	port->rx_ring.buf = data;
+
 	ret = uart_add_one_port(&atmel_uart, &port->uart);
 	if (ret)
 		goto err_add_port;
@@ -1019,6 +1142,9 @@ static int __devinit atmel_serial_probe(struct platform_device *pdev)
 	return 0;
 
 err_add_port:
+	kfree(port->rx_ring.buf);
+	port->rx_ring.buf = NULL;
+err_alloc_ring:
 	if (!atmel_is_console_port(&port->uart)) {
 		clk_disable(port->clk);
 		clk_put(port->clk);
@@ -1039,6 +1165,9 @@ static int __devexit atmel_serial_remove(struct platform_device *pdev)
 
 	ret = uart_remove_one_port(&atmel_uart, port);
 
+	tasklet_kill(&atmel_port->tasklet);
+	kfree(atmel_port->rx_ring.buf);
+
 	/* "port" is allocated statically, so we shouldn't free it */
 
 	clk_disable(atmel_port->clk);
-- 
1.5.3.4

[PATCH v2 6/6] atmel_serial: Add DMA support

[Haavard Skinnemoen]

From: Chip Coldwell <coldwell@redhat.com>

This patch is based on the DMA-patch by Chip Coldwell for the
AT91/AT32 serial USARTS, with some tweaks to make it apply neatly on
top of the other patches in this series.

The RX and TX code has been moved to a tasklet and reworked a bit.
Instead of depending on the ENDRX and TIMEOUT bits in CSR, we simply
grab as much data as we can from the DMA buffers. I think this closes
a race where the ENDRX bit is set after we read CSR but before we read
RPR, although I haven't confirmed this.

Similarly, the two TX handlers (ENDTX and TXBUFE) have been combined
into one. Since the current code only uses a single TX buffer, there's
no point in handling those interrupts separately.

This also fixes a DMA sync bug in the original patch.

[linux@bohmer.net: rebased onto irq-splitup patch]
[hskinnemoen@atmel.com: moved to tasklet, fixed dma bug, misc cleanups]
Signed-off-by: Remy Bohmer <linux@bohmer.net>
Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 drivers/serial/atmel_serial.c |  390 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 365 insertions(+), 25 deletions(-)

diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
index 0013c6a..7967054 100644
--- a/drivers/serial/atmel_serial.c
+++ b/drivers/serial/atmel_serial.c
@@ -7,6 +7,8 @@
  *  Based on drivers/char/serial_sa1100.c, by Deep Blue Solutions Ltd.
  *  Based on drivers/char/serial.c, by Linus Torvalds, Theodore Ts'o.
  *
+ *  DMA support added by Chip Coldwell.
+ *
  * 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
@@ -33,6 +35,7 @@
 #include <linux/sysrq.h>
 #include <linux/tty_flip.h>
 #include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
 #include <linux/atmel_pdc.h>
 
 #include <asm/io.h>
@@ -47,6 +50,11 @@
 
 #include "atmel_serial.h"
 
+#define SUPPORT_PDC
+#define PDC_BUFFER_SIZE		(L1_CACHE_BYTES << 3)
+#warning "Revisit"
+#define PDC_RX_TIMEOUT		(3 * 10)		/* 3 bytes */
+
 #if defined(CONFIG_SERIAL_ATMEL_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
 #define SUPPORT_SYSRQ
 #endif
@@ -111,6 +119,13 @@
 static int (*atmel_open_hook) (struct uart_port *);
 static void (*atmel_close_hook) (struct uart_port *);
 
+struct atmel_dma_buffer {
+	unsigned char	*buf;
+	dma_addr_t	dma_addr;
+	size_t		dma_size;
+	unsigned int	ofs;
+};
+
 struct atmel_uart_char {
 	u16		status;
 	u16		ch;
@@ -127,6 +142,13 @@ struct atmel_uart_port {
 	unsigned short		suspended;	/* is port suspended? */
 	int			break_active;	/* break being received */
 
+	short			use_dma_rx;	/* enable PDC receiver */
+	short			pdc_rx_idx;	/* current PDC RX buffer */
+	struct atmel_dma_buffer	pdc_rx[2];	/* PDC receier */
+
+	short			use_dma_tx;	/* enable PDC transmitter */
+	struct atmel_dma_buffer	pdc_tx;		/* PDC transmitter */
+
 	struct tasklet_struct	tasklet;
 	unsigned int		irq_status;
 	unsigned int		irq_status_prev;
@@ -136,10 +158,39 @@ struct atmel_uart_port {
 
 static struct atmel_uart_port atmel_ports[ATMEL_MAX_UART];
 
+#define PDC_RX_BUF(port)	&(port)->pdc_rx[(port)->pdc_rx_idx]
+#define PDC_RX_SWITCH(port)	(port)->pdc_rx_idx = !(port)->pdc_rx_idx
+
 #ifdef SUPPORT_SYSRQ
 static struct console atmel_console;
 #endif
 
+#ifdef SUPPORT_PDC
+static bool atmel_use_dma_rx(struct uart_port *port)
+{
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+
+	return atmel_port->use_dma_rx;
+}
+
+static bool atmel_use_dma_tx(struct uart_port *port)
+{
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+
+	return atmel_port->use_dma_tx;
+}
+#else
+static bool atmel_use_dma_rx(struct uart_port *port)
+{
+	return false;
+}
+
+static bool atmel_use_dma_tx(struct uart_port *port)
+{
+	return false;
+}
+#endif
+
 /*
  * Return TIOCSER_TEMT when transmitter FIFO and Shift register is empty.
  */
@@ -221,7 +272,12 @@ static u_int atmel_get_mctrl(struct uart_port *port)
  */
 static void atmel_stop_tx(struct uart_port *port)
 {
-	UART_PUT_IDR(port, ATMEL_US_TXRDY);
+	if (atmel_use_dma_tx(port)) {
+		/* disable PDC transmit */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS);
+		UART_PUT_IDR(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE);
+	} else
+		UART_PUT_IDR(port, ATMEL_US_TXRDY);
 }
 
 /*
@@ -229,7 +285,17 @@ static void atmel_stop_tx(struct uart_port *port)
  */
 static void atmel_start_tx(struct uart_port *port)
 {
-	UART_PUT_IER(port, ATMEL_US_TXRDY);
+	if (atmel_use_dma_tx(port)) {
+		if (UART_GET_PTSR(port) & ATMEL_PDC_TXTEN)
+			/* The transmitter is already running.  Yes, we
+			   really need this.*/
+			return;
+
+		UART_PUT_IER(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE);
+		/* re-enable PDC transmit */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);
+	} else
+		UART_PUT_IER(port, ATMEL_US_TXRDY);
 }
 
 /*
@@ -237,7 +303,12 @@ static void atmel_start_tx(struct uart_port *port)
  */
 static void atmel_stop_rx(struct uart_port *port)
 {
-	UART_PUT_IDR(port, ATMEL_US_RXRDY);
+	if (atmel_use_dma_rx(port)) {
+		/* disable PDC receive */
+		UART_PUT_PTCR(port, ATMEL_PDC_RXTDIS);
+		UART_PUT_IDR(port, ATMEL_US_ENDRX | ATMEL_US_TIMEOUT);
+	} else
+		UART_PUT_IDR(port, ATMEL_US_RXRDY);
 }
 
 /*
@@ -286,6 +357,27 @@ atmel_buffer_rx_char(struct uart_port *port, unsigned int status,
 }
 
 /*
+ * Deal with parity, framing and overrun errors.
+ */
+static void atmel_pdc_rxerr(struct uart_port *port, unsigned int status)
+{
+	/* clear error */
+	UART_PUT_CR(port, ATMEL_US_RSTSTA);
+
+	if (status & ATMEL_US_RXBRK) {
+		/* ignore side-effect */
+		status &= ~(ATMEL_US_PARE | ATMEL_US_FRAME);
+		port->icount.brk++;
+	}
+	if (status & ATMEL_US_PARE)
+		port->icount.parity++;
+	if (status & ATMEL_US_FRAME)
+		port->icount.frame++;
+	if (status & ATMEL_US_OVRE)
+		port->icount.overrun++;
+}
+
+/*
  * Characters received (called from interrupt handler)
  */
 static void atmel_rx_chars(struct uart_port *port)
@@ -372,6 +464,25 @@ atmel_handle_receive(struct uart_port *port, unsigned int pending)
 {
 	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
+	if (atmel_use_dma_rx(port)) {
+		/*
+		 * PDC receive. Just schedule the tasklet and let it
+		 * figure out the details.
+		 *
+		 * TODO: We're not handling error flags correctly at
+		 * the moment.
+		 */
+		if (pending & (ATMEL_US_ENDRX | ATMEL_US_TIMEOUT)) {
+			UART_PUT_IDR(port, (ATMEL_US_ENDRX
+						| ATMEL_US_TIMEOUT));
+			tasklet_schedule(&atmel_port->tasklet);
+		}
+
+		if (pending & (ATMEL_US_RXBRK | ATMEL_US_OVRE |
+				ATMEL_US_FRAME | ATMEL_US_PARE))
+			atmel_pdc_rxerr(port, pending);
+	}
+
 	/* Interrupt receive */
 	if (pending & ATMEL_US_RXRDY)
 		atmel_rx_chars(port);
@@ -394,10 +505,18 @@ atmel_handle_transmit(struct uart_port *port, unsigned int pending)
 {
 	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
-	/* Interrupt transmit */
-	if (pending & ATMEL_US_TXRDY) {
-		UART_PUT_IDR(port, ATMEL_US_TXRDY);
-		tasklet_schedule(&atmel_port->tasklet);
+	if (atmel_use_dma_tx(port)) {
+		/* PDC transmit */
+		if (pending & (ATMEL_US_ENDTX | ATMEL_US_TXBUFE)) {
+			UART_PUT_IDR(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE);
+			tasklet_schedule(&atmel_port->tasklet);
+		}
+	} else {
+		/* Interrupt transmit */
+		if (pending & ATMEL_US_TXRDY) {
+			UART_PUT_IDR(port, ATMEL_US_TXRDY);
+			tasklet_schedule(&atmel_port->tasklet);
+		}
 	}
 }
 
@@ -425,20 +544,67 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id)
 	struct uart_port *port = dev_id;
 	unsigned int status, pending, pass_counter = 0;
 
-	status = UART_GET_CSR(port);
-	pending = status & UART_GET_IMR(port);
-	while (pending) {
+	do {
+		status = UART_GET_CSR(port);
+		pending = status & UART_GET_IMR(port);
+		if (!pending)
+			break;
+
 		atmel_handle_receive(port, pending);
 		atmel_handle_status(port, pending, status);
 		atmel_handle_transmit(port, pending);
+	} while (pass_counter++ < ATMEL_ISR_PASS_LIMIT);
 
-		if (pass_counter++ > ATMEL_ISR_PASS_LIMIT)
-			break;
+	return IRQ_HANDLED;
+}
 
-		status = UART_GET_CSR(port);
-		pending = status & UART_GET_IMR(port);
+/*
+ * Called from tasklet with ENDTX and TXBUFE interrupts disabled.
+ */
+static void atmel_tx_dma(struct uart_port *port)
+{
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+	struct circ_buf *xmit = &port->info->xmit;
+	struct atmel_dma_buffer *pdc = &atmel_port->pdc_tx;
+	int count;
+
+	xmit->tail += pdc->ofs;
+	if (xmit->tail >= SERIAL_XMIT_SIZE)
+		xmit->tail -= SERIAL_XMIT_SIZE;
+
+	port->icount.tx += pdc->ofs;
+	pdc->ofs = 0;
+
+	if (!uart_circ_empty(xmit)) {
+		/* more to transmit - setup next transfer */
+
+		/* disable PDC transmit */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS);
+		dma_sync_single_for_device(port->dev,
+					   pdc->dma_addr,
+					   pdc->dma_size,
+					   DMA_TO_DEVICE);
+
+		if (xmit->tail < xmit->head)
+			count = xmit->head - xmit->tail;
+		else
+			count = SERIAL_XMIT_SIZE - xmit->tail;
+		pdc->ofs = count;
+
+		UART_PUT_TPR(port, pdc->dma_addr + xmit->tail);
+		UART_PUT_TCR(port, count);
+		/* re-enable PDC transmit and interrupts */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);
+		UART_PUT_IER(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE);
+	} else {
+		/* nothing left to transmit - disable the transmitter */
+
+		/* disable PDC transmit */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS);
 	}
-	return IRQ_HANDLED;
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(port);
 }
 
 static void atmel_rx_from_ring(struct uart_port *port)
@@ -503,6 +669,70 @@ static void atmel_rx_from_ring(struct uart_port *port)
 	tty_flip_buffer_push(port->info->tty);
 }
 
+static void atmel_rx_from_dma(struct uart_port *port)
+{
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+	struct tty_struct *tty = port->info->tty;
+	struct atmel_dma_buffer *pdc;
+	int rx_idx = atmel_port->pdc_rx_idx;
+	unsigned int head;
+	unsigned int tail;
+	unsigned int count;
+
+	do {
+		/* Reset the UART timeout early so that we don't miss one */
+		UART_PUT_CR(port, ATMEL_US_STTTO);
+
+		pdc = &atmel_port->pdc_rx[rx_idx];
+		head = UART_GET_RPR(port) - pdc->dma_addr;
+		tail = pdc->ofs;
+
+		/* If the PDC has switched buffers, RPR won't contain
+		 * any address within the current buffer. Since head
+		 * is unsigned, we just need a one-way comparison to
+		 * find out.
+		 *
+		 * In this case, we just need to consume the entire
+		 * buffer and resubmit it for DMA. This will clear the
+		 * ENDRX bit as well, so that we can safely re-enable
+		 * all interrupts below.
+		 */
+		if (head >= pdc->dma_size)
+			head = pdc->dma_size;
+
+		if (likely(head != tail)) {
+			dma_sync_single_for_cpu(port->dev, pdc->dma_addr,
+					pdc->dma_size, DMA_FROM_DEVICE);
+
+			count = head - tail;
+			tty_insert_flip_string(tty, pdc->buf + pdc->ofs, count);
+
+			dma_sync_single_for_device(port->dev, pdc->dma_addr,
+					pdc->dma_size, DMA_FROM_DEVICE);
+
+			port->icount.rx += count;
+			pdc->ofs = head;
+		}
+
+		/*
+		 * If the current buffer is full, we need to check if
+		 * the next one contains any additional data.
+		 */
+		if (head >= pdc->dma_size) {
+			pdc->ofs = 0;
+			UART_PUT_RNPR(port, pdc->dma_addr);
+			UART_PUT_RNCR(port, pdc->dma_size);
+
+			rx_idx = !rx_idx;
+			atmel_port->pdc_rx_idx = rx_idx;
+		}
+	} while (head >= pdc->dma_size);
+
+	tty_flip_buffer_push(tty);
+
+	UART_PUT_IER(port, ATMEL_US_ENDRX | ATMEL_US_TIMEOUT);
+}
+
 /*
  * tasklet handling tty stuff outside the interrupt handler.
  */
@@ -516,7 +746,10 @@ static void atmel_tasklet_func(unsigned long data)
 	/* The interrupt handler does not take the lock */
 	spin_lock(&port->lock);
 
-	atmel_tx_chars(port);
+	if (atmel_use_dma_tx(port))
+		atmel_tx_dma(port);
+	else
+		atmel_tx_chars(port);
 
 	status = atmel_port->irq_status;
 	status_change = status ^ atmel_port->irq_status_prev;
@@ -538,7 +771,10 @@ static void atmel_tasklet_func(unsigned long data)
 		atmel_port->irq_status_prev = status;
 	}
 
-	atmel_rx_from_ring(port);
+	if (atmel_use_dma_rx(port))
+		atmel_rx_from_dma(port);
+	else
+		atmel_rx_from_ring(port);
 
 	spin_unlock(&port->lock);
 }
@@ -548,6 +784,7 @@ static void atmel_tasklet_func(unsigned long data)
  */
 static int atmel_startup(struct uart_port *port)
 {
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 	int retval;
 
 	/*
@@ -568,6 +805,56 @@ static int atmel_startup(struct uart_port *port)
 	}
 
 	/*
+	 * Initialize DMA (if necessary)
+	 */
+	if (atmel_use_dma_rx(port)) {
+		int i;
+
+		for (i = 0; i < 2; i++) {
+			struct atmel_dma_buffer *pdc = &atmel_port->pdc_rx[i];
+
+			pdc->buf = kmalloc(PDC_BUFFER_SIZE, GFP_KERNEL);
+			if (pdc->buf == NULL) {
+				if (i != 0) {
+					dma_unmap_single(port->dev,
+						atmel_port->pdc_rx[0].dma_addr,
+						PDC_BUFFER_SIZE,
+						DMA_FROM_DEVICE);
+					kfree(atmel_port->pdc_rx[0].buf);
+				}
+				free_irq(port->irq, port);
+				return -ENOMEM;
+			}
+			pdc->dma_addr = dma_map_single(port->dev,
+						       pdc->buf,
+						       PDC_BUFFER_SIZE,
+						       DMA_FROM_DEVICE);
+			pdc->dma_size = PDC_BUFFER_SIZE;
+			pdc->ofs = 0;
+		}
+
+		atmel_port->pdc_rx_idx = 0;
+
+		UART_PUT_RPR(port, atmel_port->pdc_rx[0].dma_addr);
+		UART_PUT_RCR(port, PDC_BUFFER_SIZE);
+
+		UART_PUT_RNPR(port, atmel_port->pdc_rx[1].dma_addr);
+		UART_PUT_RNCR(port, PDC_BUFFER_SIZE);
+	}
+	if (atmel_use_dma_tx(port)) {
+		struct atmel_dma_buffer *pdc = &atmel_port->pdc_tx;
+		struct circ_buf *xmit = &port->info->xmit;
+
+		pdc->buf = xmit->buf;
+		pdc->dma_addr = dma_map_single(port->dev,
+					       pdc->buf,
+					       SERIAL_XMIT_SIZE,
+					       DMA_TO_DEVICE);
+		pdc->dma_size = SERIAL_XMIT_SIZE;
+		pdc->ofs = 0;
+	}
+
+	/*
 	 * If there is a specific "open" function (to register
 	 * control line interrupts)
 	 */
@@ -586,8 +873,18 @@ static int atmel_startup(struct uart_port *port)
 	/* enable xmit & rcvr */
 	UART_PUT_CR(port, ATMEL_US_TXEN | ATMEL_US_RXEN);
 
-	/* enable receive only */
-	UART_PUT_IER(port, ATMEL_US_RXRDY);
+	if (atmel_use_dma_rx(port)) {
+		/* set UART timeout */
+		UART_PUT_RTOR(port, PDC_RX_TIMEOUT);
+		UART_PUT_CR(port, ATMEL_US_STTTO);
+
+		UART_PUT_IER(port, ATMEL_US_ENDRX | ATMEL_US_TIMEOUT);
+		/* enable PDC controller */
+		UART_PUT_PTCR(port, ATMEL_PDC_RXTEN);
+	} else {
+		/* enable receive only */
+		UART_PUT_IER(port, ATMEL_US_RXRDY);
+	}
 
 	return 0;
 }
@@ -597,6 +894,38 @@ static int atmel_startup(struct uart_port *port)
  */
 static void atmel_shutdown(struct uart_port *port)
 {
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+	/*
+	 * Ensure everything is stopped.
+	 */
+	atmel_stop_rx(port);
+	atmel_stop_tx(port);
+
+	/*
+	 * Shut-down the DMA.
+	 */
+	if (atmel_use_dma_rx(port)) {
+		int i;
+
+		for (i = 0; i < 2; i++) {
+			struct atmel_dma_buffer *pdc = &atmel_port->pdc_rx[i];
+
+			dma_unmap_single(port->dev,
+					 pdc->dma_addr,
+					 pdc->dma_size,
+					 DMA_FROM_DEVICE);
+			kfree(pdc->buf);
+		}
+	}
+	if (atmel_use_dma_tx(port)) {
+		struct atmel_dma_buffer *pdc = &atmel_port->pdc_tx;
+
+		dma_unmap_single(port->dev,
+				 pdc->dma_addr,
+				 pdc->dma_size,
+				 DMA_TO_DEVICE);
+	}
+
 	/*
 	 * Disable all interrupts, port and break condition.
 	 */
@@ -708,6 +1037,10 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 	if (termios->c_iflag & (BRKINT | PARMRK))
 		port->read_status_mask |= ATMEL_US_RXBRK;
 
+	if (atmel_use_dma_rx(port))
+		/* need to enable error interrupts */
+		UART_PUT_IER(port, port->read_status_mask);
+
 	/*
 	 * Characters to ignore
 	 */
@@ -893,6 +1226,11 @@ static void __devinit atmel_init_port(struct atmel_uart_port *atmel_port,
 		clk_enable(atmel_port->clk);
 		port->uartclk = clk_get_rate(atmel_port->clk);
 	}
+
+	atmel_port->use_dma_rx = data->use_dma_rx;
+	atmel_port->use_dma_tx = data->use_dma_tx;
+	if (atmel_use_dma_tx(port))
+		port->fifosize = PDC_BUFFER_SIZE;
 }
 
 /*
@@ -1087,7 +1425,7 @@ static int atmel_serial_suspend(struct platform_device *pdev,
 	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
 	if (device_may_wakeup(&pdev->dev)
-	    && !at91_suspend_entering_slow_clock())
+		&& !clk_must_disable(atmel_port->clk))
 		enable_irq_wake(port->irq);
 	else {
 		uart_suspend_port(&atmel_uart, port);
@@ -1126,11 +1464,13 @@ static int __devinit atmel_serial_probe(struct platform_device *pdev)
 	port = &atmel_ports[pdev->id];
 	atmel_init_port(port, pdev);
 
-	ret = -ENOMEM;
-	data = kmalloc(ATMEL_SERIAL_RINGSIZE, GFP_KERNEL);
-	if (!data)
-		goto err_alloc_ring;
-	port->rx_ring.buf = data;
+	if (!atmel_use_dma_rx(&port->uart)) {
+		ret = -ENOMEM;
+		data = kmalloc(ATMEL_SERIAL_RINGSIZE, GFP_KERNEL);
+		if (!data)
+			goto err_alloc_ring;
+		port->rx_ring.buf = data;
+	}
 
 	ret = uart_add_one_port(&atmel_uart, &port->uart);
 	if (ret)
-- 
1.5.3.4

Re: [PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[Remy Bohmer]

Hello Haavard,

Sorry.. But I get an Oops on Preempt-RT with the latest set of
patches. I did not see it earlier today with the other set of patches.

Here it is...

Remy

root@134:~ [   13.760735] Unable to handle kernel NULL pointer
dereference at virtual address 00000000
[   13.760735] pgd = c0004000
[   13.760735] [00000000] *pgd=00000000
[   13.760735] stopped custom tracer.
[   13.760735] Internal error: Oops: 817 [#1] PREEMPT
[   13.760735] CPU: 0    Not tainted  (2.6.23.12-rt14 #76)
[   13.760735] PC is at rt_spin_lock_slowlock+0x84/0x1c8
[   13.760735] LR is at rt_spin_lock_slowlock+0x54/0x1c8
[   13.760735] pc : [<c01a46f4>]    lr : [<c01a46c4>]    psr: 60000093
[   13.760735] sp : c02e1d38  ip : c02dd5a0  fp : c02e1d94
[   13.760735] r10: c3c3cc21  r9 : c001ec58  r8 : 00000001
[   13.760735] r7 : c001e800  r6 : c0212aec  r5 : c02e0000  r4 : a0000013
[   13.760735] r3 : c02dd5a0  r2 : c02dd5a0  r1 : c0212afc  r0 : 00000000
[   13.760735] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
Segment kernel
[   13.760735] Control: c000717f  Table: 23d98000  DAC: 00000017
[   13.760735] Process softirq-tasklet (pid: 9, stack limit = 0xc02e0268)
[   13.760735] Stack: (0xc02e1d38 to 0xc02e2000)
[   13.760735] 1d20:
    00000001 00000000
[   13.760735] 1d40: c02e1d5c c02e1d50 0000001f c02dcd20 00000000
00000000 c02e1d94 c02e1d68
[   13.760735] 1d60: 00000000 c0029844 c02e1da4 c02e0000 c0212aec
c001e800 c001e800 00000001
[   13.760735] 1d80: c001ec58 c3c3cc21 c02e1dac c02e1d98 c01a4b28
c01a4680 c001e800 c0212aec
[   13.760735] 1da0: c02e1dbc c02e1db0 c01a4b44 c01a4ad8 c02e1dd4
c02e1dc0 c010e298 c01a4b44
[   13.760735] 1dc0: 0000001b c3c3cc1c c02e1de4 c02e1dd8 c010e2bc
c010e288 c02e1ea4 c02e1de8
[   13.760735] 1de0: c01007e0 c010e2bc c3c3cc22 c3c3cd22 00000000
00000000 c0035d18 c00359b4
[   13.760735] 1e00: 00000001 c01f40a4 c02e1e2c c02e1e18 c0035f7c
c0035cf4 00000001 c01f40a4
[   13.760735] 1e20: c02e1e3c c02e1e30 c0036230 c02e0000 c02e1e54
c02e1e40 c01a4af8 c02e0000
[   13.760735] 1e40: c02e1e64 c02e1e50 c01a4af8 c002a4d8 c001e800
c001e92c c02e1e74 c02e1e68
[   13.760735] 1e60: c01a4b44 c01a4ad8 c02e1e8c c02e0000 c02e1e94
c02e1e80 c01a4af8 c3c3cd1c
[   13.760735] 1e80: c3c3cc1c c001e800 c3c3cc00 00000005 c001e92c
00000001 c02e1edc c02e1ea8
[   13.760735] 1ea0: c00fb38c c00ffa64 c02e1ecc c001e80c c01a4af8
c001e800 c001e92c c001e800
[   13.760735] 1ec0: 0000001b 00000001 c02e1efe 00000004 c02e1ef4
c02e1ee0 c00fb450 c00fb2a0
[   13.760735] 1ee0: c0212aec 00001a1b c02e1f2c c02e1ef8 c011188c
c00fb424 00000015 1b002da0
[   13.760735] 1f00: c02e1f24 c0212bcc 000f4240 00000000 00000004
00000000 00000020 c0209e14
[   13.760735] 1f20: c02e1f54 c02e1f30 c0035858 c01112b0 c0209c20
c0209c20 c0209c20 ffffffdf
[   13.760735] 1f40: c0209e20 c02e0000 c02e1f64 c02e1f58 c003599c
c00357a0 c02e1fd4 c02e1f68
[   13.760735] 1f60: c00363b4 c0035974 00000000 c02cb580 61741f94
656c6b73 00302f74 c003e3e4
[   13.760735] 1f80: c02dd9e0 00000002 c02e1fbc c02e1f98 c01a31b0
c0051154 00000000 c02e0000
[   13.760735] 1fa0: c0209e14 0000001f 00000000 c02e0000 c0209e14
c003623c 00000000 00000000
[   13.760735] 1fc0: 00000000 00000000 c02e1ff4 c02e1fd8 c004510c
c003624c 00000000 00000000
[   13.760735] 1fe0: 00000000 00000000 00000000 c02e1ff8 c00336d8
c00450bc 7475706e 74616420
[   13.760735] Backtrace:
[   13.760735] [<c01a4670>] (rt_spin_lock_slowlock+0x0/0x1c8) from
[<c01a4b28>] (__rt_spin_lock+0x60/0x6c)
[   13.760735] [<c01a4ac8>] (__rt_spin_lock+0x0/0x6c) from
[<c01a4b44>] (rt_spin_lock+0x10/0x14)
[   13.760735]  r5:c0212aec r4:c001e800
[   13.760735] [<c01a4b34>] (rt_spin_lock+0x0/0x14) from [<c010e298>]
(uart_start+0x20/0x34)
[   13.760735] [<c010e278>] (uart_start+0x0/0x34) from [<c010e2bc>]
(uart_flush_chars+0x10/0x14)
[   13.760735]  r5:c3c3cc1c r4:0000001b
[   13.760735] [<c010e2ac>] (uart_flush_chars+0x0/0x14) from
[<c01007e0>] (n_tty_receive_buf+0xd8c/0xe70)
[   13.760735] [<c00ffa54>] (n_tty_receive_buf+0x0/0xe70) from
[<c00fb38c>] (flush_to_ldisc+0xfc/0x184)
[   13.760735] [<c00fb290>] (flush_to_ldisc+0x0/0x184) from
[<c00fb450>] (tty_flip_buffer_push+0x3c/0x40)
[   13.760735] [<c00fb414>] (tty_flip_buffer_push+0x0/0x40) from
[<c011188c>] (atmel_tasklet_func+0x5ec/0x5fc)
[   13.760735]  r5:00001a1b r4:c0212aec
[   13.760735] [<c01112a0>] (atmel_tasklet_func+0x0/0x5fc) from
[<c0035858>] (__tasklet_action+0xc8/0x194)
[   13.760735] [<c0035790>] (__tasklet_action+0x0/0x194) from
[<c003599c>] (tasklet_action+0x38/0x40)
[   13.760735]  r8:c02e0000 r7:c0209e20 r6:ffffffdf r5:c0209c20 r4:c0209c20
[   13.760735] [<c0035964>] (tasklet_action+0x0/0x40) from
[<c00363b4>] (ksoftirqd+0x178/0x228)
[   13.760735] [<c003623c>] (ksoftirqd+0x0/0x228) from [<c004510c>]
(kthread+0x60/0x94)
[   13.760735] [<c00450ac>] (kthread+0x0/0x94) from [<c00336d8>]
(do_exit+0x0/0x7e0)
[   13.760735]  r6:00000000 r5:00000000 r4:00000000
[   13.760735] Code: e5963010 e595200c e3c33003 e1530002 (05800000)
[   14.189445] note: softirq-tasklet[9] exited with preempt_count 1
[   14.194328] BUG: sleeping function called from invalid context
softirq-tasklet(9) at kernel/rtmutex.c:637
[   14.204094] in_atomic():1 [00000001], irqs_disabled():0
[   14.208977] [<c0023d48>] (dump_stack+0x0/0x14) from [<c002a5a8>]
(__might_sleep+0xe0/0x104)
[   14.217766] [<c002a4c8>] (__might_sleep+0x0/0x104) from
[<c01a4af8>] (__rt_spin_lock+0x30/0x6c)
[   14.226555]  r4:c02e0000
[   14.228508] [<c01a4ac8>] (__rt_spin_lock+0x0/0x6c) from
[<c01a4b44>] (rt_spin_lock+0x10/0x14)
[   14.237297]  r5:c02dd5a0 r4:c02dd818
[   14.240227] [<c01a4b34>] (rt_spin_lock+0x0/0x14) from [<c00338e8>]
(do_exit+0x210/0x7e0)
[   14.249016] [<c00336d8>] (do_exit+0x0/0x7e0) from [<c0023c4c>]
(die+0x1c8/0x214)
[   14.255852] [<c0023a84>] (die+0x0/0x214) from [<c0024f38>]
(__do_kernel_fault+0x6c/0x7c)
[   14.263664] [<c0024ecc>] (__do_kernel_fault+0x0/0x7c) from
[<c0025164>] (do_page_fault+0x21c/0x238)
[   14.272453]  r7:c02e1cf0 r6:c02dd5a0 r5:c01f0370 r4:ffffffff
[   14.278313] [<c0024f48>] (do_page_fault+0x0/0x238) from
[<c001f250>] (do_DataAbort+0x3c/0xa0)
[   14.287102] [<c001f214>] (do_DataAbort+0x0/0xa0) from [<c001fa60>]
(__dabt_svc+0x40/0x60)
[   14.294914] Exception stack(0xc02e1cf0 to 0xc02e1d38)
[   14.299797] 1ce0:                                     00000000
c0212afc c02dd5a0 c02dd5a0
[   14.308586] 1d00: a0000013 c02e0000 c0212aec c001e800 00000001
c001ec58 c3c3cc21 c02e1d94
[   14.316398] 1d20: c02dd5a0 c02e1d38 c01a46c4 c01a46f4 60000093 ffffffff
[   14.324211]  r8:00000001 r7:c001e800 r6:c0212aec r5:c02e1d24 r4:ffffffff
[   14.331047] [<c01a4670>] (rt_spin_lock_slowlock+0x0/0x1c8) from
[<c01a4b28>] (__rt_spin_lock+0x60/0x6c)
[   14.340812] [<c01a4ac8>] (__rt_spin_lock+0x0/0x6c) from
[<c01a4b44>] (rt_spin_lock+0x10/0x14)
[   14.348625]  r5:c0212aec r4:c001e800
[   14.352531] [<c01a4b34>] (rt_spin_lock+0x0/0x14) from [<c010e298>]
(uart_start+0x20/0x34)
[   14.360344] [<c010e278>] (uart_start+0x0/0x34) from [<c010e2bc>]
(uart_flush_chars+0x10/0x14)
[   14.369133]  r5:c3c3cc1c r4:0000001b
[   14.372062] [<c010e2ac>] (uart_flush_chars+0x0/0x14) from
[<c01007e0>] (n_tty_receive_buf+0xd8c/0xe70)
[   14.381828] [<c00ffa54>] (n_tty_receive_buf+0x0/0xe70) from
[<c00fb38c>] (flush_to_ldisc+0xfc/0x184)
[   14.390617] [<c00fb290>] (flush_to_ldisc+0x0/0x184) from
[<c00fb450>] (tty_flip_buffer_push+0x3c/0x40)
[   14.399406] [<c00fb414>] (tty_flip_buffer_push+0x0/0x40) from
[<c011188c>] (atmel_tasklet_func+0x5ec/0x5fc)
[   14.409172]  r5:00001a1b r4:c0212aec
[   14.413078] [<c01112a0>] (atmel_tasklet_func+0x0/0x5fc) from
[<c0035858>] (__tasklet_action+0xc8/0x194)
[   14.421867] [<c0035790>] (__tasklet_action+0x0/0x194) from
[<c003599c>] (tasklet_action+0x38/0x40)
[   14.431633]  r8:c02e0000 r7:c0209e20 r6:ffffffdf r5:c0209c20 r4:c0209c20
[   14.437492] [<c0035964>] (tasklet_action+0x0/0x40) from
[<c00363b4>] (ksoftirqd+0x178/0x228)
[   14.446281] [<c003623c>] (ksoftirqd+0x0/0x228) from [<c004510c>]
(kthread+0x60/0x94)
[   14.454094] [<c00450ac>] (kthread+0x0/0x94) from [<c00336d8>]
(do_exit+0x0/0x7e0)
[   14.460930]  r6:00000000 r5:00000000 r4:00000000


2007/12/19, Haavard Skinnemoen <hskinnemoen@atmel.com>:
> The following patchset cleans up the atmel_serial driver a bit,
> moves a significant portion of the interrupt handler into a tasklet,
> and adds DMA support. This is the result of a combined effort by Chip
> Coldwell, Remy Bohmer and me. The patches should apply cleanly onto
> Linus' latest git tree.
>
> With DMA, I see transfer rates around 92 kbps when transferring a big
> file using ZModem (both directions are roughly the same.)
>
> Note that break and error handling doesn't work too well with DMA
> enabled. This is a common problem with all the efforts I've seen
> adding DMA support to this driver (including my own). The PDC error
> handling also accesses icount without locking. I'm tempted to just
> ignore the problem for now and hopefully come up with a solution
> later.
>
> Everyone, please give it a try and/or review the code.
>
> Chip Coldwell (1):
>       atmel_serial: Add DMA support
>
> Haavard Skinnemoen (3):
>       atmel_serial: Use cpu_relax() when busy-waiting
>       atmel_serial: Use existing console options only if BRG is running
>       atmel_serial: Fix bugs in probe() error path and remove()
>
> Remy Bohmer (2):
>       atmel_serial: Clean up the code
>       atmel_serial: Split the interrupt handler
>
>  drivers/serial/atmel_serial.c |  938 ++++++++++++++++++++++++++++++++---------
>  1 files changed, 738 insertions(+), 200 deletions(-)
>

Re: [PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[Haavard Skinnemoen]

On Wed, 19 Dec 2007 16:57:04 +0100
"Remy Bohmer" <linux@bohmer.net> wrote:

> Hello Haavard,
> 
> Sorry.. But I get an Oops on Preempt-RT with the latest set of
> patches. I did not see it earlier today with the other set of patches.

Hmm...from the backtrace, it looks like lock recursion -- port->lock is
held for the whole duration of the tasklet, but we somehow end up in
uart_start(), which grabs the lock again.

Could you try the patch below? It's a bit strange that you got an oops
though...

Haavard

diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
index 7967054..948c643 100644
--- a/drivers/serial/atmel_serial.c
+++ b/drivers/serial/atmel_serial.c
@@ -666,7 +666,13 @@ static void atmel_rx_from_ring(struct uart_port *port)
 		uart_insert_char(port, status, ATMEL_US_OVRE, c.ch, flg);
 	}
 
+	/*
+	 * Drop the lock here since it might end up calling
+	 * uart_start(), which takes the lock.
+	 */
+	spin_unlock(&port->lock);
 	tty_flip_buffer_push(port->info->tty);
+	spin_lock(&port->lock);
 }
 
 static void atmel_rx_from_dma(struct uart_port *port)

Re: [PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[Remy Bohmer]

Hello Haavard,

> Could you try the patch below? It's a bit strange that you got an oops
> though...

It is not really strange... spinlocks are mutexes on preempt-rt, and
recursive mutex locking is not allowed, this is one differences with
the mainline spinlock.

But... I tried that patch, and it works a lot better, no oopses
anymore, but I noticed that I sometimes get an input overrun (ttyS0: 1
input overrun(s) ) during stress conditions.
This is something I did not notice before, maybe it was already there,
or has something changed in this area that it is now more sensitive
for this?

Kind Regards,

Remy

2007/12/19, Haavard Skinnemoen <hskinnemoen@atmel.com>:
> On Wed, 19 Dec 2007 16:57:04 +0100
> "Remy Bohmer" <linux@bohmer.net> wrote:
>
> > Hello Haavard,
> >
> > Sorry.. But I get an Oops on Preempt-RT with the latest set of
> > patches. I did not see it earlier today with the other set of patches.
>
> Hmm...from the backtrace, it looks like lock recursion -- port->lock is
> held for the whole duration of the tasklet, but we somehow end up in
> uart_start(), which grabs the lock again.
>
> Could you try the patch below? It's a bit strange that you got an oops
> though...
>
> Haavard
>
> diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
> index 7967054..948c643 100644
> --- a/drivers/serial/atmel_serial.c
> +++ b/drivers/serial/atmel_serial.c
> @@ -666,7 +666,13 @@ static void atmel_rx_from_ring(struct uart_port *port)
>                 uart_insert_char(port, status, ATMEL_US_OVRE, c.ch, flg);
>         }
>
> +       /*
> +        * Drop the lock here since it might end up calling
> +        * uart_start(), which takes the lock.
> +        */
> +       spin_unlock(&port->lock);
>         tty_flip_buffer_push(port->info->tty);
> +       spin_lock(&port->lock);
>  }
>
>  static void atmel_rx_from_dma(struct uart_port *port)
>

Re: [PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[Haavard Skinnemoen]

On Wed, 19 Dec 2007 17:40:44 +0100
"Remy Bohmer" <linux@bohmer.net> wrote:

> But... I tried that patch, and it works a lot better, no oopses
> anymore,

Good :)

> but I noticed that I sometimes get an input overrun (ttyS0: 1
> input overrun(s) ) during stress conditions.
> This is something I did not notice before, maybe it was already there,
> or has something changed in this area that it is now more sensitive
> for this?

Hmm...is this with or without DMA?

If it's with DMA, perhaps we should increase the size of the RX buffer
to compensate for moving the DMA RX code into tasklet context?

If it's without DMA, something very strange is going on -- the non-DMA
RX code is almost the only thing left in the hardirq handler, so I
would really expect overruns to be much less likely to occur now than
before...

Haavard

Re: [PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[Remy Bohmer]

> > but I noticed that I sometimes get an input overrun (ttyS0: 1
> > input overrun(s) ) during stress conditions.
> > This is something I did not notice before, maybe it was already there,
> > or has something changed in this area that it is now more sensitive
> > for this?
> Hmm...is this with or without DMA?

DBGU is without DMA.

> If it's without DMA, something very strange is going on -- the non-DMA
> RX code is almost the only thing left in the hardirq handler, so I
> would really expect overruns to be much less likely to occur now than
> before...

As mentioned, maybe it was already there, but I did not run into it
earlier. I have to figure that out. But, at 115200 and a 1 byte
receive-'fifo' on DBGU, and still interrupt locks somewhere in the
tree up to 300us, it is a simple calculation that we can run into
overrun conditions...
Notice that without these interrupt handler splitup, it was much, much
much worse...

So, for me it is not a big deal, because it is just a terminal, and
with my shaky fingers I usually do not type that fast ;-))

Remy

Kind Regards,

Remy

Re: [PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[Haavard Skinnemoen]

On Wed, 19 Dec 2007 17:59:09 +0100
"Remy Bohmer" <linux@bohmer.net> wrote:

> > > but I noticed that I sometimes get an input overrun (ttyS0: 1
> > > input overrun(s) ) during stress conditions.
> > > This is something I did not notice before, maybe it was already there,
> > > or has something changed in this area that it is now more sensitive
> > > for this?
> > Hmm...is this with or without DMA?
> 
> DBGU is without DMA.

Right.

> > If it's without DMA, something very strange is going on -- the non-DMA
> > RX code is almost the only thing left in the hardirq handler, so I
> > would really expect overruns to be much less likely to occur now than
> > before...
> 
> As mentioned, maybe it was already there, but I did not run into it
> earlier. I have to figure that out. But, at 115200 and a 1 byte
> receive-'fifo' on DBGU, and still interrupt locks somewhere in the
> tree up to 300us, it is a simple calculation that we can run into
> overrun conditions...

preempt-rt can disable interrupts for 300 us?

If so, then I guess there's really no way to avoid a few overruns.

> Notice that without these interrupt handler splitup, it was much, much
> much worse...

Ok, that's good I guess.

> So, for me it is not a big deal, because it is just a terminal, and
> with my shaky fingers I usually do not type that fast ;-))

If you do, you just need to switch to one of the USARTs instead of the
DBGU so that you can use DMA :-)

We need to fix the break- and error handling though. But my vacation
starts tomorrow, so I probably won't be able to fix it until next year.

Haavard

Re: [PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[Remy Bohmer]

Hello,

> preempt-rt can disable interrupts for 300 us?

I am not sure if it really is an interrupt lock that long, but on
these ARM cores I still run into latencies that large. I tried using
latency_trace to figure out where they come from, but that tool even
reports interrupt locks up to 10 ms while those surely do not exist
(the 2nd in size reported is about 300us). So, those tools are buggy
on AT91 as well, and I do not trust them completely yet.. (They even
do not compile cleanly, conflicts in headers, warnings etc. Maybe
those tools are not used very much outside x86)

So, I still have a long way to go to make Preempt-RT to run on these
cores just as good as preempt-rt runs on X86, although I am sure that
I never will reach the <30us as I have on x86.

> If so, then I guess there's really no way to avoid a few overruns.

And if DBGU is used as a terminal, it should be no problem at all, or
someone has to slow down the baudrate a bit.
I only get this overrun under very heavy system load (CPU load 99%,
with one single thread running at RT-prio 99, taking chunks away of
99/100ms.) and then throw in a big file of many MBs in the terminal.
But, as long as the read part still runs in hard-irq context, it
should keep up, unless interrupts are locked too long.

> > Notice that without these interrupt handler splitup, it was much, much
> > much worse...
>
> Ok, that's good I guess.

Yep, that _is_ good.

> > So, for me it is not a big deal, because it is just a terminal, and
> > with my shaky fingers I usually do not type that fast ;-))
> If you do, you just need to switch to one of the USARTs instead of the
> DBGU so that you can use DMA :-)

:-)

> We need to fix the break- and error handling though. But my vacation
> starts tomorrow, so I probably won't be able to fix it until next year.

In that case, I wish you very good holiday, and a happy new-year.
It was very pleasant working with you on this driver.

I probably see you again on the list next year.


Kind Regards,

Remy

Re: [PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[Haavard Skinnemoen]

On Wed, 19 Dec 2007 21:32:44 +0100
"Remy Bohmer" <linux@bohmer.net> wrote:

> > We need to fix the break- and error handling though. But my vacation
> > starts tomorrow, so I probably won't be able to fix it until next
> > year.  
> 
> In that case, I wish you very good holiday, and a happy new-year.
> It was very pleasant working with you on this driver.

Thanks. It was a pleasure working with you too :-)

> I probably see you again on the list next year.

I think you can count on that. The oops fix needs to be folded into the
patchset somehow, so I'll probably send out a new series before it's
ready for merging (unless someone beats me to it.) We should probably
check if the DMA RX path needs the same treatment as well.

Haavard

Re: [PATCH v2 0/6] atmel_serial: Cleanups, irq handler splitup & DMA

[Ulf Samuelsson]

| The following patchset cleans up the atmel_serial driver a bit,
| moves a significant portion of the interrupt handler into a tasklet,
| and adds DMA support. This is the result of a combined effort by Chip
| Coldwell, Remy Bohmer and me. The patches should apply cleanly onto
| Linus' latest git tree.
| 
| With DMA, I see transfer rates around 92 kbps when transferring a big
| file using ZModem (both directions are roughly the same.)
| 

The start and stop bits will use 20 % of the bit rate so the
teoretical max throughput at 115,200 BAUD is 115,200 * 0,8 = 92160 bps.

What would be interesting is to figure out if we can get reliable
transmission at higher frequencies - 230/460/921 kbps.

| Note that break and error handling doesn't work too well with DMA
| enabled. This is a common problem with all the efforts I've seen
| adding DMA support to this driver (including my own). The PDC error
| handling also accesses icount without locking. I'm tempted to just
| ignore the problem for now and hopefully come up with a solution
| later.

Have told the guys responsible for the USART IP block we
need to improve the H/W for error control for the last 3-4 years.
Why not at 120000 gates/sq mm, add error counters???

Best Regards
Ulf Samuelsson