Re: [PATCH v8 08/18] hw/arm/allwinner: add SD/MMC host controller

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 3/11/20 11:18 PM, Niek Linnenbank wrote:
> The Allwinner System on Chip families sun4i and above contain
> an integrated storage controller for Secure Digital (SD) and
> Multi Media Card (MMC) interfaces. This commit adds support
> for the Allwinner SD/MMC storage controller with the following
> emulated features:
> 
>  * DMA transfers
>  * Direct FIFO I/O
>  * Short/Long format command responses
>  * Auto-Stop command (CMD12)
>  * Insert & remove card detection
> 
> The following boards are extended with the SD host controller:
> 
>  * Cubieboard (hw/arm/cubieboard.c)
>  * Orange Pi PC (hw/arm/orangepi.c)
> 
> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/hw/arm/allwinner-a10.h   |   2 +
>  include/hw/arm/allwinner-h3.h    |   3 +
>  include/hw/sd/allwinner-sdhost.h | 135 +++++
>  hw/arm/allwinner-a10.c           |  11 +
>  hw/arm/allwinner-h3.c            |  15 +-
>  hw/arm/cubieboard.c              |  15 +
>  hw/arm/orangepi.c                |  16 +
>  hw/sd/allwinner-sdhost.c         | 854 +++++++++++++++++++++++++++++++
>  hw/arm/Kconfig                   |   1 +
>  hw/sd/Makefile.objs              |   1 +
>  hw/sd/trace-events               |   7 +
>  11 files changed, 1059 insertions(+), 1 deletion(-)
>  create mode 100644 include/hw/sd/allwinner-sdhost.h
>  create mode 100644 hw/sd/allwinner-sdhost.c
...

> +static uint64_t allwinner_sdhost_read(void *opaque, hwaddr offset,
> +                                      unsigned size)
> +{
> +    AwSdHostState *s = AW_SDHOST(opaque);
> +    uint32_t res = 0;
> +
> +    switch (offset) {
...

> +    case REG_SD_FIFO:      /* Read/Write FIFO */
> +        if (sdbus_data_ready(&s->sdbus)) {
> +            res = sdbus_read_data(&s->sdbus);
> +            res |= sdbus_read_data(&s->sdbus) << 8;
> +            res |= sdbus_read_data(&s->sdbus) << 16;
> +            res |= sdbus_read_data(&s->sdbus) << 24;

Also I'm a bit confused by the endianess here. Does the FIFO
uses a particular endianess?

> +            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__);
> +        }
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds offset %"
> +                      HWADDR_PRIx"\n", __func__, offset);
> +        res = 0;
> +        break;
> +    }
> +
> +    trace_allwinner_sdhost_read(offset, res, size);
> +    return res;
> +}
> +
> +static void allwinner_sdhost_write(void *opaque, hwaddr offset,
> +                                   uint64_t value, unsigned size)
> +{
> +    AwSdHostState *s = AW_SDHOST(opaque);
> +
> +    trace_allwinner_sdhost_write(offset, value, size);
> +
> +    switch (offset) {
...

> +    case REG_SD_FIFO:      /* Read/Write FIFO */
> +        sdbus_write_data(&s->sdbus, value & 0xff);
> +        sdbus_write_data(&s->sdbus, (value >> 8) & 0xff);
> +        sdbus_write_data(&s->sdbus, (value >> 16) & 0xff);
> +        sdbus_write_data(&s->sdbus, (value >> 24) & 0xff);
> +        allwinner_sdhost_update_transfer_cnt(s, sizeof(uint32_t));
> +        allwinner_sdhost_auto_stop(s);
> +        allwinner_sdhost_update_irq(s);
> +        break;
> +    case REG_SD_RES_CRC:   /* Response CRC from card/eMMC */
> +    case REG_SD_DATA7_CRC: /* CRC Data 7 from card/eMMC */
> +    case REG_SD_DATA6_CRC: /* CRC Data 6 from card/eMMC */
> +    case REG_SD_DATA5_CRC: /* CRC Data 5 from card/eMMC */
> +    case REG_SD_DATA4_CRC: /* CRC Data 4 from card/eMMC */
> +    case REG_SD_DATA3_CRC: /* CRC Data 3 from card/eMMC */
> +    case REG_SD_DATA2_CRC: /* CRC Data 2 from card/eMMC */
> +    case REG_SD_DATA1_CRC: /* CRC Data 1 from card/eMMC */
> +    case REG_SD_DATA0_CRC: /* CRC Data 0 from card/eMMC */
> +    case REG_SD_CRC_STA:   /* CRC status from card/eMMC in write operation */
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds offset %"
> +                      HWADDR_PRIx"\n", __func__, offset);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps allwinner_sdhost_ops = {
> +    .read = allwinner_sdhost_read,
> +    .write = allwinner_sdhost_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,

Maybe this device is little endian only?

> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .impl.min_access_size = 4,
> +};