Re: [PATCH v2 7/7] hw/arm: Add ID for NPCM7XX SMBus

[Philippe Mathieu-Daudé]

+Markus/Eduardo

On 11/1/21 18:33, Peter Maydell wrote:
> On Thu, 21 Oct 2021 at 19:40, Hao Wu <wuhaotsh@google.com> wrote:
>>
>> The ID can be used to indicate SMBus modules when adding
>> dynamic devices to them.
>>
>> Signed-off-by: Hao Wu <wuhaotsh@google.com>
>> ---
>>  hw/arm/npcm7xx.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
>> index 2ab0080e0b..72953d65ef 100644
>> --- a/hw/arm/npcm7xx.c
>> +++ b/hw/arm/npcm7xx.c
>> @@ -421,6 +421,7 @@ static void npcm7xx_init(Object *obj)
>>      for (i = 0; i < ARRAY_SIZE(s->smbus); i++) {
>>          object_initialize_child(obj, "smbus[*]", &s->smbus[i],
>>                                  TYPE_NPCM7XX_SMBUS);
>> +        DEVICE(&s->smbus[i])->id = g_strdup_printf("smbus[%d]", i);
>>      }
> 
> This one looks weird to me -- I'm pretty sure we shouldn't be messing
> about with the DeviceState id string like that. It's supposed to be
> internal to the QOM/qdev code.

I agree with you, however:

$ git grep -F -- '->id = g_strdup' hw
hw/arm/virt.c:1512:    dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE);
hw/block/xen-block.c:761:    drive->id = g_strdup(id);
hw/block/xen-block.c:853:    iothread->id = g_strdup(id);
hw/core/machine-qmp-cmds.c:169:        m->id =
g_strdup(object_get_canonical_path_component(obj));
hw/mem/pc-dimm.c:244:        di->id = g_strdup(dev->id);
hw/pci-bridge/pci_expander_bridge.c:248:        bds->id =
g_strdup(dev_name);
hw/ppc/e500.c:1009:        dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE);
hw/s390x/s390-pci-bus.c:1003:            dev->id =
g_strdup_printf("auto_%02x:%02x.%01x",
hw/sh4/sh7750.c:819:    dev->id = g_strdup("sci");
hw/sh4/sh7750.c:836:    dev->id = g_strdup("scif");
hw/virtio/virtio-mem-pci.c:69:        vi->id = g_strdup(dev->id);
hw/virtio/virtio-pmem-pci.c:74:        vi->id = g_strdup(dev->id);