* [PATCH v2] omap: serial: fix non-empty uart fifo read abort
@ 2009-12-03 23:29 Vikram Pandita
2009-12-04 19:04 ` Tony Lindgren
0 siblings, 1 reply; 4+ messages in thread
From: Vikram Pandita @ 2009-12-03 23:29 UTC (permalink / raw)
To: linux-omap; +Cc: Vikram Pandita, Cousson, Benoit
OMAP3xxx and OMAP4430 UART IP blocks have a restriction wrt RX FIFO.
Empty RX fifo read causes an abort.
OMAP3xxx:
UART IP revision >= 0x52 have this issue
MVR register format is:
Bits Field Name Description Type Reset
31:8 RESERVED RO 0x0
7:4 MAJOR Major revision number of the module. RO 0x--
3:0 MINOR Minor revision number of the module. RO 0x--
OMAP4xxx:
All revisions have this issue
Revision id check is not used as the format of MVR resigster has changed
For omap4 MVR register reads as: 0x50410602 => Revision id = 0x0602
Format of MVR register on omap4 is: (Courtesy: Cousson, Benoit)
Bits Field Name Description Type Reset
31:30 SCHEME Scheme revision number of module RO 0x1
29:28 RESERVED RO 0x1
27:16 FUNC Function revision number of module RO 0x041
15:11 RTL Rtl revision number of module RO 0x00
10:8 MAJOR Major revision number of the module. RO 0x6
7:6 CUSTOM Custom revision number of the module. RO 0x0
5:0 MINOR Minor revision number of the module. RO 0x02
Override the default 8250 read handler: mem_serial_in()
by a custom handler: serial_in_8250()
which makes sure that RX fifo is not read when empty
tested on zoom3(3630) board
Cc: Cousson, Benoit <b-cousson@ti.com>
Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
---
History of Patch:
V1:
http://patchwork.kernel.org/patch/61671/
http://patchwork.kernel.org/patch/61672/
V2: Incorporate approach suggested by Tony L
Introduce IP version check in mach-omap2/serial.c file
arch/arm/mach-omap2/serial.c | 29 +++++++++++++++++++++++++++++
1 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 2e17b57..bd646a2 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -33,6 +33,7 @@
#include "pm.h"
#include "prm-regbits-34xx.h"
+#define UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV 0x52
#define UART_OMAP_WER 0x17 /* Wake-up enable register */
#define DEFAULT_TIMEOUT (5 * HZ)
@@ -572,6 +573,23 @@ static struct omap_uart_state omap_uart[] = {
#endif
};
+/*
+ * Override the default 8250 read handler: mem_serial_in()
+ * Empty RX fifo read causes an abort on omap3630 and omap4
+ * This function makes sure that an empty rx fifo is not read on these silicons
+ * (OMAP1/2/3 are not affected)
+ */
+static unsigned int serial_in_override(struct uart_port *up, int offset)
+{
+ if (UART_RX == offset) {
+ unsigned int lsr;
+ lsr = serial_read_reg(omap_uart[up->line].p, UART_LSR);
+ if (!(lsr & UART_LSR_DR))
+ return -EPERM;
+ }
+ return serial_read_reg(omap_uart[up->line].p, offset);
+}
+
void __init omap_serial_early_init(void)
{
int i;
@@ -650,5 +668,16 @@ void __init omap_serial_init(void)
device_init_wakeup(dev, true);
DEV_CREATE_FILE(dev, &dev_attr_sleep_timeout);
}
+
+#ifdef CONFIG_ARCH_OMAP4
+ /* Never read empty UART fifo on omap4 */
+ p->serial_in = serial_in_override;
+#else
+ /* OMAP2/3 */
+ /* Never read empty UART fifo on UARTs with IP rev >=0x52 */
+ if ((serial_read_reg(uart->p, UART_OMAP_MVER) & 0xFF)
+ >= UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV)
+ uart->p->serial_in = serial_in_override;
+#endif
}
}
--
1.6.6.rc0.66.ge160d
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] omap: serial: fix non-empty uart fifo read abort
2009-12-03 23:29 [PATCH v2] omap: serial: fix non-empty uart fifo read abort Vikram Pandita
@ 2009-12-04 19:04 ` Tony Lindgren
2009-12-04 19:15 ` Pandita, Vikram
0 siblings, 1 reply; 4+ messages in thread
From: Tony Lindgren @ 2009-12-04 19:04 UTC (permalink / raw)
To: Vikram Pandita; +Cc: linux-omap, Cousson, Benoit
* Vikram Pandita <vikram.pandita@ti.com> [091203 15:27]:
> OMAP3xxx and OMAP4430 UART IP blocks have a restriction wrt RX FIFO.
> Empty RX fifo read causes an abort.
>
> OMAP3xxx:
> UART IP revision >= 0x52 have this issue
> MVR register format is:
> Bits Field Name Description Type Reset
> 31:8 RESERVED RO 0x0
> 7:4 MAJOR Major revision number of the module. RO 0x--
> 3:0 MINOR Minor revision number of the module. RO 0x--
>
> OMAP4xxx:
> All revisions have this issue
> Revision id check is not used as the format of MVR resigster has changed
> For omap4 MVR register reads as: 0x50410602 => Revision id = 0x0602
> Format of MVR register on omap4 is: (Courtesy: Cousson, Benoit)
> Bits Field Name Description Type Reset
> 31:30 SCHEME Scheme revision number of module RO 0x1
> 29:28 RESERVED RO 0x1
> 27:16 FUNC Function revision number of module RO 0x041
> 15:11 RTL Rtl revision number of module RO 0x00
> 10:8 MAJOR Major revision number of the module. RO 0x6
> 7:6 CUSTOM Custom revision number of the module. RO 0x0
> 5:0 MINOR Minor revision number of the module. RO 0x02
>
> Override the default 8250 read handler: mem_serial_in()
> by a custom handler: serial_in_8250()
> which makes sure that RX fifo is not read when empty
>
> tested on zoom3(3630) board
>
> Cc: Cousson, Benoit <b-cousson@ti.com>
> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
> ---
> History of Patch:
> V1:
> http://patchwork.kernel.org/patch/61671/
> http://patchwork.kernel.org/patch/61672/
>
> V2: Incorporate approach suggested by Tony L
> Introduce IP version check in mach-omap2/serial.c file
>
> arch/arm/mach-omap2/serial.c | 29 +++++++++++++++++++++++++++++
> 1 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 2e17b57..bd646a2 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -33,6 +33,7 @@
> #include "pm.h"
> #include "prm-regbits-34xx.h"
>
> +#define UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV 0x52
> #define UART_OMAP_WER 0x17 /* Wake-up enable register */
>
> #define DEFAULT_TIMEOUT (5 * HZ)
> @@ -572,6 +573,23 @@ static struct omap_uart_state omap_uart[] = {
> #endif
> };
>
> +/*
> + * Override the default 8250 read handler: mem_serial_in()
> + * Empty RX fifo read causes an abort on omap3630 and omap4
> + * This function makes sure that an empty rx fifo is not read on these silicons
> + * (OMAP1/2/3 are not affected)
> + */
> +static unsigned int serial_in_override(struct uart_port *up, int offset)
> +{
> + if (UART_RX == offset) {
> + unsigned int lsr;
> + lsr = serial_read_reg(omap_uart[up->line].p, UART_LSR);
> + if (!(lsr & UART_LSR_DR))
> + return -EPERM;
> + }
> + return serial_read_reg(omap_uart[up->line].p, offset);
> +}
> +
> void __init omap_serial_early_init(void)
> {
> int i;
> @@ -650,5 +668,16 @@ void __init omap_serial_init(void)
> device_init_wakeup(dev, true);
> DEV_CREATE_FILE(dev, &dev_attr_sleep_timeout);
> }
> +
> +#ifdef CONFIG_ARCH_OMAP4
> + /* Never read empty UART fifo on omap4 */
> + p->serial_in = serial_in_override;
> +#else
> + /* OMAP2/3 */
> + /* Never read empty UART fifo on UARTs with IP rev >=0x52 */
> + if ((serial_read_reg(uart->p, UART_OMAP_MVER) & 0xFF)
> + >= UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV)
> + uart->p->serial_in = serial_in_override;
> +#endif
> }
> }
We need to avoid ifdef else stuff, that just causes problems compiling
in support for many omaps. In theory, we should be able to compile in
support for all omaps starting with 16xx with v5 options..
Using cpu_is_omapxxxx() should do the trick here.
Tony
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] omap: serial: fix non-empty uart fifo read abort
2009-12-04 19:04 ` Tony Lindgren
@ 2009-12-04 19:15 ` Pandita, Vikram
2009-12-04 20:16 ` Nishanth Menon
0 siblings, 1 reply; 4+ messages in thread
From: Pandita, Vikram @ 2009-12-04 19:15 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap@vger.kernel.org, Cousson, Benoit
>-----Original Message-----
>From: Tony Lindgren [mailto:tony@atomide.com]
>Sent: Friday, December 04, 2009 1:04 PM
>To: Pandita, Vikram
>Cc: linux-omap@vger.kernel.org; Cousson, Benoit
>Subject: Re: [PATCH v2] omap: serial: fix non-empty uart fifo read abort
>
<snip>
>> +
>> +#ifdef CONFIG_ARCH_OMAP4
>> + /* Never read empty UART fifo on omap4 */
>> + p->serial_in = serial_in_override;
>> +#else
>> + /* OMAP2/3 */
>> + /* Never read empty UART fifo on UARTs with IP rev >=0x52 */
>> + if ((serial_read_reg(uart->p, UART_OMAP_MVER) & 0xFF)
>> + >= UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV)
>> + uart->p->serial_in = serial_in_override;
>> +#endif
>> }
>> }
>
>We need to avoid ifdef else stuff, that just causes problems compiling
>in support for many omaps. In theory, we should be able to compile in
>support for all omaps starting with 16xx with v5 options..
>
>Using cpu_is_omapxxxx() should do the trick here.
The intent was to have check entirely based of UART IP revision.
That worked fine for omap3xxx. But omap4 has totally different IP revision register (as per commit message)
But I can see the point, and v3 of patch I can replace:
#ifdef CONFIG_ARCH_OMAP4 by cpu_is_omap44xx()
I tried following what was done in current serial.c file of many #ifdef CONFIG_ARCH_OMAP4
>
>Tony
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] omap: serial: fix non-empty uart fifo read abort
2009-12-04 19:15 ` Pandita, Vikram
@ 2009-12-04 20:16 ` Nishanth Menon
0 siblings, 0 replies; 4+ messages in thread
From: Nishanth Menon @ 2009-12-04 20:16 UTC (permalink / raw)
To: Pandita, Vikram
Cc: Tony Lindgren, linux-omap@vger.kernel.org, Cousson, Benoit
Pandita, Vikram said the following on 12/04/2009 09:15 PM:
>
>> -----Original Message-----
>> From: Tony Lindgren [mailto:tony@atomide.com]
>> Sent: Friday, December 04, 2009 1:04 PM
>> To: Pandita, Vikram
>> Cc: linux-omap@vger.kernel.org; Cousson, Benoit
>> Subject: Re: [PATCH v2] omap: serial: fix non-empty uart fifo read abort
>>
>>
> <snip>
>
>>> +
>>> +#ifdef CONFIG_ARCH_OMAP4
>>> + /* Never read empty UART fifo on omap4 */
>>> + p->serial_in = serial_in_override;
>>> +#else
>>> + /* OMAP2/3 */
>>> + /* Never read empty UART fifo on UARTs with IP rev >=0x52 */
>>> + if ((serial_read_reg(uart->p, UART_OMAP_MVER) & 0xFF)
>>> + >= UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV)
>>> + uart->p->serial_in = serial_in_override;
>>> +#endif
>>> }
>>> }
>>>
>> We need to avoid ifdef else stuff, that just causes problems compiling
>> in support for many omaps. In theory, we should be able to compile in
>> support for all omaps starting with 16xx with v5 options..
>>
>> Using cpu_is_omapxxxx() should do the trick here.
>>
>
> The intent was to have check entirely based of UART IP revision.
> That worked fine for omap3xxx. But omap4 has totally different IP revision register (as per commit message)
>
> But I can see the point, and v3 of patch I can replace:
> #ifdef CONFIG_ARCH_OMAP4 by cpu_is_omap44xx()
>
> I tried following what was done in current serial.c file of many #ifdef CONFIG_ARCH_OMAP4
>
a cleanup patch could be welcome as well :)
>
>
>> Tony
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-12-04 20:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-03 23:29 [PATCH v2] omap: serial: fix non-empty uart fifo read abort Vikram Pandita
2009-12-04 19:04 ` Tony Lindgren
2009-12-04 19:15 ` Pandita, Vikram
2009-12-04 20:16 ` Nishanth Menon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox