On Wed, Jan 14, 2026 at 11:18 PM Cédric Le Goater <[email protected]> wrote: > > Hello Nabih, > > On 1/15/26 00:11, Nabih Estefan wrote: > > On Mon, Jan 12, 2026 at 12:38 AM Kane Chen via qemu development > > <[email protected]> wrote: > >> > >> From: Kane-Chen-AS <[email protected]> > >> > >> On some Aspeed-based machines, multiple I2C controllers may exist > >> across different components, such as the primary SoC and an external > >> IO expander or co-processor (e.g., AST1700). Using the current static > >> naming convention results in object name conflicts when multiple > >> controllers attempt to instantiate buses with the same ID. > >> > >> This patch introduces a 'bus-label' property for the Aspeed I2C > >> controller. This allows higher-level layers, such as the SoC realize > >> function, to provide a unique identifier for the buses. The I2C bus > >> object name is then constructed using this label (e.g., "ioexp0.0" > >> instead of the default "aspeed.i2c.bus.0"). > >> > >> This enhancement ensures unique bus identifiers across the system and > >> resolves naming conflicts in multi-controller configurations. > >> > >> Signed-off-by: Kane-Chen-AS <[email protected]> > > > > > Did you mean 'Reviewed-by' ? > > Thanks, > > C. >
Yes, taht is absolutely what I meant. sorry about that. Reviewed-by: Nabih Estefan <[email protected]> > > > Tested-by: Nabih Estefan <[email protected]> > > > > This is basically a rework of the bus-label patches I already tested > > and we're carrying for AST1700, re-tested just in case, and everything > > still looks good! > > > > Thank you Kane! > > > > -Nabih > > > >> --- > >> include/hw/i2c/aspeed_i2c.h | 2 ++ > >> hw/i2c/aspeed_i2c.c | 27 +++++++++++++++++++++------ > >> 2 files changed, 23 insertions(+), 6 deletions(-) > >> > >> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h > >> index ffcff2580f..68bd138026 100644 > >> --- a/include/hw/i2c/aspeed_i2c.h > >> +++ b/include/hw/i2c/aspeed_i2c.h > >> @@ -252,6 +252,7 @@ struct AspeedI2CBus { > >> MemoryRegion mr_pool; > >> > >> I2CBus *bus; > >> + char *name; > >> uint8_t id; > >> qemu_irq irq; > >> > >> @@ -269,6 +270,7 @@ struct AspeedI2CState { > >> uint32_t intr_status; > >> uint32_t ctrl_global; > >> uint32_t new_clk_divider; > >> + char *bus_label; > >> MemoryRegion pool_iomem; > >> uint8_t share_pool[ASPEED_I2C_SHARE_POOL_SIZE]; > >> > >> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c > >> index 1b8ac561c3..7cf92423c7 100644 > >> --- a/hw/i2c/aspeed_i2c.c > >> +++ b/hw/i2c/aspeed_i2c.c > >> @@ -1215,9 +1215,16 @@ static void aspeed_i2c_realize(DeviceState *dev, > >> Error **errp) > >> "aspeed.i2c", aic->mem_size); > >> sysbus_init_mmio(sbd, &s->iomem); > >> > >> + /* default value */ > >> + if (!s->bus_label) { > >> + s->bus_label = g_strdup(TYPE_ASPEED_I2C_BUS); > >> + } > >> + > >> for (i = 0; i < aic->num_busses; i++) { > >> Object *bus = OBJECT(&s->busses[i]); > >> int offset = i < aic->gap ? 1 : 5; > >> + g_autofree char *name = g_strdup_printf("%s.%d", > >> + s->bus_label, i); > >> > >> if (!object_property_set_link(bus, "controller", OBJECT(s), > >> errp)) { > >> return; > >> @@ -1227,6 +1234,10 @@ static void aspeed_i2c_realize(DeviceState *dev, > >> Error **errp) > >> return; > >> } > >> > >> + if (!object_property_set_str(bus, "bus-name", name, errp)) { > >> + return; > >> + } > >> + > >> if (!sysbus_realize(SYS_BUS_DEVICE(bus), errp)) { > >> return; > >> } > >> @@ -1263,6 +1274,7 @@ static void aspeed_i2c_realize(DeviceState *dev, > >> Error **errp) > >> static const Property aspeed_i2c_properties[] = { > >> DEFINE_PROP_LINK("dram", AspeedI2CState, dram_mr, > >> TYPE_MEMORY_REGION, MemoryRegion *), > >> + DEFINE_PROP_STRING("bus-label", AspeedI2CState, bus_label), > >> }; > >> > >> static void aspeed_i2c_class_init(ObjectClass *klass, const void *data) > >> @@ -1423,24 +1435,26 @@ static void aspeed_i2c_bus_realize(DeviceState > >> *dev, Error **errp) > >> { > >> AspeedI2CBus *s = ASPEED_I2C_BUS(dev); > >> AspeedI2CClass *aic; > >> - g_autofree char *name = g_strdup_printf(TYPE_ASPEED_I2C_BUS ".%d", > >> s->id); > >> - g_autofree char *pool_name = g_strdup_printf("%s.pool", name); > >> + g_autofree char *pool_name = NULL; > >> > >> - if (!s->controller) { > >> - error_setg(errp, TYPE_ASPEED_I2C_BUS ": 'controller' link not > >> set"); > >> + if (!s->controller || !s->name) { > >> + error_setg(errp, TYPE_ASPEED_I2C_BUS > >> + ": 'controller' or 'bus-name' not set"); > >> return; > >> } > >> > >> + pool_name = g_strdup_printf("%s.pool", s->name); > >> + > >> aic = ASPEED_I2C_GET_CLASS(s->controller); > >> > >> sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); > >> > >> - s->bus = i2c_init_bus(dev, name); > >> + s->bus = i2c_init_bus(dev, s->name); > >> s->slave = i2c_slave_create_simple(s->bus, TYPE_ASPEED_I2C_BUS_SLAVE, > >> 0xff); > >> > >> memory_region_init_io(&s->mr, OBJECT(s), &aspeed_i2c_bus_ops, > >> - s, name, aic->reg_size); > >> + s, s->name, aic->reg_size); > >> sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mr); > >> > >> memory_region_init_io(&s->mr_pool, OBJECT(s), > >> &aspeed_i2c_bus_pool_ops, > >> @@ -1452,6 +1466,7 @@ static const Property aspeed_i2c_bus_properties[] = { > >> DEFINE_PROP_UINT8("bus-id", AspeedI2CBus, id, 0), > >> DEFINE_PROP_LINK("controller", AspeedI2CBus, controller, > >> TYPE_ASPEED_I2C, > >> AspeedI2CState *), > >> + DEFINE_PROP_STRING("bus-name", AspeedI2CBus, name), > >> }; > >> > >> static void aspeed_i2c_bus_class_init(ObjectClass *klass, const void > >> *data) > >> -- > >> 2.43.0 > >> > >> >
