[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

Third attempt at the serial driver patch for the PMC-Sierra MSP71xx device.

There are three different fixes:
1. Fix for DesignWare THRE errata
- Dropped our fix since the "8250-uart-backup-timer.patch" in the "mm"
tree also fixes it. This patch needs to be applied on top of it.

2. Fix for Busy Detect on LCR write
- Dropped the addition of UPIO_DWAPB iotype to 8250_early.c as Sergei
pointed out the fix wasn't complete and we don't require it.

3. Workaround for interrupt/data concurrency issue
- Fix must remain serial8250_interrupt() in order to mark interrupt as
handled.

Thanks,
Marc

Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 3d91bfc..489ff2b 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
  		return inb(up->port.iobase + 1);

  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		return readb(up->port.membase + offset);

  	case UPIO_MEM32:
@@ -333,6 +334,8 @@ #endif
  static void
  serial_out(struct uart_8250_port *up, int offset, int value)
  {
+	/* Save the offset before it's remapped */
+	int save_offset = offset;
  	offset = map_8250_out_reg(up, offset) << up->port.regshift;

  	switch (up->port.iotype) {
@@ -359,6 +362,18 @@ #endif
  			writeb(value, up->port.membase + offset);
  		break;

+	case UPIO_DWAPB:
+		/* Save the LCR value so it can be re-written when a
+		 * Busy Detect interrupt occurs. */
+		if (save_offset == UART_LCR)
+			up->lcr = value;
+		writeb(value, up->port.membase + offset);
+		/* Read the IER to ensure any interrupt is cleared before
+		 * returning from ISR. */
+		if ((save_offset == UART_TX || save_offset == UART_IER) && in_irq())
+			value = serial_in(up, UART_IER);
+		break;
+		
  	default:
  		outb(value, up->port.iobase + offset);
  	}
@@ -373,6 +388,7 @@ serial_out_sync(struct uart_8250_port *u
  #ifdef CONFIG_SERIAL_8250_AU1X00
  	case UPIO_AU:
  #endif
+	case UPIO_DWAPB:
  		serial_out(up, offset, value);
  		(void)serial_in(up, UART_LCR); /* safe, no side-effects */
  		break;
@@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
  			handled = 1;

  			end = NULL;
+		} else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY &&
+				up->port.iotype == UPIO_DWAPB) {
+			/* The DesignWare APB UART has an Busy Detect (0x07)
+			 * interrupt meaning an LCR write attempt occured while the
+			 * UART was busy. The interrupt must be cleared by reading
+			 * the UART status register (USR) and the LCR re-written. */
+			unsigned int status;
+			status = *(volatile u32 *)up->port.data;
+			serial_out(up, UART_LCR, up->lcr);
+
+			handled = 1;
+
+			end = NULL;
  		} else if (end == NULL)
  			end = l;

@@ -2085,6 +2114,7 @@ static int serial8250_request_std_resour
  	case UPIO_TSI:
  	case UPIO_MEM32:
  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		if (!up->port.mapbase)
  			break;

@@ -2122,6 +2152,7 @@ static void serial8250_release_std_resou
  	case UPIO_TSI:
  	case UPIO_MEM32:
  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		if (!up->port.mapbase)
  			break;

@@ -2454,9 +2485,12 @@ int __init serial8250_start_console(stru

  	add_preferred_console("ttyS", line, options);
  	printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
-		line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
-		port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
-		    (unsigned long) port->iobase, options);
+		line,
+		(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
+			? "MMIO" : "I/O port",
+		(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
+			? (unsigned long) port->mapbase : (unsigned long) port->iobase,
+		options);
  	if (!(serial8250_console.flags & CON_ENABLED)) {
  		serial8250_console.flags &= ~CON_PRINTBUFFER;
  		register_console(&serial8250_console);
diff --git a/drivers/serial/8250.h b/drivers/serial/8250.h
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index f84982e..f5b6036 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2057,6 +2057,7 @@ uart_report_port(struct uart_driver *drv
  	case UPIO_MEM32:
  	case UPIO_AU:
  	case UPIO_TSI:
+	case UPIO_DWAPB:
  		snprintf(address, sizeof(address),
  			 "MMIO 0x%lx", port->mapbase);
  		break;
@@ -2401,6 +2402,7 @@ int uart_match_port(struct uart_port *po
  	case UPIO_MEM32:
  	case UPIO_AU:
  	case UPIO_TSI:
+	case UPIO_DWAPB:
  		return (port1->mapbase == port2->mapbase);
  	}
  	return 0;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cf23813..9cb9b3b 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -230,6 +230,7 @@ #define UPIO_MEM		(2)
  #define UPIO_MEM32		(3)
  #define UPIO_AU			(4)			/* Au1x00 type IO */
  #define UPIO_TSI		(5)			/* Tsi108/109 type IO */
+#define UPIO_DWAPB		(6)			/* DesignWare APB UART */

  	unsigned int		read_status_mask;	/* driver specific */
  	unsigned int		ignore_status_mask;	/* driver specific */
@@ -276,6 +277,7 @@ #define UPF_USR_MASK		((__force upf_t) (
  	struct device		*dev;			/* parent device */
  	unsigned char		hub6;			/* this should be in the 8250 driver */
  	unsigned char		unused[3];
+	void				*data;			/* generic platform data pointer */
  };

  /*
diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 3c8a6aa..b3550cc 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -37,6 +37,7 @@ #define UART_IIR_MSI		0x00 /* Modem stat
  #define UART_IIR_THRI		0x02 /* Transmitter holding register empty */
  #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
  #define UART_IIR_RLSI		0x06 /* Receiver line status interrupt */
+#define UART_IIR_BUSY		0x07 /* DesignWare APB Busy Detect */

  #define UART_FCR	2	/* Out: FIFO Control Register */
  #define UART_FCR_ENABLE_FIFO	0x01 /* Enable the FIFO */

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Alan]

>   	unsigned char		hub6;			/* this should be in the 8250 driver */
>   	unsigned char		unused[3];
> +	void				*data;			/* generic platform data pointer */
>   };

Convention is "private_data"

>
>   /*
> diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
> index 3c8a6aa..b3550cc 100644
> --- a/include/linux/serial_reg.h
> +++ b/include/linux/serial_reg.h
> @@ -37,6 +37,7 @@ #define UART_IIR_MSI		0x00 /* Modem stat
>   #define UART_IIR_THRI		0x02 /* Transmitter holding register empty */
>   #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
>   #define UART_IIR_RLSI		0x06 /* Receiver line status interrupt */
> +#define UART_IIR_BUSY		0x07 /* DesignWare APB Busy Detect */

Please move this down a line to break it from "official" values and call
it DESIGNWARE_UART_IIR_BUSY, so it is obviously designware specific.

Otherwise looks much less invasive and messy

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Sergei Shtylyov]

Marc St-Jean wrote:
> Third attempt at the serial driver patch for the PMC-Sierra MSP71xx device.
> 
> There are three different fixes:
> 1. Fix for DesignWare THRE errata
> - Dropped our fix since the "8250-uart-backup-timer.patch" in the "mm"
> tree also fixes it. This patch needs to be applied on top of it.
> 
> 2. Fix for Busy Detect on LCR write
> - Dropped the addition of UPIO_DWAPB iotype to 8250_early.c as Sergei
> pointed out the fix wasn't complete and we don't require it.
> 
> 3. Workaround for interrupt/data concurrency issue
> - Fix must remain serial8250_interrupt() in order to mark interrupt as
> handled.
> 
> Thanks,
> Marc
> 
> Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>
> 
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index 3d91bfc..489ff2b 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
>   		return inb(up->port.iobase + 1);
> 
>   	case UPIO_MEM:
> +	case UPIO_DWAPB:
>   		return readb(up->port.membase + offset);
> 
>   	case UPIO_MEM32:
> @@ -333,6 +334,8 @@ #endif
>   static void
>   serial_out(struct uart_8250_port *up, int offset, int value)
>   {
> +	/* Save the offset before it's remapped */
> +	int save_offset = offset;
>   	offset = map_8250_out_reg(up, offset) << up->port.regshift;
> 
>   	switch (up->port.iotype) {
> @@ -359,6 +362,18 @@ #endif
>   			writeb(value, up->port.membase + offset);
>   		break;
> 
> +	case UPIO_DWAPB:
> +		/* Save the LCR value so it can be re-written when a
> +		 * Busy Detect interrupt occurs. */
> +		if (save_offset == UART_LCR)
> +			up->lcr = value;
> +		writeb(value, up->port.membase + offset);
> +		/* Read the IER to ensure any interrupt is cleared before
> +		 * returning from ISR. */
> +		if ((save_offset == UART_TX || save_offset == UART_IER) && in_irq())
> +			value = serial_in(up, UART_IER);
> +		break;
> +		
>   	default:
>   		outb(value, up->port.iobase + offset);
>   	}
> @@ -373,6 +388,7 @@ serial_out_sync(struct uart_8250_port *u
>   #ifdef CONFIG_SERIAL_8250_AU1X00
>   	case UPIO_AU:
>   #endif
> +	case UPIO_DWAPB:
>   		serial_out(up, offset, value);
>   		(void)serial_in(up, UART_LCR); /* safe, no side-effects */
>   		break;
> @@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
>   			handled = 1;
> 
>   			end = NULL;
> +		} else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY &&
> +				up->port.iotype == UPIO_DWAPB) {

    Makes sense to swap the checks, i.e. to only test for UART_IIR_BUSY is 
this is UPIO_DWAPB.

> +			/* The DesignWare APB UART has an Busy Detect (0x07)
> +			 * interrupt meaning an LCR write attempt occured while the
> +			 * UART was busy. The interrupt must be cleared by reading
> +			 * the UART status register (USR) and the LCR re-written. */
> +			unsigned int status;
> +			status = *(volatile u32 *)up->port.data;
> +			serial_out(up, UART_LCR, up->lcr);
> +
> +			handled = 1;
> +
> +			end = NULL;
>   		} else if (end == NULL)
>   			end = l;

[...]

> @@ -2454,9 +2485,12 @@ int __init serial8250_start_console(stru
> 
>   	add_preferred_console("ttyS", line, options);
>   	printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
> -		line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
> -		port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
> -		    (unsigned long) port->iobase, options);
> +		line,
> +		(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
> +			? "MMIO" : "I/O port",
> +		(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
> +			? (unsigned long) port->mapbase : (unsigned long) port->iobase,
> +		options);

    Please turn this check into port->iotype >= UPIO_MEM, since this would be 
the Right Thing (RM).  All iotypes beyond UPIO_MEM are memory mapped.  And I 
thought I fixed that -- was wrong, obviously... :-/

> diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
> index 3c8a6aa..b3550cc 100644
> --- a/include/linux/serial_reg.h
> +++ b/include/linux/serial_reg.h
> @@ -37,6 +37,7 @@ #define UART_IIR_MSI		0x00 /* Modem stat
>   #define UART_IIR_THRI		0x02 /* Transmitter holding register empty */
>   #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
>   #define UART_IIR_RLSI		0x06 /* Receiver line status interrupt */
> +#define UART_IIR_BUSY		0x07 /* DesignWare APB Busy Detect */

    Alan already said about this one... :-)

    BTW, your patches are still corrupt by your mailer (space added to lines 
starting with space)

MBR, Sergei

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

Serial driver patch for the PMC-Sierra MSP71xx devices.

There are three different fixes:
1. Fix for DesignWare APB THRE errata:
In brief, this is a non-standard 16550 in that the THRE interrupt
will not re-assert itself simply by disabling and re-enabling the
THRI bit in the IER, it is only re-enabled if a character is actually
sent out.
It appears that the "8250-uart-backup-timer.patch" in the "mm" tree also
fixes it so we have dropped our initial workaround.
This patch now needs to be applied on top of that "mm" patch.

2. Fix for Busy Detect on LCR write:
The DesignWare APB UART has a feature which causes a new Busy Detect
interrupt to be generated if it's busy when the LCR is written. This
fix saves the value of the LCR and rewrites it after clearing the
interrupt.

3. Workaround for interrupt/data concurrency issue:
The SoC needs to ensure that writes that can cause interrupts to be
cleared reach the UART before returning from the ISR. This fix reads
a non-destructive register on the UART so the read transaction
completion ensures the previously queued write transaction has
also completed.

Thanks,
Marc

Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>
---

Try number eigth, changes since last patch:
-Dropped fix to serial8250_start_console for port->iotype >= UPIO_MEM
since we don't absolutely need this.
-Added platform file initializing the serial ports to patch.

 arch/mips/pmc-sierra/msp71xx/msp_serial.c |  165 ++++++++++++++++++++++++++++++
 drivers/serial/8250.c                     |   31 +++++
 drivers/serial/8250.h                     |    0 
 drivers/serial/serial_core.c              |    2 
 include/linux/serial_core.h               |    2 
 include/linux/serial_reg.h                |    2 
 6 files changed, 202 insertions(+)

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 3d91bfc..d224c35 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
 		return inb(up->port.iobase + 1);
 
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		return readb(up->port.membase + offset);
 
 	case UPIO_MEM32:
@@ -333,6 +334,8 @@ #endif
 static void
 serial_out(struct uart_8250_port *up, int offset, int value)
 {
+	/* Save the offset before it's remapped */
+	int save_offset = offset;
 	offset = map_8250_out_reg(up, offset) << up->port.regshift;
 
 	switch (up->port.iotype) {
@@ -359,6 +362,18 @@ #endif
 			writeb(value, up->port.membase + offset);
 		break;
 
+	case UPIO_DWAPB:
+		/* Save the LCR value so it can be re-written when a
+		 * Busy Detect interrupt occurs. */
+		if (save_offset == UART_LCR)
+			up->lcr = value;
+		writeb(value, up->port.membase + offset);
+		/* Read the IER to ensure any interrupt is cleared before
+		 * returning from ISR. */
+		if (save_offset == UART_TX || save_offset == UART_IER)
+			value = serial_in(up, UART_IER);
+		break;
+		
 	default:
 		outb(value, up->port.iobase + offset);
 	}
@@ -373,6 +388,7 @@ serial_out_sync(struct uart_8250_port *u
 #ifdef CONFIG_SERIAL_8250_AU1X00
 	case UPIO_AU:
 #endif
+	case UPIO_DWAPB:
 		serial_out(up, offset, value);
 		(void)serial_in(up, UART_LCR); /* safe, no side-effects */
 		break;
@@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
 			handled = 1;
 
 			end = NULL;
+		} else if (up->port.iotype == UPIO_DWAPB &&
+			  (iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
+			/* The DesignWare APB UART has an Busy Detect (0x07)
+			 * interrupt meaning an LCR write attempt occured while the
+			 * UART was busy. The interrupt must be cleared by reading
+			 * the UART status register (USR) and the LCR re-written. */
+			unsigned int status;
+			status = *(volatile u32 *)up->port.private_data;
+			serial_out(up, UART_LCR, up->lcr);
+
+			handled = 1;
+
+			end = NULL;
 		} else if (end == NULL)
 			end = l;
 
@@ -2085,6 +2114,7 @@ static int serial8250_request_std_resour
 	case UPIO_TSI:
 	case UPIO_MEM32:
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		if (!up->port.mapbase)
 			break;
 
@@ -2122,6 +2152,7 @@ static void serial8250_release_std_resou
 	case UPIO_TSI:
 	case UPIO_MEM32:
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		if (!up->port.mapbase)
 			break;
 
diff --git a/drivers/serial/8250.h b/drivers/serial/8250.h
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index f84982e..f5b6036 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2057,6 +2057,7 @@ uart_report_port(struct uart_driver *drv
 	case UPIO_MEM32:
 	case UPIO_AU:
 	case UPIO_TSI:
+	case UPIO_DWAPB:
 		snprintf(address, sizeof(address),
 			 "MMIO 0x%lx", port->mapbase);
 		break;
@@ -2401,6 +2402,7 @@ int uart_match_port(struct uart_port *po
 	case UPIO_MEM32:
 	case UPIO_AU:
 	case UPIO_TSI:
+	case UPIO_DWAPB:
 		return (port1->mapbase == port2->mapbase);
 	}
 	return 0;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cf23813..8cdd326 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -230,6 +230,7 @@ #define UPIO_MEM		(2)
 #define UPIO_MEM32		(3)
 #define UPIO_AU			(4)			/* Au1x00 type IO */
 #define UPIO_TSI		(5)			/* Tsi108/109 type IO */
+#define UPIO_DWAPB		(6)			/* DesignWare APB UART */
 
 	unsigned int		read_status_mask;	/* driver specific */
 	unsigned int		ignore_status_mask;	/* driver specific */
@@ -276,6 +277,7 @@ #define UPF_USR_MASK		((__force upf_t) (
 	struct device		*dev;			/* parent device */
 	unsigned char		hub6;			/* this should be in the 8250 driver */
 	unsigned char		unused[3];
+	void			*private_data;		/* generic platform data pointer */
 };
 
 /*
diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 3c8a6aa..1c5ed7d 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -38,6 +38,8 @@ #define UART_IIR_THRI		0x02 /* Transmitt
 #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
 #define UART_IIR_RLSI		0x06 /* Receiver line status interrupt */
 
+#define UART_IIR_BUSY		0x07 /* DesignWare APB Busy Detect */
+
 #define UART_FCR	2	/* Out: FIFO Control Register */
 #define UART_FCR_ENABLE_FIFO	0x01 /* Enable the FIFO */
 #define UART_FCR_CLEAR_RCVR	0x02 /* Clear the RCVR FIFO */
diff --git a/arch/mips/pmc-sierra/msp71xx/msp_serial.c b/arch/mips/pmc-sierra/msp71xx/msp_serial.c
new file mode 100644
index 0000000..c1afa90
--- /dev/null
+++ b/arch/mips/pmc-sierra/msp71xx/msp_serial.c
@@ -0,0 +1,165 @@
+/*
+ * The setup file for serial related hardware on PMC-Sierra MSP processors.
+ *
+ * Copyright 2005 PMC-Sierra, Inc.
+ *
+ *  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 (at your
+ *  option) any later version.
+ *
+ *  THIS  SOFTWARE  IS PROVIDED   ``AS  IS'' AND   ANY  EXPRESS OR IMPLIED
+ *  WARRANTIES,   INCLUDING, BUT NOT  LIMITED  TO, THE IMPLIED WARRANTIES OF
+ *  MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN
+ *  NO  EVENT  SHALL   THE AUTHOR  BE    LIABLE FOR ANY   DIRECT, INDIRECT,
+ *  INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ *  NOT LIMITED   TO, PROCUREMENT OF  SUBSTITUTE GOODS  OR SERVICES; LOSS OF
+ *  USE, DATA,  OR PROFITS; OR  BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ *  ANY THEORY OF LIABILITY, WHETHER IN  CONTRACT, STRICT LIABILITY, OR TORT
+ *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ *  THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ *  You should have received a copy of the  GNU General Public License along
+ *  with this program; if not, write  to the Free Software Foundation, Inc.,
+ *  675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/serial.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+
+#include <asm/bootinfo.h>
+#include <asm/io.h>
+#include <asm/processor.h>
+#include <asm/serial.h>
+
+#include <msp_prom.h>
+#include <msp_int.h>
+#include <msp_regs.h>
+
+#ifdef CONFIG_KGDB
+/*
+ * kgdb uses serial port 1 so the console can remain on port 0.
+ * To use port 0 change the definition to read as follows:
+ * #define DEBUG_PORT_BASE KSEG1ADDR(MSP_UART0_BASE)
+ */
+#define DEBUG_PORT_BASE KSEG1ADDR(MSP_UART1_BASE)
+
+int putDebugChar(char c)
+{
+	volatile uint32_t *uart = (volatile uint32_t *)DEBUG_PORT_BASE;
+	uint32_t val = (uint32_t)c;
+	
+	local_irq_disable();
+	while( !(uart[5] & 0x20) ); /* Wait for TXRDY */
+	uart[0] = val;
+	while( !(uart[5] & 0x20) ); /* Wait for TXRDY */
+	local_irq_enable();
+	
+	return 1;
+}
+
+char getDebugChar(void)
+{
+	volatile uint32_t *uart = (volatile uint32_t *)DEBUG_PORT_BASE;
+	uint32_t val;
+	
+	while( !(uart[5] & 0x01) ); /* Wait for RXRDY */
+	val = uart[0];
+	
+	return (char)val;
+}
+
+void initDebugPort(unsigned int uartclk, unsigned int baudrate)
+{
+	unsigned int baud_divisor = (uartclk + 8 * baudrate)/(16 * baudrate);
+	
+	/* Enable FIFOs */
+	writeb(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
+			UART_FCR_CLEAR_XMIT | UART_FCR_TRIGGER_4,
+		(char *)DEBUG_PORT_BASE + (UART_FCR * 4));
+	
+	/* Select brtc divisor */
+	writeb(UART_LCR_DLAB, (char *)DEBUG_PORT_BASE + (UART_LCR * 4));
+	
+	/* Store divisor lsb */
+	writeb(baud_divisor, (char *)DEBUG_PORT_BASE + (UART_TX * 4));
+
+	/* Store divisor msb */
+	writeb(baud_divisor >> 8, (char *)DEBUG_PORT_BASE + (UART_IER * 4));
+	
+	/* Set 8N1 mode */
+	writeb(UART_LCR_WLEN8, (char *)DEBUG_PORT_BASE + (UART_LCR * 4));
+	
+	/* Disable flow control */
+	writeb(0, (char *)DEBUG_PORT_BASE + (UART_MCR * 4));
+	
+	/* Disable receive interrupt(!) */
+	writeb(0, (char *)DEBUG_PORT_BASE + (UART_IER * 4));
+}
+#endif
+
+void __init msp_serial_setup(void)
+{
+	char    *s;
+	char    *endp;
+	struct uart_port up;
+	unsigned int uartclk;
+
+	memset(&up, 0, sizeof(up));
+
+	/* Check if clock was specified in environment */
+	s = prom_getenv("uartfreqhz");
+	if(!(s && *s && (uartclk = simple_strtoul(s, &endp, 10)) && *endp == 0))
+		uartclk = MSP_BASE_BAUD;
+	ppfinit("UART clock set to %d\n", uartclk);
+
+	/* Initialize first serial port */
+	up.mapbase      = MSP_UART0_BASE;
+	up.membase      = ioremap_nocache(up.mapbase,MSP_UART_REG_LEN);
+	up.irq          = MSP_INT_UART0;
+	up.uartclk      = uartclk;
+	up.regshift     = 2;
+	up.iotype       = UPIO_DWAPB; /* UPIO_MEM like */
+	up.flags        = STD_COM_FLAGS;
+	up.type         = PORT_16550A;
+	up.line         = 0;
+	up.private_data		= (void*)UART0_STATUS_REG;
+	if (early_serial_setup(&up))
+		printk(KERN_ERR "Early serial init of port 0 failed\n");
+
+	/* Initialize the second serial port, if one exists */
+	switch (mips_machtype) {
+		case MACH_MSP4200_EVAL:
+		case MACH_MSP4200_GW:
+		case MACH_MSP4200_FPGA:
+		case MACH_MSP7120_EVAL:
+		case MACH_MSP7120_GW:
+		case MACH_MSP7120_FPGA:
+			/* Enable UART1 on MSP4200 and MSP7120 */
+			*GPIO_CFG2_REG = 0x00002299;
+	
+#ifdef CONFIG_KGDB
+			/* Initialize UART1 for kgdb since PMON doesn't */
+			if( DEBUG_PORT_BASE == KSEG1ADDR(MSP_UART1_BASE) ) {
+				if( mips_machtype == MACH_MSP4200_FPGA
+				 || mips_machtype == MACH_MSP7120_FPGA )
+					initDebugPort(uartclk,19200);
+				else
+					initDebugPort(uartclk,57600);
+			}
+#endif
+			break;
+
+		default:
+			return; /* No second serial port, good-bye. */
+	}
+
+	up.mapbase      = MSP_UART1_BASE;
+	up.membase      = ioremap_nocache(up.mapbase,MSP_UART_REG_LEN);
+	up.irq          = MSP_INT_UART1;
+	up.line         = 1;
+	up.private_data		= (void*)UART1_STATUS_REG;
+	if (early_serial_setup(&up))
+		printk(KERN_ERR "Early serial init of port 1 failed\n");
+}

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

Seventh attempt at the serial driver patch for the PMC-Sierra
MSP71xx devices.

There are three different fixes:
1. Fix for DesignWare APB THRE errata:
In brief, this is a non-standard 16550 in that the THRE interrupt
will not re-assert itself simply by disabling and re-enabling the
THRI bit in the IER, it is only re-enabled if a character is actually
sent out.
It appears that the "8250-uart-backup-timer.patch" in the "mm" tree also
fixes it so we have dropped our initial workaround.
This patch now needs to be applied on top of that "mm" patch.

2. Fix for Busy Detect on LCR write:
The DesignWare APB UART has a feature which causes a new Busy Detect
interrupt to be generated if it's busy when the LCR is written. This
fix saves the value of the LCR and rewrites it after clearing the
interrupt.

3. Workaround for interrupt/data concurrency issue:
The SoC needs to ensure that writes that can cause interrupts to be
cleared reach the UART before returning from the ISR. This fix reads
a non-destructive register on the UART so the read transaction
completion ensures the previously queued write transaction has
also completed.


The differences from the last attempt is dropping the call to
in_irq() and including more detailed description of the fixes.

Thanks,
Marc

Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>


diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 3d91bfc..bfaacc5 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
 		return inb(up->port.iobase + 1);
 
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		return readb(up->port.membase + offset);
 
 	case UPIO_MEM32:
@@ -333,6 +334,8 @@ #endif
 static void
 serial_out(struct uart_8250_port *up, int offset, int value)
 {
+	/* Save the offset before it's remapped */
+	int save_offset = offset;
 	offset = map_8250_out_reg(up, offset) << up->port.regshift;
 
 	switch (up->port.iotype) {
@@ -359,6 +362,18 @@ #endif
 			writeb(value, up->port.membase + offset);
 		break;
 
+	case UPIO_DWAPB:
+		/* Save the LCR value so it can be re-written when a
+		 * Busy Detect interrupt occurs. */
+		if (save_offset == UART_LCR)
+			up->lcr = value;
+		writeb(value, up->port.membase + offset);
+		/* Read the IER to ensure any interrupt is cleared before
+		 * returning from ISR. */
+		if (save_offset == UART_TX || save_offset == UART_IER)
+			value = serial_in(up, UART_IER);
+		break;
+		
 	default:
 		outb(value, up->port.iobase + offset);
 	}
@@ -373,6 +388,7 @@ serial_out_sync(struct uart_8250_port *u
 #ifdef CONFIG_SERIAL_8250_AU1X00
 	case UPIO_AU:
 #endif
+	case UPIO_DWAPB:
 		serial_out(up, offset, value);
 		(void)serial_in(up, UART_LCR); /* safe, no side-effects */
 		break;
@@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
 			handled = 1;
 
 			end = NULL;
+		} else if (up->port.iotype == UPIO_DWAPB &&
+			  (iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
+			/* The DesignWare APB UART has an Busy Detect (0x07)
+			 * interrupt meaning an LCR write attempt occured while the
+			 * UART was busy. The interrupt must be cleared by reading
+			 * the UART status register (USR) and the LCR re-written. */
+			unsigned int status;
+			status = *(volatile u32 *)up->port.private_data;
+			serial_out(up, UART_LCR, up->lcr);
+
+			handled = 1;
+
+			end = NULL;
 		} else if (end == NULL)
 			end = l;
 
@@ -2085,6 +2114,7 @@ static int serial8250_request_std_resour
 	case UPIO_TSI:
 	case UPIO_MEM32:
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		if (!up->port.mapbase)
 			break;
 
@@ -2122,6 +2152,7 @@ static void serial8250_release_std_resou
 	case UPIO_TSI:
 	case UPIO_MEM32:
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		if (!up->port.mapbase)
 			break;
 
@@ -2454,8 +2485,8 @@ int __init serial8250_start_console(stru
 
 	add_preferred_console("ttyS", line, options);
 	printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
-		line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
-		port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
+		line, port->iotype >= UPIO_MEM ? "MMIO" : "I/O port",
+		port->iotype >= UPIO_MEM ? (unsigned long) port->mapbase :
 		    (unsigned long) port->iobase, options);
 	if (!(serial8250_console.flags & CON_ENABLED)) {
 		serial8250_console.flags &= ~CON_PRINTBUFFER;
diff --git a/drivers/serial/8250.h b/drivers/serial/8250.h
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index f84982e..f5b6036 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2057,6 +2057,7 @@ uart_report_port(struct uart_driver *drv
 	case UPIO_MEM32:
 	case UPIO_AU:
 	case UPIO_TSI:
+	case UPIO_DWAPB:
 		snprintf(address, sizeof(address),
 			 "MMIO 0x%lx", port->mapbase);
 		break;
@@ -2401,6 +2402,7 @@ int uart_match_port(struct uart_port *po
 	case UPIO_MEM32:
 	case UPIO_AU:
 	case UPIO_TSI:
+	case UPIO_DWAPB:
 		return (port1->mapbase == port2->mapbase);
 	}
 	return 0;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cf23813..8cdd326 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -230,6 +230,7 @@ #define UPIO_MEM		(2)
 #define UPIO_MEM32		(3)
 #define UPIO_AU			(4)			/* Au1x00 type IO */
 #define UPIO_TSI		(5)			/* Tsi108/109 type IO */
+#define UPIO_DWAPB		(6)			/* DesignWare APB UART */
 
 	unsigned int		read_status_mask;	/* driver specific */
 	unsigned int		ignore_status_mask;	/* driver specific */
@@ -276,6 +277,7 @@ #define UPF_USR_MASK		((__force upf_t) (
 	struct device		*dev;			/* parent device */
 	unsigned char		hub6;			/* this should be in the 8250 driver */
 	unsigned char		unused[3];
+	void			*private_data;		/* generic platform data pointer */
 };
 
 /*
diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 3c8a6aa..1c5ed7d 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -38,6 +38,8 @@ #define UART_IIR_THRI		0x02 /* Transmitt
 #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
 #define UART_IIR_RLSI		0x06 /* Receiver line status interrupt */
 
+#define UART_IIR_BUSY		0x07 /* DesignWare APB Busy Detect */
+
 #define UART_FCR	2	/* Out: FIFO Control Register */
 #define UART_FCR_ENABLE_FIFO	0x01 /* Enable the FIFO */
 #define UART_FCR_CLEAR_RCVR	0x02 /* Clear the RCVR FIFO */

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Sergei Shtylyov]

Hello.

Marc St-Jean wrote:

> There are three different fixes:
> 1. Fix for DesignWare APB THRE errata:
> In brief, this is a non-standard 16550 in that the THRE interrupt
> will not re-assert itself simply by disabling and re-enabling the
> THRI bit in the IER, it is only re-enabled if a character is actually
> sent out.
> It appears that the "8250-uart-backup-timer.patch" in the "mm" tree also
> fixes it so we have dropped our initial workaround.
> This patch now needs to be applied on top of that "mm" patch.

    This patch has hit the mainline kernel already.

> 2. Fix for Busy Detect on LCR write:
> The DesignWare APB UART has a feature which causes a new Busy Detect
> interrupt to be generated if it's busy when the LCR is written. This
> fix saves the value of the LCR and rewrites it after clearing the
> interrupt.

> 3. Workaround for interrupt/data concurrency issue:
> The SoC needs to ensure that writes that can cause interrupts to be
> cleared reach the UART before returning from the ISR. This fix reads
> a non-destructive register on the UART so the read transaction
> completion ensures the previously queued write transaction has
> also completed.

> The differences from the last attempt is dropping the call to
> in_irq() and including more detailed description of the fixes.

    The difference part would better be under the "---" cutoff line, along 
with diffstat.
This way it gets auto-removed by quilt/git when applying the patch.

> Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>

> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index 3d91bfc..bfaacc5 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
>  		return inb(up->port.iobase + 1);
>  
>  	case UPIO_MEM:
> +	case UPIO_DWAPB:
>  		return readb(up->port.membase + offset);
>  
>  	case UPIO_MEM32:
> @@ -333,6 +334,8 @@ #endif
>  static void
>  serial_out(struct uart_8250_port *up, int offset, int value)
>  {
> +	/* Save the offset before it's remapped */
> +	int save_offset = offset;
>  	offset = map_8250_out_reg(up, offset) << up->port.regshift;
>  
>  	switch (up->port.iotype) {

    I've just got an idea how to "beautify" this code -- rename the 'offset' 
arg to something like reg, and then declare/init 'offset' as local variable.
Would certainly look better (and worth doing in all serial_* accessros).

> @@ -359,6 +362,18 @@ #endif
>  			writeb(value, up->port.membase + offset);
>  		break;
>  
> +	case UPIO_DWAPB:
> +		/* Save the LCR value so it can be re-written when a
> +		 * Busy Detect interrupt occurs. */
> +		if (save_offset == UART_LCR)
> +			up->lcr = value;
> +		writeb(value, up->port.membase + offset);
> +		/* Read the IER to ensure any interrupt is cleared before
> +		 * returning from ISR. */
> +		if (save_offset == UART_TX || save_offset == UART_IER)
> +			value = serial_in(up, UART_IER);
> +		break;
> +		
>  	default:
>  		outb(value, up->port.iobase + offset);
>  	}
> @@ -2454,8 +2485,8 @@ int __init serial8250_start_console(stru
>  
>  	add_preferred_console("ttyS", line, options);
>  	printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
> -		line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
> -		port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
> +		line, port->iotype >= UPIO_MEM ? "MMIO" : "I/O port",
> +		port->iotype >= UPIO_MEM ? (unsigned long) port->mapbase :
>  		    (unsigned long) port->iobase, options);
>  	if (!(serial8250_console.flags & CON_ENABLED)) {
>  		serial8250_console.flags &= ~CON_PRINTBUFFER;

    I've remembered why I decided not to fix it: this code only gets called 
from 8250__eraly.c which can't handle anything beyond UPIO_MEM anyway.

WBR, Sergei

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Andrew Morton]

On Thu, 15 Feb 2007 13:26:29 -0600
Marc St-Jean <stjeanma@pmc-sierra.com> wrote:

> +			status = *(volatile u32 *)up->port.private_data;

It distresses me that this patch uses a variable which this patch
doesn't initialise anywhere.  It isn't complete.

The sub-driver code whch sets up this field shuld be included in the
patch, no?

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Sergei Shtylyov]

Hello.

Andrew Morton wrote:

>>+			status = *(volatile u32 *)up->port.private_data;

> It distresses me that this patch uses a variable which this patch
> doesn't initialise anywhere.  It isn't complete.

    I assume this gets passed via early_serial_setup(). Marc?

> The sub-driver code whch sets up this field shuld be included in the
> patch, no?

    Hardly so, this code (not a subdriver) resides under arch/mips/ I think.

WBR, Sergei

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

Sixth attempt at the serial driver patch for the PMC-Sierra MSP71xx device.
Identical to last patch but respun for 8 character tabs.

There are three different fixes:
1. Fix for DesignWare THRE errata
- Dropped our fix since the "8250-uart-backup-timer.patch" in the "mm"
tree also fixes it. This patch needs to be applied on top of "mm" patch.

2. Fix for Busy Detect on LCR write
- No changes since last patch.

3. Workaround for interrupt/data concurrency issue
- No changes since last patch.

Andrew, I'm emailing directly as suggested on the l-m.o list. Please let
me know if all requirements are met.

Thanks,
Marc

Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 3d91bfc..2f89dc4 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
 		return inb(up->port.iobase + 1);
 
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		return readb(up->port.membase + offset);
 
 	case UPIO_MEM32:
@@ -333,6 +334,8 @@ #endif
 static void
 serial_out(struct uart_8250_port *up, int offset, int value)
 {
+	/* Save the offset before it's remapped */
+	int save_offset = offset;
 	offset = map_8250_out_reg(up, offset) << up->port.regshift;
 
 	switch (up->port.iotype) {
@@ -359,6 +362,18 @@ #endif
 			writeb(value, up->port.membase + offset);
 		break;
 
+	case UPIO_DWAPB:
+		/* Save the LCR value so it can be re-written when a
+		 * Busy Detect interrupt occurs. */
+		if (save_offset == UART_LCR)
+			up->lcr = value;
+		writeb(value, up->port.membase + offset);
+		/* Read the IER to ensure any interrupt is cleared before
+		 * returning from ISR. */
+		if ((save_offset == UART_TX || save_offset == UART_IER) && in_irq())
+			value = serial_in(up, UART_IER);
+		break;
+		
 	default:
 		outb(value, up->port.iobase + offset);
 	}
@@ -373,6 +388,7 @@ serial_out_sync(struct uart_8250_port *u
 #ifdef CONFIG_SERIAL_8250_AU1X00
 	case UPIO_AU:
 #endif
+	case UPIO_DWAPB:
 		serial_out(up, offset, value);
 		(void)serial_in(up, UART_LCR); /* safe, no side-effects */
 		break;
@@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
 			handled = 1;
 
 			end = NULL;
+		} else if (up->port.iotype == UPIO_DWAPB &&
+			  (iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
+			/* The DesignWare APB UART has an Busy Detect (0x07)
+			 * interrupt meaning an LCR write attempt occured while the
+			 * UART was busy. The interrupt must be cleared by reading
+			 * the UART status register (USR) and the LCR re-written. */
+			unsigned int status;
+			status = *(volatile u32 *)up->port.private_data;
+			serial_out(up, UART_LCR, up->lcr);
+
+			handled = 1;
+
+			end = NULL;
 		} else if (end == NULL)
 			end = l;
 
@@ -2085,6 +2114,7 @@ static int serial8250_request_std_resour
 	case UPIO_TSI:
 	case UPIO_MEM32:
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		if (!up->port.mapbase)
 			break;
 
@@ -2122,6 +2152,7 @@ static void serial8250_release_std_resou
 	case UPIO_TSI:
 	case UPIO_MEM32:
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		if (!up->port.mapbase)
 			break;
 
@@ -2454,8 +2485,8 @@ int __init serial8250_start_console(stru
 
 	add_preferred_console("ttyS", line, options);
 	printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
-		line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
-		port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
+		line, port->iotype >= UPIO_MEM ? "MMIO" : "I/O port",
+		port->iotype >= UPIO_MEM ? (unsigned long) port->mapbase :
 		    (unsigned long) port->iobase, options);
 	if (!(serial8250_console.flags & CON_ENABLED)) {
 		serial8250_console.flags &= ~CON_PRINTBUFFER;
diff --git a/drivers/serial/8250.h b/drivers/serial/8250.h
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index f84982e..f5b6036 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2057,6 +2057,7 @@ uart_report_port(struct uart_driver *drv
 	case UPIO_MEM32:
 	case UPIO_AU:
 	case UPIO_TSI:
+	case UPIO_DWAPB:
 		snprintf(address, sizeof(address),
 			 "MMIO 0x%lx", port->mapbase);
 		break;
@@ -2401,6 +2402,7 @@ int uart_match_port(struct uart_port *po
 	case UPIO_MEM32:
 	case UPIO_AU:
 	case UPIO_TSI:
+	case UPIO_DWAPB:
 		return (port1->mapbase == port2->mapbase);
 	}
 	return 0;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cf23813..8cdd326 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -230,6 +230,7 @@ #define UPIO_MEM		(2)
 #define UPIO_MEM32		(3)
 #define UPIO_AU			(4)			/* Au1x00 type IO */
 #define UPIO_TSI		(5)			/* Tsi108/109 type IO */
+#define UPIO_DWAPB		(6)			/* DesignWare APB UART */
 
 	unsigned int		read_status_mask;	/* driver specific */
 	unsigned int		ignore_status_mask;	/* driver specific */
@@ -276,6 +277,7 @@ #define UPF_USR_MASK		((__force upf_t) (
 	struct device		*dev;			/* parent device */
 	unsigned char		hub6;			/* this should be in the 8250 driver */
 	unsigned char		unused[3];
+	void			*private_data;		/* generic platform data pointer */
 };
 
 /*
diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 3c8a6aa..1c5ed7d 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -38,6 +38,8 @@ #define UART_IIR_THRI		0x02 /* Transmitt
 #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
 #define UART_IIR_RLSI		0x06 /* Receiver line status interrupt */
 
+#define UART_IIR_BUSY		0x07 /* DesignWare APB Busy Detect */
+
 #define UART_FCR	2	/* Out: FIFO Control Register */
 #define UART_FCR_ENABLE_FIFO	0x01 /* Enable the FIFO */
 #define UART_FCR_CLEAR_RCVR	0x02 /* Clear the RCVR FIFO */

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Andrew Morton]

On Mon, 12 Feb 2007 12:04:08 -0600 Marc St-Jean <stjeanma@pmc-sierra.com> wrote:

> Sixth attempt at the serial driver patch for the PMC-Sierra MSP71xx device.
> Identical to last patch but respun for 8 character tabs.
> 
> There are three different fixes:
> 1. Fix for DesignWare THRE errata
> - Dropped our fix since the "8250-uart-backup-timer.patch" in the "mm"
> tree also fixes it. This patch needs to be applied on top of "mm" patch.
> 
> 2. Fix for Busy Detect on LCR write
> - No changes since last patch.
> 
> 3. Workaround for interrupt/data concurrency issue
> - No changes since last patch.

A couple of things.

- When preparing a changelog, please just tell us what the patch does. 
  The information about how this patch differes from a previous version of
  the patch is irrelevant when the patch hits the mainline repository hence
  I must edit it all.

  It's OK to add the what-i-changed-since-last-time details, but please make
  that separate and remember that it will be removed for when the patch goes
  upstream.

- When fixing a bug, please tell us in detail what that bug *is*.  None
  of the above three items tell us any of this, which is essential
  information for those who are to review the change.

>  
> +	case UPIO_DWAPB:
> +		/* Save the LCR value so it can be re-written when a
> +		 * Busy Detect interrupt occurs. */
> +		if (save_offset == UART_LCR)
> +			up->lcr = value;
> +		writeb(value, up->port.membase + offset);
> +		/* Read the IER to ensure any interrupt is cleared before
> +		 * returning from ISR. */
> +		if ((save_offset == UART_TX || save_offset == UART_IER) && in_irq())

The in_irq() is a worry.  If the serial driver is used as the system
console, then it can be called from _any_ interrupt handler.  eg a printk()
in the sata code.

What happens then?

> @@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
>  			handled = 1;
>  
>  			end = NULL;
> +		} else if (up->port.iotype == UPIO_DWAPB &&
> +			  (iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
> +			/* The DesignWare APB UART has an Busy Detect (0x07)
> +			 * interrupt meaning an LCR write attempt occured while the
> +			 * UART was busy. The interrupt must be cleared by reading
> +			 * the UART status register (USR) and the LCR re-written. */
> +			unsigned int status;
> +			status = *(volatile u32 *)up->port.private_data;

Are you sure this is right?  The use of volatile is generally discouraged
and is a sign that something is wrong.  What is the driver attempting to
read here?  Should it be using readl()?

Also, the newly-added private_data field is not being initialised to
anything anywhere in this patch.

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

Fifth attempt at the serial driver patch for the PMC-Sierra MSP71xx device.

There are three different fixes:
1. Fix for DesignWare THRE errata
- Dropped our fix since the "8250-uart-backup-timer.patch" in the "mm"
tree also fixes it. This patch needs to be applied on top of "mm" patch.

2. Fix for Busy Detect on LCR write
- Minor formatting changes.

3. Workaround for interrupt/data concurrency issue
- No changes since last patch.

Sending with /bin/mail, how's that for bare iron...

Thanks,
Marc

Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 3d91bfc..3362782 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
 		return inb(up->port.iobase + 1);
 
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		return readb(up->port.membase + offset);
 
 	case UPIO_MEM32:
@@ -333,6 +334,8 @@ #endif
 static void
 serial_out(struct uart_8250_port *up, int offset, int value)
 {
+	/* Save the offset before it's remapped */
+	int save_offset = offset;
 	offset = map_8250_out_reg(up, offset) << up->port.regshift;
 
 	switch (up->port.iotype) {
@@ -359,6 +362,18 @@ #endif
 			writeb(value, up->port.membase + offset);
 		break;
 
+	case UPIO_DWAPB:
+		/* Save the LCR value so it can be re-written when a
+		 * Busy Detect interrupt occurs. */
+		if (save_offset == UART_LCR)
+			up->lcr = value;
+		writeb(value, up->port.membase + offset);
+		/* Read the IER to ensure any interrupt is cleared before
+		 * returning from ISR. */
+		if ((save_offset == UART_TX || save_offset == UART_IER) && in_irq())
+			value = serial_in(up, UART_IER);
+		break;
+		
 	default:
 		outb(value, up->port.iobase + offset);
 	}
@@ -373,6 +388,7 @@ serial_out_sync(struct uart_8250_port *u
 #ifdef CONFIG_SERIAL_8250_AU1X00
 	case UPIO_AU:
 #endif
+	case UPIO_DWAPB:
 		serial_out(up, offset, value);
 		(void)serial_in(up, UART_LCR); /* safe, no side-effects */
 		break;
@@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
 			handled = 1;
 
 			end = NULL;
+		} else if (up->port.iotype == UPIO_DWAPB &&
+				  (iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
+			/* The DesignWare APB UART has an Busy Detect (0x07)
+			 * interrupt meaning an LCR write attempt occured while the
+			 * UART was busy. The interrupt must be cleared by reading
+			 * the UART status register (USR) and the LCR re-written. */
+			unsigned int status;
+			status = *(volatile u32 *)up->port.private_data;
+			serial_out(up, UART_LCR, up->lcr);
+
+			handled = 1;
+
+			end = NULL;
 		} else if (end == NULL)
 			end = l;
 
@@ -2085,6 +2114,7 @@ static int serial8250_request_std_resour
 	case UPIO_TSI:
 	case UPIO_MEM32:
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		if (!up->port.mapbase)
 			break;
 
@@ -2122,6 +2152,7 @@ static void serial8250_release_std_resou
 	case UPIO_TSI:
 	case UPIO_MEM32:
 	case UPIO_MEM:
+	case UPIO_DWAPB:
 		if (!up->port.mapbase)
 			break;
 
@@ -2454,8 +2485,8 @@ int __init serial8250_start_console(stru
 
 	add_preferred_console("ttyS", line, options);
 	printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
-		line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
-		port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
+		line, port->iotype >= UPIO_MEM ? "MMIO" : "I/O port",
+		port->iotype >= UPIO_MEM ? (unsigned long) port->mapbase :
 		    (unsigned long) port->iobase, options);
 	if (!(serial8250_console.flags & CON_ENABLED)) {
 		serial8250_console.flags &= ~CON_PRINTBUFFER;
diff --git a/drivers/serial/8250.h b/drivers/serial/8250.h
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index f84982e..f5b6036 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2057,6 +2057,7 @@ uart_report_port(struct uart_driver *drv
 	case UPIO_MEM32:
 	case UPIO_AU:
 	case UPIO_TSI:
+	case UPIO_DWAPB:
 		snprintf(address, sizeof(address),
 			 "MMIO 0x%lx", port->mapbase);
 		break;
@@ -2401,6 +2402,7 @@ int uart_match_port(struct uart_port *po
 	case UPIO_MEM32:
 	case UPIO_AU:
 	case UPIO_TSI:
+	case UPIO_DWAPB:
 		return (port1->mapbase == port2->mapbase);
 	}
 	return 0;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cf23813..8cdd326 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -230,6 +230,7 @@ #define UPIO_MEM		(2)
 #define UPIO_MEM32		(3)
 #define UPIO_AU			(4)			/* Au1x00 type IO */
 #define UPIO_TSI		(5)			/* Tsi108/109 type IO */
+#define UPIO_DWAPB		(6)			/* DesignWare APB UART */
 
 	unsigned int		read_status_mask;	/* driver specific */
 	unsigned int		ignore_status_mask;	/* driver specific */
@@ -276,6 +277,7 @@ #define UPF_USR_MASK		((__force upf_t) (
 	struct device		*dev;			/* parent device */
 	unsigned char		hub6;			/* this should be in the 8250 driver */
 	unsigned char		unused[3];
+	void			*private_data;		/* generic platform data pointer */
 };
 
 /*
diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 3c8a6aa..1c5ed7d 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -38,6 +38,8 @@ #define UART_IIR_THRI		0x02 /* Transmitt
 #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
 #define UART_IIR_RLSI		0x06 /* Receiver line status interrupt */
 
+#define UART_IIR_BUSY		0x07 /* DesignWare APB Busy Detect */
+
 #define UART_FCR	2	/* Out: FIFO Control Register */
 #define UART_FCR_ENABLE_FIFO	0x01 /* Enable the FIFO */
 #define UART_FCR_CLEAR_RCVR	0x02 /* Clear the RCVR FIFO */

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

Fourth attempt at the serial driver patch for the PMC-Sierra MSP71xx device.

There are three different fixes:
1. Fix for DesignWare THRE errata
- Dropped our fix since the "8250-uart-backup-timer.patch" in the "mm"
tree also fixes it. This patch needs to be applied on top of "mm" patch.

2. Fix for Busy Detect on LCR write
- Changed the ordering of test in serial8250_interrupt().
- Combined test for UPIO_DWAPB with UPIO_MEM in serial8250_start_console().
- Renamed new uart_8250_port member to private_data.

3. Workaround for interrupt/data concurrency issue
- No changes since last patch.

Thanks,
Marc

Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 3d91bfc..b309c4c 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
  		return inb(up->port.iobase + 1);

  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		return readb(up->port.membase + offset);

  	case UPIO_MEM32:
@@ -333,6 +334,8 @@ #endif
  static void
  serial_out(struct uart_8250_port *up, int offset, int value)
  {
+	/* Save the offset before it's remapped */
+	int save_offset = offset;
  	offset = map_8250_out_reg(up, offset) << up->port.regshift;

  	switch (up->port.iotype) {
@@ -359,6 +362,18 @@ #endif
  			writeb(value, up->port.membase + offset);
  		break;

+	case UPIO_DWAPB:
+		/* Save the LCR value so it can be re-written when a
+		 * Busy Detect interrupt occurs. */
+		if (save_offset == UART_LCR)
+			up->lcr = value;
+		writeb(value, up->port.membase + offset);
+		/* Read the IER to ensure any interrupt is cleared before
+		 * returning from ISR. */
+		if ((save_offset == UART_TX || save_offset == UART_IER) && in_irq())
+			value = serial_in(up, UART_IER);
+		break;
+		
  	default:
  		outb(value, up->port.iobase + offset);
  	}
@@ -373,6 +388,7 @@ serial_out_sync(struct uart_8250_port *u
  #ifdef CONFIG_SERIAL_8250_AU1X00
  	case UPIO_AU:
  #endif
+	case UPIO_DWAPB:
  		serial_out(up, offset, value);
  		(void)serial_in(up, UART_LCR); /* safe, no side-effects */
  		break;
@@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
  			handled = 1;

  			end = NULL;
+		} else if (up->port.iotype == UPIO_DWAPB &&
+				(iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
+			/* The DesignWare APB UART has an Busy Detect (0x07)
+			 * interrupt meaning an LCR write attempt occured while the
+			 * UART was busy. The interrupt must be cleared by reading
+			 * the UART status register (USR) and the LCR re-written. */
+			unsigned int status;
+			status = *(volatile u32 *)up->port.private_data;
+			serial_out(up, UART_LCR, up->lcr);
+
+			handled = 1;
+
+			end = NULL;
  		} else if (end == NULL)
  			end = l;

@@ -2085,6 +2114,7 @@ static int serial8250_request_std_resour
  	case UPIO_TSI:
  	case UPIO_MEM32:
  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		if (!up->port.mapbase)
  			break;

@@ -2122,6 +2152,7 @@ static void serial8250_release_std_resou
  	case UPIO_TSI:
  	case UPIO_MEM32:
  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		if (!up->port.mapbase)
  			break;

@@ -2454,8 +2485,8 @@ int __init serial8250_start_console(stru

  	add_preferred_console("ttyS", line, options);
  	printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
-		line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
-		port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
+		line, port->iotype >= UPIO_MEM ? "MMIO" : "I/O port",
+		port->iotype >= UPIO_MEM ? (unsigned long) port->mapbase :
  		    (unsigned long) port->iobase, options);
  	if (!(serial8250_console.flags & CON_ENABLED)) {
  		serial8250_console.flags &= ~CON_PRINTBUFFER;
diff --git a/drivers/serial/8250.h b/drivers/serial/8250.h
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index f84982e..f5b6036 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2057,6 +2057,7 @@ uart_report_port(struct uart_driver *drv
  	case UPIO_MEM32:
  	case UPIO_AU:
  	case UPIO_TSI:
+	case UPIO_DWAPB:
  		snprintf(address, sizeof(address),
  			 "MMIO 0x%lx", port->mapbase);
  		break;
@@ -2401,6 +2402,7 @@ int uart_match_port(struct uart_port *po
  	case UPIO_MEM32:
  	case UPIO_AU:
  	case UPIO_TSI:
+	case UPIO_DWAPB:
  		return (port1->mapbase == port2->mapbase);
  	}
  	return 0;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cf23813..bd9711a 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -230,6 +230,7 @@ #define UPIO_MEM		(2)
  #define UPIO_MEM32		(3)
  #define UPIO_AU			(4)			/* Au1x00 type IO */
  #define UPIO_TSI		(5)			/* Tsi108/109 type IO */
+#define UPIO_DWAPB		(6)			/* DesignWare APB UART */

  	unsigned int		read_status_mask;	/* driver specific */
  	unsigned int		ignore_status_mask;	/* driver specific */
@@ -276,6 +277,7 @@ #define UPF_USR_MASK		((__force upf_t) (
  	struct device		*dev;			/* parent device */
  	unsigned char		hub6;			/* this should be in the 8250 driver */
  	unsigned char		unused[3];
+	void				*private_data;		/* generic platform data pointer */
  };

  /*
diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 3c8a6aa..1c5ed7d 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -38,6 +38,8 @@ #define UART_IIR_THRI		0x02 /* Transmitt
  #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
  #define UART_IIR_RLSI		0x06 /* Receiver line status interrupt */

+#define UART_IIR_BUSY		0x07 /* DesignWare APB Busy Detect */
+
  #define UART_FCR	2	/* Out: FIFO Control Register */
  #define UART_FCR_ENABLE_FIFO	0x01 /* Enable the FIFO */
  #define UART_FCR_CLEAR_RCVR	0x02 /* Clear the RCVR FIFO */

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Sergei Shtylyov]

Hello.

Marc St-Jean wrote:
> Fourth attempt at the serial driver patch for the PMC-Sierra MSP71xx device.
> 
> There are three different fixes:
> 1. Fix for DesignWare THRE errata
> - Dropped our fix since the "8250-uart-backup-timer.patch" in the "mm"
> tree also fixes it. This patch needs to be applied on top of "mm" patch.
> 
> 2. Fix for Busy Detect on LCR write
> - Changed the ordering of test in serial8250_interrupt().
> - Combined test for UPIO_DWAPB with UPIO_MEM in serial8250_start_console().
> - Renamed new uart_8250_port member to private_data.
> 
> 3. Workaround for interrupt/data concurrency issue
> - No changes since last patch.

> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index 3d91bfc..b309c4c 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -308,6 +308,7 @@ static unsigned int serial_in(struct uar
>   		return inb(up->port.iobase + 1);
> 
>   	case UPIO_MEM:
> +	case UPIO_DWAPB:
>   		return readb(up->port.membase + offset);
> 
>   	case UPIO_MEM32:
> @@ -333,6 +334,8 @@ #endif
>   static void
>   serial_out(struct uart_8250_port *up, int offset, int value)
>   {
> +	/* Save the offset before it's remapped */
> +	int save_offset = offset;
>   	offset = map_8250_out_reg(up, offset) << up->port.regshift;
> 
>   	switch (up->port.iotype) {
> @@ -359,6 +362,18 @@ #endif
>   			writeb(value, up->port.membase + offset);
>   		break;
> 
> +	case UPIO_DWAPB:
> +		/* Save the LCR value so it can be re-written when a
> +		 * Busy Detect interrupt occurs. */
> +		if (save_offset == UART_LCR)
> +			up->lcr = value;
> +		writeb(value, up->port.membase + offset);
> +		/* Read the IER to ensure any interrupt is cleared before
> +		 * returning from ISR. */
> +		if ((save_offset == UART_TX || save_offset == UART_IER) && in_irq())
> +			value = serial_in(up, UART_IER);
> +		break;
> +		
>   	default:
>   		outb(value, up->port.iobase + offset);
>   	}
> @@ -373,6 +388,7 @@ serial_out_sync(struct uart_8250_port *u
>   #ifdef CONFIG_SERIAL_8250_AU1X00
>   	case UPIO_AU:
>   #endif
> +	case UPIO_DWAPB:
>   		serial_out(up, offset, value);
>   		(void)serial_in(up, UART_LCR); /* safe, no side-effects */
>   		break;
> @@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
>   			handled = 1;
> 
>   			end = NULL;
> +		} else if (up->port.iotype == UPIO_DWAPB &&
> +				(iir & UART_IIR_BUSY) == UART_IIR_BUSY) {

     Worth aligning this line with the opening paren of if... but's that's 
nitpicking. :-)

> +			/* The DesignWare APB UART has an Busy Detect (0x07)
> +			 * interrupt meaning an LCR write attempt occured while the
> +			 * UART was busy. The interrupt must be cleared by reading
> +			 * the UART status register (USR) and the LCR re-written. */
> +			unsigned int status;
> +			status = *(volatile u32 *)up->port.private_data;
> +			serial_out(up, UART_LCR, up->lcr);
> +
> +			handled = 1;
> +
> +			end = NULL;
>   		} else if (end == NULL)
>   			end = l;
> 
>   	return 0;

    Still, shouldn't you be doing this in serial8250_timeout() also?
What IRQ numbers this UART is using, BTW?

> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index cf23813..bd9711a 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -276,6 +277,7 @@ #define UPF_USR_MASK		((__force upf_t) (
>   	struct device		*dev;			/* parent device */
>   	unsigned char		hub6;			/* this should be in the 8250 driver */
>   	unsigned char		unused[3];
> +	void				*private_data;		/* generic platform data pointer */

    One tab to many before *private_data...

> diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
> index 3c8a6aa..1c5ed7d 100644
> --- a/include/linux/serial_reg.h
> +++ b/include/linux/serial_reg.h
> @@ -38,6 +38,8 @@ #define UART_IIR_THRI		0x02 /* Transmitt
>   #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
>   #define UART_IIR_RLSI		0x06 /* Receiver line status interrupt */
> 
> +#define UART_IIR_BUSY		0x07 /* DesignWare APB Busy Detect */
> +
>   #define UART_FCR	2	/* Out: FIFO Control Register */
>   #define UART_FCR_ENABLE_FIFO	0x01 /* Enable the FIFO */
>   #define UART_FCR_CLEAR_RCVR	0x02 /* Clear the RCVR FIFO */

    Oops, your mailer went and did it again. :-)

WBR, Sergei

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

Here is my second attempt at the serial driver patch for the
PMC-Sierra MSP71xx device.

There are three different fixes:
1. Fix for THRE errata
- I verified the UART_BUG_TXEN fix does not help with this erratum.
- I left our current fix in until I get our platform booting on
2.6.20-rc4 to try the mm tree "8250-uart-backup-timer.patch".
Feel free to ignore for now.

2. Fix for Busy Detect on LCR write
- Moved to new UPIO_DWAPB iotype. Because the new type is a memory
mapped device and there are several tests for UPIO_MEM, this involved
updating serial_core.c and 8250_early.c in addition to 8250.c.
- I tried implementing this totally in serial_in as suggested, but
it can't be done because of bit overlap between UART_IIR_NO_INT and
UART_IIR_BUSY. Also there is no way to set the interrupt "handled = 1"
from serial_in.

3. Workaround for interrupt/data concurrency issue
- Moved to new UPIO_DWAPB iotype.

Thanks,
Marc

Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>

Index: linux_2_6/drivers/serial/8250.c
===================================================================
RCS file: linux_2_6/drivers/serial/8250.c,v
retrieving revision 1.1.1.7
diff -u -r1.1.1.7 8250.c
--- linux_2_6/drivers/serial/8250.c	19 Oct 2006 21:00:58 -0000	1.1.1.7
+++ linux_2_6/drivers/serial/8250.c	24 Jan 2007 23:55:27 -0000
@@ -308,6 +308,7 @@
  		return inb(up->port.iobase + 1);

  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		return readb(up->port.membase + offset);

  	case UPIO_MEM32:
@@ -333,6 +334,8 @@
  static void
  serial_out(struct uart_8250_port *up, int offset, int value)
  {
+	/* Save the offset before it's remapped */
+	int save_offset = offset;
  	offset = map_8250_out_reg(up, offset) << up->port.regshift;

  	switch (up->port.iotype) {
@@ -359,6 +362,19 @@
  			writeb(value, up->port.membase + offset);
  		break;

+	case UPIO_DWAPB:
+		/* Save the LCR value so it can be re-written when a
+		 * Busy Detect interrupt occurs. */
+		if (save_offset == UART_LCR)
+			up->lcr = value;
+		writeb(value, up->port.membase + offset);
+		/* Read the IER to ensure any interrupt is cleared before
+		 * returning from ISR. */
+		if ((save_offset == UART_TX || save_offset == UART_IER) &&
+				in_irq())
+			value = serial_in(up, UART_IER);
+		break;
+		
  	default:
  		outb(value, up->port.iobase + offset);
  	}
@@ -1016,6 +1032,17 @@
  		up->bugs |= UART_BUG_NOMSR;
  #endif

+	/* Workaround:
+	 * The DesignWare SoC UART part has a bug for all
+	 * versions before 3.03a (2005-07-18)
+	 * In brief, this is a non-standard 16550 in that the THRE interrupt
+	 * will not re-assert itself simply by disabling and re-enabling the
+	 * THRI bit in the IER, it is only re-enabled if a character is actually
+	 * sent out.
+	 */
+	if( up->port.flags & UPF_DW_THRE_BUG )
+		up->bugs |= UART_BUG_DWTHRE;
+
  	serial_outp(up, UART_LCR, save_lcr);

  	if (up->capabilities != uart_config[up->port.type].flags) {
@@ -1141,6 +1168,12 @@
  			iir = serial_in(up, UART_IIR);
  			if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT)
  				transmit_chars(up);
+		} else if (up->bugs & UART_BUG_DWTHRE) {
+			unsigned char lsr, iir;
+			lsr = serial_in(up, UART_LSR);
+			iir = serial_in(up, UART_IIR);
+			if (lsr & UART_LSR_THRE)
+				transmit_chars(up);
  		}
  	}

@@ -1366,6 +1399,19 @@
  			handled = 1;

  			end = NULL;
+		} else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY &&
+				up->port.iotype == UPIO_DWAPB) {
+			/* The DesignWare APB UART has an Busy Detect (0x07)
+			 * interrupt meaning the LCR was written while the UART
+			 * was busy, so the LCR was not actually written. It's
+			 * cleared by reading the extended UART status register. */
+			unsigned int status;
+			status = *(volatile u32 *)up->port.user;
+			serial_out(up, UART_LCR, up->lcr);			
+
+			handled = 1;
+
+			end = NULL;
  		} else if (end == NULL)
  			end = l;

@@ -1950,6 +1996,7 @@
  		size = 0x100000;
  		/* fall thru */
  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		if (!up->port.mapbase)
  			break;

@@ -1985,6 +2032,7 @@
  		size = 0x100000;
  		/* fall thru */
  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		if (!up->port.mapbase)
  			break;

@@ -2011,6 +2059,7 @@

  	switch (up->port.iotype) {
  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		ret = -EINVAL;
  		break;

@@ -2032,6 +2081,7 @@

  	switch (up->port.iotype) {
  	case UPIO_MEM:
+	case UPIO_DWAPB:
  		break;

  	case UPIO_HUB6:
@@ -2352,9 +2402,12 @@

  	add_preferred_console("ttyS", line, options);
  	printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
-		line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
-		port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
-		    (unsigned long) port->iobase, options);
+		line,
+		(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
+			? "MMIO" : "I/O port",
+		(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
+			? (unsigned long) port->mapbase : (unsigned long) port->iobase,
+		options);
  	if (!(serial8250_console.flags & CON_ENABLED)) {
  		serial8250_console.flags &= ~CON_PRINTBUFFER;
  		register_console(&serial8250_console);
Index: linux_2_6/drivers/serial/8250.h
===================================================================
RCS file: linux_2_6/drivers/serial/8250.h,v
retrieving revision 1.1.1.6
retrieving revision 1.4
diff -u -r1.1.1.6 -r1.4
--- linux_2_6/drivers/serial/8250.h	19 Oct 2006 21:00:58 -0000	1.1.1.6
+++ linux_2_6/drivers/serial/8250.h	19 Oct 2006 22:08:15 -0000	1.4
@@ -49,6 +49,7 @@
  #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
  #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
  #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits (Au1x00) */
+#define UART_BUG_DWTHRE (1 << 3)	/* UART has buggy DesignWare THRE interrupt re-assertion */

  #define PROBE_RSA	(1 << 0)
  #define PROBE_ANY	(~0)
Index: linux_2_6/drivers/serial/8250_early.c
===================================================================
RCS file: linux_2_6/drivers/serial/8250_early.c,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 8250_early.c
--- linux_2_6/drivers/serial/8250_early.c	19 Oct 2006 20:08:20 -0000	1.1.1.3
+++ linux_2_6/drivers/serial/8250_early.c	24 Jan 2007 23:55:27 -0000
@@ -46,7 +46,7 @@

  static unsigned int __init serial_in(struct uart_port *port, int offset)
  {
-	if (port->iotype == UPIO_MEM)
+	if (port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
  		return readb(port->membase + offset);
  	else
  		return inb(port->iobase + offset);
@@ -54,7 +54,7 @@

  static void __init serial_out(struct uart_port *port, int offset, int value)
  {
-	if (port->iotype == UPIO_MEM)
+	if (port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
  		writeb(value, port->membase + offset);
  	else
  		outb(value, port->iobase + offset);
@@ -233,7 +233,7 @@
  		return 0;

  	/* Try to start the normal driver on a matching line.  */
-	mmio = (port->iotype == UPIO_MEM);
+	mmio = (port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB);
  	line = serial8250_start_console(port, device->options);
  	if (line < 0)
  		printk("No ttyS device at %s 0x%lx for console\n",
Index: linux_2_6/drivers/serial/serial_core.c
===================================================================
RCS file: linux_2_6/drivers/serial/serial_core.c,v
retrieving revision 1.1.1.7
diff -u -r1.1.1.7 serial_core.c
--- linux_2_6/drivers/serial/serial_core.c	19 Oct 2006 21:00:58 -0000	1.1.1.7
+++ linux_2_6/drivers/serial/serial_core.c	24 Jan 2007 23:55:28 -0000
@@ -1669,9 +1669,10 @@

  	ret = sprintf(buf, "%d: uart:%s %s%08lX irq:%d",
  			port->line, uart_type(port),
-			port->iotype == UPIO_MEM ? "mmio:0x" : "port:",
-			port->iotype == UPIO_MEM ? port->mapbase :
-						(unsigned long) port->iobase,
+			(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
+				? "mmio:0x" : "port:",
+			(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
+				? port->mapbase : (unsigned long) port->iobase,
  			port->irq);

  	if (port->type == PORT_UNKNOWN) {
@@ -2037,6 +2038,7 @@
  	case UPIO_MEM32:
  	case UPIO_AU:
  	case UPIO_TSI:
+	case UPIO_DWAPB:
  		snprintf(address, sizeof(address),
  			 "MMIO 0x%lx", port->mapbase);
  		break;
@@ -2380,6 +2382,7 @@
  	case UPIO_MEM32:
  	case UPIO_AU:
  	case UPIO_TSI:
+	case UPIO_DWAPB:
  		return (port1->mapbase == port2->mapbase);
  	}
  	return 0;
Index: linux_2_6/include/linux/serial_core.h
===================================================================
RCS file: linux_2_6/include/linux/serial_core.h,v
retrieving revision 1.1.1.7
diff -u -r1.1.1.7 serial_core.h
--- linux_2_6/include/linux/serial_core.h	19 Oct 2006 21:01:02 -0000	1.1.1.7
+++ linux_2_6/include/linux/serial_core.h	24 Jan 2007 23:55:28 -0000
@@ -228,6 +228,7 @@
  #define UPIO_MEM32		(3)
  #define UPIO_AU			(4)			/* Au1x00 type IO */
  #define UPIO_TSI		(5)			/* Tsi108/109 type IO */
+#define UPIO_DWAPB		(6)			/* DesignWare APB UART */

  	unsigned int		read_status_mask;	/* driver specific */
  	unsigned int		ignore_status_mask;	/* driver specific */
@@ -258,6 +259,8 @@
  #define UPF_CONS_FLOW		((__force upf_t) (1 << 23))
  #define UPF_SHARE_IRQ		((__force upf_t) (1 << 24))
  #define UPF_BOOT_AUTOCONF	((__force upf_t) (1 << 28))
+#define UPF_DW_THRE_BUG		((__force upf_t)(1 << 29)) /* DesignWare THRE hardware BUG
+														* (cannot be autodetected) */
  #define UPF_DEAD		((__force upf_t) (1 << 30))
  #define UPF_IOREMAP		((__force upf_t) (1 << 31))

@@ -274,6 +277,7 @@
  	struct device		*dev;			/* parent device */
  	unsigned char		hub6;			/* this should be in the 8250 driver */
  	unsigned char		unused[3];
+	void				*user;			/* generic platform 'user' pointer */
  };

  /*
Index: linux_2_6/include/linux/serial_reg.h
===================================================================
RCS file: linux_2_6/include/linux/serial_reg.h,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 serial_reg.h
--- linux_2_6/include/linux/serial_reg.h	19 Oct 2006 18:29:50 -0000	1.1.1.2
+++ linux_2_6/include/linux/serial_reg.h	24 Jan 2007 23:55:29 -0000
@@ -218,6 +218,10 @@
  #define UART_FCR_PXAR16	0x80	/* receive FIFO treshold = 16 */
  #define UART_FCR_PXAR32	0xc0	/* receive FIFO treshold = 32 */

+/*
+ * DesignWare APB UART
+ */
+#define UART_IIR_BUSY		0x07	/* Busy Detect */

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Sergei Shtylyov]

Marc St-Jean wrote:
> Here is my second attempt at the serial driver patch for the
> PMC-Sierra MSP71xx device.
> 
> There are three different fixes:
> 1. Fix for THRE errata
> - I verified the UART_BUG_TXEN fix does not help with this erratum.
> - I left our current fix in until I get our platform booting on
> 2.6.20-rc4 to try the mm tree "8250-uart-backup-timer.patch".
> Feel free to ignore for now.
> 
> 2. Fix for Busy Detect on LCR write
> - Moved to new UPIO_DWAPB iotype. Because the new type is a memory
> mapped device and there are several tests for UPIO_MEM, this involved
> updating serial_core.c and 8250_early.c in addition to 8250.c.
> - I tried implementing this totally in serial_in as suggested, but
> it can't be done because of bit overlap between UART_IIR_NO_INT and
> UART_IIR_BUSY. Also there is no way to set the interrupt "handled = 1"
> from serial_in.
> 
> 3. Workaround for interrupt/data concurrency issue
> - Moved to new UPIO_DWAPB iotype.

> Index: linux_2_6/drivers/serial/8250.c
> ===================================================================
> RCS file: linux_2_6/drivers/serial/8250.c,v
> retrieving revision 1.1.1.7
> diff -u -r1.1.1.7 8250.c
> --- linux_2_6/drivers/serial/8250.c	19 Oct 2006 21:00:58 -0000	1.1.1.7
> +++ linux_2_6/drivers/serial/8250.c	24 Jan 2007 23:55:27 -0000
[...]
> @@ -333,6 +334,8 @@
>   static void
>   serial_out(struct uart_8250_port *up, int offset, int value)

   Your patch is clearly garbled again, something added an extra space to all 
lines stating with space... :-/

>   {
> +	/* Save the offset before it's remapped */
> +	int save_offset = offset;

    Is there real need to save this? What regshift equals for this UART?

>   	offset = map_8250_out_reg(up, offset) << up->port.regshift;

>   	switch (up->port.iotype) {
> @@ -359,6 +362,19 @@
>   			writeb(value, up->port.membase + offset);
>   		break;
> 
> +	case UPIO_DWAPB:
> +		/* Save the LCR value so it can be re-written when a
> +		 * Busy Detect interrupt occurs. */
> +		if (save_offset == UART_LCR)
> +			up->lcr = value;
> +		writeb(value, up->port.membase + offset);
> +		/* Read the IER to ensure any interrupt is cleared before
> +		 * returning from ISR. */
> +		if ((save_offset == UART_TX || save_offset == UART_IER) &&

    Not sure how an IER read ensures that...

> +				in_irq())

    I'd suggest to either indent this line right (start below 2ns paren of if 
stmt) or keep on the same line.

> +			value = serial_in(up, UART_IER);
> +		break;
> +		
>   	default:
>   		outb(value, up->port.iobase + offset);
>   	}
> @@ -1016,6 +1032,17 @@
>   		up->bugs |= UART_BUG_NOMSR;
>   #endif
> 
> +	/* Workaround:
> +	 * The DesignWare SoC UART part has a bug for all
> +	 * versions before 3.03a (2005-07-18)
> +	 * In brief, this is a non-standard 16550 in that the THRE interrupt
> +	 * will not re-assert itself simply by disabling and re-enabling the
> +	 * THRI bit in the IER, it is only re-enabled if a character is actually
> +	 * sent out.
> +	 */
> +	if( up->port.flags & UPF_DW_THRE_BUG )
> +		up->bugs |= UART_BUG_DWTHRE;
> +
>   	serial_outp(up, UART_LCR, save_lcr);
> 
>   	if (up->capabilities != uart_config[up->port.type].flags) {
> @@ -1141,6 +1168,12 @@
>   			iir = serial_in(up, UART_IIR);
>   			if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT)
>   				transmit_chars(up);
> +		} else if (up->bugs & UART_BUG_DWTHRE) {
> +			unsigned char lsr, iir;
> +			lsr = serial_in(up, UART_LSR);
> +			iir = serial_in(up, UART_IIR);
> +			if (lsr & UART_LSR_THRE)

    Why read IIR if you don't check it?

> @@ -2352,9 +2402,12 @@
> 
>   	add_preferred_console("ttyS", line, options);
>   	printk("Adding console on ttyS%d at %s 0x%lx (options '%s')\n",
> -		line, port->iotype == UPIO_MEM ? "MMIO" : "I/O port",
> -		port->iotype == UPIO_MEM ? (unsigned long) port->mapbase :
> -		    (unsigned long) port->iobase, options);
> +		line,
> +		(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
> +			? "MMIO" : "I/O port",
> +		(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
> +			? (unsigned long) port->mapbase : (unsigned long) port->iobase,
> +		options);
>   	if (!(serial8250_console.flags & CON_ENABLED)) {
>   		serial8250_console.flags &= ~CON_PRINTBUFFER;
>   		register_console(&serial8250_console);
[...]
> Index: linux_2_6/drivers/serial/8250_early.c
> ===================================================================
> RCS file: linux_2_6/drivers/serial/8250_early.c,v
> retrieving revision 1.1.1.3
> diff -u -r1.1.1.3 8250_early.c
> --- linux_2_6/drivers/serial/8250_early.c	19 Oct 2006 20:08:20 -0000	1.1.1.3
> +++ linux_2_6/drivers/serial/8250_early.c	24 Jan 2007 23:55:27 -0000
> @@ -46,7 +46,7 @@
> 
>   static unsigned int __init serial_in(struct uart_port *port, int offset)
>   {
> -	if (port->iotype == UPIO_MEM)
> +	if (port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
>   		return readb(port->membase + offset);
>   	else
>   		return inb(port->iobase + offset);
> @@ -54,7 +54,7 @@
> 
>   static void __init serial_out(struct uart_port *port, int offset, int value)
>   {
> -	if (port->iotype == UPIO_MEM)
> +	if (port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
>   		writeb(value, port->membase + offset);
>   	else
>   		outb(value, port->iobase + offset);
> @@ -233,7 +233,7 @@
>   		return 0;
> 
>   	/* Try to start the normal driver on a matching line.  */
> -	mmio = (port->iotype == UPIO_MEM);
> +	mmio = (port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB);
>   	line = serial8250_start_console(port, device->options);
>   	if (line < 0)
>   		printk("No ttyS device at %s 0x%lx for console\n",

    From your 8250_eraly.c changes I can conclude regshift == 1 (it doesn't 
currently support other cases). So, serial_pot() doesn't need save_offset. :-)

> Index: linux_2_6/drivers/serial/serial_core.c
> ===================================================================
> RCS file: linux_2_6/drivers/serial/serial_core.c,v
> retrieving revision 1.1.1.7
> diff -u -r1.1.1.7 serial_core.c
> --- linux_2_6/drivers/serial/serial_core.c	19 Oct 2006 21:00:58 -0000	1.1.1.7
> +++ linux_2_6/drivers/serial/serial_core.c	24 Jan 2007 23:55:28 -0000
> @@ -1669,9 +1669,10 @@
> 
>   	ret = sprintf(buf, "%d: uart:%s %s%08lX irq:%d",
>   			port->line, uart_type(port),
> -			port->iotype == UPIO_MEM ? "mmio:0x" : "port:",
> -			port->iotype == UPIO_MEM ? port->mapbase :
> -						(unsigned long) port->iobase,
> +			(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
> +				? "mmio:0x" : "port:",
> +			(port->iotype == UPIO_MEM || port->iotype == UPIO_DWAPB)
> +				? port->mapbase : (unsigned long) port->iobase,
>   			port->irq);
>   	if (port->type == PORT_UNKNOWN) {

    Needless change. My patch that fixes this function is in Linus' tree since 
September, not sure why you don't have it:

http://www2.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=6c6a2334a1e8af7c3eaab992732825fa9ade77cf

> Index: linux_2_6/include/linux/serial_core.h
> ===================================================================
> RCS file: linux_2_6/include/linux/serial_core.h,v
> retrieving revision 1.1.1.7
> diff -u -r1.1.1.7 serial_core.h
> --- linux_2_6/include/linux/serial_core.h	19 Oct 2006 21:01:02 -0000	1.1.1.7
> +++ linux_2_6/include/linux/serial_core.h	24 Jan 2007 23:55:28 -0000
[...]
> @@ -274,6 +277,7 @@
>   	struct device		*dev;			/* parent device */
>   	unsigned char		hub6;			/* this should be in the 8250 driver */
>   	unsigned char		unused[3];
> +	void				*user;			/* generic platform 'user' pointer */

    Erm, 'private' or 'data' would've sounded better in the kernel context, 
IMHO... :-)

> Index: linux_2_6/include/linux/serial_reg.h
> ===================================================================
> RCS file: linux_2_6/include/linux/serial_reg.h,v
> retrieving revision 1.1.1.2
> diff -u -r1.1.1.2 serial_reg.h
> --- linux_2_6/include/linux/serial_reg.h	19 Oct 2006 18:29:50 -0000	1.1.1.2
> +++ linux_2_6/include/linux/serial_reg.h	24 Jan 2007 23:55:29 -0000
> @@ -218,6 +218,10 @@
>   #define UART_FCR_PXAR16	0x80	/* receive FIFO treshold = 16 */
>   #define UART_FCR_PXAR32	0xc0	/* receive FIFO treshold = 32 */
> 
> +/*
> + * DesignWare APB UART
> + */
> +#define UART_IIR_BUSY		0x07	/* Busy Detect */

    I'd suggest keeping this with other UART_IIR_* #defines, separated by the 
more elaborate comment.

WBR, Sergei

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

CCing linux-kernel as per AC's suggestion...

-----Original Message-----
From: Sergei Shtylyov
Sent: Friday, January 19, 2007 9:05 AM
To: Marc St-Jean
Cc: linux-mips@linux-mips.org; linux-serial@vger.kernel.org
Subject: Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

Hello.

Marc St-Jean wrote:
> Here is a serial driver patch for the PMC-Sierra MSP71xx device.

> There are three different fixes:
> 1. Fix for THRE errata
> 2. Fix for Busy Detect on LCR write
> 3. Workaround for interrupt/data concurrency issue

> The first fix is handled cleanly using a UART_BUG_* flag.

    Hm, I wouldn't call it clean...

> The second and third fixes use platform tests. I couldn't think of a 
> good way to implement them without using tests and not increase code 
> and structure sizes for platforms not requiring them.

> Does anyone have any suggestions on implementing these without the 
> platform flag?

> Thanks,
> Marc

> Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>

> Index: linux_2_6/drivers/serial/8250.c 
> ===================================================================
> RCS file: linux_2_6/drivers/serial/8250.c,v retrieving revision 
> 1.1.1.7 retrieving revision 1.9 diff -u -r1.1.1.7 -r1.9
> --- linux_2_6/drivers/serial/8250.c	19 Oct 2006 21:00:58 -0000	1.1.1.7
> +++ linux_2_6/drivers/serial/8250.c	19 Oct 2006 22:08:15 -0000	1.9
> @@ -44,6 +44,10 @@
>   #include <asm/io.h>
>   #include <asm/irq.h>
> 
> +#ifdef CONFIG_PMC_MSP
> +#include <msp_regs.h>
> +#endif
> +
>   #include "8250.h"
> 
>   /*
> @@ -130,6 +134,9 @@
>   	unsigned char		mcr_mask;	/* mask of user bits */
>   	unsigned char		mcr_force;	/* mask of forced bits */
>   	unsigned char		lsr_break_flag;
> +#ifdef CONFIG_PMC_MSP
> +	int					dwapb_lcr;	/* saved LCR for DW APB WAR */
> +#endif

    There was already 'lcr' field there, couldn't it be used?

> @@ -333,6 +340,10 @@
>   static void
>   serial_out(struct uart_8250_port *up, int offset, int value)
>   {
> +#ifdef CONFIG_PMC_MSP
> +	/* Save the offset before it's remapped */
> +	int dwapb_offset = offset;
> +#endif
>   	offset = map_8250_out_reg(up, offset) << up->port.regshift;
> 
>   	switch (up->port.iotype) {
> @@ -342,7 +353,19 @@
>   		break;
> 
>   	case UPIO_MEM:
> +#ifdef CONFIG_PMC_MSP
> +		/* Save the LCR value so it can be re-written when a
> +		 * Busy Detect interrupt occurs. */
> +		if (dwapb_offset == UART_LCR)
> +			up->dwapb_lcr = value;
> +#endif
>   		writeb(value, up->port.membase + offset);
> +#ifdef CONFIG_PMC_MSP
> +		/* Re-read the IER to ensure any interrupt disabling has
> +		 * completed before proceeding with ISR. */
> +		if (dwapb_offset == UART_IER)
> +			value = serial_in(up, dwapb_offset); #endif
>   		break;

    Hm, was there really a need for #ifdef mess here?
    I'd vote for introducing new UPIO_* here, like was done for TSi10x UARTs just for the same reason.

> @@ -1016,6 +1039,17 @@
>   		up->bugs |= UART_BUG_NOMSR;
>   #endif
> 
> +	/* Workaround:
> +	 * The DesignWare SoC UART part has a bug for all
> +	 * versions before 3.03a (2005-07-18)
> +	 * In brief, this is a non-standard 16550 in that the THRE interrupt
> +	 * will not re-assert itself simply by disabling and re-enabling the
> +	 * THRI bit in the IER, it is only re-enabled if a character is actually
> +	 * sent out.
> +	 */
> +	if( up->port.flags & UPF_DW_THRE_BUG )
> +		up->bugs |= UART_BUG_DWTHRE;
> +
>   	serial_outp(up, UART_LCR, save_lcr);
> 
>   	if (up->capabilities != uart_config[up->port.type].flags) { @@ 
> -1141,6 +1175,12 @@
>   			iir = serial_in(up, UART_IIR);
>   			if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT)
>   				transmit_chars(up);
> +		} else if (up->bugs & UART_BUG_DWTHRE) {
> +			unsigned char lsr, iir;
> +			lsr = serial_in(up, UART_LSR);
> +			iir = serial_in(up, UART_IIR);
> +			if (lsr & UART_LSR_THRE)
> +				transmit_chars(up);

    I don't see how this *really* differs from the UART_BUG_TXEN case.
    Have you tried *that* workaround? In any case, looks like this errata is auto-detectable just like UART_BUG_TXEN.

> @@ -1366,6 +1406,31 @@
>   			handled = 1;
> 
>   			end = NULL;
> +#ifdef CONFIG_PMC_MSP
> +		} else if ((iir & UART_IER_BUSY) == UART_IER_BUSY) {

    Hm, masking IIR with IER mask, is this correct? Doubt it.

> +			/*
> +			 * The MSP (DesignWare APB UART) serial subsystem has a
> +			 * non-standard interrupt condition (0x7) which means
> +			 * that the LCR was written while the UART was busy, so
> +			 * the LCR was not actually written.  It is cleared by
> +			 * reading the special non-standard extended UART status
> +			 * register.
> +			 */
> +			unsigned int tmp;
> +			if( up->port.line == 0 )
> +				tmp = *UART0_STATUS_REG;
> +			else
> +				tmp = *UART1_STATUS_REG;
> +			
> +			/* Check if saved on LCR write */
> +			if( up->dwapb_lcr != -1 )
> +				serial_outp(up, UART_LCR, up->dwapb_lcr);
> +			else
> +				printk(KERN_ERR "serial8250: UART BUSY, no LCR write!\n" );
> +
> +			handled = 1;
> +			end = NULL;
> +#endif

    Not sure if this also shouldn't be handled in other places which check for interrupt status, like serial8250_timeout()...

[...]
> Index: linux_2_6/drivers/serial/8250.h 
> ===================================================================
> RCS file: linux_2_6/drivers/serial/8250.h,v retrieving revision 
> 1.1.1.6 retrieving revision 1.4 diff -u -r1.1.1.6 -r1.4
> --- linux_2_6/drivers/serial/8250.h	19 Oct 2006 21:00:58 -0000	1.1.1.6
> +++ linux_2_6/drivers/serial/8250.h	19 Oct 2006 22:08:15 -0000	1.4
> @@ -49,6 +49,7 @@
>   #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
>   #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
>   #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits (Au1x00) */
> +#define UART_BUG_DWTHRE (1 << 3)	/* UART has buggy DesignWare THRE interrupt re-assertion */
> 
>   #define PROBE_RSA	(1 << 0)
>   #define PROBE_ANY	(~0)
> Index: linux_2_6/include/linux/serial_core.h
> ===================================================================
> RCS file: linux_2_6/include/linux/serial_core.h,v
> retrieving revision 1.1.1.7
> retrieving revision 1.5
> diff -u -r1.1.1.7 -r1.5
> --- linux_2_6/include/linux/serial_core.h	19 Oct 2006 21:01:02 -0000	1.1.1.7
> +++ linux_2_6/include/linux/serial_core.h	19 Oct 2006 22:08:16 -0000	1.5
> @@ -258,6 +258,8 @@
>   #define UPF_CONS_FLOW		((__force upf_t) (1 << 23))
>   #define UPF_SHARE_IRQ		((__force upf_t) (1 << 24))
>   #define UPF_BOOT_AUTOCONF	((__force upf_t) (1 << 28))
> +#define UPF_DW_THRE_BUG		((__force upf_t)(1 << 29)) /* DesignWare THRE hardware BUG

    The need for the new flag seems doubtful to me.

> +														* (cannot be autodetected) */

    The patch is linewrapped

> Index: linux_2_6/include/linux/serial_reg.h
> ===================================================================
> RCS file: linux_2_6/include/linux/serial_reg.h,v
> retrieving revision 1.1.1.2
> retrieving revision 1.3
> diff -u -r1.1.1.2 -r1.3
> --- linux_2_6/include/linux/serial_reg.h	19 Oct 2006 18:29:50 -0000	1.1.1.2
> +++ linux_2_6/include/linux/serial_reg.h	19 Oct 2006 19:45:04 -0000	1.3
> @@ -218,6 +218,10 @@
>   #define UART_FCR_PXAR16	0x80	/* receive FIFO treshold = 16 */
>   #define UART_FCR_PXAR32	0xc0	/* receive FIFO treshold = 32 */
> 
> +/*
> + * DesignWare APB UART
> + */
> +#define UART_IER_BUSY		0x07	/* Busy Detect */

    Are you sure it's not *IIR* value?  Doesn't look like interrupt mask for IER. And IIR value of 7 already means something else, namely, no interrupt and receiver status. Hm...

MBR, Sergei

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

CCing linux-kernel as per AC's suggestion...

On Thu, Jan 18, 2007 at 04:23:01PM -0800, Marc St-Jean wrote:

> Index: linux_2_6/drivers/serial/8250.c 
> ===================================================================
> RCS file: linux_2_6/drivers/serial/8250.c,v retrieving revision 
> 1.1.1.7 retrieving revision 1.9 diff -u -r1.1.1.7 -r1.9
> --- linux_2_6/drivers/serial/8250.c	19 Oct 2006 21:00:58 -0000	1.1.1.7
> +++ linux_2_6/drivers/serial/8250.c	19 Oct 2006 22:08:15 -0000	1.9
> @@ -44,6 +44,10 @@
>   #include <asm/io.h>
>   #include <asm/irq.h>
> 
> +#ifdef CONFIG_PMC_MSP
> +#include <msp_regs.h>
> +#endif

CONFIG_PMC_MSP is not defined anywhere.  msp_regs.h does not exist.

  Ralf

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

CCing linux-kernel as per AC's suggestion...

Here is a serial driver patch for the PMC-Sierra MSP71xx device.

There are three different fixes:
1. Fix for THRE errata
2. Fix for Busy Detect on LCR write
3. Workaround for interrupt/data concurrency issue

The first fix is handled cleanly using a UART_BUG_* flag.

The second and third fixes use platform tests. I couldn't think of a
good way to implement them without using tests and not increase code
and structure sizes for platforms not requiring them.

Does anyone have any suggestions on implementing these without the
platform flag?

Thanks,
Marc

Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>


Index: linux_2_6/drivers/serial/8250.c
===================================================================
RCS file: linux_2_6/drivers/serial/8250.c,v
retrieving revision 1.1.1.7
retrieving revision 1.9
diff -u -r1.1.1.7 -r1.9
--- linux_2_6/drivers/serial/8250.c	19 Oct 2006 21:00:58 -0000	1.1.1.7
+++ linux_2_6/drivers/serial/8250.c	19 Oct 2006 22:08:15 -0000	1.9
@@ -44,6 +44,10 @@
  #include <asm/io.h>
  #include <asm/irq.h>

+#ifdef CONFIG_PMC_MSP
+#include <msp_regs.h>
+#endif
+
  #include "8250.h"

  /*
@@ -130,6 +134,9 @@
  	unsigned char		mcr_mask;	/* mask of user bits */
  	unsigned char		mcr_force;	/* mask of forced bits */
  	unsigned char		lsr_break_flag;
+#ifdef CONFIG_PMC_MSP
+	int					dwapb_lcr;	/* saved LCR for DW APB WAR */
+#endif

  	/*
  	 * We provide a per-port pm hook.
@@ -333,6 +340,10 @@
  static void
  serial_out(struct uart_8250_port *up, int offset, int value)
  {
+#ifdef CONFIG_PMC_MSP
+	/* Save the offset before it's remapped */
+	int dwapb_offset = offset;
+#endif
  	offset = map_8250_out_reg(up, offset) << up->port.regshift;

  	switch (up->port.iotype) {
@@ -342,7 +353,19 @@
  		break;

  	case UPIO_MEM:
+#ifdef CONFIG_PMC_MSP
+		/* Save the LCR value so it can be re-written when a
+		 * Busy Detect interrupt occurs. */
+		if (dwapb_offset == UART_LCR)
+			up->dwapb_lcr = value;
+#endif
  		writeb(value, up->port.membase + offset);
+#ifdef CONFIG_PMC_MSP
+		/* Re-read the IER to ensure any interrupt disabling has
+		 * completed before proceeding with ISR. */
+		if (dwapb_offset == UART_IER)
+			value = serial_in(up, dwapb_offset);
+#endif
  		break;

  	case UPIO_MEM32:
@@ -1016,6 +1039,17 @@
  		up->bugs |= UART_BUG_NOMSR;
  #endif

+	/* Workaround:
+	 * The DesignWare SoC UART part has a bug for all
+	 * versions before 3.03a (2005-07-18)
+	 * In brief, this is a non-standard 16550 in that the THRE interrupt
+	 * will not re-assert itself simply by disabling and re-enabling the
+	 * THRI bit in the IER, it is only re-enabled if a character is actually
+	 * sent out.
+	 */
+	if( up->port.flags & UPF_DW_THRE_BUG )
+		up->bugs |= UART_BUG_DWTHRE;
+
  	serial_outp(up, UART_LCR, save_lcr);

  	if (up->capabilities != uart_config[up->port.type].flags) {
@@ -1141,6 +1175,12 @@
  			iir = serial_in(up, UART_IIR);
  			if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT)
  				transmit_chars(up);
+		} else if (up->bugs & UART_BUG_DWTHRE) {
+			unsigned char lsr, iir;
+			lsr = serial_in(up, UART_LSR);
+			iir = serial_in(up, UART_IIR);
+			if (lsr & UART_LSR_THRE)
+				transmit_chars(up);
  		}
  	}

@@ -1366,6 +1406,31 @@
  			handled = 1;

  			end = NULL;
+#ifdef CONFIG_PMC_MSP
+		} else if ((iir & UART_IER_BUSY) == UART_IER_BUSY) {
+			/*
+			 * The MSP (DesignWare APB UART) serial subsystem has a
+			 * non-standard interrupt condition (0x7) which means
+			 * that the LCR was written while the UART was busy, so
+			 * the LCR was not actually written.  It is cleared by
+			 * reading the special non-standard extended UART status
+			 * register.
+			 */
+			unsigned int tmp;
+			if( up->port.line == 0 )
+				tmp = *UART0_STATUS_REG;
+			else
+				tmp = *UART1_STATUS_REG;
+			
+			/* Check if saved on LCR write */
+			if( up->dwapb_lcr != -1 )
+				serial_outp(up, UART_LCR, up->dwapb_lcr);
+			else
+				printk(KERN_ERR "serial8250: UART BUSY, no LCR write!\n" );
+
+			handled = 1;
+			end = NULL;
+#endif
  		} else if (end == NULL)
  			end = l;

@@ -2191,6 +2256,11 @@
  	for (i = 0; i < nr_uarts; i++) {
  		struct uart_8250_port *up = &serial8250_ports[i];

+#ifdef CONFIG_PMC_MSP
+		/* Initialize saved LCR value */
+		up->dwapb_lcr = -1;
+#endif
+
  		up->port.dev = dev;
  		uart_add_one_port(drv, &up->port);
  	}
Index: linux_2_6/drivers/serial/8250.h
===================================================================
RCS file: linux_2_6/drivers/serial/8250.h,v
retrieving revision 1.1.1.6
retrieving revision 1.4
diff -u -r1.1.1.6 -r1.4
--- linux_2_6/drivers/serial/8250.h	19 Oct 2006 21:00:58 -0000	1.1.1.6
+++ linux_2_6/drivers/serial/8250.h	19 Oct 2006 22:08:15 -0000	1.4
@@ -49,6 +49,7 @@
  #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
  #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
  #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits (Au1x00) */
+#define UART_BUG_DWTHRE (1 << 3)	/* UART has buggy DesignWare THRE interrupt re-assertion */

  #define PROBE_RSA	(1 << 0)
  #define PROBE_ANY	(~0)
Index: linux_2_6/include/linux/serial_core.h
===================================================================
RCS file: linux_2_6/include/linux/serial_core.h,v
retrieving revision 1.1.1.7
retrieving revision 1.5
diff -u -r1.1.1.7 -r1.5
--- linux_2_6/include/linux/serial_core.h	19 Oct 2006 21:01:02 -0000	1.1.1.7
+++ linux_2_6/include/linux/serial_core.h	19 Oct 2006 22:08:16 -0000	1.5
@@ -258,6 +258,8 @@
  #define UPF_CONS_FLOW		((__force upf_t) (1 << 23))
  #define UPF_SHARE_IRQ		((__force upf_t) (1 << 24))
  #define UPF_BOOT_AUTOCONF	((__force upf_t) (1 << 28))
+#define UPF_DW_THRE_BUG		((__force upf_t)(1 << 29)) /* DesignWare THRE hardware BUG
+														* (cannot be autodetected) */
  #define UPF_DEAD		((__force upf_t) (1 << 30))
  #define UPF_IOREMAP		((__force upf_t) (1 << 31))

Index: linux_2_6/include/linux/serial_reg.h
===================================================================
RCS file: linux_2_6/include/linux/serial_reg.h,v
retrieving revision 1.1.1.2
retrieving revision 1.3
diff -u -r1.1.1.2 -r1.3
--- linux_2_6/include/linux/serial_reg.h	19 Oct 2006 18:29:50 -0000	1.1.1.2
+++ linux_2_6/include/linux/serial_reg.h	19 Oct 2006 19:45:04 -0000	1.3
@@ -218,6 +218,10 @@
  #define UART_FCR_PXAR16	0x80	/* receive FIFO treshold = 16 */
  #define UART_FCR_PXAR32	0xc0	/* receive FIFO treshold = 32 */

+/*
+ * DesignWare APB UART
+ */
+#define UART_IER_BUSY		0x07	/* Busy Detect */

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Alan]

> There are three different fixes:
> 1. Fix for THRE errata

That should be handled anyway. The current code actually spots this and
uses a backup timer for dodgy UARTS

> 2. Fix for Busy Detect on LCR write
> 3. Workaround for interrupt/data concurrency issue

>   	case UPIO_MEM:
> +#ifdef CONFIG_PMC_MSP
> +		/* Save the LCR value so it can be re-written when a
> +		 * Busy Detect interrupt occurs. */
> +		if (dwapb_offset == UART_LCR)
> +			up->dwapb_lcr = value;
> +#endif
>   		writeb(value, up->port.membase + offset);
> +#ifdef CONFIG_PMC_MSP
> +		/* Re-read the IER to ensure any interrupt disabling has
> +		 * completed before proceeding with ISR. */
> +		if (dwapb_offset == UART_IER)
> +			value = serial_in(up, dwapb_offset);
> +#endif
>   		break;

This I would hope you can hide in the platform specific
serial_in/serial_out functions. If you write the UART_LCR save it in
serial_out(), if you read IER etc.


> +#ifdef CONFIG_PMC_MSP
> +		} else if ((iir & UART_IER_BUSY) == UART_IER_BUSY) {
> +			/*
> +			 * The MSP (DesignWare APB UART) serial subsystem has a
> +			 * non-standard interrupt condition (0x7) which means
> +			 * that the LCR was written while the UART was busy, so
> +			 * the LCR was not actually written.  It is cleared by
> +			 * reading the special non-standard extended UART status
> +			 * register.

Ditto... spot this case and whack it in your serial methods.


And we might want to add a void * for board specific insanity to the 8250
structures if we really have to so you can hang your brain damage
privately off that ?

[PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Marc St-Jean]

Here is a serial driver patch for the PMC-Sierra MSP71xx device.

There are three different fixes:
1. Fix for THRE errata
2. Fix for Busy Detect on LCR write
3. Workaround for interrupt/data concurrency issue

The first fix is handled cleanly using a UART_BUG_* flag.

The second and third fixes use platform tests. I couldn't think of a
good way to implement them without using tests and not increase code
and structure sizes for platforms not requiring them.

Does anyone have any suggestions on implementing these without the
platform flag?

Thanks,
Marc

Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>


Index: linux_2_6/drivers/serial/8250.c
===================================================================
RCS file: linux_2_6/drivers/serial/8250.c,v
retrieving revision 1.1.1.7
retrieving revision 1.9
diff -u -r1.1.1.7 -r1.9
--- linux_2_6/drivers/serial/8250.c	19 Oct 2006 21:00:58 -0000	1.1.1.7
+++ linux_2_6/drivers/serial/8250.c	19 Oct 2006 22:08:15 -0000	1.9
@@ -44,6 +44,10 @@
  #include <asm/io.h>
  #include <asm/irq.h>

+#ifdef CONFIG_PMC_MSP
+#include <msp_regs.h>
+#endif
+
  #include "8250.h"

  /*
@@ -130,6 +134,9 @@
  	unsigned char		mcr_mask;	/* mask of user bits */
  	unsigned char		mcr_force;	/* mask of forced bits */
  	unsigned char		lsr_break_flag;
+#ifdef CONFIG_PMC_MSP
+	int					dwapb_lcr;	/* saved LCR for DW APB WAR */
+#endif

  	/*
  	 * We provide a per-port pm hook.
@@ -333,6 +340,10 @@
  static void
  serial_out(struct uart_8250_port *up, int offset, int value)
  {
+#ifdef CONFIG_PMC_MSP
+	/* Save the offset before it's remapped */
+	int dwapb_offset = offset;
+#endif
  	offset = map_8250_out_reg(up, offset) << up->port.regshift;

  	switch (up->port.iotype) {
@@ -342,7 +353,19 @@
  		break;

  	case UPIO_MEM:
+#ifdef CONFIG_PMC_MSP
+		/* Save the LCR value so it can be re-written when a
+		 * Busy Detect interrupt occurs. */
+		if (dwapb_offset == UART_LCR)
+			up->dwapb_lcr = value;
+#endif
  		writeb(value, up->port.membase + offset);
+#ifdef CONFIG_PMC_MSP
+		/* Re-read the IER to ensure any interrupt disabling has
+		 * completed before proceeding with ISR. */
+		if (dwapb_offset == UART_IER)
+			value = serial_in(up, dwapb_offset);
+#endif
  		break;

  	case UPIO_MEM32:
@@ -1016,6 +1039,17 @@
  		up->bugs |= UART_BUG_NOMSR;
  #endif

+	/* Workaround:
+	 * The DesignWare SoC UART part has a bug for all
+	 * versions before 3.03a (2005-07-18)
+	 * In brief, this is a non-standard 16550 in that the THRE interrupt
+	 * will not re-assert itself simply by disabling and re-enabling the
+	 * THRI bit in the IER, it is only re-enabled if a character is actually
+	 * sent out.
+	 */
+	if( up->port.flags & UPF_DW_THRE_BUG )
+		up->bugs |= UART_BUG_DWTHRE;
+
  	serial_outp(up, UART_LCR, save_lcr);

  	if (up->capabilities != uart_config[up->port.type].flags) {
@@ -1141,6 +1175,12 @@
  			iir = serial_in(up, UART_IIR);
  			if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT)
  				transmit_chars(up);
+		} else if (up->bugs & UART_BUG_DWTHRE) {
+			unsigned char lsr, iir;
+			lsr = serial_in(up, UART_LSR);
+			iir = serial_in(up, UART_IIR);
+			if (lsr & UART_LSR_THRE)
+				transmit_chars(up);
  		}
  	}

@@ -1366,6 +1406,31 @@
  			handled = 1;

  			end = NULL;
+#ifdef CONFIG_PMC_MSP
+		} else if ((iir & UART_IER_BUSY) == UART_IER_BUSY) {
+			/*
+			 * The MSP (DesignWare APB UART) serial subsystem has a
+			 * non-standard interrupt condition (0x7) which means
+			 * that the LCR was written while the UART was busy, so
+			 * the LCR was not actually written.  It is cleared by
+			 * reading the special non-standard extended UART status
+			 * register.
+			 */
+			unsigned int tmp;
+			if( up->port.line == 0 )
+				tmp = *UART0_STATUS_REG;
+			else
+				tmp = *UART1_STATUS_REG;
+			
+			/* Check if saved on LCR write */
+			if( up->dwapb_lcr != -1 )
+				serial_outp(up, UART_LCR, up->dwapb_lcr);
+			else
+				printk(KERN_ERR "serial8250: UART BUSY, no LCR write!\n" );
+
+			handled = 1;
+			end = NULL;
+#endif
  		} else if (end == NULL)
  			end = l;

@@ -2191,6 +2256,11 @@
  	for (i = 0; i < nr_uarts; i++) {
  		struct uart_8250_port *up = &serial8250_ports[i];

+#ifdef CONFIG_PMC_MSP
+		/* Initialize saved LCR value */
+		up->dwapb_lcr = -1;
+#endif
+
  		up->port.dev = dev;
  		uart_add_one_port(drv, &up->port);
  	}
Index: linux_2_6/drivers/serial/8250.h
===================================================================
RCS file: linux_2_6/drivers/serial/8250.h,v
retrieving revision 1.1.1.6
retrieving revision 1.4
diff -u -r1.1.1.6 -r1.4
--- linux_2_6/drivers/serial/8250.h	19 Oct 2006 21:00:58 -0000	1.1.1.6
+++ linux_2_6/drivers/serial/8250.h	19 Oct 2006 22:08:15 -0000	1.4
@@ -49,6 +49,7 @@
  #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
  #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
  #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits (Au1x00) */
+#define UART_BUG_DWTHRE (1 << 3)	/* UART has buggy DesignWare THRE interrupt re-assertion */

  #define PROBE_RSA	(1 << 0)
  #define PROBE_ANY	(~0)
Index: linux_2_6/include/linux/serial_core.h
===================================================================
RCS file: linux_2_6/include/linux/serial_core.h,v
retrieving revision 1.1.1.7
retrieving revision 1.5
diff -u -r1.1.1.7 -r1.5
--- linux_2_6/include/linux/serial_core.h	19 Oct 2006 21:01:02 -0000	1.1.1.7
+++ linux_2_6/include/linux/serial_core.h	19 Oct 2006 22:08:16 -0000	1.5
@@ -258,6 +258,8 @@
  #define UPF_CONS_FLOW		((__force upf_t) (1 << 23))
  #define UPF_SHARE_IRQ		((__force upf_t) (1 << 24))
  #define UPF_BOOT_AUTOCONF	((__force upf_t) (1 << 28))
+#define UPF_DW_THRE_BUG		((__force upf_t)(1 << 29)) /* DesignWare THRE hardware BUG
+														* (cannot be autodetected) */
  #define UPF_DEAD		((__force upf_t) (1 << 30))
  #define UPF_IOREMAP		((__force upf_t) (1 << 31))

Index: linux_2_6/include/linux/serial_reg.h
===================================================================
RCS file: linux_2_6/include/linux/serial_reg.h,v
retrieving revision 1.1.1.2
retrieving revision 1.3
diff -u -r1.1.1.2 -r1.3
--- linux_2_6/include/linux/serial_reg.h	19 Oct 2006 18:29:50 -0000	1.1.1.2
+++ linux_2_6/include/linux/serial_reg.h	19 Oct 2006 19:45:04 -0000	1.3
@@ -218,6 +218,10 @@
  #define UART_FCR_PXAR16	0x80	/* receive FIFO treshold = 16 */
  #define UART_FCR_PXAR32	0xc0	/* receive FIFO treshold = 32 */

+/*
+ * DesignWare APB UART
+ */
+#define UART_IER_BUSY		0x07	/* Busy Detect */

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Ralf Baechle]

On Thu, Jan 18, 2007 at 04:23:01PM -0800, Marc St-Jean wrote:

> Index: linux_2_6/drivers/serial/8250.c
> ===================================================================
> RCS file: linux_2_6/drivers/serial/8250.c,v
> retrieving revision 1.1.1.7
> retrieving revision 1.9
> diff -u -r1.1.1.7 -r1.9
> --- linux_2_6/drivers/serial/8250.c	19 Oct 2006 21:00:58 -0000	1.1.1.7
> +++ linux_2_6/drivers/serial/8250.c	19 Oct 2006 22:08:15 -0000	1.9
> @@ -44,6 +44,10 @@
>   #include <asm/io.h>
>   #include <asm/irq.h>
> 
> +#ifdef CONFIG_PMC_MSP
> +#include <msp_regs.h>
> +#endif

CONFIG_PMC_MSP is not defined anywhere.  msp_regs.h does not exist.

  Ralf

Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git master

[Sergei Shtylyov]

Hello.

Marc St-Jean wrote:
> Here is a serial driver patch for the PMC-Sierra MSP71xx device.

> There are three different fixes:
> 1. Fix for THRE errata
> 2. Fix for Busy Detect on LCR write
> 3. Workaround for interrupt/data concurrency issue

> The first fix is handled cleanly using a UART_BUG_* flag.

    Hm, I wouldn't call it clean...

> The second and third fixes use platform tests. I couldn't think of a
> good way to implement them without using tests and not increase code
> and structure sizes for platforms not requiring them.

> Does anyone have any suggestions on implementing these without the
> platform flag?

> Thanks,
> Marc

> Signed-off-by: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>

> Index: linux_2_6/drivers/serial/8250.c
> ===================================================================
> RCS file: linux_2_6/drivers/serial/8250.c,v
> retrieving revision 1.1.1.7
> retrieving revision 1.9
> diff -u -r1.1.1.7 -r1.9
> --- linux_2_6/drivers/serial/8250.c	19 Oct 2006 21:00:58 -0000	1.1.1.7
> +++ linux_2_6/drivers/serial/8250.c	19 Oct 2006 22:08:15 -0000	1.9
> @@ -44,6 +44,10 @@
>   #include <asm/io.h>
>   #include <asm/irq.h>
> 
> +#ifdef CONFIG_PMC_MSP
> +#include <msp_regs.h>
> +#endif
> +
>   #include "8250.h"
> 
>   /*
> @@ -130,6 +134,9 @@
>   	unsigned char		mcr_mask;	/* mask of user bits */
>   	unsigned char		mcr_force;	/* mask of forced bits */
>   	unsigned char		lsr_break_flag;
> +#ifdef CONFIG_PMC_MSP
> +	int					dwapb_lcr;	/* saved LCR for DW APB WAR */
> +#endif

    There was already 'lcr' field there, couldn't it be used?

> @@ -333,6 +340,10 @@
>   static void
>   serial_out(struct uart_8250_port *up, int offset, int value)
>   {
> +#ifdef CONFIG_PMC_MSP
> +	/* Save the offset before it's remapped */
> +	int dwapb_offset = offset;
> +#endif
>   	offset = map_8250_out_reg(up, offset) << up->port.regshift;
> 
>   	switch (up->port.iotype) {
> @@ -342,7 +353,19 @@
>   		break;
> 
>   	case UPIO_MEM:
> +#ifdef CONFIG_PMC_MSP
> +		/* Save the LCR value so it can be re-written when a
> +		 * Busy Detect interrupt occurs. */
> +		if (dwapb_offset == UART_LCR)
> +			up->dwapb_lcr = value;
> +#endif
>   		writeb(value, up->port.membase + offset);
> +#ifdef CONFIG_PMC_MSP
> +		/* Re-read the IER to ensure any interrupt disabling has
> +		 * completed before proceeding with ISR. */
> +		if (dwapb_offset == UART_IER)
> +			value = serial_in(up, dwapb_offset);
> +#endif
>   		break;

    Hm, was there really a need for #ifdef mess here?
    I'd vote for introducing new UPIO_* here, like was done for TSi10x UARTs
just for the same reason.

> @@ -1016,6 +1039,17 @@
>   		up->bugs |= UART_BUG_NOMSR;
>   #endif
> 
> +	/* Workaround:
> +	 * The DesignWare SoC UART part has a bug for all
> +	 * versions before 3.03a (2005-07-18)
> +	 * In brief, this is a non-standard 16550 in that the THRE interrupt
> +	 * will not re-assert itself simply by disabling and re-enabling the
> +	 * THRI bit in the IER, it is only re-enabled if a character is actually
> +	 * sent out.
> +	 */
> +	if( up->port.flags & UPF_DW_THRE_BUG )
> +		up->bugs |= UART_BUG_DWTHRE;
> +
>   	serial_outp(up, UART_LCR, save_lcr);
> 
>   	if (up->capabilities != uart_config[up->port.type].flags) {
> @@ -1141,6 +1175,12 @@
>   			iir = serial_in(up, UART_IIR);
>   			if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT)
>   				transmit_chars(up);
> +		} else if (up->bugs & UART_BUG_DWTHRE) {
> +			unsigned char lsr, iir;
> +			lsr = serial_in(up, UART_LSR);
> +			iir = serial_in(up, UART_IIR);
> +			if (lsr & UART_LSR_THRE)
> +				transmit_chars(up);

    I don't see how this *really* differs from the UART_BUG_TXEN case.
    Have you tried *that* workaround? In any case, looks like this errata
is auto-detectable just like UART_BUG_TXEN.

> @@ -1366,6 +1406,31 @@
>   			handled = 1;
> 
>   			end = NULL;
> +#ifdef CONFIG_PMC_MSP
> +		} else if ((iir & UART_IER_BUSY) == UART_IER_BUSY) {

    Hm, masking IIR with IER mask, is this correct? Doubt it.

> +			/*
> +			 * The MSP (DesignWare APB UART) serial subsystem has a
> +			 * non-standard interrupt condition (0x7) which means
> +			 * that the LCR was written while the UART was busy, so
> +			 * the LCR was not actually written.  It is cleared by
> +			 * reading the special non-standard extended UART status
> +			 * register.
> +			 */
> +			unsigned int tmp;
> +			if( up->port.line == 0 )
> +				tmp = *UART0_STATUS_REG;
> +			else
> +				tmp = *UART1_STATUS_REG;
> +			
> +			/* Check if saved on LCR write */
> +			if( up->dwapb_lcr != -1 )
> +				serial_outp(up, UART_LCR, up->dwapb_lcr);
> +			else
> +				printk(KERN_ERR "serial8250: UART BUSY, no LCR write!\n" );
> +
> +			handled = 1;
> +			end = NULL;
> +#endif

    Not sure if this also shouldn't be handled in other places which check for 
interrupt status, like serial8250_timeout()...

[...]
> Index: linux_2_6/drivers/serial/8250.h
> ===================================================================
> RCS file: linux_2_6/drivers/serial/8250.h,v
> retrieving revision 1.1.1.6
> retrieving revision 1.4
> diff -u -r1.1.1.6 -r1.4
> --- linux_2_6/drivers/serial/8250.h	19 Oct 2006 21:00:58 -0000	1.1.1.6
> +++ linux_2_6/drivers/serial/8250.h	19 Oct 2006 22:08:15 -0000	1.4
> @@ -49,6 +49,7 @@
>   #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
>   #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
>   #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits (Au1x00) */
> +#define UART_BUG_DWTHRE (1 << 3)	/* UART has buggy DesignWare THRE interrupt re-assertion */
> 
>   #define PROBE_RSA	(1 << 0)
>   #define PROBE_ANY	(~0)
> Index: linux_2_6/include/linux/serial_core.h
> ===================================================================
> RCS file: linux_2_6/include/linux/serial_core.h,v
> retrieving revision 1.1.1.7
> retrieving revision 1.5
> diff -u -r1.1.1.7 -r1.5
> --- linux_2_6/include/linux/serial_core.h	19 Oct 2006 21:01:02 -0000	1.1.1.7
> +++ linux_2_6/include/linux/serial_core.h	19 Oct 2006 22:08:16 -0000	1.5
> @@ -258,6 +258,8 @@
>   #define UPF_CONS_FLOW		((__force upf_t) (1 << 23))
>   #define UPF_SHARE_IRQ		((__force upf_t) (1 << 24))
>   #define UPF_BOOT_AUTOCONF	((__force upf_t) (1 << 28))
> +#define UPF_DW_THRE_BUG		((__force upf_t)(1 << 29)) /* DesignWare THRE hardware BUG

    The need for the new flag seems doubtful to me.

> +														* (cannot be autodetected) */

    The patch is linewrapped

> Index: linux_2_6/include/linux/serial_reg.h
> ===================================================================
> RCS file: linux_2_6/include/linux/serial_reg.h,v
> retrieving revision 1.1.1.2
> retrieving revision 1.3
> diff -u -r1.1.1.2 -r1.3
> --- linux_2_6/include/linux/serial_reg.h	19 Oct 2006 18:29:50 -0000	1.1.1.2
> +++ linux_2_6/include/linux/serial_reg.h	19 Oct 2006 19:45:04 -0000	1.3
> @@ -218,6 +218,10 @@
>   #define UART_FCR_PXAR16	0x80	/* receive FIFO treshold = 16 */
>   #define UART_FCR_PXAR32	0xc0	/* receive FIFO treshold = 32 */
> 
> +/*
> + * DesignWare APB UART
> + */
> +#define UART_IER_BUSY		0x07	/* Busy Detect */

    Are you sure it's not *IIR* value?  Doesn't look like interrupt mask for 
IER. And IIR value of 7 already means something else, namely, no interrupt and
receiver status. Hm...

MBR, Sergei