Re: [PATCH 1/2] hw/sd/pxa2xx_mmci: Do not create SD card within the SDHCI controller

[Alistair Francis]

On Sun, Jul 5, 2020 at 2:34 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

> ---
>  include/hw/arm/pxa.h |  3 +--
>  hw/arm/pxa2xx.c      | 39 +++++++++++++++++++++++++++++----------
>  hw/sd/pxa2xx_mmci.c  | 11 ++---------
>  3 files changed, 32 insertions(+), 21 deletions(-)
>
> diff --git a/include/hw/arm/pxa.h b/include/hw/arm/pxa.h
> index 8843e5f910..d99b6192da 100644
> --- a/include/hw/arm/pxa.h
> +++ b/include/hw/arm/pxa.h
> @@ -89,8 +89,7 @@ void pxa2xx_lcd_vsync_notifier(PXA2xxLCDState *s, qemu_irq 
> handler);
>  typedef struct PXA2xxMMCIState PXA2xxMMCIState;
>  PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
>                  hwaddr base,
> -                BlockBackend *blk, qemu_irq irq,
> -                qemu_irq rx_dma, qemu_irq tx_dma);
> +                qemu_irq irq, qemu_irq rx_dma, qemu_irq tx_dma);
>  void pxa2xx_mmci_handlers(PXA2xxMMCIState *s, qemu_irq readonly,
>                  qemu_irq coverswitch);
>
> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
> index f104a33463..78f6e69f63 100644
> --- a/hw/arm/pxa2xx.c
> +++ b/hw/arm/pxa2xx.c
> @@ -22,6 +22,7 @@
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/ssi/ssi.h"
> +#include "hw/sd/sd.h"
>  #include "chardev/char-fe.h"
>  #include "sysemu/blockdev.h"
>  #include "sysemu/qtest.h"
> @@ -2135,15 +2136,24 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space,
>
>      s->gpio = pxa2xx_gpio_init(0x40e00000, s->cpu, s->pic, 121);
>
> -    dinfo = drive_get(IF_SD, 0, 0);
> -    if (!dinfo && !qtest_enabled()) {
> -        warn_report("missing SecureDigital device");
> -    }
>      s->mmc = pxa2xx_mmci_init(address_space, 0x41100000,
> -                    dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
>                      qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC),
>                      qdev_get_gpio_in(s->dma, PXA2XX_RX_RQ_MMCI),
>                      qdev_get_gpio_in(s->dma, PXA2XX_TX_RQ_MMCI));
> +    dinfo = drive_get(IF_SD, 0, 0);
> +    if (dinfo) {
> +        DeviceState *carddev;
> +
> +        /* Create and plug in the sd card */
> +        carddev = qdev_new(TYPE_SD_CARD);
> +        qdev_prop_set_drive_err(carddev, "drive",
> +                                blk_by_legacy_dinfo(dinfo), &error_fatal);
> +        qdev_realize_and_unref(carddev, qdev_get_child_bus(DEVICE(s->mmc),
> +                                                           "sd-bus"),
> +                               &error_fatal);
> +    } else if (!qtest_enabled()) {
> +        warn_report("missing SecureDigital device");
> +    }
>
>      for (i = 0; pxa270_serial[i].io_base; i++) {
>          if (serial_hd(i)) {
> @@ -2259,15 +2269,24 @@ PXA2xxState *pxa255_init(MemoryRegion *address_space, 
> unsigned int sdram_size)
>
>      s->gpio = pxa2xx_gpio_init(0x40e00000, s->cpu, s->pic, 85);
>
> -    dinfo = drive_get(IF_SD, 0, 0);
> -    if (!dinfo && !qtest_enabled()) {
> -        warn_report("missing SecureDigital device");
> -    }
>      s->mmc = pxa2xx_mmci_init(address_space, 0x41100000,
> -                    dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
>                      qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC),
>                      qdev_get_gpio_in(s->dma, PXA2XX_RX_RQ_MMCI),
>                      qdev_get_gpio_in(s->dma, PXA2XX_TX_RQ_MMCI));
> +    dinfo = drive_get(IF_SD, 0, 0);
> +    if (dinfo) {
> +        DeviceState *carddev;
> +
> +        /* Create and plug in the sd card */
> +        carddev = qdev_new(TYPE_SD_CARD);
> +        qdev_prop_set_drive_err(carddev, "drive",
> +                                blk_by_legacy_dinfo(dinfo), &error_fatal);
> +        qdev_realize_and_unref(carddev, qdev_get_child_bus(DEVICE(s->mmc),
> +                                                           "sd-bus"),
> +                               &error_fatal);
> +    } else if (!qtest_enabled()) {
> +        warn_report("missing SecureDigital device");
> +    }
>
>      for (i = 0; pxa255_serial[i].io_base; i++) {
>          if (serial_hd(i)) {
> diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
> index 68bed24480..9482b9212d 100644
> --- a/hw/sd/pxa2xx_mmci.c
> +++ b/hw/sd/pxa2xx_mmci.c
> @@ -476,10 +476,9 @@ static const MemoryRegionOps pxa2xx_mmci_ops = {
>
>  PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
>                  hwaddr base,
> -                BlockBackend *blk, qemu_irq irq,
> -                qemu_irq rx_dma, qemu_irq tx_dma)
> +                qemu_irq irq, qemu_irq rx_dma, qemu_irq tx_dma)
>  {
> -    DeviceState *dev, *carddev;
> +    DeviceState *dev;
>      SysBusDevice *sbd;
>      PXA2xxMMCIState *s;
>
> @@ -492,12 +491,6 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
>      qdev_connect_gpio_out_named(dev, "tx-dma", 0, tx_dma);
>      sysbus_realize_and_unref(sbd, &error_fatal);
>
> -    /* Create and plug in the sd card */
> -    carddev = qdev_new(TYPE_SD_CARD);
> -    qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
> -    qdev_realize_and_unref(carddev, qdev_get_child_bus(dev, "sd-bus"),
> -                           &error_fatal);
> -
>      return s;
>  }
>
> --
> 2.21.3
>
>