Re: [PATCH 4/4] hw/sd/milkymist: Do not create SD card within the SDHCI controller

[Alistair Francis]

On Sun, Jul 5, 2020 at 2:10 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> SDHCI controllers provide a SD Bus to plug SD cards, but don't
> come with SD card plugged in :) Let the machine/board object
> create and plug the SD cards when required.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/lm32/milkymist.c       | 13 +++++++++
>  hw/sd/milkymist-memcard.c | 55 +++++++++++++++++++++++----------------
>  2 files changed, 45 insertions(+), 23 deletions(-)
>
> diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
> index 117973c967..8e2c68da5a 100644
> --- a/hw/lm32/milkymist.c
> +++ b/hw/lm32/milkymist.c
> @@ -34,6 +34,7 @@
>  #include "elf.h"
>  #include "milkymist-hw.h"
>  #include "hw/display/milkymist_tmu2.h"
> +#include "hw/sd/sd.h"
>  #include "lm32.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/cutils.h"
> @@ -83,12 +84,24 @@ static void main_cpu_reset(void *opaque)
>  static DeviceState *milkymist_memcard_create(hwaddr base)
>  {
>      DeviceState *dev;
> +    DriveInfo *dinfo;
>
>      dev = qdev_new("milkymist-memcard");
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
>      /* FIXME wire 'card is readonly' and 'card inserted' IRQs? */
>
> +    dinfo = drive_get_next(IF_SD);
> +    if (dinfo) {
> +        DeviceState *card;
> +
> +        card = qdev_new(TYPE_SD_CARD);
> +        qdev_prop_set_drive_err(card, "drive", blk_by_legacy_dinfo(dinfo),
> +                                &error_fatal);
> +        qdev_realize_and_unref(card, qdev_get_child_bus(dev, "sd-bus"),
> +                               &error_fatal);
> +    }
> +
>      return dev;
>  }
>
> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
> index fb02309f07..e9f5db5e22 100644
> --- a/hw/sd/milkymist-memcard.c
> +++ b/hw/sd/milkymist-memcard.c
> @@ -66,6 +66,8 @@ enum {
>  #define MILKYMIST_MEMCARD(obj) \
>      OBJECT_CHECK(MilkymistMemcardState, (obj), TYPE_MILKYMIST_MEMCARD)
>
> +#define TYPE_MILKYMIST_SDBUS "milkymist-sdbus"
> +
>  struct MilkymistMemcardState {
>      SysBusDevice parent_obj;
>
> @@ -253,6 +255,19 @@ static void milkymist_memcard_reset(DeviceState *d)
>      }
>  }
>
> +static void milkymist_memcard_set_readonly(DeviceState *dev, bool level)
> +{
> +    qemu_log_mask(LOG_UNIMP,
> +                  "milkymist_memcard: read-only mode not supported\n");
> +}
> +
> +static void milkymist_memcard_set_inserted(DeviceState *dev, bool level)
> +{
> +    MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
> +
> +    s->enabled = !!level;
> +}
> +
>  static void milkymist_memcard_init(Object *obj)
>  {
>      MilkymistMemcardState *s = MILKYMIST_MEMCARD(obj);
> @@ -266,27 +281,6 @@ static void milkymist_memcard_init(Object *obj)
>                          DEVICE(obj), "sd-bus");
>  }
>
> -static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
> -{
> -    MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
> -    DeviceState *carddev;
> -    BlockBackend *blk;
> -    DriveInfo *dinfo;
> -    Error *err = NULL;
> -
> -    /* Create and plug in the sd card */
> -    /* FIXME use a qdev drive property instead of drive_get_next() */
> -    dinfo = drive_get_next(IF_SD);
> -    blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
> -    carddev = qdev_new(TYPE_SD_CARD);
> -    qdev_prop_set_drive(carddev, "drive", blk);
> -    if (!qdev_realize_and_unref(carddev, BUS(&s->sdbus), &err)) {
> -        error_propagate_prepend(errp, err, "failed to init SD card: %s");
> -        return;
> -    }
> -    s->enabled = blk && blk_is_inserted(blk);
> -}
> -
>  static const VMStateDescription vmstate_milkymist_memcard = {
>      .name = "milkymist-memcard",
>      .version_id = 1,
> @@ -308,10 +302,9 @@ static void milkymist_memcard_class_init(ObjectClass 
> *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
> -    dc->realize = milkymist_memcard_realize;
>      dc->reset = milkymist_memcard_reset;
>      dc->vmsd = &vmstate_milkymist_memcard;
> -    /* Reason: init() method uses drive_get_next() */
> +    /* Reason: output IRQs should be wired up */
>      dc->user_creatable = false;
>  }
>
> @@ -323,9 +316,25 @@ static const TypeInfo milkymist_memcard_info = {
>      .class_init    = milkymist_memcard_class_init,
>  };
>
> +static void milkymist_sdbus_class_init(ObjectClass *klass, void *data)
> +{
> +    SDBusClass *sbc = SD_BUS_CLASS(klass);
> +
> +    sbc->set_inserted = milkymist_memcard_set_inserted;
> +    sbc->set_readonly = milkymist_memcard_set_readonly;
> +}
> +
> +static const TypeInfo milkymist_sdbus_info = {
> +    .name = TYPE_MILKYMIST_SDBUS,
> +    .parent = TYPE_SD_BUS,
> +    .instance_size = sizeof(SDBus),
> +    .class_init = milkymist_sdbus_class_init,
> +};
> +
>  static void milkymist_memcard_register_types(void)
>  {
>      type_register_static(&milkymist_memcard_info);
> +    type_register_static(&milkymist_sdbus_info);
>  }
>
>  type_init(milkymist_memcard_register_types)
> --
> 2.21.3
>
>