Re: [PATCH for-5.2 08/19] aspeed/sdhci: Fix reset sequence

[Joel Stanley]

On Thu, 6 Aug 2020 at 13:21, Cédric Le Goater <clg@kaod.org> wrote:
>
> BIT(0) of the ASPEED_SDHCI_INFO register is set by SW and polled until
> the bit is cleared by HW. Add definitions for the default value of
> this register and fix the reset sequence by clearing the RESET bit.

This is mentioned in the datasheet but I couldn't find if software
depends on the behaviour. Were you just trying to make the model more
accurate?

>  #define ASPEED_SDHCI_INFO            0x00
> -#define  ASPEED_SDHCI_INFO_RESET     0x00030000
> +#define  ASPEED_SDHCI_INFO_SLOT1     (1 << 17)
> +#define  ASPEED_SDHCI_INFO_SLOT0     (1 << 16)
> +#define  ASPEED_SDHCI_INFO_RESET     (1 << 0)
>  #define ASPEED_SDHCI_DEBOUNCE        0x04
>  #define  ASPEED_SDHCI_DEBOUNCE_RESET 0x00000005
>  #define ASPEED_SDHCI_BUS             0x08
> @@ -67,6 +69,9 @@ static void aspeed_sdhci_write(void *opaque, hwaddr addr, 
> uint64_t val,
>      AspeedSDHCIState *sdhci = opaque;
>
>      switch (addr) {
> +    case ASPEED_SDHCI_INFO:
> +        sdhci->regs[TO_REG(addr)] = (uint32_t)val & ~ASPEED_SDHCI_INFO_RESET;

I think bits 24 and 25 should be writable too?

        sdhci->regs[TO_REG(addr)] = (uint32_t)val &
~(ASPEED_SDHCI_INFO_RESET | ASPEED_SDHCI_INFO_SLOT10 |
ASPEED_SDHCI_INFO_SLOT1);

> +
>      case ASPEED_SDHCI_SDIO_140:
>          sdhci->slots[0].capareg = (uint64_t)(uint32_t)val;
>          break;
> @@ -155,7 +160,8 @@ static void aspeed_sdhci_reset(DeviceState *dev)
>      AspeedSDHCIState *sdhci = ASPEED_SDHCI(dev);
>
>      memset(sdhci->regs, 0, ASPEED_SDHCI_REG_SIZE);
> -    sdhci->regs[TO_REG(ASPEED_SDHCI_INFO)] = ASPEED_SDHCI_INFO_RESET;
> +    sdhci->regs[TO_REG(ASPEED_SDHCI_INFO)] =
> +        ASPEED_SDHCI_INFO_SLOT1 | ASPEED_SDHCI_INFO_SLOT0;

If we want to be super strict this is true for the "sd" devices, but
the "emmc" device in the ast2600 only sets slot0. I don't think this
distinction is important to model though.

>      sdhci->regs[TO_REG(ASPEED_SDHCI_DEBOUNCE)] = ASPEED_SDHCI_DEBOUNCE_RESET;
>  }
>
> --
> 2.25.4
>