From 43314ffae53b813c6a0d6e34723921316cf46f45 Mon Sep 17 00:00:00 2001 From: Werner Zeh Date: Wed, 17 May 2017 10:13:24 +0200 Subject: [PATCH] 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 Reviewed-on: https://review.coreboot.org/19731 Tested-by: build bot (Jenkins) Reviewed-by: Aaron Durbin --- src/drivers/uart/uart8250io.c | 3 ++- src/drivers/uart/uart8250mem.c | 4 +++- src/soc/broadcom/cygnus/ns16550.c | 4 +++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/drivers/uart/uart8250io.c b/src/drivers/uart/uart8250io.c index ac3315a421..4cc7fe3e0b 100644 --- a/src/drivers/uart/uart8250io.c +++ b/src/drivers/uart/uart8250io.c @@ -62,7 +62,8 @@ static int uart8250_can_rx_byte(unsigned base_port) 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 4e53a92a46..a142cb111a 100644 --- a/src/drivers/uart/uart8250mem.c +++ b/src/drivers/uart/uart8250mem.c @@ -82,8 +82,10 @@ static int uart8250_mem_can_rx_byte(void *base) 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 71a4cb08ef..aa9dd2d818 100644 --- a/src/soc/broadcom/cygnus/ns16550.c +++ b/src/soc/broadcom/cygnus/ns16550.c @@ -84,8 +84,10 @@ static int ns16550_tst_byte(void) 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