Improve UART power-cycling logic:

* Only enable TX by default.
* Add some magic for RX handling. When an input handler is registered:
  * Automatically enable RX-related and interrupts
  * Automatically lock the SERIAL PD on under all power modes
  * Automatically enable the UART clock under sleep and deep sleep
  * Automatically undo all of the above when the input handler becomes NULL
  * As a result, modules / examples that need UART RX no longer need to clock the UART and manipulate the SERIAL PD. They simply have to specify an input handler
* Don't automatically power on the UART whenever the CM3 is active
* Before accessing the UART, make sure it is powered and clocked
* Avoid falling edge glitches
* Fix garbage characters / Explicitly wait for UART TX to complete

cpu/cc26xx/dev/cc26xx-uart.c

@@ -40,6 +40,7 @@
 #include "sys/energest.h"
 
 #include <stdint.h>
+#include <stdbool.h>
 /*---------------------------------------------------------------------------*/
 /* Which events to trigger a UART interrupt */
 #define CC26XX_UART_RX_INTERRUPT_TRIGGERS (UART_INT_RX | UART_INT_RT)
@@ -53,35 +54,98 @@
 /*---------------------------------------------------------------------------*/
 static int (*input_handler)(unsigned char c);
 /*---------------------------------------------------------------------------*/
-static void
+static bool
-power_domain_on(void)
+usable(void)
 {
+  if(BOARD_IOID_UART_RX == IOID_UNUSED ||
+     BOARD_IOID_UART_TX == IOID_UNUSED ||
+     CC26XX_UART_CONF_ENABLE == 0) {
+    return false;
+  }
+
+  return true;
+}
+/*---------------------------------------------------------------------------*/
+static void
+power_and_clock(void)
+{
+  /* Power on the SERIAL PD */
   ti_lib_prcm_power_domain_on(PRCM_DOMAIN_SERIAL);
   while(ti_lib_prcm_power_domain_status(PRCM_DOMAIN_SERIAL)
         != PRCM_DOMAIN_POWER_ON);
+
+  /* Enable UART clock in active mode */
+  ti_lib_prcm_peripheral_run_enable(PRCM_PERIPH_UART0);
+  ti_lib_prcm_load_set();
+  while(!ti_lib_prcm_load_get());
+}
+/*---------------------------------------------------------------------------*/
+/*
+ * Returns 0 if either the SERIAL PD is off, or the PD is on but the run mode
+ * clock is gated. If this function would return 0, accessing UART registers
+ * will return a precise bus fault. If this function returns 1, it is safe to
+ * access UART registers.
+ *
+ * This function only checks the 'run mode' clock gate, since it can only ever
+ * be called with the MCU in run mode.
+ */
+static bool
+accessible(void)
+{
+  /* First, check the PD */
+  if(ti_lib_prcm_power_domain_status(PRCM_DOMAIN_SERIAL)
+     != PRCM_DOMAIN_POWER_ON) {
+    return false;
+  }
+
+  /* Then check the 'run mode' clock gate */
+  if(!(HWREG(PRCM_BASE + PRCM_O_UARTCLKGR) & PRCM_UARTCLKGR_CLK_EN)) {
+    return false;
+  }
+
+  return true;
 }
 /*---------------------------------------------------------------------------*/
 static void
-configure_baud_rate(void)
+disable_interrupts(void)
 {
+  /* Acknowledge UART interrupts */
+  ti_lib_int_disable(INT_UART0);
+
+  /* Disable all UART module interrupts */
+  ti_lib_uart_int_disable(UART0_BASE, CC26XX_UART_INTERRUPT_ALL);
+
+  /* Clear all UART interrupts */
+  ti_lib_uart_int_clear(UART0_BASE, CC26XX_UART_INTERRUPT_ALL);
+}
+/*---------------------------------------------------------------------------*/
+static void
+enable_interrupts(void)
+{
+  /* Clear all UART interrupts */
+  ti_lib_uart_int_clear(UART0_BASE, CC26XX_UART_INTERRUPT_ALL);
+
+  /* Enable RX-related interrupts only if we have an input handler */
+  if(input_handler) {
+    /* Configure which interrupts to generate: FIFO level or after RX timeout */
+    ti_lib_uart_int_enable(UART0_BASE, CC26XX_UART_RX_INTERRUPT_TRIGGERS);
+
+    /* Acknowledge UART interrupts */
+    ti_lib_int_enable(INT_UART0);
+  }
+}
+/*---------------------------------------------------------------------------*/
+static void
+configure(void)
+{
+  uint32_t ctl_val = UART_CTL_UARTEN | UART_CTL_TXE;
   /*
-   * Configure the UART for 115,200, 8-N-1 operation.
+   * Make sure the TX pin is output / high before assigning it to UART control
-   * This function uses SysCtrlClockGet() to get the system clock
+   * to avoid falling edge glitches
-   * frequency. This could be also be a variable or hard coded value
-   * instead of a function call.
    */
-  ti_lib_uart_config_set_exp_clk(UART0_BASE,
+  ti_lib_ioc_pin_type_gpio_output(BOARD_IOID_UART_TX);
-                                 ti_lib_sys_ctrl_peripheral_clock_get(
+  ti_lib_gpio_pin_write(BOARD_UART_TX, 1);
-                                   PRCM_PERIPH_UART0,
+
-                                   SYSCTRL_SYSBUS_ON),
-                                 CC26XX_UART_CONF_BAUD_RATE,
-                                 (UART_CONFIG_WLEN_8 | UART_CONFIG_STOP_ONE |
-                                  UART_CONFIG_PAR_NONE));
-}
-/*---------------------------------------------------------------------------*/
-static void
-configure_registers(void)
-{
   /*
    * Map UART signals to the correct GPIO pins and configure them as
    * hardware controlled.
@@ -89,7 +153,14 @@ configure_registers(void)
   ti_lib_ioc_pin_type_uart(UART0_BASE, BOARD_IOID_UART_RX, BOARD_IOID_UART_TX,
                            BOARD_IOID_UART_CTS, BOARD_IOID_UART_RTS);
 
-  configure_baud_rate();
+  /* Configure the UART for 115,200, 8-N-1 operation. */
+  ti_lib_uart_config_set_exp_clk(UART0_BASE,
+                                 ti_lib_sys_ctrl_peripheral_clock_get(
+                                   PRCM_PERIPH_UART0,
+                                   SYSCTRL_SYSBUS_ON),
+                                 CC26XX_UART_CONF_BAUD_RATE,
+                                 (UART_CONFIG_WLEN_8 | UART_CONFIG_STOP_ONE |
+                                  UART_CONFIG_PAR_NONE));
 
   /*
    * Generate an RX interrupt at FIFO 1/2 full.
@@ -97,116 +168,138 @@ configure_registers(void)
    */
   ti_lib_uart_fifo_level_set(UART0_BASE, UART_FIFO_TX7_8, UART_FIFO_RX4_8);
 
-  /* Configure which interrupts to generate: FIFO level or after RX timeout */
+  /* Enable FIFOs */
-  ti_lib_uart_int_enable(UART0_BASE, CC26XX_UART_RX_INTERRUPT_TRIGGERS);
+  HWREG(UART0_BASE + UART_O_LCRH) |= UART_LCRH_FEN;
-}
-/*---------------------------------------------------------------------------*/
-static void
-uart_on(void)
-{
-  power_domain_on();
 
-  /* Configure baud rate and enable */
+  if(input_handler) {
-  if((HWREG(UART0_BASE + UART_O_CTL) & UART_CTL_UARTEN) == 0) {
+    ctl_val += UART_CTL_RXE;
-    configure_registers();
-
-    /* Enable UART */
-    ti_lib_uart_enable(UART0_BASE);
   }
-}
+
-/*---------------------------------------------------------------------------*/
+  /* Enable TX, RX (conditionally), and the UART. */
-static uint8_t
+  HWREG(UART0_BASE + UART_O_CTL) = ctl_val;
-lpm_permit_max_pm_handler(void)
-{
-  return LPM_MODE_MAX_SUPPORTED;
 }
 /*---------------------------------------------------------------------------*/
 static void
 lpm_drop_handler(uint8_t mode)
 {
-  /* Do nothing if the PD is off */
+  /*
-  if(ti_lib_prcm_power_domain_status(PRCM_DOMAIN_SERIAL)
+   * First, wait for any outstanding TX to complete. If we have an input
-     != PRCM_DOMAIN_POWER_ON) {
+   * handler, the SERIAL PD will be kept on and the UART module clock will
-    return;
+   * be enabled under sleep as well as deep sleep. In theory, this means that
+   * we shouldn't lose any outgoing bytes, but we actually do on occasion.
+   * This byte loss may (or may not) be related to the freezing of IO latches
+   * between MCU and AON when we drop to deep sleep. This here is essentially a
+   * workaround
+   */
+  if(accessible() == true) {
+    while(ti_lib_uart_busy(UART0_BASE));
   }
 
-  /* Wait for outstanding TX to complete */
-  while(ti_lib_uart_busy(UART0_BASE));
-
   /*
-   * Check our clock gate under Deep Sleep. If it's off, we can shut down. If
+   * If we have a registered input_handler then we need to retain RX
-   * it's on, this means that some other code module wants UART functionality
+   * capability. Thus, if this is not a shutdown notification and we have an
-   * during deep sleep, so we stay enabled
+   * input handler, we do nothing
    */
-  if((HWREG(PRCM_BASE + PRCM_O_UARTCLKGDS) & 1) == 0) {
+  if((mode != LPM_MODE_SHUTDOWN) && (input_handler != NULL)) {
-    ti_lib_ioc_pin_type_gpio_input(BOARD_IOID_UART_RX);
-    ti_lib_ioc_pin_type_gpio_input(BOARD_IOID_UART_TX);
-
-    ti_lib_uart_disable(UART0_BASE);
-  }
-}
-/*---------------------------------------------------------------------------*/
-static void
-lpm_wakeup_handler(void)
-{
-  uart_on();
-}
-/*---------------------------------------------------------------------------*/
-/* Declare a data structure to register with LPM. */
-LPM_MODULE(uart_module, lpm_permit_max_pm_handler,
-           lpm_drop_handler, lpm_wakeup_handler);
-/*---------------------------------------------------------------------------*/
-void
-cc26xx_uart_init()
-{
-  /* Exit without initialising if ports are misconfigured */
-  if(BOARD_IOID_UART_RX == IOID_UNUSED ||
-     BOARD_IOID_UART_TX == IOID_UNUSED) {
     return;
   }
 
-  /* Enable the serial domain and wait for domain to be on */
+  /*
-  power_domain_on();
+   * If we reach here, we either don't care about staying awake or we have
+   * received a shutdown notification
+   *
+   * Only touch UART registers if the module is powered and clocked
+   */
+  if(accessible() == true) {
+    /* Disable the module */
+    ti_lib_uart_disable(UART0_BASE);
 
-  /* Enable the UART clock when running and sleeping */
+    /* Disable all UART interrupts and clear all flags */
-  ti_lib_prcm_peripheral_run_enable(PRCM_PERIPH_UART0);
+    disable_interrupts();
+  }
 
-  /* Apply clock settings and wait for them to take effect */
+  /*
+   * Always stop the clock in run mode. Also stop in Sleep and Deep Sleep if
+   * this is a request for full shutdown
+   */
+  ti_lib_prcm_peripheral_run_disable(PRCM_PERIPH_UART0);
+  if(mode == LPM_MODE_SHUTDOWN) {
+    ti_lib_prcm_peripheral_sleep_disable(PRCM_PERIPH_UART0);
+    ti_lib_prcm_peripheral_deep_sleep_disable(PRCM_PERIPH_UART0);
+  }
   ti_lib_prcm_load_set();
   while(!ti_lib_prcm_load_get());
 
-  /* Disable Interrupts */
+  /* Set pins to low leakage configuration in preparation for deep sleep */
-  ti_lib_int_master_disable();
+  lpm_pin_set_default_state(BOARD_IOID_UART_TX);
+  lpm_pin_set_default_state(BOARD_IOID_UART_RX);
+  lpm_pin_set_default_state(BOARD_IOID_UART_CTS);
+  lpm_pin_set_default_state(BOARD_IOID_UART_RTS);
+}
+/*---------------------------------------------------------------------------*/
+/* Declare a data structure to register with LPM. */
+LPM_MODULE(uart_module, NULL, lpm_drop_handler, NULL, LPM_DOMAIN_NONE);
+/*---------------------------------------------------------------------------*/
+static void
+enable(void)
+{
+  power_and_clock();
 
   /* Make sure the peripheral is disabled */
   ti_lib_uart_disable(UART0_BASE);
 
-  /* Disable all UART module interrupts */
+  /* Disable all UART interrupts and clear all flags */
-  ti_lib_uart_int_disable(UART0_BASE, CC26XX_UART_INTERRUPT_ALL);
+  disable_interrupts();
 
-  configure_registers();
+  /* Setup pins, Baud rate and FIFO levels */
+  configure();
 
-  /* Acknowledge UART interrupts */
+  /* Enable UART interrupts */
-  ti_lib_int_enable(INT_UART0);
+  enable_interrupts();
+}
+/*---------------------------------------------------------------------------*/
+void
+cc26xx_uart_init()
+{
+  bool interrupts_disabled;
 
-  /* Re-enable processor interrupts */
+  /* Return early if disabled by user conf or if ports are misconfigured */
-  ti_lib_int_master_enable();
+  if(usable() == false) {
+    return;
+  }
 
-  /* Enable UART */
+  /* Disable Interrupts */
-  ti_lib_uart_enable(UART0_BASE);
+  interrupts_disabled = ti_lib_int_master_disable();
 
   /* Register ourselves with the LPM module */
   lpm_register_module(&uart_module);
+
+  /* Only TX and EN to start with. RX will be enabled only if needed */
+  input_handler = NULL;
+
+  /*
+   * init() won't actually fire up the UART. We turn it on only when (and if)
+   * it gets requested, either to enable input or to send out a character
+   *
+   * Thus, we simply re-enable processor interrupts here
+   */
+  if(!interrupts_disabled) {
+    ti_lib_int_master_enable();
+  }
 }
 /*---------------------------------------------------------------------------*/
 void
 cc26xx_uart_write_byte(uint8_t c)
 {
-  if(ti_lib_prcm_power_domain_status(PRCM_DOMAIN_SERIAL)
+  /* Return early if disabled by user conf or if ports are misconfigured */
-     != PRCM_DOMAIN_POWER_ON) {
+  if(usable() == false) {
     return;
   }
 
+  if(accessible() == false) {
+    enable();
+  }
+
   ti_lib_uart_char_put(UART0_BASE, c);
 }
 /*---------------------------------------------------------------------------*/
@@ -214,9 +307,52 @@ void
 cc26xx_uart_set_input(int (*input)(unsigned char c))
 {
   input_handler = input;
+
+  /* Return early if disabled by user conf or if ports are misconfigured */
+  if(usable() == false) {
+    return;
+  }
+
+  if(input == NULL) {
+    /* Let the SERIAL PD power down */
+    uart_module.domain_lock = LPM_DOMAIN_NONE;
+
+    /* Disable module clocks under sleep and deep sleep */
+    ti_lib_prcm_peripheral_sleep_disable(PRCM_PERIPH_UART0);
+    ti_lib_prcm_peripheral_deep_sleep_disable(PRCM_PERIPH_UART0);
+  } else {
+    /* Request the SERIAL PD to stay on during deep sleep */
+    uart_module.domain_lock = LPM_DOMAIN_SERIAL;
+
+    /* Enable module clocks under sleep and deep sleep */
+    ti_lib_prcm_peripheral_sleep_enable(PRCM_PERIPH_UART0);
+    ti_lib_prcm_peripheral_deep_sleep_enable(PRCM_PERIPH_UART0);
+  }
+
+  ti_lib_prcm_load_set();
+  while(!ti_lib_prcm_load_get());
+
+  enable();
+
   return;
 }
 /*---------------------------------------------------------------------------*/
+uint8_t
+cc26xx_uart_busy(void)
+{
+  /* Return early if disabled by user conf or if ports are misconfigured */
+  if(usable() == false) {
+    return UART_IDLE;
+  }
+
+  /* If the UART is not accessible, it is not busy */
+  if(accessible() == false) {
+    return UART_IDLE;
+  }
+
+  return ti_lib_uart_busy(UART0_BASE);
+}
+/*---------------------------------------------------------------------------*/
 void
 cc26xx_uart_isr(void)
 {
@@ -225,6 +361,8 @@ cc26xx_uart_isr(void)
 
   ENERGEST_ON(ENERGEST_TYPE_IRQ);
 
+  power_and_clock();
+
   /* Read out the masked interrupt status */
   flags = ti_lib_uart_int_status(UART0_BASE, true);
 

cpu/cc26xx/dev/cc26xx-uart.h

@@ -63,9 +63,32 @@ void cc26xx_uart_write_byte(uint8_t b);
 /**
  * \brief Assigns a callback to be called when the UART receives a byte
  * \param input A pointer to the function
+ *
+ * If \e input is NULL, the UART driver will assume that RX functionality is
+ * not required and it will be disabled. It will also disable the module's
+ * clocks under sleep and deep sleep and allow the SERIAL PD to be powered off.
+ *
+ * If \e input is not NULL, the UART driver will assume that RX is in fact
+ * required and it will be enabled. The module's clocks will be enabled under
+ * sleep and deep sleep and the driver will not allow the SERIAL PD to turn
+ * off during deep sleep, so that the UART can still receive bytes.
+ *
+ * \note This has a significant impact on overall energy consumption, so you
+ * should only enabled UART RX input when it's actually required.
  */
 void cc26xx_uart_set_input(int (*input)(unsigned char c));
 
+/**
+ * \brief Returns the UART busy status
+ * \return UART_IDLE or UART_BUSY
+ *
+ * ti_lib_uart_busy() will access UART registers. It is our responsibility
+ * to first make sure the UART is accessible before calling it. Hence this
+ * wrapper.
+ *
+ * Return values are defined in CC26xxware's uart.h
+ */
+uint8_t cc26xx_uart_busy(void);
 /** @} */
 /*---------------------------------------------------------------------------*/
 #endif /* CC26XX_UART_H_ */

cpu/cc26xx/putchar.c

@@ -29,6 +29,7 @@
  */
 /*---------------------------------------------------------------------------*/
 #include "cc26xx-uart.h"
+#include "ti-lib.h"
 
 #include <string.h>
 /*---------------------------------------------------------------------------*/
@@ -47,9 +48,16 @@ puts(const char *str)
     return 0;
   }
   for(i = 0; i < strlen(str); i++) {
-    putchar(str[i]);
+    cc26xx_uart_write_byte(str[i]);
   }
-  putchar('\n');
+  cc26xx_uart_write_byte('\n');
+
+  /*
+   * Wait for the line to go out. This is to prevent garbage when used between
+   * UART on/off cycles
+   */
+  while(cc26xx_uart_busy() == UART_BUSY);
+
   return i;
 }
 /*---------------------------------------------------------------------------*/
@@ -62,9 +70,16 @@ dbg_send_bytes(const unsigned char *s, unsigned int len)
     if(i >= len) {
       break;
     }
-    putchar(*s++);
+    cc26xx_uart_write_byte(*s++);
     i++;
   }
+
+  /*
+   * Wait for the buffer to go out. This is to prevent garbage when used
+   * between UART on/off cycles
+   */
+  while(cc26xx_uart_busy() == UART_BUSY);
+
   return i;
 }
 /*---------------------------------------------------------------------------*/