Re: [PATCH v2 12/13] hw/sd/sdhci: Implement Freescale eSDHC device model - Philippe Mathieu-Daudé via

From: "Philippe Mathieu-Daudé via" <qemu-devel@nongnu.org>
To: Bernhard Beschow <shentey@gmail.com>, qemu-devel@nongnu.org
Cc: qemu-ppc@nongnu.org, qemu-block@nongnu.org,
	Hanna Reitz <hreitz@redhat.com>,
	Bin Meng <bin.meng@windriver.com>, Kevin Wolf <kwolf@redhat.com>
Subject: Re: [PATCH v2 12/13] hw/sd/sdhci: Implement Freescale eSDHC device model
Date: Mon, 3 Oct 2022 23:11:23 +0200	[thread overview]
Message-ID: <aa253d5e-a2e3-e7b2-dace-87f21e64dc93@amsat.org> (raw)
In-Reply-To: <20221003203142.24355-13-shentey@gmail.com>

On 3/10/22 22:31, Bernhard Beschow wrote:
> Will allow e500 boards to access SD cards using just their own devices.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/sd/sdhci.c         | 147 +++++++++++++++++++++++++++++++++++++++++-
>   include/hw/sd/sdhci.h |   3 +
>   2 files changed, 149 insertions(+), 1 deletion(-)

> +/* --- qdev Freescale eSDHC --- */
> +
> +/* Watermark Level Register */
> +#define ESDHC_WML                    0x44
> +
> +/* Host Controller Capabilities Register 2 */
> +#define ESDHC_CAPABILITIES_1        0x114

Not used?

> +
> +/* Control Register for DMA transfer */
> +#define ESDHC_DMA_SYSCTL            0x40c
> +
> +#define ESDHC_REGISTERS_MAP_SIZE    0x410
> +
> +static uint64_t esdhci_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    uint64_t ret;
> +
> +    if (size != 4) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
> +                      " wrong size\n", size, offset);
> +        return 0;
> +    }
> +
> +    if (offset & 0x3) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
> +                      " unaligned\n", size, offset);
> +        return 0;

Isn't it already enforced by esdhc_mmio_ops.valid.unaligned = false?

> +    }
> +
> +    switch (offset) {
> +    case SDHC_SYSAD:
> +    case SDHC_BLKSIZE:
> +    case SDHC_ARGUMENT:
> +    case SDHC_TRNMOD:
> +    case SDHC_RSPREG0:
> +    case SDHC_RSPREG1:
> +    case SDHC_RSPREG2:
> +    case SDHC_RSPREG3:
> +    case SDHC_BDATA:
> +    case SDHC_PRNSTS:
> +    case SDHC_HOSTCTL:
> +    case SDHC_CLKCON:
> +    case SDHC_NORINTSTS:
> +    case SDHC_NORINTSTSEN:
> +    case SDHC_NORINTSIGEN:
> +    case SDHC_ACMD12ERRSTS:
> +    case SDHC_CAPAB:
> +    case SDHC_SLOT_INT_STATUS:
> +        ret = sdhci_read(opaque, offset, size);
> +        break;
> +
> +    case ESDHC_WML:
> +    case ESDHC_DMA_SYSCTL:
> +        ret = 0;
> +        qemu_log_mask(LOG_UNIMP, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
> +                      " not implemented\n", size, offset);
> +        break;
> +
> +    default:
> +        ret = 0;
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC rd_%ub @0x%02" HWADDR_PRIx
> +                      " unknown offset\n", size, offset);
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +static void esdhci_write(void *opaque, hwaddr offset, uint64_t val,
> +                         unsigned size)
> +{
> +    if (size != 4) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
> +                      " <- 0x%08lx wrong size\n", size, offset, val);
> +        return;
> +    }
> +
> +    if (offset & 0x3) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
> +                      " <- 0x%08lx unaligned\n", size, offset, val);
> +        return;
> +    }
> +
> +    switch (offset) {
> +    case SDHC_SYSAD:
> +    case SDHC_BLKSIZE:
> +    case SDHC_ARGUMENT:
> +    case SDHC_TRNMOD:
> +    case SDHC_BDATA:
> +    case SDHC_HOSTCTL:
> +    case SDHC_CLKCON:
> +    case SDHC_NORINTSTS:
> +    case SDHC_NORINTSTSEN:
> +    case SDHC_NORINTSIGEN:
> +    case SDHC_FEAER:
> +        sdhci_write(opaque, offset, val, size);
> +        break;
> +
> +    case ESDHC_WML:
> +    case ESDHC_DMA_SYSCTL:
> +        qemu_log_mask(LOG_UNIMP, "ESDHC wr_%ub @0x%02" HWADDR_PRIx " <- 0x%08lx "
> +                      "not implemented\n", size, offset, val);
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "ESDHC wr_%ub @0x%02" HWADDR_PRIx
> +                      " <- 0x%08lx unknown offset\n", size, offset, val);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps esdhc_mmio_ops = {
> +    .read = esdhci_read,
> +    .write = esdhci_write,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 4,
> +        .unaligned = false
> +    },
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};