From 93ed0371a0a0441d88d3d0a72f5b9653ae2203fb Mon Sep 17 00:00:00 2001 From: Mingqiang Chi Date: Mon, 30 Jul 2018 11:29:34 +0800 Subject: [PATCH] hv:cleanup console/uart code -- Check uart enabled flag, it will return if the flag is false. -- Add function declaration (uart16550_set_property) in console.h -- Remove unnecessary function declaration(get_serial_handle) -- Change uart_enabled and port_mapped to bool -- Fix MISRA-C integer violations Signed-off-by: Mingqiang Chi Acked-by: Eddie Dong --- hypervisor/bsp/uefi/cmdline.c | 6 +-- hypervisor/debug/console.c | 13 ++---- hypervisor/debug/uart16550.c | 65 ++++++++++++++++++++---------- hypervisor/debug/uart16550.h | 12 +++--- hypervisor/debug/vuart.c | 10 ++--- hypervisor/include/debug/console.h | 10 ++--- 6 files changed, 64 insertions(+), 52 deletions(-) diff --git a/hypervisor/bsp/uefi/cmdline.c b/hypervisor/bsp/uefi/cmdline.c index 09cbaf93b..fc05b2e04 100644 --- a/hypervisor/bsp/uefi/cmdline.c +++ b/hypervisor/bsp/uefi/cmdline.c @@ -44,7 +44,7 @@ static void handle_cmd(const char *cmd, int len) if (i == IDX_DISABLE_UART) { /* set uart disabled*/ - uart16550_set_property(0, 0, 0); + uart16550_set_property(false, false, 0UL); } else if ((i == IDX_PORT_UART) || (i == IDX_MMIO_UART)) { uint64_t addr = strtoul_hex(cmd + tmp); @@ -54,9 +54,9 @@ static void handle_cmd(const char *cmd, int len) if (addr > MAX_PORT) addr = DEFAULT_UART_PORT; - uart16550_set_property(1, 1, addr); + uart16550_set_property(true, true, addr); } else { - uart16550_set_property(1, 0, addr); + uart16550_set_property(true, false, addr); } } } diff --git a/hypervisor/debug/console.c b/hypervisor/debug/console.c index 9dd3bf77b..cb766a680 100644 --- a/hypervisor/debug/console.c +++ b/hypervisor/debug/console.c @@ -9,7 +9,7 @@ struct hv_timer console_timer; -#define CONSOLE_KICK_TIMER_TIMEOUT 40 /* timeout is 40ms*/ +#define CONSOLE_KICK_TIMER_TIMEOUT 40UL /* timeout is 40ms*/ void console_init(void) { @@ -18,7 +18,7 @@ void console_init(void) void console_putc(const char *ch) { - (void)uart16550_puts(ch, 1); + (void)uart16550_puts(ch, 1U); } @@ -32,10 +32,11 @@ char console_getc(void) return uart16550_getc(); } -static void console_handler(void) +static int console_timer_callback(__unused void *data) { struct vuart *vu; + /* Kick HV-Shell and Uart-Console tasks */ vu = vuart_console_active(); if (vu != NULL) { /* serial Console Rx operation */ @@ -45,12 +46,6 @@ static void console_handler(void) } else { shell_kick(); } -} - -static int console_timer_callback(__unused void *data) -{ - /* Kick HV-Shell and Uart-Console tasks */ - console_handler(); return 0; } diff --git a/hypervisor/debug/uart16550.c b/hypervisor/debug/uart16550.c index 28da0a650..837ba2918 100644 --- a/hypervisor/debug/uart16550.c +++ b/hypervisor/debug/uart16550.c @@ -8,18 +8,18 @@ #include "uart16550.h" #if defined(CONFIG_SERIAL_PIO_BASE) -static int serial_port_mapped = 1; -static int uart_enabled = 1; +static bool serial_port_mapped = true; +static bool uart_enabled = true; #define UART_BASE_ADDRESS CONFIG_SERIAL_PIO_BASE #elif defined(CONFIG_SERIAL_MMIO_BASE) -static int serial_port_mapped; -static int uart_enabled = 1; +static bool serial_port_mapped; +static bool uart_enabled = true; #define UART_BASE_ADDRESS CONFIG_SERIAL_MMIO_BASE #else #warning "no uart base configure, please check!" -static int serial_port_mapped; -static int uart_enabled; -#define UART_BASE_ADDRESS 0 +static bool serial_port_mapped; +static bool uart_enabled; +#define UART_BASE_ADDRESS 0UL #endif typedef uint32_t uart_reg_t; @@ -29,20 +29,26 @@ static uint64_t uart_base_address; static spinlock_t uart_rx_lock; static spinlock_t uart_tx_lock; -static inline uint32_t uart16550_read_reg(uint64_t base, uint32_t reg_idx) +/** + * @pre uart_enabled == true + */ +static inline uint32_t uart16550_read_reg(uint64_t base, uint16_t reg_idx) { - if (serial_port_mapped != 0) { + if (serial_port_mapped) { return io_read_byte((uint16_t)base + reg_idx); } else { return mmio_read_long((void*)((uint32_t*)HPA2HVA(base) + reg_idx)); } } +/** + * @pre uart_enabled == true + */ static inline void uart16550_write_reg(uint64_t base, - uint32_t val, uint32_t reg_idx) + uint32_t val, uint16_t reg_idx) { - if (serial_port_mapped != 0) { - io_write_byte(val, (uint16_t)base + reg_idx); + if (serial_port_mapped) { + io_write_byte((uint8_t)val, (uint16_t)base + reg_idx); } else { mmio_write_long(val, (void*)((uint32_t*)HPA2HVA(base) + reg_idx)); } @@ -52,14 +58,17 @@ static void uart16550_calc_baud_div(uint32_t ref_freq, uint32_t *baud_div_ptr, uint32_t baud_rate_arg) { uint32_t baud_rate = baud_rate_arg; - uint32_t baud_multiplier = baud_rate < BAUD_460800 ? 16 : 13; + uint32_t baud_multiplier = baud_rate < BAUD_460800 ? 16U : 13U; - if (baud_rate == 0) { + if (baud_rate == 0U) { baud_rate = BAUD_115200; } *baud_div_ptr = ref_freq / (baud_multiplier * baud_rate); } +/** + * @pre uart_enabled == true + */ static void uart16550_set_baud_rate(uint32_t baud_rate) { uint32_t baud_div, duart_clock = UART_CLOCK_RATE; @@ -75,7 +84,7 @@ static void uart16550_set_baud_rate(uint32_t baud_rate) /* Write the appropriate divisor value */ uart16550_write_reg(uart_base_address, - ((baud_div >> 8) & 0xFFU), UART16550_DLM); + ((baud_div >> 8U) & 0xFFU), UART16550_DLM); uart16550_write_reg(uart_base_address, (baud_div & 0xFFU), UART16550_DLL); @@ -86,7 +95,10 @@ static void uart16550_set_baud_rate(uint32_t baud_rate) void uart16550_init(void) { - if (uart_base_address == 0) { + if (!uart_enabled) { + return; + } + if (uart_base_address == 0UL) { uart_base_address = UART_BASE_ADDRESS; } spinlock_init(&uart_rx_lock); @@ -116,6 +128,10 @@ char uart16550_getc(void) { char ret = -1; + if (!uart_enabled) { + return ret; + } + spinlock_obtain(&uart_rx_lock); /* If a character has been received, read it */ @@ -129,7 +145,10 @@ char uart16550_getc(void) return ret; } -static void uart16550_putc(const char *buf) +/** + * @pre uart_enabled == true + */ +static void uart16550_putc(const char c) { uint32_t reg; /* Ensure there are no further Transmit buffer write requests */ @@ -138,20 +157,22 @@ static void uart16550_putc(const char *buf) } while ((reg & LSR_THRE) == 0U || (reg & LSR_TEMT) == 0U); /* Transmit the character. */ - uart16550_write_reg(uart_base_address, *buf, UART16550_THR); + uart16550_write_reg(uart_base_address, c, UART16550_THR); } int uart16550_puts(const char *buf, uint32_t len) { uint32_t i; - + if (!uart_enabled) { + return (int)len; + } spinlock_obtain(&uart_tx_lock); for (i = 0U; i < len; i++) { /* Transmit character */ - uart16550_putc(buf); + uart16550_putc(*buf); if (*buf == '\n') { /* Append '\r', no need change the len */ - uart16550_putc("\r"); + uart16550_putc('\r'); } buf++; } @@ -159,7 +180,7 @@ int uart16550_puts(const char *buf, uint32_t len) return (int)len; } -void uart16550_set_property(int enabled, int port_mapped, uint64_t base_addr) +void uart16550_set_property(bool enabled, bool port_mapped, uint64_t base_addr) { uart_enabled = enabled; serial_port_mapped = port_mapped; diff --git a/hypervisor/debug/uart16550.h b/hypervisor/debug/uart16550.h index 06b41778c..06ebec6c5 100644 --- a/hypervisor/debug/uart16550.h +++ b/hypervisor/debug/uart16550.h @@ -99,14 +99,12 @@ #define UART_IER_DISABLE_ALL 0x00000000U -#define BAUD_9600 9600 -#define BAUD_115200 115200 -#define BAUD_460800 460800 +#define BAUD_9600 9600U +#define BAUD_115200 115200U +#define BAUD_460800 460800U -/* CPU oscillator clock */ -#define CPU_OSC_CLOCK 1843200 /* 1.8432 MHz */ -/* UART hardware definitions */ -#define UART_CLOCK_RATE CPU_OSC_CLOCK +/* UART oscillator clock */ +#define UART_CLOCK_RATE 1843200U /* 1.8432 MHz */ void uart16550_init(void); char uart16550_getc(void); diff --git a/hypervisor/debug/vuart.c b/hypervisor/debug/vuart.c index 5d72ac284..308d1b9f1 100644 --- a/hypervisor/debug/vuart.c +++ b/hypervisor/debug/vuart.c @@ -33,7 +33,7 @@ #include "uart16550.h" #define COM1_BASE 0x3F8 -#define COM1_IRQ 4 +#define COM1_IRQ 4U #define RX_FIFO_SIZE 256 #define TX_FIFO_SIZE 65536 @@ -173,10 +173,10 @@ static void vuart_write(__unused struct vm_io_handler *hdlr, * The FCR_ENABLE bit must be '1' for the programming * of other FCR bits to be effective. */ - if ((value & FCR_FIFOE) == 0) { + if ((value & FCR_FIFOE) == 0U) { vu->fcr = 0; } else { - if ((value & FCR_RFR) != 0) { + if ((value & FCR_RFR) != 0U) { fifo_reset(&vu->rxfifo); } @@ -364,9 +364,9 @@ void *vuart_init(struct vm *vm) ASSERT(vu != NULL, ""); /* Set baud rate*/ - divisor = UART_CLOCK_RATE / BAUD_9600 / 16; + divisor = UART_CLOCK_RATE / BAUD_9600 / 16U; vu->dll = divisor; - vu->dlh = divisor >> 16; + vu->dlh = divisor >> 16U; vu->active = false; vu->base = COM1_BASE; diff --git a/hypervisor/include/debug/console.h b/hypervisor/include/debug/console.h index f66bc4e5e..3ddcd3049 100644 --- a/hypervisor/include/debug/console.h +++ b/hypervisor/include/debug/console.h @@ -41,8 +41,6 @@ int console_gets(char *buffer, uint32_t length); void console_setup_timer(void); -uint32_t get_serial_handle(void); - static inline void suspend_console(void) { del_timer(&console_timer); @@ -52,6 +50,7 @@ static inline void resume_console(void) { console_setup_timer(); } +void uart16550_set_property(bool enabled, bool port_mapped, uint64_t base_addr); #else static inline void console_init(void) @@ -65,13 +64,12 @@ static inline int console_write(__unused const char *str, } static inline void console_putc(__unused const char *ch) { } static inline int console_getc(void) { return 0; } - -static inline int console_gets(char *buffer, uint32_t length) { return 0; } +static inline int console_gets(char *buffer, uint32_t length) { return 0; } static inline void console_setup_timer(void) {} -static inline uint32_t get_serial_handle(void) { return 0; } - static inline void suspend_console(void) {} static inline void resume_console(void) {} +static inline void uart16550_set_property(__unused bool enabled, + __unused bool port_mapped, __unused uint64_t base_addr) {} #endif #endif /* CONSOLE_H */