STMG4 Dual Bank Flash support

[tomasbarton-stemcell]

I have noticed that STMG4 support was added a few days ago:

#2294

However, this PR is not going to work correctly on dual bank flash MCUs such as STM32G474VE that I work with. The reason is that dual bank mode uses 2KB page size while single bank uses 4K pages.

I worked around this issue by deleting two pages on each call to blocking_erase_sector when I was implementing embassy bootloader several weeks ago. Additionally errata 2.2.2 should be implemented (I reused the same code from f4.rs/save_data_cache_state() as the workaround procedure is the same). Other functions beside erasing the sector work without changes.

(btw I have not tested the PR, however I took the same approach as the author and reused the code from g0.rs so I assume that there are the same issues I faced.)

[dgehriger]

@tomasbarton-stemcell : I'm using the STM32G491RE, which has the same memory layout and modes. I'm unsure about the meaning of your message. Are you saying that the swap algorithm won't work for these chips? If so, do you have a fix, and could you possibly post it? Thank you!

[tomasbarton-stemcell]

I have a quick and dirty fix here: https://github.com/embassy-rs/embassy/pull/2946/files

It works, but I haven't had a chance to clean this up yet

[tomasbarton-stemcell]

@tomasbarton-stemcell : I'm using the STM32G491RE, which has the same memory layout and modes. I'm unsure about the meaning of your message.

The problem is that the current Embassy code assumes single bank mode with 4KB sectors, however the chip by default uses dual bank mode with 2KB pages.

So my solution to get this working quickly was to modify the code in such a way that the erase function erases 2 2KB sectors for each 4KB sector call. The write function does not care about the page size. Another solution might be to switch the chip to single bank mode (I could not do that since I need double bank for something else)

[Dirbaio]

note G4 in dualbank mode has a hole in the address space between the two banks (except in the chips with max amount of flash). You can't pretend the two banks form a single contiguous flash region.

Even worse, all G4 chips are actually the same silicon die, so they actually all have the max amount of flash. The "hole" in the address space is actually working flash that you can use, but is "untested, not guaranteed to work" according to ST. So you might have the wrong memory.x but never notice because it works anyway.

If you don't need dualbank and want one contiguous chunk of flash, you really should actually turn off dualbank.

[dgehriger]

Thanks for the follow up! Maybe I'm missing something, but in the specific case of the STM32G491RE, which has 512K (2 x 256K in dual-bank config), the flash driver should work just fine in dual-bank mode.

If you look at https://github.com/embassy-rs/stm32-data-generated/blob/main/stm32-metapac/src/chips/stm32g491re/metadata.rs, the erase_size is "correctly" specified as 2048, and since there isn't a gap, the two banks are simply a contiguous memory block of 512K.

So, again for this chip specifically, using dualbank mode will work with the embassy Flash driver.

[Dirbaio]

yeah it happens to work fine if your chip has 512K which is the max amount, because then there's no hole.

[diondokter]

I just want to note that what Adam said in the linked PR in the OP:

By default, G4s with dual banks have the DBANK bit reset, so they can be treated as having one large bank

is not true for at least the G484 I have.

So right now the flash interface doesn't work out of the box for this chip.
This bit also means that the interface can only really be determined at runtime and currently the API uses consts for page, erase and write sizes. (This is also a limitation in the embedded-storage traits)

[Dirbaio]

the interface can only really be determined at runtime

It's simpler to add Cargo features for single/dual bank mode, then we can known it at compile time. i.e. you compile your firmware for "stm32g484 single-bank" or "stm32g484 dual-bank"

embassy-stm32 init would check the DBANK bit at init and panic if it's wrong (we can't make the firmware itself change it, because changing it shuffles the flash contents, causing the firmware to suicide)

[diondokter]

Yes, I think I agree. Last problem I see is that the stm32-data generates only one flash bank. But I haven't investigated yet why...

[Dirbaio]

stm32-data simply assumes single-bank mode https://github.com/embassy-rs/stm32-data/blob/main/stm32-data-gen/src/memory.rs#L183-L190

Fix would be adding proper support for "memory map in singlebank is X, memory map in dualbank is Y" to stm32-data, stm32-metapac, embassy-stm32.

[diondokter]

Tried to get some of that info into stm32-data: embassy-rs/stm32-data@a7dbd68

Not sure if this is the right approach though. It works because it generates extra dual bank versions of the jsons which generate well into the metapac.
But then the question is how to act on that in embassy-stm32. How do we know whether to do the DBANK check? I feel like that info should go into the metapac somehow...

[Dirbaio]

Not sure if this is the right approach though. It works because it generates extra dual bank versions of the jsons which generate well into the metapac.

i'm not sure if i'd duplicate the whole json due to size concerns, plus adding it as part of the chip name like that feels a bit ad-hoc. plus there's some families that are always dual-bank, these should always have the dual-bank- prefix.

I'd do either:

• Name regions FLASH for singlebank, FLASH1, FLASH2 for dualbank. Then embassy-stm32 build.rs can check which are present to infer whether the chip is singlebank, dualbank, or switchable single+dual.
• or, add support for "N memory maps" in a single json.

But then the question is how to act on that in embassy-stm32. How do we know whether to do the DBANK check? I feel like that info should go into the metapac somehow...

i'd just do it with cfgs in embassy-stm32, there's not that many families with switchable dualbank.

[diondokter]

Made a PR to implement it here: #4125