Emulate read errors in the DMA Checksum Register for high frequencies
and optimistic settings of the Read Timing Compensation Register. This
will help in tuning the SPI timing calibration algorithm.

The values below are those to expect from the first flash device of
the FMC controller of a palmetto-bmc machine.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 983066f5ad1d..da2fedfcd3cd 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -745,6 +745,30 @@ static void aspeed_smc_dma_calibration(AspeedSMCState *s)
     s->regs[s->r_ctrl0 + cs] |= CE_CTRL_CLOCK_FREQ(hclk_div);
 }
 
+static bool aspeed_smc_inject_read_failure(AspeedSMCState *s)
+{
+    uint8_t delay =
+        (s->regs[R_DMA_CTRL] >> DMA_CTRL_DELAY_SHIFT) & DMA_CTRL_DELAY_MASK;
+    uint8_t hclk_mask =
+        (s->regs[R_DMA_CTRL] >> DMA_CTRL_FREQ_SHIFT) & DMA_CTRL_FREQ_MASK;
+
+    /*
+     * Typical values of a palmetto-bmc machine.
+     */
+    switch (aspeed_smc_hclk_divisor(hclk_mask)) {
+    case 4 ... 16:
+        return false;
+    case 3: /* at least one HCLK cycle delay */
+        return (delay & 0x7) < 1;
+    case 2: /* at least two HCLK cycle delay */
+        return (delay & 0x7) < 2;
+    case 1: /* (> 100MHz) is above the max freq of the controller */
+        return true;
+    default:
+        g_assert_not_reached();
+    }
+}
+
 /*
  * Accumulate the result of the reads to provide a checksum that will
  * be used to validate the read timing settings.
@@ -774,6 +798,11 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
         s->regs[R_DMA_FLASH_ADDR] += 4;
         s->regs[R_DMA_LEN] -= 4;
     }
+
+    if (aspeed_smc_inject_read_failure(s)) {
+        s->regs[R_DMA_CHECKSUM] = 0xbadc0de;
+    }
+
 }
 
 static void aspeed_smc_dma_rw(AspeedSMCState *s)
-- 
2.17.1

The SMC controllers have a register containing the byte that will be
used as dummy output. It can be modified by software.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index da2fedfcd3cd..f31bbc895caa 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -102,8 +102,8 @@
 /* Misc Control Register #1 */
 #define R_MISC_CTRL1      (0x50 / 4)
 
-/* Misc Control Register #2 */
-#define R_MISC_CTRL2      (0x54 / 4)
+/* SPI dummy cycle data */
+#define R_DUMMY_DATA      (0x54 / 4)
 
 /* DMA Control/Status Register */
 #define R_DMA_CTRL        (0x80 / 4)
@@ -548,7 +548,7 @@ static void aspeed_smc_flash_setup(AspeedSMCFlash *fl, uint32_t addr)
      */
     if (aspeed_smc_flash_mode(fl) == CTRL_FREADMODE) {
         for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) {
-            ssi_transfer(fl->controller->spi, 0xFF);
+            ssi_transfer(fl->controller->spi, s->regs[R_DUMMY_DATA] & 0xff);
         }
     }
 }
@@ -680,6 +680,7 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
         addr == s->r_timings ||
         addr == s->r_ce_ctrl ||
         addr == R_INTR_CTRL ||
+        addr == R_DUMMY_DATA ||
         (s->ctrl->has_dma && addr == R_DMA_CTRL) ||
         (s->ctrl->has_dma && addr == R_DMA_FLASH_ADDR) ||
         (s->ctrl->has_dma && addr == R_DMA_DRAM_ADDR) ||
@@ -912,6 +913,8 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
         }
     } else if (addr == R_INTR_CTRL) {
         s->regs[addr] = value;
+    } else if (addr == R_DUMMY_DATA) {
+        s->regs[addr] = value & 0xff ;
     } else if (s->ctrl->has_dma && addr == R_DMA_CTRL) {
         aspeed_smc_dma_ctrl(s, value);
     } else if (s->ctrl->has_dma && addr == R_DMA_DRAM_ADDR) {
-- 
2.17.1

On 8/31/18 8:15 AM, Cédric Le Goater wrote:
> The SMC controllers have a register containing the byte that will be
> used as dummy output. It can be modified by software.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ssi/aspeed_smc.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index da2fedfcd3cd..f31bbc895caa 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -102,8 +102,8 @@
>  /* Misc Control Register #1 */
>  #define R_MISC_CTRL1      (0x50 / 4)
>  
> -/* Misc Control Register #2 */
> -#define R_MISC_CTRL2      (0x54 / 4)
> +/* SPI dummy cycle data */
> +#define R_DUMMY_DATA      (0x54 / 4)
>  
>  /* DMA Control/Status Register */
>  #define R_DMA_CTRL        (0x80 / 4)
> @@ -548,7 +548,7 @@ static void aspeed_smc_flash_setup(AspeedSMCFlash *fl, uint32_t addr)
>       */
>      if (aspeed_smc_flash_mode(fl) == CTRL_FREADMODE) {
>          for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) {
> -            ssi_transfer(fl->controller->spi, 0xFF);
> +            ssi_transfer(fl->controller->spi, s->regs[R_DUMMY_DATA] & 0xff);

The DUMMY_DATA register always contains a 8-bit value, so this AND
shouldn't be necessary. Regardless:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


>          }
>      }
>  }
> @@ -680,6 +680,7 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
>          addr == s->r_timings ||
>          addr == s->r_ce_ctrl ||
>          addr == R_INTR_CTRL ||
> +        addr == R_DUMMY_DATA ||
>          (s->ctrl->has_dma && addr == R_DMA_CTRL) ||
>          (s->ctrl->has_dma && addr == R_DMA_FLASH_ADDR) ||
>          (s->ctrl->has_dma && addr == R_DMA_DRAM_ADDR) ||
> @@ -912,6 +913,8 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
>          }
>      } else if (addr == R_INTR_CTRL) {
>          s->regs[addr] = value;
> +    } else if (addr == R_DUMMY_DATA) {
> +        s->regs[addr] = value & 0xff ;
>      } else if (s->ctrl->has_dma && addr == R_DMA_CTRL) {
>          aspeed_smc_dma_ctrl(s, value);
>      } else if (s->ctrl->has_dma && addr == R_DMA_DRAM_ADDR) {
>