* [U-Boot] Beagleboard: SPL hangs on serial init
2013-03-20 0:05 ` Javier Martinez Canillas
@ 2013-03-20 1:27 ` Tom Rini
2013-03-20 23:09 ` Manfred Huber
2013-03-21 19:03 ` [U-Boot] [PATCH] omap3_beagle: Enable CONFIG_SYS_NS16550_BROKEN_TEMT Manfred Huber
` (5 subsequent siblings)
6 siblings, 1 reply; 37+ messages in thread
From: Tom Rini @ 2013-03-20 1:27 UTC (permalink / raw)
To: u-boot
-----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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRSRB2AAoJENk4IS6UOR1WP9YP/2McsjKVfEFq1Xooosts6v7Y
iyLfGg/yYxszfFABTtCwMUn/GjCw5kLjLQB3zISOdR15GTneZ3NCiXcLY6Z/AgId
2gCDV7MOg1WSXyItIn6M+ra/Dt+HMT5VrwN8Z10GPpzQj58HCiJV/12115mMQKLN
fWIRCMALetJkIddogNrcZluIJDwR9ye3QTy8p6jE8uy5QBvuu3QyDr+CrJKq3gEk
BKuC0+9xLjHSMG8KcGG4QEObmzOFyw227lR6Cmfdw8cK52XE2v+11jZk/3vFHfFf
xlL117wRzd8H1r9LGY8SNnzIuz+G6hDwa1n7kpmYLW6S8I5F5b4gOsS7pmA908CQ
vognhGq4GSg6SZEqFk6s4OcqfdEG4+dCcXxFLy0iD0GeAkvet4G97SqQgvVAbJ8m
By1gsrwsK05j5rtPeA3ELSbM4AOu8jVJZI210eZYyRHW+b4qyA7Sl+W4CqDHiwLk
SGt4UT0S6/6fnV9M4oKq3xhh3R59ZYhkeUiOFoL4cUwOlp9PXbaWgkiNGyYZBCoQ
5XmfNGGyiT5bcueeyR1Swtw8UNX+FA4sncnJrtELFqRpb9SNFJq3GdTx9gNAJz6a
Q+RS41IApAUz8vPq0Kg8JWF0h7C1IqRWMzLi3mIXpxZCbYy+0ElAgLMp3XeKbYhx
SF/nnpv1PKMGWQVobw0s
=9RxY
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] Beagleboard: SPL hangs on serial init
2013-03-20 1:27 ` Tom Rini
@ 2013-03-20 23:09 ` Manfred Huber
2013-03-21 21:08 ` Javier Martinez Canillas
0 siblings, 1 reply; 37+ messages in thread
From: Manfred Huber @ 2013-03-20 23:09 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] Beagleboard: SPL hangs on serial init
2013-03-20 23:09 ` Manfred Huber
@ 2013-03-21 21:08 ` Javier Martinez Canillas
2013-03-23 10:11 ` Manfred Huber
0 siblings, 1 reply; 37+ messages in thread
From: Javier Martinez Canillas @ 2013-03-21 21:08 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] Beagleboard: SPL hangs on serial init
2013-03-21 21:08 ` Javier Martinez Canillas
@ 2013-03-23 10:11 ` Manfred Huber
0 siblings, 0 replies; 37+ messages in thread
From: Manfred Huber @ 2013-03-23 10:11 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH] omap3_beagle: Enable CONFIG_SYS_NS16550_BROKEN_TEMT
2013-03-20 0:05 ` Javier Martinez Canillas
2013-03-20 1:27 ` Tom Rini
@ 2013-03-21 19:03 ` Manfred Huber
2013-03-21 21:28 ` Javier Martinez Canillas
2013-03-21 22:21 ` Tom Rini
2013-03-25 22:02 ` [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty Manfred Huber
` (4 subsequent siblings)
6 siblings, 2 replies; 37+ messages in thread
From: Manfred Huber @ 2013-03-21 19:03 UTC (permalink / raw)
To: u-boot
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
*/
^ permalink raw reply related [flat|nested] 37+ messages in thread* [U-Boot] [PATCH] omap3_beagle: Enable CONFIG_SYS_NS16550_BROKEN_TEMT
2013-03-21 19:03 ` [U-Boot] [PATCH] omap3_beagle: Enable CONFIG_SYS_NS16550_BROKEN_TEMT Manfred Huber
@ 2013-03-21 21:28 ` Javier Martinez Canillas
2013-03-21 22:21 ` Tom Rini
1 sibling, 0 replies; 37+ messages in thread
From: Javier Martinez Canillas @ 2013-03-21 21:28 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] [PATCH] omap3_beagle: Enable CONFIG_SYS_NS16550_BROKEN_TEMT
2013-03-21 19:03 ` [U-Boot] [PATCH] omap3_beagle: Enable CONFIG_SYS_NS16550_BROKEN_TEMT Manfred Huber
2013-03-21 21:28 ` Javier Martinez Canillas
@ 2013-03-21 22:21 ` Tom Rini
2013-03-21 22:28 ` Scott Wood
1 sibling, 1 reply; 37+ messages in thread
From: Tom Rini @ 2013-03-21 22:21 UTC (permalink / raw)
To: u-boot
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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130321/91cb1a48/attachment.pgp>
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] [PATCH] omap3_beagle: Enable CONFIG_SYS_NS16550_BROKEN_TEMT
2013-03-21 22:21 ` Tom Rini
@ 2013-03-21 22:28 ` Scott Wood
0 siblings, 0 replies; 37+ messages in thread
From: Scott Wood @ 2013-03-21 22:28 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-20 0:05 ` Javier Martinez Canillas
2013-03-20 1:27 ` Tom Rini
2013-03-21 19:03 ` [U-Boot] [PATCH] omap3_beagle: Enable CONFIG_SYS_NS16550_BROKEN_TEMT Manfred Huber
@ 2013-03-25 22:02 ` Manfred Huber
2013-03-27 4:50 ` Manfred Huber
2013-03-27 13:37 ` Andreas Bießmann
2013-03-29 9:20 ` [U-Boot] [PATCH 1/1 v3] " Manfred Huber
` (3 subsequent siblings)
6 siblings, 2 replies; 37+ messages in thread
From: Manfred Huber @ 2013-03-25 22:02 UTC (permalink / raw)
To: u-boot
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)) || \
^ permalink raw reply related [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-25 22:02 ` [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty Manfred Huber
@ 2013-03-27 4:50 ` Manfred Huber
2013-03-27 9:29 ` Javier Martinez Canillas
2013-03-28 15:21 ` Tom Rini
2013-03-27 13:37 ` Andreas Bießmann
1 sibling, 2 replies; 37+ messages in thread
From: Manfred Huber @ 2013-03-27 4:50 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-27 4:50 ` Manfred Huber
@ 2013-03-27 9:29 ` Javier Martinez Canillas
2013-03-27 13:57 ` Tom Rini
` (2 more replies)
2013-03-28 15:21 ` Tom Rini
1 sibling, 3 replies; 37+ messages in thread
From: Javier Martinez Canillas @ 2013-03-27 9:29 UTC (permalink / raw)
To: 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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-27 9:29 ` Javier Martinez Canillas
@ 2013-03-27 13:57 ` Tom Rini
2013-03-28 5:55 ` Manfred Huber
2013-03-29 8:19 ` Manfred Huber
2 siblings, 0 replies; 37+ messages in thread
From: Tom Rini @ 2013-03-27 13:57 UTC (permalink / raw)
To: 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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130327/28e32fd7/attachment.pgp>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-27 9:29 ` Javier Martinez Canillas
2013-03-27 13:57 ` Tom Rini
@ 2013-03-28 5:55 ` Manfred Huber
2013-03-29 8:19 ` Manfred Huber
2 siblings, 0 replies; 37+ messages in thread
From: Manfred Huber @ 2013-03-28 5:55 UTC (permalink / raw)
To: 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:
>> 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 at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>>
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-27 9:29 ` Javier Martinez Canillas
2013-03-27 13:57 ` Tom Rini
2013-03-28 5:55 ` Manfred Huber
@ 2013-03-29 8:19 ` Manfred Huber
2 siblings, 0 replies; 37+ messages in thread
From: Manfred Huber @ 2013-03-29 8:19 UTC (permalink / raw)
To: 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>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-27 4:50 ` Manfred Huber
2013-03-27 9:29 ` Javier Martinez Canillas
@ 2013-03-28 15:21 ` Tom Rini
1 sibling, 0 replies; 37+ messages in thread
From: Tom Rini @ 2013-03-28 15:21 UTC (permalink / raw)
To: u-boot
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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130328/ae913832/attachment-0001.pgp>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-25 22:02 ` [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty Manfred Huber
2013-03-27 4:50 ` Manfred Huber
@ 2013-03-27 13:37 ` Andreas Bießmann
2013-03-27 17:22 ` Javier Martinez Canillas
2013-03-28 6:06 ` Manfred Huber
1 sibling, 2 replies; 37+ messages in thread
From: Andreas Bießmann @ 2013-03-27 13:37 UTC (permalink / raw)
To: u-boot
Dear Manfred Huber,
---8<---
abiessmann at 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 at 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
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-27 13:37 ` Andreas Bießmann
@ 2013-03-27 17:22 ` Javier Martinez Canillas
2013-03-28 6:06 ` Manfred Huber
1 sibling, 0 replies; 37+ messages in thread
From: Javier Martinez Canillas @ 2013-03-27 17:22 UTC (permalink / raw)
To: u-boot
On Wed, Mar 27, 2013 at 2:37 PM, Andreas Bie?mann
<andreas.devel@googlemail.com> wrote:
> Dear Manfred Huber,
>
> ---8<---
> abiessmann at 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 at 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 at 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
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-27 13:37 ` Andreas Bießmann
2013-03-27 17:22 ` Javier Martinez Canillas
@ 2013-03-28 6:06 ` Manfred Huber
2013-03-28 8:45 ` Andreas Bießmann
1 sibling, 1 reply; 37+ messages in thread
From: Manfred Huber @ 2013-03-28 6:06 UTC (permalink / raw)
To: u-boot
On 2013-03-27 14:37, Andreas Bie?mann wrote:
> Dear Manfred Huber,
>
> ---8<---
> abiessmann at 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 at 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 at 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
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-28 6:06 ` Manfred Huber
@ 2013-03-28 8:45 ` Andreas Bießmann
2013-03-28 9:11 ` Javier Martinez Canillas
2013-03-29 8:33 ` Manfred Huber
0 siblings, 2 replies; 37+ messages in thread
From: Andreas Bießmann @ 2013-03-28 8:45 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-28 8:45 ` Andreas Bießmann
@ 2013-03-28 9:11 ` Javier Martinez Canillas
2013-03-28 9:50 ` Andreas Bießmann
2013-03-29 8:33 ` Manfred Huber
1 sibling, 1 reply; 37+ messages in thread
From: Javier Martinez Canillas @ 2013-03-28 9:11 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-28 9:11 ` Javier Martinez Canillas
@ 2013-03-28 9:50 ` Andreas Bießmann
2013-03-28 15:21 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Andreas Bießmann @ 2013-03-28 9:50 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-28 9:50 ` Andreas Bießmann
@ 2013-03-28 15:21 ` Tom Rini
0 siblings, 0 replies; 37+ messages in thread
From: Tom Rini @ 2013-03-28 15:21 UTC (permalink / raw)
To: u-boot
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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130328/598a67b1/attachment.pgp>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-28 8:45 ` Andreas Bießmann
2013-03-28 9:11 ` Javier Martinez Canillas
@ 2013-03-29 8:33 ` Manfred Huber
1 sibling, 0 replies; 37+ messages in thread
From: Manfred Huber @ 2013-03-29 8:33 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH 1/1 v3] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-20 0:05 ` Javier Martinez Canillas
` (2 preceding siblings ...)
2013-03-25 22:02 ` [U-Boot] [PATCH 1/1 v2] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty Manfred Huber
@ 2013-03-29 9:20 ` Manfred Huber
2013-03-29 9:43 ` Albert ARIBAUD
2013-03-29 12:42 ` [U-Boot] [PATCH 1/1 v4] omap3_beagle: Flush UART3 xmit on enable if TEMT is broken Manfred Huber
` (2 subsequent siblings)
6 siblings, 1 reply; 37+ messages in thread
From: Manfred Huber @ 2013-03-29 9:20 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v3] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-29 9:20 ` [U-Boot] [PATCH 1/1 v3] " Manfred Huber
@ 2013-03-29 9:43 ` Albert ARIBAUD
2013-03-29 12:34 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Albert ARIBAUD @ 2013-03-29 9:43 UTC (permalink / raw)
To: u-boot
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_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
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v3] omap3_beagle: Enabling UART3 first allows the Transmitter to be empty
2013-03-29 9:43 ` Albert ARIBAUD
@ 2013-03-29 12:34 ` Tom Rini
0 siblings, 0 replies; 37+ messages in thread
From: Tom Rini @ 2013-03-29 12:34 UTC (permalink / raw)
To: u-boot
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_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. */
/*
* Multiline comments
* must be like this.
*/
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130329/31892ec9/attachment.pgp>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH 1/1 v4] omap3_beagle: Flush UART3 xmit on enable if TEMT is broken
2013-03-20 0:05 ` Javier Martinez Canillas
` (3 preceding siblings ...)
2013-03-29 9:20 ` [U-Boot] [PATCH 1/1 v3] " Manfred Huber
@ 2013-03-29 12:42 ` Manfred Huber
2013-03-29 12:52 ` [U-Boot] [PATCH 1/1 v5] " Manfred Huber
2013-04-10 22:12 ` [U-Boot] [PATCH v1 1/1] omap3: Display MHz instead of mHz on the console Manfred Huber
6 siblings, 0 replies; 37+ messages in thread
From: Manfred Huber @ 2013-03-29 12:42 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v5] omap3_beagle: Flush UART3 xmit on enable if TEMT is broken
2013-03-20 0:05 ` Javier Martinez Canillas
` (4 preceding siblings ...)
2013-03-29 12:42 ` [U-Boot] [PATCH 1/1 v4] omap3_beagle: Flush UART3 xmit on enable if TEMT is broken Manfred Huber
@ 2013-03-29 12:52 ` Manfred Huber
2013-04-02 7:46 ` Javier Martinez Canillas
` (2 more replies)
2013-04-10 22:12 ` [U-Boot] [PATCH v1 1/1] omap3: Display MHz instead of mHz on the console Manfred Huber
6 siblings, 3 replies; 37+ messages in thread
From: Manfred Huber @ 2013-03-29 12:52 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v5] omap3_beagle: Flush UART3 xmit on enable if TEMT is broken
2013-03-29 12:52 ` [U-Boot] [PATCH 1/1 v5] " Manfred Huber
@ 2013-04-02 7:46 ` Javier Martinez Canillas
2013-04-02 8:59 ` Andreas Bießmann
2013-04-08 16:56 ` [U-Boot] [U-Boot, 1/1, " Tom Rini
2 siblings, 0 replies; 37+ messages in thread
From: Javier Martinez Canillas @ 2013-04-02 7:46 UTC (permalink / raw)
To: u-boot
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_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
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
^ permalink raw reply [flat|nested] 37+ messages in thread* [U-Boot] [PATCH 1/1 v5] omap3_beagle: Flush UART3 xmit on enable if TEMT is broken
2013-03-29 12:52 ` [U-Boot] [PATCH 1/1 v5] " Manfred Huber
2013-04-02 7:46 ` Javier Martinez Canillas
@ 2013-04-02 8:59 ` Andreas Bießmann
2013-04-08 16:56 ` [U-Boot] [U-Boot, 1/1, " Tom Rini
2 siblings, 0 replies; 37+ messages in thread
From: Andreas Bießmann @ 2013-04-02 8:59 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [U-Boot, 1/1, v5] omap3_beagle: Flush UART3 xmit on enable if TEMT is broken
2013-03-29 12:52 ` [U-Boot] [PATCH 1/1 v5] " Manfred Huber
2013-04-02 7:46 ` Javier Martinez Canillas
2013-04-02 8:59 ` Andreas Bießmann
@ 2013-04-08 16:56 ` Tom Rini
2 siblings, 0 replies; 37+ messages in thread
From: Tom Rini @ 2013-04-08 16:56 UTC (permalink / raw)
To: u-boot
On Fri, Mar 29, 2013 at 02:52:36AM -0000, man.huber at 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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130408/92c35616/attachment.pgp>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [U-Boot] [PATCH v1 1/1] omap3: Display MHz instead of mHz on the console
2013-03-20 0:05 ` Javier Martinez Canillas
` (5 preceding siblings ...)
2013-03-29 12:52 ` [U-Boot] [PATCH 1/1 v5] " Manfred Huber
@ 2013-04-10 22:12 ` Manfred Huber
6 siblings, 0 replies; 37+ messages in thread
From: Manfred Huber @ 2013-04-10 22:12 UTC (permalink / raw)
To: u-boot
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:
^ permalink raw reply related [flat|nested] 37+ messages in thread