| Message ID | 20161225082559.2095-1-zxq_yx_007@163.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25 December 2016 at 08:25, xiaoqiang zhao <zxq_yx_007@163.com> wrote: > Drop the old Sysbus init and use instance_init and > DeviceClass::realize instead > > Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com> > --- > hw/display/g364fb.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c > index 70ef2c7453..7a0fe48dd5 100644 > --- a/hw/display/g364fb.c > +++ b/hw/display/g364fb.c > @@ -504,18 +504,24 @@ typedef struct { > G364State g364; > } G364SysBusState; > > -static int g364fb_sysbus_init(SysBusDevice *sbd) > +static void g364fb_sysbus_init(Object *obj) This isn't a sysbus init function any more, so we should rename it. Since the existing g364fb_init() is only called from one place, we should just inline its contents into the right places, and have one function 364fb_init for the instance_init and g364fb_realize for the realize function. > { > + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > DeviceState *dev = DEVICE(sbd); > G364SysBusState *sbs = G364(dev); > G364State *s = &sbs->g364; > > - g364fb_init(dev, s); > sysbus_init_irq(sbd, &s->irq); > sysbus_init_mmio(sbd, &s->mem_ctrl); > sysbus_init_mmio(sbd, &s->mem_vram); This means we call sysbus_init_mmio() on the MemoryRegion*s before we have initialized them. That seems like a bad idea. > +} > > - return 0; > +static void g364fb_sysbus_realize(DeviceState *dev, Error **errp) > +{ > + G364SysBusState *sbs = G364(dev); > + G364State *s = &sbs->g364; > + > + g364fb_init(dev, s); > } > > static void g364fb_sysbus_reset(DeviceState *d) > @@ -534,9 +540,8 @@ static Property g364fb_sysbus_properties[] = { > static void g364fb_sysbus_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > > - k->init = g364fb_sysbus_init; > + dc->realize = g364fb_sysbus_realize; > set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories); > dc->desc = "G364 framebuffer"; > dc->reset = g364fb_sysbus_reset; > @@ -548,6 +553,7 @@ static const TypeInfo g364fb_sysbus_info = { > .name = TYPE_G364, > .parent = TYPE_SYS_BUS_DEVICE, > .instance_size = sizeof(G364SysBusState), > + .instance_init = g364fb_sysbus_init, > .class_init = g364fb_sysbus_class_init, > }; This device is used in the MIPS Jazz machine. Can you cc the MIPS maintainers and also Hervé Poussineau (the Jazz maintainer) on the next version of this patch, please? thanks -- PMM
在 2017年1月5日，23:10，Peter Maydell <peter.maydell@linaro.org> 写道： >> sysbus_init_irq(sbd, &s->irq); >> sysbus_init_mmio(sbd, &s->mem_ctrl); >> sysbus_init_mmio(sbd, &s->mem_vram); > > This means we call sysbus_init_mmio() on the MemoryRegion*s > before we have initialized them. That seems like a bad idea. Opps，will fix. > This device is used in the MIPS Jazz machine. > Can you cc the MIPS maintainers and also Hervé Poussineau > (the Jazz maintainer) on the next version of this patch, > please? OK！
diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c index 70ef2c7453..7a0fe48dd5 100644 --- a/hw/display/g364fb.c +++ b/hw/display/g364fb.c @@ -504,18 +504,24 @@ typedef struct { G364State g364; } G364SysBusState; -static int g364fb_sysbus_init(SysBusDevice *sbd) +static void g364fb_sysbus_init(Object *obj) { + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); DeviceState *dev = DEVICE(sbd); G364SysBusState *sbs = G364(dev); G364State *s = &sbs->g364; - g364fb_init(dev, s); sysbus_init_irq(sbd, &s->irq); sysbus_init_mmio(sbd, &s->mem_ctrl); sysbus_init_mmio(sbd, &s->mem_vram); +} - return 0; +static void g364fb_sysbus_realize(DeviceState *dev, Error **errp) +{ + G364SysBusState *sbs = G364(dev); + G364State *s = &sbs->g364; + + g364fb_init(dev, s); } static void g364fb_sysbus_reset(DeviceState *d) @@ -534,9 +540,8 @@ static Property g364fb_sysbus_properties[] = { static void g364fb_sysbus_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = g364fb_sysbus_init; + dc->realize = g364fb_sysbus_realize; set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories); dc->desc = "G364 framebuffer"; dc->reset = g364fb_sysbus_reset; @@ -548,6 +553,7 @@ static const TypeInfo g364fb_sysbus_info = { .name = TYPE_G364, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(G364SysBusState), + .instance_init = g364fb_sysbus_init, .class_init = g364fb_sysbus_class_init, };
Drop the old Sysbus init and use instance_init and DeviceClass::realize instead Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com> --- hw/display/g364fb.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)