[PATCH] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

STALE6555d

16 messages, 4 authors, 2008-11-14 · open the first message on its own page

[PATCH] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: René Bürgel <hidden>
Date: 2008-11-03 19:32:14

Hi

This patch is a workaround for bug #364 found in the MPC52xx processor.
The errata document can be found under http://www.freescale.com/files/32bit/doc/errata/MPC5200E.pdf?fpsp=1&WT_TYPE=Errata&WT_VENDOR=FREESCALE&WT_FILE_FORMAT=pdf&WT_ASSET=Documentation

When a device with a low baudrate is connected to the serial port, but the processor "listens" on a higher baudrate, it might falsely receive breaks from the controller. During a break, the serial controller may not be reset. The appended patch provides a workaround for that situation by lowering the baudrate without resetting the controller and waiting until no break is received anymore.


-- 
René Bürgel
Software Engineer
Unicontrol Systemtechnik GmbH
OT Dittersbach
Sachsenburger Weg 34
09669 Frankenberg

Tel.: 03 72 06/ 88 73 - 19
Fax: 03 72 06/ 88 73 - 60
E-Mail: r.buergel@unicontrol.de
Internet: www.unicontrol.de

Unicontrol Systemtechnik GmbH
Geschäftsführer: Dipl.-Ing. Siegfried Heinze
Sitz der Gesellschaft: Frankenberg
Registergericht: Amtsgericht Chemnitz, HRB 15 475

Re: [PATCH] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: Grant Likely <hidden>
Date: 2008-11-03 20:55:11

On Mon, Nov 3, 2008 at 12:32 PM, Ren=E9 B=FCrgel [off-list ref] =
wrote:
Hi

This patch is a workaround for bug #364 found in the MPC52xx processor.
The errata document can be found under
http://www.freescale.com/files/32bit/doc/errata/MPC5200E.pdf?fpsp=3D1&WT_=
TYPE=3DErrata&WT_VENDOR=3DFREESCALE&WT_FILE_FORMAT=3Dpdf&WT_ASSET=3DDocumen=
tation
This is an MPC5200 errata.  It doesn't exist on the MPC5200B.  The
bugfix code should be enabled at runtime only if running on the
original MPC5200.  You can use the value of the compatible property to
determine whether or not to enable it.  Optionally you can further
reduce impact by checking if CONFIG_PPC_MPC5200_BUGFIX is defined.

More comments below
quoted hunk
diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.=
c
quoted hunk
index 6117d3d..929524b 100644
--- a/drivers/serial/mpc52xx_uart.c
+++ b/drivers/serial/mpc52xx_uart.c
@@ -496,6 +496,27 @@ mpc52xx_uart_break_ctl(struct uart_port *port, int c=
tl)
       spin_unlock_irqrestore(&port->lock, flags);
 }

+/* macro with helper macros to safely reset rx which mustn't be done in
break state.
+ * This is a workaround for processor bug #364 described in MPC5200 (L25=
R)
Errata.
+ *
+ * The workaround resets the baudrate to 4800, waits for a stable state =
and
resets break state repeatedly if necessary
+ * optionally it can release the lock while waiting.
+ * 1 character at 4800 baud takes 2ms, 3ms should be enough for 1 charac=
ter
at higher speed and 1 char at lowest
+ * works only with longer delays
+ */
+#define LOCK(code) code
+#define DONT_LOCK(code)
+#define mpc52xx_uart_reset_rx(LOCK)
                   \
+       out_8(&psc->ctur,0x01);
                    \
+       out_8(&psc->ctlr,0xae);
                    \
+       do {
                   \
+               out_8(&psc->command,MPC52xx_PSC_RST_ERR_STAT);
                   \
+               LOCK(disable_irq(port->irq);
spin_unlock_irqrestore(&port->lock, flags));       \
+               mdelay(10);
                    \
+               LOCK(spin_lock_irqsave(&port->lock, flags);
enable_irq(port->irq));             \
+       } while ((in_be16(&psc->mpc52xx_psc_status)) & MPC52xx_PSC_SR_RB)=
;
quoted hunk
                   \
+       out_8(&psc->command,MPC52xx_PSC_RST_RX);
+
ick.  ugly.  Don't mess about with a macro here, just write a
function.  Let the compiler decide if it should be inlined.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: Matt Sealey <hidden>
Date: 2008-11-03 21:57:19


Grant Likely wrote:
On Mon, Nov 3, 2008 at 12:32 PM, René Bürgel [off-list ref] wrote:
quoted
Hi

This patch is a workaround for bug #364 found in the MPC52xx processor.
The errata document can be found under
http://www.freescale.com/files/32bit/doc/errata/MPC5200E.pdf?fpsp=1&WT_TYPE=Errata&WT_VENDOR=FREESCALE&WT_FILE_FORMAT=pdf&WT_ASSET=Documentation
This is an MPC5200 errata.  It doesn't exist on the MPC5200B.  The
bugfix code should be enabled at runtime only if running on the
original MPC5200.  You can use the value of the compatible property to
determine whether or not to enable it.
I would hope not since the Efika uses mpc5200-psc and mpc5200-serial
as it's compatible properties (this is from before mpc5200-psc-uart
was defined), which would enable this bugfix across the board.

Until we have a decent update for the device tree here (it's starting
to cause some real trouble lately when people update stuff like this
and want to compare) the best way to check for the MPC5200 or MPC5200B
is to look at the PVR - you don't need the device tree for this, at
all.

Sometime this week I am going to go through the 5200b device tree in
the kernel source and make sure efika.forth follows it as best as I
can, and then make a release.. that won't fix anything for people who
don't run the script, however.
Optionally you can further
reduce impact by checking if CONFIG_PPC_MPC5200_BUGFIX is defined.
I would much prefer this.

-- 
Matt Sealey [off-list ref]
Genesi, Manager, Developer Relations

Re: [PATCH] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: Wolfram Sang <hidden>
Date: 2008-11-03 22:15:59

On Mon, Nov 03, 2008 at 03:57:09PM -0600, Matt Sealey wrote:
quoted
Optionally you can further
reduce impact by checking if CONFIG_PPC_MPC5200_BUGFIX is defined.
I would much prefer this.
I submitted a patch to enable pipelining on a MPC5200B recently. It was
disabled because of a bug in the MPC5200. The first version of this
patch used MPC5200_BUGFIX and it was mentioned, that some people might
want to run the same kernel on both kind of processors. So, the patch
that went mainline checks for the PVR. Maybe we should stick to this
here, too?

All the best,

   Wolfram Sang

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

Re: [PATCH] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: Grant Likely <hidden>
Date: 2008-11-03 22:55:20

On Mon, Nov 3, 2008 at 3:15 PM, Wolfram Sang [off-list ref] wrote:
On Mon, Nov 03, 2008 at 03:57:09PM -0600, Matt Sealey wrote:
quoted
quoted
Optionally you can further
reduce impact by checking if CONFIG_PPC_MPC5200_BUGFIX is defined.
I would much prefer this.
I submitted a patch to enable pipelining on a MPC5200B recently. It was
disabled because of a bug in the MPC5200. The first version of this
patch used MPC5200_BUGFIX and it was mentioned, that some people might
want to run the same kernel on both kind of processors. So, the patch
that went mainline checks for the PVR. Maybe we should stick to this
here, too?
Of the two solutions:
1. Run-time selection must be done.
2. Compile-time removal of the bug fix path can also be done to lessen
runtime impact for kernels never run on older chips

My view is that #1 is non-negotiable.  #2 is a nice to have, but if it
doesn't incur any cost when disabled at runtime then I don't care.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

[PATCH V2] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: René Bürgel <hidden>
Date: 2008-11-04 10:44:15

Grant Likely schrieb:
On Mon, Nov 3, 2008 at 12:32 PM, René Bürgel [off-list ref] wrote:
  
quoted
Hi

This patch is a workaround for bug #364 found in the MPC52xx processor.
The errata document can be found under
http://www.freescale.com/files/32bit/doc/errata/MPC5200E.pdf?fpsp=1&WT_TYPE=Errata&WT_VENDOR=FREESCALE&WT_FILE_FORMAT=pdf&WT_ASSET=Documentation

    
This is an MPC5200 errata.  It doesn't exist on the MPC5200B.  The
bugfix code should be enabled at runtime only if running on the
original MPC5200.  You can use the value of the compatible property to
determine whether or not to enable it.  Optionally you can further
reduce impact by checking if CONFIG_PPC_MPC5200_BUGFIX is defined.

  
This bug is definetly present on the MPC5200B, although it is not listed in the errata sheet. I've had the ability to test it. We have custom boards using CPU modules from TQ with both processor versions, MPC5200 and MPC5200B. Both versions show exactly the same behaviour and the patch fixes it.

But as the serial driver is also used for the MPC5121, we may have to distinguish anyway. Does anyone have the possibility to test if the bug in still present  on MPC5121?
I disabled the bugfix on MPC5121-processors.
quoted
diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c
index 6117d3d..929524b 100644
--- a/drivers/serial/mpc52xx_uart.c
+++ b/drivers/serial/mpc52xx_uart.c
@@ -496,6 +496,27 @@ mpc52xx_uart_break_ctl(struct uart_port *port, int ctl)
       spin_unlock_irqrestore(&port->lock, flags);
 }

+/* macro with helper macros to safely reset rx which mustn't be done in
break state.
+ * This is a workaround for processor bug #364 described in MPC5200 (L25R)
Errata.
+ *
+ * The workaround resets the baudrate to 4800, waits for a stable state and
resets break state repeatedly if necessary
+ * optionally it can release the lock while waiting.
+ * 1 character at 4800 baud takes 2ms, 3ms should be enough for 1 character
at higher speed and 1 char at lowest
+ * works only with longer delays
+ */
+#define LOCK(code) code
+#define DONT_LOCK(code)
+#define mpc52xx_uart_reset_rx(LOCK)
                   \
+       out_8(&psc->ctur,0x01);
                    \
+       out_8(&psc->ctlr,0xae);
                    \
+       do {
                   \
+               out_8(&psc->command,MPC52xx_PSC_RST_ERR_STAT);
                   \
+               LOCK(disable_irq(port->irq);
spin_unlock_irqrestore(&port->lock, flags));       \
+               mdelay(10);
                    \
+               LOCK(spin_lock_irqsave(&port->lock, flags);
enable_irq(port->irq));             \
+       } while ((in_be16(&psc->mpc52xx_psc_status)) & MPC52xx_PSC_SR_RB);
                   \

10 more quoted lines

The purpose of the macro wasn't to enforce inlining, but to enhance the readability of the call and to ensure, that the was no runtime-check needed to decide whether the lock has to be hold or may be released. I'd like to avoid the runtime-checking here Anyway, i transformed the macro into a function like suggested. Any  proposals on this are welcome :)

-- 
René Bürgel
Software Engineer
Unicontrol Systemtechnik GmbH
OT Dittersbach
Sachsenburger Weg 34
09669 Frankenberg

Tel.: 03 72 06/ 88 73 - 19
Fax: 03 72 06/ 88 73 - 60
E-Mail: r.buergel@unicontrol.de
Internet: www.unicontrol.de

Unicontrol Systemtechnik GmbH
Geschäftsführer: Dipl.-Ing. Siegfried Heinze
Sitz der Gesellschaft: Frankenberg
Registergericht: Amtsgericht Chemnitz, HRB 15 475

Re: [PATCH V2] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: Wolfram Sang <hidden>
Date: 2008-11-04 11:15:32

Hello Rene,

I haven't actually applied the patch, just a few comments from a
glimpse:
+/* macro with helper macros to safely reset rx which mustn't be done in break state.
+ * This is a workaround for processor bug #364 described in MPC5200 (L25R) Errata.
Minor nit: Kernel CodingStyle suggests long comments like this

/*
 * comment starts here
 * ...
 */

Major nit: Please add to the comment that this bug is still present on
the MPC5200B, although it is not in its errata sheet. Thils will avoid
later confusion. (Out of interest, did you contact Freescale about this
bug?)
quoted hunk
+ *
+ * The workaround resets the baudrate to 4800, waits for a stable state and resets break state repeatedly if necessary.
+ * Optionally it can release the lock while waiting.
+ * 1 character at 4800 baud takes 2ms, 3ms should be enough for 1 character at higher speed and 1 char at lowest
+ * works only with longer delays
Did I get it right that there are cases where the workaround won't work?
quoted hunk
+ */
+
+static void mpc52xx_uart_reset_rx(struct uart_port *port, int unlock, unsigned long flags)
'bool unlock'?
quoted hunk
+{
+	struct mpc52xx_psc __iomem *psc = PSC(port);
+#ifdef CONFIG_PPC_MPC52xx
+	out_8(&psc->ctur,0x01);
+	out_8(&psc->ctlr,0xae);
Those are magic values. If you can't build them using defined
constants, at least document them, please.
quoted hunk
+	do {
+		out_8(&psc->command,MPC52xx_PSC_RST_ERR_STAT);
+		if (unlock) {
+			disable_irq(port->irq);
+			spin_unlock_irqrestore(&port->lock, flags);
+		}
+		mdelay(10);
+		if (unlock) {
+			spin_lock_irqsave(&port->lock, flags);
+			enable_irq(port->irq);
+		}
+	} while ((in_be16(&psc->mpc52xx_psc_status)) & MPC52xx_PSC_SR_RB);
+#endif
+	out_8(&psc->command,MPC52xx_PSC_RST_RX);
+}
+
 static int
 mpc52xx_uart_startup(struct uart_port *port)
 {
@@ -510,7 +541,7 @@ mpc52xx_uart_startup(struct uart_port *port)
 		return ret;
 
 	/* Reset/activate the port, clear and enable interrupts */
-	out_8(&psc->command, MPC52xx_PSC_RST_RX);
+	mpc52xx_uart_reset_rx(port, false, 0);
 	out_8(&psc->command, MPC52xx_PSC_RST_TX);
 
 	out_be32(&psc->sicr, 0);	/* UART mode DCD ignored */
@@ -529,7 +560,7 @@ mpc52xx_uart_shutdown(struct uart_port *port)
 	struct mpc52xx_psc __iomem *psc = PSC(port);
 
 	/* Shut down the port.  Leave TX active if on a console port */
-	out_8(&psc->command, MPC52xx_PSC_RST_RX);
+	mpc52xx_uart_reset_rx(port, false, 0);
 	if (!uart_console(port))
 		out_8(&psc->command, MPC52xx_PSC_RST_TX);
 
@@ -588,9 +619,6 @@ mpc52xx_uart_set_termios(struct uart_port *port, struct ktermios *new,
 	/* Get the lock */
 	spin_lock_irqsave(&port->lock, flags);

21 more quoted lines

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Thanks!

   Wolfram Sang

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

Re: [PATCH V2] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: René Bürgel <hidden>
Date: 2008-11-04 14:14:36

Wolfram Sang schrieb:
Hello Rene,

I haven't actually applied the patch, just a few comments from a
glimpse:
  
Major nit: Please add to the comment that this bug is still present on
the MPC5200B, although it is not in its errata sheet. Thils will avoid
later confusion. (Out of interest, did you contact Freescale about this
bug?)
  
Comment added, contacting Freescale is still on my to-do-list :)
  
quoted
+ *
+ * The workaround resets the baudrate to 4800, waits for a stable state and resets break state repeatedly if necessary.
+ * Optionally it can release the lock while waiting.
+ * 1 character at 4800 baud takes 2ms, 3ms should be enough for 1 character at higher speed and 1 char at lowest
+ * works only with longer delays
    
Did I get it right that there are cases where the workaround won't work?
  
Yes, currently it doesn't work when receiving with less than 4800 baud. I'll try to fix this.
  
quoted
+{
+	struct mpc52xx_psc __iomem *psc = PSC(port);
+#ifdef CONFIG_PPC_MPC52xx
+	out_8(&psc->ctur,0x01);
+	out_8(&psc->ctlr,0xae);
    
Those are magic values. If you can't build them using defined
constants, at least document them, please.
  
The magic numbers will hopefully vanish after generalizing the patch to work with every baudrate.
Thanks!

   Wolfram Sang

  
Thanks for your suggestions

-- 
René Bürgel
Software Engineer
Unicontrol Systemtechnik GmbH
OT Dittersbach
Sachsenburger Weg 34
09669 Frankenberg

Tel.: 03 72 06/ 88 73 - 19
Fax: 03 72 06/ 88 73 - 60
E-Mail: r.buergel@unicontrol.de
Internet: www.unicontrol.de

Unicontrol Systemtechnik GmbH
Geschäftsführer: Dipl.-Ing. Siegfried Heinze
Sitz der Gesellschaft: Frankenberg
Registergericht: Amtsgericht Chemnitz, HRB 15 475

Re: [PATCH] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: Matt Sealey <hidden>
Date: 2008-11-04 18:18:41


Wolfram Sang wrote:
On Mon, Nov 03, 2008 at 03:57:09PM -0600, Matt Sealey wrote:
quoted
quoted
Optionally you can further
reduce impact by checking if CONFIG_PPC_MPC5200_BUGFIX is defined.
I would much prefer this.
I submitted a patch to enable pipelining on a MPC5200B recently. It was
disabled because of a bug in the MPC5200. The first version of this
patch used MPC5200_BUGFIX and it was mentioned, that some people might
want to run the same kernel on both kind of processors. So, the patch
that went mainline checks for the PVR. Maybe we should stick to this
here, too?
I would wrap a real chipset errata inside CONFIG_PPC_MPC5200_BUGFIX so
that you have the option to remove all code that fixes these errata, but
also check the PVR and only do the bugfix if you're on that processor,
so.. both :D

The pipelining thing seems to be fixed in the MPC5200B but it actually
makes the bus lock up under certain circumstances with the ATA DMA
task and certain priority selections. I am not sure what we're going
to do about that pipelining support (Tim Yamin's patch removed it! Maybe
we should have a little addition to the BestComm driver which can set
these things from the driver side using a little global API, so that
if an ATA driver loads and wants this configuration, it can get to it)

-- 
Matt Sealey [off-list ref]
Genesi, Manager, Developer Relations

Re: [PATCH V2] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: Matt Sealey <hidden>
Date: 2008-11-04 18:23:37


René Bürgel wrote:
But as the serial driver is also used for the MPC5121, we may have to distinguish anyway. Does anyone have the possibility to test if the bug in still present  on MPC5121?
Tell us what to do to get it to occur and what we're looking for and we
have a bunch of boards at Genesi, someone will find the time to test it
(if not them, then me :)

Right now I'm having a hell of a time getting a 2.6.27.x kernel running
on it though, some driver support is still missing from mainline making
it just that little bit extra frustrating to work with.. (if you're
using the system over a serial console, how are you supposed to test
serial port operation? USB doesn't work in >2.6.24 so I guess I have to
hope netconsole works :)

-- 
Matt Sealey [off-list ref]
Genesi, Manager, Developer Relations

Re: [PATCH V3] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: René Bürgel <hidden>
Date: 2008-11-04 19:40:19

Hey, folks

This is v3 of my patch to work around erratum #364 of the MPC5200(B). I removed most "magic" looking numbers, documenting the rest. As mentioned before, the previous patches weren't working for low baudrates (<9600). This should be fixed now.

But there's still one thing, that bothers me a bit - if there is REALLY a break on the line, closing the driver may take until it's gone. I don't know whether this is really satisfying, but i think it's better than the alternative: no serial connection until the next reboot.

-- 
René Bürgel
Software Engineer
Unicontrol Systemtechnik GmbH
OT Dittersbach
Sachsenburger Weg 34
09669 Frankenberg

Tel.: 03 72 06/ 88 73 - 19
Fax: 03 72 06/ 88 73 - 60
E-Mail: r.buergel@unicontrol.de
Internet: www.unicontrol.de

Unicontrol Systemtechnik GmbH
Geschäftsführer: Dipl.-Ing. Siegfried Heinze
Sitz der Gesellschaft: Frankenberg
Registergericht: Amtsgericht Chemnitz, HRB 15 475

Re: [PATCH V3] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: Wolfram Sang <hidden>
Date: 2008-11-04 21:21:58

Hi René,

On Tue, Nov 04, 2008 at 08:40:09PM +0100, René Bürgel wrote:
Hey, folks

This is v3 of my patch to work around erratum #364 of the MPC5200(B). I  
removed most "magic" looking numbers, documenting the rest. As mentioned  
before, the previous patches weren't working for low baudrates (<9600).  
This should be fixed now.

But there's still one thing, that bothers me a bit - if there is REALLY  
a break on the line, closing the driver may take until it's gone. I  
don't know whether this is really satisfying, but i think it's better  
than the alternative: no serial connection until the next reboot.
I think we should CC linux-serial to get some opinions about this. At
least, if it stays like this, it should be mentioned in the source.
-- 
René Bürgel
Software Engineer
Unicontrol Systemtechnik GmbH
OT Dittersbach
Sachsenburger Weg 34
09669 Frankenberg

Tel.: 03 72 06/ 88 73 - 19
Fax: 03 72 06/ 88 73 - 60
E-Mail: r.buergel@unicontrol.de
Internet: www.unicontrol.de

Unicontrol Systemtechnik GmbH
Geschäftsführer: Dipl.-Ing. Siegfried Heinze
Sitz der Gesellschaft: Frankenberg
Registergericht: Amtsgericht Chemnitz, HRB 15 475
quoted hunk
diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c
index 6117d3d..a2bb4d9 100644
--- a/drivers/serial/mpc52xx_uart.c
+++ b/drivers/serial/mpc52xx_uart.c
@@ -496,6 +496,59 @@ mpc52xx_uart_break_ctl(struct uart_port *port, int ctl)
 	spin_unlock_irqrestore(&port->lock, flags);
 }
 
+/*
+ * This is a workaround for processor bug #364 described in MPC5200 (L25R) Errata.
+ * The bug is still present in MPC5200B, but currently not listed in its errata sheet
+ *
+ * The workaround resets the baudrate to the slowest possible,
+ * waits for a stable state and resets break state repeatedly if necessary.
+ * Optionally it can release the lock while waiting.
+ *
+ * That baudrate is roughly port->uartclk / (1000 * 1000)
+ * The minimum wait time for the first try has to include the time to wait for stop-bits and a character,
+ * we wait for 2 chars to be sure
+ * Consecutive waits must just receive one character.
+ */
Here are lines >80 chars. Please let scripts/checkpatch.pl check this
patch (--strict doesn't hurt). And please try also
Documentation/CodingStyle.txt. It really helps maintaining that huge
amount of code the kernel is.
quoted hunk
+
+static void mpc52xx_uart_reset_rx(struct uart_port *port, bool unlock, unsigned long flags)
+{
+#ifdef CONFIG_PPC_MPC52xx
+	void reset_errors_and_wait(struct uart_port *port, bool unlock, unsigned long flags, unsigned int delay)
+	{
+		struct mpc52xx_psc __iomem *psc = PSC(port);
+		out_8(&psc->command,MPC52xx_PSC_RST_ERR_STAT);
+		if (unlock) {
+			disable_irq(port->irq);
+			spin_unlock_irqrestore(&port->lock, flags);
+		}
+		mdelay( delay );
+		if (unlock) {
+			spin_lock_irqsave(&port->lock, flags);
+			enable_irq(port->irq);
+		}
+	}
Function inside a function? Haven't seen this before in kernel code. I
don't think it will be accepted, but leave this to maintainers.
quoted hunk
+
+
+	struct mpc52xx_psc __iomem *psc = PSC(port);
+
+	// One character on the serial port may consist of up to 12 bits.
+	// So the time to receive one char is 12 /*bits*/ / (port->uartclk / (1000 * 1000) /*MHz -> Hz*/) * 1000 /*s -> ms*/,
+	// but we'll wait 50% longer just to make sure
No // comments please.
quoted hunk
+	unsigned int one_char_receive_duration = (12 * 1000) / (port->uartclk / (1000 * 1000) );
+
+	/* CT=0xFFFF sets the serial line to the minimal possible baudrate depending on the uartclk. */
+	out_8(&psc->ctur, 0xFF);
+	out_8(&psc->ctlr, 0xFF);
+
+	reset_errors_and_wait( port, unlock, flags, one_char_receive_duration * 2 );
+
+	while ((in_be16(&psc->mpc52xx_psc_status)) & MPC52xx_PSC_SR_RB) {
+		reset_errors_and_wait( port, unlock, flags, one_char_receive_duration );
+	}
+#endif
+	out_8(&psc->command,MPC52xx_PSC_RST_RX);
+}
+
 static int
 mpc52xx_uart_startup(struct uart_port *port)
 {
@@ -510,7 +563,7 @@ mpc52xx_uart_startup(struct uart_port *port)
 		return ret;
 
 	/* Reset/activate the port, clear and enable interrupts */
-	out_8(&psc->command, MPC52xx_PSC_RST_RX);
+	mpc52xx_uart_reset_rx(port, false, 0);
 	out_8(&psc->command, MPC52xx_PSC_RST_TX);
 
 	out_be32(&psc->sicr, 0);	/* UART mode DCD ignored */
@@ -529,7 +582,7 @@ mpc52xx_uart_shutdown(struct uart_port *port)
 	struct mpc52xx_psc __iomem *psc = PSC(port);
 
 	/* Shut down the port.  Leave TX active if on a console port */
-	out_8(&psc->command, MPC52xx_PSC_RST_RX);
+	mpc52xx_uart_reset_rx(port, false, 0);
 	if (!uart_console(port))
 		out_8(&psc->command, MPC52xx_PSC_RST_TX);
 
@@ -588,9 +641,6 @@ mpc52xx_uart_set_termios(struct uart_port *port, struct ktermios *new,
 	/* Get the lock */
 	spin_lock_irqsave(&port->lock, flags);
 

20 more quoted lines

Bye,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

Re: [PATCH V4] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: René Bürgel <hidden>
Date: 2008-11-06 08:10:37

This patch is a workaround for bug #364 found in the MPC52xx processor.
The errata document can be found under http://www.freescale.com/files/32bit/doc/errata/MPC5200E.pdf?fpsp=1&WT_TYPE=Errata&WT_VENDOR=FREESCALE&WT_FILE_FORMAT=pdf&WT_ASSET=Documentation


When a device with a low baudrate is connected to the serial port, but the processor "listens" on a higher baudrate, it might falsely receive breaks from the controller. During a break, the serial controller may not be reset. The appended patch provides a workaround for that situation by lowering the baudrate without resetting the controller and waiting until no break is received anymore.

This is v4 if the patch, just reformatted to fit the linux kernel coding style without functional changes.

Wolfram Sang schrieb:
Hi René,

On Tue, Nov 04, 2008 at 08:40:09PM +0100, René Bürgel wrote
quoted
But there's still one thing, that bothers me a bit - if there is REALLY  a break on the line, closing the driver may take until it's gone. I  don't know whether this is really satisfying, but i think it's better  than the alternative: no serial connection until the next reboot.
    
I think we should CC linux-serial to get some opinions about this. At
least, if it stays like this, it should be mentioned in the source.
What's the opinion from the linux-serial folks about this issue?

-- 
René Bürgel
Software Engineer
Unicontrol Systemtechnik GmbH
OT Dittersbach
Sachsenburger Weg 34
09669 Frankenberg

Tel.: 03 72 06/ 88 73 - 19
Fax: 03 72 06/ 88 73 - 60
E-Mail: r.buergel@unicontrol.de
Internet: www.unicontrol.de

Unicontrol Systemtechnik GmbH
Geschäftsführer: Dipl.-Ing. Siegfried Heinze
Sitz der Gesellschaft: Frankenberg
Registergericht: Amtsgericht Chemnitz, HRB 15 475

Wichtiger Hinweis: Diese E-Mail und etwaige Anlagen können Betriebs- und Geschäftsgeheimnisse, dem Anwaltsgeheimnis unterliegende oder sonstige vertrauliche Informationen enthalten. Sollten Sie diese E-Mail irrtümlich erhalten haben, ist Ihnen der Status dieser E-Mail bekannt. Bitte benachrichtigen Sie uns in diesem Falle sofort durch Antwort-Mail und löschen Sie diese E-Mail nebst etwaigen Anlagen aus Ihrem System. Ebenso dürfen Sie diese E-Mail oder ihre Anlagen nicht kopieren oder an Dritte weitergeben. Vielen Dank!

Important Note: This e-mail and any attachments are confidential, may contain trade secrets and may well also be legally privileged or otherwise protected from disclosure. If you have received it in error, you are on notice of its status. Please notify us immediately by reply e-mail and then delete this e-mail and any attachment from your system. If you are not the intended recipient please understand that you must not copy this e-mail or any attachments or disclose the contents to any other person. Thank you.

Re: [PATCH V2] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: René Bürgel <hidden>
Date: 2008-11-06 17:02:13

Matt Sealey schrieb:

René Bürgel wrote:
quoted
But as the serial driver is also used for the MPC5121, we may have to distinguish anyway. Does anyone have the possibility to test if the bug in still present  on MPC5121?
Tell us what to do to get it to occur and what we're looking for and we
have a bunch of boards at Genesi, someone will find the time to test it
(if not them, then me :)

Right now I'm having a hell of a time getting a 2.6.27.x kernel running
on it though, some driver support is still missing from mainline making
it just that little bit extra frustrating to work with.. (if you're
using the system over a serial console, how are you supposed to test
serial port operation? USB doesn't work in >2.6.24 so I guess I have to
hope netconsole works :)
Hi, Matt

Thanks for your offer. I'm using telnet to debug the serial port.  I'll append a testcase for you, which is just opening a serial port, trying to receive for a second, switching the baudrate and doing it receiving again. Just run it and connect a slow device to the serial port sending data continuously. I'm using a GPS mouse here, working at 9600baud. The higher the rate you are receiving at, the higher the chance to falsely receive a break. When the serial port is switch off in that moment (the filedescriptor is closed), the serial won't receive anything from that time, if the bug is present

Alternativly, if you have more control over your serial device, just send breaks continuously, open and close the serial port. Open it again and receiving data fails, if the bug is present.

Just btw: if USB is not working, did you miss the initialisation of the USB-controller in your bootloader? I had similar problems getting from 2.6.22 to 2.6.25 with my mpc5200.

-- 
René Bürgel
Software Engineer
Unicontrol Systemtechnik GmbH
OT Dittersbach
Sachsenburger Weg 34
09669 Frankenberg

Tel.: 03 72 06/ 88 73 - 19
Fax: 03 72 06/ 88 73 - 60
E-Mail: r.buergel@unicontrol.de
Internet: www.unicontrol.de

Unicontrol Systemtechnik GmbH
Geschäftsführer: Dipl.-Ing. Siegfried Heinze
Sitz der Gesellschaft: Frankenberg
Registergericht: Amtsgericht Chemnitz, HRB 15 475

Re: [PATCH V2] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: Matt Sealey <hidden>
Date: 2008-11-06 22:41:08


René Bürgel wrote:
Matt Sealey schrieb:

Alternativly, if you have more control over your serial device, just send breaks continuously, open and close the serial port. Open it again and receiving data fails, if the bug is present.
Well I have a couple systems I can write data from here, a little app to
send data really isn't that hard to come up with.
Just btw: if USB is not working, did you miss the initialisation of the USB-controller in your bootloader? I had similar problems getting from 2.6.22 to 2.6.25 with my mpc5200.
Actually it's more to do with the fact that the patches to get USB working
(along with On The Go support) in the LTIB are quite intrusive, and written
for 2.6.24 - since then the USB cores seem to have been reshuffled so it
is far from a clean patch.

Someday I will get bored and port it to 2.6.28 :)

-- 
Matt Sealey [off-list ref]
Genesi, Manager, Developer Relations

Re: [PATCH V4] workaround for mpc52xx erratum #364 (serial may not be reset in break state)

From: Grant Likely <hidden>
Date: 2008-11-14 19:09:55

On Thu, Nov 6, 2008 at 1:11 AM, Ren=E9 B=FCrgel [off-list ref] w=
rote:
This patch is a workaround for bug #364 found in the MPC52xx processor.
The errata document can be found under
http://www.freescale.com/files/32bit/doc/errata/MPC5200E.pdf?fpsp=3D1&WT_=
TYPE=3DErrata&WT_VENDOR=3DFREESCALE&WT_FILE_FORMAT=3Dpdf&WT_ASSET=3DDocumen=
tation
When a device with a low baudrate is connected to the serial port, but th=
e
processor "listens" on a higher baudrate, it might falsely receive breaks
from the controller. During a break, the serial controller may not be res=
et.
The appended patch provides a workaround for that situation by lowering t=
he
baudrate without resetting the controller and waiting until no break is
received anymore.
I'm still don't like the state of this patch for two reasons:
1. It is enabled/disabled by a #ifdef.  It is quite possible that we
will eventually be able to build a multiplaform kernel that boots on
both mpc5200 and mpc5121.  The workaround needs to be detected and
enabled at runtime based on the data in the device tree (ie. if the
compatible property is "fsl,mpc5200-psc-uart").

2. I'm do not like the mdelay() busywait loop.  The long busy wait
just wastes CPU time.  Doing it with IRQs off means that irq latencies
become unbounded while this is happening.  This needs to be reworked
to use a workqueue or something similar.

Also, I'm not convinced that this is the best fix.  It doesn't
actually solve the problem, it just makes it less likely to occur.
What happens if you instead manipulate portconfig to change the PSC
pins to GPIO?  I wonder if that will disconnect the RX pin from the
PSC entirely.  If it does, then that might be a suitable method to
eliminate the break condition entirely.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help