omap2/3/4: serial fixes

PATCH In review Nishanth Menon 3 participants 8 messages Jun 23, 2010 – Jul 29, 2010

Subsystems (3)

Arm (32-bit) Arm sub-architectures OMAP2+ support

lore

Share & tools

lore

Nishanth Menon

author

2010-06-23 04:18 UTC #0 lore raw

v2 cover omap2/3/4: serial fixes

V1:
http://marc.info/?l=linux-omap&m=127074926903371&w=2

The patch for Errata i202 went through a series of discussions ending in
V3:
http://marc.info/?l=linux-omap&m=127109794814030&w=2

This series introduces an errata variable, does a few sparse cleanups and provides fix for an new errata.

Deepak K (1):
omap2/3/4: serial: errata i202: fix for MDR1 access

Govindraj R (1):
omap3: serial: Add context save and restore for mcr

Nishanth Menon(3):
omap2/3/4: serial: remove initialization sparse warnings omap2/3/4: serial: kill dev_attr_sleep_timeout sparse warn omap2/3/4: serial: introduce errata handling

 arch/arm/mach-omap2/serial.c |   80 ++++++++++++++++++++++++++++++++++++------
 1 files changed, 69 insertions(+), 11 deletions(-)

Regards,
Nishanth Menon

Cc Deepak K <[email protected]>

Cc Govindraj R <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Tero Kristo <[email protected]>

Cc Tony Lindgren <[email protected]>

Nishanth Menon

author

2010-06-23 04:18 UTC #1 lore raw

PATCH v2 1/5 omap3: serial: Add context save and restore for mcr

From: Govindraj R <[email protected]>

Ref:
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=c74952c3077e4b19e649b5b48c785c01f73ab9d4

Adds context save/restore for mcr register as state of mcr register is lost after core off.

Cc Deepak K <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Tero Kristo <[email protected]>

Cc Tony Lindgren <[email protected]>

Signed-off-by Govindraj R <[email protected]>

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/serial.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/serial.c +6 -0

index 3771254..804dbb2 100644
@@ -74,6 +74,7 @@ struct omap_uart_state {
 	u16 sysc;
 	u16 scr;
 	u16 wer;
+	u16 mcr;
 #endif
 };

@@ -197,6 +198,9 @@ static void omap_uart_save_context(struct omap_uart_state *uart)
 	uart->sysc = serial_read_reg(p, UART_OMAP_SYSC);
 	uart->scr = serial_read_reg(p, UART_OMAP_SCR);
 	uart->wer = serial_read_reg(p, UART_OMAP_WER);
+	serial_write_reg(p, UART_LCR, 0x80);
+	uart->mcr = serial_read_reg(p, UART_MCR);
+	serial_write_reg(p, UART_LCR, lcr);
 
 	uart->context_valid = 1;
 }

@@ -225,6 +229,8 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
 	serial_write_reg(p, UART_DLM, uart->dlh);
 	serial_write_reg(p, UART_LCR, 0x0); /* Operational mode */
 	serial_write_reg(p, UART_IER, uart->ier);
+	serial_write_reg(p, UART_LCR, 0x80);
+	serial_write_reg(p, UART_MCR, uart->mcr);
 	serial_write_reg(p, UART_FCR, 0xA1);
 	serial_write_reg(p, UART_LCR, 0xBF); /* Config B mode */
 	serial_write_reg(p, UART_EFR, efr);

Signature · 1 line

1.6.3.3

Nishanth Menon

author

2010-06-23 04:18 UTC #2 lore raw

PATCH v2 2/5 omap2/3/4: serial: remove initialization sparse warnings

In reply to Nishanth Menon

Initialization of pointer should be done with NULL. Removes sparse warnings:
arch/arm/mach-omap2/serial.c:566:17: warning: Using plain integer as NULL pointer arch/arm/mach-omap2/serial.c:567:17: warning: Using plain integer as NULL pointer

Cc Deepak K <[email protected]>

Cc Govindraj R <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Tero Kristo <[email protected]>

Cc Tony Lindgren <[email protected]>

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/serial.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/serial.c +2 -2

index 804dbb2..9476c1c 100644
@@ -495,8 +495,8 @@ static void omap_uart_idle_init(struct omap_uart_state *uart)
 		}
 		uart->wk_mask = wk_mask;
 	} else {
-		uart->wk_en = 0;
-		uart->wk_st = 0;
+		uart->wk_en = NULL;
+		uart->wk_st = NULL;
 		uart->wk_mask = 0;
 		uart->padconf = 0;
 	}

Signature · 1 line

1.6.3.3

Nishanth Menon

author

2010-06-23 04:18 UTC #3 lore raw

PATCH v2 3/5 omap2/3/4: serial: kill dev_attr_sleep_timeout sparse warn

In reply to Nishanth Menon

Remove the following sparse warnings by declaring attr as static:
arch/arm/mach-omap2/serial.c:627:1: warning: symbol 'dev_attr_sleep_timeout' was not declared. Should it be static?

Cc Deepak K <[email protected]>

Cc Govindraj R <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Tero Kristo <[email protected]>

Cc Tony Lindgren <[email protected]>

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/serial.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/serial.c +2 -1

index 9476c1c..584a2ed 100644
@@ -558,7 +558,8 @@ static ssize_t sleep_timeout_store(struct device *dev,
 	return n;
 }
 
-DEVICE_ATTR(sleep_timeout, 0644, sleep_timeout_show, sleep_timeout_store);
+static DEVICE_ATTR(sleep_timeout, 0644, sleep_timeout_show,
+		sleep_timeout_store);
 #define DEV_CREATE_FILE(dev, attr) WARN_ON(device_create_file(dev, attr))
 #else
 static inline void omap_uart_idle_init(struct omap_uart_state *uart) {}

Signature · 1 line

1.6.3.3

Nishanth Menon

author

2010-06-23 04:18 UTC #4 lore raw

PATCH v2 4/5 omap2/3/4: serial: introduce errata handling

In reply to Nishanth Menon

introduce silicon specific quirks as a errata handling mechanism

as a start UART_ERRATA_FIFO_FULL_ABORT is used to handle the override for fifo full condition for rx and tx.

Cc Deepak K <[email protected]>

Cc Govindraj R <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Tero Kristo <[email protected]>

Cc Tony Lindgren <[email protected]>

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/serial.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

arch/arm/mach-omap2/serial.c +10 -5

index 584a2ed..009b63f 100644
@@ -37,6 +37,8 @@
 #define UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV	0x52
 #define UART_OMAP_WER		0x17	/* Wake-up enable register */
 
+#define UART_ERRATA_FIFO_FULL_ABORT	(0x1 << 0)
+
 /*
  * NOTE: By default the serial timeout is disabled as it causes lost characters
  * over the serial ports. This means that the UART clocks will stay on until

@@ -64,6 +66,7 @@ struct omap_uart_state {
 	struct list_head node;
 	struct platform_device pdev;
 
+	u32 errata;
 #if defined(CONFIG_ARCH_OMAP3) && defined(CONFIG_PM)
 	int context_valid;

@@ -756,11 +759,13 @@ void __init omap_serial_init_port(int port)
 	 * omap3xxx: Never read empty UART fifo on UARTs
 	 * with IP rev >=0x52
 	 */
-	if (cpu_is_omap44xx()) {
-		uart->p->serial_in = serial_in_override;
-		uart->p->serial_out = serial_out_override;
-	} else if ((serial_read_reg(uart->p, UART_OMAP_MVER) & 0xFF)
-			>= UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV) {
+	if (cpu_is_omap44xx())
+		uart->errata |= UART_ERRATA_FIFO_FULL_ABORT;
+	else if ((serial_read_reg(uart->p, UART_OMAP_MVER) & 0xFF)
+			>= UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV)
+		uart->errata |= UART_ERRATA_FIFO_FULL_ABORT;
+
+	if (uart->errata & UART_ERRATA_FIFO_FULL_ABORT) {
 		uart->p->serial_in = serial_in_override;
 		uart->p->serial_out = serial_out_override;
 	}

Signature · 1 line

1.6.3.3

Nishanth Menon

author

2010-06-23 04:18 UTC #5 lore raw

PATCH v4 5/5 omap2/3/4: serial: errata i202: fix for MDR1 access

In reply to Nishanth Menon

From: Deepak K <[email protected]>

Original patch:
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=42d4a342c009bd9727c100abc8a4bc3063c22f0c

Errata i202 (OMAP3430 - 1.12, OMAP3630 - 1.6):
UART module MDR1 register access can cause a dummy underrun condition which could result in a freeze in the case of IrDA communication or if used as UART, corrupted data.

Workaround is as follows for everytime MDR1 register is changed:
* setup all required UART registers
* setup MDR1.MODE_SELECT bit field
* Wait 5 L4 clk cycles + 5 UART functional clock cycles
* Clear the Tx and RX fifo using FCR register

Note: The following step is not done as I am assuming it is not needed due to reconfiguration being done and there is no halted operation perse.
* Read if required, the RESUME register to resume halted operation

Cc Govindraj R <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Tero Kristo <[email protected]>

Cc Tony Lindgren <[email protected]>

Signed-off-by Deepak K <[email protected]>

Signed-off-by Nishanth Menon <[email protected]>

---
 arch/arm/mach-omap2/serial.c |   52 +++++++++++++++++++++++++++++++++++++++--
 1 files changed, 49 insertions(+), 3 deletions(-)

arch/arm/mach-omap2/serial.c +49 -3

index 009b63f..566e991 100644
@@ -38,6 +38,7 @@
 #define UART_OMAP_WER		0x17	/* Wake-up enable register */
 
 #define UART_ERRATA_FIFO_FULL_ABORT	(0x1 << 0)
+#define UART_ERRATA_i202_MDR1_ACCESS	(0x1 << 1)
 
 /*
  * NOTE: By default the serial timeout is disabled as it causes lost characters

@@ -184,6 +185,42 @@ static inline void __init omap_uart_reset(struct omap_uart_state *uart)
 
 #if defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3)
 
+/*
+ * Work Around for Errata i202 (3430 - 1.12, 3630 - 1.6)
+ * The access to uart register after MDR1 Access
+ * causes UART to corrupt data.
+ *
+ * Need a delay =
+ * 5 L4 clock cycles + 5 UART functional clock cycle (@48MHz = ~0.2uS)
+ * give 10 times as much
+ */
+static void omap_uart_mdr1_errataset(struct omap_uart_state *uart, u8 mdr1_val,
+		u8 fcr_val)
+{
+	struct plat_serial8250_port *p = uart->p;
+	u8 timeout = 255;
+
+	serial_write_reg(p, UART_OMAP_MDR1, mdr1_val);
+	udelay(2);
+	serial_write_reg(p, UART_FCR, fcr_val | UART_FCR_CLEAR_XMIT |
+			UART_FCR_CLEAR_RCVR);
+	/*
+	 * Wait for FIFO to empty: when empty, RX_FIFO_E bit is 0 and
+	 * TX_FIFO_E bit is 1.
+	 */
+	while (UART_LSR_THRE != (serial_read_reg(p, UART_LSR) &
+				(UART_LSR_THRE | UART_LSR_DR))) {
+		timeout--;
+		if (!timeout) {
+			/* Should *never* happen. we warn and carry on */
+			dev_crit(&uart->pdev.dev, "Errata i202: timedout %x\n",
+				serial_read_reg(p, UART_LSR));
+			break;
+		}
+		udelay(1);
+	}
+}
+
 static void omap_uart_save_context(struct omap_uart_state *uart)
 {
 	u16 lcr = 0;

@@ -221,7 +258,10 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
 
 	uart->context_valid = 0;
 
-	serial_write_reg(p, UART_OMAP_MDR1, 0x7);
+	if (uart->errata & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_uart_mdr1_errataset(uart, 0x07, 0xA0);
+	else
+		serial_write_reg(p, UART_OMAP_MDR1, 0x7);
 	serial_write_reg(p, UART_LCR, 0xBF); /* Config B mode */
 	efr = serial_read_reg(p, UART_EFR);
 	serial_write_reg(p, UART_EFR, UART_EFR_ECB);

@@ -234,14 +274,16 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
 	serial_write_reg(p, UART_IER, uart->ier);
 	serial_write_reg(p, UART_LCR, 0x80);
 	serial_write_reg(p, UART_MCR, uart->mcr);
-	serial_write_reg(p, UART_FCR, 0xA1);
 	serial_write_reg(p, UART_LCR, 0xBF); /* Config B mode */
 	serial_write_reg(p, UART_EFR, efr);
 	serial_write_reg(p, UART_LCR, UART_LCR_WLEN8);
 	serial_write_reg(p, UART_OMAP_SCR, uart->scr);
 	serial_write_reg(p, UART_OMAP_WER, uart->wer);
 	serial_write_reg(p, UART_OMAP_SYSC, uart->sysc);
-	serial_write_reg(p, UART_OMAP_MDR1, 0x00); /* UART 16x mode */
+	if (uart->errata & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_uart_mdr1_errataset(uart, 0x00, 0xA1);
+	else
+		serial_write_reg(p, UART_OMAP_MDR1, 0x00); /* UART 16x mode */
 }
 #else
 static inline void omap_uart_save_context(struct omap_uart_state *uart) {}

@@ -769,6 +811,10 @@ void __init omap_serial_init_port(int port)
 		uart->p->serial_in = serial_in_override;
 		uart->p->serial_out = serial_out_override;
 	}
+
+	/* Enable the MDR1 errata for OMAP3 */
+	if (cpu_is_omap34xx())
+		uart->errata |= UART_ERRATA_i202_MDR1_ACCESS;
 }
 
 /**

Signature · 1 line

1.6.3.3

Roger Quadros

2010-07-29 09:07 UTC #6 lore raw

Re: [PATCH 5/5 v4] omap2/3/4: serial: errata i202: fix for MDR1 access

Hi,

Quoted 88 lines · ext Nishanth Menon

From: Deepak K<[email protected]>
Original patch:
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=42d4a342c009bd9727c100abc8a4bc3063c22f0c
Errata i202 (OMAP3430 - 1.12, OMAP3630 - 1.6):
UART module MDR1 register access can cause a dummy underrun condition which could result in a freeze in the case of IrDA communication or if used as UART, corrupted data.
Workaround is as follows for everytime MDR1 register is changed:
* setup all required UART registers
* setup MDR1.MODE_SELECT bit field
* Wait 5 L4 clk cycles + 5 UART functional clock cycles
* Clear the Tx and RX fifo using FCR register
Note: The following step is not done as I am assuming it is not needed due to reconfiguration being done and there is no halted operation perse.
* Read if required, the RESUME register to resume halted operation
Cc Govindraj R<[email protected]>
Cc Kevin Hilman<[email protected]>
Cc Tero Kristo<[email protected]>
Cc Tony Lindgren<[email protected]>
Signed-off-by Deepak K<[email protected]>
Signed-off-by Nishanth Menon<[email protected]>
---
  arch/arm/mach-omap2/serial.c |   52 +++++++++++++++++++++++++++++++++++++++--
  1 files changed, 49 insertions(+), 3 deletions(-)
arch/arm/mach-omap2/serial.c +37 -0
index 009b63f..566e991 100644
@@ -38,6 +38,7 @@
  #define UART_OMAP_WER		0x17	/* Wake-up enable register */
 
  #define UART_ERRATA_FIFO_FULL_ABORT	(0x1<<  0)
+#define UART_ERRATA_i202_MDR1_ACCESS	(0x1<<  1)
 
  /*
   * NOTE: By default the serial timeout is disabled as it causes lost characters
@@ -184,6 +185,42 @@ static inline void __init omap_uart_reset(struct omap_uart_state *uart)
 
  #if defined(CONFIG_PM)&&  defined(CONFIG_ARCH_OMAP3)
 
+/*
+ * Work Around for Errata i202 (3430 - 1.12, 3630 - 1.6)
+ * The access to uart register after MDR1 Access
+ * causes UART to corrupt data.
+ *
+ * Need a delay =
+ * 5 L4 clock cycles + 5 UART functional clock cycle (@48MHz = ~0.2uS)
+ * give 10 times as much
+ */
+static void omap_uart_mdr1_errataset(struct omap_uart_state *uart, u8 mdr1_val,
+		u8 fcr_val)
+{
+	struct plat_serial8250_port *p = uart->p;
+	u8 timeout = 255;
+
+	serial_write_reg(p, UART_OMAP_MDR1, mdr1_val);
+	udelay(2);
+	serial_write_reg(p, UART_FCR, fcr_val | UART_FCR_CLEAR_XMIT |
+			UART_FCR_CLEAR_RCVR);
+	/*
+	 * Wait for FIFO to empty: when empty, RX_FIFO_E bit is 0 and
+	 * TX_FIFO_E bit is 1.
+	 */
+	while (UART_LSR_THRE != (serial_read_reg(p, UART_LSR)&
+				(UART_LSR_THRE | UART_LSR_DR))) {
+		timeout--;
+		if (!timeout) {
+			/* Should *never* happen. we warn and carry on */
+			dev_crit(&uart->pdev.dev, "Errata i202: timedout %x\n",
+				serial_read_reg(p, UART_LSR));
+			break;
+		}
+		udelay(1);
+	}
+}
+
  static void omap_uart_save_context(struct omap_uart_state *uart)
  {
  	u16 lcr = 0;

arch/arm/mach-omap2/serial.c static void omap_uart_restore_context(struct omap_uart_state *uart) · line 262

Quoted 6 lines

arch/arm/mach-omap2/serial.c +2 -1

@@ -221,7 +258,10 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
 
  	uart->context_valid = 0;
 
-	serial_write_reg(p, UART_OMAP_MDR1, 0x7);
+	if (uart->errata&  UART_ERRATA_i202_MDR1_ACCESS)
+		omap_uart_mdr1_errataset(uart, 0x07, 0xA0);

If errata you set both MDR1 and FCR

arch/arm/mach-omap2/serial.c static void omap_uart_restore_context(struct omap_uart_state *uart) · line 264

Quoted 2 lines

arch/arm/mach-omap2/serial.c +2 -0
+	else
+		serial_write_reg(p, UART_OMAP_MDR1, 0x7);

If not errata you set only MDR1

arch/arm/mach-omap2/serial.c static void omap_uart_restore_context(struct omap_uart_state *uart) · line 237

Quoted 8 lines

arch/arm/mach-omap2/serial.c +0 -1

  	serial_write_reg(p, UART_LCR, 0xBF); /* Config B mode */
  	efr = serial_read_reg(p, UART_EFR);
  	serial_write_reg(p, UART_EFR, UART_EFR_ECB);

@@ -234,14 +274,16 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
  	serial_write_reg(p, UART_IER, uart->ier);
  	serial_write_reg(p, UART_LCR, 0x80);
  	serial_write_reg(p, UART_MCR, uart->mcr);
-	serial_write_reg(p, UART_FCR, 0xA1);

If not errata, FCR is never set.

arch/arm/mach-omap2/serial.c static void omap_uart_restore_context(struct omap_uart_state *uart) · line 286

Quoted 11 lines

arch/arm/mach-omap2/serial.c +4 -1

  	serial_write_reg(p, UART_LCR, 0xBF); /* Config B mode */
  	serial_write_reg(p, UART_EFR, efr);
  	serial_write_reg(p, UART_LCR, UART_LCR_WLEN8);
  	serial_write_reg(p, UART_OMAP_SCR, uart->scr);
  	serial_write_reg(p, UART_OMAP_WER, uart->wer);
  	serial_write_reg(p, UART_OMAP_SYSC, uart->sysc);
-	serial_write_reg(p, UART_OMAP_MDR1, 0x00); /* UART 16x mode */
+	if (uart->errata&  UART_ERRATA_i202_MDR1_ACCESS)
+		omap_uart_mdr1_errataset(uart, 0x00, 0xA1);
+	else
+		serial_write_reg(p, UART_OMAP_MDR1, 0x00); /* UART 16x mode */

same here

FCR is never touched if no errata condition?
Is this going to be a problem for non erroneous devices?

Quoted 14 lines

arch/arm/mach-omap2/serial.c +4 -0

  }
  #else
  static inline void omap_uart_save_context(struct omap_uart_state *uart) {}

@@ -769,6 +811,10 @@ void __init omap_serial_init_port(int port)
  		uart->p->serial_in = serial_in_override;
  		uart->p->serial_out = serial_out_override;
  	}
+
+	/* Enable the MDR1 errata for OMAP3 */
+	if (cpu_is_omap34xx())
+		uart->errata |= UART_ERRATA_i202_MDR1_ACCESS;
  }
 
  /**

Tony Lindgren

maintainer

2010-07-07 09:54 UTC #7 lore raw

In reply to Nishanth Menon

Hi,

Quoted 10 lines · Nishanth Menon

V1:
http://marc.info/?l=linux-omap&m=127074926903371&w=2
The patch for Errata i202 went through a series of discussions ending in
V3:
http://marc.info/?l=linux-omap&m=127109794814030&w=2
This series introduces an errata variable, does a few sparse cleanups and provides fix for an new errata.

I've added these to devel-serial-errata branch with some minor cosmetic edits. Left out the obvious Cc for people that we know are reading the mailing lists, then left out the link for the first patch, and moved the link in the last patch at the end of the commit message.

Will repost shortly after some tests so they get reviewed also on LAKML.

Regards,

Tony