[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[Heiko Schocher]

commit: 65f83802b7a5b "serial: 16550: Add getfcr accessor"
breaks u-boot commandline working with long commands
sending to the board.

Since the above patch, you have to setup the fcr register.

For board/archs which enable OF_PLATDATA, the new field
fcr in struct ns16550_platdata is not filled with a
default value ...

This leads in not setting up the uarts fifo, which ends
in problems, when you send long commands to u-boots
commandline.

Detected this issue with automated tbot tests on am335x
based shc board.

The error does not popup, if you type commands. You need
to copy&paste a long command to u-boots commandshell
(or send a long command with tbot)

Possible boards/plattforms with problems:
./arch/arm/cpu/arm926ejs/lpc32xx/devices.c
./arch/arm/mach-tegra/board.c
./board/isee/igep00x0/igep00x0.c
./board/overo/overo.c
./board/quipos/cairo/cairo.c
./board/logicpd/omap3som/omap3logic.c
./board/logicpd/zoom1/zoom1.c
./board/timll/devkit8000/devkit8000.c
./board/lg/sniper/sniper.c
./board/ti/beagle/beagle.c
./drivers/serial/serial_rockchip.c

This patch fixes only:
./arch/arm/mach-omap2/am33xx/board.c

Signed-off-by: Heiko Schocher <hs@denx.de>
---

 arch/arm/mach-omap2/am33xx/board.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c
index 581c0ab..a0ce62c 100644
--- a/arch/arm/mach-omap2/am33xx/board.c
+++ b/arch/arm/mach-omap2/am33xx/board.c
@@ -39,15 +39,26 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 #if !CONFIG_IS_ENABLED(OF_CONTROL)
+/* Clear & enable FIFOs */
+#define UART_FCRVAL (UART_FCR_FIFO_EN |	\
+		     UART_FCR_RXSR |	\
+		     UART_FCR_TXSR)
+
 static const struct ns16550_platdata am33xx_serial[] = {
-	{ .base = CONFIG_SYS_NS16550_COM1, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
+	{ .base = CONFIG_SYS_NS16550_COM1, .reg_shift = 2,
+	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
 # ifdef CONFIG_SYS_NS16550_COM2
-	{ .base = CONFIG_SYS_NS16550_COM2, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
+	{ .base = CONFIG_SYS_NS16550_COM2, .reg_shift = 2,
+	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
 #  ifdef CONFIG_SYS_NS16550_COM3
-	{ .base = CONFIG_SYS_NS16550_COM3, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
-	{ .base = CONFIG_SYS_NS16550_COM4, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
-	{ .base = CONFIG_SYS_NS16550_COM5, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
-	{ .base = CONFIG_SYS_NS16550_COM6, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
+	{ .base = CONFIG_SYS_NS16550_COM3, .reg_shift = 2,
+	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
+	{ .base = CONFIG_SYS_NS16550_COM4, .reg_shift = 2,
+	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
+	{ .base = CONFIG_SYS_NS16550_COM5, .reg_shift = 2,
+	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
+	{ .base = CONFIG_SYS_NS16550_COM6, .reg_shift = 2,
+	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
 #  endif
 # endif
 };
-- 
2.7.4

[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[Ladislav Michl]

Hi Heiko,

On Tue, Jan 10, 2017 at 08:08:51AM +0100, Heiko Schocher wrote:
> commit: 65f83802b7a5b "serial: 16550: Add getfcr accessor"
> breaks u-boot commandline working with long commands
> sending to the board.
> 
> Since the above patch, you have to setup the fcr register.
> 
> For board/archs which enable OF_PLATDATA, the new field
> fcr in struct ns16550_platdata is not filled with a
> default value ...
> 
> This leads in not setting up the uarts fifo, which ends
> in problems, when you send long commands to u-boots
> commandline.
> 
> Detected this issue with automated tbot tests on am335x
> based shc board.
> 
> The error does not popup, if you type commands. You need
> to copy&paste a long command to u-boots commandshell
> (or send a long command with tbot)
> 
> Possible boards/plattforms with problems:
> ./arch/arm/cpu/arm926ejs/lpc32xx/devices.c
> ./arch/arm/mach-tegra/board.c
> ./board/isee/igep00x0/igep00x0.c
> ./board/overo/overo.c
> ./board/quipos/cairo/cairo.c
> ./board/logicpd/omap3som/omap3logic.c
> ./board/logicpd/zoom1/zoom1.c
> ./board/timll/devkit8000/devkit8000.c
> ./board/lg/sniper/sniper.c
> ./board/ti/beagle/beagle.c
> ./drivers/serial/serial_rockchip.c

Quick test shows igep00x0 also suffers from this issue.
Are you going to collect fixes and apply them as one patch or
shall I sent a separate patch?

Anyway, here's fix for igep00x0:

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>

diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c
index ae7959b1eb..8bea199a44 100644
--- a/board/isee/igep00x0/igep00x0.c
+++ b/board/isee/igep00x0/igep00x0.c
@@ -32,7 +32,8 @@ DECLARE_GLOBAL_DATA_PTR;
 static const struct ns16550_platdata igep_serial = {
 	.base = OMAP34XX_UART3,
 	.reg_shift = 2,
-	.clock = V_NS16550_CLK
+	.clock = V_NS16550_CLK,
+	.fcr = UART_FCR_FIFO_EN | UART_FCR_RXSR | UART_FCR_TXSR,
 };
 
 U_BOOT_DEVICE(igep_uart) = {

> This patch fixes only:
> ./arch/arm/mach-omap2/am33xx/board.c
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
> 
>  arch/arm/mach-omap2/am33xx/board.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c
> index 581c0ab..a0ce62c 100644
> --- a/arch/arm/mach-omap2/am33xx/board.c
> +++ b/arch/arm/mach-omap2/am33xx/board.c
> @@ -39,15 +39,26 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  #if !CONFIG_IS_ENABLED(OF_CONTROL)
> +/* Clear & enable FIFOs */
> +#define UART_FCRVAL (UART_FCR_FIFO_EN |	\
> +		     UART_FCR_RXSR |	\
> +		     UART_FCR_TXSR)
> +
>  static const struct ns16550_platdata am33xx_serial[] = {
> -	{ .base = CONFIG_SYS_NS16550_COM1, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
> +	{ .base = CONFIG_SYS_NS16550_COM1, .reg_shift = 2,
> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
>  # ifdef CONFIG_SYS_NS16550_COM2
> -	{ .base = CONFIG_SYS_NS16550_COM2, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
> +	{ .base = CONFIG_SYS_NS16550_COM2, .reg_shift = 2,
> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
>  #  ifdef CONFIG_SYS_NS16550_COM3
> -	{ .base = CONFIG_SYS_NS16550_COM3, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
> -	{ .base = CONFIG_SYS_NS16550_COM4, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
> -	{ .base = CONFIG_SYS_NS16550_COM5, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
> -	{ .base = CONFIG_SYS_NS16550_COM6, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
> +	{ .base = CONFIG_SYS_NS16550_COM3, .reg_shift = 2,
> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
> +	{ .base = CONFIG_SYS_NS16550_COM4, .reg_shift = 2,
> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
> +	{ .base = CONFIG_SYS_NS16550_COM5, .reg_shift = 2,
> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
> +	{ .base = CONFIG_SYS_NS16550_COM6, .reg_shift = 2,
> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
>  #  endif
>  # endif
>  };
> -- 
> 2.7.4

[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[Heiko Schocher]

Hello Ladislav,

Am 10.01.2017 um 12:50 schrieb Ladislav Michl:
> Hi Heiko,
>
> On Tue, Jan 10, 2017 at 08:08:51AM +0100, Heiko Schocher wrote:
>> commit: 65f83802b7a5b "serial: 16550: Add getfcr accessor"
>> breaks u-boot commandline working with long commands
>> sending to the board.
>>
>> Since the above patch, you have to setup the fcr register.
>>
>> For board/archs which enable OF_PLATDATA, the new field
>> fcr in struct ns16550_platdata is not filled with a
>> default value ...
>>
>> This leads in not setting up the uarts fifo, which ends
>> in problems, when you send long commands to u-boots
>> commandline.
>>
>> Detected this issue with automated tbot tests on am335x
>> based shc board.
>>
>> The error does not popup, if you type commands. You need
>> to copy&paste a long command to u-boots commandshell
>> (or send a long command with tbot)
>>
>> Possible boards/plattforms with problems:
>> ./arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>> ./arch/arm/mach-tegra/board.c
>> ./board/isee/igep00x0/igep00x0.c
>> ./board/overo/overo.c
>> ./board/quipos/cairo/cairo.c
>> ./board/logicpd/omap3som/omap3logic.c
>> ./board/logicpd/zoom1/zoom1.c
>> ./board/timll/devkit8000/devkit8000.c
>> ./board/lg/sniper/sniper.c
>> ./board/ti/beagle/beagle.c
>> ./drivers/serial/serial_rockchip.c
>
> Quick test shows igep00x0 also suffers from this issue.

Thanks for testing!

> Are you going to collect fixes and apply them as one patch or
> shall I sent a separate patch?

I don;t know ... if my approach fixing this issue is ok,
I can collect it ... Tom decides ;-)

bye,
Heiko
> Anyway, here's fix for igep00x0:
>
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
>
> diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c
> index ae7959b1eb..8bea199a44 100644
> --- a/board/isee/igep00x0/igep00x0.c
> +++ b/board/isee/igep00x0/igep00x0.c
> @@ -32,7 +32,8 @@ DECLARE_GLOBAL_DATA_PTR;
>   static const struct ns16550_platdata igep_serial = {
>   	.base = OMAP34XX_UART3,
>   	.reg_shift = 2,
> -	.clock = V_NS16550_CLK
> +	.clock = V_NS16550_CLK,
> +	.fcr = UART_FCR_FIFO_EN | UART_FCR_RXSR | UART_FCR_TXSR,
>   };
>
>   U_BOOT_DEVICE(igep_uart) = {
>
>> This patch fixes only:
>> ./arch/arm/mach-omap2/am33xx/board.c
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> ---
>>
>>   arch/arm/mach-omap2/am33xx/board.c | 23 +++++++++++++++++------
>>   1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c
>> index 581c0ab..a0ce62c 100644
>> --- a/arch/arm/mach-omap2/am33xx/board.c
>> +++ b/arch/arm/mach-omap2/am33xx/board.c
>> @@ -39,15 +39,26 @@
>>   DECLARE_GLOBAL_DATA_PTR;
>>
>>   #if !CONFIG_IS_ENABLED(OF_CONTROL)
>> +/* Clear & enable FIFOs */
>> +#define UART_FCRVAL (UART_FCR_FIFO_EN |	\
>> +		     UART_FCR_RXSR |	\
>> +		     UART_FCR_TXSR)
>> +
>>   static const struct ns16550_platdata am33xx_serial[] = {
>> -	{ .base = CONFIG_SYS_NS16550_COM1, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
>> +	{ .base = CONFIG_SYS_NS16550_COM1, .reg_shift = 2,
>> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
>>   # ifdef CONFIG_SYS_NS16550_COM2
>> -	{ .base = CONFIG_SYS_NS16550_COM2, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
>> +	{ .base = CONFIG_SYS_NS16550_COM2, .reg_shift = 2,
>> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
>>   #  ifdef CONFIG_SYS_NS16550_COM3
>> -	{ .base = CONFIG_SYS_NS16550_COM3, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
>> -	{ .base = CONFIG_SYS_NS16550_COM4, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
>> -	{ .base = CONFIG_SYS_NS16550_COM5, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
>> -	{ .base = CONFIG_SYS_NS16550_COM6, .reg_shift = 2, .clock = CONFIG_SYS_NS16550_CLK },
>> +	{ .base = CONFIG_SYS_NS16550_COM3, .reg_shift = 2,
>> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
>> +	{ .base = CONFIG_SYS_NS16550_COM4, .reg_shift = 2,
>> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
>> +	{ .base = CONFIG_SYS_NS16550_COM5, .reg_shift = 2,
>> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
>> +	{ .base = CONFIG_SYS_NS16550_COM6, .reg_shift = 2,
>> +	  .clock = CONFIG_SYS_NS16550_CLK, .fcr = UART_FCRVAL },
>>   #  endif
>>   # endif
>>   };
>> --
>> 2.7.4
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[Tom Rini]

On Tue, Jan 10, 2017 at 08:08:51AM +0100, Heiko Schocher wrote:
> commit: 65f83802b7a5b "serial: 16550: Add getfcr accessor"
> breaks u-boot commandline working with long commands
> sending to the board.
> 
> Since the above patch, you have to setup the fcr register.
> 
> For board/archs which enable OF_PLATDATA, the new field
> fcr in struct ns16550_platdata is not filled with a
> default value ...
> 
> This leads in not setting up the uarts fifo, which ends
> in problems, when you send long commands to u-boots
> commandline.
> 
> Detected this issue with automated tbot tests on am335x
> based shc board.
> 
> The error does not popup, if you type commands. You need
> to copy&paste a long command to u-boots commandshell
> (or send a long command with tbot)
> 
> Possible boards/plattforms with problems:
> ./arch/arm/cpu/arm926ejs/lpc32xx/devices.c
> ./arch/arm/mach-tegra/board.c
> ./board/isee/igep00x0/igep00x0.c
> ./board/overo/overo.c
> ./board/quipos/cairo/cairo.c
> ./board/logicpd/omap3som/omap3logic.c
> ./board/logicpd/zoom1/zoom1.c
> ./board/timll/devkit8000/devkit8000.c
> ./board/lg/sniper/sniper.c
> ./board/ti/beagle/beagle.c
> ./drivers/serial/serial_rockchip.c
> 
> This patch fixes only:
> ./arch/arm/mach-omap2/am33xx/board.c
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>

Good find!  I didn't catch this in my setups as nothing in test.py is
currently long enough to trigger this apparently (I have both an am335x
evm and a beagleboard in my setup).  By inspection, the right fix for
all platforms would be to put the old default value in as this is what
was missed in the original patch from Marek.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170110/f55900c4/attachment.sig>

[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[Heiko Schocher]

Hello Tom,

Am 10.01.2017 um 17:40 schrieb Tom Rini:
> On Tue, Jan 10, 2017 at 08:08:51AM +0100, Heiko Schocher wrote:
>> commit: 65f83802b7a5b "serial: 16550: Add getfcr accessor"
>> breaks u-boot commandline working with long commands
>> sending to the board.
>>
>> Since the above patch, you have to setup the fcr register.
>>
>> For board/archs which enable OF_PLATDATA, the new field
>> fcr in struct ns16550_platdata is not filled with a
>> default value ...
>>
>> This leads in not setting up the uarts fifo, which ends
>> in problems, when you send long commands to u-boots
>> commandline.
>>
>> Detected this issue with automated tbot tests on am335x
>> based shc board.
>>
>> The error does not popup, if you type commands. You need
>> to copy&paste a long command to u-boots commandshell
>> (or send a long command with tbot)
>>
>> Possible boards/plattforms with problems:
>> ./arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>> ./arch/arm/mach-tegra/board.c
>> ./board/isee/igep00x0/igep00x0.c
>> ./board/overo/overo.c
>> ./board/quipos/cairo/cairo.c
>> ./board/logicpd/omap3som/omap3logic.c
>> ./board/logicpd/zoom1/zoom1.c
>> ./board/timll/devkit8000/devkit8000.c
>> ./board/lg/sniper/sniper.c
>> ./board/ti/beagle/beagle.c
>> ./drivers/serial/serial_rockchip.c
>>
>> This patch fixes only:
>> ./arch/arm/mach-omap2/am33xx/board.c
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>
> Good find!  I didn't catch this in my setups as nothing in test.py is
> currently long enough to trigger this apparently (I have both an am335x
> evm and a beagleboard in my setup).  By inspection, the right fix for
> all platforms would be to put the old default value in as this is what
> was missed in the original patch from Marek.

The problems are boards/archs, which create "struct ns16550_platdata"
on their own ... I added the original value for fcr in
./arch/arm/mach-omap2/am33xx/board.c

Do you mean, I should grep for all this places and
fix them in my patch (without having the chance to test it) ?

Ok, seems a valid solution ... I try to do this today.

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[Tom Rini]

On Wed, Jan 11, 2017 at 06:38:22AM +0100, Heiko Schocher wrote:
> Hello Tom,
> 
> Am 10.01.2017 um 17:40 schrieb Tom Rini:
> >On Tue, Jan 10, 2017 at 08:08:51AM +0100, Heiko Schocher wrote:
> >>commit: 65f83802b7a5b "serial: 16550: Add getfcr accessor"
> >>breaks u-boot commandline working with long commands
> >>sending to the board.
> >>
> >>Since the above patch, you have to setup the fcr register.
> >>
> >>For board/archs which enable OF_PLATDATA, the new field
> >>fcr in struct ns16550_platdata is not filled with a
> >>default value ...
> >>
> >>This leads in not setting up the uarts fifo, which ends
> >>in problems, when you send long commands to u-boots
> >>commandline.
> >>
> >>Detected this issue with automated tbot tests on am335x
> >>based shc board.
> >>
> >>The error does not popup, if you type commands. You need
> >>to copy&paste a long command to u-boots commandshell
> >>(or send a long command with tbot)
> >>
> >>Possible boards/plattforms with problems:
> >>./arch/arm/cpu/arm926ejs/lpc32xx/devices.c
> >>./arch/arm/mach-tegra/board.c
> >>./board/isee/igep00x0/igep00x0.c
> >>./board/overo/overo.c
> >>./board/quipos/cairo/cairo.c
> >>./board/logicpd/omap3som/omap3logic.c
> >>./board/logicpd/zoom1/zoom1.c
> >>./board/timll/devkit8000/devkit8000.c
> >>./board/lg/sniper/sniper.c
> >>./board/ti/beagle/beagle.c
> >>./drivers/serial/serial_rockchip.c
> >>
> >>This patch fixes only:
> >>./arch/arm/mach-omap2/am33xx/board.c
> >>
> >>Signed-off-by: Heiko Schocher <hs@denx.de>
> >
> >Good find!  I didn't catch this in my setups as nothing in test.py is
> >currently long enough to trigger this apparently (I have both an am335x
> >evm and a beagleboard in my setup).  By inspection, the right fix for
> >all platforms would be to put the old default value in as this is what
> >was missed in the original patch from Marek.
> 
> The problems are boards/archs, which create "struct ns16550_platdata"
> on their own ... I added the original value for fcr in
> ./arch/arm/mach-omap2/am33xx/board.c
> 
> Do you mean, I should grep for all this places and
> fix them in my patch (without having the chance to test it) ?
> 
> Ok, seems a valid solution ... I try to do this today.

Yes, you've got it.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170111/bca21fcb/attachment.sig>

[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[Marek Vasut]

On 01/10/2017 05:40 PM, Tom Rini wrote:
> On Tue, Jan 10, 2017 at 08:08:51AM +0100, Heiko Schocher wrote:
>> commit: 65f83802b7a5b "serial: 16550: Add getfcr accessor"
>> breaks u-boot commandline working with long commands
>> sending to the board.
>>
>> Since the above patch, you have to setup the fcr register.
>>
>> For board/archs which enable OF_PLATDATA, the new field
>> fcr in struct ns16550_platdata is not filled with a
>> default value ...
>>
>> This leads in not setting up the uarts fifo, which ends
>> in problems, when you send long commands to u-boots
>> commandline.
>>
>> Detected this issue with automated tbot tests on am335x
>> based shc board.
>>
>> The error does not popup, if you type commands. You need
>> to copy&paste a long command to u-boots commandshell
>> (or send a long command with tbot)
>>
>> Possible boards/plattforms with problems:
>> ./arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>> ./arch/arm/mach-tegra/board.c
>> ./board/isee/igep00x0/igep00x0.c
>> ./board/overo/overo.c
>> ./board/quipos/cairo/cairo.c
>> ./board/logicpd/omap3som/omap3logic.c
>> ./board/logicpd/zoom1/zoom1.c
>> ./board/timll/devkit8000/devkit8000.c
>> ./board/lg/sniper/sniper.c
>> ./board/ti/beagle/beagle.c
>> ./drivers/serial/serial_rockchip.c
>>
>> This patch fixes only:
>> ./arch/arm/mach-omap2/am33xx/board.c
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
> 
> Good find!  I didn't catch this in my setups as nothing in test.py is
> currently long enough to trigger this apparently (I have both an am335x
> evm and a beagleboard in my setup).  By inspection, the right fix for
> all platforms would be to put the old default value in as this is what
> was missed in the original patch from Marek.
> 
It was probably missed because this patch pre-dates the OF_PLATDATA
addition and was stuck in the ML for too long.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[Heiko Schocher]

Hi Marek,

Am 11.01.2017 um 16:11 schrieb Marek Vasut:
> On 01/10/2017 05:40 PM, Tom Rini wrote:
>> On Tue, Jan 10, 2017 at 08:08:51AM +0100, Heiko Schocher wrote:
>>> commit: 65f83802b7a5b "serial: 16550: Add getfcr accessor"
>>> breaks u-boot commandline working with long commands
>>> sending to the board.
>>>
>>> Since the above patch, you have to setup the fcr register.
>>>
>>> For board/archs which enable OF_PLATDATA, the new field
>>> fcr in struct ns16550_platdata is not filled with a
>>> default value ...
>>>
>>> This leads in not setting up the uarts fifo, which ends
>>> in problems, when you send long commands to u-boots
>>> commandline.
>>>
>>> Detected this issue with automated tbot tests on am335x
>>> based shc board.
>>>
>>> The error does not popup, if you type commands. You need
>>> to copy&paste a long command to u-boots commandshell
>>> (or send a long command with tbot)
>>>
>>> Possible boards/plattforms with problems:
>>> ./arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>>> ./arch/arm/mach-tegra/board.c
>>> ./board/isee/igep00x0/igep00x0.c
>>> ./board/overo/overo.c
>>> ./board/quipos/cairo/cairo.c
>>> ./board/logicpd/omap3som/omap3logic.c
>>> ./board/logicpd/zoom1/zoom1.c
>>> ./board/timll/devkit8000/devkit8000.c
>>> ./board/lg/sniper/sniper.c
>>> ./board/ti/beagle/beagle.c
>>> ./drivers/serial/serial_rockchip.c
>>>
>>> This patch fixes only:
>>> ./arch/arm/mach-omap2/am33xx/board.c
>>>
>>> Signed-off-by: Heiko Schocher <hs@denx.de>
>>
>> Good find!  I didn't catch this in my setups as nothing in test.py is
>> currently long enough to trigger this apparently (I have both an am335x
>> evm and a beagleboard in my setup).  By inspection, the right fix for
>> all platforms would be to put the old default value in as this is what
>> was missed in the original patch from Marek.
>>
> It was probably missed because this patch pre-dates the OF_PLATDATA
> addition and was stuck in the ML for too long.

No problem, I prepare a fix. But it shows how important would be,
that we have as much as possible boards in a real life test setup...
(and of course, as much as possible testcases)

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[Adam Ford]

On Tue, Jan 10, 2017 at 1:08 AM, Heiko Schocher <hs@denx.de> wrote:
> commit: 65f83802b7a5b "serial: 16550: Add getfcr accessor"
> breaks u-boot commandline working with long commands
> sending to the board.
>
> Since the above patch, you have to setup the fcr register.
>
> For board/archs which enable OF_PLATDATA, the new field
> fcr in struct ns16550_platdata is not filled with a
> default value ...
>
> This leads in not setting up the uarts fifo, which ends
> in problems, when you send long commands to u-boots
> commandline.
>
> Detected this issue with automated tbot tests on am335x
> based shc board.
>
> The error does not popup, if you type commands. You need
> to copy&paste a long command to u-boots commandshell
> (or send a long command with tbot)
>
> Possible boards/plattforms with problems:
> ./arch/arm/cpu/arm926ejs/lpc32xx/devices.c
> ./arch/arm/mach-tegra/board.c
> ./board/isee/igep00x0/igep00x0.c
> ./board/overo/overo.c
> ./board/quipos/cairo/cairo.c
> ./board/logicpd/omap3som/omap3logic.c
> ./board/logicpd/zoom1/zoom1.c
> ./board/timll/devkit8000/devkit8000.c
> ./board/lg/sniper/sniper.c
> ./board/ti/beagle/beagle.c
> ./drivers/serial/serial_rockchip.c
>
> This patch fixes only:
> ./arch/arm/mach-omap2/am33xx/board.c
>
> Signed-off-by: Heiko Schocher <hs@denx.de>

The omap3logic is also affected.  It also appears as if you hold a
character down while booting, it may hang the system. I applied a
similar patch to omap3logic.c and it went away.

Thanks for catching that!

adam
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] serial, ns16550: bugfix: ns16550 fifo not enabled

[Heiko Schocher]

Hello Adam,

Am 11.01.2017 um 15:44 schrieb Adam Ford:
> On Tue, Jan 10, 2017 at 1:08 AM, Heiko Schocher <hs@denx.de> wrote:
>> commit: 65f83802b7a5b "serial: 16550: Add getfcr accessor"
>> breaks u-boot commandline working with long commands
>> sending to the board.
>>
>> Since the above patch, you have to setup the fcr register.
>>
>> For board/archs which enable OF_PLATDATA, the new field
>> fcr in struct ns16550_platdata is not filled with a
>> default value ...
>>
>> This leads in not setting up the uarts fifo, which ends
>> in problems, when you send long commands to u-boots
>> commandline.
>>
>> Detected this issue with automated tbot tests on am335x
>> based shc board.
>>
>> The error does not popup, if you type commands. You need
>> to copy&paste a long command to u-boots commandshell
>> (or send a long command with tbot)
>>
>> Possible boards/plattforms with problems:
>> ./arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>> ./arch/arm/mach-tegra/board.c
>> ./board/isee/igep00x0/igep00x0.c
>> ./board/overo/overo.c
>> ./board/quipos/cairo/cairo.c
>> ./board/logicpd/omap3som/omap3logic.c
>> ./board/logicpd/zoom1/zoom1.c
>> ./board/timll/devkit8000/devkit8000.c
>> ./board/lg/sniper/sniper.c
>> ./board/ti/beagle/beagle.c
>> ./drivers/serial/serial_rockchip.c
>>
>> This patch fixes only:
>> ./arch/arm/mach-omap2/am33xx/board.c
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>
> The omap3logic is also affected.  It also appears as if you hold a
> character down while booting, it may hang the system. I applied a
> similar patch to omap3logic.c and it went away.

Thanks for testing!

> Thanks for catching that!

Thanks goes to tbot [1] and weekly automated u-boot tests with
tbot integrated into buildbot [2]

(I also automated "git bisect" to find the, which commit breaks
  a board ... if interested in a demo video, see[5])

bye,
Heiko
[1] https://github.com/hsdenx/tbot
[2] http://xeidos.ddns.net/buildbot/tgrid
[3] tbot result webpage
     http://xeidos.ddns.net/tests/test_db_auslesen.php
[4] tbot dokumentation
     http://www.tbot.tools/main.html
[5] tbot demo on youtube
     https://youtu.be/zfjpj3DLsx4
     Its uncutted, so very long, but in the video comment I added
     timemarkers, where the interesting parts are ...
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany