Skip to contentSkip to conversation

OMAP3: Serial Misc fixes

PATCH Superseded Nishanth Menon 3 participants 10 messages Apr 8, 2010 – May 5, 2010

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

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Nishanth Menon

author
#0 lore raw

cover OMAP3: Serial Misc fixes

Hi All,

Starting off from Android kernel, found a few fixes whic have been backported to l-o pm branch and a basic sanity tests run against SDP3430/SDP3630.

Deepak K (1):
OMAP3: Serial: Errata i202: fix for MDR1 access

Govindraj R (1):
OMAP3: Serial: Add context save and restore for mcr

Nishanth Menon (2):
OMAP3: Serial: remove initialization sparse warnings
OMAP3: Serial: kill dev_attr_sleep_timeout sparse warn

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

Regards,
Nishanth Menon

Cc Deepak K <[email protected]>
Cc Govindraj R <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Tero Kristo <[email protected]>
author
#1 lore raw

PATCH 1/4 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 Kevin Hilman <[email protected]>
Cc Tero Kristo <[email protected]>
Signed-off-by Govindraj R <[email protected]>
Signed-off-by Nishanth Menon <[email protected]>

---
Note: Original Patch was done for android kernel, was ported and sanity tested to l-o pm. A quick check did not reveal any mcr access in arch/arm/mach-omap2/serial.c though drivers/serial/8250.c seems to hit on it a lot, hence posting.

 arch/arm/mach-omap2/serial.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/serial.c +6 -0
index b731556..86d3b47 100644
@@ -78,6 +78,7 @@ struct omap_uart_state {
 	u16 sysc;
 	u16 scr;
 	u16 wer;
+	u16 mcr;
 #endif
 };
 

@@ -201,6 +202,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;
 }

@@ -229,6 +233,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
author
#2 lore raw

PATCH 2/4 OMAP3: Serial: Errata i202: fix for MDR1 access

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]>
Signed-off-by Deepak K <[email protected]>
Signed-off-by Nishanth Menon <[email protected]>

---
Note: original patch was done for android kernel, ported and sanity tested

 arch/arm/mach-omap2/serial.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/serial.c +17 -1
index 86d3b47..144033f 100644
@@ -223,6 +223,19 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
 	uart->context_valid = 0;
 
 	serial_write_reg(p, UART_OMAP_MDR1, 0x7);
+	/*
+	 * 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
+	 */
+	udelay(2);
+	/* TX and RX FIFO Clear; FIFO dis */
+	serial_write_reg(p, UART_FCR, 0xA6);
+
 	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);

@@ -235,7 +248,6 @@ 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);

@@ -243,6 +255,10 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
 	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 */
+	/* Errata: i202 */
+	udelay(2);
+	/* TX and RX FIFO Clear; FIFO en */
+	serial_write_reg(p, UART_FCR, 0xA7);
 }
 #else
 static inline void omap_uart_save_context(struct omap_uart_state *uart) {}
Signature · 1 line
1.6.3.3
author
#3 lore raw

PATCH 3/4 OMAP3: Serial: remove initialization sparse warnings

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 Govindraj R <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Tero Kristo <[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 144033f..c7b2326 100644
@@ -563,8 +563,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
author
#4 lore raw

PATCH 4/4 OMAP3: Serial: kill dev_attr_sleep_timeout sparse warn

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 Govindraj R <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Tero Kristo <[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 c7b2326..cea0a9e 100644
@@ -624,7 +624,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
#5 lore raw

Re: [PM][PATCH 4/4] OMAP3: Serial: kill dev_attr_sleep_timeout sparse warn

Quoted 8 lines · 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 Govindraj R <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Tero Kristo <[email protected]>
Signed-off-by Nishanth Menon <[email protected]>

Thanks, adding to pm-next, queing for 2.6.35.

Kevin

Quoted 20 lines
---
 arch/arm/mach-omap2/serial.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/serial.c +2 -1
index c7b2326..cea0a9e 100644
@@ -624,7 +624,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
author NAK
#6 lore raw

Re: [PM][PATCH 2/4] OMAP3: Serial: Errata i202: fix for MDR1 access

In reply to Nishanth Menon

Menon, Nishanth had written, on 04/08/2010 12:54 PM, the following:

Quoted 1 line

From: Deepak K <[email protected]>

[...]

Quoted 24 lines
Cc Govindraj R <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Tero Kristo <[email protected]>
Signed-off-by Deepak K <[email protected]>
Signed-off-by Nishanth Menon <[email protected]>

---
Note: original patch was done for android kernel, ported and sanity tested

 arch/arm/mach-omap2/serial.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/serial.c +2 -0
index 86d3b47..144033f 100644
@@ -223,6 +223,19 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
 	uart->context_valid = 0;
 
 	serial_write_reg(p, UART_OMAP_MDR1, 0x7);
+	/*
+	 * Work Around for Errata i202 (3430 - 1.12, 3630 - 1.6)

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

Quoted 10 lines
arch/arm/mach-omap2/serial.c +10 -0
+	 * 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
+	 */
+	udelay(2);
+	/* TX and RX FIFO Clear; FIFO dis */
+	serial_write_reg(p, UART_FCR, 0xA6);

NAK
Late realization: should be checking if FIFO clear actually occurred before proceeding (FIFO clear bits will be cleared by h/w once it is complete, so need to add a
/* wait till TX and RX fifos are cleard by h/w */
while(serial_read_reg(p, UART_FCR) & 0x06)

	;
Quoted 13 lines
arch/arm/mach-omap2/serial.c +1 -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);

@@ -235,7 +248,6 @@ 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);

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

Quoted 7 lines
arch/arm/mach-omap2/serial.c +4 -0
@@ -243,6 +255,10 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
 	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 */
+	/* Errata: i202 */
+	udelay(2);
+	/* TX and RX FIFO Clear; FIFO en */
+	serial_write_reg(p, UART_FCR, 0xA7);

NAK
same reason

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

Quoted 3 lines
arch/arm/mach-omap2/serial.c +0 -0
 }
 #else
 static inline void omap_uart_save_context(struct omap_uart_state *uart) {}

I will post a v2 patch in a few mins..

Signature · 2 lines
Regards,
Nishanth Menon
NAK
#7 lore raw

RE: [PM][PATCH 2/4] OMAP3: Serial: Errata i202: fix for MDR1 access

Hi Nishanth

My comments below.

Regards
Deepak

-----Original Message-----
From: Menon, Nishanth
Sent: Friday, April 09, 2010 12:00 PM
To: linux-omap
Cc: Kattungal, Deepak; Raja, Govindraj; Kevin Hilman; Tero Kristo
Subject: Re: [PM][PATCH 2/4] OMAP3: Serial: Errata i202: fix for MDR1 access

Menon, Nishanth had written, on 04/08/2010 12:54 PM, the following:

Quoted 1 line

From: Deepak K <[email protected]>

[...]

Quoted 24 lines
Cc Govindraj R <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Tero Kristo <[email protected]>
Signed-off-by Deepak K <[email protected]>
Signed-off-by Nishanth Menon <[email protected]>

---
Note: original patch was done for android kernel, ported and sanity tested

 arch/arm/mach-omap2/serial.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/serial.c +2 -0
index 86d3b47..144033f 100644
@@ -223,6 +223,19 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
 	uart->context_valid = 0;
 
 	serial_write_reg(p, UART_OMAP_MDR1, 0x7);
+	/*
+	 * Work Around for Errata i202 (3430 - 1.12, 3630 - 1.6)

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

Quoted 10 lines
arch/arm/mach-omap2/serial.c +10 -0
+	 * 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
+	 */
+	udelay(2);
+	/* TX and RX FIFO Clear; FIFO dis */
+	serial_write_reg(p, UART_FCR, 0xA6);

NAK
Late realization: should be checking if FIFO clear actually occurred before proceeding (FIFO clear bits will be cleared by h/w once it is complete,
so need to add a
/* wait till TX and RX fifos are cleard by h/w */ while(serial_read_reg(p, UART_FCR) & 0x06)

	;

Deepak : The FCR is a Write-Only Register, the read to it wont return the correct value. If at all we need to add a check, the LSR should be read for the status of the FIFO. The same follows for the below section also.

Quoted 13 lines
arch/arm/mach-omap2/serial.c +1 -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);

@@ -235,7 +248,6 @@ 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);

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

Quoted 7 lines
arch/arm/mach-omap2/serial.c +4 -0
@@ -243,6 +255,10 @@ static void omap_uart_restore_context(struct omap_uart_state *uart)
 	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 */
+	/* Errata: i202 */
+	udelay(2);
+	/* TX and RX FIFO Clear; FIFO en */
+	serial_write_reg(p, UART_FCR, 0xA7);

NAK
same reason

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

Quoted 3 lines
arch/arm/mach-omap2/serial.c +0 -0
 }
 #else
 static inline void omap_uart_save_context(struct omap_uart_state *uart) {}

I will post a v2 patch in a few mins..

Signature · 2 lines
Regards,
Nishanth Menon
author
#8 lore raw

Re: [PM][PATCH 2/4] OMAP3: Serial: Errata i202: fix for MDR1 access

Kattungal, Deepak had written, on 04/09/2010 12:06 PM, the following:
[..]

Quoted 11 lines

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

Quoted 2 lines
arch/arm/mach-omap2/serial.c +2 -0
+	/* TX and RX FIFO Clear; FIFO dis */
+	serial_write_reg(p, UART_FCR, 0xA6);

NAK
Late realization: should be checking if FIFO clear actually occurred before proceeding (FIFO clear bits will be cleared by h/w once it is complete, so need to add a
/* wait till TX and RX fifos are cleard by h/w */
while(serial_read_reg(p, UART_FCR) & 0x06)

	;

Deepak : The FCR is a Write-Only Register, the read to it wont return the correct value. If at all we need to add a check, the LSR should be read for the status of the FIFO. The same follows for the below section also.

Gee thx. Since RX fifo empty bit is 0 when empty and tx_fifo_empty bit is 1 when empty, i will change to:
while( 0x20 != (serial_read_reg(p, UART_LSR) & 0x21))
;
Also From errata it does not require to make sure that shift register also needs to be empty.. we could save a few nano seconds by not using that.

Signature · 2 lines
Regards,
Nishanth Menon
#9 lore raw

Re: [PM][PATCH 1/4] OMAP3: Serial: Add context save and restore for mcr

In reply to Nishanth Menon
Quoted 5 lines · Nishanth Menon

From: Govindraj R <[email protected]>

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

Quoted 8 lines

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

Cc Kevin Hilman <[email protected]>
Cc Tero Kristo <[email protected]>
Signed-off-by Govindraj R <[email protected]>
Signed-off-by Nishanth Menon <[email protected]>

OK, adding to pm-next, and queing (hopefully) for 2.6.35.

Kevin

Quoted 42 lines

---
Note: Original Patch was done for android kernel, was ported and sanity tested to l-o pm. A quick check did not reveal any mcr access in arch/arm/mach-omap2/serial.c though drivers/serial/8250.c seems to hit on it a lot, hence posting.

 arch/arm/mach-omap2/serial.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/serial.c +6 -0
index b731556..86d3b47 100644
@@ -78,6 +78,7 @@ struct omap_uart_state {
 	u16 sysc;
 	u16 scr;
 	u16 wer;
+	u16 mcr;
 #endif
 };
 

@@ -201,6 +202,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;
 }

@@ -229,6 +233,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