flash: fence ordering to ensure pg bit is set before write to flash memory

[ost-ing]

Following from issue: #382

The following code in certain circumstances yields a BankError::PROGRAMMING_SEQUENCE:

let mut buffer: [u8; 256] = [0xFF; 256];
// ... fill buffer with data
flash.erase_sector(Bank::UserBank2, 7).unwrap();
flash
    .write_sector(Bank::UserBank2, 7, 0, &buffer)
    .unwrap();

Reading the PM0433 reference manual; 4.7.3:

Programming sequence error (PGSERR):

More specifically, PGSERR1/2 flag is set if one of below conditions is met:
• A write operation is requested but the program enable bit (PG1/2) has not been set in FLASH_CR1/2 register prior to the request.
• The inconsistency error (INCERR1/2) has not been cleared to 0 before requesting a new write operation.

Solution:

I've found that enabling the Program Enable Bit before enabling parallelism resolves this issue.

Additionally I have fixed some clippy related issues and clarified some comments

[ost-ing]

@richardeoin @lachlansneff-parallel it would be great to have a review/opinion of this, as its blocking me.

[richardeoin]

Ok, thanks for the explaination and clarifications in the PR. In future I'd like to improve the Flash support in general so we support more parts, but let's merge this first.

I wonder if your issue could be caused by re-ordering? That might be why the flash peripheral observes the write to Normal memory (the flash region) before the write to Device memory (Read-Modify-Write to set the PG bit)? ARMv7-M provides no guarantees about ordering between Normal and Device memory types (see Type column in RM0433 Table 7). Did you see this issue in both debug and release mode, or just release?

You could try adding this line after the last modification of the CR register

use sync::atomic::{fence, Ordering};

fence(Ordering::SeqCst);

[ost-ing]

@richardeoin I can replicate the issue only in release mode using commit SHA 778ecf7a14d713e4387dbd477a2455d2f71a3424. If I switch to debug the issue doesn't occur, also if I use the latest HEAD the issue does not occur in release or debug modes, despite there not being any changes to flash.rs.

The inconsistency here is a little strange to me, anyhow, I ended up testing the Ordering fence you suggested, without my rearrangement change and it seems to fix the issue!

[ost-ing]

@richardeoin also, maybe to prevent any other surprise bugs in the future, do you see anywhere else in the flash code that may require an ordering fence?

I had a look again and I think just the one fence should be enough, but maybe you have any more suggestions?

Thanks again for all the help with this, its been crucial for me!

[richardeoin]

Interesting to know that adding the reordering fence fixed it in one situation. There are potentially some more places in the code that need a fence: in theory one could be needed at every switch between accessing a register and accessing the flash memory. But I suspect the one you've added is the most important and I think sufficient for now.

Thanks for the work on this PR!

bors r+

[bors]

Build succeeded:

• ci (1.59.0, stm32h735)
• ci (1.59.0, stm32h743)
• ci (1.59.0, stm32h743v)
• ci (1.59.0, stm32h747cm7)
• ci (1.59.0, stm32h753)
• ci (1.59.0, stm32h753v)
• ci (1.59.0, stm32h7b0)
• ci (1.59.0, stm32h7b3)
• ci (stable, stm32h735)
• ci (stable, stm32h743)
• ci (stable, stm32h743v)
• ci (stable, stm32h747cm7)
• ci (stable, stm32h753)
• ci (stable, stm32h753v)
• ci (stable, stm32h7b0)
• ci (stable, stm32h7b3)
• Rustfmt