> -----Original Message-----
> From: Nabih Estefan <[email protected]>
> Sent: Thursday, January 15, 2026 7:12 AM
> To: Kane Chen <[email protected]>
> Cc: Cédric Le Goater <[email protected]>; Peter Maydell
> <[email protected]>; Steven Lee <[email protected]>; Troy
> Lee <[email protected]>; Jamin Lin <[email protected]>; Andrew
> Jeffery <[email protected]>; Joel Stanley <[email protected]>;
> open list:ASPEED BMCs <[email protected]>; open list:All patches CC
> here <[email protected]>; Troy Lee <[email protected]>
> Subject: Re: [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize
> bus naming
>
> 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]>
>
> Signed-off-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
>
Hi Nabih,
Thanks for your review and feedback.
Best Regards,
Kane
> > ---
> > 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
> >
> >
