[RESEND,3/5] hw/display: QOM'ify milkymist-tmu2.c

Commit Message

zhao xiao qiang May 5, 2016, 3:04 a.m. UTC

* Drop the old SysBus init function and use instance_init
* Move tmu2_glx_init into realize stage

Signed-off-by: xiaoqiang zhao <[e-mail]>
---
 hw/display/milkymist-tmu2.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

Comments

Peter Maydell May 5, 2016, 12:47 p.m. UTC | #1

On 5 May 2016 at 04:04, xiaoqiang zhao <[e-mail]> wrote:
> * Drop the old SysBus init function and use instance_init
> * Move tmu2_glx_init into realize stage
>
> Signed-off-by: xiaoqiang zhao <[e-mail]>

Reviewed-by: Peter Maydell <[e-mail]>

> +static void milkymist_tmu2_realize(DeviceState *dev, Error **errp)
> +{
> +    MilkymistTMU2State *s = MILKYMIST_TMU2(dev);
> +
> +    if (tmu2_glx_init(s)) {
> +        error_setg(errp, "tmu2_glx_init failed.");
> +    }
>  }

The milkymist maintainer might have a suggestion for a
more informative error message here.

thanks
-- PMM

Markus Armbruster May 6, 2016, 7:32 a.m. UTC | #2

Peter Maydell <[e-mail]> writes:

> On 5 May 2016 at 04:04, xiaoqiang zhao <[e-mail]> wrote:
>> * Drop the old SysBus init function and use instance_init
>> * Move tmu2_glx_init into realize stage
>>
>> Signed-off-by: xiaoqiang zhao <[e-mail]>
>
> Reviewed-by: Peter Maydell <[e-mail]>
>
>> +static void milkymist_tmu2_realize(DeviceState *dev, Error **errp)
>> +{
>> +    MilkymistTMU2State *s = MILKYMIST_TMU2(dev);
>> +
>> +    if (tmu2_glx_init(s)) {
>> +        error_setg(errp, "tmu2_glx_init failed.");
>> +    }
>>  }
>
> The milkymist maintainer might have a suggestion for a
> more informative error message here.

Also, error_setg() doesn't want the period:

 * The resulting message should be a single phrase, with no newline or
 * trailing punctuation.

Michael Walle May 9, 2016, 8:59 a.m. UTC | #3

Am 2016-05-06 09:32, schrieb Markus Armbruster:
> Peter Maydell <[e-mail]> writes:
> 
>> On 5 May 2016 at 04:04, xiaoqiang zhao <[e-mail]> wrote:
>>> * Drop the old SysBus init function and use instance_init
>>> * Move tmu2_glx_init into realize stage
>>> 
>>> Signed-off-by: xiaoqiang zhao <[e-mail]>
>> 
>> Reviewed-by: Peter Maydell <[e-mail]>
>> 
>>> +static void milkymist_tmu2_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    MilkymistTMU2State *s = MILKYMIST_TMU2(dev);
>>> +
>>> +    if (tmu2_glx_init(s)) {
>>> +        error_setg(errp, "tmu2_glx_init failed.");
>>> +    }
>>>  }
>> 
>> The milkymist maintainer might have a suggestion for a
>> more informative error message here.
> 
> Also, error_setg() doesn't want the period:
> 
>  * The resulting message should be a single phrase, with no newline or
>  * trailing punctuation.

Sorry, I can't think of a better description. The TMU2 code isn't my 
code. I had a look, but basically, it's because some glX functions might 
fail. But glx being not available isn't one of them because that is 
already checked in milkymist_tmu2_create().

-michael

Patch

diff --git a/hw/display/milkymist-tmu2.c b/hw/display/milkymist-tmu2.c
index 9bc88f9..df10bf4 100644
--- a/hw/display/milkymist-tmu2.c
+++ b/hw/display/milkymist-tmu2.c
@@ -443,21 +443,25 @@  static void milkymist_tmu2_reset(DeviceState *d)
     }
 }
 
-static int milkymist_tmu2_init(SysBusDevice *dev)
+static void milkymist_tmu2_init(Object *obj)
 {
-    MilkymistTMU2State *s = MILKYMIST_TMU2(dev);
-
-    if (tmu2_glx_init(s)) {
-        return 1;
-    }
+    MilkymistTMU2State *s = MILKYMIST_TMU2(obj);
+    SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
     sysbus_init_irq(dev, &s->irq);
 
-    memory_region_init_io(&s->regs_region, OBJECT(s), &tmu2_mmio_ops, s,
+    memory_region_init_io(&s->regs_region, obj, &tmu2_mmio_ops, s,
             "milkymist-tmu2", R_MAX * 4);
     sysbus_init_mmio(dev, &s->regs_region);
+}
 
-    return 0;
+static void milkymist_tmu2_realize(DeviceState *dev, Error **errp)
+{
+    MilkymistTMU2State *s = MILKYMIST_TMU2(dev);
+
+    if (tmu2_glx_init(s)) {
+        error_setg(errp, "tmu2_glx_init failed.");
+    }
 }
 
 static const VMStateDescription vmstate_milkymist_tmu2 = {
@@ -473,9 +477,8 @@  static const VMStateDescription vmstate_milkymist_tmu2 = {
 static void milkymist_tmu2_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = milkymist_tmu2_init;
+    dc->realize = milkymist_tmu2_realize;
     dc->reset = milkymist_tmu2_reset;
     dc->vmsd = &vmstate_milkymist_tmu2;
 }
@@ -484,6 +487,7 @@  static const TypeInfo milkymist_tmu2_info = {
     .name          = TYPE_MILKYMIST_TMU2,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(MilkymistTMU2State),
+    .instance_init = milkymist_tmu2_init,
     .class_init    = milkymist_tmu2_class_init,
 };