Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rethinking NorFlash and writing abilities #58

Open
diondokter opened this issue Aug 4, 2024 · 3 comments
Open

Rethinking NorFlash and writing abilities #58

diondokter opened this issue Aug 4, 2024 · 3 comments

Comments

@diondokter
Copy link
Contributor

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:

  • Put bounds on functions so people can only call them with a flash that has the correct write ability
  • Have general APIs that can still respond to the write abilities (to e.g. adapt the memory layout to thee abilities)

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:

  • Write once
  • Write twice, second write is bitwise AND

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
Copy link
Contributor Author

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:

  • Read and write wordsize are not always equal to each other
  • Writing can happen in three-ish ways
    • Write small word
    • Write big word (block)
    • Write small word to AND bits and big word (page) erase
      • Once
      • Twice, second to 0x00
      • Unlimited AND
  • Reading can happen in two ways
    • Read small word
    • Read big word (block)
  • Erasing can happen in three ways
    • Built-in to 0xFF, required
    • Built-in to 0x00, optional
    • Not supported/required but can be emulated

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:

  • This trait would encode all information necessary to write libraries that can support a wide range of memory devices.
  • Even libs that are made for a specific device type could trivially support other device types by doing some extra calls. i.e. norflash could emulate a block device if the lib also calls erase if WRITE_METHOD is AndWithPrevious.

Cons:

  • Complex trait, easy to make mistakes
  • No way in current rust to put where-bounds on data, so libs may need to do runtime asserts if they only support some device types (possibly in a const function)

@korken89
Copy link

korken89 commented Aug 9, 2024

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!

@diondokter
Copy link
Contributor Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants