rust-embedded-community / embedded-storage

An Embedded Storage Abstraction Layer
Apache License 2.0
68 stars 23 forks source link

Rethinking NorFlash and writing abilities #58

Open diondokter opened 3 months ago

diondokter commented 3 months ago

Hi all!

In #57 an extension was proposed to add a second write must be 0 ability. However, the way this is done doesn't really help anyone who wants to abstract over the different write abilities.

Personally for sequential-storage I want to do these two things:

This last one isn't possible at the moment, but hasn't been a big issue for me thus far.

Currently there are two (cap?)abilities:

It's worked ok so far, but when adding more abilities like Write twice, second write is 0, things get more complex. To support this in s-s without making things worse for real multiwrite flashes, we need to be able to query the abilities in general APIs that can't use the trait bounds (because they need to be general).

So, I propose we discuss something along these lines: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=4479663da7dd48eb5ad8402260d75d24

It's verbose sadly, but it does make everything work. If someone can come up with something better I'm all ears!

We need the chain of traits because const generics aren't stable yet and we can't put const bounds on our functions and structs. We need the associated consts so we can query the abilities on APIs that only use the NorFlash trait as a bound. (This would be solved by reflection, but alas)

This is now all implemented as an associated type to make the implementation less bug-prone.

Could this be a good direction to go in?

There's also #56 which proposes block devices like SD cards. Now I've been thinking about how to run s-s on SD cards and there's no good way since they would be separate traits. However, that might not be necessary with the write abilities I propose here. Maybe we can unify these all and make one trait based on the current NorFlash trait and add a Block write ability? I don't know, maybe there are reasons why that can't work...

diondokter commented 2 months ago

Let's create an overview of all(ish) different random(ish) access memory types and their properties to see if we can find a common usable interface:

overview - RAM - Small read words (1-16 bytes) - Small write words (1-16 bytes) - Uses addresses - No pages/blocks - No erase required - Standalone write (fully overwrites old values) - Write infinite times - (NOR) flash (like winbond or nordic internal) - Small read words (1-16 bytes) - Smallish write words (1-32 bytes) - Uses addresses - Big pages (2kB - 128kB) - Erase required per page, sets data to 0xFF - Dependent write (Logical AND of new and old value) - Write infinite times - (NOR) flash with ECC - Small read words (1-16 bytes) - Smallish write words (1-32 bytes) - Uses addresses - Big pages (2kB - 128kB) - Erase required per page, sets data to 0xFF - Dependent write (Logical AND of new and old value) - Write only once before next erase - (NOR) flash with ECC+ (Many STM intern flash) - Small read words (1-16 bytes) - Smallish write words (1-32 bytes) - Uses addresses - Big pages (2kB - 128kB) - Erase required per page, sets data to 0xFF - Dependent write (Logical AND of new and old value) - Write twice before next erase, but second write can only be 0 - SD card (and NAND flash in general?) (block device) - Huge read words (512 bytes or more) - Huge write words (512 bytes or more) - Uses block indices - No pages, but blocks - No erase required, but available to set 0x00 - Standalone write (fully overwrites old values) - Write infinite times - EEPROM - Small read words (1-8 bytes) - Small write words (1-8 bytes) - Uses addresses - Small pages (equal to word size) so sort of a block device with small blocks? - Erase required per word, sets data to 0xFF - Dependent write (Logical AND of new and old value) - Write only once before next erase (or auto erase word) - SCSI (block device) (used in USB mass storage) - Huge read words (512 bytes typ) - Huge write words (512 bytes typ) - Uses block indices - No pages, but blocks - No erase required - Standalone write (fully overwrites old values) - Write infinite times - And more, but those are often block devices like SCSI I think

Let's summarize what we see:

Not mentioned is memory reliability with regards to bitflips and corruption.

This is all information that could be encoded into a trait:

trait Storage {
    const READ_SIZE: usize;
    const WRITE_SIZE: usize;
    const ERASE_SIZE: Option<usize>;

    const IS_BLOCK_DEVICE: bool = READ_SIZE == WRITE_SIZE && match ERASE_SIZE { Some(READ_SIZE) => true, _ => false };

    const WRITE_METHOD: WriteMethod;
    const WRITE_MAX_COUNT: WriteMaxCount;
    const ERASE_VALUE: u8; // 0xFF or 0x00

    fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<(), Self::Error>;
    fn capacity(&self) -> usize;
    fn erase(&mut self, from: u32, to: u32) -> Result<(), Self::Error>;
    fn write(&mut self, offset: u32, bytes: &[u8]) -> Result<(), Self::Error>;
}

enum WriteMethod {
    Direct, // No erase required
    AndWithPrevious, // So erase required
}

/// How many times can a word be written before erase is required?
enum WriteMaxCount {
    Once,
    TwiceSecondZero,
    Infinite,
}

Block devices would use the address of each block instead of a block index. Also, no more readonly devices. I've never seen that exist that uses the existing traits.

Pros:

Cons:

korken89 commented 2 months ago

Thank you for doing the analysis!

Personally I find this less complex than what embedded-storage is going right now, as with a single source of truth for any flash device you do not need to check which of the extension traits it implements. It's all there.

I did a small test on const asserting on the proposed trait and it works well. :+1: While you cannot see the bounds in auto-generated interface documentation for some implementation is a minus, where the current way would simply be manually written. Not great, not terrible.

Looking forward to more discussions on this!

diondokter commented 2 months ago

Thank you for doing the analysis!

Personally I find this less complex than what embedded-storage is going right now, as with a single source of truth for any flash device you do not need to check which of the extension traits it implements. It's all there.

I did a small test on const asserting on the proposed trait and it works well. 👍 While you cannot see the bounds in auto-generated interface documentation for some implementation is a minus, where the current way would simply be manually written. Not great, not terrible.

Looking forward to more discussions on this!

Ha, you're the only one with such a positive outlook on this. Most people in the embedded matrix chat thought it was way too complex.

So I'm gonna think about it some more and see if I can find a way to keep the flexibility while also supporting a wide range of storage types. But I also invite anyone to think along and come up with problems, solutions and proposals!