[coreboot-gerrit] Change in coreboot[master]: uart: Fix bug in {uart8250, uart8250_mem, ns16550}_rx_byte f...
Werner Zeh (Code Review)
gerrit at coreboot.org
Thu May 18 13:09:19 CEST 2017
Werner Zeh has submitted this change and it was merged. ( https://review.coreboot.org/19731 )
Change subject: uart: Fix bug in {uart8250, uart8250_mem, ns16550}_rx_byte functions
......................................................................
uart: Fix bug in {uart8250, uart8250_mem, ns16550}_rx_byte functions
We have several different UART implementations of which three support a
timeout when receiving characters. In all of these three implementations
there is a bug where when the timeout is hit the last received character
will be returned instead of the needed 0.
The problem is that the timeout variable i is decremented after it has
been checked in the while-loop. That leads to the fact that when the
while-loop is aborted due to a timeout i will contain 0xffffffff and not
0. Thus in turn will fool the following if-statement leading to wrong
return value to the caller in this case. Therefore the caller will see a
received character event if there is none.
Change-Id: I23ff531a1e729e816764f1a071484c924dcb0f85
Signed-off-by: Werner Zeh <werner.zeh at siemens.com>
Reviewed-on: https://review.coreboot.org/19731
Tested-by: build bot (Jenkins) <no-reply at coreboot.org>
Reviewed-by: Aaron Durbin <adurbin at chromium.org>
---
M src/drivers/uart/uart8250io.c
M src/drivers/uart/uart8250mem.c
M src/soc/broadcom/cygnus/ns16550.c
3 files changed, 8 insertions(+), 3 deletions(-)
Approvals:
Aaron Durbin: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/src/drivers/uart/uart8250io.c b/src/drivers/uart/uart8250io.c
index ac3315a..4cc7fe3 100644
--- a/src/drivers/uart/uart8250io.c
+++ b/src/drivers/uart/uart8250io.c
@@ -62,7 +62,8 @@
static unsigned char uart8250_rx_byte(unsigned base_port)
{
unsigned long int i = SINGLE_CHAR_TIMEOUT;
- while (i-- && !uart8250_can_rx_byte(base_port));
+ while (i && !uart8250_can_rx_byte(base_port))
+ i--;
if (i)
return inb(base_port + UART8250_RBR);
diff --git a/src/drivers/uart/uart8250mem.c b/src/drivers/uart/uart8250mem.c
index 4e53a92..a142cb1 100644
--- a/src/drivers/uart/uart8250mem.c
+++ b/src/drivers/uart/uart8250mem.c
@@ -82,8 +82,10 @@
static unsigned char uart8250_mem_rx_byte(void *base)
{
unsigned long int i = SINGLE_CHAR_TIMEOUT;
- while (i-- && !uart8250_mem_can_rx_byte(base))
+ while (i && !uart8250_mem_can_rx_byte(base)) {
udelay(1);
+ i--;
+ }
if (i)
return uart8250_read(base, UART8250_RBR);
else
diff --git a/src/soc/broadcom/cygnus/ns16550.c b/src/soc/broadcom/cygnus/ns16550.c
index 71a4cb0..aa9dd2d 100644
--- a/src/soc/broadcom/cygnus/ns16550.c
+++ b/src/soc/broadcom/cygnus/ns16550.c
@@ -84,8 +84,10 @@
static unsigned char ns16550_rx_byte(void)
{
unsigned long int i = SINGLE_CHAR_TIMEOUT;
- while (i-- && !ns16550_tst_byte())
+ while (i && !ns16550_tst_byte()) {
udelay(1);
+ i--;
+ }
if (i)
return read32(®s->rbr);
else
--
To view, visit https://review.coreboot.org/19731
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I23ff531a1e729e816764f1a071484c924dcb0f85
Gerrit-PatchSet: 2
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Werner Zeh <werner.zeh at siemens.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer at siemens.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude at gmail.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh at siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
More information about the coreboot-gerrit
mailing list