[PATCH v3 22/26] hw/i2c/smbus_eeprom: Prefer DEFINE_TYPES() macro

Bernhard Beschow posted 26 patches 3 weeks ago
There is a newer version of this series
[PATCH v3 22/26] hw/i2c/smbus_eeprom: Prefer DEFINE_TYPES() macro
Posted by Bernhard Beschow 3 weeks ago
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i2c/smbus_eeprom.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
index 9e62c27a1a..1d4d9704bf 100644
--- a/hw/i2c/smbus_eeprom.c
+++ b/hw/i2c/smbus_eeprom.c
@@ -151,19 +151,16 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)
     dc->user_creatable = false;
 }
 
-static const TypeInfo smbus_eeprom_info = {
-    .name          = TYPE_SMBUS_EEPROM,
-    .parent        = TYPE_SMBUS_DEVICE,
-    .instance_size = sizeof(SMBusEEPROMDevice),
-    .class_init    = smbus_eeprom_class_initfn,
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_SMBUS_EEPROM,
+        .parent        = TYPE_SMBUS_DEVICE,
+        .instance_size = sizeof(SMBusEEPROMDevice),
+        .class_init    = smbus_eeprom_class_initfn,
+    },
 };
 
-static void smbus_eeprom_register_types(void)
-{
-    type_register_static(&smbus_eeprom_info);
-}
-
-type_init(smbus_eeprom_register_types)
+DEFINE_TYPES(types)
 
 void smbus_eeprom_init_one(I2CBus *smbus, uint8_t address, uint8_t *eeprom_buf)
 {
-- 
2.47.0


Re: [PATCH v3 22/26] hw/i2c/smbus_eeprom: Prefer DEFINE_TYPES() macro
Posted by Corey Minyard 3 weeks ago
On Sat, Nov 2, 2024 at 8:25 AM Bernhard Beschow <shentey@gmail.com> wrote:
>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/i2c/smbus_eeprom.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
> index 9e62c27a1a..1d4d9704bf 100644
> --- a/hw/i2c/smbus_eeprom.c
> +++ b/hw/i2c/smbus_eeprom.c
> @@ -151,19 +151,16 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)
>      dc->user_creatable = false;
>  }
>
> -static const TypeInfo smbus_eeprom_info = {
> -    .name          = TYPE_SMBUS_EEPROM,
> -    .parent        = TYPE_SMBUS_DEVICE,
> -    .instance_size = sizeof(SMBusEEPROMDevice),
> -    .class_init    = smbus_eeprom_class_initfn,
> +static const TypeInfo types[] = {

This is better, but why did you change the name to "types".  The
previous name was fairly descriptive, though you might change "info"
to "types".

-corey

> +    {
> +        .name          = TYPE_SMBUS_EEPROM,
> +        .parent        = TYPE_SMBUS_DEVICE,
> +        .instance_size = sizeof(SMBusEEPROMDevice),
> +        .class_init    = smbus_eeprom_class_initfn,
> +    },
>  };
>
> -static void smbus_eeprom_register_types(void)
> -{
> -    type_register_static(&smbus_eeprom_info);
> -}
> -
> -type_init(smbus_eeprom_register_types)
> +DEFINE_TYPES(types)
>
>  void smbus_eeprom_init_one(I2CBus *smbus, uint8_t address, uint8_t *eeprom_buf)
>  {
> --
> 2.47.0
>
>
Re: [PATCH v3 22/26] hw/i2c/smbus_eeprom: Prefer DEFINE_TYPES() macro
Posted by Bernhard Beschow 2 weeks, 6 days ago

Am 2. November 2024 17:24:25 UTC schrieb Corey Minyard <corey@minyard.net>:
>On Sat, Nov 2, 2024 at 8:25 AM Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>  hw/i2c/smbus_eeprom.c | 19 ++++++++-----------
>>  1 file changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
>> index 9e62c27a1a..1d4d9704bf 100644
>> --- a/hw/i2c/smbus_eeprom.c
>> +++ b/hw/i2c/smbus_eeprom.c
>> @@ -151,19 +151,16 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)
>>      dc->user_creatable = false;
>>  }
>>
>> -static const TypeInfo smbus_eeprom_info = {
>> -    .name          = TYPE_SMBUS_EEPROM,
>> -    .parent        = TYPE_SMBUS_DEVICE,
>> -    .instance_size = sizeof(SMBusEEPROMDevice),
>> -    .class_init    = smbus_eeprom_class_initfn,
>> +static const TypeInfo types[] = {
>
>This is better, but why did you change the name to "types".  The
>previous name was fairly descriptive, though you might change "info"
>to "types".

I took inspiration from https://lore.kernel.org/qemu-devel/20240215175752.82828-20-philmd@linaro.org . I could preserve the old names (also in the other patches) by simply converting to plural form. Here it would be: smbus_eeprom_infos. OK?

Best regards,
Bernhard

>
>-corey
>
>> +    {
>> +        .name          = TYPE_SMBUS_EEPROM,
>> +        .parent        = TYPE_SMBUS_DEVICE,
>> +        .instance_size = sizeof(SMBusEEPROMDevice),
>> +        .class_init    = smbus_eeprom_class_initfn,
>> +    },
>>  };
>>
>> -static void smbus_eeprom_register_types(void)
>> -{
>> -    type_register_static(&smbus_eeprom_info);
>> -}
>> -
>> -type_init(smbus_eeprom_register_types)
>> +DEFINE_TYPES(types)
>>
>>  void smbus_eeprom_init_one(I2CBus *smbus, uint8_t address, uint8_t *eeprom_buf)
>>  {
>> --
>> 2.47.0
>>
>>
Re: [PATCH v3 22/26] hw/i2c/smbus_eeprom: Prefer DEFINE_TYPES() macro
Posted by Bernhard Beschow 2 weeks, 6 days ago

Am 3. November 2024 07:51:46 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 2. November 2024 17:24:25 UTC schrieb Corey Minyard <corey@minyard.net>:
>>On Sat, Nov 2, 2024 at 8:25 AM Bernhard Beschow <shentey@gmail.com> wrote:
>>>
>>> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>>  hw/i2c/smbus_eeprom.c | 19 ++++++++-----------
>>>  1 file changed, 8 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
>>> index 9e62c27a1a..1d4d9704bf 100644
>>> --- a/hw/i2c/smbus_eeprom.c
>>> +++ b/hw/i2c/smbus_eeprom.c
>>> @@ -151,19 +151,16 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)
>>>      dc->user_creatable = false;
>>>  }
>>>
>>> -static const TypeInfo smbus_eeprom_info = {
>>> -    .name          = TYPE_SMBUS_EEPROM,
>>> -    .parent        = TYPE_SMBUS_DEVICE,
>>> -    .instance_size = sizeof(SMBusEEPROMDevice),
>>> -    .class_init    = smbus_eeprom_class_initfn,
>>> +static const TypeInfo types[] = {
>>
>>This is better, but why did you change the name to "types".  The
>>previous name was fairly descriptive, though you might change "info"
>>to "types".
>
>I took inspiration from https://lore.kernel.org/qemu-devel/20240215175752.82828-20-philmd@linaro.org . I could preserve the old names (also in the other patches) by simply converting to plural form. Here it would be: smbus_eeprom_infos. OK?

Well, the plural form of " info" is also "info". So I'll keep the names in the patches as they are in master, except when multiple types are defined where I'll draw inspiration from the file names.

Best regards,
Bernhard

>
>Best regards,
>Bernhard
>
>>
>>-corey
>>
>>> +    {
>>> +        .name          = TYPE_SMBUS_EEPROM,
>>> +        .parent        = TYPE_SMBUS_DEVICE,
>>> +        .instance_size = sizeof(SMBusEEPROMDevice),
>>> +        .class_init    = smbus_eeprom_class_initfn,
>>> +    },
>>>  };
>>>
>>> -static void smbus_eeprom_register_types(void)
>>> -{
>>> -    type_register_static(&smbus_eeprom_info);
>>> -}
>>> -
>>> -type_init(smbus_eeprom_register_types)
>>> +DEFINE_TYPES(types)
>>>
>>>  void smbus_eeprom_init_one(I2CBus *smbus, uint8_t address, uint8_t *eeprom_buf)
>>>  {
>>> --
>>> 2.47.0
>>>
>>>
Re: [PATCH v3 22/26] hw/i2c/smbus_eeprom: Prefer DEFINE_TYPES() macro
Posted by Bernhard Beschow 2 weeks, 6 days ago

Am 3. November 2024 11:52:40 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>
>
>Am 3. November 2024 07:51:46 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>>
>>
>>Am 2. November 2024 17:24:25 UTC schrieb Corey Minyard <corey@minyard.net>:
>>>On Sat, Nov 2, 2024 at 8:25 AM Bernhard Beschow <shentey@gmail.com> wrote:
>>>>
>>>> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> ---
>>>>  hw/i2c/smbus_eeprom.c | 19 ++++++++-----------
>>>>  1 file changed, 8 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
>>>> index 9e62c27a1a..1d4d9704bf 100644
>>>> --- a/hw/i2c/smbus_eeprom.c
>>>> +++ b/hw/i2c/smbus_eeprom.c
>>>> @@ -151,19 +151,16 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)
>>>>      dc->user_creatable = false;
>>>>  }
>>>>
>>>> -static const TypeInfo smbus_eeprom_info = {
>>>> -    .name          = TYPE_SMBUS_EEPROM,
>>>> -    .parent        = TYPE_SMBUS_DEVICE,
>>>> -    .instance_size = sizeof(SMBusEEPROMDevice),
>>>> -    .class_init    = smbus_eeprom_class_initfn,
>>>> +static const TypeInfo types[] = {
>>>
>>>This is better, but why did you change the name to "types".  The
>>>previous name was fairly descriptive, though you might change "info"
>>>to "types".
>>
>>I took inspiration from https://lore.kernel.org/qemu-devel/20240215175752.82828-20-philmd@linaro.org . I could preserve the old names (also in the other patches) by simply converting to plural form. Here it would be: smbus_eeprom_infos. OK?
>
>Well, the plural form of " info" is also "info". So I'll keep the names in the patches as they are in master, except when multiple types are defined where I'll draw inspiration from the file names.

Checking other usages of DEFINE_TYPES(), the majority by far uses a "types" suffix while qom.rst suggests "info". Still, 2nd place is "infos" suffix. I'll go with "types" suffix then which makes hcd-ehci-sysbus consistent with hcd-ohci-sysbus.

Best regards,
Bernhard

>
>Best regards,
>Bernhard
>
>>
>>Best regards,
>>Bernhard
>>
>>>
>>>-corey
>>>
>>>> +    {
>>>> +        .name          = TYPE_SMBUS_EEPROM,
>>>> +        .parent        = TYPE_SMBUS_DEVICE,
>>>> +        .instance_size = sizeof(SMBusEEPROMDevice),
>>>> +        .class_init    = smbus_eeprom_class_initfn,
>>>> +    },
>>>>  };
>>>>
>>>> -static void smbus_eeprom_register_types(void)
>>>> -{
>>>> -    type_register_static(&smbus_eeprom_info);
>>>> -}
>>>> -
>>>> -type_init(smbus_eeprom_register_types)
>>>> +DEFINE_TYPES(types)
>>>>
>>>>  void smbus_eeprom_init_one(I2CBus *smbus, uint8_t address, uint8_t *eeprom_buf)
>>>>  {
>>>> --
>>>> 2.47.0
>>>>
>>>>