Re: [PATCH]: Atmel Serial Console interrupt handler splitup

["Remy Bohmer" <linux@bohmer.net>]

[-- Attachment #1: Type: text/plain, Size: 1883 bytes --]

Hello Andrew,

> > * atmel_serial_cleanup_no_at91.patch
> I don't know what coding rules you're following...   :)

Funny... I intended to follow these rules: Documentation/CodingStyle
I know I should not do it, but I cannot resist pointing you these
rules (for the fun, and because you ask me to... :-)))
----------------------------------
Chapter 6: Functions
Functions should be short and sweet, and do just one thing.  They should
fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
as we all know), and do one thing and do that well.
----------------------------------
For now it is not absolutely necessary, but the routine become quite
long when the DMA patch comes in.

AND:
----------------------------------
Chapter 15: The inline disease
A reasonable rule of thumb is to not put inline at functions that have more
than 3 lines of code in them.
+ Often people argue that adding inline to functions that are static and used
only once is always a win since there is no space tradeoff. While this is
technically correct, ***gcc is capable of inlining these automatically without
help***, and the maintenance issue of removing the inline when a second user
appears outweighs the potential value of the hint that tells gcc to do
something it would have done anyway.
----------------------------------

> The code you've moved out of the interrupt handler should probably now
> be marked as inline.

Because I know it is common practice in the kernel, I attached 3 new
patches to inline these :-))
To begin with, I made the cleanup patch separate, because otherwise it
would make the split-up quite unclear to follow, and
scripts/checkpatch.pl generated a huge list of violations on this
file.

BTW: I am currently generating a re-diff for the DMA patch so that it
can be applied on top of this set to make this patch queue easier to
handle.


Regards,

Remy

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: atmel_serial_cleanup_no_at91.patch --]
[-- Type: text/x-patch; name=atmel_serial_cleanup_no_at91.patch, Size: 22910 bytes --]

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>
---
 drivers/serial/atmel_serial.c |  351 +++++++++++++++++++++++-------------------
 1 file changed, 199 insertions(+), 152 deletions(-)

Index: linux-2.6.23.1-rt5/drivers/serial/atmel_serial.c
===================================================================
--- linux-2.6.23.1-rt5.orig/drivers/serial/atmel_serial.c	2007-10-09 22:31:38.000000000 +0200
+++ linux-2.6.23.1-rt5/drivers/serial/atmel_serial.c	2007-12-13 20:59:16.000000000 +0100
@@ -74,47 +74,51 @@
 
 #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 lread(port)		__raw_readl(port)
+#define lwrite(v, port)		__raw_writel(v, port)
 
-// #define UART_GET_CR(port)	__raw_readl((port)->membase + ATMEL_US_CR)		// is write-only
+#define UART_PUT_CR(port, v)	lwrite(v, (port)->membase + ATMEL_US_CR)
+#define UART_PUT_MR(port, v)	lwrite(v, (port)->membase + ATMEL_US_MR)
+#define UART_PUT_IER(port, v)	lwrite(v, (port)->membase + ATMEL_US_IER)
+#define UART_PUT_IDR(port, v)	lwrite(v, (port)->membase + ATMEL_US_IDR)
+#define UART_PUT_CHAR(port, v)	lwrite(v, (port)->membase + ATMEL_US_THR)
+#define UART_PUT_BRGR(port, v)	lwrite(v, (port)->membase + ATMEL_US_BRGR)
+#define UART_PUT_RTOR(port, v)	lwrite(v, (port)->membase + ATMEL_US_RTOR)
+
+#define UART_GET_MR(port)	lread((port)->membase + ATMEL_US_MR)
+#define UART_GET_IMR(port)	lread((port)->membase + ATMEL_US_IMR)
+#define UART_GET_CSR(port)	lread((port)->membase + ATMEL_US_CSR)
+#define UART_GET_CHAR(port)	lread((port)->membase + ATMEL_US_RHR)
+#define UART_GET_BRGR(port)	lread((port)->membase + ATMEL_US_BRGR)
+
+/* is write-only */
+/* #define UART_GET_CR(port)    lread((port)->membase + 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_PTCR(port, v)	lwrite(v, (port)->membase + ATMEL_PDC_PTCR)
+#define UART_PUT_RPR(port, v)	lwrite(v, (port)->membase + ATMEL_PDC_RPR)
+#define UART_PUT_RCR(port, v)	lwrite(v, (port)->membase + ATMEL_PDC_RCR)
+#define UART_PUT_RNPR(port, v)	lwrite(v, (port)->membase + ATMEL_PDC_RNPR)
+#define UART_PUT_RNCR(port, v)	lwrite(v, (port)->membase + ATMEL_PDC_RNCR)
+#define UART_PUT_TPR(port, v)	lwrite(v, (port)->membase + ATMEL_PDC_TPR)
+#define UART_PUT_TCR(port, v)	lwrite(v, (port)->membase + ATMEL_PDC_TCR)
+/*#define UART_PUT_TNPR(port, v) lwrite(v, (port)->membase + ATMEL_PDC_TNPR) */
+/*#define UART_PUT_TNCR(port, v) lwrite(v, (port)->membase + ATMEL_PDC_TNCR) */
 
-#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)
+#define UART_GET_PTSR(port)	lread((port)->membase + ATMEL_PDC_PTSR)
+#define UART_GET_RPR(port)	lread((port)->membase + ATMEL_PDC_RPR)
 
-static int (*atmel_open_hook)(struct uart_port *);
-static void (*atmel_close_hook)(struct uart_port *);
+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.
  */
 struct atmel_uart_port {
-	struct uart_port	uart;		/* uart */
-	struct clk		*clk;		/* uart clock */
-	unsigned short		suspended;	/* is port suspended? */
-	int			break_active;	/* break being received */
+	struct uart_port 	uart;		/* uart */
+	struct clk 	 	*clk;		/* uart clock */
+	unsigned short 		suspended;	/* is port suspended? */
+	int 			break_active;	/* break being received */
 };
 
 static struct atmel_uart_port atmel_ports[ATMEL_MAX_UART];
@@ -142,8 +146,8 @@ static void atmel_set_mctrl(struct uart_
 #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)
@@ -204,8 +208,6 @@ static u_int atmel_get_mctrl(struct uart
  */
 static void atmel_stop_tx(struct uart_port *port)
 {
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
-
 	UART_PUT_IDR(port, ATMEL_US_TXRDY);
 }
 
@@ -214,8 +216,6 @@ static void atmel_stop_tx(struct uart_po
  */
 static void atmel_start_tx(struct uart_port *port)
 {
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
-
 	UART_PUT_IER(port, ATMEL_US_TXRDY);
 }
 
@@ -224,8 +224,6 @@ static void atmel_start_tx(struct uart_p
  */
 static void atmel_stop_rx(struct uart_port *port)
 {
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
-
 	UART_PUT_IDR(port, ATMEL_US_RXRDY);
 }
 
@@ -234,7 +232,9 @@ static void atmel_stop_rx(struct uart_po
  */
 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);
 }
 
 /*
@@ -253,7 +253,7 @@ static void atmel_break_ctl(struct uart_
  */
 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;
 
@@ -272,10 +272,12 @@ static void atmel_rx_chars(struct uart_p
 		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);
@@ -315,7 +317,7 @@ static void atmel_rx_chars(struct uart_p
 
 		uart_insert_char(port, status, ATMEL_US_OVRE, ch, flg);
 
-	ignore_char:
+ignore_char:
 		status = UART_GET_CSR(port);
 	}
 
@@ -356,44 +358,75 @@ static void atmel_tx_chars(struct uart_p
 }
 
 /*
+ * receive interrupt handler.
+ */
+static inline 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 inline 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 inline 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;
@@ -409,7 +442,6 @@ static irqreturn_t atmel_interrupt(int i
  */
 static int atmel_startup(struct uart_port *port)
 {
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
 	int retval;
 
 	/*
@@ -422,7 +454,9 @@ static int atmel_startup(struct uart_por
 	/*
 	 * 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;
@@ -444,9 +478,11 @@ static int atmel_startup(struct uart_por
 	 * 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;
 }
@@ -456,8 +492,6 @@ static int atmel_startup(struct uart_por
  */
 static void atmel_shutdown(struct uart_port *port)
 {
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
-
 	/*
 	 * Disable all interrupts, port and break condition.
 	 */
@@ -480,45 +514,49 @@ static void atmel_shutdown(struct uart_p
 /*
  * 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;
 	}
@@ -545,18 +583,17 @@ static void atmel_set_termios(struct uar
 
 	/* 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);
@@ -582,16 +619,16 @@ static void atmel_set_termios(struct uar
 		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(); }
+	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();
 
 	/* disable receiver and transmitter */
 	UART_PUT_CR(port, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
@@ -717,7 +754,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;
@@ -740,7 +778,8 @@ static void __devinit atmel_init_port(st
 		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);
@@ -764,7 +803,6 @@ void __init atmel_register_uart_fns(stru
 	atmel_pops.set_wake	= fns->set_wake;
 }
 
-
 #ifdef CONFIG_SERIAL_ATMEL_CONSOLE
 static void atmel_console_putchar(struct uart_port *port, int ch)
 {
@@ -782,7 +820,7 @@ static void atmel_console_write(struct c
 	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 */
 	UART_PUT_IDR(port, ATMEL_US_RXRDY | ATMEL_US_TXRDY);
@@ -790,30 +828,32 @@ static void atmel_console_write(struct c
 	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;
 
-// TODO: CR is a write-only register
-//	unsigned int cr;
+/* 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 */
-//	}
+//      cr = UART_GET_CR(port) & (ATMEL_US_RXEN | ATMEL_US_TXEN);
+//      if (cr == (ATMEL_US_RXEN | ATMEL_US_TXEN)) {
+//              / * ok, the port was enabled * /
+//      }*/
 
 	mr = UART_GET_MR(port) & ATMEL_US_CHRL;
 	if (mr == ATMEL_US_CHRL_8)
@@ -845,10 +885,10 @@ static int __init atmel_console_setup(st
 	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);	/* disable interrupts */
 	UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
 	UART_PUT_CR(port, ATMEL_US_TXEN | ATMEL_US_RXEN);
 
@@ -863,13 +903,13 @@ static int __init atmel_console_setup(st
 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
@@ -880,13 +920,17 @@ 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);
 
 /*
@@ -894,11 +938,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
@@ -906,22 +952,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);
@@ -934,13 +982,12 @@ static int atmel_serial_suspend(struct p
 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;
@@ -970,7 +1017,7 @@ static int __devinit atmel_serial_probe(
 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);

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: atmel_serial_irq_splitup_no_at91.patch --]
[-- Type: text/x-patch; name=atmel_serial_irq_splitup_no_at91.patch, Size: 7589 bytes --]

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

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.

2 tasklets are used:
* one for handling the error statuses
* one for pushing the incoming characters into the tty-layer.

The Transmit path was IRQF_NODELAY safe by itself, and is not adapted.
The read path for DMA(PDC) is also not adapted, because that code
does not run in IRQF_NODELAY context due to irq-sharing. The DBGU 
which is shared with the timer-irq does not work with DMA, so 
therefor this is no problem.

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

This patch demands the following patches to be installed first:
* atmel_serial_cleanup.patch

Signed-off-by: Remy Bohmer <linux@bohmer.net>
---
 drivers/serial/atmel_serial.c |  150 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 133 insertions(+), 17 deletions(-)

Index: linux-2.6.23/drivers/serial/atmel_serial.c
===================================================================
--- linux-2.6.23.orig/drivers/serial/atmel_serial.c	2007-12-13 17:30:48.000000000 +0100
+++ linux-2.6.23/drivers/serial/atmel_serial.c	2007-12-13 17:32:12.000000000 +0100
@@ -111,6 +111,22 @@
 static int (*atmel_open_hook) (struct uart_port *);
 static void (*atmel_close_hook) (struct uart_port *);
 
+struct atmel_uart_char {
+	unsigned int status;
+	unsigned int overrun;
+	unsigned int ch;
+	unsigned int flg;
+};
+
+#define ATMEL_SERIAL_RINGSIZE 1024
+
+struct atmel_uart_ring {
+	unsigned int          head;
+	unsigned int          tail;
+	unsigned int          count;
+	struct atmel_uart_char data[ATMEL_SERIAL_RINGSIZE];
+};
+
 /*
  * We wrap our port structure around the generic uart_port.
  */
@@ -119,6 +135,13 @@ struct atmel_uart_port {
 	struct clk 	 	*clk;		/* uart clock */
 	unsigned short 		suspended;	/* is port suspended? */
 	int 			break_active;	/* break being received */
+
+	struct tasklet_struct   rx_task;
+	struct tasklet_struct   status_task;
+	unsigned int 		irq_pending;
+	unsigned int 		irq_status;
+
+	struct atmel_uart_ring  rx_ring;
 };
 
 static struct atmel_uart_port atmel_ports[ATMEL_MAX_UART];
@@ -249,12 +272,41 @@ static void atmel_break_ctl(struct uart_
 }
 
 /*
+ * Stores the incoming character in the ring buffer
+ */
+static void
+atmel_buffer_rx_char(struct uart_port *port, unsigned int status,
+		     unsigned int overrun,   unsigned int ch,
+		     unsigned int flg)
+{
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+	struct atmel_uart_ring *ring = &atmel_port->rx_ring;
+	struct atmel_uart_char *c;
+
+	spin_lock(&port->lock);
+
+	if (ring->count == ATMEL_SERIAL_RINGSIZE)
+		goto out; /* Buffer overflow, ignore char */
+
+	c = &ring->data[ring->head];
+	c->status  = status;
+	c->overrun = overrun;
+	c->ch      = ch;
+	c->flg     = flg;
+
+	ring->head++;
+	ring->head %= ATMEL_SERIAL_RINGSIZE;
+	ring->count++;
+out:
+	spin_unlock(&port->lock);
+}
+
+/*
  * 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;
 
 	status = UART_GET_CSR(port);
@@ -315,13 +367,13 @@ static void atmel_rx_chars(struct uart_p
 		if (uart_handle_sysrq_char(port, ch))
 			goto ignore_char;
 
-		uart_insert_char(port, status, ATMEL_US_OVRE, ch, flg);
+		atmel_buffer_rx_char(port, status, ATMEL_US_OVRE, ch, flg);
 
 ignore_char:
 		status = UART_GET_CSR(port);
 	}
 
-	tty_flip_buffer_push(tty);
+	tasklet_schedule(&atmel_port->rx_task);
 }
 
 /*
@@ -380,7 +432,7 @@ static void atmel_handle_receive(struct 
 }
 
 /*
- * transmit interrupt handler.
+ * transmit interrupt handler. (Transmit is IRQF_NODELAY safe)
  */
 static void atmel_handle_transmit(struct uart_port *port, unsigned int pending)
 {
@@ -395,19 +447,16 @@ static void atmel_handle_transmit(struct
 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);
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+
+	spin_lock(&port->lock);
+
+	atmel_port->irq_pending = pending;
+	atmel_port->irq_status  = status;
+
+	spin_unlock(&port->lock);
+
+	tasklet_schedule(&atmel_port->status_task);
 }
 
 /*
@@ -435,6 +484,66 @@ static irqreturn_t atmel_interrupt(int i
 }
 
 /*
+ * tasklet handling tty stuff outside the interrupt handler.
+ */
+static void atmel_rx_handler_task(unsigned long data)
+{
+	struct uart_port *port = (struct uart_port *)data;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+	struct atmel_uart_ring *ring = &atmel_port->rx_ring;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	while (ring->count) {
+		struct atmel_uart_char c = ring->data[ring->tail];
+
+		ring->tail++;
+		ring->tail %= ATMEL_SERIAL_RINGSIZE;
+		ring->count--;
+
+		spin_unlock_irqrestore(&port->lock, flags);
+
+		uart_insert_char(port, c.status, c.overrun, c.ch, c.flg);
+
+		spin_lock_irqsave(&port->lock, flags);
+	}
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	tty_flip_buffer_push(port->info->tty);
+}
+
+static void atmel_status_handler_task(unsigned long data)
+{
+	struct uart_port *port = (struct uart_port *)data;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
+	unsigned long flags;
+	unsigned int pending, status;
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	pending = atmel_port->irq_pending;
+	status = atmel_port->irq_status;
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	/* 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);
+}
+
+/*
  * Perform initialization and enable port for reception
  */
 static int atmel_startup(struct uart_port *port)
@@ -767,6 +876,13 @@ static void __devinit atmel_init_port(st
 	port->mapbase	= pdev->resource[0].start;
 	port->irq	= pdev->resource[1].start;
 
+	tasklet_init(&atmel_port->rx_task, atmel_rx_handler_task,
+			(unsigned long)port);
+	tasklet_init(&atmel_port->status_task, atmel_status_handler_task,
+			(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;

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: atmel_serial_irqf_nodelay.patch --]
[-- Type: text/x-patch; name=atmel_serial_irqf_nodelay.patch, Size: 1134 bytes --]

On PREEMPT-RT we may not block on a normal spinlock in IRQ/IRQF_NODELAY-context.
This patch fixes this by making the lock a raw_spinlock_t for the
Atmel serial console.

This patch demands the following patches to be installed first:
* atmel_serial_cleanup.patch
* atmel_serial_irq_splitup.patch

Signed-off-by: Remy Bohmer <linux@bohmer.net>
---
 include/linux/serial_core.h |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: linux-2.6.23/include/linux/serial_core.h
===================================================================
--- linux-2.6.23.orig/include/linux/serial_core.h	2007-12-13 13:31:27.000000000 +0100
+++ linux-2.6.23/include/linux/serial_core.h	2007-12-13 16:32:22.000000000 +0100
@@ -226,7 +226,12 @@ struct uart_icount {
 typedef unsigned int __bitwise__ upf_t;
 
 struct uart_port {
-	spinlock_t		lock;			/* port lock */
+
+#ifdef CONFIG_SERIAL_ATMEL
+	raw_spinlock_t 		lock;			/* port lock */
+#else
+	spinlock_t 		lock;			/* port lock */
+#endif
 	unsigned int		iobase;			/* in/out[bwl] */
 	unsigned char __iomem	*membase;		/* read/write[bwl] */
 	unsigned int		irq;			/* irq number */