You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2018/09/20 13:28:37 UTC

[GitHub] mkiiskila closed pull request #1406: hw/hal; hal_flash_isempty() also returns data from the location.

mkiiskila closed pull request #1406: hw/hal; hal_flash_isempty() also returns data from the location.
URL: https://github.com/apache/mynewt-core/pull/1406
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/fs/fcb/src/fcb.c b/fs/fcb/src/fcb.c
index b70f5f50ac..59d4054f21 100644
--- a/fs/fcb/src/fcb.c
+++ b/fs/fcb/src/fcb.c
@@ -196,12 +196,11 @@ fcb_sector_hdr_read(struct fcb *fcb, struct flash_area *fap,
     if (!fdap) {
         fdap = &fda;
     }
-    if (flash_area_isempty_at(fap, 0, sizeof(*fdap))) {
-        return 0;
-    }
-    rc = flash_area_read(fap, 0, fdap, sizeof(*fdap));
-    if (rc) {
+    rc = flash_area_read_is_empty(fap, 0, fdap, sizeof(*fdap));
+    if (rc < 0) {
         return FCB_ERR_FLASH;
+    } else if (rc == 1) {
+        return 0;
     }
     if (fdap->fd_magic != fcb->f_magic) {
         return FCB_ERR_MAGIC;
diff --git a/fs/fcb/src/fcb_elem_info.c b/fs/fcb/src/fcb_elem_info.c
index ad1c4e1a80..5af34a565e 100644
--- a/fs/fcb/src/fcb_elem_info.c
+++ b/fs/fcb/src/fcb_elem_info.c
@@ -41,12 +41,11 @@ fcb_elem_crc8(struct fcb *fcb, struct fcb_entry *loc, uint8_t *c8p)
     if (loc->fe_elem_off + 2 > loc->fe_area->fa_size) {
         return FCB_ERR_NOVAR;
     }
-    if (flash_area_isempty_at(loc->fe_area, loc->fe_elem_off, 2)) {
-        return FCB_ERR_NOVAR;
-    }
-    rc = flash_area_read(loc->fe_area, loc->fe_elem_off, tmp_str, 2);
-    if (rc) {
+    rc = flash_area_read_is_empty(loc->fe_area, loc->fe_elem_off, tmp_str, 2);
+    if (rc < 0) {
         return FCB_ERR_FLASH;
+    } else if (rc == 1) {
+        return FCB_ERR_NOVAR;
     }
 
     cnt = fcb_get_len(tmp_str, &len);
diff --git a/hw/drivers/flash/enc_flash/src/enc_flash.c b/hw/drivers/flash/enc_flash/src/enc_flash.c
index 83de4bc294..7f170bfc46 100644
--- a/hw/drivers/flash/enc_flash/src/enc_flash.c
+++ b/hw/drivers/flash/enc_flash/src/enc_flash.c
@@ -36,7 +36,7 @@ static int enc_flash_erase_sector(const struct hal_flash *h_dev,
 static int enc_flash_sector_info(const struct hal_flash *h_dev, int idx,
                                  uint32_t *addr, uint32_t *sz);
 static int enc_flash_is_empty(const struct hal_flash *h_dev, uint32_t addr,
-                              uint32_t len);
+                              void *buf, uint32_t len);
 static int enc_flash_init(const struct hal_flash *h_dev);
 
 const struct hal_flash_funcs enc_flash_funcs = {
@@ -153,16 +153,32 @@ enc_flash_sector_info(const struct hal_flash *h_dev, int idx,
 }
 
 static int
-enc_flash_is_empty(const struct hal_flash *h_dev, uint32_t addr, uint32_t len)
+enc_flash_is_empty(const struct hal_flash *h_dev, uint32_t addr, void *buf,
+                   uint32_t len)
 {
     struct enc_flash_dev *dev = HAL_TO_ENC(h_dev);
+    const struct hal_flash *hwdev;
+    int rc;
+    int rc2;
 
-    h_dev = dev->efd_hwdev;
+    hwdev = dev->efd_hwdev;
 
-    if (h_dev->hf_itf->hff_is_empty) {
-        return h_dev->hf_itf->hff_is_empty(h_dev, addr, len);
+    if (hwdev->hf_itf->hff_is_empty) {
+        return hwdev->hf_itf->hff_is_empty(hwdev, addr, buf, len);
     } else {
-        return hal_flash_is_erased(h_dev, addr, len);
+        rc = hal_flash_is_erased(hwdev, addr, buf, len);
+        if (rc < 0) {
+            return rc;
+        }
+
+        /*
+         * Also read the underlying data.
+         */
+        rc2 = enc_flash_read(h_dev, addr, buf, len);
+        if (rc2 < 0) {
+            return rc2;
+        }
+        return rc;
     }
 }
 
diff --git a/hw/drivers/flash/enc_flash/test/src/testcases/enc_flash_flash_map.c b/hw/drivers/flash/enc_flash/test/src/testcases/enc_flash_flash_map.c
index 1cf056d54c..ee5112fc01 100644
--- a/hw/drivers/flash/enc_flash/test/src/testcases/enc_flash_flash_map.c
+++ b/hw/drivers/flash/enc_flash/test/src/testcases/enc_flash_flash_map.c
@@ -26,6 +26,8 @@ TEST_CASE(enc_flash_test_flash_map)
     int rc;
     struct flash_area *fa;
     int i;
+    int off;
+    int blk_sz;
     bool b;
     char writedata[128];
     char readdata[128];
@@ -38,8 +40,14 @@ TEST_CASE(enc_flash_test_flash_map)
         rc = flash_area_is_empty(fa, &b);
         TEST_ASSERT(rc == 0);
         TEST_ASSERT(b == true);
-        rc = flash_area_isempty_at(fa, 0, fa->fa_size);
-        TEST_ASSERT(rc == 1);
+        for (off = 0; off < fa->fa_size; off += blk_sz) {
+            blk_sz = fa->fa_size - off;
+            if (blk_sz > sizeof(readdata)) {
+                blk_sz = sizeof(readdata);
+            }
+            rc = flash_area_read_is_empty(fa, off, readdata, blk_sz);
+            TEST_ASSERT(rc == 1);
+        }
         fa++;
     }
 
@@ -59,7 +67,8 @@ TEST_CASE(enc_flash_test_flash_map)
     rc = flash_area_is_empty(fa, &b);
     TEST_ASSERT(rc == 0);
     TEST_ASSERT(b == false);
-    rc = flash_area_isempty_at(fa, 0, fa->fa_size);
+    memset(readdata, 0, sizeof(readdata));
+    rc = flash_area_read_is_empty(fa, 0, readdata, sizeof(readdata));
     TEST_ASSERT(rc == 0);
-
+    TEST_ASSERT(!memcmp(writedata, readdata, sizeof(writedata)));
 }
diff --git a/hw/drivers/flash/enc_flash/test/src/testcases/enc_flash_hal.c b/hw/drivers/flash/enc_flash/test/src/testcases/enc_flash_hal.c
index b4a8e0d88d..fce211afe8 100644
--- a/hw/drivers/flash/enc_flash/test/src/testcases/enc_flash_hal.c
+++ b/hw/drivers/flash/enc_flash/test/src/testcases/enc_flash_hal.c
@@ -27,6 +27,8 @@ TEST_CASE(enc_flash_test_hal)
     int rc;
     struct flash_area *fa;
     int i;
+    int off;
+    int blk_sz;
     char writedata[128];
     char readdata[128];
 
@@ -35,8 +37,15 @@ TEST_CASE(enc_flash_test_hal)
     for (i = 0; i < ENC_TEST_FLASH_AREA_CNT; i++) {
         rc = hal_flash_erase(fa->fa_id, fa->fa_off, fa->fa_size);
         TEST_ASSERT(rc == 0);
-        rc = hal_flash_isempty(fa->fa_id, fa->fa_off, fa->fa_size);
-        TEST_ASSERT(rc == 1);
+        for (off = 0; off < fa->fa_size; off += blk_sz) {
+            blk_sz = fa->fa_size - off;
+            if (blk_sz > sizeof(readdata)) {
+                blk_sz = sizeof(readdata);
+            }
+            rc = hal_flash_isempty(fa->fa_id, fa->fa_off + off, readdata,
+                                   blk_sz);
+            TEST_ASSERT(rc == 1);
+        }
         fa++;
     }
 
@@ -50,10 +59,10 @@ TEST_CASE(enc_flash_test_hal)
 
     rc = hal_flash_read(fa->fa_id, fa->fa_off, readdata, sizeof(readdata));
     TEST_ASSERT(rc == 0);
-
     TEST_ASSERT(!memcmp(writedata, readdata, sizeof(writedata)));
 
-    rc = hal_flash_isempty(fa->fa_id, fa->fa_off, fa->fa_size);
+    memset(readdata, 0, sizeof(readdata));
+    rc = hal_flash_isempty(fa->fa_id, fa->fa_off, readdata, sizeof(readdata));
     TEST_ASSERT(rc == 0);
-
+    TEST_ASSERT(!memcmp(writedata, readdata, sizeof(writedata)));
 }
diff --git a/hw/hal/include/hal/hal_flash.h b/hw/hal/include/hal/hal_flash.h
index cc42c2b3a8..d945147e3e 100644
--- a/hw/hal/include/hal/hal_flash.h
+++ b/hw/hal/include/hal/hal_flash.h
@@ -40,7 +40,8 @@ int hal_flash_write(uint8_t flash_id, uint32_t address, const void *src,
   uint32_t num_bytes);
 int hal_flash_erase_sector(uint8_t flash_id, uint32_t sector_address);
 int hal_flash_erase(uint8_t flash_id, uint32_t address, uint32_t num_bytes);
-int hal_flash_isempty(uint8_t flash_id, uint32_t address, uint32_t num_bytes);
+int hal_flash_isempty(uint8_t flash_id, uint32_t address, void *dst,
+                      uint32_t num_bytes);
 uint8_t hal_flash_align(uint8_t flash_id);
 uint8_t hal_flash_erased_val(uint8_t flash_id);
 int hal_flash_init(void);
diff --git a/hw/hal/include/hal/hal_flash_int.h b/hw/hal/include/hal/hal_flash_int.h
index 8ac9ef8514..947079bb3b 100644
--- a/hw/hal/include/hal/hal_flash_int.h
+++ b/hw/hal/include/hal/hal_flash_int.h
@@ -41,7 +41,7 @@ struct hal_flash_funcs {
     int (*hff_sector_info)(const struct hal_flash *dev, int idx,
             uint32_t *address, uint32_t *size);
     int (*hff_is_empty)(const struct hal_flash *dev, uint32_t address,
-            uint32_t num_bytes);
+            void *dst, uint32_t num_bytes);
     int (*hff_init)(const struct hal_flash *dev);
 };
 
@@ -59,7 +59,7 @@ struct hal_flash {
  */
 uint32_t hal_flash_sector_size(const struct hal_flash *hf, int sec_idx);
 
-int hal_flash_is_erased(const struct hal_flash *, uint32_t, uint32_t);
+int hal_flash_is_erased(const struct hal_flash *, uint32_t, void *, uint32_t);
 
 #ifdef __cplusplus
 }
diff --git a/hw/hal/src/hal_flash.c b/hw/hal/src/hal_flash.c
index bed168af86..283dde3f57 100644
--- a/hw/hal/src/hal_flash.c
+++ b/hw/hal/src/hal_flash.c
@@ -311,33 +311,27 @@ hal_flash_erase(uint8_t id, uint32_t address, uint32_t num_bytes)
 }
 
 int
-hal_flash_is_erased(const struct hal_flash *hf, uint32_t address,
+hal_flash_is_erased(const struct hal_flash *hf, uint32_t address, void *dst,
         uint32_t num_bytes)
 {
-    uint8_t buf[32];
-    uint32_t blksz;
-    int i;
+    uint32_t i;
+    uint8_t *buf;
 
-    while (num_bytes) {
-        blksz = sizeof(buf);
-        if (blksz > num_bytes) {
-            blksz = num_bytes;
-        }
-        if (hf->hf_itf->hff_read(hf, address, buf, blksz)) {
-            return -1;
-        }
-        for (i = 0; i < blksz; i++) {
-            if (buf[i] != hf->hf_erased_val) {
-                return 0;
-            }
+    buf = dst;
+
+    if (hf->hf_itf->hff_read(hf, address, dst, num_bytes)) {
+        return -1;
+    }
+    for (i = 0; i < num_bytes; i++) {
+        if (buf[i] != hf->hf_erased_val) {
+            return 0;
         }
-        num_bytes -= blksz;
     }
     return 1;
 }
 
 int
-hal_flash_isempty(uint8_t id, uint32_t address, uint32_t num_bytes)
+hal_flash_isempty(uint8_t id, uint32_t address, void *dst, uint32_t num_bytes)
 {
     const struct hal_flash *hf;
 
@@ -350,9 +344,9 @@ hal_flash_isempty(uint8_t id, uint32_t address, uint32_t num_bytes)
         return -1;
     }
     if (hf->hf_itf->hff_is_empty) {
-        return hf->hf_itf->hff_is_empty(hf, address, num_bytes);
+        return hf->hf_itf->hff_is_empty(hf, address, dst, num_bytes);
     } else {
-        return hal_flash_is_erased(hf, address, num_bytes);
+        return hal_flash_is_erased(hf, address, dst, num_bytes);
     }
 }
 
diff --git a/mgmt/imgmgr/src/imgmgr.c b/mgmt/imgmgr/src/imgmgr.c
index ac60a9d6b8..1a7674fd8a 100644
--- a/mgmt/imgmgr/src/imgmgr.c
+++ b/mgmt/imgmgr/src/imgmgr.c
@@ -219,8 +219,8 @@ imgr_read_info(int image_slot, struct image_version *ver, uint8_t *hash,
     if (rc2) {
         return -1;
     }
-    rc2 = flash_area_read(fa, 0, hdr, sizeof(*hdr));
-    if (rc2) {
+    rc2 = flash_area_read_is_empty(fa, 0, hdr, sizeof(*hdr));
+    if (rc2 < 0) {
         goto end;
     }
 
@@ -231,7 +231,8 @@ imgr_read_info(int image_slot, struct image_version *ver, uint8_t *hash,
         if (ver) {
             memcpy(ver, &hdr->ih_ver, sizeof(*ver));
         }
-    } else if (hdr->ih_magic == 0xffffffff) {
+    } else if (rc2 == 1) {
+        /* Area is empty */
         rc = 2;
         goto end;
     } else {
@@ -259,11 +260,11 @@ imgr_read_info(int image_slot, struct image_version *ver, uint8_t *hash,
     }
     tlv = (struct image_tlv *)data;
     while (data_off + sizeof(*tlv) <= data_end) {
-        rc2 = flash_area_read(fa, data_off, tlv, sizeof(*tlv));
-        if (rc2) {
+        rc2 = flash_area_read_is_empty(fa, data_off, tlv, sizeof(*tlv));
+        if (rc2 < 0) {
             goto end;
         }
-        if (tlv->it_type == 0xff && tlv->it_len == 0xffff) {
+        if (rc2 == 1) {
             break;
         }
         if (tlv->it_type != IMAGE_TLV_SHA256 ||
diff --git a/sys/flash_map/include/flash_map/flash_map.h b/sys/flash_map/include/flash_map/flash_map.h
index 454c8ffb2e..4590cc43c6 100644
--- a/sys/flash_map/include/flash_map/flash_map.h
+++ b/sys/flash_map/include/flash_map/flash_map.h
@@ -83,9 +83,12 @@ int flash_area_erase(const struct flash_area *, uint32_t off, uint32_t len);
 int flash_area_is_empty(const struct flash_area *, bool *);
 
 /*
- * Whether a region of flash_area is empty
+ * Reads data. Return code indicates whether it thinks that
+ * underlying area is in erased state.
+ *
+ * Returns 1 if empty, 0 if not. <0 in case of an error.
  */
-int flash_area_isempty_at(const struct flash_area *, uint32_t off,
+int flash_area_read_is_empty(const struct flash_area *, uint32_t off, void *dst,
   uint32_t len);
 
 /*
diff --git a/sys/flash_map/src/flash_map.c b/sys/flash_map/src/flash_map.c
index 028bbe7780..98c3d35f80 100644
--- a/sys/flash_map/src/flash_map.c
+++ b/sys/flash_map/src/flash_map.c
@@ -189,23 +189,32 @@ flash_area_erased_val(const struct flash_area *fa)
 int
 flash_area_is_empty(const struct flash_area *fa, bool *empty)
 {
+    uint32_t data[64 >> 2];
+    uint32_t data_off = 0;
+    int8_t bytes_to_read;
     int rc;
 
-    rc = hal_flash_isempty(fa->fa_device_id, fa->fa_off, fa->fa_size);
-    if (rc < 0) {
-        return rc;
-    } else if (rc == 1) {
-        *empty = true;
-    } else {
-        *empty = false;
-    }
-    return 0;
+     while (data_off < fa->fa_size) {
+         bytes_to_read = min(64, fa->fa_size - data_off);
+         rc = hal_flash_isempty(fa->fa_device_id, fa->fa_off + data_off, data,
+                                bytes_to_read);
+         if (rc < 0) {
+             return rc;
+         } else if (rc == 0) {
+             *empty = false;
+             return 0;
+         }
+         data_off += bytes_to_read;
+     }
+     *empty = true;
+     return 0;
 }
 
 int
-flash_area_isempty_at(const struct flash_area *fa, uint32_t off, uint32_t len)
+flash_area_read_is_empty(const struct flash_area *fa, uint32_t off, void *dst,
+                         uint32_t len)
 {
-    return hal_flash_isempty(fa->fa_device_id, fa->fa_off + off, len);
+    return hal_flash_isempty(fa->fa_device_id, fa->fa_off + off, dst, len);
 }
 
 /**


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services