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

[Bernhard Beschow]

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