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

@@ -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,
@@ -233,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;
@@ -290,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__);
@@ -314,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;
+        dtimer = 10 * ctl->bus_freq;
-        reg_varl(ctl->regs, SDMMC_DLENR) = buff_size;
+        dlenr = buff_size;
-
-        /* DCTRL must be written last */
-        reg_varl(ctl->regs, SDMMC_DCTRL) = dctrl;
 
         /* Enable data phase */
         reg_vwritef(cmdr, SDMMC_CMDR, CMDTRANS(1));
@@ -329,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;
+            dtimer = 1 * ctl->bus_freq;
-        else
-            reg_varl(ctl->regs, SDMMC_DTIMER) = 0;
     }
 
+    /* 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
@@ -360,6 +385,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);