| Message ID | 1371520688-24949-3-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
Hi Andreas, On Tue, Jun 18, 2013 at 11:58 AM, Andreas Färber <afaerber@suse.de> wrote: > Split qdev initfn into instance_init and realize functions. > Change one occurrence of "klass" while at it. > > Signed-off-by: Andreas Färber <afaerber@suse.de> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > hw/intc/openpic.c | 34 +++++++++++++++++++--------------- > 1 file changed, 19 insertions(+), 15 deletions(-) > > diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c > index 875c6b8..2d6b05c 100644 > --- a/hw/intc/openpic.c > +++ b/hw/intc/openpic.c > @@ -1531,8 +1531,16 @@ static void map_list(OpenPICState *opp, const MemReg *list, int *count) > } > } > > -static int openpic_init(SysBusDevice *dev) > +static void openpic_init(Object *obj) > { > + OpenPICState *opp = OPENPIC(obj); > + > + memory_region_init(&opp->mem, "openpic", 0x40000); > +} > + > +static void openpic_realize(DeviceState *dev, Error **errp) > +{ > + SysBusDevice *d = SYS_BUS_DEVICE(dev); FWIW, i have been using "sbd" for this variable name in similar conversions (sdhci, xilinx_spips, axidma, axienet and a few friends). There are also a few other precedents out there such as arm_gic. Regards, Peter > OpenPICState *opp = OPENPIC(dev); > int i, j; > int list_count = 0; > @@ -1566,8 +1574,6 @@ static int openpic_init(SysBusDevice *dev) > {NULL} > }; > > - memory_region_init(&opp->mem, "openpic", 0x40000); > - > switch (opp->model) { > case OPENPIC_MODEL_FSL_MPIC_20: > default: > @@ -1610,9 +1616,9 @@ static int openpic_init(SysBusDevice *dev) > opp->brr1 = -1; > opp->mpic_mode_mask = GCR_MODE_MIXED; > > - /* Only UP supported today */ > if (opp->nb_cpus != 1) { > - return -EINVAL; > + error_setg(errp, "Only UP supported today"); > + return; > } > > map_list(opp, list_le, &list_count); > @@ -1622,17 +1628,15 @@ static int openpic_init(SysBusDevice *dev) > for (i = 0; i < opp->nb_cpus; i++) { > opp->dst[i].irqs = g_new(qemu_irq, OPENPIC_OUTPUT_NB); > for (j = 0; j < OPENPIC_OUTPUT_NB; j++) { > - sysbus_init_irq(dev, &opp->dst[i].irqs[j]); > + sysbus_init_irq(d, &opp->dst[i].irqs[j]); > } > } > > - register_savevm(DEVICE(opp), "openpic", 0, 2, > + register_savevm(dev, "openpic", 0, 2, > openpic_save, openpic_load, opp); > > - sysbus_init_mmio(dev, &opp->mem); > - qdev_init_gpio_in(&dev->qdev, openpic_set_irq, opp->max_irq); > - > - return 0; > + sysbus_init_mmio(d, &opp->mem); > + qdev_init_gpio_in(dev, openpic_set_irq, opp->max_irq); > } > > static Property openpic_properties[] = { > @@ -1641,12 +1645,11 @@ static Property openpic_properties[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > -static void openpic_class_init(ObjectClass *klass, void *data) > +static void openpic_class_init(ObjectClass *oc, void *data) > { > - DeviceClass *dc = DEVICE_CLASS(klass); > - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > + DeviceClass *dc = DEVICE_CLASS(oc); > > - k->init = openpic_init; > + dc->realize = openpic_realize; > dc->props = openpic_properties; > dc->reset = openpic_reset; > } > @@ -1655,6 +1658,7 @@ static const TypeInfo openpic_info = { > .name = TYPE_OPENPIC, > .parent = TYPE_SYS_BUS_DEVICE, > .instance_size = sizeof(OpenPICState), > + .instance_init = openpic_init, > .class_init = openpic_class_init, > }; > > -- > 1.8.1.4 > >
Hi, Am 18.06.2013 05:28, schrieb Peter Crosthwaite: > On Tue, Jun 18, 2013 at 11:58 AM, Andreas Färber <afaerber@suse.de> wrote: >> Split qdev initfn into instance_init and realize functions. >> Change one occurrence of "klass" while at it. >> >> Signed-off-by: Andreas Färber <afaerber@suse.de> > > Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > >> --- >> hw/intc/openpic.c | 34 +++++++++++++++++++--------------- >> 1 file changed, 19 insertions(+), 15 deletions(-) >> >> diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c >> index 875c6b8..2d6b05c 100644 >> --- a/hw/intc/openpic.c >> +++ b/hw/intc/openpic.c >> @@ -1531,8 +1531,16 @@ static void map_list(OpenPICState *opp, const MemReg *list, int *count) >> } >> } >> >> -static int openpic_init(SysBusDevice *dev) >> +static void openpic_init(Object *obj) >> { >> + OpenPICState *opp = OPENPIC(obj); >> + >> + memory_region_init(&opp->mem, "openpic", 0x40000); >> +} >> + >> +static void openpic_realize(DeviceState *dev, Error **errp) >> +{ >> + SysBusDevice *d = SYS_BUS_DEVICE(dev); > > FWIW, i have been using "sbd" for this variable name in similar > conversions (sdhci, xilinx_spips, axidma, axienet and a few friends). > There are also a few other precedents out there such as arm_gic. So far we don't seem to have a consistent convention. I've seen busdev, sysbusdev, d; also pcidev vs. pci_dev vs. d for PCIDevice etc. sbd is fine with me, too. But since we're not yet consistent in using oc rather than klass either (including in your super class RFC), do you see a strong need to respin? Or can we just follow-up with a sed across the tree at some point once there is agreement on the naming? We should collect naming conventions into the QOMConventions Wiki page. Regards, Andreas
Hi Andreas, On Wed, Jun 19, 2013 at 1:49 AM, Andreas Färber <afaerber@suse.de> wrote: > Hi, > > Am 18.06.2013 05:28, schrieb Peter Crosthwaite: >> On Tue, Jun 18, 2013 at 11:58 AM, Andreas Färber <afaerber@suse.de> wrote: >>> Split qdev initfn into instance_init and realize functions. >>> Change one occurrence of "klass" while at it. >>> >>> Signed-off-by: Andreas Färber <afaerber@suse.de> >> >> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> >>> --- >>> hw/intc/openpic.c | 34 +++++++++++++++++++--------------- >>> 1 file changed, 19 insertions(+), 15 deletions(-) >>> >>> diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c >>> index 875c6b8..2d6b05c 100644 >>> --- a/hw/intc/openpic.c >>> +++ b/hw/intc/openpic.c >>> @@ -1531,8 +1531,16 @@ static void map_list(OpenPICState *opp, const MemReg *list, int *count) >>> } >>> } >>> >>> -static int openpic_init(SysBusDevice *dev) >>> +static void openpic_init(Object *obj) >>> { >>> + OpenPICState *opp = OPENPIC(obj); >>> + >>> + memory_region_init(&opp->mem, "openpic", 0x40000); >>> +} >>> + >>> +static void openpic_realize(DeviceState *dev, Error **errp) >>> +{ >>> + SysBusDevice *d = SYS_BUS_DEVICE(dev); >> >> FWIW, i have been using "sbd" for this variable name in similar >> conversions (sdhci, xilinx_spips, axidma, axienet and a few friends). >> There are also a few other precedents out there such as arm_gic. > > So far we don't seem to have a consistent convention. I've seen busdev, > sysbusdev, d; also pcidev vs. pci_dev vs. d for PCIDevice etc. > > sbd is fine with me, too. But since we're not yet consistent in using oc > rather than klass either (including in your super class RFC), do you see > a strong need to respin? No definitely not. Just trying to open the discussion so we can do this consistently in future. > Or can we just follow-up with a sed across the > tree at some point once there is agreement on the naming? > Yes. Sound like a plan. No point blocking these cleanups on undecided issues. Regards. Peter > We should collect naming conventions into the QOMConventions Wiki page. > > Regards, > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c index 875c6b8..2d6b05c 100644 --- a/hw/intc/openpic.c +++ b/hw/intc/openpic.c @@ -1531,8 +1531,16 @@ static void map_list(OpenPICState *opp, const MemReg *list, int *count) } } -static int openpic_init(SysBusDevice *dev) +static void openpic_init(Object *obj) { + OpenPICState *opp = OPENPIC(obj); + + memory_region_init(&opp->mem, "openpic", 0x40000); +} + +static void openpic_realize(DeviceState *dev, Error **errp) +{ + SysBusDevice *d = SYS_BUS_DEVICE(dev); OpenPICState *opp = OPENPIC(dev); int i, j; int list_count = 0; @@ -1566,8 +1574,6 @@ static int openpic_init(SysBusDevice *dev) {NULL} }; - memory_region_init(&opp->mem, "openpic", 0x40000); - switch (opp->model) { case OPENPIC_MODEL_FSL_MPIC_20: default: @@ -1610,9 +1616,9 @@ static int openpic_init(SysBusDevice *dev) opp->brr1 = -1; opp->mpic_mode_mask = GCR_MODE_MIXED; - /* Only UP supported today */ if (opp->nb_cpus != 1) { - return -EINVAL; + error_setg(errp, "Only UP supported today"); + return; } map_list(opp, list_le, &list_count); @@ -1622,17 +1628,15 @@ static int openpic_init(SysBusDevice *dev) for (i = 0; i < opp->nb_cpus; i++) { opp->dst[i].irqs = g_new(qemu_irq, OPENPIC_OUTPUT_NB); for (j = 0; j < OPENPIC_OUTPUT_NB; j++) { - sysbus_init_irq(dev, &opp->dst[i].irqs[j]); + sysbus_init_irq(d, &opp->dst[i].irqs[j]); } } - register_savevm(DEVICE(opp), "openpic", 0, 2, + register_savevm(dev, "openpic", 0, 2, openpic_save, openpic_load, opp); - sysbus_init_mmio(dev, &opp->mem); - qdev_init_gpio_in(&dev->qdev, openpic_set_irq, opp->max_irq); - - return 0; + sysbus_init_mmio(d, &opp->mem); + qdev_init_gpio_in(dev, openpic_set_irq, opp->max_irq); } static Property openpic_properties[] = { @@ -1641,12 +1645,11 @@ static Property openpic_properties[] = { DEFINE_PROP_END_OF_LIST(), }; -static void openpic_class_init(ObjectClass *klass, void *data) +static void openpic_class_init(ObjectClass *oc, void *data) { - DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); + DeviceClass *dc = DEVICE_CLASS(oc); - k->init = openpic_init; + dc->realize = openpic_realize; dc->props = openpic_properties; dc->reset = openpic_reset; } @@ -1655,6 +1658,7 @@ static const TypeInfo openpic_info = { .name = TYPE_OPENPIC, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(OpenPICState), + .instance_init = openpic_init, .class_init = openpic_class_init, };
Split qdev initfn into instance_init and realize functions. Change one occurrence of "klass" while at it. Signed-off-by: Andreas Färber <afaerber@suse.de> --- hw/intc/openpic.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-)