[PATCH]: Atmel Serial Console interrupt handler splitup

[PATCH]: Atmel Serial Console interrupt handler splitup

[Remy Bohmer]

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

Hello Andrew,

Attached I have put 3 patches for the AT91-series for the serial port
driver on AT91.
The interrupt handler of the serial console is quite long, and if
Preempt-RT is used on AT91 the interrupt handler is even doing illegal
things. These illegal things are related to the DBGU port of which the
interrupt handler is shared with the system_interrupt which
automagically runs in IRQF_NODELAY context due to the timer interrupt.
The old interrupt handler calls some kernel interfaces that can block
on a mutex.

To solve this, I have split up the interrupt handler in a IRQF_NODELAY
safe/unsafe part, with as result that the code running in real
interrupt context is shortened a lot (the tty driver part is now
called from a tasklet). On AT91 David Brownell noticed several months
ago that the DBGU can miss some characters on NO_HZ. I would expect
that this would be better now due to the shorter interrupt handler,
although it was not my goal to solve it with these patches. (David can
you verify if this is better now?)

So, I have here 3 patches:
* atmel_serial_cleanup -> This patch adapts the driver to the coding
rules, splits the interrupt handler into 3 routines (cuts the routine
in more readable pieces) , but there is no functional change involved.
* atmel_serial_irq_splitup -> This patch splits up the interrupt handler.
* atmel_serial_irqf_nodelay -> This patch is additionally required to
get it properly working on Preempt-RT. (This patch should thus go into
the RT-patch, AFTER integration of the other 2 patches into mainline)

BUT: I based the patch on the 2.6.23.1 + your patch collection at
http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz

I did this because this driver conflicts with the DMA(PDC) patches
that are in the patchset on  maxim.org.za. I found out that these DMA
patches are still not integrated into mainline, although it is in your
patchset for several kernel releases. I can make a series based on
kernel mainline, but that would harden the integration for you in your
patchset.
The patchset itself is not dependant on the DMA changes, so I can
split it up, but the DMA changes itself are quite heavy.

But there is also a relation with Preempt-RT. To get the patch in
preempt RT the other patches has to be in mainline, so things are
stacking up now.

What is wise here? should I create a new patchset for mainline? Or can
you push the DMA patch also to mainline together with this set? I have
it working here for months, so I see no reason not to, maybe you have
a reason?

I tested it on AT91rm9200-EK (+proprietary boards) + AT91SAM9261-EK,
in combination with: 2.6.23.1 and 2.6.23.1-rt5 up to serial speed to
115200 (also with 99% CPU load on prio 99 on RT, no missing characters
detected.)

Note: Preempt-RT CANNOT run without these patches on AT91.


Kind Regards,

Remy Bohmer

[-- Attachment #2: patch_atmel_serial_cleanup --]
[-- Type: application/octet-stream, Size: 31627 bytes --]

This patch cleans up the atmel_serial driver to conform the coding rules.

This patch demands the patch http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz
to be installed first.

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

Index: linux-2.6.23/drivers/serial/atmel_serial.c
===================================================================
--- linux-2.6.23.orig/drivers/serial/atmel_serial.c	2007-12-05 14:48:28.000000000 +0100
+++ linux-2.6.23/drivers/serial/atmel_serial.c	2007-12-05 15:08:24.000000000 +0100
@@ -53,7 +53,7 @@
 #define SUPPORT_PDC
 #define PDC_BUFFER_SIZE		(L1_CACHE_BYTES << 3)
 #warning "Revisit"
-#define PDC_RX_TIMEOUT		(3 * 10)		/* 3 bytes */
+#define PDC_RX_TIMEOUT		(3 * 10)	/* 3 bytes */
 
 #if defined(CONFIG_SERIAL_ATMEL_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
 #define SUPPORT_SYSRQ
@@ -82,44 +82,50 @@
 
 #define ATMEL_ISR_PASS_LIMIT	256
 
-#define UART_PUT_CR(port,v)	__raw_writel(v, (port)->membase + ATMEL_US_CR)
+#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_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_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_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
+/* write-only: */
+/* #define UART_GET_CR(port)    __raw_readl((port)->membase + ATMEL_US_CR) */
 
  /* PDC registers */
-#define UART_PUT_PTCR(port,v)	__raw_writel(v, (port)->membase + ATMEL_PDC_PTCR)
+#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_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_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 *);
+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;
+	unsigned char   *buf;
+	dma_addr_t      dma_addr;
+	size_t          dma_size;
+	unsigned        int ofs;
 };
 
 /*
@@ -127,16 +133,16 @@ struct atmel_dma_buffer {
  */
 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 */
-
-	short			use_dma_rx;	/* enable PDC receiver */
-	short			pdc_rx_idx;	/* current PDC RX buffer */
-	struct atmel_dma_buffer	pdc_rx[2];	/* PDC receier */
+	struct clk 		*clk;		/* uart clock */
+	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 receiver */
 
-	short			use_dma_tx;	/* enable PDC transmitter */
-	struct atmel_dma_buffer	pdc_tx;		/* PDC transmitter */
+	short 			use_dma_tx;	/* enable PDC transmitter */
+	struct atmel_dma_buffer pdc_tx;		/* PDC transmitter */
 };
 
 static struct atmel_uart_port atmel_ports[ATMEL_MAX_UART];
@@ -167,8 +173,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)
@@ -229,13 +235,13 @@ 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;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
 	if (atmel_port->use_dma_tx) {
-		UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS);		/* disable PDC transmit */
+		/* disable PDC transmit */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS);
 		UART_PUT_IDR(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE);
-	}
-	else
+	} else
 		UART_PUT_IDR(port, ATMEL_US_TXRDY);
 }
 
@@ -244,18 +250,18 @@ 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;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
 	if (atmel_port->use_dma_tx) {
 		if (UART_GET_PTSR(port) & ATMEL_PDC_TXTEN)
 			/* The transmitter is already running.  Yes, we
-			   really need this.*/
+			   really need this. */
 			return;
 
 		UART_PUT_IER(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE);
-		UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);		/* re-enable PDC transmit */
-	}
-	else
+		/* re-enable PDC transmit */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);
+	} else
 		UART_PUT_IER(port, ATMEL_US_TXRDY);
 }
 
@@ -264,13 +270,13 @@ 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;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
 	if (atmel_port->use_dma_rx) {
-		UART_PUT_PTCR(port, ATMEL_PDC_RXTDIS);		/* disable PDC receive */
+		/* disable PDC receive */
+		UART_PUT_PTCR(port, ATMEL_PDC_RXTDIS);
 		UART_PUT_IDR(port, ATMEL_US_ENDRX | ATMEL_US_TIMEOUT);
-	}
-	else
+	} else
 		UART_PUT_IDR(port, ATMEL_US_RXRDY);
 }
 
@@ -279,7 +285,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);
 }
 
 /*
@@ -298,14 +306,15 @@ static void atmel_break_ctl(struct uart_
  */
 static void atmel_pdc_endrx(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;
 	struct atmel_dma_buffer *pdc = PDC_RX_BUF(atmel_port);
 	unsigned int count;
 
 	count = pdc->dma_size - pdc->ofs;
 	if (likely(count > 0)) {
-		dma_sync_single_for_cpu(port->dev, pdc->dma_addr, pdc->dma_size, DMA_FROM_DEVICE);
+		dma_sync_single_for_cpu(port->dev, pdc->dma_addr,
+					pdc->dma_size, DMA_FROM_DEVICE);
 		tty_insert_flip_string(tty, pdc->buf + pdc->ofs, count);
 		tty_flip_buffer_push(tty);
 
@@ -318,7 +327,7 @@ static void atmel_pdc_endrx(struct uart_
 	UART_PUT_RNCR(port, pdc->dma_size);
 
 	/* Switch to next buffer */
-	PDC_RX_SWITCH(atmel_port);		/* next PDC buffer */
+	PDC_RX_SWITCH(atmel_port);	/* next PDC buffer */
 }
 
 /*
@@ -327,16 +336,17 @@ static void atmel_pdc_endrx(struct uart_
  */
 static void atmel_pdc_timeout(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;
 	struct atmel_dma_buffer *pdc = PDC_RX_BUF(atmel_port);
 	/* unsigned */ int ofs, count;
 
-	ofs = UART_GET_RPR(port) - pdc->dma_addr;	/* current DMA adress */
+	ofs = UART_GET_RPR(port) - pdc->dma_addr; /* current DMA adress */
 	count = ofs - pdc->ofs;
 
 	if (likely(count > 0)) {
-		dma_sync_single_for_cpu(port->dev, pdc->dma_addr, pdc->dma_size, DMA_FROM_DEVICE);
+		dma_sync_single_for_cpu(port->dev, pdc->dma_addr, pdc->dma_size,
+					DMA_FROM_DEVICE);
 		tty_insert_flip_string(tty, pdc->buf + pdc->ofs, count);
 		tty_flip_buffer_push(tty);
 
@@ -357,7 +367,8 @@ static void atmel_pdc_rxerr(struct uart_
 	UART_PUT_CR(port, ATMEL_US_RSTSTA);
 
 	if (status & ATMEL_US_RXBRK) {
-		status &= ~(ATMEL_US_PARE | ATMEL_US_FRAME);	/* ignore side-effect */
+		/* ignore side-effect */
+		status &= ~(ATMEL_US_PARE | ATMEL_US_FRAME);
 		port->icount.brk++;
 	}
 	if (status & ATMEL_US_PARE)
@@ -373,7 +384,7 @@ static void atmel_pdc_rxerr(struct uart_
  */
 static void atmel_pdc_endtx(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 circ_buf *xmit = &port->info->xmit;
 	struct atmel_dma_buffer *pdc = &atmel_port->pdc_tx;
 
@@ -394,15 +405,16 @@ static void atmel_pdc_endtx(struct uart_
  */
 static void atmel_pdc_txbufe(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 circ_buf *xmit = &port->info->xmit;
 	struct atmel_dma_buffer *pdc = &atmel_port->pdc_tx;
 	int count;
 
 	if (!uart_circ_empty(xmit)) {
 		/* more to transmit - setup next transfer */
-		UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS);			/* disable PDC transmit */
-		dma_sync_single_for_device(port->dev, pdc->dma_addr, pdc->dma_size, DMA_TO_DEVICE);
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS);/* disable PDC transmit */
+		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;
@@ -412,11 +424,11 @@ static void atmel_pdc_txbufe(struct uart
 
 		UART_PUT_TPR(port, pdc->dma_addr + xmit->tail);
 		UART_PUT_TCR(port, count);
-		UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);			/* re-enable PDC transmit */
-	}
-	else {
+		/* re-enable PDC transmit */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);
+	} else {
 		/* nothing left to transmit - disable the transmitter */
-		UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS);			/* disable PDC transmit */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS);/* disable PDC transmit */
 		UART_PUT_IDR(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE);
 	}
 }
@@ -426,7 +438,7 @@ static void atmel_pdc_txbufe(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;
 
@@ -445,10 +457,11 @@ 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 */
+			UART_PUT_CR(port, ATMEL_US_RSTSTA); /* clear error */
 			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);
@@ -529,58 +542,88 @@ static void atmel_tx_chars(struct uart_p
 }
 
 /*
+ * 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;
+
+	/* PDC receive */
+	if (pending & ATMEL_US_ENDRX)
+		atmel_pdc_endrx(port);
+	if (pending & ATMEL_US_TIMEOUT)
+		atmel_pdc_timeout(port);
+	if (atmel_port->use_dma_rx
+	    && 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);
+	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)
+{
+	/* PDC transmit */
+	if (pending & ATMEL_US_ENDTX)
+		atmel_pdc_endtx(port);
+	if (pending & ATMEL_US_TXBUFE)
+		atmel_pdc_txbufe(port);
+
+	/* 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) {
-		/* PDC receive */
-		if (pending & ATMEL_US_ENDRX)
-			atmel_pdc_endrx(port);
-		if (pending & ATMEL_US_TIMEOUT)
-			atmel_pdc_timeout(port);
-		if (atmel_port->use_dma_rx && 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);
-		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);
-
-		/* PDC transmit */
-		if (pending & ATMEL_US_ENDTX)
-			atmel_pdc_endtx(port);
-		if (pending & ATMEL_US_TXBUFE)
-			atmel_pdc_txbufe(port);
-
-		/* 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;
@@ -596,7 +639,7 @@ 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;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 	int retval;
 
 	/*
@@ -609,7 +652,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;
@@ -627,13 +672,19 @@ static int atmel_startup(struct uart_por
 			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);
+					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_addr =
+			    dma_map_single(port->dev, pdc->buf, PDC_BUFFER_SIZE,
+					   DMA_FROM_DEVICE);
 			pdc->dma_size = PDC_BUFFER_SIZE;
 			pdc->ofs = 0;
 		}
@@ -651,7 +702,9 @@ static int atmel_startup(struct uart_por
 		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_addr =
+		    dma_map_single(port->dev, pdc->buf, SERIAL_XMIT_SIZE,
+				   DMA_TO_DEVICE);
 		pdc->dma_size = SERIAL_XMIT_SIZE;
 		pdc->ofs = 0;
 	}
@@ -672,18 +725,21 @@ 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);
 
 	if (atmel_port->use_dma_rx) {
-		UART_PUT_RTOR(port, PDC_RX_TIMEOUT);		/* set UART timeout */
+		/* 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);
-		UART_PUT_PTCR(port, ATMEL_PDC_RXTEN);		/* enable PDC controller */
+		/* enable PDC controller */
+		UART_PUT_PTCR(port, ATMEL_PDC_RXTEN);
+	} else {
+		/* enable receive only */
+		UART_PUT_IER(port, ATMEL_US_RXRDY);
 	}
-	else
-		UART_PUT_IER(port, ATMEL_US_RXRDY);		/* enable receive only */
-
 	return 0;
 }
 
@@ -692,7 +748,7 @@ 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;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
 	/*
 	 * Ensure everything is stopped.
@@ -709,14 +765,16 @@ static void atmel_shutdown(struct uart_p
 		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);
+			dma_unmap_single(port->dev, pdc->dma_addr,
+					 pdc->dma_size, DMA_FROM_DEVICE);
 			kfree(pdc->buf);
 		}
 	}
 	if (atmel_port->use_dma_tx) {
 		struct atmel_dma_buffer *pdc = &atmel_port->pdc_tx;
 
-		dma_unmap_single(port->dev, pdc->dma_addr, pdc->dma_size, DMA_TO_DEVICE);
+		dma_unmap_single(port->dev, pdc->dma_addr, pdc->dma_size,
+				 DMA_TO_DEVICE);
 	}
 
 	/*
@@ -741,43 +799,47 @@ 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)
 {
-	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 	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 */
@@ -807,18 +869,16 @@ static void atmel_set_termios(struct uar
 
 	/* parity */
 	if (termios->c_cflag & PARENB) {
-		if (termios->c_cflag & CMSPAR) {			/* Mark or Space parity */
+		if (termios->c_cflag & CMSPAR) { /* Mark or Space parity */
 			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);
@@ -847,16 +907,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(); }
+	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);
@@ -982,7 +1042,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;
@@ -1001,16 +1062,16 @@ static void __devinit atmel_init_port(st
 		/* Already mapped by setup code */
 		port->membase = data->regs;
 	else {
-		port->flags	|= UPF_IOREMAP;
-		port->membase	= NULL;
+		port->flags |= UPF_IOREMAP;
+		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);
 	}
-
 #ifdef SUPPORT_PDC
 	atmel_port->use_dma_rx = data->use_dma_rx;
 	atmel_port->use_dma_tx = data->use_dma_tx;
@@ -1036,7 +1097,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)
 {
@@ -1054,7 +1114,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);
@@ -1062,30 +1122,31 @@ 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 */
+	UART_PUT_IER(port, imr); /* set interrupts back the way they were */
 }
 
 /*
- * 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)
@@ -1117,10 +1178,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);
 
@@ -1152,13 +1213,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);
 
 /*
@@ -1166,11 +1231,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
@@ -1178,23 +1245,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)
-			&& !clk_must_disable(atmel_port->clk))
+	    && !clk_must_disable(atmel_port->clk))
 		enable_irq_wake(port->irq);
 	else {
 		uart_suspend_port(&atmel_uart, port);
@@ -1207,13 +1275,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;
@@ -1243,7 +1310,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);
@@ -1266,9 +1333,9 @@ static struct platform_driver atmel_seri
 	.suspend	= atmel_serial_suspend,
 	.resume		= atmel_serial_resume,
 	.driver		= {
-		.name	= "atmel_usart",
-		.owner	= THIS_MODULE,
-	},
+			  .name = "atmel_usart",
+			  .owner = THIS_MODULE,
+			  },
 };
 
 static int __init atmel_serial_init(void)

[-- Attachment #3: patch_atmel_serial_irq_splitup --]
[-- Type: application/octet-stream, Size: 8012 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:
* http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz
* atmel_serial_cleanup.patch

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

Index: linux-2.6.23/drivers/serial/atmel_serial.c
===================================================================
--- linux-2.6.23.orig/drivers/serial/atmel_serial.c	2007-12-07 15:31:48.000000000 +0100
+++ linux-2.6.23/drivers/serial/atmel_serial.c	2007-12-07 15:33:34.000000000 +0100
@@ -128,6 +128,22 @@ struct atmel_dma_buffer {
 	unsigned        int ofs;
 };
 
+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.
  */
@@ -143,6 +159,13 @@ struct atmel_uart_port {
 
 	short 			use_dma_tx;	/* enable PDC transmitter */
 	struct atmel_dma_buffer pdc_tx;		/* PDC transmitter */
+
+	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];
@@ -434,12 +457,42 @@ static void atmel_pdc_txbufe(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_bh(&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_bh(&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);
@@ -499,13 +552,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);
 }
 
 /*
@@ -550,9 +603,9 @@ static void atmel_handle_receive(struct 
 
 	/* PDC receive */
 	if (pending & ATMEL_US_ENDRX)
-		atmel_pdc_endrx(port);
+		atmel_pdc_endrx(port); 	/*Not IRQF_NODELAY Safe*/
 	if (pending & ATMEL_US_TIMEOUT)
-		atmel_pdc_timeout(port);
+		atmel_pdc_timeout(port);/*Not IRQF_NODELAY Safe*/
 	if (atmel_port->use_dma_rx
 	    && pending & (ATMEL_US_RXBRK | ATMEL_US_OVRE | ATMEL_US_FRAME |
 			  ATMEL_US_PARE))
@@ -574,7 +627,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)
 {
@@ -595,19 +648,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++;
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
-	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);
+	spin_lock_bh(&port->lock);
+
+	atmel_port->irq_pending = pending;
+	atmel_port->irq_status  = status;
+
+	spin_unlock_bh(&port->lock);
+
+	tasklet_schedule(&atmel_port->status_task);
 }
 
 /*
@@ -635,6 +685,67 @@ 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)
@@ -1058,6 +1169,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;

[-- Attachment #4: patch_atmel_serial_irqf_nodelay --]
[-- Type: application/octet-stream, Size: 1234 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:
* http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz
* atmel_serial_cleanup.patch
* atmel_serial_irq_splitup.patch

Signed-off-by: Remy Bohmer <linux@bohmer.net>
---
 include/linux/serial_core.h |    8 +++++++-
 1 file changed, 7 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-07 14:40:35.000000000 +0100
+++ linux-2.6.23/include/linux/serial_core.h	2007-12-07 15:08:33.000000000 +0100
@@ -225,8 +225,14 @@ struct uart_icount {
 
 typedef unsigned int __bitwise__ upf_t;
 
+#ifdef CONFIG_SERIAL_ATMEL
+typedef raw_spinlock_t uart_spinlock_t;
+#else
+typedef spinlock_t uart_spinlock_t;
+#endif
+
 struct uart_port {
-	spinlock_t		lock;			/* port lock */
+	uart_spinlock_t		lock;			/* port lock */
 	unsigned int		iobase;			/* in/out[bwl] */
 	unsigned char __iomem	*membase;		/* read/write[bwl] */
 	unsigned int		irq;			/* irq number */

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

[David Brownell]

> 		 On AT91 David Brownell noticed several months
> ago that the DBGU can miss some characters on NO_HZ. I would expect
> that this would be better now due to the shorter interrupt handler,
> although it was not my goal to solve it with these patches.

The current (kernel.org git) AT91 timer handler incorporates a bugfix
in that area, which seemed to resolve most of those problems.

- Dave

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

[Andrew Victor]

hi Remy,

> What is wise here? should I create a new patchset for mainline? Or can
> you push the DMA patch also to mainline together with this set? I have
> it working here for months, so I see no reason not to, maybe you have
> a reason?

The serial driver is also used for Atmel's AVR32 architecture.
There was some bug in the DMA patch that causes it to fail there -
Haavard was going to look into in when he had time.

Once that is resolved, the DMA patch will be submitted to mainline.


Regards,
  Andrew Victor

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

[Remy Bohmer]

Hello Andrew,

> The serial driver is also used for Atmel's AVR32 architecture.
> There was some bug in the DMA patch that causes it to fail there -
> Haavard was going to look into in when he had time.
> Once that is resolved, the DMA patch will be submitted to mainline.

In that case, shall I create a new patchset for this compared to mainline?
Then we can merge the DMA patch in the interrupt handler later on. (it
is really difficult to do)

Kind Regards,

Remy

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

[Remy Bohmer]

> In that case, shall I create a new patchset for this compared to mainline?
> Then we can merge the DMA patch in the interrupt handler later on. (it
> is really difficult to do)

Oops, Typo, I meant: it is NOT really difficult...

Remy

2007/12/7, Remy Bohmer <linux@bohmer.net>:
> Hello Andrew,
>
> > The serial driver is also used for Atmel's AVR32 architecture.
> > There was some bug in the DMA patch that causes it to fail there -
> > Haavard was going to look into in when he had time.
> > Once that is resolved, the DMA patch will be submitted to mainline.
>
> In that case, shall I create a new patchset for this compared to mainline?
> Then we can merge the DMA patch in the interrupt handler later on. (it
> is really difficult to do)
>
> Kind Regards,
>
> Remy
>

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

[Steven Rostedt]

[ keeping full email for Ingo and Thomas ]

On Fri, 7 Dec 2007, Remy Bohmer wrote:

> Hello Andrew,
>
> Attached I have put 3 patches for the AT91-series for the serial port
> driver on AT91.
> The interrupt handler of the serial console is quite long, and if
> Preempt-RT is used on AT91 the interrupt handler is even doing illegal
> things. These illegal things are related to the DBGU port of which the
> interrupt handler is shared with the system_interrupt which
> automagically runs in IRQF_NODELAY context due to the timer interrupt.
> The old interrupt handler calls some kernel interfaces that can block
> on a mutex.
>
> To solve this, I have split up the interrupt handler in a IRQF_NODELAY
> safe/unsafe part, with as result that the code running in real
> interrupt context is shortened a lot (the tty driver part is now
> called from a tasklet). On AT91 David Brownell noticed several months
> ago that the DBGU can miss some characters on NO_HZ. I would expect
> that this would be better now due to the shorter interrupt handler,
> although it was not my goal to solve it with these patches. (David can
> you verify if this is better now?)
>
> So, I have here 3 patches:
> * atmel_serial_cleanup -> This patch adapts the driver to the coding
> rules, splits the interrupt handler into 3 routines (cuts the routine
> in more readable pieces) , but there is no functional change involved.
> * atmel_serial_irq_splitup -> This patch splits up the interrupt handler.
> * atmel_serial_irqf_nodelay -> This patch is additionally required to
> get it properly working on Preempt-RT. (This patch should thus go into
> the RT-patch, AFTER integration of the other 2 patches into mainline)
>
> BUT: I based the patch on the 2.6.23.1 + your patch collection at
> http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz
>
> I did this because this driver conflicts with the DMA(PDC) patches
> that are in the patchset on  maxim.org.za. I found out that these DMA
> patches are still not integrated into mainline, although it is in your
> patchset for several kernel releases. I can make a series based on
> kernel mainline, but that would harden the integration for you in your
> patchset.
> The patchset itself is not dependant on the DMA changes, so I can
> split it up, but the DMA changes itself are quite heavy.
>
> But there is also a relation with Preempt-RT. To get the patch in
> preempt RT the other patches has to be in mainline, so things are
> stacking up now.

Not really.

>
> What is wise here? should I create a new patchset for mainline? Or can
> you push the DMA patch also to mainline together with this set? I have
> it working here for months, so I see no reason not to, maybe you have
> a reason?
>
> I tested it on AT91rm9200-EK (+proprietary boards) + AT91SAM9261-EK,
> in combination with: 2.6.23.1 and 2.6.23.1-rt5 up to serial speed to
> 115200 (also with 99% CPU load on prio 99 on RT, no missing characters
> detected.)
>
> Note: Preempt-RT CANNOT run without these patches on AT91.

I could pull all the patches into RT (although I would like Thomas to take
a look first and give an OK). And then apply your patches on top. I'm
assuming that this only affects the ARM architecture and the AT91 device?

I'm leaving this out for -rt13 and for the next cut of 2.6.24-rc-rt. But
if Thomas is OK with pulling in the external patch queue, I'll do it for
-rt14.

It is best if the patch queue in question makes it into mainline.

-- Steve

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

[Remy Bohmer]

Hello Steven,

> > So, I have here 3 patches:
> > * atmel_serial_cleanup -> This patch adapts the driver to the coding
> > rules, splits the interrupt handler into 3 routines (cuts the routine
> > in more readable pieces) , but there is no functional change involved.
> > * atmel_serial_irq_splitup -> This patch splits up the interrupt handler.
> > * atmel_serial_irqf_nodelay -> This patch is additionally required to
> > get it properly working on Preempt-RT. (This patch should thus go into
> > the RT-patch, AFTER integration of the other 2 patches into mainline)
> >
> > BUT: I based the patch on the 2.6.23.1 + Andrews patch collection at
> > http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz
>
> I could pull all the patches into RT (although I would like Thomas to take
> a look first and give an OK).

Instead of pulling the complete patchset, we can also pull the minimal
set of patches to make Preempt-RT to work properly on AT91; Preempt-RT
depends on:
1) ClockSource & ClockEvents for AT91RM9200. [David Brownell, and others]
2) Support configurable HZ. [David Brownell]
3) The Serial port interrupt handler splitup patchset.
4) And of course, the
use-edge-irq-instead-of-simple-irq-interrupt-handler patch for AT91, I
posted last week.

Maybe, Andrew can provide the patches 1 and 2 separate from his
patchset, so you can add them to RT for 2.6.23 only? patch 1 seems to
be in 2.6.24-rc4 already.

> And then apply your patches on top. I'm
> assuming that this only affects the ARM architecture and the AT91 device?

The complete patchset from Andrew also effects other Atmel cores, and
I only focussed on AT91sam/at91rm. Andrew mentioned that the DMA patch
for Serial in his set did not work on AVR32, so, we probably do not
want to have that change until it is really stable.
Tomorrow, I can easily make my set separate from that DMA part, so
that the patches 1-4 can be mainlined without other dependencies,
including the irqf_nodelay-patch which can be put in the RT-patchset.
I can post this mainline-able patch (3) tomorrow.

> I'm leaving this out for -rt13 and for the next cut of 2.6.24-rc-rt. But
> if Thomas is OK with pulling in the external patch queue, I'll do it for
> -rt14.

OK.

> It is best if the patch queue in question makes it into mainline.

Agree.


Kind Regards,

Remy

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

[Remy Bohmer]

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

Hello Steven and Andrew,

Attached I have put a patchset that is independant from the patchset at:
http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz

These patches should go to mainline:
* atmel_serial_cleanup_no_at91.patch
* atmel_serial_irq_splitup_no_at91.patch

And this one into the RT-patch:
* atmel_serial_irqf_nodelay.patch

I hope this set is easier to integrate.

Kind Regards,

Remy

2007/12/12, Remy Bohmer <linux@bohmer.net>:
> Hello Steven,
>
> > > So, I have here 3 patches:
> > > * atmel_serial_cleanup -> This patch adapts the driver to the coding
> > > rules, splits the interrupt handler into 3 routines (cuts the routine
> > > in more readable pieces) , but there is no functional change involved.
> > > * atmel_serial_irq_splitup -> This patch splits up the interrupt handler.
> > > * atmel_serial_irqf_nodelay -> This patch is additionally required to
> > > get it properly working on Preempt-RT. (This patch should thus go into
> > > the RT-patch, AFTER integration of the other 2 patches into mainline)
> > >
> > > BUT: I based the patch on the 2.6.23.1 + Andrews patch collection at
> > > http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz
> >
> > I could pull all the patches into RT (although I would like Thomas to take
> > a look first and give an OK).
>
> Instead of pulling the complete patchset, we can also pull the minimal
> set of patches to make Preempt-RT to work properly on AT91; Preempt-RT
> depends on:
> 1) ClockSource & ClockEvents for AT91RM9200. [David Brownell, and others]
> 2) Support configurable HZ. [David Brownell]
> 3) The Serial port interrupt handler splitup patchset.
> 4) And of course, the
> use-edge-irq-instead-of-simple-irq-interrupt-handler patch for AT91, I
> posted last week.
>
> Maybe, Andrew can provide the patches 1 and 2 separate from his
> patchset, so you can add them to RT for 2.6.23 only? patch 1 seems to
> be in 2.6.24-rc4 already.
>
> > And then apply your patches on top. I'm
> > assuming that this only affects the ARM architecture and the AT91 device?
>
> The complete patchset from Andrew also effects other Atmel cores, and
> I only focussed on AT91sam/at91rm. Andrew mentioned that the DMA patch
> for Serial in his set did not work on AVR32, so, we probably do not
> want to have that change until it is really stable.
> Tomorrow, I can easily make my set separate from that DMA part, so
> that the patches 1-4 can be mainlined without other dependencies,
> including the irqf_nodelay-patch which can be put in the RT-patchset.
> I can post this mainline-able patch (3) tomorrow.
>
> > I'm leaving this out for -rt13 and for the next cut of 2.6.24-rc-rt. But
> > if Thomas is OK with pulling in the external patch queue, I'll do it for
> > -rt14.
>
> OK.
>
> > It is best if the patch queue in question makes it into mainline.
>
> Agree.
>
>
> Kind 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: 22866 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 |  348 +++++++++++++++++++++++-------------------
 1 file changed, 196 insertions(+), 152 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:46.000000000 +0100
+++ linux-2.6.23/drivers/serial/atmel_serial.c	2007-12-13 17:30:48.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,72 @@ static void atmel_tx_chars(struct uart_p
 }
 
 /*
+ * 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;
@@ -409,7 +439,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 +451,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 +475,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 +489,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 +511,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 +580,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 +616,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 +751,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 +775,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 +800,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 +817,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 +825,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 +882,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 +900,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 +917,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 +935,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 +949,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 +979,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 +1014,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 */

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

[Andrew Victor]

hi Remy,

> These patches should go to mainline:
> * atmel_serial_cleanup_no_at91.patch

I don't know what coding rules you're following...   :)

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

Regards,
  Andrew Victor

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

[Remy Bohmer]

[-- 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 */

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

[Remy Bohmer]

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

> Because I know it is common practice in the kernel, I attached 3 new
> patches to inline these :-))

Grmbl, 1 wrong file attached. Here is the correct one.

Regards,

Remy

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: atmel_serial_irq_splitup_no_at91.patch --]
[-- Type: text/x-patch; name=atmel_serial_irq_splitup_no_at91.patch, Size: 7580 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.1-rt5/drivers/serial/atmel_serial.c
===================================================================
--- linux-2.6.23.1-rt5.orig/drivers/serial/atmel_serial.c	2007-12-13 21:33:08.000000000 +0100
+++ linux-2.6.23.1-rt5/drivers/serial/atmel_serial.c	2007-12-13 21:33:10.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);
 }
 
 /*
@@ -381,7 +433,7 @@ atmel_handle_receive(struct uart_port *p
 }
 
 /*
- * transmit interrupt handler.
+ * transmit interrupt handler. (Transmit is IRQF_NODELAY safe)
  */
 static inline void
 atmel_handle_transmit(struct uart_port *port, unsigned int pending)
@@ -398,19 +450,16 @@ 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);
+	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);
 }
 
 /*
@@ -438,6 +487,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)
@@ -770,6 +879,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;

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

[Remy Bohmer]

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

Hello Andrew,

So, to come to a conclusion about this complex patch series, I
attached all the latest versions to this mail. The latest patches from
yesterday including inline are also included to make the set complete.

So, this is the order in which the patches should be installed:
1) atmel_serial_cleanup.patch
2) atmel_serial_irq_splitup.patch
3) NEW: optional:  add-atmel-serial-dma.patch, this merged the DMA
code (from Chip Coldwell) in your 2.6.23 patch back on top of this
series. Because the AT32 bug is not been fixed for a very long time, I
do not expect it to be fixed soon, so I think a reordering is better
to make preempt-RT work on AT91.
4) For RT only: atmel_serial_irqf_nodelay.patch can be applied
anywhere after 1 and 2

Steven, for the RT-patch, patches 1,2 and 4 are the _only_ patches
needed to make RT work stable on AT91 (based on 2.6.23.9-rt13), where
I think that 1 and 2 should go to mainline during time. I tested this
and it works properly, so we can, at least create a working/bootable
preempt-RT system on AT91. Notice that without the other patches from
Andrew the clock granularity is bound to 10ms. (Partly solved in
2.6.24-rc* by mainlining of the clocksource/events patches form David
Brownell)

If any of you have any comments, or suggestions, please let me know.

Kind Regards,

Remy Bohmer

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: atmel_serial_cleanup.patch --]
[-- Type: text/x-patch; name=atmel_serial_cleanup.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.patch --]
[-- Type: text/x-patch; name=atmel_serial_irq_splitup.patch, Size: 7580 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.1-rt5/drivers/serial/atmel_serial.c
===================================================================
--- linux-2.6.23.1-rt5.orig/drivers/serial/atmel_serial.c	2007-12-13 21:33:08.000000000 +0100
+++ linux-2.6.23.1-rt5/drivers/serial/atmel_serial.c	2007-12-13 21:33:10.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);
 }
 
 /*
@@ -381,7 +433,7 @@ atmel_handle_receive(struct uart_port *p
 }
 
 /*
- * transmit interrupt handler.
+ * transmit interrupt handler. (Transmit is IRQF_NODELAY safe)
  */
 static inline void
 atmel_handle_transmit(struct uart_port *port, unsigned int pending)
@@ -398,19 +450,16 @@ 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);
+	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);
 }
 
 /*
@@ -438,6 +487,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)
@@ -770,6 +879,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: add-atmel-serial-dma.patch --]
[-- Type: text/x-patch; name=add-atmel-serial-dma.patch, Size: 14391 bytes --]

This patch adds the DMA-patch build by Chip Coldwell for the AT91/AT32
serial USARTS to the patch stack, but the order of the patches
has been changed.

Because the DMA-patch does not seem to work on AT32 and has not 
been fixed since months and several kernel releases, this DMA 
patch is holding things up. Currently the DBGU and the USART does
not work on Preempt-RT, and needs a splitup of the interrupt handler.
The biggest part of the interrupt handler splitup patch is not related
to RT, and can therefor be integrated in mainline seperately.
When that is done, the RT-patch can be fixed. Due to the bug in the 
DMA-patch this integration gets too complicated, and a reorder of
patches is required.

Previously this order was valid:
* http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz
* Serial-port interrupt handler splitup patches

With this patch, the order needs to be:
* Serial-port splitup patches.
* This patch for adding DMA-support.

The older DMA-patch has to be removed from the patch at:
http://maxim.org.za/AT91RM9200/2.6/2.6.23-at91.patch.gz

Signed-off-by: Remy Bohmer <linux@bohmer.net>

---
 drivers/serial/atmel_serial.c |  325 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 319 insertions(+), 6 deletions(-)

Index: linux-2.6.23/drivers/serial/atmel_serial.c
===================================================================
--- linux-2.6.23.orig/drivers/serial/atmel_serial.c	2007-12-14 12:00:20.000000000 +0100
+++ linux-2.6.23/drivers/serial/atmel_serial.c	2007-12-14 12:00:23.000000000 +0100
@@ -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 {
 	unsigned int status;
 	unsigned int overrun;
@@ -136,6 +151,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   rx_task;
 	struct tasklet_struct   status_task;
 	unsigned int 		irq_pending;
@@ -146,6 +168,9 @@ 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
@@ -231,7 +256,14 @@ static u_int atmel_get_mctrl(struct uart
  */
 static void atmel_stop_tx(struct uart_port *port)
 {
-	UART_PUT_IDR(port, ATMEL_US_TXRDY);
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
+
+	if (atmel_port->use_dma_tx) {
+		/* 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);
 }
 
 /*
@@ -239,7 +271,19 @@ static void atmel_stop_tx(struct uart_po
  */
 static void atmel_start_tx(struct uart_port *port)
 {
-	UART_PUT_IER(port, ATMEL_US_TXRDY);
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
+
+	if (atmel_port->use_dma_tx) {
+		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);
 }
 
 /*
@@ -247,7 +291,14 @@ static void atmel_start_tx(struct uart_p
  */
 static void atmel_stop_rx(struct uart_port *port)
 {
-	UART_PUT_IDR(port, ATMEL_US_RXRDY);
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
+
+	if (atmel_port->use_dma_rx) {
+		/* 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);
 }
 
 /*
@@ -302,6 +353,148 @@ out:
 }
 
 /*
+ * Receive data via the PDC.  A buffer has been fulled.
+ */
+static void atmel_pdc_endrx(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 = PDC_RX_BUF(atmel_port);
+	unsigned int count;
+
+	count = pdc->dma_size - pdc->ofs;
+	if (likely(count > 0)) {
+		dma_sync_single_for_cpu(port->dev,
+					pdc->dma_addr,
+					pdc->dma_size,
+					DMA_FROM_DEVICE);
+		tty_insert_flip_string(tty, pdc->buf + pdc->ofs, count);
+		tty_flip_buffer_push(tty);
+
+		port->icount.rx += count;
+	}
+
+	/* Set this buffer as the next receive buffer */
+	pdc->ofs = 0;
+	UART_PUT_RNPR(port, pdc->dma_addr);
+	UART_PUT_RNCR(port, pdc->dma_size);
+
+	/* Switch to next buffer */
+	PDC_RX_SWITCH(atmel_port);		/* next PDC buffer */
+}
+
+/*
+ * Receive data via the PDC.  At least one byte was received, but the
+ * buffer was not full when the inter-character timeout expired.
+ */
+static void atmel_pdc_timeout(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 = PDC_RX_BUF(atmel_port);
+	/* unsigned */ int ofs, count;
+
+	ofs = UART_GET_RPR(port) - pdc->dma_addr; /* current DMA adress */
+	count = ofs - pdc->ofs;
+
+	if (likely(count > 0)) {
+		dma_sync_single_for_cpu(port->dev,
+					pdc->dma_addr,
+					pdc->dma_size,
+					DMA_FROM_DEVICE);
+		tty_insert_flip_string(tty, pdc->buf + pdc->ofs, count);
+		tty_flip_buffer_push(tty);
+
+		pdc->ofs = ofs;
+		port->icount.rx += count;
+	}
+
+	/* reset the UART timeout */
+	UART_PUT_CR(port, ATMEL_US_STTTO);
+}
+
+/*
+ * 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++;
+}
+
+/*
+ * A transmission via the PDC is complete.
+ */
+static void atmel_pdc_endtx(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;
+
+	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_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(port);
+}
+
+/*
+ * The PDC transmitter is idle, so either start the next transfer or
+ * disable the transmitter.
+ */
+static void atmel_pdc_txbufe(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;
+
+	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 */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);
+	} else {
+		/* nothing left to transmit - disable the transmitter */
+
+		/* disable PDC transmit */
+		UART_PUT_PTCR(port, ATMEL_PDC_TXTDIS);
+		UART_PUT_IDR(port, ATMEL_US_ENDTX | ATMEL_US_TXBUFE);
+	}
+}
+
+/*
  * Characters received (called from interrupt handler)
  */
 static void atmel_rx_chars(struct uart_port *port)
@@ -417,6 +610,16 @@ atmel_handle_receive(struct uart_port *p
 {
 	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 
+	/* PDC receive */
+	if (pending & ATMEL_US_ENDRX)
+		atmel_pdc_endrx(port);
+	if (pending & ATMEL_US_TIMEOUT)
+		atmel_pdc_timeout(port);
+	if (atmel_port->use_dma_rx &&
+	    (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);
@@ -438,6 +641,12 @@ atmel_handle_receive(struct uart_port *p
 static inline void
 atmel_handle_transmit(struct uart_port *port, unsigned int pending)
 {
+	/* PDC transmit */
+	if (pending & ATMEL_US_ENDTX)
+		atmel_pdc_endtx(port);
+	if (pending & ATMEL_US_TXBUFE)
+		atmel_pdc_txbufe(port);
+
 	/* Interrupt transmit */
 	if (pending & ATMEL_US_TXRDY)
 		atmel_tx_chars(port);
@@ -551,6 +760,7 @@ static void atmel_status_handler_task(un
  */
 static int atmel_startup(struct uart_port *port)
 {
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *)port;
 	int retval;
 
 	/*
@@ -572,6 +782,56 @@ static int atmel_startup(struct uart_por
 	}
 
 	/*
+	 * Initialize DMA (if necessary)
+	 */
+	if (atmel_port->use_dma_rx) {
+		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_port->use_dma_tx) {
+		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)
 	 */
@@ -590,8 +850,18 @@ static int atmel_startup(struct uart_por
 	/* 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_port->use_dma_rx) {
+		/* 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;
 }
@@ -601,6 +871,38 @@ 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;
+	/*
+	 * Ensure everything is stopped.
+	 */
+	atmel_stop_rx(port);
+	atmel_stop_tx(port);
+
+	/*
+	 * Shut-down the DMA.
+	 */
+	if (atmel_port->use_dma_rx) {
+		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_port->use_dma_tx) {
+		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.
 	 */
@@ -654,6 +956,7 @@ static void atmel_serial_pm(struct uart_
 static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 			      struct ktermios *old)
 {
+	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
 	unsigned long flags;
 	unsigned int mode, imr, quot, baud;
 
@@ -713,6 +1016,9 @@ static void atmel_set_termios(struct uar
 	if (termios->c_iflag & (BRKINT | PARMRK))
 		port->read_status_mask |= ATMEL_US_RXBRK;
 
+	if (atmel_port->use_dma_rx)	/* need to enable error interrupts */
+		UART_PUT_IER(port, port->read_status_mask);
+
 	/*
 	 * Characters to ignore
 	 */
@@ -900,6 +1206,13 @@ static void __devinit atmel_init_port(st
 		clk_enable(atmel_port->clk);
 		port->uartclk = clk_get_rate(atmel_port->clk);
 	}
+
+#ifdef SUPPORT_PDC
+	atmel_port->use_dma_rx = data->use_dma_rx;
+	atmel_port->use_dma_tx = data->use_dma_tx;
+	if (atmel_port->use_dma_tx)
+		port->fifosize = PDC_BUFFER_SIZE;
+#endif
 }
 
 /*
@@ -1085,7 +1398,7 @@ static int atmel_serial_suspend(struct p
 	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);

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 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 */

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

[Haavard Skinnemoen]

On Fri, 14 Dec 2007 12:46:09 +0100
"Remy Bohmer" <linux@bohmer.net> wrote:

> Hello Andrew,
> 
> So, to come to a conclusion about this complex patch series, I
> attached all the latest versions to this mail. The latest patches from
> yesterday including inline are also included to make the set complete.
> 
> So, this is the order in which the patches should be installed:
> 1) atmel_serial_cleanup.patch
> 2) atmel_serial_irq_splitup.patch
> 3) NEW: optional:  add-atmel-serial-dma.patch, this merged the DMA
> code (from Chip Coldwell) in your 2.6.23 patch back on top of this
> series. Because the AT32 bug is not been fixed for a very long time, I
> do not expect it to be fixed soon, so I think a reordering is better
> to make preempt-RT work on AT91.

I'll give it a shot, but first I have some comments on your other
patches.

Btw, it would be nice if patches that affect more or less
architecture-independent drivers were posted to linux-kernel (added to
Cc.)

Also, it would be easier to review if you posted just one patch per
e-mail. I'm going to cut & paste a bit from your attachments.

> 4) For RT only: atmel_serial_irqf_nodelay.patch can be applied
> anywhere after 1 and 2

I'll ignore this for now.

> +#define lread(port)		__raw_readl(port)
> +#define lwrite(v, port)		__raw_writel(v, port)

Why is this necessary, and what does 'l' stand for?

> -	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 */

Looks like you're adding one or more spaces before each TAB here. Why
is that an improvement?

> 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);
>  }

These conflict with David Brownell's "atmel_serial build warnings
begone" patch which was merged into mainline a few weeks ago.

> 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);

This looks a bit funny...IMO it would be better to keep a few of them
on the first line and indent the next line with an extra tab. But I'm
not going to be difficult about it if it conforms with the existing
style in the driver.

>  /*
> + * receive interrupt handler.
> + */
> +static inline void
> +atmel_handle_receive(struct uart_port *port, unsigned int pending)

Please drop "inline" here. The compiler will do it automatically if it
has only one caller, and if it at some point gets several callers, we
might not want to inline it after all.

> +{
> +	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.
> +			*/

Looks like the indentation got messed up here.

> +		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);

Perhaps indent the big OR expression with another TAB to separate it
from the body?

> @@ -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);

I think request_irq() belongs on the same line as "retval =".

>  static void atmel_shutdown(struct uart_port *port)
>  {
> -	struct atmel_uart_port *atmel_port = (struct atmel_uart_port *) port;
> -

Already in mainline.

>  	/* 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 */

Please use TABs, not spaces. Might as well remove those comments...they
don't seem all that useful.

> +	while (!(UART_GET_CSR(port) & ATMEL_US_TXEMPTY))
> +		barrier();

Should probably use cpu_relax(), but it's probably out of scope for a
codingstyle cleanup patch (and I don't think it matters on AVR32 or
ARM.)

>  	/*
> -	 *	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
>  	 */

Looks like you're replacing TABs with spaces. Why?

> -// 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 * /
> +//      }*/

That's a funny mix of C and C++ comments. Why not simply use #if 0 and
kill all the C++ comments?

That said, I don't understand what "TODO" means in this context. CR
isn't going to be readable any time soon.

> @@ -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;

Looks better if you move the comment inside the body IMO.

> -	UART_PUT_IDR(port, -1);				/* disable interrupts */
> +	UART_PUT_IDR(port, -1);	/* disable interrupts */

Just kill that useless comment.

> @@ -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);

That looks a bit funny. If you're having trouble moving
&(atmel_ports... onto the same line as atmel_init_port, please consider
removing the redundant parentheses.

Moving on to the next patch...

> 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.

What calls are we talking about here? uart_insert_char() and
tty_flip_buffer_push()?

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

Why is it necessary with two tasklets?

> +struct atmel_uart_char {
> +	unsigned int status;
> +	unsigned int overrun;
> +	unsigned int ch;
> +	unsigned int flg;
> +};

Hmm. 16 bytes per char is a bit excessive, isn't it? How about

struct atmel_uart_char {
	u16	ch;
	u16	status;
};

where ch is the received character (up to 9 bits) and status is the
lowest 16 bits of the status register?

> +#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];
> +};

Why not use struct circ_buf? Why do you need "count"?

Ok, that's all for now. I'm going to take a closer look at the DMA
patch and hopefully figure out why it isn't working on AVR32.

Thanks for your efforts in cleaning this stuff up.

Haavard

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

[Haavard Skinnemoen]

On Mon, 17 Dec 2007 13:17:01 +0100
Haavard Skinnemoen <hskinnemoen@atmel.com> wrote:

> > 3) NEW: optional:  add-atmel-serial-dma.patch, this merged the DMA
> > code (from Chip Coldwell) in your 2.6.23 patch back on top of this
> > series. Because the AT32 bug is not been fixed for a very long time, I
> > do not expect it to be fixed soon, so I think a reordering is better
> > to make preempt-RT work on AT91.  
> 
> I'll give it a shot, but first I have some comments on your other
> patches.

I found a bug. The RX DMA buffer isn't invalidated before it's handed
over to the hardware. With the below patch, it seems to behave a lot
better on AVR32, but it still does funny things now and then (it seems
to dump a good chunk of stale TX data before rebooting), and magic sysrq
doesn't work.

I think I've spotted other potential races in the DMA code as well, but
they look a bit more difficult to hunt down. I'll give it another try
tomorrow.

Haavard

From: Haavard Skinnemoen <hskinnemoen@atmel.com>
Subject: [PATCH] atmel_serial: Sync DMA RX buffer after copying from it

Calling dma_sync_single_for_cpu() before reading the buffer isn't
enough. We need to call dma_sync_single_for_device() after we're done
reading as well.

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

diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
index 0610ad9..e23a3ba 100644
--- a/drivers/serial/atmel_serial.c
+++ b/drivers/serial/atmel_serial.c
@@ -391,6 +391,8 @@ static void atmel_pdc_endrx(struct uart_port *port)
 		tty_insert_flip_string(tty, pdc->buf + pdc->ofs, count);
 		tty_flip_buffer_push(tty);
 
+		dma_sync_single_for_device(port->dev, pdc->dma_addr,
+				pdc->dma_size, DMA_FROM_DEVICE);
 		port->icount.rx += count;
 	}
 
@@ -425,6 +427,8 @@ static void atmel_pdc_timeout(struct uart_port *port)
 		tty_insert_flip_string(tty, pdc->buf + pdc->ofs, count);
 		tty_flip_buffer_push(tty);
 
+		dma_sync_single_for_device(port->dev, pdc->dma_addr,
+				pdc->dma_size, DMA_FROM_DEVICE);
 		pdc->ofs = ofs;
 		port->icount.rx += count;
 	}
-- 
1.5.3.4

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

[Remy Bohmer]

Hello Haavard,

> I'll give it a shot, but first I have some comments on your other
> patches.

Good news someone is working on this bug again. Also good news you
already found a bug in there.

> Btw, it would be nice if patches that affect more or less
> architecture-independent drivers were posted to linux-kernel (added to
> Cc.)

Not really architecture independant, I believe, because thos are
drivers for peripherals _inside_ Atmel Cores ;-)

> Also, it would be easier to review if you posted just one patch per
> e-mail. I'm going to cut & paste a bit from your attachments.

I know, but I have some troubles to get 'quilt mail' to work from
behind a proxy server, and attaching to a mail, at least it does not
corrupt the contents of the patches.

> > 4) For RT only: atmel_serial_irqf_nodelay.patch can be applied
> > anywhere after 1 and 2
>
> I'll ignore this for now.

OK.

> > +#define lread(port)          __raw_readl(port)
> > +#define lwrite(v, port)              __raw_writel(v, port)
>
> Why is this necessary, and what does 'l' stand for?

There is a huge list of macros below these definitions. By doing it
this way, the macros still fit on 80 characters wide, while without
them, I had split up the macros over several lines, which does not
make it more readable. That's all.
'l' refers at the last letter of __raw_readl, and writel. Nothing special.

>
> > -     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 */
>
> Looks like you're adding one or more spaces before each TAB here. Why
> is that an improvement?

I used scripts/Lindent to reformat the file, and then I removed the
quirks Lindent put in the file. Apparantly I missed that one.

> These conflict with David Brownell's "atmel_serial build warnings
> begone" patch which was merged into mainline a few weeks ago.

Hmm, I seem to have missed that one. Why is it not there in a
big-AT91-patch from Andrew?

> >  /*
> > + * receive interrupt handler.
> > + */
> > +static inline void
> > +atmel_handle_receive(struct uart_port *port, unsigned int pending)
>
> Please drop "inline" here. The compiler will do it automatically if it
> has only one caller, and if it at some point gets several callers, we
> might not want to inline it after all.

Funny, This was the first thing that Andrew started complaining about.
He suggested to put an inline there which I had not. I already
mentioned that this was against the CodingStyle, but I also mentioned
that I did not wanted to start a fight about this :-)
So, to prevent a discussion, I added the inline...

> > @@ -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);
>
> I think request_irq() belongs on the same line as "retval =".

I blame scripts/Lindent ;-)

> Please use TABs, not spaces. Might as well remove those comments...they
> don't seem all that useful.

Go ahead...

I did not remove any comment, even if they appear useless to me. I am
not the maintainer of this driver, and just wanted to improve it, so
that it was able of running on Preempt-RT.
Before being able to submit the change that really mattered to me, I
had to make the driver pass the scripts/checkpatch.pl check, otherwise
the patch-that-matters would be completely unreadable.

>
> > +     while (!(UART_GET_CSR(port) & ATMEL_US_TXEMPTY))
> > +             barrier();
>
> Should probably use cpu_relax(), but it's probably out of scope for a
> codingstyle cleanup patch (and I don't think it matters on AVR32 or
> ARM.)

Agree.

> >       /*
> > -      *      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
> >        */
>
> Looks like you're replacing TABs with spaces. Why?

????

> > -// 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 * /
> > +//      }*/
>
> That's a funny mix of C and C++ comments. Why not simply use #if 0 and
> kill all the C++ comments?

As mentioned, did not want to change it that much.

> That said, I don't understand what "TODO" means in this context. CR
> isn't going to be readable any time soon.

I also did not understand what was meant here.

> > @@ -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;
>
> Looks better if you move the comment inside the body IMO.

And would add some extra brackets etc.
I did not try to make it perfect ;-)

>
> > -     UART_PUT_IDR(port, -1);                         /* disable interrupts */
> > +     UART_PUT_IDR(port, -1); /* disable interrupts */
>
> Just kill that useless comment.

Agree

> > @@ -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);
>
> That looks a bit funny. If you're having trouble moving
> &(atmel_ports... onto the same line as atmel_init_port, please consider
> removing the redundant parentheses.

Guess, that wouldn't fit either?

> Moving on to the next patch...
>
> > 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.
>
> What calls are we talking about here? uart_insert_char() and
> tty_flip_buffer_push()?

Yep. All calls that block on a Mutex somehow on Preempt-RT. (such as
spinlocks, wakeup_interruptible() and many of its friends.)

> > 2 tasklets are used:
> > * one for handling the error statuses
> > * one for pushing the incoming characters into the tty-layer.
>
> Why is it necessary with two tasklets?

To prevent rewriting the code completely. 1 change was in a read
routine, the other at a different location.

> > +struct atmel_uart_char {
> > +     unsigned int status;
> > +     unsigned int overrun;
> > +     unsigned int ch;
> > +     unsigned int flg;
> > +};
>
> Hmm. 16 bytes per char is a bit excessive, isn't it? How about
>
> struct atmel_uart_char {
>         u16     ch;
>         u16     status;
> };
>
> where ch is the received character (up to 9 bits) and status is the
> lowest 16 bits of the status register?

I used the NODELAY patch for the 8250 serial port as reference, it
contains similar code, and I tried to make both look the same.

>
> > +#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];
> > +};
>
> Why not use struct circ_buf? Why do you need "count"?

See previous remark.

> Ok, that's all for now. I'm going to take a closer look at the DMA
> patch and hopefully figure out why it isn't working on AVR32.
>
> Thanks for your efforts in cleaning this stuff up.
>
> Haavard
>

Thank you for reviewing it.

I will have a look at it in again later this week.

Have you any idea how we can integrate both patch series (yours and
mine) without interfering each other? This patchset was quite
annoying, because through multiple channels patches are submitted, and
even stacked up...

Kind Regards,

Remy

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

[Haavard Skinnemoen]

On Mon, 17 Dec 2007 21:56:30 +0100
"Remy Bohmer" <linux@bohmer.net> wrote:

> > Btw, it would be nice if patches that affect more or less
> > architecture-independent drivers were posted to linux-kernel (added
> > to Cc.)
> 
> Not really architecture independant, I believe, because thos are
> drivers for peripherals _inside_ Atmel Cores ;-)

Well, those Atmel cores cover two different architectures: ARM and
AVR32. But yeah, "architecture independent" is perhaps a bit of a
stretch ;-)

> > Also, it would be easier to review if you posted just one patch per
> > e-mail. I'm going to cut & paste a bit from your attachments.
> 
> I know, but I have some troubles to get 'quilt mail' to work from
> behind a proxy server, and attaching to a mail, at least it does not
> corrupt the contents of the patches.

Ok, attachments are a lot better than corrupted patches.

> > > +#define lread(port)          __raw_readl(port)
> > > +#define lwrite(v, port)              __raw_writel(v, port)
> >
> > Why is this necessary, and what does 'l' stand for?
> 
> There is a huge list of macros below these definitions. By doing it
> this way, the macros still fit on 80 characters wide, while without
> them, I had split up the macros over several lines, which does not
> make it more readable. That's all.
> 'l' refers at the last letter of __raw_readl, and writel. Nothing
> special.

Ok, makes sense.

> > Looks like you're adding one or more spaces before each TAB here.
> > Why is that an improvement?
> 
> I used scripts/Lindent to reformat the file, and then I removed the
> quirks Lindent put in the file. Apparantly I missed that one.

Hmm. Lindent does such things? That's not very helpful...

> > These conflict with David Brownell's "atmel_serial build warnings
> > begone" patch which was merged into mainline a few weeks ago.
> 
> Hmm, I seem to have missed that one. Why is it not there in a
> big-AT91-patch from Andrew?

I think it went in via Andrew Morton.

But it's fine by me -- it all worked out automatically when I applied it
to 2.6.23 and rebased.

> > >  /*
> > > + * receive interrupt handler.
> > > + */
> > > +static inline void
> > > +atmel_handle_receive(struct uart_port *port, unsigned int
> > > pending)
> >
> > Please drop "inline" here. The compiler will do it automatically if
> > it has only one caller, and if it at some point gets several
> > callers, we might not want to inline it after all.
> 
> Funny, This was the first thing that Andrew started complaining about.
> He suggested to put an inline there which I had not. I already
> mentioned that this was against the CodingStyle, but I also mentioned
> that I did not wanted to start a fight about this :-)
> So, to prevent a discussion, I added the inline...

Hmm, ok. Andrew, could you elaborate on why you want it to be inline?

> I did not remove any comment, even if they appear useless to me. I am
> not the maintainer of this driver, and just wanted to improve it, so
> that it was able of running on Preempt-RT.
> Before being able to submit the change that really mattered to me, I
> had to make the driver pass the scripts/checkpatch.pl check, otherwise
> the patch-that-matters would be completely unreadable.

I understand that.

> > >       /*
> > > -      *      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
> > >        */
> >
> > Looks like you're replacing TABs with spaces. Why?
> 
> ????

Originally, there's a TAB between '*' and First/Finally. The patch
replaces it with spaces.

That said, it's probably better to just replace the tab with a single
space...

> > > -// 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 * /
> > > +//      }*/
> >
> > That's a funny mix of C and C++ comments. Why not simply use #if 0
> > and kill all the C++ comments?
> 
> As mentioned, did not want to change it that much.

Understandably. However, I think we should just kill that code
altogether and check BRGR instead. If BRGR is 0, the baud rate
generator doesn't run, so we can assume the port hasn't been
initialized.

I'll cook up a separate patch for that. Until then, I think we should
just leave it as it was.

> > That said, I don't understand what "TODO" means in this context. CR
> > isn't going to be readable any time soon.
> 
> I also did not understand what was meant here.

I think I do. We want to check if the port is already enabled, but we
can't read it from CR. So I think we should use BRGR instead.

> > > @@ -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;
> >
> > Looks better if you move the comment inside the body IMO.
> 
> And would add some extra brackets etc.
> I did not try to make it perfect ;-)

No extra brackets are needed. Comments don't count.

> > > @@ -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);
> >
> > That looks a bit funny. If you're having trouble moving
> > &(atmel_ports... onto the same line as atmel_init_port, please
> > consider removing the redundant parentheses.
> 
> Guess, that wouldn't fit either?

It does. Barely.

> > Moving on to the next patch...
> >
> > > 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.
> >
> > What calls are we talking about here? uart_insert_char() and
> > tty_flip_buffer_push()?
> 
> Yep. All calls that block on a Mutex somehow on Preempt-RT. (such as
> spinlocks, wakeup_interruptible() and many of its friends.)

Right. Looks like the DMA patch call these functions from irq context
too...I guess it'll need the same treatment?

> > > 2 tasklets are used:
> > > * one for handling the error statuses
> > > * one for pushing the incoming characters into the tty-layer.
> >
> > Why is it necessary with two tasklets?
> 
> To prevent rewriting the code completely. 1 change was in a read
> routine, the other at a different location.

Ok, but both originate from the interrupt handler...

> > > +struct atmel_uart_char {
> > > +     unsigned int status;
> > > +     unsigned int overrun;
> > > +     unsigned int ch;
> > > +     unsigned int flg;
> > > +};
> >
> > Hmm. 16 bytes per char is a bit excessive, isn't it? How about
> >
> > struct atmel_uart_char {
> >         u16     ch;
> >         u16     status;
> > };
> >
> > where ch is the received character (up to 9 bits) and status is the
> > lowest 16 bits of the status register?
> 
> I used the NODELAY patch for the 8250 serial port as reference, it
> contains similar code, and I tried to make both look the same.

Ok, I see. But...

> > > +#define ATMEL_SERIAL_RINGSIZE 1024

This means that the buffer ends up at 16K. Since the buffer itself is
kept in a struct along with a few other pieces of data, we end up
kmalloc()ing 32KB of memory...

> > > +struct atmel_uart_ring {
> > > +     unsigned int          head;
> > > +     unsigned int          tail;
> > > +     unsigned int          count;
> > > +     struct atmel_uart_char data[ATMEL_SERIAL_RINGSIZE];
> > > +};
> >
> > Why not use struct circ_buf? Why do you need "count"?
> 
> See previous remark.

Ok.

> > Ok, that's all for now. I'm going to take a closer look at the DMA
> > patch and hopefully figure out why it isn't working on AVR32.
> >
> > Thanks for your efforts in cleaning this stuff up.
> >
> > Haavard
> >
> 
> Thank you for reviewing it.
> 
> I will have a look at it in again later this week.
> 
> Have you any idea how we can integrate both patch series (yours and
> mine) without interfering each other? This patchset was quite
> annoying, because through multiple channels patches are submitted, and
> even stacked up...

I'm a bit tempted to just go ahead and do the changes we agree on and
post the result. Then I'll see if I can make the DMA stuff behave. I've
got a different driver lying around which is DMA-only and has a few
problems on its own. Integrating the good bits into the atmel_serial
driver will hopefully result in something that is better than either of
them.

Also, I think the DMA patch needs to be more integrated with your
"interrupt handler splitup" patch. No point in keeping two RX buffers
around, and the DMA code needs to obey the same rules as the rest of
the driver if it's going to work in -rt.

And I'd really like to have working DMA support on avr32 before
christmas ;-)

Haavard

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

[Remy Bohmer]

Hello Haavard,

> > Yep. All calls that block on a Mutex somehow on Preempt-RT. (such as
> > spinlocks, wakeup_interruptible() and many of its friends.)
> Right. Looks like the DMA patch call these functions from irq context
> too...I guess it'll need the same treatment?

That is correct. DMA code does do that, but the DMA code is not used
for DBGU, and _only_ the interrupt handler of DBGU runs in
IRQF_NODELAY context (because it is part of the System-IRQ and thus
shared with the timer-irq). The DMA code always runs in IRQ-thread
context, where it is safe to block on a mutex.
So, it is not mandatory to change that also. (It may be nice to do it anyway)

> > > > +struct atmel_uart_char {
> > > > +     unsigned int status;
> > > > +     unsigned int overrun;
> > > > +     unsigned int ch;
> > > > +     unsigned int flg;
> > > > +};
> > >
> > > Hmm. 16 bytes per char is a bit excessive, isn't it? How about
> > >
> > > struct atmel_uart_char {
> > >         u16     ch;
> > >         u16     status;
> > > };
> > >
> > > where ch is the received character (up to 9 bits) and status is the
> > > lowest 16 bits of the status register?
> > I used the NODELAY patch for the 8250 serial port as reference, it
> > contains similar code, and I tried to make both look the same.
> Ok, I see. But...
> > > > +#define ATMEL_SERIAL_RINGSIZE 1024
> This means that the buffer ends up at 16K. Since the buffer itself is
> kept in a struct along with a few other pieces of data, we end up
> kmalloc()ing 32KB of memory...

Oops... 32kB on X86 is not much, relatively, on ARM it is somewhat different.
We can also decrease the total RingSize, 128 would be good enough also.
I can look at it later this week.

> I'm a bit tempted to just go ahead and do the changes we agree on and
> post the result. Then I'll see if I can make the DMA stuff behave. I've
> got a different driver lying around which is DMA-only and has a few
> problems on its own.

Beware that, according to several older discussions, DBGU cannot make
use of DMA... (If I understood it correctly, it was because the buffer
first have to be full, before an interupt is generated. That would be
very annoying while typing 1 character at the time ;-)

> Also, I think the DMA patch needs to be more integrated with your
> "interrupt handler splitup" patch. No point in keeping two RX buffers
> around, and the DMA code needs to obey the same rules as the rest of
> the driver if it's going to work in -rt.

As mentioned, not necessarily...

> And I'd really like to have working DMA support on avr32 before
> christmas ;-)

Me too ;-)


Kind Regards,

Remy

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

[Russell King - ARM Linux]

On Mon, Dec 17, 2007 at 09:56:30PM +0100, Remy Bohmer wrote:
> > > +#define lread(port)          __raw_readl(port)
> > > +#define lwrite(v, port)              __raw_writel(v, port)
> >
> > Why is this necessary, and what does 'l' stand for?
> 
> There is a huge list of macros below these definitions. By doing it
> this way, the macros still fit on 80 characters wide, while without
> them, I had split up the macros over several lines, which does not
> make it more readable. That's all.
> 'l' refers at the last letter of __raw_readl, and writel. Nothing special.

So why not keep to the Linux convention?  How about at_readl() and
at_writel() ?

> > >  /*
> > > + * receive interrupt handler.
> > > + */
> > > +static inline void
> > > +atmel_handle_receive(struct uart_port *port, unsigned int pending)
> >
> > Please drop "inline" here. The compiler will do it automatically if it
> > has only one caller, and if it at some point gets several callers, we
> > might not want to inline it after all.
> 
> Funny, This was the first thing that Andrew started complaining about.
> He suggested to put an inline there which I had not. I already
> mentioned that this was against the CodingStyle, but I also mentioned
> that I did not wanted to start a fight about this :-)
> So, to prevent a discussion, I added the inline...

There's two schools of thought - those who want to add 'inline' keywords
all over the place and those who don't.  It's quite correct that if a
static function will be inlined by the compiler as it sees fit.  It
_can_ be that the compiler will chose not to inline it and that may
result in better register allocation in the caller, resulting in overall
faster code.

> >
> > > +     while (!(UART_GET_CSR(port) & ATMEL_US_TXEMPTY))
> > > +             barrier();
> >
> > Should probably use cpu_relax(), but it's probably out of scope for a
> > codingstyle cleanup patch (and I don't think it matters on AVR32 or
> > ARM.)
> 
> Agree.

Even though it doesn't matter at the moment, I rather like to think a
bit about the future.  If the kernel has a simple and cheap mechanism
there's no reason to avoid using it.

> 
> > >       /*
> > > -      *      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
> > >        */
> >
> > Looks like you're replacing TABs with spaces. Why?
> 
> ????

I think someone's mailer might be messing with the patches.  The above
removed and added lines appear to be identical.

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

[Haavard Skinnemoen]

On Mon, 17 Dec 2007 23:49:32 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Mon, Dec 17, 2007 at 09:56:30PM +0100, Remy Bohmer wrote:
> > > > +#define lread(port)          __raw_readl(port)
> > > > +#define lwrite(v, port)              __raw_writel(v, port)
> > >
> > > Why is this necessary, and what does 'l' stand for?
> > 
> > There is a huge list of macros below these definitions. By doing it
> > this way, the macros still fit on 80 characters wide, while without
> > them, I had split up the macros over several lines, which does not
> > make it more readable. That's all.
> > 'l' refers at the last letter of __raw_readl, and writel. Nothing special.
> 
> So why not keep to the Linux convention?  How about at_readl() and
> at_writel() ?

Something like this perhaps?

#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)

That said, I wonder if it may actually be nicer to just use
at_writel()/at_readl() throughout the driver...but that's for a later
cleanup.

> > >
> > > > +     while (!(UART_GET_CSR(port) & ATMEL_US_TXEMPTY))
> > > > +             barrier();
> > >
> > > Should probably use cpu_relax(), but it's probably out of scope for a
> > > codingstyle cleanup patch (and I don't think it matters on AVR32 or
> > > ARM.)
> > 
> > Agree.
> 
> Even though it doesn't matter at the moment, I rather like to think a
> bit about the future.  If the kernel has a simple and cheap mechanism
> there's no reason to avoid using it.

I can do it in a separate patch.

> > 
> > > >       /*
> > > > -      *      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
> > > >        */
> > >
> > > Looks like you're replacing TABs with spaces. Why?
> > 
> > ????
> 
> I think someone's mailer might be messing with the patches.  The above
> removed and added lines appear to be identical.

Yes, the difference was wiped out after a few times back and forth. It
was visible (or...not actually visible, but you know how to detect
these things) in the first couple of e-mails.

Haavard

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

[Remy Bohmer]

Hi,

2010/10/29 Aras Vaichas <arasv@magellan-technology.com>:
>> >                On AT91 David Brownell noticed several months
>> > ago that the DBGU can miss some characters on NO_HZ. I would expect
>> > that this would be better now due to the shorter interrupt handler,
>> > although it was not my goal to solve it with these patches.
>> The current (kernel.org git) AT91 timer handler incorporates a bugfix
>> in that area, which seemed to resolve most of those problems.
>> - Dave
>
> Hello,
>
> I'm sorry to bother you two off-list, but I'm trying to avoid too much
> noise on the lists.

Please, always at least CC to the mailinglist as well.

> My issue is related to these two posts (and the re: above):
>
> http://www.mail-archive.com/linux-rt-users@vger.kernel.org/msg02115.html
> http://permalink.gmane.org/gmane.linux.ports.arm.kernel/94652
>
> If I change the Atmel serial interrupt to IRQF_NODELAY, then *most* of
> the overruns go away and DBGU works as it should.
>
> --- A/drivers/serial/atmel_serial.c   2010-08-03 03:27:18.000000000 +1000
> +++ B/drivers/serial/atmel_serial.c 2010-10-29 10:24:54.000000000 +1100
> @@ -808,7 +818,7 @@
>        /*
>         * Allocate the IRQ
>         */
> -       retval = request_irq(port->irq, atmel_interrupt, IRQF_SHARED,
> +       retval = request_irq(port->irq, atmel_interrupt, IRQF_NODELAY,
>                        tty ? tty->name : "atmel_serial", port);
>        if (retval) {
>                printk("atmel_serial: atmel_startup - Can't get irq\n");
>
>
> But shouldn't the DBGU interrupt already be running in NODELAY context
> according to Remy's post?  I understand that NODELAY has many other
> complications, this is just a test to get an idea of which direction
> to move in.

On preempt-RT this driver always has NODELAY set by default. AFAIK, it
is not in mainline.

Notice that on recent kernels also the request_threaded_irq()
mechanism can be used, which is much cleaner compared to the NODELAY
change.
So, I guess the NODELAY change will never hit mainline since this
better solution exist.

Kind regards,

Remy
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html