From 9f9a872176e23f1113c9b524f9f138b5481d7e5e Mon Sep 17 00:00:00 2001
From: Kevin O'Connor <kevin@koconnor.net>
Date: Sun, 15 May 2022 19:25:25 -0400
Subject: [PATCH] stm32: Avoid read-modify-write operations in flash.c

Avoid read-modify-write operations on the FLASH->CR register.  Write
out the desired valid explicitly.

Use writew() and writel() to write the flash bytes out to avoid gcc
reordering the memory write relative to the flash register writes.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
 src/stm32/flash.c | 101 ++++++++++++++++++++++------------------------
 1 file changed, 49 insertions(+), 52 deletions(-)

diff --git a/src/stm32/flash.c b/src/stm32/flash.c
index 91dbe8f..2b68d01 100644
--- a/src/stm32/flash.c
+++ b/src/stm32/flash.c
@@ -5,6 +5,7 @@
 // This file may be distributed under the terms of the GNU GPLv3 license.
 
 #include "autoconf.h" // CONFIG_MACH_STM32F103
+#include "board/io.h" // writew
 #include "flash.h" // flash_write_page
 #include "internal.h" // FLASH
 
@@ -27,32 +28,37 @@ flash_get_page_size(void)
     return CONFIG_MAX_FLASH_PAGE_SIZE;
 }
 
+static void
+wait_flash(void)
+{
+    while (FLASH->SR & FLASH_SR_BSY)
+        ;
+}
+
 static void
 unlock_flash(void)
 {
-    // Unlock Flash Erase
-    FLASH->KEYR = FLASH_KEY1;
-    FLASH->KEYR = FLASH_KEY2;
-    while (FLASH->SR & FLASH_SR_BSY);
+    if (FLASH->CR & FLASH_CR_LOCK) {
+        // Unlock Flash Erase
+        FLASH->KEYR = FLASH_KEY1;
+        FLASH->KEYR = FLASH_KEY2;
+    }
+    wait_flash();
 }
 
-void
-flash_complete(void)
+static void
+lock_flash(void)
 {
-    // Lock flash when done
-    FLASH->CR |= FLASH_CR_LOCK;
+    FLASH->CR = FLASH_CR_LOCK;
 }
 
-void
+static void
 flash_write_stm32f4xx(uint32_t page_address, uint32_t *data)
 {
 #if CONFIG_MACH_STM32F4
     uint32_t flash_page_size = flash_get_page_size();
     uint32_t* page = (uint32_t*)(page_address);
 
-    if (FLASH->CR & FLASH_CR_LOCK)
-        unlock_flash();
-
     uint8_t need_erase = 0;
     uint32_t offset_addr = page_address - CONFIG_FLASH_START;
     uint32_t sector_index = offset_addr / STM32F4_MAX_SECTOR_SIZE;
@@ -67,36 +73,29 @@ flash_write_stm32f4xx(uint32_t page_address, uint32_t *data)
         need_erase = ((page_address % STM32F4_MAX_SECTOR_SIZE) == 0);
         sector_index += 4;
     }
-    while (FLASH->SR & FLASH_SR_BSY);
-    FLASH->CR &= ~FLASH_CR_PSIZE;
-    FLASH->CR &= ~FLASH_CR_SNB;
+
+    // make sure flash is unlocked
+    unlock_flash();
+
+    // Erase page
     if (need_erase) {
-        FLASH->CR |= FLASH_CR_PSIZE_1;
-        FLASH->CR |= FLASH_CR_SER | ((sector_index & 0xF) << 3);
-        FLASH->CR |= FLASH_CR_STRT;
-        while (FLASH->SR & FLASH_SR_BSY);
-        FLASH->CR &= ~FLASH_CR_SNB;
-        do {
-            FLASH->CR &= ~FLASH_CR_SER;
-        } while(FLASH->CR & FLASH_CR_SER);
+        FLASH->CR = (FLASH_CR_PSIZE_1 | FLASH_CR_STRT | FLASH_CR_SER
+                     | ((sector_index & 0xF) << 3));
+        wait_flash();
     }
 
-    FLASH->CR |= FLASH_CR_PSIZE_1;
-    FLASH->CR |= FLASH_CR_PG;
-    for (uint16_t i = 0; i < flash_page_size / 4; i++)
-    {
-        page[i] = data[i];
-        while (FLASH->SR & FLASH_SR_BSY);
+    // Write page
+    FLASH->CR = FLASH_CR_PSIZE_1 | FLASH_CR_PG;
+    for (int i = 0; i < flash_page_size / 4; i++) {
+        writel(&page[i], data[i]);
+        wait_flash();
     }
-    do {
-        FLASH->CR &= ~FLASH_CR_PG;
-    } while(FLASH->CR & FLASH_CR_PG);
 
-    FLASH->CR |= FLASH_CR_LOCK;
+    lock_flash();
 #endif
 }
 
-void
+static void
 flash_write_stm32f1xx(uint32_t page_address, uint16_t *data)
 {
 #if CONFIG_MACH_STM32F0 || CONFIG_MACH_STM32F1
@@ -104,30 +103,22 @@ flash_write_stm32f1xx(uint32_t page_address, uint16_t *data)
     uint16_t* page = (uint16_t*)(page_address);
 
     // make sure flash is unlocked
-    if (FLASH->CR & FLASH_CR_LOCK)
-        unlock_flash();
+    unlock_flash();
 
     // Erase page
-    FLASH->CR |= FLASH_CR_PER;
-    FLASH->AR = (uint32_t)page;
-    FLASH->CR |= FLASH_CR_STRT;
-    while (FLASH->SR & FLASH_SR_BSY);
-    do {
-        FLASH->CR &= ~FLASH_CR_PER;
-    } while (FLASH->CR & FLASH_CR_PER);
+    FLASH->CR = FLASH_CR_PER;
+    FLASH->AR = page_address;
+    FLASH->CR = FLASH_CR_PER | FLASH_CR_STRT;
+    wait_flash();
 
     // Write page
-    FLASH->CR |= FLASH_CR_PG;
-    for (uint16_t i = 0; i < flash_page_size / 2; i++)
-    {
-        page[i] = data[i];
-        while (FLASH->SR & FLASH_SR_BSY);
+    FLASH->CR = FLASH_CR_PG;
+    for (int i = 0; i < flash_page_size / 2; i++) {
+        writew(&page[i], data[i]);
+        wait_flash();
     }
-    do {
-        FLASH->CR &= ~FLASH_CR_PG;
-    } while (FLASH->CR & FLASH_CR_PG);
 
-    FLASH->CR |= FLASH_CR_LOCK;
+    lock_flash();
 #endif
 }
 
@@ -140,3 +131,9 @@ flash_write_page(uint32_t page_address, void *data)
         flash_write_stm32f1xx(page_address, (uint16_t *)data);
     }
 }
+
+void
+flash_complete(void)
+{
+    lock_flash();
+}