diff mbox series

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

Message ID 20241102131715.548849-23-shentey@gmail.com (mailing list archive)
State New
Headers show
Series E500 Cleanup | expand

Commit Message

Bernhard Beschow Nov. 2, 2024, 1:17 p.m. UTC 
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(-)

Comments

Corey Minyard Nov. 2, 2024, 5:24 p.m. UTC | #1 
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
>
>
Bernhard Beschow Nov. 3, 2024, 7:51 a.m. UTC | #2 
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
>>
>>
Bernhard Beschow Nov. 3, 2024, 11:52 a.m. UTC | #3 
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
>>>
>>>
Bernhard Beschow Nov. 3, 2024, 1:06 p.m. UTC | #4 
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
>>>>
>>>>
diff mbox series

Patch

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)
 {