Re: [PATCH v2 08/21] aspeed/sdhci: Fix reset sequence

[Joel Stanley]

On Wed, 19 Aug 2020 at 10:10, 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.
>
> Use the number of supported slots to define the default value of this
> register (The AST2600 eMMC Controller only has one). Fix the reset
> sequence by clearing automatically the RESET bit.
>
> Cc: Eddie James <eajames@linux.ibm.com>
> Fixes: 2bea128c3d0b ("hw/sd/aspeed_sdhci: New device")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Joel Stanley <joel@jms.id.au>

> ---
>  hw/sd/aspeed_sdhci.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sd/aspeed_sdhci.c b/hw/sd/aspeed_sdhci.c
> index 22cafce0fbdc..4f24b7d2f942 100644
> --- a/hw/sd/aspeed_sdhci.c
> +++ b/hw/sd/aspeed_sdhci.c
> @@ -16,7 +16,9 @@
>  #include "hw/qdev-properties.h"
>
>  #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,10 @@ static void aspeed_sdhci_write(void *opaque, hwaddr addr, 
> uint64_t val,
>      AspeedSDHCIState *sdhci = opaque;
>
>      switch (addr) {
> +    case ASPEED_SDHCI_INFO:
> +        /* The RESET bit automatically clears. */
> +        sdhci->regs[TO_REG(addr)] = (uint32_t)val & ~ASPEED_SDHCI_INFO_RESET;
> +        break;
>      case ASPEED_SDHCI_SDIO_140:
>          sdhci->slots[0].capareg = (uint64_t)(uint32_t)val;
>          break;
> @@ -155,7 +161,11 @@ 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_SLOT0;
> +    if (sdhci->num_slots == 2) {
> +        sdhci->regs[TO_REG(ASPEED_SDHCI_INFO)] |= ASPEED_SDHCI_INFO_SLOT1;
> +    }
>      sdhci->regs[TO_REG(ASPEED_SDHCI_DEBOUNCE)] = ASPEED_SDHCI_DEBOUNCE_RESET;
>  }
>
> --
> 2.25.4
>