This will make the type name constant consistent with the name of
the type checking macro.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: "Hervé Poussineau" <hpoussin@reactos.org>
Cc: qemu-ppc@nongnu.org
Cc: qemu-devel@nongnu.org
---
 include/hw/isa/pc87312.h | 4 ++--
 hw/isa/pc87312.c         | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
index a65168a157..da8dc5ddf5 100644
--- a/include/hw/isa/pc87312.h
+++ b/include/hw/isa/pc87312.h
@@ -29,10 +29,10 @@
 #include "qom/object.h"
 
 
-#define TYPE_PC87312_SUPERIO "pc87312"
+#define TYPE_PC87312 "pc87312"
 typedef struct PC87312State PC87312State;
 DECLARE_INSTANCE_CHECKER(PC87312State, PC87312,
-                         TYPE_PC87312_SUPERIO)
+                         TYPE_PC87312)
 
 struct PC87312State {
     /*< private >*/
diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
index 0cacbbc91b..8d7b8d3db2 100644
--- a/hw/isa/pc87312.c
+++ b/hw/isa/pc87312.c
@@ -371,7 +371,7 @@ static void pc87312_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo pc87312_type_info = {
-    .name          = TYPE_PC87312_SUPERIO,
+    .name          = TYPE_PC87312,
     .parent        = TYPE_ISA_SUPERIO,
     .instance_size = sizeof(PC87312State),
     .instance_init = pc87312_initfn,
-- 
2.26.2

Le 03/09/2020 à 00:42, Eduardo Habkost a écrit :
> This will make the type name constant consistent with the name of
> the type checking macro.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
>   include/hw/isa/pc87312.h | 4 ++--
>   hw/isa/pc87312.c         | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>

On 9/3/20 12:42 AM, Eduardo Habkost wrote:
> This will make the type name constant consistent with the name of
> the type checking macro.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
>  include/hw/isa/pc87312.h | 4 ++--
>  hw/isa/pc87312.c         | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
> index a65168a157..da8dc5ddf5 100644
> --- a/include/hw/isa/pc87312.h
> +++ b/include/hw/isa/pc87312.h
> @@ -29,10 +29,10 @@
>  #include "qom/object.h"
>  
>  
> -#define TYPE_PC87312_SUPERIO "pc87312"
> +#define TYPE_PC87312 "pc87312"

We loose self-documentation. What is a TYPE_PC87312
when reviewing a board setup code? Should we add a
comment /* Create the Super I/O */? The current name
is self-describing...

Is it easier to rename the type as 'pc87312-superio'?

   #define TYPE_PC87312_SUPERIO "pc87312-superio"

>  typedef struct PC87312State PC87312State;
>  DECLARE_INSTANCE_CHECKER(PC87312State, PC87312,
> -                         TYPE_PC87312_SUPERIO)
> +                         TYPE_PC87312)
>  
>  struct PC87312State {
>      /*< private >*/
> diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
> index 0cacbbc91b..8d7b8d3db2 100644
> --- a/hw/isa/pc87312.c
> +++ b/hw/isa/pc87312.c
> @@ -371,7 +371,7 @@ static void pc87312_class_init(ObjectClass *klass, void *data)
>  }
>  
>  static const TypeInfo pc87312_type_info = {
> -    .name          = TYPE_PC87312_SUPERIO,
> +    .name          = TYPE_PC87312,
>      .parent        = TYPE_ISA_SUPERIO,
>      .instance_size = sizeof(PC87312State),
>      .instance_init = pc87312_initfn,
> 

On Thu, Sep 03, 2020 at 02:45:12PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/3/20 12:42 AM, Eduardo Habkost wrote:
> > This will make the type name constant consistent with the name of
> > the type checking macro.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> > Cc: qemu-ppc@nongnu.org
> > Cc: qemu-devel@nongnu.org
> > ---
> >  include/hw/isa/pc87312.h | 4 ++--
> >  hw/isa/pc87312.c         | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
> > index a65168a157..da8dc5ddf5 100644
> > --- a/include/hw/isa/pc87312.h
> > +++ b/include/hw/isa/pc87312.h
> > @@ -29,10 +29,10 @@
> >  #include "qom/object.h"
> >  
> >  
> > -#define TYPE_PC87312_SUPERIO "pc87312"
> > +#define TYPE_PC87312 "pc87312"
> 
> We loose self-documentation. What is a TYPE_PC87312
> when reviewing a board setup code? Should we add a
> comment /* Create the Super I/O */? The current name
> is self-describing...
> 
> Is it easier to rename the type as 'pc87312-superio'?

This is an option.  In that case, I would like to rename the
PC87312 type checking macro to PC87312_SUPERIO, if that's OK.

The actual string name doesn't matter for the QOM macros, by the
way.  We can still rename it if you want to, but we don't have
to.

-- 
Eduardo

On Thu, Sep 03, 2020 at 12:16:47PM -0400, Eduardo Habkost wrote:
> On Thu, Sep 03, 2020 at 02:45:12PM +0200, Philippe Mathieu-Daudé wrote:
> > On 9/3/20 12:42 AM, Eduardo Habkost wrote:
> > > This will make the type name constant consistent with the name of
> > > the type checking macro.
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > > Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> > > Cc: qemu-ppc@nongnu.org
> > > Cc: qemu-devel@nongnu.org
> > > ---
> > >  include/hw/isa/pc87312.h | 4 ++--
> > >  hw/isa/pc87312.c         | 2 +-
> > >  2 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
> > > index a65168a157..da8dc5ddf5 100644
> > > --- a/include/hw/isa/pc87312.h
> > > +++ b/include/hw/isa/pc87312.h
> > > @@ -29,10 +29,10 @@
> > >  #include "qom/object.h"
> > >  
> > >  
> > > -#define TYPE_PC87312_SUPERIO "pc87312"
> > > +#define TYPE_PC87312 "pc87312"
> > 
> > We loose self-documentation. What is a TYPE_PC87312
> > when reviewing a board setup code? Should we add a
> > comment /* Create the Super I/O */? The current name
> > is self-describing...

I've just realized that TYPE_PC87312_SUPERIO is not used anywhere
in the code, so I don't understand where exactly this comment
applies.

> > 
> > Is it easier to rename the type as 'pc87312-superio'?
> 
> This is an option.  In that case, I would like to rename the
> PC87312 type checking macro to PC87312_SUPERIO, if that's OK.
> 
> The actual string name doesn't matter for the QOM macros, by the
> way.  We can still rename it if you want to, but we don't have
> to.

Based on Daniel's suggestion of keeping the macro names
consistent with the QOM type name string, I'd like to keep the
original color of the bike shed and keep this patch as is.

I will queue this patch on machine-next with Hervé's Reviewed-by
line.

If anybody wants to rename the user-visible QOM type name string
later, that's OK.  But I don't think this should be done as part
of the QOM boilerplate cleanup work.

-- 
Eduardo

On 9/3/20 7:22 PM, Eduardo Habkost wrote:
> On Thu, Sep 03, 2020 at 12:16:47PM -0400, Eduardo Habkost wrote:
>> On Thu, Sep 03, 2020 at 02:45:12PM +0200, Philippe Mathieu-Daudé wrote:
>>> On 9/3/20 12:42 AM, Eduardo Habkost wrote:
>>>> This will make the type name constant consistent with the name of
>>>> the type checking macro.
>>>>
>>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>>> ---
>>>> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
>>>> Cc: qemu-ppc@nongnu.org
>>>> Cc: qemu-devel@nongnu.org
>>>> ---
>>>>  include/hw/isa/pc87312.h | 4 ++--
>>>>  hw/isa/pc87312.c         | 2 +-
>>>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
>>>> index a65168a157..da8dc5ddf5 100644
>>>> --- a/include/hw/isa/pc87312.h
>>>> +++ b/include/hw/isa/pc87312.h
>>>> @@ -29,10 +29,10 @@
>>>>  #include "qom/object.h"
>>>>  
>>>>  
>>>> -#define TYPE_PC87312_SUPERIO "pc87312"
>>>> +#define TYPE_PC87312 "pc87312"
>>>
>>> We loose self-documentation. What is a TYPE_PC87312
>>> when reviewing a board setup code? Should we add a
>>> comment /* Create the Super I/O */? The current name
>>> is self-describing...
> 
> I've just realized that TYPE_PC87312_SUPERIO is not used anywhere
> in the code, so I don't understand where exactly this comment
> applies.
> 
>>>
>>> Is it easier to rename the type as 'pc87312-superio'?
>>
>> This is an option.  In that case, I would like to rename the
>> PC87312 type checking macro to PC87312_SUPERIO, if that's OK.
>>
>> The actual string name doesn't matter for the QOM macros, by the
>> way.  We can still rename it if you want to, but we don't have
>> to.
> 
> Based on Daniel's suggestion of keeping the macro names
> consistent with the QOM type name string, I'd like to keep the
> original color of the bike shed and keep this patch as is.
> 
> I will queue this patch on machine-next with Hervé's Reviewed-by
> line.
> 
> If anybody wants to rename the user-visible QOM type name string
> later, that's OK.  But I don't think this should be done as part
> of the QOM boilerplate cleanup work.

Understood, no problem.