Low-level IO driver for serial flash memory via SPI and QSPI

[bgould]

Read functionality currently works using SPI, and QSPI on SAMD51. Will work on QSPI for nRF chips next

[bgould]

On atsamd51, to use regular SPI over the QSPI requires using a mechanism separate from SERCOM... however upon further investigation, nrf seems a little easier to use the existing SPI implementation, so I'm not going to focus on implementing QSPI on nrf right now. Instead I'm going to work on cleaning up the code I've already got working.

[conejoninja]

Great job! I tried the examples and work fine. I've been reading on this topic ( https://learn.adafruit.com/introducing-adafruit-itsybitsy-m4/circuitpython-storage ) and looks like it's needed to pull up some pins in order to get write permissions, at least in CircuitPython.

Have you looked into this too?

[deadprogram]

@conejoninja I think that is a feature of CircuitPython itself:

adafruit/circuitpython@1a0596a

We will probably need to implement something similar.

[bgould]

I did notice pieces of that in the CircuitPython implementation... I think that is mostly related to the functionality they have where they present the flash chip as a USB drive to the host computer. I think that they intend for most of the file to written from the host and read from CircuitPython.

I haven't gotten nearly so far yet; my intention for the scope of this driver is to be able to interface with SPI flash chips in a filesystem-agnostic way. Some applications like data logging might not need a filesystem at all, so this could be immediately useful for that. We should be able to support writing to/from the memory device and also write-protecting the device by just issuing the appropriate commands over SPI - https://github.com/tinygo-org/drivers/pull/124/files#diff-3eea81db4fb1722dbc92e4b8bcf45223R17

As far as next steps once the low level IO is working well enough, it might be possible to build some USB endpoints that allow for the host computer to format/write a FAT filesystem (or even just flash CircuitPython onto a board and write some files) and then I have some Go code that is mostly capable of reading those files (does not yet support writing though). I'll probably maintain that FAT library outside of the drivers repo for a while until it is more battle-tested. Eventually I would want to support a more full-featured solution of course, but this setup would at least provide a solution for things like being able to read wifi credentials from a file for example. CircuitPython/Micropython use a embedded FAT implementation written in C that would be useful for that purpose, but it is going to take me a bit to wrap my head around how that should work (maybe some smarter will figure it out first :P)

As @aykevl pointed out previously, there are also embedded filesystems that are easier to implement than FAT. I think he mentioned the micro:bit filesystem for instance, so I'm thinking if we can at least tackle the low-level IO first then we could go that direction as well.

[conejoninja]

Cool! Thanks for the explanation. I was a bit puzzled , but much clear now 👍

[aykevl]

I didn't go through it all but there is one main concern I have with the code as it is now: QSPI is implemented directly in the flash package. I think it should go into the machine package to make it more widely available for other use cases than SPI flash.

Is there a reason to couple the flash package and the samd51 QSPI peripheral?

[bgould]

I didn't go through it all but there is one main concern I have with the code as it is now: QSPI is implemented directly in the flash package. I think it should go into the machine package to make it more widely available for other use cases than SPI flash.

Is there a reason to couple the flash package and the samd51 QSPI peripheral?

This a good question. The QSPI peripheral for SAMD chips has 2 modes of operation; the first mode is as a general-purpose SPI peripheral that can be used with any sort of SPI device. The second mode (which is the mode used in this component) is specific to serial flash memory devices. In this mode, it is possible to perform single, dual, or quad bit transfers and to "memory map" the flash device into the 0x00400000-0x00500000 address space.

This first mode could definitely be made to fit into the current SPI abstraction, and would certainly make sense to have in the machine package if implemented. I'm not sure there is a ton of advantage of this peripheral over using a SERCOM for SPI, except that from what I can tell the QSPI in SPI mode can run at the same speed as the CPU and the chip select line is handled automatically by the hardware.

Since the second mode is really only suitable for serial flash memory, I thought it would make sense to keep it in this driver. I am fine with moving it out of here if that is preferred, would just need to brainstorm what the abstraction should look like for this mode of operation.

[aykevl]

Is there a reason to couple the flash package and the samd51 QSPI peripheral?

This a good question. The QSPI peripheral for SAMD chips has 2 modes of operation; the first mode is as a general-purpose SPI peripheral that can be used with any sort of SPI device. The second mode (which is the mode used in this component) is specific to serial flash memory devices.

Okay, thank you for explaining. It's not ideal but I agree it's best to keep it in the driver for now. Maybe there are other chips with a similar abstraction at which point it can be split up and moved into the machine package. Coming up with a good abstraction is almost impossible without at least two or three implementations to see whether the abstraction is the right one.

[aykevl]

I've added a few comments below.

One thing this PR really needs is better docs (especially for the public interface, but preferably also for internal functions). That not only makes the API easier to use, but also makes it easier to review and in my experience helps make the API itself better (if you can't explain a method in a few words, it may be that it is the wrong abstraction).

[aykevl]

Some questions:

1. What is the intended usage of this API? It currently has a relatively wide surface, and I wonder whether it can be made smaller and thus more generic. For example, it would be very useful to share the same interface with on-chip flash storage.
2. Is there a difference between EraseBlock and EraseSector, apart from the size? The APIs I looked at previously (for on-chip flash) only had a single page size, usually 1024 or 4096 bytes in size. No smaller area could be erased at a time.

[bgould]

I agree that it is possible and preferable to distill a smaller and more general interface out of this code, and also I agree 100% about having an implementation over on-chip flash (I was even considering if this package should be renamed to "spiflash" or something for that distinction). The main reason I was more focused on the low-level implementation at this time is that I thought it might be good to try implementing a couple of filesystems first to see if a sensible interface emerges out of that.

That said, the FAT library I've been messing with has this interface, which sort of covers the aspects you mentioned (implements ReaderAt/WriterAt, etc): https://github.com/bgould/go-fs/blob/tinygo/block_device.go

On the other hand, I've also been experimenting with LittleFS (https://github.com/ARMmbed/littlefs) which has a BlockDevice that looks more like this for its hardware callbacks:

type BlockDevice interface {
	ReadBlock(block uint32, offset uint32, buf []byte) error
	ProgramBlock(block uint32, offset uint32, buf []byte) error
	EraseBlock(block uint32) error
	Sync() error
}

That can probably be made to play nice with the aforementioned interface (as long as Sync() is accounted for)

Also I was planning to look into SPIFFS next which has roughly this interface for its callbacks (https://github.com/pellepl/spiffs/wiki/Integrate-spiffs):

  void my_spi_read(int addr, int size, char *buf)
  void my_spi_write(int addr, int size, char *buf)
  void my_spi_erase(int addr, int size)

so that is basically ReaderAt+WriterAt as you suggested, plus a method for erase

I will keep those things in mind and see if I can finish getting some filesystem code written to prove out what a small set of common functionality might look like, and then if you prefer I can go through this low-level driver and un-export functions that do not necessarily need to be exposed.

[aykevl]

Thank you for the detailed explanation!

I've read a bit more about flash sizes (including this post) and it seems like:

• "page size" refers to the smallest writable area. Because this is NOR flash, if you need to write a smaller area you have to extend it on both sizes with all-ones to emulate writing to a part of it (zeroes will not be set back to ones after being written).
• "sector size" refers to the smallest erasable area.
• "block size" is a larger area that can be erased, probably as an optimization.

So what do you think of this interface?

type BlockFlash interface {
    // Size returns the size of this memory, in bytes.
    Size() int64

    // ReadAt reads the given number of bytes from the flash storage.    
    io.ReaderAt    
    
    // WriteAt overwrites an area of flash with the given data. The to be
    // written area must first be erased using EraseBlocks, the behavior is
    // otherwise memory dependent. Some flash memories will only set ones back
    // to zeroes, others may have a different behavior.
    // This function should work correctly for any address and buffer size, but
    // clients may use WriteBlockSize as an optimization.
    io.WriterAt
    
    // WriteBlockSize returns the block size in which data can be written to
    // memory. It can be used by a client to optimize writes, non-aligned writes
    // should always work correctly.
    // It must be a power of two, and may be as small as 1. A typical size is 256.
    WriteBlockSize() int64
    
    // EraseBlockSize returns the smallest erasable area on this particular chip
    // in bytes. This is used for the block size in EraseBlocks.
    // It must be a power of two, and may be as small as 1. A typical size is 4096.
    EraseBlockSize() int64
    
    // EraseBlocks erases the given number of blocks. An implementation may
    // transparently coalesce ranges of blocks into larger bundles if the chip
    // supports this. The start and len parameters are in block numbers, use
    // EraseBlockSize to map addresses to blocks.
    EraseBlocks(start, len int64) error
}

I have no strong feelings about the particular naming of methods, but would like to make it as generic as reasonably possible (this is why I called EraseBlockSize that way and not SectorSize).
I don't have a lot of experience with SPI flash but I did look at on-chip flash in the past and that kind of flash tends to have very small write block sizes (just 2 or 4 bytes for the chips I looked at).

I used int64 for offsets instead of uint32. This is for future extensibility and to match io.ReaderAt, the Size() method, etc. Using big integers is generally quite fast: it mainly requires a bit more storage and perhaps twice the operations. The only thing that is much slower is divide, but only if the compiler cannot optimize it to a multiply/bitshift/etc.

I intentionally used block numbers instead of addresses in EraseBlocks. This shields the SPI flash implementation from having to think about non-aligned cases with associated error conditions and forces the client to actually call EraseBlockSize to get the right block size. The block size will likely be core to the design of the filesystem so it's best to make that explicit. Also, by giving a range of blocks the implementation is free to use a larger erase size for consecutive blocks if that fits in the range.

The main reason I was more focused on the low-level implementation at this time is that I thought it might be good to try implementing a couple of filesystems first to see if a sensible interface emerges out of that.

That's a very smart choice. Maybe we should wait a bit until pinning down one particular API.

[deadprogram]

Any action on this PR? I just really really want this functionality. 🔢

[bgould]

@deadprogram I've been testing this out with https://github.com/bgould/go-littlefs for a bit and its working pretty well, especially for simple/common use cases. I have a few tweaks to make as per the suggestions from @aykevl above, but those should be quick and then I think this can be reviewed/merged. I'll try to get wrapped up in the next couple of days.

[deadprogram]

That is very exciting!

[bgould]

This is ready for review please. I've incorporated the suggestions from @aykevl above, except that I did not actually declare the BlockFlash interface in the package yet. The flash.Device struct does satisfy the suggested interface however, so it should be simple to add that later if it turns out that is the contract we'd like use for flash block devices.

This can be tested using examples/flash/console/spi (should work out of the box with Itsy Bitsy M0) and examples/flash/console/qspi (should work out of the box with PyPortal or Itsy Bitsy M4). This example allows for querying the details about the chip and performing some low-level operations.

More practical examples how a filesystem can be implemented on top of this driver can be found at github.com/bgould/go-littlefs/tree/master/examples/flash/spi or github.com/bgould/go-littlefs/tree/master/examples/flash/qspi - these examples are similar to the ones above except that there commands like ls, cat, mkdir, etc for operating on files. You'll first want to format and mount in order to initialize the filesystem on a new chip. Also using those examples require the dev branch of TinyGo with this patch included: tinygo-org/tinygo#997

[aykevl]

The API looks good to me. Thank you for updating!
Can you also add smoke tests to the Makefile? One for the SPI example and one for the QSPI example. This makes sure the most obvious breakage will be caught when it happens.

except that I did not actually declare the BlockFlash interface in the package yet.

That's fine, the interface can be added at a later time.

[aykevl]

This is in fact the right way to make code byte order independent. Based on this code the serial number appears to be sent in little endian form, is that correct?
(As an aside, TinyGo does not yet support any big endian architectures and I'm not aware of a major embedded architecture that's big endian).

[aykevl]

@bgould I've experimented with the ability to mount filesystems in the os package, see tinygo-org/tinygo#1012. Any feedback would be appreciated (whether it works for littlefs and whether we would need such an abstraction at all).

[bgould]

Hi @aykevl I am taking a look at tinygo-org/tinygo#1012 and will add some comments over there, I think it is a good idea and should be very useful IMO.

[bgould]

What do you think, can this be merged? I think this is in a pretty good state for implementing filesystems on top of.

[deadprogram]

This is just amazing work @bgould thank you for another impressive contribution.

Now squash/merging, I assume there will be another PR to add some docs on "how to wire everything up into a usable setup" as mentioned #45 (comment)