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
This patch cleans up the atmel_serial driver to conform the coding rules.
It contains no functional change.

Signed-off-by: Remy Bohmer <[EMAIL PROTECTED]>
---
 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);
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 <[EMAIL PROTECTED]>
---
 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;
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 <[EMAIL PROTECTED]>
---
 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 */

Reply via email to