| Message ID | 1449851831-4966-7-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Dec 11, 2015 at 8:37 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > Move the creation of the SD card device from the sdhci_sysbus > device itself into the boards that create these devices. > This allows us to remove the cannot_instantiate_with_device_add > notation because we no longer call drive_get_next in the device > model. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/arm/xilinx_zynq.c | 17 ++++++++++++++++- > hw/arm/xlnx-ep108.c | 19 +++++++++++++++++++ > hw/sd/sdhci.c | 22 ---------------------- > 3 files changed, 35 insertions(+), 23 deletions(-) > > diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c > index 1c1a445..3195055 100644 > --- a/hw/arm/xilinx_zynq.c > +++ b/hw/arm/xilinx_zynq.c > @@ -27,6 +27,7 @@ > #include "hw/misc/zynq-xadc.h" > #include "hw/ssi.h" > #include "qemu/error-report.h" > +#include "hw/sd/sd.h" > > #define NUM_SPI_FLASHES 4 > #define NUM_QSPI_FLASHES 2 > @@ -153,8 +154,10 @@ static void zynq_init(MachineState *machine) > MemoryRegion *address_space_mem = get_system_memory(); > MemoryRegion *ext_ram = g_new(MemoryRegion, 1); > MemoryRegion *ocm_ram = g_new(MemoryRegion, 1); > - DeviceState *dev; > + DeviceState *dev, *carddev; > SysBusDevice *busdev; > + DriveInfo *di; > + BlockBackend *blk; > qemu_irq pic[64]; > Error *err = NULL; > int n; > @@ -260,11 +263,23 @@ static void zynq_init(MachineState *machine) > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000); > sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]); > > + di = drive_get_next(IF_SD); > + blk = di ? blk_by_legacy_dinfo(di) : NULL; > + carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD); > + qdev_prop_set_drive(carddev, "drive", blk, &error_fatal); > + object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal); > + > dev = qdev_create(NULL, "generic-sdhci"); > qdev_init_nofail(dev); > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000); > sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]); > > + di = drive_get_next(IF_SD); > + blk = di ? blk_by_legacy_dinfo(di) : NULL; > + carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD); > + qdev_prop_set_drive(carddev, "drive", blk, &error_fatal); > + object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal); > + > dev = qdev_create(NULL, TYPE_ZYNQ_XADC); > qdev_init_nofail(dev); > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100); > diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c > index 85b978f..cb95d32 100644 > --- a/hw/arm/xlnx-ep108.c > +++ b/hw/arm/xlnx-ep108.c > @@ -34,6 +34,7 @@ static void xlnx_ep108_init(MachineState *machine) > { > XlnxEP108 *s = g_new0(XlnxEP108, 1); > Error *err = NULL; > + int i; > > object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP); > object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc), > @@ -45,6 +46,24 @@ static void xlnx_ep108_init(MachineState *machine) > exit(1); > } > > + /* Create and plug in the SD cards */ > + for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) { > + BusState *bus; > + DriveInfo *di = drive_get_next(IF_SD); > + BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL; > + DeviceState *carddev; > + > + bus = qdev_get_child_bus(DEVICE(&s->soc.sdhci[i]), "sd-bus"); This looks like the same thing I was trying to avoid with my SPI patches. We were trying to avoid the machine reaching into the SoC when getting the child busses. Instead expose the bus to the SoC so the board can just get it straight from there. > + if (!bus) { > + error_report("No SD bus found for SD card %d", i); > + exit(1); > + } > + carddev = qdev_create(bus, TYPE_SD); > + qdev_prop_set_drive(carddev, "drive", blk, &error_fatal); > + object_property_set_bool(OBJECT(carddev), true, "realized", > + &error_fatal); > + } > + I also think this should go after the other memory creation, not before. Thanks, Alistair > if (machine->ram_size > EP108_MAX_RAM_SIZE) { > error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, " > "reduced to %llx", machine->ram_size, EP108_MAX_RAM_SIZE); > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c > index 17e08a2..c8e3865 100644 > --- a/hw/sd/sdhci.c > +++ b/hw/sd/sdhci.c > @@ -1296,26 +1296,6 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp) > { > SDHCIState *s = SYSBUS_SDHCI(dev); > SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > - DriveInfo *di; > - BlockBackend *blk; > - DeviceState *carddev; > - > - /* Create and plug in the sd card. > - * FIXME: this should be done by the users of this device so we > - * do not use drive_get_next() here. > - */ > - di = drive_get_next(IF_SD); > - blk = di ? blk_by_legacy_dinfo(di) : NULL; > - > - carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD); > - qdev_prop_set_drive(carddev, "drive", blk, errp); > - if (*errp) { > - return; > - } > - object_property_set_bool(OBJECT(carddev), true, "realized", errp); > - if (*errp) { > - return; > - } > > s->buf_maxsz = sdhci_get_fifolen(s); > s->fifo_buffer = g_malloc0(s->buf_maxsz); > @@ -1333,8 +1313,6 @@ static void sdhci_sysbus_class_init(ObjectClass *klass, void *data) > dc->vmsd = &sdhci_vmstate; > dc->props = sdhci_sysbus_properties; > dc->realize = sdhci_sysbus_realize; > - /* Reason: instance_init() method uses drive_get_next() */ > - dc->cannot_instantiate_with_device_add_yet = true; > } > > static const TypeInfo sdhci_sysbus_info = { > -- > 1.9.1 > >
On 18 December 2015 at 00:18, Alistair Francis <alistair.francis@xilinx.com> wrote: > On Fri, Dec 11, 2015 at 8:37 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> >> + /* Create and plug in the SD cards */ >> + for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) { >> + BusState *bus; >> + DriveInfo *di = drive_get_next(IF_SD); >> + BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL; >> + DeviceState *carddev; >> + >> + bus = qdev_get_child_bus(DEVICE(&s->soc.sdhci[i]), "sd-bus"); > > This looks like the same thing I was trying to avoid with my SPI > patches. We were trying to avoid the machine reaching into the SoC > when getting the child busses. Instead expose the bus to the SoC so > the board can just get it straight from there. Yes, I wrote this code first and then saw your patches second. Whatever we do, we should deal with the problem the same way. thanks -- PMM
On Thu, Dec 17, 2015 at 04:18:19PM -0800, Alistair Francis wrote: > On Fri, Dec 11, 2015 at 8:37 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > > Move the creation of the SD card device from the sdhci_sysbus > > device itself into the boards that create these devices. > > This allows us to remove the cannot_instantiate_with_device_add > > notation because we no longer call drive_get_next in the device > > model. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > hw/arm/xilinx_zynq.c | 17 ++++++++++++++++- > > hw/arm/xlnx-ep108.c | 19 +++++++++++++++++++ > > hw/sd/sdhci.c | 22 ---------------------- > > 3 files changed, 35 insertions(+), 23 deletions(-) > > > > diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c > > index 1c1a445..3195055 100644 > > --- a/hw/arm/xilinx_zynq.c > > +++ b/hw/arm/xilinx_zynq.c > > @@ -27,6 +27,7 @@ > > #include "hw/misc/zynq-xadc.h" > > #include "hw/ssi.h" > > #include "qemu/error-report.h" > > +#include "hw/sd/sd.h" > > > > #define NUM_SPI_FLASHES 4 > > #define NUM_QSPI_FLASHES 2 > > @@ -153,8 +154,10 @@ static void zynq_init(MachineState *machine) > > MemoryRegion *address_space_mem = get_system_memory(); > > MemoryRegion *ext_ram = g_new(MemoryRegion, 1); > > MemoryRegion *ocm_ram = g_new(MemoryRegion, 1); > > - DeviceState *dev; > > + DeviceState *dev, *carddev; > > SysBusDevice *busdev; > > + DriveInfo *di; > > + BlockBackend *blk; > > qemu_irq pic[64]; > > Error *err = NULL; > > int n; > > @@ -260,11 +263,23 @@ static void zynq_init(MachineState *machine) > > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000); > > sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]); > > > > + di = drive_get_next(IF_SD); > > + blk = di ? blk_by_legacy_dinfo(di) : NULL; > > + carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD); > > + qdev_prop_set_drive(carddev, "drive", blk, &error_fatal); > > + object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal); > > + > > dev = qdev_create(NULL, "generic-sdhci"); > > qdev_init_nofail(dev); > > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000); > > sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]); > > > > + di = drive_get_next(IF_SD); > > + blk = di ? blk_by_legacy_dinfo(di) : NULL; > > + carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD); > > + qdev_prop_set_drive(carddev, "drive", blk, &error_fatal); > > + object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal); > > + > > dev = qdev_create(NULL, TYPE_ZYNQ_XADC); > > qdev_init_nofail(dev); > > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100); > > diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c > > index 85b978f..cb95d32 100644 > > --- a/hw/arm/xlnx-ep108.c > > +++ b/hw/arm/xlnx-ep108.c > > @@ -34,6 +34,7 @@ static void xlnx_ep108_init(MachineState *machine) > > { > > XlnxEP108 *s = g_new0(XlnxEP108, 1); > > Error *err = NULL; > > + int i; > > > > object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP); > > object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc), > > @@ -45,6 +46,24 @@ static void xlnx_ep108_init(MachineState *machine) > > exit(1); > > } > > > > + /* Create and plug in the SD cards */ > > + for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) { > > + BusState *bus; > > + DriveInfo *di = drive_get_next(IF_SD); > > + BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL; > > + DeviceState *carddev; > > + > > + bus = qdev_get_child_bus(DEVICE(&s->soc.sdhci[i]), "sd-bus"); > > This looks like the same thing I was trying to avoid with my SPI > patches. We were trying to avoid the machine reaching into the SoC > when getting the child busses. Instead expose the bus to the SoC so > the board can just get it straight from there. > I have an alternate QOM fix that should enable this. Will comment the SPI discussion. Regards, Peter > > + if (!bus) { > > + error_report("No SD bus found for SD card %d", i); > > + exit(1); > > + } > > + carddev = qdev_create(bus, TYPE_SD); > > + qdev_prop_set_drive(carddev, "drive", blk, &error_fatal); > > + object_property_set_bool(OBJECT(carddev), true, "realized", > > + &error_fatal); > > + } > > + > > I also think this should go after the other memory creation, not before. > > Thanks, > > Alistair > > > if (machine->ram_size > EP108_MAX_RAM_SIZE) { > > error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, " > > "reduced to %llx", machine->ram_size, EP108_MAX_RAM_SIZE); > > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c > > index 17e08a2..c8e3865 100644 > > --- a/hw/sd/sdhci.c > > +++ b/hw/sd/sdhci.c > > @@ -1296,26 +1296,6 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp) > > { > > SDHCIState *s = SYSBUS_SDHCI(dev); > > SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > > - DriveInfo *di; > > - BlockBackend *blk; > > - DeviceState *carddev; > > - > > - /* Create and plug in the sd card. > > - * FIXME: this should be done by the users of this device so we > > - * do not use drive_get_next() here. > > - */ > > - di = drive_get_next(IF_SD); > > - blk = di ? blk_by_legacy_dinfo(di) : NULL; > > - > > - carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD); > > - qdev_prop_set_drive(carddev, "drive", blk, errp); > > - if (*errp) { > > - return; > > - } > > - object_property_set_bool(OBJECT(carddev), true, "realized", errp); > > - if (*errp) { > > - return; > > - } > > > > s->buf_maxsz = sdhci_get_fifolen(s); > > s->fifo_buffer = g_malloc0(s->buf_maxsz); > > @@ -1333,8 +1313,6 @@ static void sdhci_sysbus_class_init(ObjectClass *klass, void *data) > > dc->vmsd = &sdhci_vmstate; > > dc->props = sdhci_sysbus_properties; > > dc->realize = sdhci_sysbus_realize; > > - /* Reason: instance_init() method uses drive_get_next() */ > > - dc->cannot_instantiate_with_device_add_yet = true; > > } > > > > static const TypeInfo sdhci_sysbus_info = { > > -- > > 1.9.1 > > > >
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c index 1c1a445..3195055 100644 --- a/hw/arm/xilinx_zynq.c +++ b/hw/arm/xilinx_zynq.c @@ -27,6 +27,7 @@ #include "hw/misc/zynq-xadc.h" #include "hw/ssi.h" #include "qemu/error-report.h" +#include "hw/sd/sd.h" #define NUM_SPI_FLASHES 4 #define NUM_QSPI_FLASHES 2 @@ -153,8 +154,10 @@ static void zynq_init(MachineState *machine) MemoryRegion *address_space_mem = get_system_memory(); MemoryRegion *ext_ram = g_new(MemoryRegion, 1); MemoryRegion *ocm_ram = g_new(MemoryRegion, 1); - DeviceState *dev; + DeviceState *dev, *carddev; SysBusDevice *busdev; + DriveInfo *di; + BlockBackend *blk; qemu_irq pic[64]; Error *err = NULL; int n; @@ -260,11 +263,23 @@ static void zynq_init(MachineState *machine) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000); sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]); + di = drive_get_next(IF_SD); + blk = di ? blk_by_legacy_dinfo(di) : NULL; + carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD); + qdev_prop_set_drive(carddev, "drive", blk, &error_fatal); + object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal); + dev = qdev_create(NULL, "generic-sdhci"); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000); sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]); + di = drive_get_next(IF_SD); + blk = di ? blk_by_legacy_dinfo(di) : NULL; + carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD); + qdev_prop_set_drive(carddev, "drive", blk, &error_fatal); + object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal); + dev = qdev_create(NULL, TYPE_ZYNQ_XADC); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100); diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c index 85b978f..cb95d32 100644 --- a/hw/arm/xlnx-ep108.c +++ b/hw/arm/xlnx-ep108.c @@ -34,6 +34,7 @@ static void xlnx_ep108_init(MachineState *machine) { XlnxEP108 *s = g_new0(XlnxEP108, 1); Error *err = NULL; + int i; object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP); object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc), @@ -45,6 +46,24 @@ static void xlnx_ep108_init(MachineState *machine) exit(1); } + /* Create and plug in the SD cards */ + for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) { + BusState *bus; + DriveInfo *di = drive_get_next(IF_SD); + BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL; + DeviceState *carddev; + + bus = qdev_get_child_bus(DEVICE(&s->soc.sdhci[i]), "sd-bus"); + if (!bus) { + error_report("No SD bus found for SD card %d", i); + exit(1); + } + carddev = qdev_create(bus, TYPE_SD); + qdev_prop_set_drive(carddev, "drive", blk, &error_fatal); + object_property_set_bool(OBJECT(carddev), true, "realized", + &error_fatal); + } + if (machine->ram_size > EP108_MAX_RAM_SIZE) { error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, " "reduced to %llx", machine->ram_size, EP108_MAX_RAM_SIZE); diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 17e08a2..c8e3865 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -1296,26 +1296,6 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp) { SDHCIState *s = SYSBUS_SDHCI(dev); SysBusDevice *sbd = SYS_BUS_DEVICE(dev); - DriveInfo *di; - BlockBackend *blk; - DeviceState *carddev; - - /* Create and plug in the sd card. - * FIXME: this should be done by the users of this device so we - * do not use drive_get_next() here. - */ - di = drive_get_next(IF_SD); - blk = di ? blk_by_legacy_dinfo(di) : NULL; - - carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD); - qdev_prop_set_drive(carddev, "drive", blk, errp); - if (*errp) { - return; - } - object_property_set_bool(OBJECT(carddev), true, "realized", errp); - if (*errp) { - return; - } s->buf_maxsz = sdhci_get_fifolen(s); s->fifo_buffer = g_malloc0(s->buf_maxsz); @@ -1333,8 +1313,6 @@ static void sdhci_sysbus_class_init(ObjectClass *klass, void *data) dc->vmsd = &sdhci_vmstate; dc->props = sdhci_sysbus_properties; dc->realize = sdhci_sysbus_realize; - /* Reason: instance_init() method uses drive_get_next() */ - dc->cannot_instantiate_with_device_add_yet = true; } static const TypeInfo sdhci_sysbus_info = {
Move the creation of the SD card device from the sdhci_sysbus device itself into the boards that create these devices. This allows us to remove the cannot_instantiate_with_device_add notation because we no longer call drive_get_next in the device model. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/xilinx_zynq.c | 17 ++++++++++++++++- hw/arm/xlnx-ep108.c | 19 +++++++++++++++++++ hw/sd/sdhci.c | 22 ---------------------- 3 files changed, 35 insertions(+), 23 deletions(-)