From 65bf8d419248de008a1035529e354e96a5562a1c Mon Sep 17 00:00:00 2001 From: Aidan MacDonald Date: Tue, 24 Feb 2026 20:39:02 +0000 Subject: [PATCH 1/2] stm32h7: sdmmc: ensure minimum time between CLKCR/CMDR writes According to RM0433 consecutive writes to the CLKCR and CMDR registers must be separated by at least 7 AHB clock cycles. The initialization code didn't respect this and it seemed to be causing a nasty bug, where the SDMMC bus clock got stuck at 400 KHz in hardware. Despite the CLKCR register reading back the correct value, it could not be written with a new value even in the debugger; resetting the peripheral was the only way out of this state. Adding some dummy register reads after any access to CLKCR should insert the necessary number of wait states. Without the fix, the SDMMC clock gets stuck about 12% of the time. With this fix, the clock always initializes correctly. Change-Id: Iba85b8e1e3c60992ddc42fb4c1e66c37941ed617 --- firmware/target/arm/stm32/sdmmc-stm32h7.c | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/firmware/target/arm/stm32/sdmmc-stm32h7.c b/firmware/target/arm/stm32/sdmmc-stm32h7.c index 56e7d70e4c..014e5f6340 100644 --- a/firmware/target/arm/stm32/sdmmc-stm32h7.c +++ b/firmware/target/arm/stm32/sdmmc-stm32h7.c @@ -74,6 +74,28 @@ static size_t abs_diff(size_t a, size_t b) return a > b ? a - b : b - a; } +/* + * The CLKCR and CMDR registers must not be written twice + * within 7 AHB clock cycles. If this is not respected the + * hardware seems to misbehave, for example updating the + * clock divider in CLKCR can fail (the register field is + * updated, but the actual clock output is not). + * + * Reading a peripheral register should require at least + * two cycles on the AHB bus (1 address + 1 data cycle), + * assuming they are single transfers and no pipelining + * can take place. Doing four reads of an SDMMC register + * should therefore be enough to add the correct number + * of wait states. + */ +static void stm32h7_sdmmc_regwrite_delay(struct stm32h7_sdmmc_controller *ctl) +{ + reg_readl(ctl->regs, SDMMC_STAR); + reg_readl(ctl->regs, SDMMC_STAR); + reg_readl(ctl->regs, SDMMC_STAR); + reg_readl(ctl->regs, SDMMC_STAR); +} + static bool stm32h7_sdmmc_is_powered_off(struct stm32h7_sdmmc_controller *ctl) { uint32_t pwrctrl = reg_readlf(ctl->regs, SDMMC_POWER, PWRCTRL); @@ -131,6 +153,7 @@ void stm32h7_sdmmc_set_power_enabled(void *controller, bool enabled) /* Automatically stop clock when bus is not in use */ reg_writelf(ctl->regs, SDMMC_CLKCR, PWRSAV(1), HWFC_EN(1)); + stm32h7_sdmmc_regwrite_delay(ctl); } else { @@ -173,6 +196,8 @@ void stm32h7_sdmmc_set_bus_width(void *controller, uint32_t width) reg_writelf(ctl->regs, SDMMC_CLKCR, WIDBUS_V(8BIT)); else panicf("%s", __func__); + + stm32h7_sdmmc_regwrite_delay(ctl); } void stm32h7_sdmmc_set_bus_clock(void *controller, uint32_t clock) @@ -222,6 +247,8 @@ void stm32h7_sdmmc_set_bus_clock(void *controller, uint32_t clock) DDR(0), NEGEDGE(0), CLKDIV(div[idx] / 2)); + + stm32h7_sdmmc_regwrite_delay(ctl); } int stm32h7_sdmmc_submit_command(void *controller, @@ -360,6 +387,7 @@ int stm32h7_sdmmc_submit_command(void *controller, reg_varl(ctl->regs, SDMMC_MASKR) = maskr; reg_varl(ctl->regs, SDMMC_ARGR) = cmd->argument; reg_varl(ctl->regs, SDMMC_CMDR) = cmdr; + stm32h7_sdmmc_regwrite_delay(ctl); /* Wait for command completion */ semaphore_wait(&ctl->sem, TIMEOUT_BLOCK); From 55183c0b2a82423835aaa0eee276357d9beae3e8 Mon Sep 17 00:00:00 2001 From: Aidan MacDonald Date: Tue, 24 Feb 2026 20:33:27 +0000 Subject: [PATCH 2/2] stm32h7: sdmmc: write DCTRL after DTIMER/DLENR for non-data commands According to RM0433 the DCTRL register needs to be written after DLENR and DTIMER. This is respected for data transfer commands but not for non-data commands. Nothing bad seems to be happening because of this, but it seems wise to rectify the issue. Change-Id: I55d8f2c1994bff747e5978847fda57445f001b02 --- firmware/target/arm/stm32/sdmmc-stm32h7.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/firmware/target/arm/stm32/sdmmc-stm32h7.c b/firmware/target/arm/stm32/sdmmc-stm32h7.c index 014e5f6340..2fd9ff1cb4 100644 --- a/firmware/target/arm/stm32/sdmmc-stm32h7.c +++ b/firmware/target/arm/stm32/sdmmc-stm32h7.c @@ -260,6 +260,7 @@ int stm32h7_sdmmc_submit_command(void *controller, uint32_t maskr = CMD_ERROR_BITS; uint32_t cmdr = __reg_orf(SDMMC_CMDR, CPSMEN(1), CMDINDEX(cmd->command)); uint32_t cmd_wait = WAIT_CMD; + uint32_t dctrl = 0, dtimer = 0, dlenr = 0; void *buff_addr = cmd->buffer; size_t buff_size = cmd->nr_blocks * cmd->block_len; @@ -317,7 +318,6 @@ int stm32h7_sdmmc_submit_command(void *controller, panicf("%s: buffer too big", __func__); /* Set block size */ - uint32_t dctrl = 0; uint32_t dblocksize = find_first_set_bit(cmd->block_len); if (dblocksize > 14 || (cmd->block_len & (cmd->block_len - 1))) panicf("%s: incorrect block size", __func__); @@ -341,11 +341,8 @@ int stm32h7_sdmmc_submit_command(void *controller, reg_assignlf(ctl->regs, SDMMC_IDMACTRLR, IDMAEN(1)); /* Use a 10 second timeout (DTIMER is in units of bus clocks) */ - reg_varl(ctl->regs, SDMMC_DTIMER) = 10 * ctl->bus_freq; - reg_varl(ctl->regs, SDMMC_DLENR) = buff_size; - - /* DCTRL must be written last */ - reg_varl(ctl->regs, SDMMC_DCTRL) = dctrl; + dtimer = 10 * ctl->bus_freq; + dlenr = buff_size; /* Enable data phase */ reg_vwritef(cmdr, SDMMC_CMDR, CMDTRANS(1)); @@ -356,16 +353,17 @@ int stm32h7_sdmmc_submit_command(void *controller, { /* Disable data transfer */ reg_assignlf(ctl->regs, SDMMC_IDMACTRLR, IDMAEN(0)); - reg_varl(ctl->regs, SDMMC_DLENR) = 0; - reg_varl(ctl->regs, SDMMC_DCTRL) = 0; /* DTIMER is the wait time for the busy signal */ if (cmd->flags & SDMMC_RESP_BUSY) - reg_varl(ctl->regs, SDMMC_DTIMER) = 1 * ctl->bus_freq; - else - reg_varl(ctl->regs, SDMMC_DTIMER) = 0; + dtimer = 1 * ctl->bus_freq; } + /* Set data transfer registers */ + reg_varl(ctl->regs, SDMMC_DLENR) = dlenr; + reg_varl(ctl->regs, SDMMC_DTIMER) = dtimer; + reg_varl(ctl->regs, SDMMC_DCTRL) = dctrl; + /* * Set CMDSTOP bit for CMD12 (stop transmission) command; * this is needed to stop the DPSM in case of an error in