Merge pull request #11960 from fjmolinas/pr_msp430_flashpage_raw

cpu/msp430_common: add flashpage_raw

cpu/msp430_common/Makefile.features

@@ -1,4 +1,5 @@
 FEATURES_PROVIDED += arch_16bit
 FEATURES_PROVIDED += arch_msp430
 FEATURES_PROVIDED += periph_flashpage
+FEATURES_PROVIDED += periph_flashpage_raw
 FEATURES_PROVIDED += periph_pm

cpu/msp430_common/include/cpu_conf.h

@@ -26,21 +26,28 @@ extern "C" {
  * @name   Configure the internal flash memory
  * @{
  */
-#define FLASHPAGE_SIZE      (512)
+#define FLASHPAGE_SIZE      (512U)
 
 #if defined (CPU_MODEL_MSP430F1611)
 #define CPU_FLASH_BASE      (0x4000)
-#define FLASHPAGE_NUMOF     (96)        /* 48K */
+#define FLASHPAGE_NUMOF     (96U)        /* 48K */
 #elif defined (CPU_MODEL_MSP430F1612)
-#define CPU_FLASH_BASE      (0x2600)
+#define CPU_FLASH_BASE      (0x2600)    /* first sector is only 256 byte, skip it*/
-#define FLASHPAGE_NUMOF     (110)       /* 56K */
+#define FLASHPAGE_NUMOF     (109U)      /* 54.5K */
 #elif defined (CPU_MODEL_MSP430F2617)
-#define CPU_FLASH_BASE      (0x3100)
+#define CPU_FLASH_BASE      (0x3200)    /* first sector is only 256 byte, skip it*/
-#define FLASHPAGE_NUMOF     (128)       /* we can currently only access 52K */
+#define FLASHPAGE_NUMOF     (103U)      /* we can currently only access 51.5K */
 #elif defined (CPU_MODEL_CC430F6137)
 #define CPU_FLASH_BASE      (0x8000)
-#define FLASHPAGE_NUMOF     (64)        /* 32K */
+#define FLASHPAGE_NUMOF     (64U)        /* 32K */
 #endif
+
+/* The minimum block size which can be written is 1B. However, the erase
+ * block is always FLASHPAGE_SIZE.
+ */
+#define FLASHPAGE_RAW_BLOCKSIZE    (1U)
+/* Writing should be always 2 byte aligned */
+#define FLASHPAGE_RAW_ALIGNMENT    (2U)
 /** @} */
 
 /**

cpu/msp430_common/periph/flashpage.c

@@ -24,34 +24,79 @@
 #include "irq.h"
 #include "periph/flashpage.h"
 
-void flashpage_write(int page, const void *data)
+static inline int _unlock(void)
 {
-    assert(page < FLASHPAGE_NUMOF);
+    int state;
-
+    state = irq_disable();
-    const uint8_t *src = data;
-    uint8_t *dst = (uint8_t *)flashpage_addr(page);
-    unsigned istate;
-
-    /* disable interrupts and unlock flash */
-    istate = irq_disable();
     FCTL3 = FWKEY;
     while (FCTL3 & BUSY) {}
+    return state;
+}
+
+static inline void _lock(int state)
+{
+    FCTL3 = (FWKEY | LOCK);
+    irq_restore(state);
+}
+
+static inline void _erase(uint16_t *page_addr)
+{
+    /* disable interrupts and unlock flash */
+    int state = _unlock();
 
     /* erase page */
     FCTL1 = (FWKEY | ERASE);
-    *dst = 0;                   /* erases the page */
+    *page_addr = 0;
     while (FCTL3 & BUSY) {}
 
-    if (data) {
+    /* lock flash and re-enable interrupts */
-        FCTL1 = (FWKEY | WRT);
+    _lock(state);
-        for (unsigned i = 0; i < FLASHPAGE_SIZE; i++) {
+}
-            *(dst++) = *(src++);
+
-            while (!(FCTL3 & WAIT)) {}
+void flashpage_write_raw(void *target_addr, const void *data, size_t len)
-        }
+{
+    /* assert multiples of FLASHPAGE_RAW_BLOCKSIZE are written and no less of
+       that length. */
+    assert(!(len % FLASHPAGE_RAW_BLOCKSIZE));
+
+    /* ensure writes are aligned */
+    assert(!(((unsigned)target_addr % FLASHPAGE_RAW_ALIGNMENT) ||
+            ((unsigned)data % FLASHPAGE_RAW_ALIGNMENT)));
+
+    /* ensure the length doesn't exceed the actual flash size */
+    assert(((unsigned)target_addr + len) <
+           (CPU_FLASH_BASE + (FLASHPAGE_SIZE * FLASHPAGE_NUMOF) - 1));
+
+    uint8_t *page_addr = target_addr;
+    const uint8_t *data_addr = data;
+
+   /* disable interrupts and unlock flash */
+    int state = _unlock();
+
+   /* enable write access, and write*/
+    FCTL1 = (FWKEY | WRT);
+    for (unsigned i = 0; i < len; i++) {
+        *(page_addr++) = *(data_addr++);
+        while (!(FCTL3 & WAIT)) {}
     }
+   /* disable write access */
+    FCTL1 = (FWKEY);
 
     /* lock flash and re-enable interrupts */
-    FCTL1 = (FWKEY);
+    _lock(state);
-    FCTL3 = (FWKEY | LOCK);
+}
-    irq_restore(istate);
+
+void flashpage_write(int page, const void *data)
+{
+    assert((unsigned) page < FLASHPAGE_NUMOF);
+
+    uint16_t *page_addr = (uint16_t *)flashpage_addr(page);
+
+    /* erase page */
+    _erase(page_addr);
+
+    /* write page */
+    if (data != NULL) {
+        flashpage_write_raw(page_addr, data, FLASHPAGE_SIZE);
+    }
 }

drivers/mtd_flashpage/mtd_flashpage.c

@@ -27,7 +27,7 @@
 #include "mtd_flashpage.h"
 #include "periph/flashpage.h"
 
-#define MTD_FLASHPAGE_END_ADDR     CPU_FLASH_BASE + (FLASHPAGE_NUMOF * FLASHPAGE_SIZE)
+#define MTD_FLASHPAGE_END_ADDR     ((uint32_t) CPU_FLASH_BASE + (FLASHPAGE_NUMOF * FLASHPAGE_SIZE))
 
 static int _init(mtd_dev_t *dev)
 {
@@ -39,13 +39,20 @@ static int _init(mtd_dev_t *dev)
 static int _read(mtd_dev_t *dev, void *buf, uint32_t addr, uint32_t size)
 {
     assert(addr < MTD_FLASHPAGE_END_ADDR);
+
     (void)dev;
 
     if (addr % FLASHPAGE_RAW_ALIGNMENT) {
         return -EINVAL;
     }
 
-    memcpy(buf, (void*)addr, size);
+#if (__SIZEOF_POINTER__ == 2)
+    uint16_t dst_addr = addr;
+#else
+    uint32_t dst_addr = addr;
+#endif
+
+    memcpy(buf, (void *)dst_addr, size);
 
     return size;
 }
@@ -53,6 +60,7 @@ static int _read(mtd_dev_t *dev, void *buf, uint32_t addr, uint32_t size)
 static int _write(mtd_dev_t *dev, const void *buf, uint32_t addr, uint32_t size)
 {
     (void)dev;
+
     if (addr % FLASHPAGE_RAW_ALIGNMENT) {
         return -EINVAL;
     }
@@ -65,7 +73,14 @@ static int _write(mtd_dev_t *dev, const void *buf, uint32_t addr, uint32_t size)
     if (addr + size > MTD_FLASHPAGE_END_ADDR) {
         return -EOVERFLOW;
     }
-    flashpage_write_raw((void *)addr, buf, size);
+
+#if (__SIZEOF_POINTER__ == 2)
+    uint16_t dst_addr = addr;
+#else
+    uint32_t dst_addr = addr;
+#endif
+
+    flashpage_write_raw((void *)dst_addr, buf, size);
 
     return size;
 }
@@ -78,13 +93,20 @@ int _erase(mtd_dev_t *dev, uint32_t addr, uint32_t size)
         return -EOVERFLOW;
     }
     if (addr + size > MTD_FLASHPAGE_END_ADDR) {
-        return - EOVERFLOW;
+        return -EOVERFLOW;
     }
     if (addr % sector_size) {
-        return - EOVERFLOW;
+        return -EOVERFLOW;
     }
+
+#if (__SIZEOF_POINTER__ == 2)
+    uint16_t dst_addr = addr;
+#else
+    uint32_t dst_addr = addr;
+#endif
+
     for (size_t i = 0; i < size; i += sector_size) {
-        flashpage_write(flashpage_page((void *)addr), NULL);
+        flashpage_write(flashpage_page((void *)dst_addr), NULL);
     }
 
     return 0;

tests/mtd_flashpage/main.c

@@ -19,8 +19,22 @@
 #include "mtd.h"
 #include "mtd_flashpage.h"
 
-#define TEST_ADDRESS1       (uint32_t)flashpage_addr(FLASHPAGE_NUMOF - 1)
+/* For MSP430 cpu's the last page holds the interrupt vector, although the api
-#define TEST_ADDRESS2       (uint32_t)flashpage_addr(FLASHPAGE_NUMOF - 2)
+   should not limit erasing that page, we don't want to break when testing */
+#if defined(CPU_CC430) || defined(CPU_MSP430FXYZ)
+#define LAST_AVAILABLE_PAGE    (FLASHPAGE_NUMOF - 2)
+#else
+#define LAST_AVAILABLE_PAGE    (FLASHPAGE_NUMOF - 1)
+#endif
+
+#if (__SIZEOF_POINTER__ == 2)
+#define TEST_ADDRESS1       (uint16_t)flashpage_addr(LAST_AVAILABLE_PAGE)
+#define TEST_ADDRESS2       (uint16_t)flashpage_addr(LAST_AVAILABLE_PAGE - 1)
+#else
+#define TEST_ADDRESS1       (uint32_t)flashpage_addr(LAST_AVAILABLE_PAGE)
+#define TEST_ADDRESS2       (uint32_t)flashpage_addr(LAST_AVAILABLE_PAGE - 1)
+#endif
+#define TEST_ADDRESS0       (FLASHPAGE_NUMOF - 1)
 
 static mtd_dev_t _dev = MTD_FLASHPAGE_INIT_VAL(8);
 static mtd_dev_t *dev = &_dev;
@@ -59,13 +73,13 @@ static void test_mtd_erase(void)
     ret = mtd_erase(dev, TEST_ADDRESS1 + dev->page_size, dev->page_size);
     TEST_ASSERT_EQUAL_INT(-EOVERFLOW, ret);
 
-    /* Erase 2 last sectors */
+    /* Erase 2 last available pages */
     ret = mtd_erase(dev, TEST_ADDRESS2,
                     FLASHPAGE_SIZE * 2);
     TEST_ASSERT_EQUAL_INT(0, ret);
 
     /* Erase out of memory area */
-    ret = mtd_erase(dev, TEST_ADDRESS1,
+    ret = mtd_erase(dev, TEST_ADDRESS0,
                     FLASHPAGE_SIZE * 2);
     TEST_ASSERT_EQUAL_INT(-EOVERFLOW, ret);
 }

tests/periph_flashpage/main.c

@@ -28,6 +28,14 @@
 
 #define LINE_LEN            (16)
 
+/* For MSP430 cpu's the last page holds the interrupt vector, although the api
+   should not limit erasing that page, we don't want to break when testing */
+#if defined(CPU_CC430) || defined(CPU_MSP430FXYZ)
+#define TEST_LAST_AVAILABLE_PAGE    (FLASHPAGE_NUMOF - 2)
+#else
+#define TEST_LAST_AVAILABLE_PAGE    (FLASHPAGE_NUMOF - 1)
+#endif
+
 /* When writing raw bytes on flash, data must be correctly aligned. */
 #ifdef MODULE_PERIPH_FLASHPAGE_RAW
 #define ALIGNMENT_ATTR __attribute__ ((aligned (FLASHPAGE_RAW_ALIGNMENT)))
@@ -196,22 +204,33 @@ static uint32_t getaddr(const char *str)
 
 static int cmd_write_raw(int argc, char **argv)
 {
+#if (__SIZEOF_POINTER__ == 2)
+    uint16_t addr;
+#else
     uint32_t addr;
+#endif
 
     if (argc < 3) {
         printf("usage: %s <addr> <data>\n", argv[0]);
         return 1;
     }
 
+#if (__SIZEOF_POINTER__ == 2)
+    addr = (uint16_t) getaddr(argv[1]);
+#else
     addr = getaddr(argv[1]);
-
+#endif
     /* try to align */
     memcpy(raw_buf, argv[2], strlen(argv[2]));
 
     flashpage_write_raw((void*)addr, raw_buf, strlen(raw_buf));
-
+#if (__SIZEOF_POINTER__ == 2)
+    printf("wrote local data to flash address %#" PRIx16 " of len %u\n",
+           addr, strlen(raw_buf));
+#else
     printf("wrote local data to flash address %#" PRIx32 " of len %u\n",
            addr, strlen(raw_buf));
+#endif
     return 0;
 }
 #endif
@@ -313,8 +332,10 @@ static int cmd_test_last(int argc, char **argv)
             fill = 'a';
         }
     }
-
+#if defined(CPU_CC430) || defined(CPU_MSP430FXYZ)
-    if (flashpage_write_and_verify((int)FLASHPAGE_NUMOF - 1, page_mem) != FLASHPAGE_OK) {
+    printf("The last page holds the ISR vector, so test page %d\n", TEST_LAST_AVAILABLE_PAGE);
+#endif
+    if (flashpage_write_and_verify(TEST_LAST_AVAILABLE_PAGE, page_mem) != FLASHPAGE_OK) {
         puts("error verifying the content of last page");
         return 1;
     }
@@ -338,14 +359,17 @@ static int cmd_test_last_raw(int argc, char **argv)
 
     /* try to align */
     memcpy(raw_buf, "test12344321tset", 16);
+#if defined(CPU_CC430) || defined(CPU_MSP430FXYZ)
+    printf("The last page holds the ISR vector, so test page %d\n", TEST_LAST_AVAILABLE_PAGE);
+#endif
 
     /* erase the page first */
-    flashpage_write(((int)FLASHPAGE_NUMOF - 1), NULL);
+    flashpage_write(TEST_LAST_AVAILABLE_PAGE, NULL);
 
-    flashpage_write_raw(flashpage_addr((int)FLASHPAGE_NUMOF - 1), raw_buf, strlen(raw_buf));
+    flashpage_write_raw(flashpage_addr(TEST_LAST_AVAILABLE_PAGE), raw_buf, strlen(raw_buf));
 
     /* verify that previous write_raw effectively wrote the desired data */
-    if (memcmp(flashpage_addr((int)FLASHPAGE_NUMOF - 1), raw_buf, strlen(raw_buf)) != 0) {
+    if (memcmp(flashpage_addr(TEST_LAST_AVAILABLE_PAGE), raw_buf, strlen(raw_buf)) != 0) {
         puts("error verifying the content of last page");
         return 1;
     }