Peer review fixes (thanks Copilot)

hal/stm32g4.c:
- hal_flash_write unaligned path: rebase the aligned doubleword on
  (address + i), not the initial address; index as dst[0]/dst[1].
  The old form skipped to the wrong DW once i advanced past 8 with
  an unaligned starting address.
- hal_flash_erase: end_address was address + len - 1 paired with a
  strict less-than, which dropped the last page when len straddled a
  page boundary by one byte. Use address + len.
- flash_clear_errors / EOP clear at end of hal_flash_write: FLASH_SR
  bits are W1C. Read-modify-write via |= can clear unrelated W1C
  bits that happen to be set. Write the mask directly.
- uart_write: cast through uint8_t before promoting to uint32_t so
  high-bit chars do not sign-extend into TDR.

hal/stm32g4.h:
- DMB/ISB/DSB: add the "memory" clobber so the compiler also treats
  them as scheduling barriers, not just hardware-level fences.

hal/stm32g4.ld:
- Rename the output section .edidx to .ARM.exidx to match the
  input-section pattern and standard Cortex-M linker scripts.
pull/789/head
David Garske 2026-05-13 13:54:51 -07:00 committed by Daniele Lacamera
parent c075549ea2
commit 8fe46bb40f
3 changed files with 25 additions and 20 deletions

View File

@ -36,10 +36,13 @@ static RAMFUNCTION void flash_wait_complete(void)
static void RAMFUNCTION flash_clear_errors(void)
{
FLASH_SR |= (FLASH_SR_OPERR | FLASH_SR_PROGERR | FLASH_SR_WRPERR |
FLASH_SR_PGAERR | FLASH_SR_SIZERR | FLASH_SR_PGSERR |
FLASH_SR_MISERR | FLASH_SR_FASTERR | FLASH_SR_RDERR |
FLASH_SR_OPTVERR);
/* FLASH_SR error/EOP bits are write-1-to-clear. Assign the mask
* directly so a read-modify-write doesn't accidentally clear any
* other W1C bits that happen to be set. */
FLASH_SR = (FLASH_SR_OPERR | FLASH_SR_PROGERR | FLASH_SR_WRPERR |
FLASH_SR_PGAERR | FLASH_SR_SIZERR | FLASH_SR_PGSERR |
FLASH_SR_MISERR | FLASH_SR_FASTERR | FLASH_SR_RDERR |
FLASH_SR_OPTVERR);
}
int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
@ -63,21 +66,21 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
} else {
uint32_t val[2];
uint8_t *vbytes = (uint8_t *)(val);
int off = (address + i) - (((address + i) >> 3) << 3);
uint32_t base_addr = address & (~0x07); /* aligned to 64 bit */
int u32_idx = (i >> 2);
uint32_t base_addr = (address + i) & ~0x7u; /* DW we touch */
int off = (address + i) & 0x7; /* byte in DW */
dst = (uint32_t *)(base_addr);
val[0] = dst[u32_idx];
val[1] = dst[u32_idx + 1];
val[0] = dst[0];
val[1] = dst[1];
while ((off < 8) && (i < len))
vbytes[off++] = data[i++];
dst[u32_idx] = val[0];
dst[u32_idx + 1] = val[1];
flash_wait_complete();
dst[0] = val[0];
dst[1] = val[1];
flash_wait_complete();
}
}
if ((FLASH_SR & FLASH_SR_EOP) == FLASH_SR_EOP)
FLASH_SR |= FLASH_SR_EOP;
/* W1C: assign directly; no RMW. Harmless if EOP isn't set. */
FLASH_SR = FLASH_SR_EOP;
FLASH_CR &= ~FLASH_CR_PG;
return 0;
}
@ -113,7 +116,7 @@ int RAMFUNCTION hal_flash_erase(uint32_t address, int len)
if (len == 0)
return -1;
address -= FLASHMEM_ADDRESS_SPACE;
end_address = address + len - 1;
end_address = address + len;
for (p = address; p < end_address; p += FLASH_PAGE_SIZE) {
flash_wait_complete();
flash_clear_errors();
@ -258,7 +261,7 @@ void uart_write(const char *buf, unsigned int len)
for (i = 0; i < len; i++) {
while ((LPUART1_ISR & USART_ISR_TXE) == 0)
;
LPUART1_TDR = (uint32_t)buf[i];
LPUART1_TDR = (uint32_t)(uint8_t)buf[i];
}
}

View File

@ -30,10 +30,12 @@
#include <stdint.h>
/* Assembly helpers */
#define DMB() __asm__ volatile ("dmb")
#define ISB() __asm__ volatile ("isb")
#define DSB() __asm__ volatile ("dsb")
/* Assembly helpers. The "memory" clobber tells the compiler not to
* reorder loads/stores across the barrier; without it the asm only
* orders hardware accesses, not the compiler's own scheduling. */
#define DMB() __asm__ volatile ("dmb" ::: "memory")
#define ISB() __asm__ volatile ("isb" ::: "memory")
#define DSB() __asm__ volatile ("dsb" ::: "memory")
/* -------- RCC (AHB1 + 0x1000 = 0x40021000) -------- RM0440 - 7.4 -------- */

View File

@ -16,7 +16,7 @@ SECTIONS
_end_text = .;
} > FLASH
.edidx :
.ARM.exidx :
{
. = ALIGN(4);
*(.ARM.exidx*)