diff --git a/ports/zephyr-cp/prj.conf b/ports/zephyr-cp/prj.conf index be3c56ef755..39b4206ff88 100644 --- a/ports/zephyr-cp/prj.conf +++ b/ports/zephyr-cp/prj.conf @@ -33,6 +33,7 @@ CONFIG_CDC_ACM_SERIAL_INITIALIZE_AT_BOOT=n CONFIG_USBD_MSC_CLASS=y CONFIG_USBD_MSC_LUNS_PER_INSTANCE=1 +CONFIG_USBD_MSC_SCSI_BUFFER_SIZE=4096 CONFIG_HWINFO=y CONFIG_REBOOT=y diff --git a/ports/zephyr-cp/supervisor/flash.c b/ports/zephyr-cp/supervisor/flash.c index 9f0db1f3031..ad1158431db 100644 --- a/ports/zephyr-cp/supervisor/flash.c +++ b/ports/zephyr-cp/supervisor/flash.c @@ -616,7 +616,9 @@ static bool _flash_write_block(const uint8_t *data, uint32_t block) { return write_flash(address, data, FILESYSTEM_BLOCK_SIZE); } if (_current_page_address != NO_PAGE_LOADED) { - supervisor_flash_flush(); + // Flush only the old page. supervisor_flash_flush() would also + // clear the dirty flag, which littlefs sets before writing. + _flush_keep_cache(true); } if (flash_cache_table == NULL && !allocate_ram_cache()) { erase_page(_scratch_page_address); diff --git a/ports/zephyr-cp/supervisor/usb.c b/ports/zephyr-cp/supervisor/usb.c index b6e7d3c9d07..becbdee41b6 100644 --- a/ports/zephyr-cp/supervisor/usb.c +++ b/ports/zephyr-cp/supervisor/usb.c @@ -199,7 +199,8 @@ int _zephyr_disk_ioctl(struct disk_info *disk, uint8_t cmd, void *buff) { disk_ioctl(root, GET_SECTOR_SIZE, buff); return 0; case DISK_IOCTL_CTRL_SYNC: - disk_ioctl(root, CTRL_SYNC, buff); + // Zephyr's MSC class sends this after every WRITE10. Don't flush + // here; the background flush writes the cached page. autoreload_resume(AUTORELOAD_SUSPEND_USB); // This write is complete; initiate an autoreload if this was a file data or metadata write, diff --git a/ports/zephyr-cp/tests/test_flash.py b/ports/zephyr-cp/tests/test_flash.py index 7882d1f31a8..8e81491ba24 100644 --- a/ports/zephyr-cp/tests/test_flash.py +++ b/ports/zephyr-cp/tests/test_flash.py @@ -194,6 +194,96 @@ def test_flash_256k_overwrite(circuitpython, board): assert content == "second version" +PAGE_CHANGE_CODE = """\ +import board +import storage +import time +# The display's auto refresh keeps the supervisor tick running, which would +# flush the cache whether or not the filesystem is marked dirty. +board.DISPLAY.auto_refresh = False +storage.remount("/", readonly=False) +# Program the sectors first. Writes to erased flash skip the page cache. +with open("/data.bin", "wb") as f: + f.write(bytes(16384)) +# Overwrite one sector, then one three pages later, so the last write moves +# the flash cache to a new page. Don't flush or close the file. +f = open("/data.bin", "r+b") +f.write(b"A" * 512) +f.seek(12288) +f.write(b"Z" * 512) +# The background flush runs after 1000 supervisor ticks. native_sim's 100 Hz +# kernel tick makes that about 10 seconds. +time.sleep(15) +print("slept") +while True: + time.sleep(1) +""" + + +@pytest.mark.circuitpy_drive({"code.py": PAGE_CHANGE_CODE}) +@pytest.mark.fat_filesystem_only +def test_flash_background_flush_after_page_change(circuitpython): + """Test that the background flush writes the page cached by a page change. + + The file is never flushed or closed, so only the background flush can + write the last sector before the simulator is stopped. + """ + circuitpython.serial.wait_for("slept") + circuitpython.shutdown() + + assert b"Z" * 512 in circuitpython.flash_file.read_bytes() + + +LFS_PAGE_CHANGE_CODE = """\ +import board +import os +import storage +import time +# The display's auto refresh keeps the supervisor tick running, which would +# flush the cache whether or not the filesystem is marked dirty. +board.DISPLAY.auto_refresh = False +storage.remount("/", readonly=False) +# littlefs writes new data to unused blocks, and writes to erased flash skip +# the page cache. Fill the free space and free it again so they don't. +f = open("/burn.bin", "wb") +chunk = bytes(8192) +try: + while True: + f.write(chunk) +except OSError: + pass +f.close() +os.remove("/burn.bin") +# Creating the file syncs, so the cache holds the metadata page. The first +# block of the file is on another page, so writing it changes pages. Don't +# flush or close the file. +f = open("/z.bin", "wb") +f.write(b"Z" * 512) +# The background flush runs after 1000 supervisor ticks. native_sim's 100 Hz +# kernel tick makes that about 10 seconds. +time.sleep(15) +print("slept") +while True: + time.sleep(1) +""" + + +@pytest.mark.circuitpy_drive({"code.py": LFS_PAGE_CHANGE_CODE}) +@pytest.mark.parametrize("board", ["native_native_sim_lfs"], indirect=True) +def test_flash_background_flush_after_page_change_lfs(circuitpython): + """Test that the background flush writes the page cached by a littlefs write. + + littlefs marks the filesystem dirty before writing, so the page change + must not clear the dirty flag. The file is never flushed or closed, so + only the background flush can write the block before the simulator is + stopped. + """ + circuitpython.serial.wait_for("slept") + circuitpython.shutdown() + + assert b"Z" * 512 in circuitpython.flash_file.read_bytes() + + SCRATCH_PAGE_CODE = """\ import gc import storage diff --git a/supervisor/shared/flash.c b/supervisor/shared/flash.c index 61a6f03c444..ef7c32aadd9 100644 --- a/supervisor/shared/flash.c +++ b/supervisor/shared/flash.c @@ -178,27 +178,31 @@ static mp_uint_t flash_write_blocks(mp_obj_t self_in, const uint8_t *src, uint32 // can't write MBR, but pretend we did return 0; } else { - supervisor_flash_mark_dirty(); block_num -= PART1_START_BLOCK; #if CIRCUITPY_SAVES_PARTITION_SIZE > 0 mp_vfs_blockdev_t *self = (mp_vfs_blockdev_t *)self_in; block_num += self->offset / self->block_size; #endif - return supervisor_flash_write_blocks(src, block_num, num_blocks); + mp_uint_t result = supervisor_flash_write_blocks(src, block_num, num_blocks); + // Mark dirty after the write, so a flush running in another thread + // can't clear the flag between marking and caching the block. + supervisor_flash_mark_dirty(); + return result; } } void PLACE_IN_ITCM(supervisor_flash_flush)(void) { + // Clear the flag before flushing. A write that lands during the flush + // marks the filesystem dirty again and turns the tick back on. + if (filesystem_dirty) { + filesystem_dirty = false; + supervisor_disable_tick(); + } #if INTERNAL_FLASH_FILESYSTEM port_internal_flash_flush(); #else supervisor_external_flash_flush(); #endif - // Turn off ticks now that our filesystem has been flushed. - if (filesystem_dirty) { - supervisor_disable_tick(); - } - filesystem_dirty = false; } static mp_obj_t supervisor_flash_obj_readblocks(mp_obj_t self, mp_obj_t block_num, mp_obj_t buf) {