1. Patchew
  2. QEMU
  3. [PATCH 00/23] E500 Cleanup
  4. View patch

[PATCH 11/23] hw/i2c/mpc_i2c: Prefer DEFINE_TYPES() macro

Bernhard Beschow posted 23 patches 4 weeks, 1 day ago
There is a newer version of this series
[PATCH 11/23] hw/i2c/mpc_i2c: Prefer DEFINE_TYPES() macro
Posted by Bernhard Beschow 4 weeks, 1 day ago 
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i2c/mpc_i2c.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/i2c/mpc_i2c.c b/hw/i2c/mpc_i2c.c
index 3d79c15653..16f4309ea9 100644
--- a/hw/i2c/mpc_i2c.c
+++ b/hw/i2c/mpc_i2c.c
@@ -20,7 +20,6 @@
 #include "qemu/osdep.h"
 #include "hw/i2c/i2c.h"
 #include "hw/irq.h"
-#include "qemu/module.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "qom/object.h"
@@ -345,16 +344,13 @@ static void mpc_i2c_class_init(ObjectClass *klass, void *data)
     dc->desc = "MPC I2C Controller";
 }
 
-static const TypeInfo mpc_i2c_type_info = {
-    .name          = TYPE_MPC_I2C,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(MPCI2CState),
-    .class_init    = mpc_i2c_class_init,
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_MPC_I2C,
+        .parent        = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(MPCI2CState),
+        .class_init    = mpc_i2c_class_init,
+    },
 };
 
-static void mpc_i2c_register_types(void)
-{
-    type_register_static(&mpc_i2c_type_info);
-}
-
-type_init(mpc_i2c_register_types)
+DEFINE_TYPES(types)
-- 
2.46.1
Re: [PATCH 11/23] hw/i2c/mpc_i2c: Prefer DEFINE_TYPES() macro
Posted by Cédric Le Goater 3 weeks, 6 days ago 
On 9/23/24 11:30, Bernhard Beschow wrote:
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/i2c/mpc_i2c.c | 20 ++++++++------------
>   1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/i2c/mpc_i2c.c b/hw/i2c/mpc_i2c.c
> index 3d79c15653..16f4309ea9 100644
> --- a/hw/i2c/mpc_i2c.c
> +++ b/hw/i2c/mpc_i2c.c
> @@ -20,7 +20,6 @@
>   #include "qemu/osdep.h"
>   #include "hw/i2c/i2c.h"
>   #include "hw/irq.h"
> -#include "qemu/module.h"
>   #include "hw/sysbus.h"
>   #include "migration/vmstate.h"
>   #include "qom/object.h"
> @@ -345,16 +344,13 @@ static void mpc_i2c_class_init(ObjectClass *klass, void *data)
>       dc->desc = "MPC I2C Controller";
>   }
>   
> -static const TypeInfo mpc_i2c_type_info = {
> -    .name          = TYPE_MPC_I2C,
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(MPCI2CState),
> -    .class_init    = mpc_i2c_class_init,
> +static const TypeInfo types[] = {
> +    {
> +        .name          = TYPE_MPC_I2C,
> +        .parent        = TYPE_SYS_BUS_DEVICE,
> +        .instance_size = sizeof(MPCI2CState),
> +        .class_init    = mpc_i2c_class_init,
> +    },
>   };
>   
> -static void mpc_i2c_register_types(void)
> -{
> -    type_register_static(&mpc_i2c_type_info);
> -}
> -
> -type_init(mpc_i2c_register_types)
> +DEFINE_TYPES(types)
Re: [PATCH 11/23] hw/i2c/mpc_i2c: Prefer DEFINE_TYPES() macro
Posted by BALATON Zoltan 4 weeks, 1 day ago 
On Mon, 23 Sep 2024, Bernhard Beschow wrote:
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/i2c/mpc_i2c.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/hw/i2c/mpc_i2c.c b/hw/i2c/mpc_i2c.c
> index 3d79c15653..16f4309ea9 100644
> --- a/hw/i2c/mpc_i2c.c
> +++ b/hw/i2c/mpc_i2c.c
> @@ -20,7 +20,6 @@
> #include "qemu/osdep.h"
> #include "hw/i2c/i2c.h"
> #include "hw/irq.h"
> -#include "qemu/module.h"
> #include "hw/sysbus.h"
> #include "migration/vmstate.h"
> #include "qom/object.h"
> @@ -345,16 +344,13 @@ static void mpc_i2c_class_init(ObjectClass *klass, void *data)
>     dc->desc = "MPC I2C Controller";
> }
>
> -static const TypeInfo mpc_i2c_type_info = {
> -    .name          = TYPE_MPC_I2C,
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(MPCI2CState),
> -    .class_init    = mpc_i2c_class_init,
> +static const TypeInfo types[] = {
> +    {
> +        .name          = TYPE_MPC_I2C,
> +        .parent        = TYPE_SYS_BUS_DEVICE,
> +        .instance_size = sizeof(MPCI2CState),
> +        .class_init    = mpc_i2c_class_init,
> +    },
> };
>
> -static void mpc_i2c_register_types(void)
> -{
> -    type_register_static(&mpc_i2c_type_info);
> -}
> -
> -type_init(mpc_i2c_register_types)
> +DEFINE_TYPES(types)

What's the advantage of this when we have a single device? For these 
devices this looks like just code churn to me.

Regards,
BALATON Zoltan
Re: [PATCH 11/23] hw/i2c/mpc_i2c: Prefer DEFINE_TYPES() macro
Posted by Bernhard Beschow 4 weeks, 1 day ago 

Am 23. September 2024 10:49:53 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Mon, 23 Sep 2024, Bernhard Beschow wrote:
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/i2c/mpc_i2c.c | 20 ++++++++------------
>> 1 file changed, 8 insertions(+), 12 deletions(-)
>> 
>> diff --git a/hw/i2c/mpc_i2c.c b/hw/i2c/mpc_i2c.c
>> index 3d79c15653..16f4309ea9 100644
>> --- a/hw/i2c/mpc_i2c.c
>> +++ b/hw/i2c/mpc_i2c.c
>> @@ -20,7 +20,6 @@
>> #include "qemu/osdep.h"
>> #include "hw/i2c/i2c.h"
>> #include "hw/irq.h"
>> -#include "qemu/module.h"
>> #include "hw/sysbus.h"
>> #include "migration/vmstate.h"
>> #include "qom/object.h"
>> @@ -345,16 +344,13 @@ static void mpc_i2c_class_init(ObjectClass *klass, void *data)
>>     dc->desc = "MPC I2C Controller";
>> }
>> 
>> -static const TypeInfo mpc_i2c_type_info = {
>> -    .name          = TYPE_MPC_I2C,
>> -    .parent        = TYPE_SYS_BUS_DEVICE,
>> -    .instance_size = sizeof(MPCI2CState),
>> -    .class_init    = mpc_i2c_class_init,
>> +static const TypeInfo types[] = {
>> +    {
>> +        .name          = TYPE_MPC_I2C,
>> +        .parent        = TYPE_SYS_BUS_DEVICE,
>> +        .instance_size = sizeof(MPCI2CState),
>> +        .class_init    = mpc_i2c_class_init,
>> +    },
>> };
>> 
>> -static void mpc_i2c_register_types(void)
>> -{
>> -    type_register_static(&mpc_i2c_type_info);
>> -}
>> -
>> -type_init(mpc_i2c_register_types)
>> +DEFINE_TYPES(types)
>
>What's the advantage of this when we have a single device? For these devices this looks like just code churn to me.

It is still shorter and also more modern style. As a nice side effect it also helps in my experimental branch (which may never ship).

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
Re: [PATCH 11/23] hw/i2c/mpc_i2c: Prefer DEFINE_TYPES() macro
Posted by BALATON Zoltan 4 weeks ago 
On Mon, 23 Sep 2024, Bernhard Beschow wrote:
> Am 23. September 2024 10:49:53 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Mon, 23 Sep 2024, Bernhard Beschow wrote:
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>> hw/i2c/mpc_i2c.c | 20 ++++++++------------
>>> 1 file changed, 8 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/hw/i2c/mpc_i2c.c b/hw/i2c/mpc_i2c.c
>>> index 3d79c15653..16f4309ea9 100644
>>> --- a/hw/i2c/mpc_i2c.c
>>> +++ b/hw/i2c/mpc_i2c.c
>>> @@ -20,7 +20,6 @@
>>> #include "qemu/osdep.h"
>>> #include "hw/i2c/i2c.h"
>>> #include "hw/irq.h"
>>> -#include "qemu/module.h"
>>> #include "hw/sysbus.h"
>>> #include "migration/vmstate.h"
>>> #include "qom/object.h"
>>> @@ -345,16 +344,13 @@ static void mpc_i2c_class_init(ObjectClass *klass, void *data)
>>>     dc->desc = "MPC I2C Controller";
>>> }
>>>
>>> -static const TypeInfo mpc_i2c_type_info = {
>>> -    .name          = TYPE_MPC_I2C,
>>> -    .parent        = TYPE_SYS_BUS_DEVICE,
>>> -    .instance_size = sizeof(MPCI2CState),
>>> -    .class_init    = mpc_i2c_class_init,
>>> +static const TypeInfo types[] = {
>>> +    {
>>> +        .name          = TYPE_MPC_I2C,
>>> +        .parent        = TYPE_SYS_BUS_DEVICE,
>>> +        .instance_size = sizeof(MPCI2CState),
>>> +        .class_init    = mpc_i2c_class_init,
>>> +    },
>>> };
>>>
>>> -static void mpc_i2c_register_types(void)
>>> -{
>>> -    type_register_static(&mpc_i2c_type_info);
>>> -}
>>> -
>>> -type_init(mpc_i2c_register_types)
>>> +DEFINE_TYPES(types)
>>
>> What's the advantage of this when we have a single device? For these devices this looks like just code churn to me.
>
> It is still shorter and also more modern style. As a nice side effect it also helps in my experimental branch (which may never ship).

I don't mind changing this but I see no real advantage either. It removes 
a one line function but adds a one element array instead which is about 
the same level of boilerplate and not less confusing for new people so it 
does not seem to help much.

Regards,
BALATON Zoltan

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.