Patchwork [2/2] intc/openpic: Convert to QOM realize

login
register
mail settings
Submitter Andreas Färber
Date June 18, 2013, 1:58 a.m.
Message ID <1371520688-24949-3-git-send-email-afaerber@suse.de>
Download mbox | patch
Permalink /patch/252093/
State New
Headers show

Comments

Andreas Färber - June 18, 2013, 1:58 a.m.
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(-)
Peter Crosthwaite - June 18, 2013, 3:28 a.m.
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
>
>
Andreas Färber - June 18, 2013, 3:49 p.m.
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
Peter Crosthwaite - June 18, 2013, 11:06 p.m.
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
>

Patch

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,
 };