[PATCH] lib: utils: Fix shakti uart implementation

Jessica Clarke jrtc27 at jrtc27.com
Mon Dec 7 12:14:06 EST 2020


> On 7 Dec 2020, at 16:54, Vijai Kumar K <vijai at behindbytes.com> wrote:
> 
> Fix uart_putc implementation.
> Due to a bug in the IP, this went unnoticed.
> Use macros instead of magic numbers to make the code
> more readable.

Yeah, using macros is good, but this patch isn't doing anything other
than adding macros; there's no change of behaviour? Either you've
missed something or your commit message needs to be fixed.

Also see below for a style comment.

Jess

> Signed-off-by: Vijai Kumar K <vijai at behindbytes.com>
> ---
> lib/utils/serial/shakti-uart.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/utils/serial/shakti-uart.c b/lib/utils/serial/shakti-uart.c
> index 493edcf..b2deb70 100644
> --- a/lib/utils/serial/shakti-uart.c
> +++ b/lib/utils/serial/shakti-uart.c
> @@ -18,18 +18,21 @@
> #define REG_IQ_CYCLES	0x1C
> #define REG_RX_THRES	0x20
> 
> +#define UART_TX_FULL  0x2
> +#define UART_RX_FULL  0x8
> +
> static volatile void *uart_base;
> 
> void shakti_uart_putc(char ch)
> {
> -	while((readw(uart_base + REG_STATUS) & 0x2) == 0);
> +	while((readw(uart_base + REG_STATUS) & UART_TX_FULL));

You should have a space after the while. Also, the existing style is to
put the semicolon on a new line so it's not accidentally overlooked.

> 	writeb(ch, uart_base + REG_TX);
> }
> 
> int shakti_uart_getc(void)
> {
> 	u16 status = readw(uart_base + REG_STATUS);
> -	if (status & 0x8)
> +	if (status & UART_RX_FULL)
> 		return readb(uart_base + REG_RX);
> 	return -1;
> }
> -- 
> 2.25.1



More information about the opensbi mailing list