Embedded-hal: Thoughts on Flash API?

[idubrov]

Do you mind taking a quick look at https://github.com/idubrov/stm32-hal/blob/master/src/flash.rs to see if Flash trait I created is a good candidate to be included into this embedded-hal crate?

It appears to me that the trait itself could go into embedded-hal and implementation (line #97 and below) would go into device-specific crate (which in my case is stm32f103xx, but there is not stm32f103xx-hal yet, as far as I can see).

I also have an EEPROM emulation crate (https://github.com/idubrov/eeprom) which uses Flash trait, but, I guess, it should stay as a separate "driver" crate, which will depend on embedded-hal, right?

[thenewwazoo]

I'm resurrecting this, as I'll be adding flash support to the stm32l0x1-hal crate today or tomorrow, and so am looking for traits to use. @idubrov, I think I'll probably adapt your proposal into its own flash-only traits crate with an eye toward adding its contents to an embedded-hal PR soon-ish.

[thenewwazoo]

I'm currently experimenting with these traits. I've impl'd them for stm32l0x1-hal, and will be doing lpc177x_8x-hal soon.

[ryankurte]

Neat! i think it'd be good to have a page write as well, and maybe it could be generic over word size? (edit: I see that's already the case for write, just not read yet)

I don't know if it's the right approach / maybe there's something simpler, but maybe write_word(&mut self, word: W) with default impls for write_u8/write_u16/write_u32 where W is u8, u16 and u32 would work?

[eldruin]

Sounds good @thenewwazoo! I wrote this EEPROM 24x driver and what I would miss in your traits would mostly be reading an array of words and writing a page (array of words) (see docs). These devices also support reading sequentially (read current address + advance address pointer) but this is probably not implemented in all devices.

I think I would only implement Read and WriteRead and would have no need for Latency or Locking.
The erase_page would not make sense for these devices but I guess I could overwrite the whole page with 0s and that would be fine.

The flash traits presented at the beginning of this thread do not seem to fit the EEPROM 24x devices very well.

Is the Latency used for devices that support communicating at different rates? If so, would it make sense to have a getter method for latency for generic drivers that read the latency from the device?

About the wording, I am not very sure about program_word, I think I would name it write_word but maybe I associate the word program with programming (as in writing code) too much.

There is one thing that I would not know how to model, though: In these devices, after you write a word or page, the device enters an internally-timed write cycle and it does not respond to I2C for a short while (device-dependent).
However, this is not available in the form of some BUSY bit or IO pin. Do you have any idea on how to implement an is_busy()?

[thenewwazoo]

I haven’t had a chance to do any coding in a few days but I did just have an idea about the is_busy question: you could take a non-Periodic hal::timer::Timer in the constructor, and start it when you write; is_busy then becomes a thin shim over Timer::wait

[eldruin]

I haven’t had a chance to do any coding in a few days but I did just have an idea about the is_busy question: you could take a non-Periodic hal::timer::Timer in the constructor, and start it when you write; is_busy then becomes a thin shim over Timer::wait

Interesting solution! I think that could be done. However, it seems to me like the whole API will have to use nb::Result if I do not want to forcefully block on is_busy() in the rest of methods, but I think it could be worth it.
Ok I have to apologize I forgot about this: The devices support "Acknowledge polling". I will try to implement the is_busy() just using normal I2C and mapping the errors although that could mask other errors. If that does not work, I will try with the timer.
Thanks @thenewwazoo!

[nickray]

Should the program_* method be generic over data/value size, taking &[u8; N]? Implementations would specify N; I need this for STM32L432 where N = 8.
Would something like a wear-leveled counter fit in a HAL? It's not exactly a driver either.

[nickray]

I've started to iterate on these Flash traits: https://github.com/nickray/stm32l4xx-hal/blob/wip/src/extra_traits/flash.rs

One tricky issue is the fact that in a general Flash trait, the size/alignment rules for reads, writes and erases are chip dependent. I'm currently using the generic-array approach, an alternative would be the "config crate" approach demo'd in https://github.com/jamesmunns/mcf-config-demo

Question: are there any (many?) chips where e.g. read/write/erase sizes and alignment requirements are different? Because then there would be at least 6 instead of 3 constants to configure.

[austinglaser]

Many chips I'm aware of (specifically the STM32 line) do have different alignment and size requirements for all three. Reads are usually allowed pretty freely -- down to byte resolution, and aligned to the size of the read. Writes must be aligned to 4-byte words, and usually only clear bits (never set them). Erases usually are page-aligned -- 1024 bytes is a common size for a page.

I'd encourage you to double check this on some device datasheets, since I'm going from memory here and this is just for one family of MCUs.

[nickray]

I'm aware of that, e.g. the STM32L432 I'm using has read/write/erase sizes of 8/8/2048 bytes. My question is whether something like like read size 2 but read alignment 1 occurs (i.e., not "Reads are usually allowed (...) aligned to the size of the read"), or write size != write alignment, or erase size != erase alignment.

[IGBC]

has anyone considered opening this up to all forms of NVS?

while not as popular as it used to be some chips are still shipping with eeprom which is really useful when available.

[austinglaser]

Ah, I misunderstood -- apologies. Unfortunately I can't provide as clear-cut an answer there -- I've never _seen_ such an MCU, but my experience is far from comprehensive.

It does seem to me that for a given operation alignment can never be _greater_ than size -- otherwise, there'd be inaccessible holes in your address space. So if you assume that size == alignment, and encounter an architecture where size > alignment, all you're missing is a potential optimization (and probably a minor one at that), rather than running into a fundamental incompatibility.

With that said, my assumption should probably be challenged before anyone relies on it.

has anyone considered opening this up to all forms of NVS?

while not as popular as it used to be some chips are still shipping with eeprom which is really useful when available.

I think this could be extremely useful. However, I'd propose a generic byte-oriented nonvolatile storage trait rather than trying to make a flash trait encompass all necessary functionality. Such a trait could be built on top of a flash implementer, or directly on top of a hardware EEPROM, or on top of an I2C trait accessing an off-SoC part.

[nickray]

One of my goals is to have traits that easily implement the Storage trait used in the LittleFS wrapper: https://docs.rs/littlefs/0.2.0/littlefs/trait.Storage.html

[twitchyliquid64]

The traits as implemented seems to be very specific to on-die flash. Would it make sense to have a more generic Storage or Memory trait, and then a Flash trait can get more specific from there?

There's lots of memory types to consider:

• SRAM
• EEPROM
• Flash
• FRAM
• SDIO/MMC
• Remote (such as iSCSI)
• Magnetic core (lol)

[eldruin]

@MathiasKoch has created embedded-storage and it would be great to move the discussion around this there for a more focused forum. Once we achieve a good API, we could merge it to embedded-hal but we can discuss this separately.
For this reason, I will close this issue now. If you think otherwise, feel free to ask for reopening.