Relevant Matrix discussion

[MathiasKoch]

First up

For reference around the current implementation of this crate: rust-embedded/embedded-hal#248

Matrix discussion based on this:

I just wanted to anchor this discussion here, as i think it is very relevant for this repo:

@MathiasKoch

Just circling back to my storage traits from before, do you have any suggestions to make the erase function more usable? I think my main problem is that i have no knowledge of the erase resolution of the implementor..

@Dirbaio

dunno, maybe have it take an address range
and return error if it can't erase that range
ie if flash has 4kb pages, start and end should be page-aligned

@MathiasKoch

Yeah, that was my suggestion as well. I guess it could work, but it kinda runs into the same "alignment" issues

@Dirbaio

yeah :(
and maybe add a funcition where the user can request what's the erase size
but to complicate it more, there are flahes with multiple erase sizes..

@MathiasKoch

Yeah

@Dirbaio

mx25r6435f can erase 64kb, 32kb or 4kb
and doing a 64kb erase is faster than 16 4kb erases

@MathiasKoch

maybe a const EraseSizes: [u8] as an associated const?

@Dirbaio

and I've seen somewhere a flash that had differently-sized blocks
can't remember where

@MathiasKoch

Yeah, i have one of those :p

@Dirbaio

but like
it has N 4kb blocks and M 16kb blocks
and the 4kb blocks must be erased with a 4kb erase and the 16kb blocks must be erased with a 16kb erase
so you can't even say the flash has an "erase size" :D

@MathiasKoch

Ahh.. true!
Cause the traits i have currently suggested should support differently-sized blocks fully, but at the expense of only having an "erase all"..
That might be okay though, as write can be implemented to check and erase rather easy, due to the added iterator helpers
ie

fn try_write(&mut self, address: Address, bytes: &[u8]) -> nb::Result<(), Self::Error> {
    for (block, page, addr) in self.memory_map.pages().overlaps(bytes, address) {
        let merge_buffer = &mut [0u8; MAX_PAGE_SIZE][0..page.size];
        let offset_into_page = addr.0.saturating_sub(page.location.0) as usize;

        self.try_read(page.location, merge_buffer)?;

        if block.is_subset_of(&merge_buffer[offset_into_page..page.size]) {
            self.write_bytes(block, addr)?;
        } else {
            self.erase_page(&page)?;
            merge_buffer
                .iter_mut()
                .skip(offset_into_page)
                .zip(block)
                .for_each(|(byte, input)| *byte = *input);
            self.write_bytes(merge_buffer, page.location)?;
        }
    }

    Ok(())
}

@Dirbaio

not sure if implementing "write" with "read modify write" is a good idea
if you're writing a single word in a page, you'd expect that if the device powers down mid-write, only that word will become garbage

@MathiasKoch

Well, it's a necessity some places.. NOR flash eg

@Dirbaio

but if the driver does RMW behind your back, the entire page can become garbage
if you as a user know the page is erased it's OK to write word-by-word
and you can even write the same word multiple times, the result is the bitwise AND
the nrf52 internal flash explicitly allows this, for example
not sure if the trait should abstract that
or maybe there's room for a trait that does and a trait that doesn't hahaha
my firmware uses a key-value flash store that requires the flash hal to allow word-writes
it's really hard (impossible?) to atomically write records otherwsie

@MathiasKoch

I don't think the trait itself should, but the implementation should.. So when implemented for a NOR flash i think it makes sense to do so..
That said i agree with you, it probably shouldn't for an internal flash.
If it's not that way, i think we loose the whole point of embedded-hal traits (the drop-in replaceability)

@Dirbaio

if the impl does RMW, it becomes useless for many usecases such as that one

@MathiasKoch

But if a driver author can't call try_write with a byte slice, and expect it to be written to "some" storage, without having knowledge of which kind of storage that is (NOR flash, NAND flash, eeprom, internal flash etc), i think the traits become utterly useless.

Eg a simple try_write to a NAND flash, should probably contain nothing but alignment checks and a write (atomically?), but the same write to a NOR chip, would have to do RMW, as you cannot write high bits

@Dirbaio

maybe then have a "NorFlash" trait specifiying "erase sets everything to 0xFF, write does AND"
and a "Flash" trait specifying "write overwrites whatever was before. Careful that it may do RMW if the device is a NOR flash"

@MathiasKoch

And how would i target those traits as a driver author?

@Dirbaio

and even a generic impl that gives you a Flash from a NorFlash
well if you're writing a driver for a NOR flash, implement NorFlash
and let the user use the generic adapter to Flash if he wants RMW..?

@MathiasKoch

Nono, the highlevel user author.. just wanting some non-volatile storage
Sorry, might not be "driver"..
middleware author?

@whitequark

is it a good idea to have a trait for all kinds of flash?
even further, is it a good idea to have traits specifically about flash?

@MathiasKoch

I don't think it is, i would much rather just have a single "storage" trait of some kind

@Dirbaio

I'm halfway done writing a "key-value database" for NOR flash
that specifically takes advantage of NOR flash properties for better efficiency
(for example, to delete a record I overwrite its magic header with another magic header that just changes 1s to 0s)
that would definitely benefit from a "NorFlash" trait
so you can run it on top of any NorFlash

@whitequark

oh yeah that makes sense

@MathiasKoch

Hmm..

@Dirbaio

an alternative would be a "BlockDevice" trait
where you can only overwrite full blocks
and if the write gets interrupted the block can be garbage (ie writes are not atomic)
that's like the "block device" concept of Linux
and yet another alternative would be a generic "Storage" trait
that does RMW
but doing RMW makes it quite useless for writing databases / filesystems, because you can't know how much data can be trashed by an interrupted write

@MathiasKoch

I think my point would be that i would like to see some "default"/super-trait that is basically a "give me some non-volatile storage" trait. That said, i think it could be made up of "sub-traits" with default implementations. That way a crate using the special features of eg NOR, can have a bound on only Nor, but crates that just want any storage can bound on the "super" trait?

@Dirbaio

yeah that makes sens
highlevel "storage" trait
lowlevel "NorFlash" trait
and if you're writing a driver, you just need to implement NorFlash

@MathiasKoch

Yeah, and a default impl Storage for T where T: NorFlash that "adds" RMW

@Dirbaio

the hal could have a NorFlashStorage struct that takes a NorFlash and impls Storage
impl Storage for NorFlash would work too, but then drivers can't have their own Storage impl if the hardware has some faster way or something

@MathiasKoch

Hmm... Specialization where are you? :p

@Dirbaio

hahaha

@whitequark

it's unclear if "give me some non-volatile storage" works in general
though maybe i'm pessimistic
do you really want a completely transparent FTL?
sure, thumbdrives exist, but they also fail in really bad ways because they're fully transparent

@MathiasKoch

I don't see why it shouldn't? Of course with the tradeoff that there will be lots of cases where the result is sub-optimal
FTL?

@whitequark

flash translation layer

@MathiasKoch

ahh

@Dirbaio

but even these FTLs are block-level, not byte-level
they let you overwrite a block, not a byte
512b / 4kb

@whitequark

FTLs get pretty weird
there are flashes with subpage writes for example
even if you limit yourself to only "simple" ONFI NAND configurations, it's still not exactly block level
Emil Karlson
there is no reason you could not have byte addressable FTL, most nand FTLs expose smallest logical write size smaller than smallest physical write size

@whitequark

because the RMW unit is larger than the block

@Dirbaio

very unfun

@whitequark

of course it's technically possible to have an unifying trait that abstracts over all kinds of flash
in API terms
the issue is that you need different algorithms to work with those, so having this API might not be very helpful
can you put plain FAT on NOR flash? yes but since the block erase times are really high you might not like the result
personally what i would do is define kinds of high-level storage (filesystem, key-value, etc) and implement that directly on top of various kinds of flash
because if you know what the end use looks like, even approximately, you can actually use flash in a way that provides nice results
klob left the room.

@whitequark

like i said, it's obviously technically possible, the issue is that you might not want to actually use the result
you say that most NAND FTLs expose smaller logical write size than the page size, that's true
but it's done for compatibility and actually using that feature leads to write amplification

@jschievink

I could see value in a trait for NAND Flash access without an FTL

[MathiasKoch]

I'll close this for now, as i think we are past a point where this discussion is relevant.