drivers/mtd_flashpage: implement pagewise API, don't use raw addresses #19258

benpicco · 2023-02-07T16:59:32Z

Contribution description

The mtd_flashpage API is a bit special in that it experts the user to pass raw flash addresses to it.

This is very usual because every other MTD device will start at address 0, however with mtd_flashpage writing to address 0 will overwrite the firmware.
If the Flash is not mapped to zero, this would just crash.

To fix this an .offset member is added that should be set to the first writable page after the firmware.

With this, mtd_flashpage can then be used like any other MTD device.

#18608 will add a mechanism to set this offset at compile-time to allow for a MTD slot on the internal flash.

Testing procedure

tests/mtd_flashpage should still work.

Issues/PRs references

Split off #18608
alternative to #17964

drivers/mtd_flashpage/mtd_flashpage.c

jue89 · 2023-02-09T12:39:29Z

~~Sry, I have very little time currently, but have you seen #17964 ?~~

Oh, you did :D Nevermind ...

fabian18 · 2023-02-09T19:38:38Z

I was asking myself the same question, why the MTD flashpage sector is further divided into smaller pages.
I tried to add a test function for page wise MTD flashpage writes and reads.

Test

diff --git a/tests/mtd_flashpage/main.c b/tests/mtd_flashpage/main.c
index 0c62d13d2b..ed04b8a242 100644
--- a/tests/mtd_flashpage/main.c
+++ b/tests/mtd_flashpage/main.c
@@ -36,7 +36,9 @@
 #endif
 #define TEST_ADDRESS0       ((FLASHPAGE_NUMOF - 1) - flashpage_page((void *)CPU_FLASH_BASE))
 
-static mtd_flashpage_t _dev = MTD_FLASHPAGE_INIT_VAL(8);
+#define PAGES_PER_SECTOR    8
+
+static mtd_flashpage_t _dev = MTD_FLASHPAGE_INIT_VAL(PAGES_PER_SECTOR);
 static mtd_dev_t *dev = &_dev.base;
 
 static void setup(void)
@@ -143,6 +145,59 @@ static void test_mtd_write_read(void)
     TEST_ASSERT_EQUAL_INT(0, ret);
 }
 
+static void test_mtd_write_read_page(void)
+{
+#if IS_USED(MODULE_MTD_WRITE_PAGE)
+        const char buf[] __attribute__ ((aligned (FLASHPAGE_WRITE_BLOCK_ALIGNMENT)))
+            = "abcdefghijklmno";
+
+        uint8_t buf_empty[3];
+        memset(buf_empty, FLASHPAGE_ERASE_STATE, sizeof(buf_empty));
+        char buf_read[sizeof(buf) + sizeof(buf_empty)];
+        memset(buf_read, 0, sizeof(buf_read));
+        int ret;
+        /* convert last flash page to MTD page */
+        uint32_t last_vpage = LAST_AVAILABLE_PAGE * dev->pages_per_sector;
+        size_t vpage_size = dev->page_size;
+
+        /* write to the beginning of last available page */
+        ret = mtd_write_page(dev, buf, last_vpage, 0, sizeof(buf));
+        TEST_ASSERT_EQUAL_INT(0, ret);
+
+        /* read back data from which some is erased */
+        ret = mtd_read_page(dev, buf_read, last_vpage, 0, sizeof(buf_read));
+        TEST_ASSERT_EQUAL_INT(0, ret);
+        TEST_ASSERT_EQUAL_INT(0, memcmp(buf, buf_read, sizeof(buf)));
+        TEST_ASSERT_EQUAL_INT(0, memcmp(buf_empty, buf_read + sizeof(buf), sizeof(buf_empty)));
+
+        /* clean */
+        ret = mtd_erase_sector(dev, LAST_AVAILABLE_PAGE, 1);
+
+        /* write to the beginning of the MTD page before the last available flash page */
+        ret = mtd_write_page(dev, buf, last_vpage - 1, 0, sizeof(buf));
+        TEST_ASSERT_EQUAL_INT(0, ret);
+
+        /* read back data from which some is erased */
+        ret = mtd_read_page(dev, buf_read, last_vpage - 1, 0, sizeof(buf_read));
+        TEST_ASSERT_EQUAL_INT(0, ret);
+        TEST_ASSERT_EQUAL_INT(0, memcmp(buf, buf_read, sizeof(buf)));
+        TEST_ASSERT_EQUAL_INT(0, memcmp(buf_empty, buf_read + sizeof(buf), sizeof(buf_empty)));
+
+        /* clean*/
+        ret = mtd_erase_sector(dev, LAST_AVAILABLE_PAGE, 1);
+
+        /* write across flash page boundary */
+        ret = mtd_write_page(dev, buf, last_vpage - 1, vpage_size - sizeof(buf) + 1, sizeof(buf));
+        TEST_ASSERT_EQUAL_INT(0, ret);
+
+        /* read back data from which some is erased */
+        ret = mtd_read_page(dev, buf_read, last_vpage - 1, vpage_size - sizeof(buf) + 1, sizeof(buf_read));
+        TEST_ASSERT_EQUAL_INT(0, ret);
+        TEST_ASSERT_EQUAL_INT(0, memcmp(buf, buf_read, sizeof(buf)));
+        TEST_ASSERT_EQUAL_INT(0, memcmp(buf_empty, buf_read + sizeof(buf), sizeof(buf_empty)));
+#endif
+}
+
 Test *tests_mtd_flashpage_tests(void)
 {
     EMB_UNIT_TESTFIXTURES(fixtures) {
@@ -150,6 +205,7 @@ Test *tests_mtd_flashpage_tests(void)
         new_TestFixture(test_mtd_erase),
         new_TestFixture(test_mtd_write_erase),
         new_TestFixture(test_mtd_write_read),
+        new_TestFixture(test_mtd_write_read_page),
     };
 
     EMB_UNIT_TESTCALLER(mtd_flashpage_tests, setup, teardown, fixtures);

I was debugging it and found some changes to make it work.

Diff

diff --git a/drivers/mtd_flashpage/mtd_flashpage.c b/drivers/mtd_flashpage/mtd_flashpage.c
index 8820e89f50..1d19ae503b 100644
--- a/drivers/mtd_flashpage/mtd_flashpage.c
+++ b/drivers/mtd_flashpage/mtd_flashpage.c
@@ -51,7 +51,9 @@ static int _read_page(mtd_dev_t *dev, void *buf, uint32_t page,
     page += super->offset;
 
     /* mtd flashpage maps multiple pages to one virtual sector for unknown reason */
-    uintptr_t addr = (uintptr_t)flashpage_addr(page / dev->pages_per_sector) + offset;
+    uint32_t fpage = page / dev->pages_per_sector;
+    offset += (page % dev->pages_per_sector) * dev->page_size;
+    uintptr_t addr = (uintptr_t)flashpage_addr(fpage) + offset;
 
     DEBUG("flashpage: read %"PRIu32" bytes from %p to %p\n", size, (void *)addr, buf);
 
@@ -142,7 +144,6 @@ static int _erase_page(mtd_dev_t *dev, uint32_t page, uint32_t count)
 {
     mtd_flashpage_t *super = container_of(dev, mtd_flashpage_t, base);
 
-    count *= dev->pages_per_sector;
     page += super->offset;
     while (count--) {
         DEBUG("flashpage: erase page %"PRIu32"\n", page);

fabian18 · 2023-02-09T20:51:14Z

I was debugging it and found some changes to make it work.

The MTD page to flash page conversion and offset correction seems to be required too for _write_page().
Because mtd_write_page() calls _write_sector(), writes are always sector aligned, so I did not notice before.
But if MTD_DRIVER_FLAG_DIRECT_WRITE would be set, it would be required.

benpicco · 2023-02-09T20:55:41Z

or in the mtd_write_page_raw() case - thank you for providing the test case!

I'm very much inclined to just remove this virtual sector shenanigans (but in a follow-up PR) because I really don't see what it could be useful for.

I wrote @vincent-d a mail to better understand what his use-case was for adding this back then.

benpicco · 2023-02-10T10:47:28Z

btw, I remember you did something similar (#18141) for littleFS.
If there is a general need for this kind of thing we should probably provide it in the MTD layer as that's the common layer that could translate between virtual blocks and hardware blocks.

tests/mtd_flashpage/main.c

drivers/mtd_flashpage/mtd_flashpage.c

fabian18 · 2023-02-11T14:24:20Z

btw, I remember you did something similar (#18141) for littleFS.
If there is a general need for this kind of thing we should probably provide it in the MTD layer as that's the common layer that could translate between virtual blocks and hardware blocks.

I think 'blocks' are a software configuration. AFAIK there is no hardware block size, but the minimum block size, a filesystem is initialized with, is the size of a hardware sector, but in general it can be a multiple of it for optimization reasons.

fabian18 · 2023-02-11T14:25:53Z

Does this make sense or did you have something else in mind?

benpicco · 2023-02-11T22:47:23Z

Ah right so for littleFS you combine multiple sectors into one block for more efficient handling.

MTD_FLASHPAGE_INIT_VAL however splits one sector/page into multiple sub-pages for yet unknown reason.

fabian18 · 2023-02-12T13:59:43Z

Last commit brakes the test.
Maybe try this:

diff --git a/drivers/mtd_flashpage/mtd_flashpage.c b/drivers/mtd_flashpage/mtd_flashpage.c
index 8be1f5bf7b..940023bb2e 100644
--- a/drivers/mtd_flashpage/mtd_flashpage.c
+++ b/drivers/mtd_flashpage/mtd_flashpage.c
@@ -38,8 +38,10 @@
 
 static int _init(mtd_dev_t *dev)
 {
-    (void)dev;
+    mtd_flashpage_t *super = container_of(dev, mtd_flashpage_t, base);
+    (void)super;
     assert(dev->pages_per_sector * dev->page_size == FLASHPAGE_SIZE);
+    assert(!(super->offset % dev->pages_per_sector));
     return 0;
 }
 
@@ -117,16 +119,15 @@ static int _write_page(mtd_dev_t *dev, const void *buf, uint32_t page, uint32_t
     return size;
 }
 
-static int _erase_page(mtd_dev_t *dev, uint32_t page, uint32_t count)
+static int _erase_sector(mtd_dev_t *dev, uint32_t sector, uint32_t count)
 {
     mtd_flashpage_t *super = container_of(dev, mtd_flashpage_t, base);
 
-    page += super->offset;
-    page /= dev->pages_per_sector;
+    sector += (super->offset / dev->pages_per_sector);
 
     while (count--) {
-        DEBUG("flashpage: erase page %"PRIu32"\n", page);
-        flashpage_erase(page++);
+        DEBUG("flashpage: erase sector %"PRIu32"\n", sector);
+        flashpage_erase(sector++);
     }
 
     return 0;
@@ -136,5 +137,5 @@ const mtd_desc_t mtd_flashpage_driver = {
     .init = _init,
     .read_page = _read_page,
     .write_page = _write_page,
-    .erase_sector = _erase_page,
+    .erase_sector = _erase_sector,
 };

fabian18 · 2023-02-12T14:01:02Z

And document what the offset member is at the moment:

diff --git a/drivers/include/mtd_flashpage.h b/drivers/include/mtd_flashpage.h
index 072ba8dc3d..7d555dabf9 100644
--- a/drivers/include/mtd_flashpage.h
+++ b/drivers/include/mtd_flashpage.h
@@ -57,7 +57,9 @@ extern const mtd_desc_t mtd_flashpage_driver;
  */
 typedef struct {
     mtd_dev_t base;     /**< MTD generic device */
-    size_t offset;      /**< offset (in pages) from the start of the flash */
+    size_t offset;      /**< Offset in terms of MTD pages, which must comprise
+                             a whole number of sectors from the start of the
+                             flash */
 } mtd_flashpage_t;
 
 #ifdef __cplusplus

benpicco · 2023-02-12T22:17:09Z

Thank you for the fixes, applied them!

fabian18 · 2023-02-13T13:29:51Z

We should protect for out of bounds access because we crash if that happens:

diff --git a/drivers/mtd_flashpage/mtd_flashpage.c b/drivers/mtd_flashpage/mtd_flashpage.c
index 940023bb2e..660e63e769 100644
--- a/drivers/mtd_flashpage/mtd_flashpage.c
+++ b/drivers/mtd_flashpage/mtd_flashpage.c
@@ -57,6 +57,12 @@ static int _read_page(mtd_dev_t *dev, void *buf, uint32_t page,
     offset += (page % dev->pages_per_sector) * dev->page_size;
     uintptr_t addr = (uintptr_t)flashpage_addr(fpage) + offset;
 
+    /* protect for out of bounds reads */
+    if (addr + size < addr ||
+        addr + size > MTD_FLASHPAGE_END_ADDR) {
+        return -EOVERFLOW;
+    }
+
     DEBUG("flashpage: read %"PRIu32" bytes from %p to %p\n", size, (void *)addr, buf);
 
 #ifndef CPU_HAS_UNALIGNED_ACCESS
@@ -90,6 +96,12 @@ static int _write_page(mtd_dev_t *dev, const void *buf, uint32_t page, uint32_t
     offset += (page % dev->pages_per_sector) * dev->page_size;
     uintptr_t addr = (uintptr_t)flashpage_addr(fpage) + offset;
 
+    /* protect for out of bounds writes */
+    if (addr + size < addr ||
+        addr + size > MTD_FLASHPAGE_END_ADDR) {
+        return -EOVERFLOW;
+    }
+
     DEBUG("flashpage: write %"PRIu32" bytes from %p to %p\n", size, buf, (void *)addr);
 
     size = MIN(flashpage_size(fpage) - offset, size);
@@ -125,6 +137,12 @@ static int _erase_sector(mtd_dev_t *dev, uint32_t sector, uint32_t count)
 
     sector += (super->offset / dev->pages_per_sector);
 
+    /* protect for out of bounds erase */
+    if (sector + count < sector ||
+        sector + count > dev->sector_count) {
+        return -EOVERFLOW;
+    }
+
     while (count--) {
         DEBUG("flashpage: erase sector %"PRIu32"\n", sector);
         flashpage_erase(sector++);
diff --git a/tests/mtd_flashpage/main.c b/tests/mtd_flashpage/main.c
index de9e89171a..1ed470252b 100644
--- a/tests/mtd_flashpage/main.c
+++ b/tests/mtd_flashpage/main.c
@@ -34,7 +34,8 @@
 #define TEST_ADDRESS1       (uint32_t)((uintptr_t)flashpage_addr(LAST_AVAILABLE_PAGE) - (uintptr_t)CPU_FLASH_BASE)
 #define TEST_ADDRESS2       (uint32_t)((uintptr_t)flashpage_addr(LAST_AVAILABLE_PAGE - 1) - (uintptr_t)CPU_FLASH_BASE)
 #endif
-#define TEST_ADDRESS0       (FLASHPAGE_NUMOF - 1)
+/* Address of last flash page and not last available flashpage */
+#define TEST_ADDRESS0       (uint32_t)((uintptr_t)flashpage_addr((FLASHPAGE_NUMOF - 1)) - (uintptr_t)CPU_FLASH_BASE)
 
 #define PAGES_PER_SECTOR    8
 
@@ -136,6 +137,14 @@ static void test_mtd_write_read(void)
     ret = mtd_erase(dev, TEST_ADDRESS1, dev->pages_per_sector * dev->page_size);
     TEST_ASSERT_EQUAL_INT(0, ret);
 
+    /* Out of bounds read */
+    ret = mtd_read(dev, buf_read, TEST_ADDRESS0 + FLASHPAGE_SIZE - 1, sizeof(buf_read));
+    TEST_ASSERT_EQUAL_INT(-EOVERFLOW, ret);
+
+    /* Out of bounds write */
+    ret = mtd_write(dev, buf, TEST_ADDRESS0 + FLASHPAGE_SIZE - 1, sizeof(buf));
+    TEST_ASSERT_EQUAL_INT(-EOVERFLOW, ret);
+
     /* Unaligned write / read */
     ret = mtd_write(dev, &buf[1], TEST_ADDRESS1 + sizeof(buf_empty), sizeof(buf) - 1);
     TEST_ASSERT_EQUAL_INT(0, ret);
@@ -195,6 +204,16 @@ static void test_mtd_write_read_page(void)
         TEST_ASSERT_EQUAL_INT(0, ret);
         TEST_ASSERT_EQUAL_INT(0, memcmp(buf, buf_read, sizeof(buf)));
         TEST_ASSERT_EQUAL_INT(0, memcmp(buf_empty, buf_read + sizeof(buf), sizeof(buf_empty)));
+
+        /* Out of bounds read */
+        ret = mtd_read_page(dev, buf_read,
+                            FLASHPAGE_NUMOF * dev->pages_per_sector, 0, sizeof(buf_read));
+        TEST_ASSERT_EQUAL_INT(-EOVERFLOW, ret);
+
+        /* Out of bounds write */
+        ret = mtd_write_page(dev, buf,
+                             FLASHPAGE_NUMOF * dev->pages_per_sector, 0, sizeof(buf));
+        TEST_ASSERT_EQUAL_INT(-EOVERFLOW, ret);
 #endif
 }

fabian18 · 2023-02-13T13:30:51Z

Since the MTD layer does not know about the offset member, it cannot provide that check, so it is done in the driver.

fabian18 · 2023-02-13T13:40:18Z

Ok, please squash if you like and let´s see what the CI thinks

riot-ci · 2023-02-13T13:47:10Z

Murdock results

✔️ PASSED

43fffcd drivers/mtd: add bounds checking

Success	Failures	Total	Runtime
6864	0	6865	10m:26s

Artifacts

Documentation preview

fabian18 · 2023-02-13T13:52:15Z

A quick test of tests/mtd_flashpage on nucleo-l031k6, samr21-xpro and esp32s2-devkit was successful.

I am testing with bluepill

fabian18

One last thing I guess.
We could even be a bit more overflow safe.

diff --git a/drivers/include/mtd_flashpage.h b/drivers/include/mtd_flashpage.h
index 7d555dabf9..932efc2bcc 100644
--- a/drivers/include/mtd_flashpage.h
+++ b/drivers/include/mtd_flashpage.h
@@ -57,7 +57,7 @@ extern const mtd_desc_t mtd_flashpage_driver;
  */
 typedef struct {
     mtd_dev_t base;     /**< MTD generic device */
-    size_t offset;      /**< Offset in terms of MTD pages, which must comprise
+    uint32_t offset;    /**< Offset in terms of MTD pages, which must comprise
                              a whole number of sectors from the start of the
                              flash */
 } mtd_flashpage_t;
diff --git a/drivers/mtd_flashpage/mtd_flashpage.c b/drivers/mtd_flashpage/mtd_flashpage.c
index 3ba1ba5835..d90a1fa93e 100644
--- a/drivers/mtd_flashpage/mtd_flashpage.c
+++ b/drivers/mtd_flashpage/mtd_flashpage.c
@@ -51,18 +51,22 @@ static int _read_page(mtd_dev_t *dev, void *buf, uint32_t page,
 {
     mtd_flashpage_t *super = container_of(dev, mtd_flashpage_t, base);
 
+    if (page + super->offset < page) {
+        return -EOVERFLOW;
+    }
     page += super->offset;
 
     /* mtd flashpage maps multiple pages to one virtual sector for unknown reason */
     uint32_t fpage = page / dev->pages_per_sector;
     offset += (page % dev->pages_per_sector) * dev->page_size;
-    uintptr_t addr = (uintptr_t)flashpage_addr(fpage) + offset;
+    uintptr_t addr = (uintptr_t)flashpage_addr(fpage);
 
     /* protect for out of bounds reads */
-    if (addr + size < addr ||
-        addr + size > MTD_FLASHPAGE_END_ADDR) {
+    if (addr + offset + size < addr ||
+        addr + offset + size > MTD_FLASHPAGE_END_ADDR) {
         return -EOVERFLOW;
     }
+    addr += offset;
 
     DEBUG("flashpage: read %"PRIu32" bytes from %p to %p\n", size, (void *)addr, buf);
 
@@ -90,18 +94,22 @@ static int _write_page(mtd_dev_t *dev, const void *buf, uint32_t page, uint32_t
 {
     mtd_flashpage_t *super = container_of(dev, mtd_flashpage_t, base);
 
+    if (page + super->offset < page) {
+        return -EOVERFLOW;
+    }
     page += super->offset;
 
     /* mtd flashpage maps multiple pages to one virtual sector for unknown reason */
     uint32_t fpage = page / dev->pages_per_sector;
     offset += (page % dev->pages_per_sector) * dev->page_size;
-    uintptr_t addr = (uintptr_t)flashpage_addr(fpage) + offset;
+    uintptr_t addr = (uintptr_t)flashpage_addr(fpage);
 
     /* protect for out of bounds writes */
-    if (addr + size < addr ||
-        addr + size > MTD_FLASHPAGE_END_ADDR) {
+    if (addr + offset + size < addr ||
+        addr + offset + size > MTD_FLASHPAGE_END_ADDR) {
         return -EOVERFLOW;
     }
+    addr += offset;
 
     DEBUG("flashpage: write %"PRIu32" bytes from %p to %p\n", size, buf, (void *)addr);
 
@@ -136,6 +144,9 @@ static int _erase_sector(mtd_dev_t *dev, uint32_t sector, uint32_t count)
 {
     mtd_flashpage_t *super = container_of(dev, mtd_flashpage_t, base);
 
+    if (sector + (super->offset / dev->pages_per_sector) < sector) {
+        return -EOVERFLOW;
+    }
     sector += (super->offset / dev->pages_per_sector);
 
     /* protect for out of bounds erase */

benpicco · 2023-02-14T15:05:19Z

Added them, but we should better add those overflow checks to mtd.c.
I'm not so worried about offset as it's completely internal. For MTD the device starts at 0 and is sector_count sectors long.

We can still check for config errors to ensure sector_count is not beyond the end of the device, but it's enough to do that on init.

fabian18 · 2023-02-14T18:30:56Z

Added them, but we should better add those overflow checks to mtd.c.
I'm not so worried about offset as it's completely internal. For MTD the device starts at 0 and is sector_count sectors long.

You mean in addition to the checks with super->offset, do you?
Would be safer I guess.

We can still check for config errors to ensure sector_count is not beyond the end of the device, but it's enough to do that on init.

Sure, add this if you want. You mean in mtd_flashpage _init(), do you?

benpicco · 2023-02-14T23:27:21Z

You mean in addition to the checks with super->offset, do you?

No, they are not needed.
If we can ensure that

offset + sector_count < MTD_FLASHPAGE_END_ADDR (in _init())
any access via MTD does not go beyond sector_count

we can conclude that no access that passes the check in 2. will be beyond the end of the device, no matter what offset is because of 1.

fabian18

Ok the out of bounds checks can be in the MTD layer because if a device has for example 10 physical sectors and offset is 1, the MTD sector_count must be initialized with 9, right?

drivers/mtd_flashpage/mtd_flashpage.c

benpicco · 2023-02-15T21:01:12Z

if a device has for example 10 physical sectors and offset is 1, the MTD sector_count must be initialized with 9, right?

exactly!

fabian18

I think it is good. ACK one of two.

fabian18 · 2023-02-16T14:51:40Z

Or does it not need 2 ACK anaymore?

benpicco · 2023-02-16T14:57:33Z

Does it?
It's only an API change insofar that things that would previously return error are now allowed because they are handled by the MTD layer.

The dreaded virtual pages are still there.

fabian18 · 2023-02-16T14:59:43Z

Thought API change always needs 2 ACK. If not then even better.

benpicco · 2023-02-16T15:02:03Z

Depends on the severity of the change, nothing's set in stone.

Thank you for the helpful suggestions btw!

bors merge

bors · 2023-02-16T18:07:31Z

Build succeeded:

benpicco requested review from smlng, leandrolanzieri, aabadie, MichelRottleuthner and fjmolinas as code owners February 7, 2023 16:59

benpicco requested a review from jue89 February 7, 2023 16:59

github-actions bot added Area: drivers Area: Device drivers Area: tests Area: tests and testing framework labels Feb 7, 2023

benpicco changed the title ~~drivers/mtd_flashpage: implement pagewise API, don't use raw addresses~~ drivers/mtd_flashpage: implement pagewise API, don't require to use raw addresses Feb 7, 2023

benpicco changed the title ~~drivers/mtd_flashpage: implement pagewise API, don't require to use raw addresses~~ drivers/mtd_flashpage: implement pagewise API, don't use raw addresses Feb 7, 2023

benpicco added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Feb 7, 2023

fabian18 reviewed Feb 8, 2023

View reviewed changes

drivers/mtd_flashpage/mtd_flashpage.c Outdated Show resolved Hide resolved

drivers/mtd_flashpage/mtd_flashpage.c Outdated Show resolved Hide resolved

benpicco force-pushed the mtd_flashpage-pagewise branch from df31831 to 6c78cc4 Compare February 9, 2023 13:33

benpicco requested a review from vincent-d February 9, 2023 14:49

fabian18 reviewed Feb 11, 2023

View reviewed changes

tests/mtd_flashpage/main.c Outdated Show resolved Hide resolved

drivers/mtd_flashpage/mtd_flashpage.c Outdated Show resolved Hide resolved

benpicco force-pushed the mtd_flashpage-pagewise branch from 260454c to 2ea7637 Compare February 13, 2023 13:42

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 13, 2023

benpicco force-pushed the mtd_flashpage-pagewise branch from 2ea7637 to 68d1123 Compare February 13, 2023 13:48

fabian18 reviewed Feb 13, 2023

View reviewed changes

fabian18 reviewed Feb 15, 2023

View reviewed changes

drivers/mtd_flashpage/mtd_flashpage.c Outdated Show resolved Hide resolved

drivers/mtd_flashpage/mtd_flashpage.c Show resolved Hide resolved

benpicco mentioned this pull request Feb 16, 2023

drivers/at24cxxx: merge mtd driver with at24cxxx.c #19281

Merged

fabian18 approved these changes Feb 16, 2023

View reviewed changes

benpicco and others added 5 commits February 16, 2023 15:50

drivers/mtd_flashpage: implement pagewise API

5d5d88b

tests/mtd_flashpage: adapt to mtd_flashpage API change

68694a0

drivers/mtd_flashpage: drop legacy API

d8ab11b

tests/mtd_flashpage: test unaligned read/write

8e902bd

drivers/mtd: add bounds checking

43fffcd

benpicco force-pushed the mtd_flashpage-pagewise branch from 628b841 to 43fffcd Compare February 16, 2023 14:51

This was referenced Feb 16, 2023

mtd/*: drop .write() if .write_page() is implemented #15380

Merged

mtd: drop .write() function #19283

Closed

bors bot merged commit 2221cbf into RIOT-OS:master Feb 16, 2023

benpicco mentioned this pull request Feb 22, 2023

drivers/mtd_flashpage: fix address mapping #17964

Closed

benpicco deleted the mtd_flashpage-pagewise branch February 22, 2023 15:10

benpicco mentioned this pull request Feb 27, 2023

drivers/mtd_flashpage: allow to define AUX slot on flash #18608

Merged

MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers/mtd_flashpage: implement pagewise API, don't use raw addresses #19258

drivers/mtd_flashpage: implement pagewise API, don't use raw addresses #19258

benpicco commented Feb 7, 2023 •

edited

Loading

jue89 commented Feb 9, 2023 •

edited

Loading

fabian18 commented Feb 9, 2023

fabian18 commented Feb 9, 2023

benpicco commented Feb 9, 2023

benpicco commented Feb 10, 2023

fabian18 commented Feb 11, 2023

fabian18 commented Feb 11, 2023

benpicco commented Feb 11, 2023

fabian18 commented Feb 12, 2023

fabian18 commented Feb 12, 2023

benpicco commented Feb 12, 2023

fabian18 commented Feb 13, 2023

fabian18 commented Feb 13, 2023

fabian18 commented Feb 13, 2023

riot-ci commented Feb 13, 2023 •

edited

Loading

fabian18 commented Feb 13, 2023

fabian18 left a comment

benpicco commented Feb 14, 2023

fabian18 commented Feb 14, 2023

benpicco commented Feb 14, 2023

fabian18 left a comment

benpicco commented Feb 15, 2023

fabian18 left a comment

fabian18 commented Feb 16, 2023

benpicco commented Feb 16, 2023

fabian18 commented Feb 16, 2023

benpicco commented Feb 16, 2023

bors bot commented Feb 16, 2023

drivers/mtd_flashpage: implement pagewise API, don't use raw addresses #19258

drivers/mtd_flashpage: implement pagewise API, don't use raw addresses #19258

Conversation

benpicco commented Feb 7, 2023 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

jue89 commented Feb 9, 2023 • edited Loading

fabian18 commented Feb 9, 2023

fabian18 commented Feb 9, 2023

benpicco commented Feb 9, 2023

benpicco commented Feb 10, 2023

fabian18 commented Feb 11, 2023

fabian18 commented Feb 11, 2023

benpicco commented Feb 11, 2023

fabian18 commented Feb 12, 2023

fabian18 commented Feb 12, 2023

benpicco commented Feb 12, 2023

fabian18 commented Feb 13, 2023

fabian18 commented Feb 13, 2023

fabian18 commented Feb 13, 2023

riot-ci commented Feb 13, 2023 • edited Loading

Murdock results

Artifacts

fabian18 commented Feb 13, 2023

fabian18 left a comment

Choose a reason for hiding this comment

benpicco commented Feb 14, 2023

fabian18 commented Feb 14, 2023

benpicco commented Feb 14, 2023

fabian18 left a comment

Choose a reason for hiding this comment

benpicco commented Feb 15, 2023

fabian18 left a comment

Choose a reason for hiding this comment

fabian18 commented Feb 16, 2023

benpicco commented Feb 16, 2023

fabian18 commented Feb 16, 2023

benpicco commented Feb 16, 2023

bors bot commented Feb 16, 2023

benpicco commented Feb 7, 2023 •

edited

Loading

jue89 commented Feb 9, 2023 •

edited

Loading

riot-ci commented Feb 13, 2023 •

edited

Loading