Add FlashRAM save storage driver - #925
Open
meeq wants to merge 13 commits into
Open
Conversation
rasky
reviewed
Jul 5, 2026
rasky
left a comment
rasky
left a comment
Collaborator
There was a problem hiding this comment.
First round mainly focusing on public API, and just a quick look at implementation
| * @param len Number of bytes to write. | ||
| * @return Number of bytes written, or a negative value on error (sets @c errno). | ||
| */ | ||
| int flashram_write(const void* src, size_t offset, size_t len); |
Collaborator
There was a problem hiding this comment.
I have two main discussion points on this API:
- A blocking API for an operation that takes tens of milliseconds seems rather unfortunate. Multithreading is still a hard sell, if we could design an asynchronous API, we would probably be in a better situation.
- I understand the idea of providing a "higher level API" here but this one seems to be very hard to be successfully used:
- With the default layout, up to 1/8th of the flash would need to be cleared for any write basically
- This means that internally this function has to allocate a buffer of non trivial size for the RMW cycle
- The operation is extremely slow; callers will want to minimize the number of unnecessary sector clears
- There is no context to know whether a sector really needs to be cleared or can just be "appended" by programming more pages. So this API has to assume the worst.
It seems to me this is not really ideal. I doubt there will be many users that will want to cope with these compromises.
| *PI_BSD_DOM2_PWD = 0xc; | ||
| *PI_BSD_DOM2_PGS = 0xd; | ||
| *PI_BSD_DOM2_RLS = 0x2; | ||
| enable_interrupts(); |
Collaborator
There was a problem hiding this comment.
Unfortunately the way PI domains are designed makes them a bit unfortunate to use. Here you're basically assuming that nobody else will need to access any other peripheral in the Domain 2, during the whole run. This might be true and even likely, but it's still a very strict requirements.
In theory we should work towards a better lower-level PI API where we describe peripherals and setup domain parameters for them, and then lazily update PI registers when needed. Anyway this is probably not required to be sorted out now.
| // Partial sector: read-modify-write to preserve untouched bytes. | ||
| if (sector_buf == NULL) | ||
| { | ||
| sector_buf = memalign(16, FLASHRAM_SECTOR_SIZE); |
Collaborator
There was a problem hiding this comment.
isn't this a good use case for scratch memory?
Contributor
Author
There was a problem hiding this comment.
I didn't use scratch memory for this because I wasn't sure if we were going to move forward with the eventual consistency model. If the allocation needs to live longer than the function call, scratch becomes riskier.
For a short-lived allocation in a synchronous function with no other allocations it doesn't really make a difference.
bsmiles32
reviewed
Jul 5, 2026
bsmiles32
left a comment
bsmiles32
left a comment
There was a problem hiding this comment.
Thanks for working on this ! Just did a first round of review.
|
Not necessarily critical for this PR, but I wanted to capture the suggestion from Discord to add an example that leverages this feature. |
The new savedoodle example is very cool! Nice work constructing an elegant way to exercise the various save types. |
Contributor
Author
|
Related: #951 |
New flashram.c/.h module for the 128 KiB (1 Mibit) Macronix-family FlashRAM save chip found in some N64 cartridges. Unlike SRAM it is command-driven (identify / read / status / sector-erase / page-program) rather than flat memory; the protocol matches real hardware and is compatible with ares and SC64. Two API levels: a low-level page/sector interface, and a high-level flat byte-range flashram_read/flashram_write that behaves like SRAM. flashram_write does a per-sector read-modify-write so a partial-sector write preserves the untouched bytes in that 16 KiB erase sector. The driver manages its own PI-DMA cache coherency.
Make the FlashRAM driver widely compatible with the different Macronix / Matsushita parts a physical cart might ship, rather than assuming the one byte-indexed layout emulators present: - Detect the chip. flashram_detect() now fills a flashram_info_t (silicon ID, manufacturer/device, model name, addressing mode, layout) and caches the byte- vs word-indexed convention looked up from a model table. The addressing mode cannot be probed at runtime, only identified. - Support word-indexed parts (e.g. MX29L1100): the read path halves the logical offset to reach the right 16-bit word. Both modes share the same 0x8000 logical-byte DMA no-cross boundary (word-mode's 0x4000 PI-address boundary maps to 0x8000 logical), so PGS is set to max (never auto-split) and every DMA is split manually. - CIR double-write for status / silicon-ID mode entry (MX29L1100 needs two writes to actually switch); read/erase/program still use a single write. - Status is 8-bit now (mask off the upper bits, which are garbage from the previous mode on MX29L1100). Add flashram_clear_status() and reset the OK/error latch after every erase/program so a prior result cannot mask the next. The busy-poll reads the auto-latched status directly instead of re-commanding status mode each iteration (which can clear the OK bit). - PI DOM2 LAT 0x05 -> 0x40 (FlashRAM needs a longer latch than SRAM). - Rename the 0xB4 command to LOAD_BYTE_PAGE and 0xA5 to PROGRAM_PAGE to match the documented protocol (0xB4 loads the page buffer, 0xA5 programs it); the old FLASHRAM_CMD_PAGE_PROGRAM name pointed at the wrong step. Byte path validated end-to-end on ares (write/RMW/erase/program/read round-trip + untouched-neighbor preservation).
Move the low-level page/sector interface (status, clear_status, erase, program_page) out of the public header into a new src/flashram_internal.h. The public API is now just the SRAM-like byte-range interface (init/detect/read/write) plus the info struct and geometry constants, while the shape of a low-level API is still being decided. Following the hardware formalism, the erase operation is addressed by page number (the chip derives the enclosing sector), so rename it to flashram_erase_sector_at_page() and thread page numbers through the write path. Add flashram_erase_chip() (0x3C setup + 0x78 execute) to the internal API for protocol completeness.
A save-backed doodle canvas that probes the hardware to discover which save chip is present (EEPROM, SRAM, or FlashRAM) and drives all three through a uniform read/write path, persisting the drawing across power cycles. Built into one ROM per save type from a single source. Probe order is EEPROM -> SRAM -> FlashRAM: FlashRAM detection writes to the command register at +0x10000, which aliases into a 32 KiB SRAM cart's window, so FlashRAM is only probed once SRAM is ruled out. The cursor moves via D-Pad (discrete cells) or the analog stick (continuous, deflection-proportional for precise control), and drawing is done with the RDP via rdpq.
Evolve the FlashRAM public API following PR review: - Describe the chip with a bits-per-component flashram_layout_t (unit/offset/page/sector/read-page bits), replacing the byte/word addressing enum. It uniformly covers byte- and word-indexed parts and encodes the DMA read boundary, so both the addressing shift and the 0x8000 boundary are now derived rather than hardcoded. - Base every read/write/erase/program on the detected layout cached in flashram_info_t rather than the fixed geometry macros (which now only seed the pre-detection default and bound the page-program buffer). - Merge detection into flashram_init(): it configures the PI timings, probes the silicon ID, caches the layout, and returns presence (optionally filling flashram_info_t). flashram_detect() is removed -- the layout must be known for correct addressing, so there is no useful init-without-detect state. - Let flashram_init() take optional PI DOM2 timings (NULL = the standard wiki defaults: latency 0x05, pulse 0x0C, page size 0x0F, release 0x02). - Assert a chip was actually detected before any read/write, drop the now-unused FLASHRAM_PAGES_PER_SECTOR, and stop referencing the internal API from the public header docs.
Detect FlashRAM via the merged flashram_init(NULL, &info), which now returns presence and fills the info in one call.
The PI_BSD_DOM2 LAT/PWD/PGS/RLS registers are a generic PI-domain concept, not FlashRAM-specific, so move the timings struct into dma.h (libdragon's PI interface) as pi_dom_timings_t and have flashram_init() take it. The page-size-must-be-0x0F requirement is documented on flashram_init(), where it belongs, rather than on the generic type.
The chip geometry is now detected and reported per chip in flashram_info_t, so the public FLASHRAM_SIZE / PAGE_SIZE / SECTOR_SIZE / NUM_PAGES / NUM_SECTORS macros were redundant (and misleading, implying a fixed layout). Remove them, keeping only FLASHRAM_ADDRESS. The page-program buffer's compile-time bound becomes an internal FLASHRAM_MAX_PAGE_SIZE, and __flashram_info is left zero-initialized since reads/writes assert a chip was detected before reading it.
FLASHRAM_LAYOUT_BYTE/WORD lacked doc comments, failing the -Werror Doxygen build. Give each a one-line description of the geometry it encodes.
Sector-erase/page-program commands OR the page number into a 32-bit opcode word (opcode in bits [31:24]), so page numbers must fit in the low 24 bits. The per-op range checks only compare against num_pages, so a table entry with an oversized geometry would pass yet silently corrupt the opcode. Assert num_pages <= 2^24 in flashram_derive_geometry, where the geometry is adopted, turning the implicit invariant into a checked one at no hot-path cost.
flashram_read()'s fast path guarded only 8-byte destination alignment and even length before data_cache_hit_writeback_invalidate + DMA. The cache op works on whole 16-byte D-cache lines, so an 8-but-not-16-aligned edge extended it into the ADJACENT allocation's shared line: the writeback half can flush stale CPU bytes over up to 8 freshly-DMA'd neighbor bytes, and the invalidate half discards the neighbor's cached state — layout-dependent corruption on every read. Tighten the guard to 16-byte start AND length granularity; anything less aligned already takes the bounce-buffer path, which is edge-safe (16-aligned bounce, memcpy out).
bsmiles32 noted the identify command is called "SiliconID mode" in the wiki and datasheets, not "identify mode".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New flashram.c/.h module for the 128 KiB (1 Mibit) Macronix-family FlashRAM save chip found in some N64 cartridges. Unlike SRAM it is command-driven (identify / read / status / sector-erase / page-program) rather than flat memory; the protocol matches real hardware and is compatible with ares and SC64.
Two API levels: a low-level page/sector interface, and a high-level flat byte-range flashram_read/flashram_write that behaves like SRAM. flashram_write does a per-sector read-modify-write so a partial-sector write preserves the untouched bytes in that 16 KiB erase sector. The driver manages its own PI-DMA cache coherency.