Re: [PATCH] hw/sd: Fix sun4i allwinner-sdhost for U-Boot

[Peter Maydell]

On Sat, 12 Nov 2022 at 21:49, Strahinja Jankovic
<strahinjapjankovic@gmail.com> wrote:
>
> Trying to run U-Boot for Cubieboard (Allwinner A10) fails because it cannot
> access SD card. The problem is that FIFO register in current
> allwinner-sdhost implementation is at the address corresponding to
> Allwinner H3, but not A10.
> Linux kernel is not affected since Linux driver uses DMA access and does
> not use FIFO register for reading/writing.
>
> This patch adds new class parameter `is_sun4i` and based on that
> parameter uses register at offset 0x100 either as FIFO register (if
> sun4i) or as threshold register (if not sun4i; in this case register at
> 0x200 is FIFO register).
>
> Tested with U-Boot and Linux kernel image built for Cubieboard and
> OrangePi PC.
>
> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> ---
>  hw/sd/allwinner-sdhost.c         | 67 ++++++++++++++++++++++----------
>  include/hw/sd/allwinner-sdhost.h |  1 +
>  2 files changed, 47 insertions(+), 21 deletions(-)
>
> diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c
> index 455d6eabf6..51e5e90830 100644
> --- a/hw/sd/allwinner-sdhost.c
> +++ b/hw/sd/allwinner-sdhost.c
> @@ -65,7 +65,7 @@ enum {
>      REG_SD_DLBA       = 0x84,  /* Descriptor List Base Address */
>      REG_SD_IDST       = 0x88,  /* Internal DMA Controller Status */
>      REG_SD_IDIE       = 0x8C,  /* Internal DMA Controller IRQ Enable */
> -    REG_SD_THLDC      = 0x100, /* Card Threshold Control */
> +    REG_SD_THLDC      = 0x100, /* Card Threshold Control / FIFO (sun4i 
> only)*/
>      REG_SD_DSBD       = 0x10C, /* eMMC DDR Start Bit Detection Control */
>      REG_SD_RES_CRC    = 0x110, /* Response CRC from card/eMMC */
>      REG_SD_DATA7_CRC  = 0x114, /* CRC Data 7 from card/eMMC */
> @@ -415,10 +415,29 @@ static void allwinner_sdhost_dma(AwSdHostState *s)
>      }
>  }
>
> +static uint32_t allwinner_sdhost_fifo_read(AwSdHostState *s)
> +{
> +    uint32_t res = 0;
> +
> +    if (sdbus_data_ready(&s->sdbus)) {
> +        sdbus_read_data(&s->sdbus, &res, sizeof(uint32_t));
> +        le32_to_cpus(&res);
> +        allwinner_sdhost_update_transfer_cnt(s, sizeof(uint32_t));
> +        allwinner_sdhost_auto_stop(s);
> +        allwinner_sdhost_update_irq(s);
> +    } else {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: no data ready on SD bus\n",
> +                      __func__);
> +    }
> +
> +    return res;
> +}
> +
>  static uint64_t allwinner_sdhost_read(void *opaque, hwaddr offset,
>                                        unsigned size)
>  {
>      AwSdHostState *s = AW_SDHOST(opaque);
> +    AwSdHostClass *sc = AW_SDHOST_GET_CLASS(s);
>      uint32_t res = 0;
>
>      switch (offset) {
> @@ -508,8 +527,12 @@ static uint64_t allwinner_sdhost_read(void *opaque, 
> hwaddr offset,
>      case REG_SD_IDIE:      /* Internal DMA Controller Interrupt Enable */
>          res = s->dmac_irq;
>          break;
> -    case REG_SD_THLDC:     /* Card Threshold Control */
> -        res = s->card_threshold;
> +    case REG_SD_THLDC:     /* Card Threshold Control or FIFO register 
> (sun4i) */
> +        if (sc->is_sun4i) {
> +            res = allwinner_sdhost_fifo_read(s);
> +        } else {
> +            res = s->card_threshold;
> +        }
>          break;
>      case REG_SD_DSBD:      /* eMMC DDR Start Bit Detection Control */
>          res = s->startbit_detect;
> @@ -531,16 +554,7 @@ static uint64_t allwinner_sdhost_read(void *opaque, 
> hwaddr offset,
>          res = s->status_crc;
>          break;
>      case REG_SD_FIFO:      /* Read/Write FIFO */
> -        if (sdbus_data_ready(&s->sdbus)) {
> -            sdbus_read_data(&s->sdbus, &res, sizeof(uint32_t));
> -            le32_to_cpus(&res);
> -            allwinner_sdhost_update_transfer_cnt(s, sizeof(uint32_t));
> -            allwinner_sdhost_auto_stop(s);
> -            allwinner_sdhost_update_irq(s);
> -        } else {
> -            qemu_log_mask(LOG_GUEST_ERROR, "%s: no data ready on SD bus\n",
> -                          __func__);
> -        }
> +        res = allwinner_sdhost_fifo_read(s);

Does the sun4i really have the FIFO at both addresses, or should
this one do something else for sun4i ?

thanks
-- PMM