Improve the SPI and external flash driver for the sensortag

For the SPI
* We improve the return semantics of _read() and _write()
* We set speed based on the value returned from ti_lib_sys_ctrl_clock_get() instead of using a hard-coded value

External flash changes:
* Rename macros to match instruction names
* verify_part(): Return a different value when the device is powered down and when communication fails
* Change return value semantics of static functions
* Adjust checks of board_spi_ return values
* Wait for BUSY to clear before attempting to send PD
* Accept two possible flash parts: W25X40CL (4MBit) as well as the W25X20CL (2MBit)

platform/srf06-cc26xx/sensortag/board-spi.c

@@ -43,8 +43,6 @@
 
 #include <stdbool.h>
 /*---------------------------------------------------------------------------*/
-#define CPU_FREQ      48000000ul
-/*---------------------------------------------------------------------------*/
 static bool
 accessible(void)
 {
@@ -62,11 +60,11 @@ accessible(void)
   return true;
 }
 /*---------------------------------------------------------------------------*/
-int
+bool
 board_spi_write(const uint8_t *buf, size_t len)
 {
   if(accessible() == false) {
-    return 0;
+    return false;
   }
 
   while(len > 0) {
@@ -78,14 +76,14 @@ board_spi_write(const uint8_t *buf, size_t len)
     buf++;
   }
 
-  return 0;
+  return true;
 }
 /*---------------------------------------------------------------------------*/
-int
+bool
 board_spi_read(uint8_t *buf, size_t len)
 {
   if(accessible() == false) {
-    return 0;
+    return false;
   }
 
   while(len > 0) {
@@ -93,14 +91,14 @@ board_spi_read(uint8_t *buf, size_t len)
 
     if(!ti_lib_rom_ssi_data_put_non_blocking(SSI0_BASE, 0)) {
       /* Error */
-      return -1;
+      return false;
     }
     ti_lib_rom_ssi_data_get(SSI0_BASE, &ul);
     *buf = (uint8_t)ul;
     len--;
     buf++;
   }
-  return 0;
+  return true;
 }
 /*---------------------------------------------------------------------------*/
 void
@@ -132,7 +130,8 @@ board_spi_open(uint32_t bit_rate, uint32_t clk_pin)
   /* SPI configuration */
   ti_lib_ssi_int_disable(SSI0_BASE, SSI_RXOR | SSI_RXFF | SSI_RXTO | SSI_TXFF);
   ti_lib_ssi_int_clear(SSI0_BASE, SSI_RXOR | SSI_RXTO);
-  ti_lib_rom_ssi_config_set_exp_clk(SSI0_BASE, CPU_FREQ, SSI_FRF_MOTO_MODE_0,
+  ti_lib_rom_ssi_config_set_exp_clk(SSI0_BASE, ti_lib_sys_ctrl_clock_get(),
+                                    SSI_FRF_MOTO_MODE_0,
                                     SSI_MODE_MASTER, bit_rate, 8);
   ti_lib_rom_ioc_pin_type_ssi_master(SSI0_BASE, BOARD_IOID_SPI_MISO,
                                      BOARD_IOID_SPI_MOSI, IOID_UNUSED, clk_pin);

platform/srf06-cc26xx/sensortag/board-spi.h

@@ -51,12 +51,21 @@
  * \param bit_rate The bit rate to use
  * \param clk_pin The IOID for the clock pin. This can be IOID_0 etc
  * \return none
+ *
+ * This function will make sure the peripheral is powered, clocked and
+ * initialised. A chain of calls to board_spi_read(), board_spi_write() and
+ * board_spi_flush() must be preceded by a call to this function. It is
+ * recommended to call board_spi_close() after such chain of calls.
  */
 void board_spi_open(uint32_t bit_rate, uint32_t clk_pin);
 
 /**
  * \brief Close the SPI interface
  * \return True when successful.
+ *
+ * This function will stop clocks to the SSI module and will set MISO, MOSI
+ * and CLK to a low leakage state. It is recommended to call this function
+ * after a chain of calls to board_spi_read() and board_spi_write()
  */
 void board_spi_close(void);
 
@@ -71,16 +80,22 @@ void board_spi_flush(void);
  * \param buf The buffer to store data
  * \param length The number of bytes to read
  * \return True when successful.
+ *
+ * Calls to this function must be preceded by a call to board_spi_open(). It is
+ * recommended to call board_spi_close() at the end of an operation.
  */
-int board_spi_read(uint8_t *buf, size_t length);
+bool board_spi_read(uint8_t *buf, size_t length);
 
 /**
  * \brief Write to an SPI device
  * \param buf The buffer with the data to write
  * \param length The number of bytes to write
  * \return True when successful.
+ *
+ * Calls to this function must be preceded by a call to board_spi_open(). It is
+ * recommended to call board_spi_close() at the end of an operation.
  */
-int board_spi_write(const uint8_t *buf, size_t length);
+bool board_spi_write(const uint8_t *buf, size_t length);
 /*---------------------------------------------------------------------------*/
 #endif /* BOARD_SPI_H_ */
 /*---------------------------------------------------------------------------*/

platform/srf06-cc26xx/sensortag/ext-flash.c

@@ -50,8 +50,8 @@
 #define BLS_CODE_SECTOR_ERASE     0x20 /**< Sector Erase */
 #define BLS_CODE_MDID             0x90 /**< Manufacturer Device ID */
 
-#define BLS_CODE_DP               0xB9 /**< Power down */
+#define BLS_CODE_PD               0xB9 /**< Power down */
-#define BLS_CODE_RDP              0xAB /**< Power standby */
+#define BLS_CODE_RPD              0xAB /**< Release Power-Down */
 /*---------------------------------------------------------------------------*/
 /* Erase instructions */
 
@@ -70,13 +70,18 @@
 #define BLS_STATUS_BIT_BUSY       0x01 /**< Busy bit of the status register */
 /*---------------------------------------------------------------------------*/
 /* Part specific constants */
+#define BLS_DEVICE_ID_W25X20CL    0x11
+#define BLS_DEVICE_ID_W25X40CL    0x12
 
 #define BLS_MANUFACTURER_ID       0xEF
-#define BLS_DEVICE_ID             0x12
 
 #define BLS_PROGRAM_PAGE_SIZE      256
 #define BLS_ERASE_SECTOR_SIZE     4096
 /*---------------------------------------------------------------------------*/
+#define VERIFY_PART_ERROR           -1
+#define VERIFY_PART_POWERED_DOWN     0
+#define VERIFY_PART_OK               1
+/*---------------------------------------------------------------------------*/
 /**
  * Clear external flash CSN line
  */
@@ -97,11 +102,12 @@ deselect(void)
 /*---------------------------------------------------------------------------*/
 /**
  * \brief Wait till previous erase/program operation completes.
- * \return Zero when successful.
+ * \return True when successful.
  */
-static int
+static bool
 wait_ready(void)
 {
+  bool ret;
   const uint8_t wbuf[1] = { BLS_CODE_READ_STATUS };
 
   select();
@@ -109,11 +115,11 @@ wait_ready(void)
   /* Throw away all garbages */
   board_spi_flush();
 
-  int ret = board_spi_write(wbuf, sizeof(wbuf));
+  ret = board_spi_write(wbuf, sizeof(wbuf));
 
-  if(ret) {
+  if(ret == false) {
     deselect();
-    return -2;
+    return false;
   }
 
   for(;;) {
@@ -125,10 +131,10 @@ wait_ready(void)
      */
     ret = board_spi_read(&buf, sizeof(buf));
 
-    if(ret) {
+    if(ret == false) {
       /* Error */
       deselect();
-      return -2;
+      return false;
     }
     if(!(buf & BLS_STATUS_BIT_BUSY)) {
       /* Now ready */
@@ -136,36 +142,44 @@ wait_ready(void)
     }
   }
   deselect();
-  return 0;
+  return true;
 }
 /*---------------------------------------------------------------------------*/
 /**
  * \brief Verify the flash part.
- * \return True when successful.
+ * \retval VERIFY_PART_OK The part was identified successfully
+ * \retval VERIFY_PART_ERROR There was an error communicating with the part
+ * \retval VERIFY_PART_POWERED_DOWN Communication was successful, but the part
+ *         was powered down
  */
-static bool
+static uint8_t
 verify_part(void)
 {
   const uint8_t wbuf[] = { BLS_CODE_MDID, 0xFF, 0xFF, 0x00 };
-  uint8_t rbuf[2];
+  uint8_t rbuf[2] = {0, 0};
-  int ret;
+  bool ret;
 
   select();
 
   ret = board_spi_write(wbuf, sizeof(wbuf));
 
-  if(ret) {
+  if(ret == false) {
     deselect();
-    return false;
+    return VERIFY_PART_ERROR;
   }
 
   ret = board_spi_read(rbuf, sizeof(rbuf));
   deselect();
 
-  if(ret || rbuf[0] != BLS_MANUFACTURER_ID || rbuf[1] != BLS_DEVICE_ID) {
+  if(ret == false) {
-    return false;
+    return VERIFY_PART_ERROR;
   }
-  return true;
+
+  if(rbuf[0] != BLS_MANUFACTURER_ID ||
+     (rbuf[1] != BLS_DEVICE_ID_W25X20CL && rbuf[1] != BLS_DEVICE_ID_W25X40CL)) {
+    return VERIFY_PART_POWERED_DOWN;
+  }
+  return VERIFY_PART_OK;
 }
 /*---------------------------------------------------------------------------*/
 /**
@@ -178,15 +192,21 @@ power_down(void)
   uint8_t cmd;
   uint8_t i;
 
-  cmd = BLS_CODE_DP;
+  /* First, wait for the device to be ready */
+  if(wait_ready() == false) {
+    /* Entering here will leave the device in standby instead of powerdown */
+    return;
+  }
+
+  cmd = BLS_CODE_PD;
   select();
   board_spi_write(&cmd, sizeof(cmd));
   deselect();
 
   i = 0;
   while(i < 10) {
-    if(!verify_part()) {
+    if(verify_part() == VERIFY_PART_POWERED_DOWN) {
-      /* Verify Part failed: Device is powered down */
+      /* Device is powered down */
       return;
     }
     i++;
@@ -206,12 +226,12 @@ power_standby(void)
   uint8_t cmd;
   bool success;
 
-  cmd = BLS_CODE_RDP;
+  cmd = BLS_CODE_RPD;
   select();
   success = board_spi_write(&cmd, sizeof(cmd));
 
   if(success) {
-    success = wait_ready() == 0;
+    success = wait_ready() == true ? true : false;
   }
 
   deselect();
@@ -221,21 +241,22 @@ power_standby(void)
 /*---------------------------------------------------------------------------*/
 /**
  * \brief Enable write.
- * \return Zero when successful.
+ * \return True when successful.
  */
-static int
+static bool
 write_enable(void)
 {
+  bool ret;
   const uint8_t wbuf[] = { BLS_CODE_WRITE_ENABLE };
 
   select();
-  int ret = board_spi_write(wbuf, sizeof(wbuf));
+  ret = board_spi_write(wbuf, sizeof(wbuf));
   deselect();
 
-  if(ret) {
+  if(ret == false) {
-    return -3;
+    return false;
   }
-  return 0;
+  return true;
 }
 /*---------------------------------------------------------------------------*/
 bool
@@ -252,7 +273,7 @@ ext_flash_open()
   /* Put the part is standby mode */
   power_standby();
 
-  return verify_part();
+  return verify_part() == VERIFY_PART_OK ? true : false;
 }
 /*---------------------------------------------------------------------------*/
 void
@@ -270,8 +291,8 @@ ext_flash_read(size_t offset, size_t length, uint8_t *buf)
   uint8_t wbuf[4];
 
   /* Wait till previous erase/program operation completes */
-  int ret = wait_ready();
+  bool ret = wait_ready();
-  if(ret) {
+  if(ret == false) {
     return false;
   }
 
@@ -286,7 +307,7 @@ ext_flash_read(size_t offset, size_t length, uint8_t *buf)
 
   select();
 
-  if(board_spi_write(wbuf, sizeof(wbuf))) {
+  if(board_spi_write(wbuf, sizeof(wbuf)) == false) {
     /* failure */
     deselect();
     return false;
@@ -296,25 +317,25 @@ ext_flash_read(size_t offset, size_t length, uint8_t *buf)
 
   deselect();
 
-  return ret == 0;
+  return ret;
 }
 /*---------------------------------------------------------------------------*/
 bool
 ext_flash_write(size_t offset, size_t length, const uint8_t *buf)
 {
   uint8_t wbuf[4];
-  int ret;
+  bool ret;
   size_t ilen; /* interim length per instruction */
 
   while(length > 0) {
     /* Wait till previous erase/program operation completes */
     ret = wait_ready();
-    if(ret) {
+    if(ret == false) {
       return false;
     }
 
     ret = write_enable();
-    if(ret) {
+    if(ret == false) {
       return false;
     }
 
@@ -338,13 +359,13 @@ ext_flash_write(size_t offset, size_t length, const uint8_t *buf)
      * as much. */
     select();
 
-    if(board_spi_write(wbuf, sizeof(wbuf))) {
+    if(board_spi_write(wbuf, sizeof(wbuf)) == false) {
       /* failure */
       deselect();
       return false;
     }
 
-    if(board_spi_write(buf, ilen)) {
+    if(board_spi_write(buf, ilen) == false) {
       /* failure */
       deselect();
       return false;
@@ -365,6 +386,7 @@ ext_flash_erase(size_t offset, size_t length)
    * sector erase is used blindly.
    */
   uint8_t wbuf[4];
+  bool ret;
   size_t i, numsectors;
   size_t endoffset = offset + length - 1;
 
@@ -375,13 +397,13 @@ ext_flash_erase(size_t offset, size_t length)
 
   for(i = 0; i < numsectors; i++) {
     /* Wait till previous erase/program operation completes */
-    int ret = wait_ready();
+    ret = wait_ready();
-    if(ret) {
+    if(ret == false) {
       return false;
     }
 
     ret = write_enable();
-    if(ret) {
+    if(ret == false) {
       return false;
     }
 
@@ -391,7 +413,7 @@ ext_flash_erase(size_t offset, size_t length)
 
     select();
 
-    if(board_spi_write(wbuf, sizeof(wbuf))) {
+    if(board_spi_write(wbuf, sizeof(wbuf)) == false) {
       /* failure */
       deselect();
       return false;