| Message ID | 20180103162400.10396-3-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | SDHCI: convert legacy devices to the SDBus API | expand |
On Wed, Jan 3, 2018 at 8:23 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Create the SDCard in the realize() function. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com> Alistair > --- > hw/sd/milkymist-memcard.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c > index 5de3e00e2f..5df3a0f815 100644 > --- a/hw/sd/milkymist-memcard.c > +++ b/hw/sd/milkymist-memcard.c > @@ -255,24 +255,29 @@ static void milkymist_memcard_reset(DeviceState *d) > static int milkymist_memcard_init(SysBusDevice *dev) > { > MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); > - DriveInfo *dinfo; > + > + memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s, > + "milkymist-memcard", R_MAX * 4); > + sysbus_init_mmio(dev, &s->regs_region); > + > + return 0; > +} > + > +static void milkymist_memcard_realize(DeviceState *dev, Error **errp) > +{ > + MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); > BlockBackend *blk; > + DriveInfo *dinfo; > > /* 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; > s->card = sd_init(blk, false); > if (s->card == NULL) { > - return -1; > + error_setg(errp, "failed to init SD card"); > + return; > } > - > s->enabled = blk && blk_is_inserted(blk); > - > - memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s, > - "milkymist-memcard", R_MAX * 4); > - sysbus_init_mmio(dev, &s->regs_region); > - > - return 0; > } > > static const VMStateDescription vmstate_milkymist_memcard = { > @@ -298,6 +303,7 @@ static void milkymist_memcard_class_init(ObjectClass *klass, void *data) > SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > > k->init = milkymist_memcard_init; > + dc->realize = milkymist_memcard_realize; > dc->reset = milkymist_memcard_reset; > dc->vmsd = &vmstate_milkymist_memcard; > /* Reason: init() method uses drive_get_next() */ > -- > 2.15.1 > >
Hey Philippe, Am 2018-01-03 17:23, schrieb Philippe Mathieu-Daudé: > Create the SDCard in the realize() function. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/sd/milkymist-memcard.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c > index 5de3e00e2f..5df3a0f815 100644 > --- a/hw/sd/milkymist-memcard.c > +++ b/hw/sd/milkymist-memcard.c > @@ -255,24 +255,29 @@ static void milkymist_memcard_reset(DeviceState > *d) > static int milkymist_memcard_init(SysBusDevice *dev) > { > MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); > - DriveInfo *dinfo; > + > + memory_region_init_io(&s->regs_region, OBJECT(s), > &memcard_mmio_ops, s, > + "milkymist-memcard", R_MAX * 4); > + sysbus_init_mmio(dev, &s->regs_region); > + > + return 0; > +} Creating the device (milkymist_memcard_create()) fails with an assertion: sysbus_mmio_map_common: Assertion `n >= 0 && n < dev->num_mmio' failed. because dev->num_mmio is still 0. Seems that sysbus_init_mmio() hasn't been called yet. Do we have to move the milkymist_memcard_init() to .instance_init? Eg. diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c index 5df3a0f815..21c0e91926 100644 --- a/hw/sd/milkymist-memcard.c +++ b/hw/sd/milkymist-memcard.c @@ -252,15 +252,14 @@ static void milkymist_memcard_reset(DeviceState *d) } } -static int milkymist_memcard_init(SysBusDevice *dev) +static void milkymist_memcard_init(Object *obj) { - MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); + MilkymistMemcardState *s = MILKYMIST_MEMCARD(obj); + SysBusDevice *dev = SYS_BUS_DEVICE(obj); memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s, "milkymist-memcard", R_MAX * 4); sysbus_init_mmio(dev, &s->regs_region); - - return 0; } static void milkymist_memcard_realize(DeviceState *dev, Error **errp) @@ -300,9 +299,7 @@ static const VMStateDescription vmstate_milkymist_memcard = { static void milkymist_memcard_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = milkymist_memcard_init; dc->realize = milkymist_memcard_realize; dc->reset = milkymist_memcard_reset; dc->vmsd = &vmstate_milkymist_memcard; @@ -314,6 +311,7 @@ static const TypeInfo milkymist_memcard_info = { .name = TYPE_MILKYMIST_MEMCARD, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(MilkymistMemcardState), + .instance_init = milkymist_memcard_init, .class_init = milkymist_memcard_class_init, }; -michael > + > +static void milkymist_memcard_realize(DeviceState *dev, Error **errp) > +{ > + MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); > BlockBackend *blk; > + DriveInfo *dinfo; > > /* 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; > s->card = sd_init(blk, false); > if (s->card == NULL) { > - return -1; > + error_setg(errp, "failed to init SD card"); > + return; > } > - > s->enabled = blk && blk_is_inserted(blk); > - > - memory_region_init_io(&s->regs_region, OBJECT(s), > &memcard_mmio_ops, s, > - "milkymist-memcard", R_MAX * 4); > - sysbus_init_mmio(dev, &s->regs_region); > - > - return 0; > } > > static const VMStateDescription vmstate_milkymist_memcard = { > @@ -298,6 +303,7 @@ static void > milkymist_memcard_class_init(ObjectClass *klass, void *data) > SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > > k->init = milkymist_memcard_init; > + dc->realize = milkymist_memcard_realize; > dc->reset = milkymist_memcard_reset; > dc->vmsd = &vmstate_milkymist_memcard; > /* Reason: init() method uses drive_get_next() */
Hi Michael, > Am 2018-01-03 17:23, schrieb Philippe Mathieu-Daudé: >> >> Create the SDCard in the realize() function. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> hw/sd/milkymist-memcard.c | 24 +++++++++++++++--------- >> 1 file changed, 15 insertions(+), 9 deletions(-) >> >> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c >> index 5de3e00e2f..5df3a0f815 100644 >> --- a/hw/sd/milkymist-memcard.c >> +++ b/hw/sd/milkymist-memcard.c >> @@ -255,24 +255,29 @@ static void milkymist_memcard_reset(DeviceState *d) >> static int milkymist_memcard_init(SysBusDevice *dev) >> { >> MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); >> - DriveInfo *dinfo; >> + >> + memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, >> s, >> + "milkymist-memcard", R_MAX * 4); >> + sysbus_init_mmio(dev, &s->regs_region); >> + >> + return 0; >> +} > > > Creating the device (milkymist_memcard_create()) fails with an assertion: > sysbus_mmio_map_common: Assertion `n >= 0 && n < dev->num_mmio' failed. Thanks for trying this. Odd 'make check-qtest-lm32' didn't catch this... > because dev->num_mmio is still 0. Seems that sysbus_init_mmio() hasn't been > called yet. Do we have to move the milkymist_memcard_init() to > .instance_init? Your patch seems correct :) Do you have some milky one prebuilt image that I can use for testing? Regards, Phil. > > Eg. > > diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c > index 5df3a0f815..21c0e91926 100644 > --- a/hw/sd/milkymist-memcard.c > +++ b/hw/sd/milkymist-memcard.c > @@ -252,15 +252,14 @@ static void milkymist_memcard_reset(DeviceState *d) > } > } > > -static int milkymist_memcard_init(SysBusDevice *dev) > +static void milkymist_memcard_init(Object *obj) > { > - MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); > + MilkymistMemcardState *s = MILKYMIST_MEMCARD(obj); > + SysBusDevice *dev = SYS_BUS_DEVICE(obj); > > memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s, > "milkymist-memcard", R_MAX * 4); > sysbus_init_mmio(dev, &s->regs_region); > - > - return 0; > } > > static void milkymist_memcard_realize(DeviceState *dev, Error **errp) > @@ -300,9 +299,7 @@ static const VMStateDescription > vmstate_milkymist_memcard = { > static void milkymist_memcard_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > > - k->init = milkymist_memcard_init; > dc->realize = milkymist_memcard_realize; > dc->reset = milkymist_memcard_reset; > dc->vmsd = &vmstate_milkymist_memcard; > @@ -314,6 +311,7 @@ static const TypeInfo milkymist_memcard_info = { > .name = TYPE_MILKYMIST_MEMCARD, > .parent = TYPE_SYS_BUS_DEVICE, > .instance_size = sizeof(MilkymistMemcardState), > + .instance_init = milkymist_memcard_init, > .class_init = milkymist_memcard_class_init, > }; > > -michael > > >> + >> +static void milkymist_memcard_realize(DeviceState *dev, Error **errp) >> +{ >> + MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); >> BlockBackend *blk; >> + DriveInfo *dinfo; >> >> /* 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; >> s->card = sd_init(blk, false); >> if (s->card == NULL) { >> - return -1; >> + error_setg(errp, "failed to init SD card"); >> + return; >> } >> - >> s->enabled = blk && blk_is_inserted(blk); >> - >> - memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, >> s, >> - "milkymist-memcard", R_MAX * 4); >> - sysbus_init_mmio(dev, &s->regs_region); >> - >> - return 0; >> } >> >> static const VMStateDescription vmstate_milkymist_memcard = { >> @@ -298,6 +303,7 @@ static void >> milkymist_memcard_class_init(ObjectClass *klass, void *data) >> SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); >> >> k->init = milkymist_memcard_init; >> + dc->realize = milkymist_memcard_realize; >> dc->reset = milkymist_memcard_reset; >> dc->vmsd = &vmstate_milkymist_memcard; >> /* Reason: init() method uses drive_get_next() */
>> Creating the device (milkymist_memcard_create()) fails with an assertion: >> sysbus_mmio_map_common: Assertion `n >= 0 && n < dev->num_mmio' failed. > > Thanks for trying this. > > Odd 'make check-qtest-lm32' didn't catch this... Oh it actually does... My bad! GTESTER check-qtest-lm32 qemu-system-lm32: hw/core/sysbus.c:131: sysbus_mmio_map_common: Assertion `n >= 0 && n < dev->num_mmio' failed. Broken pipe tests/Makefile.include:854: recipe for target 'check-qtest-lm32' failed make: *** [check-qtest-lm32] Error 1
Hi Philippe, Am 2018-01-09 19:16, schrieb Philippe Mathieu-Daudé: > Hi Michael, > >> Am 2018-01-03 17:23, schrieb Philippe Mathieu-Daudé: >>> >>> Create the SDCard in the realize() function. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>> --- >>> hw/sd/milkymist-memcard.c | 24 +++++++++++++++--------- >>> 1 file changed, 15 insertions(+), 9 deletions(-) >>> >>> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c >>> index 5de3e00e2f..5df3a0f815 100644 >>> --- a/hw/sd/milkymist-memcard.c >>> +++ b/hw/sd/milkymist-memcard.c >>> @@ -255,24 +255,29 @@ static void milkymist_memcard_reset(DeviceState >>> *d) >>> static int milkymist_memcard_init(SysBusDevice *dev) >>> { >>> MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); >>> - DriveInfo *dinfo; >>> + >>> + memory_region_init_io(&s->regs_region, OBJECT(s), >>> &memcard_mmio_ops, >>> s, >>> + "milkymist-memcard", R_MAX * 4); >>> + sysbus_init_mmio(dev, &s->regs_region); >>> + >>> + return 0; >>> +} >> >> >> Creating the device (milkymist_memcard_create()) fails with an >> assertion: >> sysbus_mmio_map_common: Assertion `n >= 0 && n < dev->num_mmio' >> failed. > > Thanks for trying this. > > Odd 'make check-qtest-lm32' didn't catch this... > >> because dev->num_mmio is still 0. Seems that sysbus_init_mmio() hasn't >> been >> called yet. Do we have to move the milkymist_memcard_init() to >> .instance_init? > > Your patch seems correct :) > > Do you have some milky one prebuilt image that I can use for testing? You can use this one: http://milkymist.walle.cc/updates/2012-03-01/flickernoise http://milkymist.walle.cc/README.qemu -michael
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c index 5de3e00e2f..5df3a0f815 100644 --- a/hw/sd/milkymist-memcard.c +++ b/hw/sd/milkymist-memcard.c @@ -255,24 +255,29 @@ static void milkymist_memcard_reset(DeviceState *d) static int milkymist_memcard_init(SysBusDevice *dev) { MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); - DriveInfo *dinfo; + + memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s, + "milkymist-memcard", R_MAX * 4); + sysbus_init_mmio(dev, &s->regs_region); + + return 0; +} + +static void milkymist_memcard_realize(DeviceState *dev, Error **errp) +{ + MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev); BlockBackend *blk; + DriveInfo *dinfo; /* 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; s->card = sd_init(blk, false); if (s->card == NULL) { - return -1; + error_setg(errp, "failed to init SD card"); + return; } - s->enabled = blk && blk_is_inserted(blk); - - memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s, - "milkymist-memcard", R_MAX * 4); - sysbus_init_mmio(dev, &s->regs_region); - - return 0; } static const VMStateDescription vmstate_milkymist_memcard = { @@ -298,6 +303,7 @@ static void milkymist_memcard_class_init(ObjectClass *klass, void *data) SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); k->init = milkymist_memcard_init; + dc->realize = milkymist_memcard_realize; dc->reset = milkymist_memcard_reset; dc->vmsd = &vmstate_milkymist_memcard; /* Reason: init() method uses drive_get_next() */
Create the SDCard in the realize() function. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/sd/milkymist-memcard.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)