Patchwork [06/12] target-i386: replace FROM_SYSBUS() with QOM type cast

login
register
mail settings
Submitter Igor Mammedov
Date March 21, 2013, 2:28 p.m.
Message ID <1363876125-8264-7-git-send-email-imammedo@redhat.com>
Download mbox | patch
Permalink /patch/229717/
State New
Headers show

Comments

Igor Mammedov - March 21, 2013, 2:28 p.m.
... and define type name and type cast macro for kvmvapic according
to accepted convention.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/kvmvapic.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
Paolo Bonzini - March 27, 2013, 10:22 a.m.
Il 21/03/2013 15:28, Igor Mammedov ha scritto:
> ... and define type name and type cast macro for kvmvapic according
> to accepted convention.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/i386/kvmvapic.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> index c151c95..21551a5 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -62,6 +62,9 @@ typedef struct VAPICROMState {
>      bool rom_mapped_writable;
>  } VAPICROMState;
>  
> +#define TYPE_VAPIC_DEVICE "kvmvapic"
> +#define VAPIC_DEVICE(obj) OBJECT_CHECK(VAPICROMState, (obj), TYPE_VAPIC_DEVICE)
> +
>  #define TPR_INSTR_ABS_MODRM             0x1
>  #define TPR_INSTR_MATCH_MODRM_REG       0x2
>  
> @@ -692,7 +695,7 @@ static const MemoryRegionOps vapic_ops = {
>  
>  static int vapic_init(SysBusDevice *dev)
>  {
> -    VAPICROMState *s = FROM_SYSBUS(VAPICROMState, dev);
> +    VAPICROMState *s = VAPIC_DEVICE(dev);
>  
>      memory_region_init_io(&s->io, &vapic_ops, s, "kvmvapic", 2);
>      sysbus_add_io(dev, VAPIC_IO_PORT, &s->io);
> @@ -808,7 +811,7 @@ static void vapic_class_init(ObjectClass *klass, void *data)
>  }
>  
>  static const TypeInfo vapic_type = {
> -    .name          = "kvmvapic",
> +    .name          = TYPE_VAPIC_DEVICE,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(VAPICROMState),
>      .class_init    = vapic_class_init,
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Andreas Färber - April 4, 2013, 9:03 a.m.
Am 21.03.2013 15:28, schrieb Igor Mammedov:
> ... and define type name and type cast macro for kvmvapic according
> to accepted convention.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

This looks great and a cherry-pick candidate if you agree?

Just wondering, was there a name conflict for shorter VAPIC()?
It's file-local, so doesn't really matter.

Andreas

> ---
>  hw/i386/kvmvapic.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> index c151c95..21551a5 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -62,6 +62,9 @@ typedef struct VAPICROMState {
>      bool rom_mapped_writable;
>  } VAPICROMState;
>  
> +#define TYPE_VAPIC_DEVICE "kvmvapic"
> +#define VAPIC_DEVICE(obj) OBJECT_CHECK(VAPICROMState, (obj), TYPE_VAPIC_DEVICE)
> +
>  #define TPR_INSTR_ABS_MODRM             0x1
>  #define TPR_INSTR_MATCH_MODRM_REG       0x2
>  
> @@ -692,7 +695,7 @@ static const MemoryRegionOps vapic_ops = {
>  
>  static int vapic_init(SysBusDevice *dev)
>  {
> -    VAPICROMState *s = FROM_SYSBUS(VAPICROMState, dev);
> +    VAPICROMState *s = VAPIC_DEVICE(dev);
>  
>      memory_region_init_io(&s->io, &vapic_ops, s, "kvmvapic", 2);
>      sysbus_add_io(dev, VAPIC_IO_PORT, &s->io);
> @@ -808,7 +811,7 @@ static void vapic_class_init(ObjectClass *klass, void *data)
>  }
>  
>  static const TypeInfo vapic_type = {
> -    .name          = "kvmvapic",
> +    .name          = TYPE_VAPIC_DEVICE,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(VAPICROMState),
>      .class_init    = vapic_class_init,
>
Igor Mammedov - April 4, 2013, 9:59 a.m.
On Thu, 04 Apr 2013 11:03:53 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 21.03.2013 15:28, schrieb Igor Mammedov:
> > ... and define type name and type cast macro for kvmvapic according
> > to accepted convention.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> 
> This looks great and a cherry-pick candidate if you agree?
Np, BTW I have a second similar clean-up in v2 for IOAPIC

> 
> Just wondering, was there a name conflict for shorter VAPIC()?
> It's file-local, so doesn't really matter.
Nope, I just followed *_DEVICE model like for SYS_BUS_DEVICE()

> 
> Andreas
> 
> > ---
> >  hw/i386/kvmvapic.c |    7 +++++--
> >  1 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> > index c151c95..21551a5 100644
> > --- a/hw/i386/kvmvapic.c
> > +++ b/hw/i386/kvmvapic.c
> > @@ -62,6 +62,9 @@ typedef struct VAPICROMState {
> >      bool rom_mapped_writable;
> >  } VAPICROMState;
> >  
> > +#define TYPE_VAPIC_DEVICE "kvmvapic"
> > +#define VAPIC_DEVICE(obj) OBJECT_CHECK(VAPICROMState, (obj), TYPE_VAPIC_DEVICE)
> > +
> >  #define TPR_INSTR_ABS_MODRM             0x1
> >  #define TPR_INSTR_MATCH_MODRM_REG       0x2
> >  
> > @@ -692,7 +695,7 @@ static const MemoryRegionOps vapic_ops = {
> >  
> >  static int vapic_init(SysBusDevice *dev)
> >  {
> > -    VAPICROMState *s = FROM_SYSBUS(VAPICROMState, dev);
> > +    VAPICROMState *s = VAPIC_DEVICE(dev);
> >  
> >      memory_region_init_io(&s->io, &vapic_ops, s, "kvmvapic", 2);
> >      sysbus_add_io(dev, VAPIC_IO_PORT, &s->io);
> > @@ -808,7 +811,7 @@ static void vapic_class_init(ObjectClass *klass, void *data)
> >  }
> >  
> >  static const TypeInfo vapic_type = {
> > -    .name          = "kvmvapic",
> > +    .name          = TYPE_VAPIC_DEVICE,
> >      .parent        = TYPE_SYS_BUS_DEVICE,
> >      .instance_size = sizeof(VAPICROMState),
> >      .class_init    = vapic_class_init,
> > 
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
Andreas Färber - April 4, 2013, 10:05 a.m.
Am 04.04.2013 11:59, schrieb Igor Mammedov:
> On Thu, 04 Apr 2013 11:03:53 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Am 21.03.2013 15:28, schrieb Igor Mammedov:
>>> ... and define type name and type cast macro for kvmvapic according
>>> to accepted convention.
>>>
>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>
>> This looks great and a cherry-pick candidate if you agree?
> Np, BTW I have a second similar clean-up in v2 for IOAPIC
> 
>>
>> Just wondering, was there a name conflict for shorter VAPIC()?
>> It's file-local, so doesn't really matter.
> Nope, I just followed *_DEVICE model like for SYS_BUS_DEVICE()

Okay, then let's avoid unnecessary _DEVICE in your upcoming resend. The
only other place for actual devices where I remember we needed it was to
distinguish between PCI host bridge and its PCI device representation on
the bus. There is also no "Device" in the struct name.

Andreas
Igor Mammedov - April 4, 2013, 10:22 a.m.
On Thu, 04 Apr 2013 12:05:22 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 04.04.2013 11:59, schrieb Igor Mammedov:
> > On Thu, 04 Apr 2013 11:03:53 +0200
> > Andreas Färber <afaerber@suse.de> wrote:
> > 
> >> Am 21.03.2013 15:28, schrieb Igor Mammedov:
> >>> ... and define type name and type cast macro for kvmvapic according
> >>> to accepted convention.
> >>>
> >>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >>
> >> This looks great and a cherry-pick candidate if you agree?
> > Np, BTW I have a second similar clean-up in v2 for IOAPIC
> > 
> >>
> >> Just wondering, was there a name conflict for shorter VAPIC()?
> >> It's file-local, so doesn't really matter.
> > Nope, I just followed *_DEVICE model like for SYS_BUS_DEVICE()
> 
> Okay, then let's avoid unnecessary _DEVICE in your upcoming resend. The
> only other place for actual devices where I remember we needed it was to
> distinguish between PCI host bridge and its PCI device representation on
> the bus. There is also no "Device" in the struct name.
done

> 
> 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/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index c151c95..21551a5 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -62,6 +62,9 @@  typedef struct VAPICROMState {
     bool rom_mapped_writable;
 } VAPICROMState;
 
+#define TYPE_VAPIC_DEVICE "kvmvapic"
+#define VAPIC_DEVICE(obj) OBJECT_CHECK(VAPICROMState, (obj), TYPE_VAPIC_DEVICE)
+
 #define TPR_INSTR_ABS_MODRM             0x1
 #define TPR_INSTR_MATCH_MODRM_REG       0x2
 
@@ -692,7 +695,7 @@  static const MemoryRegionOps vapic_ops = {
 
 static int vapic_init(SysBusDevice *dev)
 {
-    VAPICROMState *s = FROM_SYSBUS(VAPICROMState, dev);
+    VAPICROMState *s = VAPIC_DEVICE(dev);
 
     memory_region_init_io(&s->io, &vapic_ops, s, "kvmvapic", 2);
     sysbus_add_io(dev, VAPIC_IO_PORT, &s->io);
@@ -808,7 +811,7 @@  static void vapic_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo vapic_type = {
-    .name          = "kvmvapic",
+    .name          = TYPE_VAPIC_DEVICE,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(VAPICROMState),
     .class_init    = vapic_class_init,