Clean up UART module

And change some direct uart_printf()/uart_puts() output to console
output methods instead.  Disable unused comxtest debug command.  No
other functional changes.

BUG=chrome-os-partner:15579
BRANCH=none
TEST=boot system; should still see debug output with reset flags

Change-Id: I57fe6bb781a1ba7884afa6d090b74a92f45a53cc
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/36835

5 files changed, 184 insertions, 114 deletions

diff --git a/chip/lm4/uart.c b/chip/lm4/uart.c
index 3595721441..bc5cadb3fe 100644
--- a/chip/lm4/uart.c
+++ b/chip/lm4/uart.c
@@ -5,7 +5,7 @@
 
 /* UART module for Chrome EC */
 
-#include "board.h"
+#include "common.h"
 #include "console.h"
 #include "gpio.h"
 #include "lpc.h"
@@ -17,10 +17,8 @@
 /* Baud rate for UARTs */
 #define BAUD_RATE 115200
 
-
 static int init_done;
 
-
 int uart_init_done(void)
 {
 	return init_done;
@@ -28,10 +26,12 @@ int uart_init_done(void)
 
 void uart_tx_start(void)
 {
-	/* Re-enable the transmit interrupt, then forcibly trigger the
+	/*
+	 * Re-enable the transmit interrupt, then forcibly trigger the
 	 * interrupt.  This works around a hardware problem with the
 	 * UART where the FIFO only triggers the interrupt when its
-	 * threshold is _crossed_, not just met. */
+	 * threshold is _crossed_, not just met.
+	 */
 	LM4_UART_IM(0) |= 0x20;
 	task_trigger_irq(LM4_IRQ_UART0);
 }
@@ -94,7 +94,9 @@ void uart_enable_interrupt(void)
 	task_enable_irq(LM4_IRQ_UART0);
 }
 
-/* Interrupt handler for UART0 */
+/**
+ * Interrupt handler for UART0
+ */
 static void uart_0_interrupt(void)
 {
 	/* Clear transmit and receive interrupt status */
@@ -106,26 +108,31 @@ static void uart_0_interrupt(void)
 }
 DECLARE_IRQ(LM4_IRQ_UART0, uart_0_interrupt, 1);
 
-
-/* Interrupt handler for UART1 */
+/**
+ * Interrupt handler for UART1
+ */
 static void uart_1_interrupt(void)
 {
 	/* Clear transmit and receive interrupt status */
 	LM4_UART_ICR(1) = 0x70;
 
 #ifdef CONFIG_LPC
-	/* If we have space in our FIFO and a character is pending in LPC,
-	 * handle that character. */
+	/*
+	 * If we have space in our FIFO and a character is pending in LPC,
+	 * handle that character.
+	 */
 	if (!(LM4_UART_FR(1) & 0x20) && lpc_comx_has_char()) {
 		/* Copy the next byte then disable transmit interrupt */
 		LM4_UART_DR(1) = lpc_comx_get_char();
 		LM4_UART_IM(1) &= ~0x20;
 	}
 
-	/* Handle received character.  There is no flow control on input;
+	/*
+	 * Handle received character.  There is no flow control on input;
 	 * received characters are blindly forwarded to LPC.  This is ok
 	 * because LPC is much faster than UART, and we don't have flow control
-	 * on the UART receive-side either. */
+	 * on the UART receive-side either.
+	 */
 	if (!(LM4_UART_FR(1) & 0x10))
 		lpc_comx_put_char(LM4_UART_DR(1));
 #endif
@@ -133,8 +140,9 @@ static void uart_1_interrupt(void)
 /* Must be same prio as LPC interrupt handler so they don't preempt */
 DECLARE_IRQ(LM4_IRQ_UART1, uart_1_interrupt, 2);
 
-
-/* Configure GPIOs for the UART module. */
+/**
+ * Configure GPIOs for the UART module.
+ */
 static void configure_gpio(void)
 {
 #ifdef BOARD_link
@@ -150,8 +158,7 @@ static void configure_gpio(void)
 #endif
 }
 
-
-int uart_init(void)
+void uart_init(void)
 {
 	volatile uint32_t scratch  __attribute__((unused));
 	int ch;
@@ -174,57 +181,53 @@ int uart_init(void)
 		LM4_UART_FBRD(ch) =
 			(((INTERNAL_CLOCK / 16) % BAUD_RATE) * 64
 			 + BAUD_RATE / 2) / BAUD_RATE;
-		/* 8-N-1, FIFO enabled.  Must be done after setting
-		 * the divisor for the new divisor to take effect. */
+		/*
+		 * 8-N-1, FIFO enabled.  Must be done after setting
+		 * the divisor for the new divisor to take effect.
+		 */
 		LM4_UART_LCRH(ch) = 0x70;
-		/* Interrupt when RX fifo at minimum (>= 1/8 full), and TX fifo
-		 * when <= 1/4 full */
+		/*
+		 * Interrupt when RX fifo at minimum (>= 1/8 full), and TX fifo
+		 * when <= 1/4 full
+		 */
 		LM4_UART_IFLS(ch) = 0x01;
-		/* Unmask receive-FIFO, receive-timeout.  We need
+		/*
+		 * Unmask receive-FIFO, receive-timeout.  We need
 		 * receive-timeout because the minimum RX FIFO depth is 1/8 = 2
 		 * bytes; without the receive-timeout we'd never be notified
-		 * about single received characters. */
+		 * about single received characters.
+		 */
 		LM4_UART_IM(ch) = 0x50;
 		/* Enable the port */
 		LM4_UART_CTL(ch) |= 0x0001;
 	}
 
-	/* Enable interrupts for UART0 only. UART1 will have to wait until the
+	/*
+	 * Enable interrupts for UART0 only. UART1 will have to wait until the
 	 * LPC bus is initialized.
 	 */
 	uart_clear_rx_fifo(0);
 	task_enable_irq(LM4_IRQ_UART0);
 
 	init_done = 1;
-
-	return EC_SUCCESS;
 }
 
-
 /*****************************************************************************/
 /* COMx functions */
 
-/* Write a character to COMx, waiting for space in the output buffer if
- * necessary. */
-static void uart_comx_putc_wait(int c)
-{
-		while (!uart_comx_putc_ok()) {}
-		uart_comx_putc(c);
-}
-
-
 void uart_comx_enable(void)
 {
 	uart_clear_rx_fifo(1);
 	task_enable_irq(LM4_IRQ_UART1);
 }
 
-
 int uart_comx_putc_ok(void)
 {
 	if (LM4_UART_FR(1) & 0x20) {
-		/* FIFO is full, so enable transmit interrupt to let us know
-		 * when it empties. */
+		/*
+		 * FIFO is full, so enable transmit interrupt to let us know
+		 * when it empties.
+		 */
 		LM4_UART_IM(1) |= 0x20;
 		return 0;
 	} else {
@@ -232,16 +235,27 @@ int uart_comx_putc_ok(void)
 	}
 }
 
-
 void uart_comx_putc(int c)
 {
 	LM4_UART_DR(1) = c;
 }
 
-
 /*****************************************************************************/
 /* Console commands */
 
+#ifdef CONFIG_CMD_COMXTEST
+
+/**
+ * Write a character to COMx, waiting for space in the output buffer if
+ * necessary.
+ */
+static void uart_comx_putc_wait(int c)
+{
+		while (!uart_comx_putc_ok())
+			;
+		uart_comx_putc(c);
+}
+
 static int command_comxtest(int argc, char **argv)
 {
 	/* Put characters to COMX port */
@@ -261,3 +275,5 @@ DECLARE_CONSOLE_COMMAND(comxtest, command_comxtest,
 			"[string]",
 			"Write test data to COMx uart",
 			NULL);
+
+#endif /* CONFIG_COMX_TEST */
diff --git a/chip/stm32/uart.c b/chip/stm32/uart.c
index 77e0ba2668..2cd661d9e4 100644
--- a/chip/stm32/uart.c
+++ b/chip/stm32/uart.c
@@ -112,7 +112,7 @@ static void uart_interrupt(void)
 }
 DECLARE_IRQ(STM32_IRQ_USART(UARTN), uart_interrupt, 2);
 
-int uart_init(void)
+void uart_init(void)
 {
 	/* Enable USART clock */
 	if (UARTN == 1)
@@ -146,6 +146,4 @@ int uart_init(void)
 	task_enable_irq(STM32_IRQ_USART(UARTN));
 
 	init_done = 1;
-
-	return EC_SUCCESS;
 }
diff --git a/common/main.c b/common/main.c
index eb02ce8005..103cb42d19 100644
--- a/common/main.c
+++ b/common/main.c
@@ -7,6 +7,7 @@
 
 #include "clock.h"
 #include "common.h"
+#include "console.h"
 #include "cpu.h"
 #include "eeprom.h"
 #include "eoption.h"
@@ -22,6 +23,10 @@
 #include "uart.h"
 #include "watchdog.h"
 
+/* Console output macros */
+#define CPUTS(outstr) cputs(CC_SYSTEM, outstr)
+#define CPRINTF(format, args...) cprintf(CC_SYSTEM, format, ## args)
+
 int main(void)
 {
 	/*
@@ -76,20 +81,19 @@ int main(void)
 	/* Main initialization stage.  Modules may enable interrupts here. */
 	cpu_init();
 
-	/* Initialize UART.  uart_printf(), etc. may now be used. */
+	/* Initialize UART.  Console output functions may now be used. */
 	uart_init();
-	if (system_jumped_to_this_image())
-		uart_printf("[%T UART initialized after sysjump]\n");
-	else {
-		uart_puts("\n\n--- UART initialized after reboot ---\n");
-		uart_puts("[Reset cause: ");
+
+	if (system_jumped_to_this_image()) {
+		CPRINTF("[%T UART initialized after sysjump]\n");
+	} else {
+		CPUTS("\n\n--- UART initialized after reboot ---\n");
+		CPUTS("[Reset cause: ");
 		system_print_reset_flags();
-		uart_puts("]\n");
+		CPUTS("]\n");
 	}
-	uart_printf("[Image: %s, %s]\n",
-		    system_get_image_copy_string(),
-		    system_get_build_info());
-
+	CPRINTF("[Image: %s, %s]\n",
+		 system_get_image_copy_string(), system_get_build_info());
 
 #ifdef CONFIG_TASK_WATCHDOG
 	/*
@@ -129,7 +133,7 @@ int main(void)
 	 * into account the time before timer_init(), but it'll at least catch
 	 * the majority of the time.
 	 */
-	uart_printf("[%T Inits done]\n");
+	CPRINTF("[%T Inits done]\n");
 
 	/* Launch task scheduling (never returns) */
 	return task_start();
diff --git a/common/system_common.c b/common/system_common.c
index 8434d5faec..d77d82070e 100644
--- a/common/system_common.c
+++ b/common/system_common.c
@@ -137,16 +137,16 @@ void system_print_reset_flags(void)
 	int i;
 
 	if (!reset_flags) {
-		uart_puts("unknown");
+		CPUTS("unknown");
 		return;
 	}
 
 	for (i = 0; i < ARRAY_SIZE(reset_flag_descs); i++) {
 		if (reset_flags & (1 << i)) {
 			if (count++)
-				uart_puts(" ");
+				CPUTS(" ");
 
-			uart_puts(reset_flag_descs[i]);
+			CPUTS(reset_flag_descs[i]);
 		}
 	}
 }
diff --git a/include/uart.h b/include/uart.h
index 115896f54a..d4685cb2e7 100644
--- a/include/uart.h
+++ b/include/uart.h
@@ -11,60 +11,90 @@
 #include <stdarg.h>  /* For va_list */
 #include "common.h"
 
+/**
+ * Initialize the UART module.
+ */
+void uart_init(void);
 
-/* Initializes the UART module. */
-int uart_init(void);
-
-/* Return non-zero if UART init has completed. */
+/**
+ * Return non-zero if UART init has completed.
+ */
 int uart_init_done(void);
 
-/*****************************************************************************/
-/* Output functions
+/*
+ * Output functions
  *
  * Output is buffered.  If the buffer overflows, subsequent output is
- * discarded. */
+ * discarded.
+ *
+ * Modules should use the output functions in console.h in preference to these
+ * routines, so that output can be filtered on a module-by-module basis.
+ */
 
-/* Put a null-terminated string to the UART, like fputs().
+/**
+ * Put a null-terminated string to the UART, like fputs().
  *
- * Returns error if output was truncated. */
+ * @return EC_SUCCESS, or non-zero if output was truncated.
+ */
 int uart_puts(const char *outstr);
 
-/* Print formatted output to the UART, like printf().
+/**
+ * Print formatted output to the UART, like printf().
+ *
+ * See printf.h for valid formatting codes.
  *
- * See printf.h for valid formatting codes. */
+ * @return EC_SUCCESS, or non-zero if output was truncated.
+ */
 int uart_printf(const char *format, ...);
 
-/* Print formatted output to the UART, like vprintf().
+/**
+ * Print formatted output to the UART, like vprintf().
+ *
+ * See printf.h for valid formatting codes.
  *
- * See printf.h for valid formatting codes. */
+ * @return EC_SUCCESS, or non-zero if output was truncated.
+ */
 int uart_vprintf(const char *format, va_list args);
 
-/* Flushes output.  Blocks until UART has transmitted all output. */
+/**
+ * Flush output.  Blocks until UART has transmitted all output.
+ */
 void uart_flush_output(void);
 
-/*****************************************************************************/
-/* Input functions
+/*
+ * Input functions
  *
  * Input is buffered.  If the buffer overflows, the oldest input in
  * the buffer is discarded to make room for the new input.
  *
  * Input lines may be terminated by CR ('\r'), LF ('\n'), or CRLF; all
- * are translated to newline. */
+ * are translated to newline.
+ */
 
-/* Flushes input buffer, discarding all input. */
+/**
+ * Flush input buffer, discarding all input.
+ */
 void uart_flush_input(void);
 
-/* Non-destructively checks for a character in the input buffer.
+/**
+ * Non-destructively check for a character in the input buffer.
  *
- * Returns the offset into the input buffer of character <c>, or -1 if
- * it is not in the input buffer. */
+ * @param c		Character to search for
+ *
+ * @return the offset into the input buffer of the first match, or -1 if no
+ * match found in the input buffer.
+ */
 int uart_peek(int c);
 
-/* Reads a single character of input, similar to fgetc().  Returns the
- * character, or -1 if no input waiting. */
+/**
+ * Read a single character of input, similar to fgetc().
+ *
+ * @return the character, or -1 if no input waiting.
+ */
 int uart_getc(void);
 
-/* Reads characters from the UART, similar to fgets().
+/**
+ * Read characters from the UART, similar to fgets().
  *
  * Reads input until one of the following conditions is met:
  *    (1)  <size-1> characters have been read.
@@ -73,9 +103,9 @@ int uart_getc(void);
  *
  * Condition (3) means this call never blocks.  This is important
  * because it prevents a race condition where the caller calls
- * UartPeek() to see if input is waiting, or is notified by the
+ * uart_peek() to see if input is waiting, or is notified by the
  * callack that input is waiting, but then the input buffer overflows
- * or someone else grabs the input before UartGets() is called.
+ * or someone else grabs the input before uart_gets() is called.
  *
  * Characters are stored in <dest> and are null-terminated.
  * Characters include the newline if present, so that the caller can
@@ -83,83 +113,105 @@ int uart_getc(void);
  * input buffer is empty, a null-terminated empty string ("") is
  * returned.
  *
- * Returns the number of characters read (not counting the terminating
- * null). */
+ * @param dest		Destination for input
+ * @param size		Size of buffer pointed to by dest
+ *
+ * @return the number of characters read, not counting the terminating null.
+ */
 int uart_gets(char *dest, int size);
 
-/* TODO: getc(), putc() equivalents? */
-
-/*****************************************************************************/
-/* Hardware UART driver functions */
+/*
+ * Hardware UART driver functions
+ */
 
-/* Flushes the transmit FIFO. */
+/**
+ * Flush the transmit FIFO.
+ */
 void uart_tx_flush(void);
 
-/* Returns true if there is room to transmit a character immediatly. */
+/**
+ * Return non-zero if there is room to transmit a character immediately.
+ */
 int uart_tx_ready(void);
 
-/* Returns true if the UART has character available. */
+/**
+ * Return non-zero if the UART has a character available to read.
+ */
 int uart_rx_available(void);
 
 /**
- * Sends a character to the UART data register.
- * If the transmit FIFO is full, this function blocks until there is space.
+ * Send a character to the UART data register.
  *
- * c : byte to send.
+ * If the transmit FIFO is full, blocks until there is space.
+ *
+ * @param c		Character to send.
  */
 void uart_write_char(char c);
 
 /**
- * Reads and returns one char from the UART data register.
+ * Read one char from the UART data register.
  *
- * Called when uart_rx_available once returns true.
+ * @return		The character read.
  */
 int uart_read_char(void);
 
 /**
- * Disables all UART related IRQs.
+ * Disable all UART related IRQs.
  *
- * To avoid concurrent accesses on UART management variables.
+ * Used to avoid concurrent accesses on UART management variables.
  */
 void uart_disable_interrupt(void);
 
-/* Re-enables UART IRQs. */
+/**
+ * Re-enable UART IRQs.
+ */
 void uart_enable_interrupt(void);
 
 /**
- * Re-enables the UART transmit interrupt.
+ * Re-enable the UART transmit interrupt.
  *
- * It also forces triggering an interrupt if the hardware doesn't automatically
- * trigger it when the transmit buffer was filled beforehand.
+ * This also forces triggering an interrupt if the hardware doesn't
+ * automatically trigger it when the transmit buffer was filled beforehand.
  */
 void uart_tx_start(void);
 
-/* Disables the UART transmit interrupt. */
+/**
+ * Disable the UART transmit interrupt.
+ */
 void uart_tx_stop(void);
 
-/* Returns true if the UART transmit interrupt is disabled */
+/**
+ * Return non-zero if the UART transmit interrupt is disabled.
+ */
 int uart_tx_stopped(void);
 
 /**
  * Helper for UART processing.
- * Read the input FIFO until empty, then fill the output FIFO until the transmit
- * buffer is empty or the FIFO full.
+ *
+ * Reads the input FIFO until empty, then fills the output FIFO until the
+ * transmit buffer is empty or the FIFO full.
  *
  * Designed to be called from the driver interrupt handler.
  */
 void uart_process(void);
 
+/*
+ * COMx functions
+ */
 
-/*****************************************************************************/
-/* COMx functions */
-
-/* Enables comx interrupts */
+/**
+ * Enable COMx interrupts
+ */
 void uart_comx_enable(void);
 
-/* Returns non-zero if ok to put a character via uart_comx_putc(). */
+/**
+ * Return non-zero if ok to put a character via uart_comx_putc().
+ */
 int uart_comx_putc_ok(void);
 
-/* Puts a character to the COMx UART interface. */
+/**
+ * Write a character to the COMx UART interface.
+ */
 void uart_comx_putc(int c);
 
 #endif  /* __CROS_EC_UART_H */