SAMD: Support SAMD internal flash and QSPI flash simultaneously

[ironss-iotec]

Description

My application needs access to raw flash regions in both QSPI flash and SAMD internal flash, as well as a LittleFS filesystem in QSPI flash. The current implementation of modsamd.c prevents this.

The purpose is to allow safe remote (over the air) firmware upgrades. The application (rather than a bootloader) downloads firmware into a filesystem, verifies its integrity, writes the new version into a separate area of flash, then reboots.

To write the new image, it need access to raw flash blocks, not via a filesystem.

The bootloader identifies that a different version exists in the separate flash area, checks its integrity, swaps the old and new versions ensuring that it can recover from an unexpected power failure, then starts the new version. This is all handled by the bootloader (MCUboot).

The bootloader can load the new image from either MCU flash or QSPI flash. Currently, it works with internal flash, but adding support for QSPI flash is not too hard.

Micropython v1.23.0 does not have the required functionality to do its part.

• The flash drivers samd_flash.c and samd_qspiflash.c (and samd_spiflash.c) have two limitations

  • they only expose the VFS protocol methods, and hide the methods that operate on raw flash blocks
  • they do not support the concept of separate flash regions, instead allocating the entire flash device for the VFS [1]. This is different from the PyBoard Flash() device, which allow keyword parameters start and len to define separate regions and get separate instances.
    • [1] Well, samd_flash.c uses the left-over internal flash that is not used by the binary image

• The module samd allows only a single flash hardware device

  • It allows only one of MICROPY_HW_MCUFLASH, MICROPY_HW_SPIFLASH or MICROPY_HW_QSPIFLASH to be defined,
  • If more than one of them is defined in mpconfigboard.h, it causes a compile error.
  • The single device is called samd.Flash()

Please consider changes to samd_flash.c, samd_qspiflash.c, samd_spiflash.c, mod_samd.c and perhaps mpconfigboard.h

• Modify samd_flash.c, samd_qspiflash.c (and samd_spiflash.c) to operate on separate regions of flash, by implementing the keyword parameters start and len, and returning a dynamically-allocated object, rather than the singleton they currently return. These separate regions can then be passed to a VFS driver.
• Allow access to raw flash blocks in one of two ways:
  • Use the VFS methods to read and write flash sectors.
  • Modify samd_flash.c, samd_qspiflash.c (and samd_spiflash.c) to expose methods that operate on raw flash blocks
• It is easy to modify modsamd.c to allow any or all of the MICROPY_HW_*FLASH to be defined, giving each device an appropriate class name (samd.MCUFlash, samd.SPIFlash and samd.QSPIFlash).
  • If none is enabled, force MICROPY_HW_MCUFLASH to be enabled.
• It is slightly more tricky to keep the class samd.Flash for backward compatibility. Not difficult, just a bit of preprocessor magic
  • A new configuration point (MICROPY_HW_FLASH_TYPE) that specifies which should be used.
  • If specified, ensure that the corresponding type is enabled
  • If not specified, arbitrarily choose one of the types that is enabled

Code Size

• Code size will increase depending on which flash types are enabled. This is unavoidable and to be expected.
• Trivial change in code-size if only one flash type is enabled -- only one extra entry in the module-globals table, and even that could be removed.

Implementation

I intend to implement this feature and would submit a Pull Request if desirable

Code of Conduct

Yes, I agree

[ironss-iotec]

To start, a patch to allow any or all of samd.MCUFlash(), samd.SPIFlash() and samd.QSPIFlash() (corresponding to MICROPY_HW_MCUFLASH, MICROPY_HW_SPIFLASH and MICROPY_HW_QSPIFLASH respectively), and select one of them as samd.Flash().

This gets part of the way there. I modify the linker scripts (symbols _oflash_fs and _sflash_fs) to ensure that MCUFlash() has access to the appropriate MCU flash area, and just take care not to overwrite the bootloader and running application. This is enough to write a new application image to the second half of MCU flash, and to test the bootloader. Just don't try to mount a filesystem on MCUFlash().

modsamd.c.patch

The first part of the first chunk checks which flash-type to use as samd.Flash(), and ensures that the corresponding type is enabled.

The second part of the first chunk declares the object types for the enabled flash types.

The final chunk adds the relevant names to the module-globals dictionary

diff --git a/ports/samd/modsamd.c b/ports/samd/modsamd.c
index 2afe1d122..c842ff418 100644
--- a/ports/samd/modsamd.c
+++ b/ports/samd/modsamd.c
@@ -32,19 +32,41 @@
 #include "pin_af.h"
 #include "samd_soc.h"
 
-#if MICROPY_HW_MCUFLASH
+
+#if MICROPY_HW_FLASH_TYPE == MCUFLASH
+#define FLASH_TYPE samd_flash_type
+#ifndef MICROPY_HW_MCUFLASH
+#define MICROPY_HW_MCUFLASH
+#endif
+
+#elif MICROPY_HW_FLASH_TYPE == QSPIFLASH
+#define FLASH_TYPE samd_qspiflash_type
+#ifndef MICROPY_HW_QSPIFLASH
+#define MICROPY_HW_QSPIFLASH
+#endif
+
+#elif MICROPY_HW_FLASH_TYPE == SPIFLASH
+#define FLASH_TYPE samd_spiflash_type
+#ifndef MICROPY_HW_SPIFLASH
+#define MICROPY_HW_SPIFLASH
+#endif
+
+#endif
+
+
+#ifdef MICROPY_HW_MCUFLASH
 extern const mp_obj_type_t samd_flash_type;
-#define SPIFLASH_TYPE samd_flash_type
 #endif
+
 #ifdef MICROPY_HW_QSPIFLASH
 extern const mp_obj_type_t samd_qspiflash_type;
-#define SPIFLASH_TYPE samd_qspiflash_type
 #endif
-#if MICROPY_HW_SPIFLASH
+
+#ifdef MICROPY_HW_SPIFLASH
 extern const mp_obj_type_t samd_spiflash_type;
-#define SPIFLASH_TYPE samd_spiflash_type
 #endif
 
+
 static mp_obj_t samd_pininfo(mp_obj_t pin_obj) {
     const machine_pin_obj_t *pin_af = pin_find(pin_obj);
     #if defined(MCU_SAMD21)
@@ -78,8 +100,24 @@ static MP_DEFINE_CONST_FUN_OBJ_1(samd_pininfo_obj, samd_pininfo);
 
 static const mp_rom_map_elem_t samd_module_globals_table[] = {
     { MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_samd) },
-    { MP_ROM_QSTR(MP_QSTR_Flash),    MP_ROM_PTR(&SPIFLASH_TYPE) },
-    { MP_ROM_QSTR(MP_QSTR_pininfo),  MP_ROM_PTR(&samd_pininfo_obj) },
+
+#ifdef FLASH_TYPE
+    { MP_ROM_QSTR(MP_QSTR_Flash),    MP_ROM_PTR(&FLASH_TYPE) },
+#endif
+
+#ifdef MICROPY_HW_MCUFLASH
+    { MP_ROM_QSTR(MP_QSTR_MCUFlash),    MP_ROM_PTR(&samd_flash_type) },
+#endif
+
+#ifdef MICROPY_HW_QSPIFLASH
+    { MP_ROM_QSTR(MP_QSTR_QSPIFlash),    MP_ROM_PTR(&samd_qspiflash_type) },
+#endif
+
+#ifdef MICROPY_HW_SPIFLASH
+    { MP_ROM_QSTR(MP_QSTR_SPIFlash),    MP_ROM_PTR(&samd_spiflash_type) },
+#endif
+
+	{ MP_ROM_QSTR(MP_QSTR_pininfo),  MP_ROM_PTR(&samd_pininfo_obj) },
 };
 static MP_DEFINE_CONST_DICT(samd_module_globals, samd_module_globals_table);

Except from mpconfigboard.h might look like

#define MICROPY_HW_QSPIFLASH        IS25LPWP064D
#define MICROPY_HW_MCUFLASH         (1)
#define MICROPY_HW_FLASH_TYPE       MCUFLASH

[ironss-iotec]

Some more testing shows that I can simulate raw-block access with minor changes to the SAMD flash driver (samd_flash.c, samd_spiflash.c and samd_qspiflash.c).

These changes do NOT provide proper partitions. Rather, they allow the application to specify where in flash the filesystem will be, then use the standard blockdev functions to access flash outside the filesystem. The application must take care not to write on the filesystem region.

• Add optional parameters start and len to the ..._make_new() function, with default values -1, identical to the Pyboard pyb/storage.c
• After finding out the actual size of flash, set some new values in samd_flash_obj_t, spiflash_obj_t and qspiflash_obj_t
  • self->vfs_start is set to the start value, or to 0 if the value is -1
  • self->vfs_size is set to the len value, or to self->size if the value is -1
• Modify readblocks() and writeblocks(), adding self->vfs_start to offset
• Modify ioctl(6) (erase block), adding self->vfs_start to the offset
• Modify ioctl(4) (number of blocks) to use self->vfs_size, rather than self->size

This allows the application to set the location of the filesystem in each flash device.

The blockdev functions readblocks(), writeblocks() and ioctl(6) eraseblocks() do not check their parameter values. So it is possible to use block numbers outside the range used by the VFS to operate on flash blocks not used by the filesystem. This includes using a negative block number to access flash with a lower address than the filesystem region.

One further improvement is to prevent changing the values of vfs_start and vfs_size once they have been set. This allows one chunk of code to call samd.QSPIFlash(start=0x00400000, len=0x00400000) to set the start and length of the filesystem region to the last 4 MB of an 8 MB flash. Later code can call x = samd.QSPIFlash() without parameters, to operate on the flash without changing the location of the filesystem in flash.

Problems with this implementation

• Abusing the blockdev functions. If they later get parameter checking, this hack will stop working.
• Subsequent calls to samd.QSPIFlash() get an instance that will work. However, they have no way to discover what area of flash is used by the filesystem, so they cannot be made safe.

Once again, this is not a safe implementation. However, it does get me unstuck, and might provide ideas for a future implementation.

Changes to samd/samd_qspiflash.c

diff --git a/ports/samd/samd_qspiflash.c b/ports/samd/samd_qspiflash.c
index d027a0495..ed96885dc 100644
--- a/ports/samd/samd_qspiflash.c
+++ b/ports/samd/samd_qspiflash.c
@@ -96,6 +96,8 @@ typedef struct _samd_qspiflash_obj_t {
     uint32_t size;
     uint8_t phase;
     uint8_t polarity;
+    uint32_t vfs_start;
+    uint32_t vfs_size;
 } samd_qspiflash_obj_t;
 
 /// List of all possible flash devices used by Adafruit boards
@@ -258,10 +260,21 @@ int get_sfdp_table(uint8_t *table, int maxlen) {
 }
 
 static mp_obj_t samd_qspiflash_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) {
-    mp_arg_check_num(n_args, n_kw, 0, 0, false);
+    // Parse arguments
+    enum { ARG_start, ARG_len };
+    static const mp_arg_t allowed_args[] = {
+        { MP_QSTR_start, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = -1} },
+        { MP_QSTR_len,   MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = -1} },
+    };
+    mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
+    mp_arg_parse_all_kw_array(n_args, n_kw, all_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
     // The QSPI is a singleton
     samd_qspiflash_obj_t *self = &qspiflash_obj;
+    if (self->size != 0) {
+    	return self;
+    }
+
     self->phase = 0;
     self->polarity = 0;
     self->pagesize = PAGE_SIZE;
@@ -334,6 +347,16 @@ static mp_obj_t samd_qspiflash_make_new(const mp_obj_type_t *type, size_t n_args
 
     self->size = flash_device->total_size;
 
+	self->vfs_start = 0;
+	self->vfs_size = self->size;
+
+	if (args[ARG_start].u_int != -1) {
+		self->vfs_start = args[ARG_start].u_int;
+	}
+	if (args[ARG_len].u_int == -1) {
+		self->vfs_size = args[ARG_len].u_int;
+	}
+
     // The write in progress bit should be low.
     while (read_status() & 0x01) {
     }
@@ -419,7 +442,7 @@ static mp_obj_t samd_qspiflash_erase(uint32_t addr) {
 
 static mp_obj_t samd_qspiflash_readblocks(size_t n_args, const mp_obj_t *args) {
     samd_qspiflash_obj_t *self = MP_OBJ_TO_PTR(args[0]);
-    uint32_t offset = (mp_obj_get_int(args[1]) * self->sectorsize);
+    uint32_t offset = (mp_obj_get_int(args[1]) * self->sectorsize) + self->vfs_start;
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[2], &bufinfo, MP_BUFFER_WRITE);
     if (n_args == 4) {
@@ -435,7 +458,7 @@ static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(samd_qspiflash_readblocks_obj, 3, 4,
 
 static mp_obj_t samd_qspiflash_writeblocks(size_t n_args, const mp_obj_t *args) {
     samd_qspiflash_obj_t *self = MP_OBJ_TO_PTR(args[0]);
-    uint32_t offset = (mp_obj_get_int(args[1]) * self->sectorsize);
+    uint32_t offset = (mp_obj_get_int(args[1]) * self->sectorsize) + self->vfs_start;
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[2], &bufinfo, MP_BUFFER_READ);
     if (n_args == 3) {
@@ -463,11 +486,11 @@ static mp_obj_t samd_qspiflash_ioctl(mp_obj_t self_in, mp_obj_t cmd_in, mp_obj_t
         case MP_BLOCKDEV_IOCTL_SYNC:
             return MP_OBJ_NEW_SMALL_INT(0);
         case MP_BLOCKDEV_IOCTL_BLOCK_COUNT:
-            return MP_OBJ_NEW_SMALL_INT(self->size / self->sectorsize);
+            return MP_OBJ_NEW_SMALL_INT(self->vfs_size / self->sectorsize);
         case MP_BLOCKDEV_IOCTL_BLOCK_SIZE:
             return MP_OBJ_NEW_SMALL_INT(self->sectorsize);
         case MP_BLOCKDEV_IOCTL_BLOCK_ERASE: {
-            samd_qspiflash_erase(mp_obj_get_int(arg_in) * self->sectorsize);
+            samd_qspiflash_erase((mp_obj_get_int(arg_in) * self->sectorsize) + self->vfs_start);
             // TODO check return value
             return MP_OBJ_NEW_SMALL_INT(0);
         }

[robert-hh]

As you have noticed in your way through the sources, the three block device drivers for internal flash, QSPI flash and SPI flash are independent. And in the initial PR they were added independently to the firmware, such that internal flash + QSPI flash or internal flash + SPI flash were configured, depending on the respective hardware. The default flash system to be used for the files wad selected in _boot.py. While that seemed flexible, it was of minor practical use, so for simplicity the decision was made to use one one of them in a configuration for a file system.

Notes:

• The location and size of the area available for the internal file system is defined in the Makefiles and loader files.
• Since the SAMD port officially does support only UART or USB as bidirectional communication methods, it requires an external instance for over-the-air firmware updates. That one can as well enforce a firmware update. Unless however you use one of the PRs for WiFi support.
• The internal flash is in most cases too small to hold two copies of the firmware. So you have to use the external flash for the temporary storage of the firmware. I understand that then the flashing of the new firmware image to the internal flash must be done by the special bootloader, which is different from the MicroPython firmware, which for simplicity just uses the block protocol. But that will have to use it's own copies of the block device drivers. Meaning that the only requires change would be to make the location and size of the VFS file system in the external flash devices configurable.

Question: Which hardware do you use for your project,

[ironss-iotec]

You are right: there are two separate problems. I was conflating the two, because of the way I got there.

• Does my application really need access to both MCU flash and (Q)SPI flash?
• How can I use only part of a flash device as VFS storage, and access the remainder as raw flash blocks?

Hardware is ATSAME51J20A with 1 MB internal flash, a QSPI external flash, and a cellular modem for communication (including OTA firmware updates).

We need to store the bootloader, two copies of application (running version and new version) and VFS data somewhere in non-volatile storage, either MCU or QSPI flash, with some obvious constraints.

A month ago, our design intended to store the custom bootloader and two copies of firmware in the MCU flash, and use the entire QSPI flash for VFS. In this design, we needed access to both MCU flash and QSPI flash: raw block access to MCU flash and only VFS blockdev access to QSPI.

Further development shows that it is tight to fit two copies of firmware into 1M of MCU flash: each copy uses 85% of its allocated space; our preference is for less than 50% usage when the product is new.

Our new design proposes to store only the bootloader and the running copy of firmware in the MCU flash, and the second copy of firmware and VFS in QSPI flash. In this design

• We need to allocate only part of the QSPI device to the blockdev, and provide raw access to the remainder.
• We probably do not need access to MCU flash. The bootloader can provide various status flags for the application, and can read various configuration settings from the application, communicated via flash. but I think we can use the QSPI flash for that.

The changes needed are then something like those in the second patch

• pass optional arguments to samd_qspiflash_make_new() to set the VFS size and start address, defaulting to entire-device and start address 0 respectively
• modify samd_qspiflash_readblocks(), samd_qspiflash_writeblocks() and samd_qspiflash_ioctl() to use the VFS offset
• the application can then use iotcl(6, block_num) to erase blocks, and Flash.readblocks() and Flash.writeblocks() to read and write the flash.

[ironss-iotec]

It turns out that about 50 lines of Micropython gives me a Partition() class that lets me set the offset and size of a number of partitions in a Flash(). The Partition() class

• uses the Flash() blockdev API to read, write and erase the flash hardware
• provides a raw-flash-like API to read, write and erase at any offset in its allocated region, calculating the block_num to pass to the underlying Flash() blockdev.
• provides a blockdev API (readblocks, writeblocks and ioctl) that gives the size of the partition (rather than the raw flash), and calculates the block_num to pass to the underlying Flash() blockdev

I mount the LFS filesystem on one of the Partition()s, and use the other partitions as raw flash blocks.

This means that no change is needed to samd_qspiflash.c; I just needed to adjust my expectations.

At present, I don't need access to both MCU flash and QSPI flash. If that changes, I will start a new discussion about that change.

Thank you for your constructive thoughts.