[U-Boot] Beagleboard: SPL hangs on serial init
SPL hangs on a beagleboard during the serial initialization (line 40 of drivers/serial/ns16550.c). The reason is the non-empty shift register of TX (transmit FIFO is empty). To avoid the hangup include/configs/omap3_beagle.h has to be changed like in include/configs/igep00x0.h:
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 59255c4..63cd30b 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -81,6 +81,9 @@ #define CONFIG_SYS_NS16550_REG_SIZE (-4) #define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
+/* define to avoid U-Boot to hang while waiting for TEMT */ +#define CONFIG_SYS_NS16550_BROKEN_TEMT + /* * select serial console configuration */
Best regards, Manfred Huber
I'm surprised that no one is interested in a functioning Beagleboard. Has no one tested the Beagleboard since 2012-09-19?
Best regards, Manfred Huber
On 2013-02-27 14:09, man.huber@arcor.de wrote:
SPL hangs on a beagleboard during the serial initialization (line 40 of drivers/serial/ns16550.c). The reason is the non-empty shift register of TX (transmit FIFO is empty). To avoid the hangup include/configs/omap3_beagle.h has to be changed like in include/configs/igep00x0.h:
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 59255c4..63cd30b 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -81,6 +81,9 @@ #define CONFIG_SYS_NS16550_REG_SIZE (-4) #define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
+/* define to avoid U-Boot to hang while waiting for TEMT */ +#define CONFIG_SYS_NS16550_BROKEN_TEMT
- /*
*/
- select serial console configuration
Best regards, Manfred Huber _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Sat, Mar 16, 2013 at 02:13:54PM +0100, Manfred Huber wrote:
I'm surprised that no one is interested in a functioning Beagleboard. Has no one tested the Beagleboard since 2012-09-19?
I don't see this problem on mine (classic and xM), which is probably part of the why. I'm inclined to accept the patch, but can you try two things please: - How reproducible is this problem, with the host and beagleboard combination you have? 100%? - Do you have another beagleboard or another host PC (or USB-Serial dongle) you can try?
Thanks!
On 2013-03-19 15:49, Tom Rini wrote:
On Sat, Mar 16, 2013 at 02:13:54PM +0100, Manfred Huber wrote:
I'm surprised that no one is interested in a functioning Beagleboard. Has no one tested the Beagleboard since 2012-09-19?
I don't see this problem on mine (classic and xM), which is probably part of the why. I'm inclined to accept the patch, but can you try two things please:
- How reproducible is this problem, with the host and beagleboard combination you have? 100%?
- Do you have another beagleboard or another host PC (or USB-Serial dongle) you can try?
Thanks!
Thanks for your answer. I have only one Beagleboard Revision C4 (CONTROL_IDCODE of the OMAP3530 is 0x4B7AE02F). My host is a Ubuntu 12.10 (Quantal Quetzal) and I use the gcc-arm-linux-gnueabihf cross-compiler version 4.7.2-1.
Compiling the actual u-boot leaded to the reported error. All version since commit cb55b3320014b7f6014416c556fe506efbf0a84b led to the hanging. All version before boots correctly.
Adding the define to omap3_beagle.h leads to a correct booting for all versions. Also a reset of the uart at the beginning of the init function leads to a correct booting. So I exclude that the hardware is defective.
If you can tell me your compiler and version I can try to use your cross-compiler.
Best regards, Manfred
On Tue, Mar 19, 2013 at 3:49 PM, Tom Rini trini@ti.com wrote:
On Sat, Mar 16, 2013 at 02:13:54PM +0100, Manfred Huber wrote:
I'm surprised that no one is interested in a functioning Beagleboard. Has no one tested the Beagleboard since 2012-09-19?
I don't see this problem on mine (classic and xM), which is probably part of the why. I'm inclined to accept the patch, but can you try two things please:
- How reproducible is this problem, with the host and beagleboard combination you have? 100%?
- Do you have another beagleboard or another host PC (or USB-Serial dongle) you can try?
Thanks!
-- Tom
Hi,
I had this issue on another TI OMAP3 based board (IGEPv2) which use an DM3730 processor. Other IGEP board users reported that U-Boot hung on their boards and that a patch to not wait for the Transmitter Empty (TEMT) to initialize the serial console fixed the issue. So I added the CONFIG_SYS_NS16550_BROKEN_TEMT config option and used it for IGEP boards (igep00x0) to make them boot again.
Back then I also tested on a Beagleboard Rev. C4 since it has the same ns16550 UART controller than the IGEPv2 as far as I understood. I used the exact U-Boot version, USB-Serial cable, host PC and terminal emulation program that I used for the IGEPv2 and the Beagleboard booted correctly. This is the same behavior that Tom had on his Beagleboard.
Since it worked on the Beagle I thought the issue was only present on IGEP boards but now Manfred says that he has the same issue on his Beagleboard. I now wonder if all IGEPv2 are broken or only my board and the ones of the users that cared to report this and other IGEPv2 can boot without CONFIG_SYS_NS16550_BROKEN_TEMT.
Thanks a lot and best regards, Javier
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 03/19/2013 08:05 PM, Javier Martinez Canillas wrote:
On Tue, Mar 19, 2013 at 3:49 PM, Tom Rini trini@ti.com wrote:
On Sat, Mar 16, 2013 at 02:13:54PM +0100, Manfred Huber wrote:
I'm surprised that no one is interested in a functioning Beagleboard. Has no one tested the Beagleboard since 2012-09-19?
I don't see this problem on mine (classic and xM), which is probably part of the why. I'm inclined to accept the patch, but can you try two things please: - How reproducible is this problem, with the host and beagleboard combination you have? 100%? - Do you have another beagleboard or another host PC (or USB-Serial dongle) you can try?
Thanks!
-- Tom
Hi,
I had this issue on another TI OMAP3 based board (IGEPv2) which use an DM3730 processor. Other IGEP board users reported that U-Boot hung on their boards and that a patch to not wait for the Transmitter Empty (TEMT) to initialize the serial console fixed the issue. So I added the CONFIG_SYS_NS16550_BROKEN_TEMT config option and used it for IGEP boards (igep00x0) to make them boot again.
Back then I also tested on a Beagleboard Rev. C4 since it has the same ns16550 UART controller than the IGEPv2 as far as I understood. I used the exact U-Boot version, USB-Serial cable, host PC and terminal emulation program that I used for the IGEPv2 and the Beagleboard booted correctly. This is the same behavior that Tom had on his Beagleboard.
Since it worked on the Beagle I thought the issue was only present on IGEP boards but now Manfred says that he has the same issue on his Beagleboard. I now wonder if all IGEPv2 are broken or only my board and the ones of the users that cared to report this and other IGEPv2 can boot without CONFIG_SYS_NS16550_BROKEN_TEMT.
Yeah, this is very perplexing. I'm thinking we need to enable this "quirk" for all of the "omap" platforms except for OMAP3_ZOOM (which iirc has a different uart chip wired up rather than the SoC uart).
- -- Tom
Am 20.03.2013 02:27, schrieb Tom Rini:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 03/19/2013 08:05 PM, Javier Martinez Canillas wrote:
On Tue, Mar 19, 2013 at 3:49 PM, Tom Rini trini@ti.com wrote:
On Sat, Mar 16, 2013 at 02:13:54PM +0100, Manfred Huber wrote:
I'm surprised that no one is interested in a functioning Beagleboard. Has no one tested the Beagleboard since 2012-09-19?
I don't see this problem on mine (classic and xM), which is probably part of the why. I'm inclined to accept the patch, but can you try two things please: - How reproducible is this problem, with the host and beagleboard combination you have? 100%? - Do you have another beagleboard or another host PC (or USB-Serial dongle) you can try?
Thanks!
-- Tom
Hi,
I had this issue on another TI OMAP3 based board (IGEPv2) which use an DM3730 processor. Other IGEP board users reported that U-Boot hung on their boards and that a patch to not wait for the Transmitter Empty (TEMT) to initialize the serial console fixed the issue. So I added the CONFIG_SYS_NS16550_BROKEN_TEMT config option and used it for IGEP boards (igep00x0) to make them boot again.
Back then I also tested on a Beagleboard Rev. C4 since it has the same ns16550 UART controller than the IGEPv2 as far as I understood. I used the exact U-Boot version, USB-Serial cable, host PC and terminal emulation program that I used for the IGEPv2 and the Beagleboard booted correctly. This is the same behavior that Tom had on his Beagleboard.
Since it worked on the Beagle I thought the issue was only present on IGEP boards but now Manfred says that he has the same issue on his Beagleboard. I now wonder if all IGEPv2 are broken or only my board and the ones of the users that cared to report this and other IGEPv2 can boot without CONFIG_SYS_NS16550_BROKEN_TEMT.
Yeah, this is very perplexing. I'm thinking we need to enable this "quirk" for all of the "omap" platforms except for OMAP3_ZOOM (which iirc has a different uart chip wired up rather than the SoC uart).
Tom
As I described in my first mail the reason is the missing UART_LSR_TEMT bit in the line status register. Comparing the UART_LSR_THRE bit instead works in my case.
Maybe Javier can test on his IGEP board if it works for him to compare the UART_LSR_THRE bit instead of the UART_LSR_TEMT bit.
Another solution would be to make a soft reset of the UART as first command in the init function. This works also in my case.
On the other hand we can try to identify which OMAP3 has this behavior. The module version register of the UART returns version 4.6.
Best regards, Manfred
On Thu, Mar 21, 2013 at 12:09 AM, Manfred Huber man.huber@arcor.de wrote:
Am 20.03.2013 02:27, schrieb Tom Rini:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 03/19/2013 08:05 PM, Javier Martinez Canillas wrote:
On Tue, Mar 19, 2013 at 3:49 PM, Tom Rini trini@ti.com wrote:
On Sat, Mar 16, 2013 at 02:13:54PM +0100, Manfred Huber wrote:
I'm surprised that no one is interested in a functioning Beagleboard. Has no one tested the Beagleboard since 2012-09-19?
I don't see this problem on mine (classic and xM), which is probably part of the why. I'm inclined to accept the patch, but can you try two things please: - How reproducible is this problem, with the host and beagleboard combination you have? 100%? - Do you have another beagleboard or another host PC (or USB-Serial dongle) you can try?
Thanks!
-- Tom
Hi,
I had this issue on another TI OMAP3 based board (IGEPv2) which use an DM3730 processor. Other IGEP board users reported that U-Boot hung on their boards and that a patch to not wait for the Transmitter Empty (TEMT) to initialize the serial console fixed the issue. So I added the CONFIG_SYS_NS16550_BROKEN_TEMT config option and used it for IGEP boards (igep00x0) to make them boot again.
Back then I also tested on a Beagleboard Rev. C4 since it has the same ns16550 UART controller than the IGEPv2 as far as I understood. I used the exact U-Boot version, USB-Serial cable, host PC and terminal emulation program that I used for the IGEPv2 and the Beagleboard booted correctly. This is the same behavior that Tom had on his Beagleboard.
Since it worked on the Beagle I thought the issue was only present on IGEP boards but now Manfred says that he has the same issue on his Beagleboard. I now wonder if all IGEPv2 are broken or only my board and the ones of the users that cared to report this and other IGEPv2 can boot without CONFIG_SYS_NS16550_BROKEN_TEMT.
Yeah, this is very perplexing. I'm thinking we need to enable this "quirk" for all of the "omap" platforms except for OMAP3_ZOOM (which iirc has a different uart chip wired up rather than the SoC uart).
Tom
As I described in my first mail the reason is the missing UART_LSR_TEMT bit in the line status register. Comparing the UART_LSR_THRE bit instead works in my case.
Maybe Javier can test on his IGEP board if it works for him to compare the UART_LSR_THRE bit instead of the UART_LSR_TEMT bit.
I see the same behavior in the IGEPv2. I tested mainline U-boot + your "[PATCH] omap3_beagle: Enable CONFIG_SYS_NS16550_BROKEN_TEMT" patch and SPL boots OK on my board.
Another solution would be to make a soft reset of the UART as first command in the init function. This works also in my case.
I prefer this approach if it doesn't affect other boards.
On the other hand we can try to identify which OMAP3 has this behavior. The module version register of the UART returns version 4.6.
Best regards, Manfred
Best regards, Javier
Hi all,
probably I found the error. It should be a bug in the ROM code of the OMAP3.
Depending on the Booting Sequence of the OMAP3, SPL is hanging or not. If ROM code configures UART3 before SPL starts from NAND it will lead to a hang of the SPL. If UART3 is not configured, SPL boots correctly from NAND.
If ROM code configures UART3 it leaves the UART in a bad state. Although Transmitter is not empty (TEMT is not set), ROM code disables the UART. Therefore Transmitter can never get empty.
In a normal Booting Sequence where NAND is for UART3 the bug will never appear.
There are some solutions:
1. Configuring UART3 first and let Transmitter send until Transmitter is empty. 2. Soft reset of Transmitter. (OMAP3 specific, NS16550 has no soft reset) 3. ...
Please tell me your assessment.
Best regards, Manfred Huber
From: Manfred Huber
Beagleboard UART (ns16550) doesn't set the Transmitter Empty (TEMT) Bit in SPL. Only Transmitter Hold Register Empty (THRE) Bit is set. This makes SPL to hang while waiting for TEMT. Adding the CONFIG_SYS_NS16550_BROKEN_TEMT config option and waiting for THRE avoid this issue.
Signed-off-by: Manfred Huber man.huber@arcor.de --- drivers/serial/ns16550.c | 5 ++++- include/configs/omap3_beagle.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..6379bcc 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,7 +36,10 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SYS_NS16550_BROKEN_TEMT) + while (!(serial_in(&com_port->lsr) & UART_LSR_THRE)) + ; +#else while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ; #endif diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 48ce4c0..6ab46d5 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -82,6 +82,9 @@ #define CONFIG_SYS_NS16550_REG_SIZE (-4) #define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
+/* define to avoid U-Boot to hang while waiting for TEMT */ +#define CONFIG_SYS_NS16550_BROKEN_TEMT + /* * select serial console configuration */
Hi Manfred,
On Thu, Mar 21, 2013 at 8:03 PM, Manfred Huber man.huber@arcor.de wrote:
From: Manfred Huber
Beagleboard UART (ns16550) doesn't set the Transmitter Empty (TEMT) Bit in SPL. Only Transmitter Hold Register Empty (THRE) Bit is set. This makes SPL to hang while waiting for TEMT. Adding the CONFIG_SYS_NS16550_BROKEN_TEMT config option and waiting for THRE avoid this issue.
Signed-off-by: Manfred Huber man.huber@arcor.de
drivers/serial/ns16550.c | 5 ++++- include/configs/omap3_beagle.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..6379bcc 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,7 +36,10 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SYS_NS16550_BROKEN_TEMT)
while (!(serial_in(&com_port->lsr) & UART_LSR_THRE));+#else
I'm not sure to add this behavior under CONFIG_SYS_NS16550_BROKEN_TEMT since this option just means:
"On some broken platforms the Transmitter Empty (TEMT) bit is not set in SPL making U-Boot to hang while waiting for it"
According to your findings it seems that some OMAP3 platforms (at least DM3730 and 3530) set THRE but I wonder if other broken platforms will behave the same.
The current CONFIG_SYS_NS16550_BROKEN_TEMT just skips this test so it can be reused by other platforms, but now your change makes it less generic since it will only work on platforms that set THRE.
So I would just leave CONFIG_SYS_NS16550_BROKEN_TEMT as is now and be able to reuse it on other platforms or add a new config option CONFIG_SYS_NS16550_WAIT_THRE or something that would test for THRE instead of TEMT and use that for OMAP3 based boards.
I do like your change that adds a test for CONFIG_SPL_BUILD since even in the README it says that the issue is only present in SPL. So I just forgot to add this when added the config option.
while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;#endif diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 48ce4c0..6ab46d5 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -82,6 +82,9 @@ #define CONFIG_SYS_NS16550_REG_SIZE (-4) #define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
+/* define to avoid U-Boot to hang while waiting for TEMT */ +#define CONFIG_SYS_NS16550_BROKEN_TEMT
/*
- select serial console configuration
*/
This part of your patch looks good to me but you should split your changes in two patches. One to change the ns16550 drvier and another one to add this config option for the Beagleboard.
Thanks a lot and best regards, Javier
On Thu, Mar 21, 2013 at 08:03:59PM +0100, Manfred Huber wrote:
From: Manfred Huber
Beagleboard UART (ns16550) doesn't set the Transmitter Empty (TEMT) Bit in SPL. Only Transmitter Hold Register Empty (THRE) Bit is set. This makes SPL to hang while waiting for TEMT. Adding the CONFIG_SYS_NS16550_BROKEN_TEMT config option and waiting for THRE avoid this issue.
Signed-off-by: Manfred Huber man.huber@arcor.de
drivers/serial/ns16550.c | 5 ++++- include/configs/omap3_beagle.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..6379bcc 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,7 +36,10 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SYS_NS16550_BROKEN_TEMT)
- while (!(serial_in(&com_port->lsr) & UART_LSR_THRE))
;+#else while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ; #endif
Scott, do you still have access to the failing systems that made us introduce this change to start with? Could we perhaps go with the THRE test instead in all cases? Thanks!
On 03/21/2013 05:21:00 PM, Tom Rini wrote:
On Thu, Mar 21, 2013 at 08:03:59PM +0100, Manfred Huber wrote:
From: Manfred Huber
Beagleboard UART (ns16550) doesn't set the Transmitter Empty (TEMT) Bit in SPL.
The serial port behaves differently based on the stage of U-Boot that is running?
Or is it that the bit doesn't get set until the port has been properly initialized? Couldn't that happen in a case where SPL isn't used at all?
Only Transmitter Hold Register Empty (THRE) Bit is set. This makes SPL to hang while waiting for TEMT. Adding the CONFIG_SYS_NS16550_BROKEN_TEMT config option and waiting for THRE avoid this issue.
Signed-off-by: Manfred Huber man.huber@arcor.de
drivers/serial/ns16550.c | 5 ++++- include/configs/omap3_beagle.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..6379bcc 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,7 +36,10 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if defined(CONFIG_SPL_BUILD) &&
defined(CONFIG_SYS_NS16550_BROKEN_TEMT)
- while (!(serial_in(&com_port->lsr) & UART_LSR_THRE))
;+#else while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ; #endif
Scott, do you still have access to the failing systems that made us introduce this change to start with?
It was an intermittent failure seen on a development tree, so not really. You could try testing it by printing something immediately before calling NS16550_init(), either in a situation where you know the serial port is already configured (e.g. by SPL) or by calling NS16550_init() twice.
Could we perhaps go with the THRE test instead in all cases? Thanks!
Wouldn't that still allow the last character to possibly be corrupted?
-Scott
From: Manfred Huber
Due to a Bug in the ROM code of some OMAP3 devices, the TEMT bit is not set if UART3 is configured before (only THRE is set). Reason is the disabling of UART3 even though the Transmitter is not empty. Enabling UART3 allows the Transmitter to be empty.
Signed-off-by: Manfred Huber man.huber@arcor.de --- drivers/serial/ns16550.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..24ff84f 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,18 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX)) + if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE)) == UART_LSR_THRE) { + serial_out(UART_LCR_DLAB, &com_port->lcr); + serial_out(baud_divisor & 0xff, &com_port->dll); + serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm); + serial_out(UART_LCRVAL, &com_port->lcr); + serial_out(0, &com_port->mdr1); + } +#endif + while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ; -#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier); #if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
Please test the Patch. It is very simple on a Beagleboard. I guess you have flashed the actual SPL and u-boot and Beagleboard boots correctly. Now press and hold 'User' button and connect power. SPL should hang. You can see some symbols on the console from the ROM code.
Install the Patch, compile it and flash the NAND. Beagleboard still boots correctly. Now press and hold 'User' button again and Beagleboard should also boot correctly. The Patch works.
I suspect the IGEP board has the same bug. If so, the Patch should work on this board too and we don't need CONFIG_SYS_NS16550_BROKEN_TEMT anymore.
If you don't want a patch for this bug please let me know. I will not bother you again.
Best regards, Manfred
On 2013-03-25 23:02, Manfred Huber wrote:
From: Manfred Huber
Due to a Bug in the ROM code of some OMAP3 devices, the TEMT bit is not set if UART3 is configured before (only THRE is set). Reason is the disabling of UART3 even though the Transmitter is not empty. Enabling UART3 allows the Transmitter to be empty.
Signed-off-by: Manfred Huber man.huber@arcor.de
drivers/serial/ns16550.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..24ff84f 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,18 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
- if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))
== UART_LSR_THRE) {
serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);- }
+#endif
while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;-#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);#if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Wed, Mar 27, 2013 at 5:50 AM, Manfred Huber man.huber@arcor.de wrote:
Please test the Patch. It is very simple on a Beagleboard. I guess you have flashed the actual SPL and u-boot and Beagleboard boots correctly. Now press and hold 'User' button and connect power. SPL should hang. You can see some symbols on the console from the ROM code.
Hi Manfred,
I don't have access to my IGEP board right now but I'll test your patch as soon as posible.
Install the Patch, compile it and flash the NAND. Beagleboard still boots correctly. Now press and hold 'User' button again and Beagleboard should also boot correctly. The Patch works.
I suspect the IGEP board has the same bug. If so, the Patch should work on this board too and we don't need CONFIG_SYS_NS16550_BROKEN_TEMT anymore.
I still think that we should keep CONFIG_SYS_NS16550_BROKEN_TEMT or something similar instead of just checking for CONFIG_OMAP34XX. Since we don't know if this problem is also present on other TI OMAP platforms (or any other platform). I would just change CONFIG_SYS_NS16550_BROKEN_TEMT semantics once is confirmed that this also fixes the issue on IGEP boards.
Also, if you are removing CONFIG_SYS_NS16550_BROKEN_TEMT then you have to update the README too since it is explained there what this config option does.
If you don't want a patch for this bug please let me know. I will not bother you again.
Thanks a lot for working on this. Some people work on mainline U-Boot on their free time (like myself) so it can take a few days until you get a response about a patch. Please be patient :-)
Best regards, Manfred
Thank a lot and best regards, Javier
On 2013-03-25 23:02, Manfred Huber wrote:
From: Manfred Huber
Due to a Bug in the ROM code of some OMAP3 devices, the TEMT bit is not set if UART3 is configured before (only THRE is set). Reason is the disabling of UART3 even though the Transmitter is not empty. Enabling UART3 allows the Transmitter to be empty.
Signed-off-by: Manfred Huber man.huber@arcor.de
drivers/serial/ns16550.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..24ff84f 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,18 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
- if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))
== UART_LSR_THRE) {
serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);- }
+#endif
while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;-#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);#if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Wed, Mar 27, 2013 at 10:29:00AM +0100, Javier Martinez Canillas wrote:
On Wed, Mar 27, 2013 at 5:50 AM, Manfred Huber man.huber@arcor.de wrote:
Please test the Patch. It is very simple on a Beagleboard. I guess you have flashed the actual SPL and u-boot and Beagleboard boots correctly. Now press and hold 'User' button and connect power. SPL should hang. You can see some symbols on the console from the ROM code.
Hi Manfred,
I don't have access to my IGEP board right now but I'll test your patch as soon as posible.
Install the Patch, compile it and flash the NAND. Beagleboard still boots correctly. Now press and hold 'User' button again and Beagleboard should also boot correctly. The Patch works.
I suspect the IGEP board has the same bug. If so, the Patch should work on this board too and we don't need CONFIG_SYS_NS16550_BROKEN_TEMT anymore.
I still think that we should keep CONFIG_SYS_NS16550_BROKEN_TEMT or something similar instead of just checking for CONFIG_OMAP34XX. Since we don't know if this problem is also present on other TI OMAP platforms (or any other platform). I would just change CONFIG_SYS_NS16550_BROKEN_TEMT semantics once is confirmed that this also fixes the issue on IGEP boards.
I'm attempting to see if this ROM bug is (a) known and (b) is or was fixed at some point. Based on what I know (or at least think I know) of how the ROM code lived for "OMAP3", given that you're seeing this on beagle and igep, it's a valid assumption that OMAP34XX is buggy. The question I have (and hope for resolution on) is if it's a problem on am335x/omap4 or later as well. If I follow how to reproduce this issue (and I think I do), assuming I can I'll setup an am335x platform in a similar position.
If you don't want a patch for this bug please let me know. I will not bother you again.
Thanks a lot for working on this. Some people work on mainline U-Boot on their free time (like myself) so it can take a few days until you get a response about a patch. Please be patient :-)
Yes, thanks for working this issue through, I also appreciate it!
On 2013-03-27 10:29, Javier Martinez Canillas wrote:
On Wed, Mar 27, 2013 at 5:50 AM, Manfred Huber man.huber@arcor.de wrote:
Please test the Patch. It is very simple on a Beagleboard. I guess you have flashed the actual SPL and u-boot and Beagleboard boots correctly. Now press and hold 'User' button and connect power. SPL should hang. You can see some symbols on the console from the ROM code.
Hi Manfred,
I don't have access to my IGEP board right now but I'll test your patch as soon as posible.
Install the Patch, compile it and flash the NAND. Beagleboard still boots correctly. Now press and hold 'User' button again and Beagleboard should also boot correctly. The Patch works.
I suspect the IGEP board has the same bug. If so, the Patch should work on this board too and we don't need CONFIG_SYS_NS16550_BROKEN_TEMT anymore.
I still think that we should keep CONFIG_SYS_NS16550_BROKEN_TEMT or something similar instead of just checking for CONFIG_OMAP34XX. Since we don't know if this problem is also present on other TI OMAP platforms (or any other platform). I would just change CONFIG_SYS_NS16550_BROKEN_TEMT semantics once is confirmed that this also fixes the issue on IGEP boards.
Also, if you are removing CONFIG_SYS_NS16550_BROKEN_TEMT then you have to update the README too since it is explained there what this config option does.
If you don't want a patch for this bug please let me know. I will not bother you again.
Thanks a lot for working on this. Some people work on mainline U-Boot on their free time (like myself) so it can take a few days until you get a response about a patch. Please be patient :-)
Hi Javier,
Also like me. The patch is not for me, because I have written my own serial driver. So this comment was only not to waste your and my time if no one needs this patch.
Best regards, Manfred
Best regards, Manfred
Thank a lot and best regards, Javier
On 2013-03-25 23:02, Manfred Huber wrote:
From: Manfred Huber
Due to a Bug in the ROM code of some OMAP3 devices, the TEMT bit is not set if UART3 is configured before (only THRE is set). Reason is the disabling of UART3 even though the Transmitter is not empty. Enabling UART3 allows the Transmitter to be empty.
Signed-off-by: Manfred Huber man.huber@arcor.de
drivers/serial/ns16550.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..24ff84f 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,18 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
- if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))
== UART_LSR_THRE) {
serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);- }
+#endif
while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;-#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);#if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 2013-03-27 10:29, Javier Martinez Canillas wrote:
On Wed, Mar 27, 2013 at 5:50 AM, Manfred Huber man.huber@arcor.de wrote:
<snip>
I still think that we should keep CONFIG_SYS_NS16550_BROKEN_TEMT or something similar instead of just checking for CONFIG_OMAP34XX. Since we don't know if this problem is also present on other TI OMAP platforms (or any other platform). I would just change CONFIG_SYS_NS16550_BROKEN_TEMT semantics once is confirmed that this also fixes the issue on IGEP boards.
If we continue using CONFIG_SYS_NS16550_BROKEN_TEMT, there is no benefit by my patch. It works with the define alone.
Also, if you are removing CONFIG_SYS_NS16550_BROKEN_TEMT then you have to update the README too since it is explained there what this config option does.
And also in igep00x0.h
<snip>
Thank a lot and best regards, Javier
On 2013-03-25 23:02, Manfred Huber wrote:
<snip>
On Wed, Mar 27, 2013 at 05:50:17AM +0100, Manfred Huber wrote:
Please test the Patch. It is very simple on a Beagleboard. I guess you have flashed the actual SPL and u-boot and Beagleboard boots correctly. Now press and hold 'User' button and connect power. SPL should hang. You can see some symbols on the console from the ROM code.
I cannot reproduce this problem on my Beagleboard C5. I note this only so it's clear I tried things out. I'm still supportive of the changes overall as they also don't break anything here. I'm also still waiting to hear back from some folks that might know what the ROM code does or doesn't do.
Dear Manfred Huber,
---8<--- abiessmann@punisher % pwclient get 230994 Saved patch to U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch abiessmann@punisher % git am U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch Patch does not have a valid e-mail address. abiessmann@punisher % ./tools/checkpatch.pl U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch ERROR: trailing whitespace #39: FILE: drivers/serial/ns16550.c:40: +^Iif ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE)) == $
ERROR: patch seems to be corrupt (line wrapped?) #40: FILE: drivers/serial/ns16550.c:40: UART_LSR_THRE) {
total: 2 errors, 0 warnings, 20 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or scripts/cleanfile
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch has style problems, please review.
If any of these errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. --->8---
Can you please fix these errors?
On 03/25/2013 11:02 PM, Manfred Huber wrote:
From: Manfred Huber
Due to a Bug in the ROM code of some OMAP3 devices, the TEMT bit is not set if UART3 is configured before (only THRE is set). Reason is the disabling of UART3 even though the Transmitter is not empty. Enabling UART3 allows the Transmitter to be empty.
Signed-off-by: Manfred Huber man.huber@arcor.de
drivers/serial/ns16550.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..24ff84f 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,18 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
I think a comment here would be good.
- if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))
== UART_LSR_THRE) {
The patch is broken here, even if you fix the wrapping you will get an 'over 80 char' error here.
serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);
Do we need to setup baud a.s.o. here? Isn't it enough to issue an soft reset of the UART? I'm not in this material, I just wonder if we can omit some of the lines here cause we set e.g. the BAUD later on.
- }
+#endif
- while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;
-#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);#if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
I managed to apply your patch anyhow. A short test on a tricorder board showed no harm to the boot process. So please get your patch clean and resend, then I will add my tested-by.
As Javier pointed out please think about using the CONFIG_SYS_NS16550_BROKEN_TEMT instead of SPL && OMAP34XX. Another solution could be to have this TEMT | THRE check in unconditionally, this however would require a lot more testing. Especially with the release date in mind.
Best regards
Andreas Bießmann
On Wed, Mar 27, 2013 at 2:37 PM, Andreas Bießmann andreas.devel@googlemail.com wrote:
Dear Manfred Huber,
---8<--- abiessmann@punisher % pwclient get 230994 Saved patch to U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch abiessmann@punisher % git am U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch Patch does not have a valid e-mail address. abiessmann@punisher % ./tools/checkpatch.pl U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch ERROR: trailing whitespace #39: FILE: drivers/serial/ns16550.c:40: +^Iif ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE)) == $
ERROR: patch seems to be corrupt (line wrapped?) #40: FILE: drivers/serial/ns16550.c:40: UART_LSR_THRE) {
total: 2 errors, 0 warnings, 20 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or scripts/cleanfile
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch has style problems, please review.
If any of these errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. --->8---
Can you please fix these errors?
On 03/25/2013 11:02 PM, Manfred Huber wrote:
From: Manfred Huber
Due to a Bug in the ROM code of some OMAP3 devices, the TEMT bit is not set if UART3 is configured before (only THRE is set). Reason is the disabling of UART3 even though the Transmitter is not empty. Enabling UART3 allows the Transmitter to be empty.
Signed-off-by: Manfred Huber man.huber@arcor.de
drivers/serial/ns16550.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..24ff84f 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,18 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
I think a comment here would be good.
- if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))
== UART_LSR_THRE) {
The patch is broken here, even if you fix the wrapping you will get an 'over 80 char' error here.
serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);Do we need to setup baud a.s.o. here? Isn't it enough to issue an soft reset of the UART? I'm not in this material, I just wonder if we can omit some of the lines here cause we set e.g. the BAUD later on.
- }
+#endif
- while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;
-#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);#if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
I managed to apply your patch anyhow. A short test on a tricorder board showed no harm to the boot process. So please get your patch clean and resend, then I will add my tested-by.
As Javier pointed out please think about using the CONFIG_SYS_NS16550_BROKEN_TEMT instead of SPL && OMAP34XX. Another solution could be to have this TEMT | THRE check in unconditionally, this however would require a lot more testing. Especially with the release date in mind.
Best regards
Andreas Bießmann
Hi Manfred,
I just tested your patch and my IGEPv2 boots correctly.
So, after addressing the issues pointed out by Andreas feel free to add my Tested-by too.
Thanks a lot and best regards, Javier
On 2013-03-27 14:37, Andreas Bießmann wrote:
Dear Manfred Huber,
---8<--- abiessmann@punisher % pwclient get 230994 Saved patch to U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch abiessmann@punisher % git am U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch Patch does not have a valid e-mail address. abiessmann@punisher % ./tools/checkpatch.pl U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch ERROR: trailing whitespace #39: FILE: drivers/serial/ns16550.c:40: +^Iif ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE)) == $
ERROR: patch seems to be corrupt (line wrapped?) #40: FILE: drivers/serial/ns16550.c:40: UART_LSR_THRE) {
total: 2 errors, 0 warnings, 20 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or scripts/cleanfile
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch has style problems, please review.
If any of these errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. --->8---
Can you please fix these errors?
I will do it.
On 03/25/2013 11:02 PM, Manfred Huber wrote:
From: Manfred Huber
Due to a Bug in the ROM code of some OMAP3 devices, the TEMT bit is not set if UART3 is configured before (only THRE is set). Reason is the disabling of UART3 even though the Transmitter is not empty. Enabling UART3 allows the Transmitter to be empty.
Signed-off-by: Manfred Huber man.huber@arcor.de
drivers/serial/ns16550.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..24ff84f 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,18 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
I think a comment here would be good.
I will do it.
- if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))
== UART_LSR_THRE) {
The patch is broken here, even if you fix the wrapping you will get an 'over 80 char' error here.
serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);Do we need to setup baud a.s.o. here? Isn't it enough to issue an soft reset of the UART? I'm not in this material, I just wonder if we can omit some of the lines here cause we set e.g. the BAUD later on.
The reason to setup the baud is for the shift register. It only works with programmed baud registers. A soft reset would also work, but as Scott Wood said it would corrupt the last character. On the other hand the character should be corrupted by disabling the UART. I have no preferred solution: programming the UART or a soft reset. Maybe someone wants to decide.
- }
+#endif
while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;-#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);#if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
I managed to apply your patch anyhow. A short test on a tricorder board showed no harm to the boot process. So please get your patch clean and resend, then I will add my tested-by.
As Javier pointed out please think about using the CONFIG_SYS_NS16550_BROKEN_TEMT instead of SPL && OMAP34XX. Another solution could be to have this TEMT | THRE check in unconditionally, this however would require a lot more testing. Especially with the release date in mind.
It's not critical. So I guess it's not needed for this release.
Best regards
Andreas Bießmann
Best regards, Manfred
Dear Manfred Huber,
On 03/28/2013 07:06 AM, Manfred Huber wrote:
On 2013-03-27 14:37, Andreas Bießmann wrote:
<snip>
On 03/25/2013 11:02 PM, Manfred Huber wrote:
<snip>
serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);Do we need to setup baud a.s.o. here? Isn't it enough to issue an soft reset of the UART? I'm not in this material, I just wonder if we can omit some of the lines here cause we set e.g. the BAUD later on.
The reason to setup the baud is for the shift register. It only works with programmed baud registers. A soft reset would also work, but as Scott Wood said it would corrupt the last character. On the other hand the character should be corrupted by disabling the UART. I have no preferred solution: programming the UART or a soft reset. Maybe someone wants to decide.
Well, I think also that re-programming the UART will destroy the last character in shift register anyway. I wonder which use-case requires UART flushing in u-boot context before initializing the UART for u-boot correctly. Can someone explain this to me? Shouldn't we always start here from the very beginning and setup UART as configured?
- }
+#endif
while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;-#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);#if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
I managed to apply your patch anyhow. A short test on a tricorder board showed no harm to the boot process. So please get your patch clean and resend, then I will add my tested-by.
As Javier pointed out please think about using the CONFIG_SYS_NS16550_BROKEN_TEMT instead of SPL && OMAP34XX. Another solution could be to have this TEMT | THRE check in unconditionally, this however would require a lot more testing. Especially with the release date in mind.
It's not critical. So I guess it's not needed for this release.
Well, if there are boards in the field that will not boot with the next release I think it is critical. We do have some omap3 (omap35xx and am37xx) based boards here. I can recall a situation where some few boards did not boot from sd-card while serial debug cable was attached (AFAIR this was not the case when booting from NAND). The root cause was never investigated, so maybe we suffered exactly this bug.
Best regards
Andreas Bießmann
On Thu, Mar 28, 2013 at 9:45 AM, Andreas Bießmann andreas.devel@googlemail.com wrote:
Dear Manfred Huber,
On 03/28/2013 07:06 AM, Manfred Huber wrote:
On 2013-03-27 14:37, Andreas Bießmann wrote:
<snip>
On 03/25/2013 11:02 PM, Manfred Huber wrote:
<snip>
serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);Do we need to setup baud a.s.o. here? Isn't it enough to issue an soft reset of the UART? I'm not in this material, I just wonder if we can omit some of the lines here cause we set e.g. the BAUD later on.
The reason to setup the baud is for the shift register. It only works with programmed baud registers. A soft reset would also work, but as Scott Wood said it would corrupt the last character. On the other hand the character should be corrupted by disabling the UART. I have no preferred solution: programming the UART or a soft reset. Maybe someone wants to decide.
Well, I think also that re-programming the UART will destroy the last character in shift register anyway. I wonder which use-case requires UART flushing in u-boot context before initializing the UART for u-boot correctly. Can someone explain this to me? Shouldn't we always start here from the very beginning and setup UART as configured?
- }
+#endif
while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;-#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);#if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
I managed to apply your patch anyhow. A short test on a tricorder board showed no harm to the boot process. So please get your patch clean and resend, then I will add my tested-by.
As Javier pointed out please think about using the CONFIG_SYS_NS16550_BROKEN_TEMT instead of SPL && OMAP34XX. Another solution could be to have this TEMT | THRE check in unconditionally, this however would require a lot more testing. Especially with the release date in mind.
It's not critical. So I guess it's not needed for this release.
Well, if there are boards in the field that will not boot with the next release I think it is critical. We do have some omap3 (omap35xx and am37xx) based boards here. I can recall a situation where some few boards did not boot from sd-card while serial debug cable was attached (AFAIR this was not the case when booting from NAND). The root cause was never investigated, so maybe we suffered exactly this bug.
Best regards
Andreas Bießmann
Hi Andreas,
When I first hit this bug I tried removing the serial debug cable and this made my IGEPv2 to boot correctly. Then I looked at the latest changes to the serial ns16550 driver and found that cb55b332 "serial/ns16550: wait for TEMT before initializing" was the culprit commit that made my board to not boot.
So, if I remember correctly, it seems as you hit the exact same bug (I didn't try to boot from NAND back then though)
Best regards, Javier
Hi Javier,
On 03/28/2013 10:11 AM, Javier Martinez Canillas wrote:
On Thu, Mar 28, 2013 at 9:45 AM, Andreas Bießmann andreas.devel@googlemail.com wrote:
Dear Manfred Huber,
On 03/28/2013 07:06 AM, Manfred Huber wrote:
On 2013-03-27 14:37, Andreas Bießmann wrote:
<snip>
The reason to setup the baud is for the shift register. It only works with programmed baud registers. A soft reset would also work, but as Scott Wood said it would corrupt the last character. On the other hand the character should be corrupted by disabling the UART. I have no preferred solution: programming the UART or a soft reset. Maybe someone wants to decide.
Well, I think also that re-programming the UART will destroy the last character in shift register anyway. I wonder which use-case requires UART flushing in u-boot context before initializing the UART for u-boot correctly. Can someone explain this to me? Shouldn't we always start here from the very beginning and setup UART as configured?
<snip>
When I first hit this bug I tried removing the serial debug cable and this made my IGEPv2 to boot correctly. Then I looked at the latest changes to the serial ns16550 driver and found that cb55b332 "serial/ns16550: wait for TEMT before initializing" was the culprit commit that made my board to not boot.
Thanks for clarification. So there is a need to flush UART before initializing it in u-boot context as said by Scott's comment in cb55b332, at least for some systems.
I think Manfred's proposed solution in this patch is ok. It fixes a bug when transiting from (some) OMAP ROM code to SPL, therefore we should have the code conditionally on SPL and OMAP. Maybe we find out later, that this also matches other combinations. But for now I think we should just take Manfred's solution in this release, Tom?
Many thanks to you Manfred for forcing attention on this bug and providing a solution.
Best regards
Andreas Bießmann
On Thu, Mar 28, 2013 at 10:50:44AM +0100, Andreas Bie??mann wrote:
Hi Javier,
On 03/28/2013 10:11 AM, Javier Martinez Canillas wrote:
On Thu, Mar 28, 2013 at 9:45 AM, Andreas Bie??mann andreas.devel@googlemail.com wrote:
Dear Manfred Huber,
On 03/28/2013 07:06 AM, Manfred Huber wrote:
On 2013-03-27 14:37, Andreas Bie??mann wrote:
<snip>
The reason to setup the baud is for the shift register. It only works with programmed baud registers. A soft reset would also work, but as Scott Wood said it would corrupt the last character. On the other hand the character should be corrupted by disabling the UART. I have no preferred solution: programming the UART or a soft reset. Maybe someone wants to decide.
Well, I think also that re-programming the UART will destroy the last character in shift register anyway. I wonder which use-case requires UART flushing in u-boot context before initializing the UART for u-boot correctly. Can someone explain this to me? Shouldn't we always start here from the very beginning and setup UART as configured?
We shouldn't be concerned with what is destroyed here as it's going to be related to ROM trying (and then failing and moving on from) to load via UART the next program. We've already started and are running, so the UART is ours to do with as we need. If ROM did load us over UART it really should already be in a clean state.
<snip>
When I first hit this bug I tried removing the serial debug cable and this made my IGEPv2 to boot correctly. Then I looked at the latest changes to the serial ns16550 driver and found that cb55b332 "serial/ns16550: wait for TEMT before initializing" was the culprit commit that made my board to not boot.
Thanks for clarification. So there is a need to flush UART before initializing it in u-boot context as said by Scott's comment in cb55b332, at least for some systems.
I think Manfred's proposed solution in this patch is ok. It fixes a bug when transiting from (some) OMAP ROM code to SPL, therefore we should have the code conditionally on SPL and OMAP. Maybe we find out later, that this also matches other combinations. But for now I think we should just take Manfred's solution in this release, Tom?
Agreed.
Many thanks to you Manfred for forcing attention on this bug and providing a solution.
And also very much agreed!
Am 28.03.2013 09:45, schrieb Andreas Bießmann:
Dear Manfred Huber,
On 03/28/2013 07:06 AM, Manfred Huber wrote:
On 2013-03-27 14:37, Andreas Bießmann wrote:
<snip>
On 03/25/2013 11:02 PM, Manfred Huber wrote:
<snip>
serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);
<snip>
I wonder which use-case requires UART flushing in u-boot context before initializing the UART for u-boot correctly. Can someone explain this to me? Shouldn't we always start here from the very beginning and setup UART as configured?
Beagleboard has several ways to boot (NAND, SD/MMC, UART, ...). For the boot mode with UART, Beagleboard configures the UART and ends with a non empty transmitter. In a booting sequence where UART is before NAND, SD/MMC or wherever SPL starts from, we have not a clean UART.
<snip>
It's not critical. So I guess it's not needed for this release.
Well, if there are boards in the field that will not boot with the next release I think it is critical. We do have some omap3 (omap35xx and am37xx) based boards here. I can recall a situation where some few boards did not boot from sd-card while serial debug cable was attached (AFAIR this was not the case when booting from NAND). The root cause was never investigated, so maybe we suffered exactly this bug.
Can you test this boars with my patch?
Best regards
Andreas Bießmann
On some OMAP3 devices when UART3 is configured for boot mode before SPL starts only THRE bit is set. We have to empty the transmitter before initialization starts. This patch avoids the use of CONFIG_SYS_NS16550_BROKEN_TEMT.
Signed-off-by: Manfred Huber man.huber@arcor.de --- README | 8 -------- drivers/serial/ns16550.c | 16 ++++++++++++++-- include/configs/igep00x0.h | 3 --- 3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/README b/README index a336476..e6b3a50 100644 --- a/README +++ b/README @@ -616,14 +616,6 @@ The following options need to be configured: boot loader that has already initialized the UART. Define this variable to flush the UART at init time.
- CONFIG_SYS_NS16550_BROKEN_TEMT - - 16550 UART set the Transmitter Empty (TEMT) Bit when all output - has finished and the transmitter is totally empty. U-Boot waits - for this bit to be set to initialize the serial console. On some - broken platforms this bit is not set in SPL making U-Boot to - hang while waiting for TEMT. Define this option to avoid it. -
- Console Interface: Depending on board, define exactly one serial port
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 87a0917..2922a2c 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,22 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX)) + /* On some OMAP3 devices when UART3 is configured for boot mode before + SPL starts only THRE bit is set. We have to empty the transmitter + before initialization starts. */ + if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE)) + == UART_LSR_THRE) { + serial_out(UART_LCR_DLAB, &com_port->lcr); + serial_out(baud_divisor & 0xff, &com_port->dll); + serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm); + serial_out(UART_LCRVAL, &com_port->lcr); + serial_out(0, &com_port->mdr1); + } +#endif + while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ; -#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier); #if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
diff --git a/include/configs/igep00x0.h b/include/configs/igep00x0.h index f8131b1..0617a58 100644 --- a/include/configs/igep00x0.h +++ b/include/configs/igep00x0.h @@ -67,9 +67,6 @@ #define CONFIG_SYS_NS16550_REG_SIZE (-4) #define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
-/* define to avoid U-Boot to hang while waiting for TEMT */ -#define CONFIG_SYS_NS16550_BROKEN_TEMT - /* select serial console configuration */ #define CONFIG_CONS_INDEX 3 #define CONFIG_SYS_NS16550_COM3 OMAP34XX_UART3
Hi Manfred,
On Fri, 29 Mar 2013 10:20:25 +0100, Manfred Huber man.huber@arcor.de wrote:
On some OMAP3 devices when UART3 is configured for boot mode before SPL starts only THRE bit is set. We have to empty the transmitter before initialization starts. This patch avoids the use of CONFIG_SYS_NS16550_BROKEN_TEMT.
Signed-off-by: Manfred Huber man.huber@arcor.de
Patch history is missing here.
Also, I would like the commit summary (first commit message line, also subject of the patch mail) to clearly state what the patch *does* as opposed to what *happens*, because it is unclear right now if you're describing the solution or the issue. Something like :
"omap3_beagle: flush UART3 xmit on enable if TEMT is broken"
README | 8 -------- drivers/serial/ns16550.c | 16 ++++++++++++++-- include/configs/igep00x0.h | 3 --- 3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/README b/README index a336476..e6b3a50 100644 --- a/README +++ b/README @@ -616,14 +616,6 @@ The following options need to be configured: boot loader that has already initialized the UART. Define this variable to flush the UART at init time.
CONFIG_SYS_NS16550_BROKEN_TEMT16550 UART set the Transmitter Empty (TEMT) Bit when all outputhas finished and the transmitter is totally empty. U-Boot waitsfor this bit to be set to initialize the serial console. On somebroken platforms this bit is not set in SPL making U-Boot tohang while waiting for TEMT. Define this option to avoid it.Console Interface: Depending on board, define exactly one serial port
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 87a0917..2922a2c 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,22 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
- /* On some OMAP3 devices when UART3 is configured for boot mode before
SPL starts only THRE bit is set. We have to empty the transmitterbefore initialization starts. */- if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))
== UART_LSR_THRE) {serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);- }
+#endif
- while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;
-#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier); #if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
diff --git a/include/configs/igep00x0.h b/include/configs/igep00x0.h index f8131b1..0617a58 100644 --- a/include/configs/igep00x0.h +++ b/include/configs/igep00x0.h @@ -67,9 +67,6 @@ #define CONFIG_SYS_NS16550_REG_SIZE (-4) #define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
-/* define to avoid U-Boot to hang while waiting for TEMT */ -#define CONFIG_SYS_NS16550_BROKEN_TEMT
/* select serial console configuration */ #define CONFIG_CONS_INDEX 3 #define CONFIG_SYS_NS16550_COM3 OMAP34XX_UART3 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Amicalement,
On Fri, Mar 29, 2013 at 10:43:34AM +0100, Albert ARIBAUD wrote:
Hi Manfred,
On Fri, 29 Mar 2013 10:20:25 +0100, Manfred Huber man.huber@arcor.de wrote:
On some OMAP3 devices when UART3 is configured for boot mode before SPL starts only THRE bit is set. We have to empty the transmitter before initialization starts. This patch avoids the use of CONFIG_SYS_NS16550_BROKEN_TEMT.
Signed-off-by: Manfred Huber man.huber@arcor.de
Patch history is missing here.
That's largely because each iteration has been fairly different (we started with adding CONFIG_SYS_NS16550_BROKEN_TEMPT to omap3_beagle).
Also, I would like the commit summary (first commit message line, also subject of the patch mail) to clearly state what the patch *does* as opposed to what *happens*, because it is unclear right now if you're describing the solution or the issue. Something like :
"omap3_beagle: flush UART3 xmit on enable if TEMT is broken"
Agreed.
README | 8 -------- drivers/serial/ns16550.c | 16 ++++++++++++++-- include/configs/igep00x0.h | 3 --- 3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/README b/README index a336476..e6b3a50 100644 --- a/README +++ b/README @@ -616,14 +616,6 @@ The following options need to be configured: boot loader that has already initialized the UART. Define this variable to flush the UART at init time.
CONFIG_SYS_NS16550_BROKEN_TEMT16550 UART set the Transmitter Empty (TEMT) Bit when all outputhas finished and the transmitter is totally empty. U-Boot waitsfor this bit to be set to initialize the serial console. On somebroken platforms this bit is not set in SPL making U-Boot tohang while waiting for TEMT. Define this option to avoid it.Console Interface: Depending on board, define exactly one serial port
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 87a0917..2922a2c 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,22 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
- /* On some OMAP3 devices when UART3 is configured for boot mode before
SPL starts only THRE bit is set. We have to empty the transmitterbefore initialization starts. */
/* * Multiline comments * must be like this. */
From: Manfred Huber man.huber@arcor.de
Flush UART3 xmit on enable if TEMT is broken
On some OMAP3 devices when UART3 is configured for boot mode before SPL starts only THRE bit is set. We have to empty the transmitter before initialization starts. This patch avoids the use of CONFIG_SYS_NS16550_BROKEN_TEMT.
Signed-off-by: Manfred Huber man.huber@arcor.de --- Changes since v3: - Changed commit summary and added version history.
Changes since v2: - Removing CONFIG_SYS_NS16550_BROKEN_TEMT in README and igep00x0.h.
Changes since v1: - Replaced CONFIG_SYS_NS16550_BROKEN_TEMT with initial configuration of UART.
README | 8 -------- drivers/serial/ns16550.c | 16 ++++++++++++++-- include/configs/igep00x0.h | 3 --- 3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/README b/README index a336476..e6b3a50 100644 --- a/README +++ b/README @@ -616,14 +616,6 @@ The following options need to be configured: boot loader that has already initialized the UART. Define this variable to flush the UART at init time.
- CONFIG_SYS_NS16550_BROKEN_TEMT - - 16550 UART set the Transmitter Empty (TEMT) Bit when all output - has finished and the transmitter is totally empty. U-Boot waits - for this bit to be set to initialize the serial console. On some - broken platforms this bit is not set in SPL making U-Boot to - hang while waiting for TEMT. Define this option to avoid it. -
- Console Interface: Depending on board, define exactly one serial port
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 87a0917..2922a2c 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,22 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX)) + /* On some OMAP3 devices when UART3 is configured for boot mode before + SPL starts only THRE bit is set. We have to empty the transmitter + before initialization starts. */ + if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE)) + == UART_LSR_THRE) { + serial_out(UART_LCR_DLAB, &com_port->lcr); + serial_out(baud_divisor & 0xff, &com_port->dll); + serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm); + serial_out(UART_LCRVAL, &com_port->lcr); + serial_out(0, &com_port->mdr1); + } +#endif + while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ; -#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier); #if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
diff --git a/include/configs/igep00x0.h b/include/configs/igep00x0.h index f8131b1..0617a58 100644 --- a/include/configs/igep00x0.h +++ b/include/configs/igep00x0.h @@ -67,9 +67,6 @@ #define CONFIG_SYS_NS16550_REG_SIZE (-4) #define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
-/* define to avoid U-Boot to hang while waiting for TEMT */ -#define CONFIG_SYS_NS16550_BROKEN_TEMT - /* select serial console configuration */ #define CONFIG_CONS_INDEX 3 #define CONFIG_SYS_NS16550_COM3 OMAP34XX_UART3
From: Manfred Huber man.huber@arcor.de
Flush UART3 xmit on enable if TEMT is broken
On some OMAP3 devices when UART3 is configured for boot mode before SPL starts only THRE bit is set. We have to empty the transmitter before initialization starts. This patch avoids the use of CONFIG_SYS_NS16550_BROKEN_TEMT.
Signed-off-by: Manfred Huber man.huber@arcor.de --- Changes since v4: - Changed multiline comment.
Changes since v3: - Changed commit summary and added version history.
Changes since v2: - Removing CONFIG_SYS_NS16550_BROKEN_TEMT in README and igep00x0.h.
Changes since v1: - Replaced CONFIG_SYS_NS16550_BROKEN_TEMT with initial configuration of UART.
README | 8 -------- drivers/serial/ns16550.c | 18 ++++++++++++++++-- include/configs/igep00x0.h | 3 --- 3 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/README b/README index a336476..e6b3a50 100644 --- a/README +++ b/README @@ -616,14 +616,6 @@ The following options need to be configured: boot loader that has already initialized the UART. Define this variable to flush the UART at init time.
- CONFIG_SYS_NS16550_BROKEN_TEMT - - 16550 UART set the Transmitter Empty (TEMT) Bit when all output - has finished and the transmitter is totally empty. U-Boot waits - for this bit to be set to initialize the serial console. On some - broken platforms this bit is not set in SPL making U-Boot to - hang while waiting for TEMT. Define this option to avoid it. -
- Console Interface: Depending on board, define exactly one serial port diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 87a0917..a9352fb 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,24 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX)) + /* + * On some OMAP3 devices when UART3 is configured for boot mode before + * SPL starts only THRE bit is set. We have to empty the transmitter + * before initialization starts. + */ + if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE)) + == UART_LSR_THRE) { + serial_out(UART_LCR_DLAB, &com_port->lcr); + serial_out(baud_divisor & 0xff, &com_port->dll); + serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm); + serial_out(UART_LCRVAL, &com_port->lcr); + serial_out(0, &com_port->mdr1); + } +#endif + while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ; -#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier); #if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \ diff --git a/include/configs/igep00x0.h b/include/configs/igep00x0.h index f8131b1..0617a58 100644 --- a/include/configs/igep00x0.h +++ b/include/configs/igep00x0.h @@ -67,9 +67,6 @@ #define CONFIG_SYS_NS16550_REG_SIZE (-4) #define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
-/* define to avoid U-Boot to hang while waiting for TEMT */ -#define CONFIG_SYS_NS16550_BROKEN_TEMT - /* select serial console configuration */ #define CONFIG_CONS_INDEX 3 #define CONFIG_SYS_NS16550_COM3 OMAP34XX_UART3
On Fri, Mar 29, 2013 at 1:52 PM, Manfred Huber man.huber@arcor.de wrote:
From: Manfred Huber man.huber@arcor.de
Flush UART3 xmit on enable if TEMT is broken
On some OMAP3 devices when UART3 is configured for boot mode before SPL starts only THRE bit is set. We have to empty the transmitter before initialization starts. This patch avoids the use of CONFIG_SYS_NS16550_BROKEN_TEMT.
Signed-off-by: Manfred Huber man.huber@arcor.de
Changes since v4: - Changed multiline comment.
Changes since v3: - Changed commit summary and added version history.
Changes since v2: - Removing CONFIG_SYS_NS16550_BROKEN_TEMT in README and igep00x0.h.
Changes since v1: - Replaced CONFIG_SYS_NS16550_BROKEN_TEMT with initial configuration of UART.
README | 8 -------- drivers/serial/ns16550.c | 18 ++++++++++++++++-- include/configs/igep00x0.h | 3 --- 3 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/README b/README index a336476..e6b3a50 100644 --- a/README +++ b/README @@ -616,14 +616,6 @@ The following options need to be configured: boot loader that has already initialized the UART. Define this variable to flush the UART at init time.
CONFIG_SYS_NS16550_BROKEN_TEMT16550 UART set the Transmitter Empty (TEMT) Bit when all outputhas finished and the transmitter is totally empty. U-Boot waitsfor this bit to be set to initialize the serial console. On somebroken platforms this bit is not set in SPL making U-Boot tohang while waiting for TEMT. Define this option to avoid it.Console Interface: Depending on board, define exactly one serial port
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 87a0917..a9352fb 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,10 +36,24 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
/** On some OMAP3 devices when UART3 is configured for boot mode before* SPL starts only THRE bit is set. We have to empty the transmitter* before initialization starts.*/if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))== UART_LSR_THRE) {serial_out(UART_LCR_DLAB, &com_port->lcr);serial_out(baud_divisor & 0xff, &com_port->dll);serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);serial_out(UART_LCRVAL, &com_port->lcr);serial_out(0, &com_port->mdr1);}+#endif
while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;-#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);#if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \ diff --git a/include/configs/igep00x0.h b/include/configs/igep00x0.h index f8131b1..0617a58 100644 --- a/include/configs/igep00x0.h +++ b/include/configs/igep00x0.h @@ -67,9 +67,6 @@ #define CONFIG_SYS_NS16550_REG_SIZE (-4) #define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
-/* define to avoid U-Boot to hang while waiting for TEMT */ -#define CONFIG_SYS_NS16550_BROKEN_TEMT
/* select serial console configuration */ #define CONFIG_CONS_INDEX 3 #define CONFIG_SYS_NS16550_COM3 OMAP34XX_UART3
Hi Manfred,
I tested your patch on my IGEPv2 and is booting correctly, so:
Tested-by: Javier Martinez Canillas javier@dowhile0.org
Thanks a lot for working on this!
Best regards, Javier
Dear Manfred Huber,
On 03/29/2013 01:52 PM, Manfred Huber wrote:
From: Manfred Huber man.huber@arcor.de
Flush UART3 xmit on enable if TEMT is broken
On some OMAP3 devices when UART3 is configured for boot mode before SPL starts only THRE bit is set. We have to empty the transmitter before initialization starts. This patch avoids the use of CONFIG_SYS_NS16550_BROKEN_TEMT.
Signed-off-by: Manfred Huber man.huber@arcor.de
Tested-by: Andreas Bießmann andreas.devel@googlemail.com
on tricorder board.
On Fri, Mar 29, 2013 at 02:52:36AM -0000, man.huber@arcor.de wrote:
From: Manfred Huber man.huber@arcor.de
Flush UART3 xmit on enable if TEMT is broken
On some OMAP3 devices when UART3 is configured for boot mode before SPL starts only THRE bit is set. We have to empty the transmitter before initialization starts. This patch avoids the use of CONFIG_SYS_NS16550_BROKEN_TEMT.
Signed-off-by: Manfred Huber man.huber@arcor.de Tested-by: Javier Martinez Canillas javier@dowhile0.org Tested-by: Andreas Bie??mann andreas.devel@googlemail.com
Applied to u-boot-ti/master, thanks!
The processor is hopefully running with M(ega)Hz and not with m(illi)Hz.
Signed-off-by: Manfred Huber man.huber@arcor.de --- arch/arm/cpu/armv7/omap3/sys_info.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap3/sys_info.c b/arch/arm/cpu/armv7/omap3/sys_info.c index 3c80113..08a63d2 100644 --- a/arch/arm/cpu/armv7/omap3/sys_info.c +++ b/arch/arm/cpu/armv7/omap3/sys_info.c @@ -299,9 +299,9 @@ int print_cpuinfo (void) } if ((get_cpu_rev() >= CPU_3XX_ES31) && (get_sku_id() == SKUID_CLK_720MHZ)) - max_clk = "720 mHz"; + max_clk = "720 MHz"; else - max_clk = "600 mHz"; + max_clk = "600 MHz";
break; case CPU_AM35XX:
participants (8)
-
Albert ARIBAUD -
Andreas Bießmann -
Javier Martinez Canillas -
Javier Martinez Canillas -
man.huber＠arcor.de -
Manfred Huber
-
Scott Wood -
Tom Rini