All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
@ 2026-01-12  8:30 ` Kane Chen via qemu development
  0 siblings, 0 replies; 21+ messages in thread
From: Kane Chen via @ 2026-01-12  8:30 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee, Kane-Chen-AS

From: Kane-Chen-AS <kane_chen@aspeedtech.com>

Currently, the Aspeed I2C controller uses a static naming convention
for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
object name conflicts in machine models that incorporate multiple I2C
controllers, such as an Aspeed SoC paired with an external IO expander
or a co-processor like the AST1700.

This patch series introduces a 'bus-label' property to the Aspeed I2C
controller. By setting this property, higher-level platform code
(e.g., the SoC realize function) can provide a unique prefix for the
I2C buses.

Changes in v1:

Added bus-label property to AspeedI2CState.
Added bus-name property to AspeedI2CBus.
Modified aspeed_i2c_realize to generate bus names based on the label.
Updated aspeed_i2c_bus_realize to use the customized name for bus
initialization and memory region naming.

Example usage: If a controller's bus-label is set to "ioexp0", its buses
will be named "ioexp0.0", "ioexp0.1", etc., instead of the default
"aspeed.i2c.bus.0".

Any feedback or suggestions are appreciated.

Best Regards,
Kane

Kane-Chen-AS (1):
  hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming

 include/hw/i2c/aspeed_i2c.h |  2 ++
 hw/i2c/aspeed_i2c.c         | 27 +++++++++++++++++++++------
 2 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
@ 2026-01-12  8:30 ` Kane Chen via qemu development
  0 siblings, 0 replies; 21+ messages in thread
From: Kane Chen via qemu development @ 2026-01-12  8:30 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee, Kane-Chen-AS

From: Kane-Chen-AS <kane_chen@aspeedtech.com>

Currently, the Aspeed I2C controller uses a static naming convention
for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
object name conflicts in machine models that incorporate multiple I2C
controllers, such as an Aspeed SoC paired with an external IO expander
or a co-processor like the AST1700.

This patch series introduces a 'bus-label' property to the Aspeed I2C
controller. By setting this property, higher-level platform code
(e.g., the SoC realize function) can provide a unique prefix for the
I2C buses.

Changes in v1:

Added bus-label property to AspeedI2CState.
Added bus-name property to AspeedI2CBus.
Modified aspeed_i2c_realize to generate bus names based on the label.
Updated aspeed_i2c_bus_realize to use the customized name for bus
initialization and memory region naming.

Example usage: If a controller's bus-label is set to "ioexp0", its buses
will be named "ioexp0.0", "ioexp0.1", etc., instead of the default
"aspeed.i2c.bus.0".

Any feedback or suggestions are appreciated.

Best Regards,
Kane

Kane-Chen-AS (1):
  hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming

 include/hw/i2c/aspeed_i2c.h |  2 ++
 hw/i2c/aspeed_i2c.c         | 27 +++++++++++++++++++++------
 2 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-12  8:30 ` Kane Chen via qemu development
@ 2026-01-12  8:30   ` Kane Chen via qemu development
  -1 siblings, 0 replies; 21+ messages in thread
From: Kane Chen via @ 2026-01-12  8:30 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee, Kane-Chen-AS

From: Kane-Chen-AS <kane_chen@aspeedtech.com>

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 <kane_chen@aspeedtech.com>
---
 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



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
@ 2026-01-12  8:30   ` Kane Chen via qemu development
  0 siblings, 0 replies; 21+ messages in thread
From: Kane Chen via qemu development @ 2026-01-12  8:30 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee, Kane-Chen-AS

From: Kane-Chen-AS <kane_chen@aspeedtech.com>

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 <kane_chen@aspeedtech.com>
---
 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



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-12  8:30   ` Kane Chen via qemu development
  (?)
@ 2026-01-14 23:11   ` Nabih Estefan
  2026-01-15  2:55     ` Kane Chen
  2026-01-15  7:18     ` Cédric Le Goater
  -1 siblings, 2 replies; 21+ messages in thread
From: Nabih Estefan @ 2026-01-14 23:11 UTC (permalink / raw)
  To: Kane Chen
  Cc: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here, troy_lee

On Mon, Jan 12, 2026 at 12:38 AM Kane Chen via qemu development
<qemu-devel@nongnu.org> wrote:
>
> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>
> 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 <kane_chen@aspeedtech.com>

Signed-off-by: Nabih Estefan <nabihestefan@google.com>
Tested-by: Nabih Estefan <nabihestefan@google.com>

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-14 23:11   ` Nabih Estefan
@ 2026-01-15  2:55     ` Kane Chen
  2026-01-15  7:18     ` Cédric Le Goater
  1 sibling, 0 replies; 21+ messages in thread
From: Kane Chen @ 2026-01-15  2:55 UTC (permalink / raw)
  To: Nabih Estefan
  Cc: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here, Troy Lee

> -----Original Message-----
> From: Nabih Estefan <nabihestefan@google.com>
> Sent: Thursday, January 15, 2026 7:12 AM
> To: Kane Chen <kane_chen@aspeedtech.com>
> Cc: Cédric Le Goater <clg@kaod.org>; Peter Maydell
> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
> Lee <leetroy@gmail.com>; Jamin Lin <jamin_lin@aspeedtech.com>; Andrew
> Jeffery <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>;
> open list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC
> here <qemu-devel@nongnu.org>; Troy Lee <troy_lee@aspeedtech.com>
> 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
> <qemu-devel@nongnu.org> wrote:
> >
> > From: Kane-Chen-AS <kane_chen@aspeedtech.com>
> >
> > 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 <kane_chen@aspeedtech.com>
> 
> Signed-off-by: Nabih Estefan <nabihestefan@google.com>
> Tested-by: Nabih Estefan <nabihestefan@google.com>
> 
> 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
> >
> >

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-12  8:30   ` Kane Chen via qemu development
  (?)
  (?)
@ 2026-01-15  7:15   ` Cédric Le Goater
  2026-01-15  7:19     ` Kane Chen
  -1 siblings, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2026-01-15  7:15 UTC (permalink / raw)
  To: Kane Chen, Peter Maydell, Steven Lee, Troy Lee, Jamin Lin,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: troy_lee

On 1/12/26 09:30, Kane Chen via wrote:
> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
> 
> 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 <kane_chen@aspeedtech.com>

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

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



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-14 23:11   ` Nabih Estefan
  2026-01-15  2:55     ` Kane Chen
@ 2026-01-15  7:18     ` Cédric Le Goater
  2026-01-15 16:37       ` Nabih Estefan
  1 sibling, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2026-01-15  7:18 UTC (permalink / raw)
  To: Nabih Estefan, Kane Chen
  Cc: Peter Maydell, Steven Lee, Troy Lee, Jamin Lin, Andrew Jeffery,
	Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here, troy_lee

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
> <qemu-devel@nongnu.org> wrote:
>>
>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>
>> 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 <kane_chen@aspeedtech.com>
> 
> Signed-off-by: Nabih Estefan <nabihestefan@google.com>


Did you mean 'Reviewed-by' ?

Thanks,

C.


> Tested-by: Nabih Estefan <nabihestefan@google.com>
> 
> 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
>>
>>



^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-15  7:15   ` Cédric Le Goater
@ 2026-01-15  7:19     ` Kane Chen
  2026-01-15 17:19       ` Cédric Le Goater
  0 siblings, 1 reply; 21+ messages in thread
From: Kane Chen @ 2026-01-15  7:19 UTC (permalink / raw)
  To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
	Jamin Lin, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: Troy Lee

> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Thursday, January 15, 2026 3:15 PM
> To: Kane Chen <kane_chen@aspeedtech.com>; Peter Maydell
> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
> Lee <leetroy@gmail.com>; Jamin Lin <jamin_lin@aspeedtech.com>; Andrew
> Jeffery <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>;
> open list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC
> here <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_lee@aspeedtech.com>
> Subject: Re: [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize
> bus naming
> 
> On 1/12/26 09:30, Kane Chen via wrote:
> > From: Kane-Chen-AS <kane_chen@aspeedtech.com>
> >
> > 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 <kane_chen@aspeedtech.com>
> 
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> 
> Thanks,
> 
> C.
Hi Cédric,

Thanks for your review.

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)



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-15  7:18     ` Cédric Le Goater
@ 2026-01-15 16:37       ` Nabih Estefan
  0 siblings, 0 replies; 21+ messages in thread
From: Nabih Estefan @ 2026-01-15 16:37 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Kane Chen, Peter Maydell, Steven Lee, Troy Lee, Jamin Lin,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here, troy_lee

On Wed, Jan 14, 2026 at 11:18 PM Cédric Le Goater <clg@kaod.org> 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
> > <qemu-devel@nongnu.org> wrote:
> >>
> >> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
> >>
> >> 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 <kane_chen@aspeedtech.com>
> >
>
>
> Did you mean 'Reviewed-by' ?
>
> Thanks,
>
> C.
>

Yes, taht is absolutely what I meant. sorry about that.

Reviewed-by: Nabih Estefan <nabihestefan@google.com>

>
> > Tested-by: Nabih Estefan <nabihestefan@google.com>
> >
> > 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
> >>
> >>
>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-15  7:19     ` Kane Chen
@ 2026-01-15 17:19       ` Cédric Le Goater
  0 siblings, 0 replies; 21+ messages in thread
From: Cédric Le Goater @ 2026-01-15 17:19 UTC (permalink / raw)
  To: Kane Chen, Peter Maydell, Steven Lee, Troy Lee, Jamin Lin,
	Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
	open list:All patches CC here
  Cc: Troy Lee

On 1/15/26 08:19, Kane Chen wrote:
>> -----Original Message-----
>> From: Cédric Le Goater <clg@kaod.org>
>> Sent: Thursday, January 15, 2026 3:15 PM
>> To: Kane Chen <kane_chen@aspeedtech.com>; Peter Maydell
>> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
>> Lee <leetroy@gmail.com>; Jamin Lin <jamin_lin@aspeedtech.com>; Andrew
>> Jeffery <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>;
>> open list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC
>> here <qemu-devel@nongnu.org>
>> Cc: Troy Lee <troy_lee@aspeedtech.com>
>> Subject: Re: [PATCH v1 1/1] hw/i2c/aspeed: Introduce 'bus-label' to customize
>> bus naming
>>
>> On 1/12/26 09:30, Kane Chen via wrote:
>>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>>
>>> 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 <kane_chen@aspeedtech.com>
>>
>> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>>
>> Thanks,
>>
>> C.
> Hi Cédric,
> 
> Thanks for your review.

Applied to aspeed-next.

Thanks,

C.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-12  8:30 ` Kane Chen via qemu development
  (?)
  (?)
@ 2026-01-15 19:47 ` Philippe Mathieu-Daudé
  2026-01-16  5:16   ` Cédric Le Goater
  -1 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-01-15 19:47 UTC (permalink / raw)
  To: Kane Chen, Cédric Le Goater, Peter Maydell, Steven Lee,
	Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley,
	open list:ASPEED BMCs, open list:All patches CC here,
	Markus Armbruster
  Cc: troy_lee

Hi,

Cc'ing Markus.

On 12/1/26 09:30, Kane Chen via qemu development wrote:
> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
> 
> Currently, the Aspeed I2C controller uses a static naming convention
> for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
> object name conflicts in machine models that incorporate multiple I2C
> controllers, such as an Aspeed SoC paired with an external IO expander
> or a co-processor like the AST1700.
> 

Is this a side-effect of Problem 4: 'The /machine/unattached/ orphanage'
described here?
https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/

This problem isn't specific to I2C nor Aspeed.

> This patch series introduces a 'bus-label' property to the Aspeed I2C
> controller. By setting this property, higher-level platform code
> (e.g., the SoC realize function) can provide a unique prefix for the
> I2C buses.
> 
> Changes in v1:
> 
> Added bus-label property to AspeedI2CState.
> Added bus-name property to AspeedI2CBus.
> Modified aspeed_i2c_realize to generate bus names based on the label.
> Updated aspeed_i2c_bus_realize to use the customized name for bus
> initialization and memory region naming.
> 
> Example usage: If a controller's bus-label is set to "ioexp0", its buses
> will be named "ioexp0.0", "ioexp0.1", etc., instead of the default
> "aspeed.i2c.bus.0".
> 
> Any feedback or suggestions are appreciated.
> 
> Best Regards,
> Kane
> 
> Kane-Chen-AS (1):
>    hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
> 
>   include/hw/i2c/aspeed_i2c.h |  2 ++
>   hw/i2c/aspeed_i2c.c         | 27 +++++++++++++++++++++------
>   2 files changed, 23 insertions(+), 6 deletions(-)
> 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-15 19:47 ` [PATCH v1 0/1] " Philippe Mathieu-Daudé
@ 2026-01-16  5:16   ` Cédric Le Goater
  2026-01-19 14:25     ` Markus Armbruster
  0 siblings, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2026-01-16  5:16 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Kane Chen, Peter Maydell, Steven Lee,
	Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley,
	open list:ASPEED BMCs, open list:All patches CC here,
	Markus Armbruster
  Cc: troy_lee

On 1/15/26 20:47, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> Cc'ing Markus.
> 
> On 12/1/26 09:30, Kane Chen via qemu development wrote:
>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>
>> Currently, the Aspeed I2C controller uses a static naming convention
>> for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
>> object name conflicts in machine models that incorporate multiple I2C
>> controllers, such as an Aspeed SoC paired with an external IO expander
>> or a co-processor like the AST1700.
>>
> 
> Is this a side-effect of Problem 4: 'The /machine/unattached/ orphanage'
> described here?
> https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/
> 
> This problem isn't specific to I2C nor Aspeed.


See the discussion here :

   https://lore.kernel.org/qemu-devel/006fa26f-6b84-4e82-b6e1-7d1353579441@kaod.org/

The Aspeed SoC has 3*16 I2C buses attached on 3 different I2C
controllers plus the I2C/I3C buses. We need to find a way to
distinguish these groups at the QEMU machine level to be able
to add devices on the right bus when using the command line.

Suggestions welcome !

Thanks,

C.


> 
>> This patch series introduces a 'bus-label' property to the Aspeed I2C
>> controller. By setting this property, higher-level platform code
>> (e.g., the SoC realize function) can provide a unique prefix for the
>> I2C buses.
>>
>> Changes in v1:
>>
>> Added bus-label property to AspeedI2CState.
>> Added bus-name property to AspeedI2CBus.
>> Modified aspeed_i2c_realize to generate bus names based on the label.
>> Updated aspeed_i2c_bus_realize to use the customized name for bus
>> initialization and memory region naming.
>>
>> Example usage: If a controller's bus-label is set to "ioexp0", its buses
>> will be named "ioexp0.0", "ioexp0.1", etc., instead of the default
>> "aspeed.i2c.bus.0".
>>
>> Any feedback or suggestions are appreciated.
>>
>> Best Regards,
>> Kane
>>
>> Kane-Chen-AS (1):
>>    hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
>>
>>   include/hw/i2c/aspeed_i2c.h |  2 ++
>>   hw/i2c/aspeed_i2c.c         | 27 +++++++++++++++++++++------
>>   2 files changed, 23 insertions(+), 6 deletions(-)
>>
> 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-16  5:16   ` Cédric Le Goater
@ 2026-01-19 14:25     ` Markus Armbruster
  2026-01-19 17:38       ` Cédric Le Goater
  0 siblings, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2026-01-19 14:25 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Philippe Mathieu-Daudé, Kane Chen, Peter Maydell, Steven Lee,
	Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley,
	open list:ASPEED BMCs, open list:All patches CC here, troy_lee

Cédric Le Goater <clg@kaod.org> writes:

> On 1/15/26 20:47, Philippe Mathieu-Daudé wrote:
>> Hi,
>> Cc'ing Markus.
>> On 12/1/26 09:30, Kane Chen via qemu development wrote:
>>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>>
>>> Currently, the Aspeed I2C controller uses a static naming convention
>>> for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
>>> object name conflicts in machine models that incorporate multiple I2C
>>> controllers, such as an Aspeed SoC paired with an external IO expander
>>> or a co-processor like the AST1700.
>>>
>> Is this a side-effect of Problem 4: 'The /machine/unattached/ orphanage'
>> described here?
>> https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/
>> This problem isn't specific to I2C nor Aspeed.
>
>
> See the discussion here :
>
>   https://lore.kernel.org/qemu-devel/006fa26f-6b84-4e82-b6e1-7d1353579441@kaod.org/
>
> The Aspeed SoC has 3*16 I2C buses attached on 3 different I2C
> controllers plus the I2C/I3C buses. We need to find a way to
> distinguish these groups at the QEMU machine level to be able
> to add devices on the right bus when using the command line.
>
> Suggestions welcome !

Please show me how to start a QEMU with the 48 I2C mentioned above,
complete with output of "info qtree".



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-19 14:25     ` Markus Armbruster
@ 2026-01-19 17:38       ` Cédric Le Goater
  2026-01-20 10:36         ` Markus Armbruster
  0 siblings, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2026-01-19 17:38 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Philippe Mathieu-Daudé, Kane Chen, Peter Maydell, Steven Lee,
	Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley,
	open list:ASPEED BMCs, open list:All patches CC here, troy_lee

Hello,

On 1/19/26 15:25, Markus Armbruster wrote:
> Cédric Le Goater <clg@kaod.org> writes:
> 
>> On 1/15/26 20:47, Philippe Mathieu-Daudé wrote:
>>> Hi,
>>> Cc'ing Markus.
>>> On 12/1/26 09:30, Kane Chen via qemu development wrote:
>>>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>>>
>>>> Currently, the Aspeed I2C controller uses a static naming convention
>>>> for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
>>>> object name conflicts in machine models that incorporate multiple I2C
>>>> controllers, such as an Aspeed SoC paired with an external IO expander
>>>> or a co-processor like the AST1700.
>>>>
>>> Is this a side-effect of Problem 4: 'The /machine/unattached/ orphanage'
>>> described here?
>>> https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/
>>> This problem isn't specific to I2C nor Aspeed.
>>
>>
>> See the discussion here :
>>
>>    https://lore.kernel.org/qemu-devel/006fa26f-6b84-4e82-b6e1-7d1353579441@kaod.org/
>>
>> The Aspeed SoC has 3*16 I2C buses attached on 3 different I2C
>> controllers plus the I2C/I3C buses. We need to find a way to
>> distinguish these groups at the QEMU machine level to be able
>> to add devices on the right bus when using the command line.
>>
>> Suggestions welcome !
> 
> Please show me how to start a QEMU with the 48 I2C mentioned above,
> complete with output of "info qtree".

Clone

   https://github.com/legoater/qemu/commits/aspeed-11.0

Download

   https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.08/ast2700-default-obmc.tar.gz

And run :

   qemu-system-aarch64 -M ast2700a1-evb -m 8G -smp 4 -net nic,macaddr=,netdev=net0 -netdev user,id=net0,hostfwd=::2207-:22 -drive file=path/to/ast2700-default/image-bmc,format=raw,if=mtd -nographic -serial mon:stdio -snapshot

Today, to attach an I2C device on one of the Aspeed SoC I2C buses :

   -device tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test

The Aspeed SoC I2C bus names follow the "aspeed.i2c.bus.X" format.
This is the model typename. The 2 new IO expander models attached
to the Aspeed SoC have an extra 16 I2C buses each. These buses use
an "ioexpX.Y" name, as proposed in the aspeed-next branch.

Attaching a device to one of the IO expanders I2C buses would be :

   -device tmp105,bus=ioexp0.1,address=0x4d,id=tmp-test

See the qtree below.


Thanks,

C.

(qemu) info qtree
bus: main-system-bus
   type System
   dev: unimplemented-device, id ""
     size = 16777216 (0x1000000)
     name = "aspeed.iomem1"
     mmio ffffffffffffffff/0000000001000000
   dev: unimplemented-device, id ""
     size = 16777216 (0x1000000)
     name = "aspeed.iomem0"
     mmio ffffffffffffffff/0000000001000000
   dev: unimplemented-device, id ""
     size = 16646144 (0xfe0000)
     name = "aspeed.io"
     mmio ffffffffffffffff/0000000000fe0000
   dev: unimplemented-device, id ""
     size = 262144 (0x40000)
     name = "aspeed.dpmcu"
     mmio ffffffffffffffff/0000000000040000
   dev: unimplemented-device, id ""
     size = 65536 (0x10000)
     name = "ioexp-i3c"
     mmio ffffffffffffffff/0000000000010000
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[1]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[1]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[1]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[1]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[1]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[1]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[1]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[1]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[1]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.sgpio-ast2700, id ""
     gpio-out "sysbus-irq" 1
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.sgpio-ast2700, id ""
     gpio-out "sysbus-irq" 1
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.ltpi-ctrl, id ""
     mmio ffffffffffffffff/0000000000000900
   dev: aspeed.pwm, id ""
     gpio-out "sysbus-irq" 1
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 15 (0xf)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.15"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.15
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 14 (0xe)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.14"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.14
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 13 (0xd)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.13"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.13
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 12 (0xc)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.12"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.12
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 11 (0xb)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.11"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.11
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 10 (0xa)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.10"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.10
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 9 (0x9)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.9"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.9
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 8 (0x8)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.8"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.8
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 7 (0x7)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.7"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.7
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 6 (0x6)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.6"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.6
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 5 (0x5)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.5"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.5
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 4 (0x4)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.4"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.4
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 3 (0x3)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.3"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.3
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 2 (0x2)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.2"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.2
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 1 (0x1)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.1"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.1
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 0 (0x0)
     controller = "/machine/soc/ioexp[1]/ioexp-i2c[0]"
     bus-name = "ioexp1.0"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp1.0
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c-ast2700, id ""
     gpio-out "sysbus-irq" 1
     dram = "/machine/soc/ioexp[1]/aspeed.ast1700[0]"
     bus-label = "ioexp1"
     mmio ffffffffffffffff/0000000000002000
   dev: aspeed.gpio-ast2700, id ""
     gpio-out "sysbus-irq" 213
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.scu-ast2700, id ""
     silicon-rev = 100729091 (0x6010103)
     hw-strap1 = 0 (0x0)
     hw-strap2 = 0 (0x0)
     hw-prot-key = 0 (0x0)
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.adc.engine, id ""
     gpio-out "sysbus-irq" 1
     engine-id = 1 (0x1)
     nr-channels = 8 (0x8)
     mmio ffffffffffffffff/0000000000000100
   dev: aspeed.adc.engine, id ""
     gpio-out "sysbus-irq" 1
     engine-id = 0 (0x0)
     nr-channels = 8 (0x8)
     mmio ffffffffffffffff/0000000000000100
   dev: aspeed.adc-ast2700, id ""
     gpio-out "sysbus-irq" 1
     gpio-in "" 2
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.smc.flash, id ""
     cs = 1 (0x1)
     controller = "/machine/soc/ioexp[1]/ioexp-spi[0]"
     mmio ffffffffffffffff/0000000008000000
   dev: aspeed.smc.flash, id ""
     cs = 0 (0x0)
     controller = "/machine/soc/ioexp[1]/ioexp-spi[0]"
     mmio ffffffffffffffff/0000000008000000
   dev: aspeed.spi0-ast2700, id ""
     gpio-out "cs" 2
     gpio-out "sysbus-irq" 1
     inject-failure = false
     dram-base = 0 (0x0)
     dram = "/machine/soc/ioexp[1]/aspeed.ast1700[0]"
     mmio ffffffffffffffff/0000000000000100
     mmio ffffffffffffffff/0000000040000000
     bus: ssi.5
       type SSI
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: aspeed.ast1700, id ""
     board-idx = 1 (0x1)
     silicon-rev = 100729091 (0x6010103)
     mmio 0000000050000000/0000000001000000
   dev: unimplemented-device, id ""
     size = 65536 (0x10000)
     name = "ioexp-i3c"
     mmio ffffffffffffffff/0000000000010000
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[0]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[0]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[0]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[0]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[0]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[0]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[0]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[0]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/ioexp[0]/ioexp-scu[0]"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.sgpio-ast2700, id ""
     gpio-out "sysbus-irq" 1
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.sgpio-ast2700, id ""
     gpio-out "sysbus-irq" 1
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.ltpi-ctrl, id ""
     mmio ffffffffffffffff/0000000000000900
   dev: aspeed.pwm, id ""
     gpio-out "sysbus-irq" 1
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 15 (0xf)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.15"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.15
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 14 (0xe)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.14"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.14
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 13 (0xd)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.13"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.13
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 12 (0xc)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.12"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.12
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 11 (0xb)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.11"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.11
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 10 (0xa)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.10"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.10
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 9 (0x9)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.9"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.9
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 8 (0x8)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.8"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.8
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 7 (0x7)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.7"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.7
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 6 (0x6)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.6"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.6
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 5 (0x5)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.5"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.5
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 4 (0x4)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.4"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.4
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 3 (0x3)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.3"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.3
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 2 (0x2)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.2"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.2
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 1 (0x1)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.1"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.1
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 0 (0x0)
     controller = "/machine/soc/ioexp[0]/ioexp-i2c[0]"
     bus-name = "ioexp0.0"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: ioexp0.0
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c-ast2700, id ""
     gpio-out "sysbus-irq" 1
     dram = "/machine/soc/ioexp[0]/aspeed.ast1700[0]"
     bus-label = "ioexp0"
     mmio ffffffffffffffff/0000000000002000
   dev: aspeed.gpio-ast2700, id ""
     gpio-out "sysbus-irq" 213
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.scu-ast2700, id ""
     silicon-rev = 100729091 (0x6010103)
     hw-strap1 = 0 (0x0)
     hw-strap2 = 0 (0x0)
     hw-prot-key = 0 (0x0)
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.adc.engine, id ""
     gpio-out "sysbus-irq" 1
     engine-id = 1 (0x1)
     nr-channels = 8 (0x8)
     mmio ffffffffffffffff/0000000000000100
   dev: aspeed.adc.engine, id ""
     gpio-out "sysbus-irq" 1
     engine-id = 0 (0x0)
     nr-channels = 8 (0x8)
     mmio ffffffffffffffff/0000000000000100
   dev: aspeed.adc-ast2700, id ""
     gpio-out "sysbus-irq" 1
     gpio-in "" 2
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.smc.flash, id ""
     cs = 1 (0x1)
     controller = "/machine/soc/ioexp[0]/ioexp-spi[0]"
     mmio ffffffffffffffff/0000000008000000
   dev: aspeed.smc.flash, id ""
     cs = 0 (0x0)
     controller = "/machine/soc/ioexp[0]/ioexp-spi[0]"
     mmio ffffffffffffffff/0000000008000000
   dev: aspeed.spi0-ast2700, id ""
     gpio-out "cs" 2
     gpio-out "sysbus-irq" 1
     inject-failure = false
     dram-base = 0 (0x0)
     dram = "/machine/soc/ioexp[0]/aspeed.ast1700[0]"
     mmio ffffffffffffffff/0000000000000100
     mmio ffffffffffffffff/0000000040000000
     bus: ssi.4
       type SSI
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: aspeed.ast1700, id ""
     board-idx = 0 (0x0)
     silicon-rev = 100729091 (0x6010103)
     mmio 0000000030000000/0000000001000000
   dev: aspeed.ltpi-ctrl, id ""
     mmio ffffffffffffffff/0000000000000900
   dev: aspeed.ltpi-ctrl, id ""
     mmio ffffffffffffffff/0000000000000900
   dev: aspeed.pcie-rc, id ""
     gpio-out "sysbus-irq" 1
     bus-nr = 0 (0x0)
     has-rd = false
     rp-addr = 0 (0x0)
     msi-addr = 240 (0xf0)
     dram-base = 17179869184 (0x400000000)
     dram = "/objects/ram/ram[0]"
     x-config-reg-migration-enabled = true
     bypass-iommu = false
     mmio ffffffffffffffff/0000000010000000
     mmio ffffffffffffffff/ffffffffffffffff
     mmio ffffffffffffffff/0000000000010000
     bus: pcie.rc2
       type PCIE
       dev: aspeed.pcie-root-port, id ""
         power_controller_present = true
         disable-acs = false
         chassis = 2 (0x2)
         slot = 0 (0x0)
         hotplug = true
         x-do-not-expose-native-hotplug-cap = false
         port = 0 (0x0)
         aer_log_max = 8 (0x8)
         x-pci-express-writeable-slt-bug = false
         addr = 00.0
         romfile = ""
         romsize = 4294967295 (0xffffffff)
         rombar = -1 (0xffffffffffffffff)
         multifunction = false
         x-pcie-lnksta-dllla = true
         x-pcie-extcap-init = true
         failover_pair_id = ""
         acpi-index = 0 (0x0)
         x-pcie-err-unc-mask = true
         x-pcie-ari-nextfn-1 = false
         x-max-bounce-buffer-size = 4096 (4 KiB)
         sriov-pf = ""
         x-pcie-ext-tag = true
         busnr = 0 (0x0)
         class PCI bridge, addr 00:00.0, pci id 1a03:1150 (sub 0000:0000)
         bus: pcie.2
           type PCIE
   dev: aspeed.pcie-cfg-ast2700, id ""
     id = 2 (0x2)
     mmio ffffffffffffffff/0000000000000100
   dev: aspeed.pcie-phy-ast2700, id ""
     id = 2 (0x2)
     mmio ffffffffffffffff/0000000000000800
   dev: aspeed.pcie-rc, id ""
     gpio-out "sysbus-irq" 1
     bus-nr = 0 (0x0)
     has-rd = false
     rp-addr = 0 (0x0)
     msi-addr = 240 (0xf0)
     dram-base = 17179869184 (0x400000000)
     dram = "/objects/ram/ram[0]"
     x-config-reg-migration-enabled = true
     bypass-iommu = false
     mmio ffffffffffffffff/0000000010000000
     mmio ffffffffffffffff/ffffffffffffffff
     mmio ffffffffffffffff/0000000000010000
     bus: pcie.rc1
       type PCIE
       dev: aspeed.pcie-root-port, id ""
         power_controller_present = true
         disable-acs = false
         chassis = 1 (0x1)
         slot = 0 (0x0)
         hotplug = true
         x-do-not-expose-native-hotplug-cap = false
         port = 0 (0x0)
         aer_log_max = 8 (0x8)
         x-pci-express-writeable-slt-bug = false
         addr = 00.0
         romfile = ""
         romsize = 4294967295 (0xffffffff)
         rombar = -1 (0xffffffffffffffff)
         multifunction = false
         x-pcie-lnksta-dllla = true
         x-pcie-extcap-init = true
         failover_pair_id = ""
         acpi-index = 0 (0x0)
         x-pcie-err-unc-mask = true
         x-pcie-ari-nextfn-1 = false
         x-max-bounce-buffer-size = 4096 (4 KiB)
         sriov-pf = ""
         x-pcie-ext-tag = true
         busnr = 0 (0x0)
         class PCI bridge, addr 00:00.0, pci id 1a03:1150 (sub 0000:0000)
         bus: pcie.1
           type PCIE
   dev: aspeed.pcie-cfg-ast2700, id ""
     id = 1 (0x1)
     mmio ffffffffffffffff/0000000000000100
   dev: aspeed.pcie-phy-ast2700, id ""
     id = 1 (0x1)
     mmio ffffffffffffffff/0000000000000800
   dev: aspeed.pcie-rc, id ""
     gpio-out "sysbus-irq" 1
     bus-nr = 0 (0x0)
     has-rd = false
     rp-addr = 0 (0x0)
     msi-addr = 240 (0xf0)
     dram-base = 17179869184 (0x400000000)
     dram = "/objects/ram/ram[0]"
     x-config-reg-migration-enabled = true
     bypass-iommu = false
     mmio ffffffffffffffff/0000000010000000
     mmio ffffffffffffffff/ffffffffffffffff
     mmio ffffffffffffffff/0000000000010000
     bus: pcie.rc0
       type PCIE
       dev: aspeed.pcie-root-port, id ""
         power_controller_present = true
         disable-acs = false
         chassis = 0 (0x0)
         slot = 0 (0x0)
         hotplug = true
         x-do-not-expose-native-hotplug-cap = false
         port = 0 (0x0)
         aer_log_max = 8 (0x8)
         x-pci-express-writeable-slt-bug = false
         addr = 00.0
         romfile = ""
         romsize = 4294967295 (0xffffffff)
         rombar = -1 (0xffffffffffffffff)
         multifunction = false
         x-pcie-lnksta-dllla = true
         x-pcie-extcap-init = true
         failover_pair_id = ""
         acpi-index = 0 (0x0)
         x-pcie-err-unc-mask = true
         x-pcie-ari-nextfn-1 = false
         x-max-bounce-buffer-size = 4096 (4 KiB)
         sriov-pf = ""
         x-pcie-ext-tag = true
         busnr = 0 (0x0)
         class PCI bridge, addr 00:00.0, pci id 1a03:1150 (sub 0000:0000)
         bus: pcie.0
           type PCIE
   dev: aspeed.pcie-cfg-ast2700, id ""
     id = 0 (0x0)
     mmio ffffffffffffffff/0000000000000100
   dev: aspeed.pcie-phy-ast2700, id ""
     id = 0 (0x0)
     mmio ffffffffffffffff/0000000000000800
   dev: aspeed.hace-ast2700, id ""
     gpio-out "sysbus-irq" 1
     dram = "/objects/ram/ram[0]"
     mmio ffffffffffffffff/000000000000009c
   dev: aspeed.timer-ast2700, id ""
     gpio-out "sysbus-irq" 8
     scu = "/machine/soc/scu"
     mmio ffffffffffffffff/0000000000001000
   dev: generic-sdhci, id ""
     gpio-out "sysbus-irq" 1
     endianness = 2 (0x2)
     sd-spec-version = 2 (0x2)
     uhs = 0 (0x0)
     vendor = 0 (0x0)
     capareg = 30567563392 (0x71df80080)
     maxcurr = 0 (0x0)
     pending-insert-quirk = false
     dma = ""
     wp-inverted = false
     mmio ffffffffffffffff/0000000000000100
     bus: sd-bus
       type sdhci-bus
   dev: aspeed.sdhci-ast2700, id ""
     gpio-out "sysbus-irq" 1
     gpio-in "" 1
     num-slots = 1 (0x1)
     mmio ffffffffffffffff/0000000000001000
   dev: generic-sdhci, id ""
     gpio-out "sysbus-irq" 1
     endianness = 2 (0x2)
     sd-spec-version = 2 (0x2)
     uhs = 0 (0x0)
     vendor = 0 (0x0)
     capareg = 30567563392 (0x71df80080)
     maxcurr = 0 (0x0)
     pending-insert-quirk = false
     dma = ""
     wp-inverted = false
     mmio ffffffffffffffff/0000000000000100
     bus: sd-bus
       type sdhci-bus
   dev: aspeed.sdhci-ast2700, id ""
     gpio-out "sysbus-irq" 1
     gpio-in "" 1
     num-slots = 1 (0x1)
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.rtc, id ""
     gpio-out "sysbus-irq" 1
     mmio ffffffffffffffff/0000000000000018
   dev: aspeed.sgpio-ast2700, id ""
     gpio-out "sysbus-irq" 1
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.sgpio-ast2700, id ""
     gpio-out "sysbus-irq" 1
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.gpio-ast2700, id ""
     gpio-out "sysbus-irq" 213
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 15 (0xf)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.15"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.15
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 14 (0xe)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.14"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.14
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 13 (0xd)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.13"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.13
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 12 (0xc)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.12"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.12
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 11 (0xb)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.11"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.11
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 10 (0xa)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.10"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.10
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 9 (0x9)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.9"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.9
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 8 (0x8)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.8"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.8
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 7 (0x7)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.7"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.7
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 6 (0x6)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.6"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.6
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 5 (0x5)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.5"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.5
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 4 (0x4)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.4"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.4
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 3 (0x3)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.3"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.3
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 2 (0x2)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.2"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.2
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 1 (0x1)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.1"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.1
       type i2c-bus
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c.bus, id ""
     gpio-out "sysbus-irq" 1
     bus-id = 0 (0x0)
     controller = "/machine/soc/i2c"
     bus-name = "aspeed.i2c.bus.0"
     mmio ffffffffffffffff/0000000000000080
     mmio ffffffffffffffff/0000000000000020
     bus: aspeed.i2c.bus.0
       type i2c-bus
       dev: tmp105, id ""
         gpio-out "" 1
         address = 77 (0x4d)
       dev: aspeed.i2c.slave, id ""
         address = 255 (0xff)
   dev: aspeed.i2c-ast2700, id ""
     gpio-out "sysbus-irq" 1
     dram = "/objects/ram/ram[0]"
     bus-label = "aspeed.i2c.bus"
     mmio ffffffffffffffff/0000000000002000
   dev: aspeed.adc.engine, id ""
     gpio-out "sysbus-irq" 1
     engine-id = 1 (0x1)
     nr-channels = 8 (0x8)
     mmio ffffffffffffffff/0000000000000100
   dev: aspeed.adc.engine, id ""
     gpio-out "sysbus-irq" 1
     engine-id = 0 (0x0)
     nr-channels = 8 (0x8)
     mmio ffffffffffffffff/0000000000000100
   dev: aspeed.adc-ast2700, id ""
     gpio-out "sysbus-irq" 1
     gpio-in "" 2
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.sliio-ast2700, id ""
     mmio ffffffffffffffff/0000000000000500
   dev: aspeed.sli-ast2700, id ""
     mmio ffffffffffffffff/0000000000000500
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/scu"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/scu"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/scu"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/scu"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/scu"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/scu"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/scu"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed.wdt-ast2700, id ""
     scu = "/machine/soc/scu"
     mmio ffffffffffffffff/0000000000000080
   dev: aspeed-mmi, id ""
     nic = "/machine/soc/ftgmac100[2]"
     mmio ffffffffffffffff/0000000000000008
   dev: ftgmac100, id ""
     gpio-out "sysbus-irq" 1
     aspeed = true
     mac = "52:54:00:12:34:58"
     netdev = ""
     dma64 = true
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed-mmi, id ""
     nic = "/machine/soc/ftgmac100[1]"
     mmio ffffffffffffffff/0000000000000008
   dev: ftgmac100, id ""
     gpio-out "sysbus-irq" 1
     aspeed = true
     mac = "52:54:00:12:34:57"
     netdev = ""
     dma64 = true
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed-mmi, id ""
     nic = "/machine/soc/ftgmac100[0]"
     mmio ffffffffffffffff/0000000000000008
   dev: ftgmac100, id ""
     gpio-out "sysbus-irq" 1
     aspeed = true
     mac = "52:54:00:12:34:56"
     netdev = "net0"
     dma64 = true
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.sdmc-ast2700, id ""
     max-ram-size = 8589934592 (0x200000000)
     unlocked = true
     mmio ffffffffffffffff/0000000000001000
   dev: platform-ehci-usb, id ""
     gpio-out "sysbus-irq" 1
     maxframes = 128 (0x80)
     companion-enable = false
     mmio ffffffffffffffff/0000000000001000
     bus: usb-bus.3
       type usb-bus
   dev: platform-ehci-usb, id ""
     gpio-out "sysbus-irq" 1
     maxframes = 128 (0x80)
     companion-enable = false
     mmio ffffffffffffffff/0000000000001000
     bus: usb-bus.2
       type usb-bus
   dev: platform-ehci-usb, id ""
     gpio-out "sysbus-irq" 1
     maxframes = 128 (0x80)
     companion-enable = false
     mmio ffffffffffffffff/0000000000001000
     bus: usb-bus.1
       type usb-bus
   dev: platform-ehci-usb, id ""
     gpio-out "sysbus-irq" 1
     maxframes = 128 (0x80)
     companion-enable = false
     mmio ffffffffffffffff/0000000000001000
     bus: usb-bus.0
       type usb-bus
   dev: aspeed.smc.flash, id ""
     cs = 1 (0x1)
     controller = "/machine/soc/spi[2]"
     mmio ffffffffffffffff/0000000000000000
   dev: aspeed.smc.flash, id ""
     cs = 0 (0x0)
     controller = "/machine/soc/spi[2]"
     mmio ffffffffffffffff/0000000008000000
   dev: aspeed.spi2-ast2700, id ""
     gpio-out "cs" 2
     gpio-out "sysbus-irq" 1
     inject-failure = false
     dram-base = 0 (0x0)
     dram = "/objects/ram/ram[0]"
     mmio ffffffffffffffff/0000000000000100
     mmio ffffffffffffffff/0000000040000000
     bus: ssi.3
       type SSI
   dev: aspeed.smc.flash, id ""
     cs = 1 (0x1)
     controller = "/machine/soc/spi[1]"
     mmio ffffffffffffffff/0000000000000000
   dev: aspeed.smc.flash, id ""
     cs = 0 (0x0)
     controller = "/machine/soc/spi[1]"
     mmio ffffffffffffffff/0000000008000000
   dev: aspeed.spi1-ast2700, id ""
     gpio-out "cs" 2
     gpio-out "sysbus-irq" 1
     inject-failure = false
     dram-base = 0 (0x0)
     dram = "/objects/ram/ram[0]"
     mmio ffffffffffffffff/0000000000000100
     mmio ffffffffffffffff/0000000040000000
     bus: ssi.2
       type SSI
   dev: aspeed.smc.flash, id ""
     cs = 1 (0x1)
     controller = "/machine/soc/spi[0]"
     mmio ffffffffffffffff/0000000008000000
   dev: aspeed.smc.flash, id ""
     cs = 0 (0x0)
     controller = "/machine/soc/spi[0]"
     mmio ffffffffffffffff/0000000008000000
   dev: aspeed.spi0-ast2700, id ""
     gpio-out "cs" 2
     gpio-out "sysbus-irq" 1
     inject-failure = false
     dram-base = 0 (0x0)
     dram = "/objects/ram/ram[0]"
     mmio ffffffffffffffff/0000000000000100
     mmio ffffffffffffffff/0000000040000000
     bus: ssi.1
       type SSI
       dev: w25q512jv, id ""
         gpio-in "WP#" 1
         gpio-in "ssi-gpio-cs" 1
         write-enable = false
         nonvolatile-cfg = 36863 (0x8fff)
         spansion-cr1nv = 0 (0x0)
         spansion-cr2nv = 8 (0x8)
         spansion-cr3nv = 2 (0x2)
         spansion-cr4nv = 16 (0x10)
         drive = ""
         cs = 0 (0x0)
   dev: aspeed.smc.flash, id ""
     cs = 2 (0x2)
     controller = "/machine/soc/fmc"
     mmio ffffffffffffffff/0000000008000000
   dev: aspeed.smc.flash, id ""
     cs = 1 (0x1)
     controller = "/machine/soc/fmc"
     mmio ffffffffffffffff/0000000008000000
   dev: aspeed.smc.flash, id ""
     cs = 0 (0x0)
     controller = "/machine/soc/fmc"
     mmio ffffffffffffffff/0000000008010000
   dev: aspeed.fmc-ast2700, id ""
     gpio-out "cs" 3
     gpio-out "sysbus-irq" 1
     inject-failure = false
     dram-base = 17179869184 (0x400000000)
     dram = "/objects/ram/ram[0]"
     mmio ffffffffffffffff/0000000000000100
     mmio ffffffffffffffff/0000000040000000
     bus: ssi.0
       type SSI
       dev: w25q01jvq, id ""
         gpio-in "WP#" 1
         gpio-in "ssi-gpio-cs" 1
         write-enable = false
         nonvolatile-cfg = 36863 (0x8fff)
         spansion-cr1nv = 0 (0x0)
         spansion-cr2nv = 8 (0x8)
         spansion-cr3nv = 2 (0x2)
         spansion-cr4nv = 16 (0x10)
         drive = ""
         cs = 1 (0x1)
       dev: w25q01jvq, id ""
         gpio-in "WP#" 1
         gpio-in "ssi-gpio-cs" 1
         write-enable = false
         nonvolatile-cfg = 36863 (0x8fff)
         spansion-cr1nv = 0 (0x0)
         spansion-cr2nv = 8 (0x8)
         spansion-cr3nv = 2 (0x2)
         spansion-cr4nv = 16 (0x10)
         drive = "mtd0"
         cs = 0 (0x0)
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: serial-mm, id ""
     gpio-out "sysbus-irq" 1
     regshift = 2 (0x2)
     endianness = 2 (0x2)
     mmio ffffffffffffffff/0000000000000020
   dev: aspeed.scuio-ast2700, id ""
     silicon-rev = 100729091 (0x6010103)
     hw-strap1 = 1792 (0x700)
     hw-strap2 = 0 (0x0)
     hw-prot-key = 0 (0x0)
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.scu-ast2700, id ""
     silicon-rev = 100729091 (0x6010103)
     hw-strap1 = 2048 (0x800)
     hw-strap2 = 0 (0x0)
     hw-prot-key = 0 (0x0)
     mmio ffffffffffffffff/0000000000001000
   dev: aspeed.intcast2700-ioexp2, id ""
     gpio-out "sysbus-irq" 10
     gpio-in "" 2
     mmio ffffffffffffffff/0000000000000400
   dev: aspeed.intcast2700-ioexp1, id ""
     gpio-out "sysbus-irq" 10
     gpio-in "" 2
     mmio ffffffffffffffff/0000000000000400
   dev: aspeed.intcio-ast2700, id ""
     gpio-out "sysbus-irq" 6
     gpio-in "" 6
     mmio ffffffffffffffff/0000000000000400
   dev: aspeed.intc-ast2700, id ""
     gpio-out "sysbus-irq" 10
     gpio-in "" 1
     mmio ffffffffffffffff/0000000000004000
   dev: arm-gicv3, id ""
     gpio-out "sysbus-irq" 24
     gpio-in "" 384
     num-cpu = 4 (0x4)
     num-irq = 288 (0x120)
     revision = 3 (0x3)
     has-lpi = false
     has-nmi = false
     has-security-extensions = false
     maintenance-interrupt-id = 0 (0x0)
     force-8-bit-prio = false
     redist-region-count = 4 (0x4)
     sysmem = ""
     first-cpu-index = 0 (0x0)
     mmio ffffffffffffffff/0000000000010000
     mmio ffffffffffffffff/0000000000080000



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-19 17:38       ` Cédric Le Goater
@ 2026-01-20 10:36         ` Markus Armbruster
  2026-01-22 10:42           ` Cédric Le Goater
  0 siblings, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2026-01-20 10:36 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Philippe Mathieu-Daudé, Kane Chen, Peter Maydell, Steven Lee,
	Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley,
	open list:ASPEED BMCs, open list:All patches CC here, troy_lee,
	Paolo Bonzini, Daniel P. Berrange, Eduardo Habkost

Cc: QOM/qdev maintainers

Cédric Le Goater <clg@kaod.org> writes:

> Hello,
>
> On 1/19/26 15:25, Markus Armbruster wrote:
>> Cédric Le Goater <clg@kaod.org> writes:
>> 
>>> On 1/15/26 20:47, Philippe Mathieu-Daudé wrote:
>>>> Hi,
>>>> Cc'ing Markus.
>>>> On 12/1/26 09:30, Kane Chen via qemu development wrote:
>>>>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>>>>
>>>>> Currently, the Aspeed I2C controller uses a static naming convention
>>>>> for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
>>>>> object name conflicts in machine models that incorporate multiple I2C
>>>>> controllers, such as an Aspeed SoC paired with an external IO expander
>>>>> or a co-processor like the AST1700.
>>>>>
>>>> Is this a side-effect of Problem 4: 'The /machine/unattached/ orphanage'
>>>> described here?
>>>> https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/

No, but ...

>>>> This problem isn't specific to I2C nor Aspeed.

... yes, indeed.  Details below.

>>> See the discussion here :
>>>
>>>    https://lore.kernel.org/qemu-devel/006fa26f-6b84-4e82-b6e1-7d1353579441@kaod.org/
>>>
>>> The Aspeed SoC has 3*16 I2C buses attached on 3 different I2C
>>> controllers plus the I2C/I3C buses. We need to find a way to
>>> distinguish these groups at the QEMU machine level to be able
>>> to add devices on the right bus when using the command line.
>>>
>>> Suggestions welcome !
>>
>> Please show me how to start a QEMU with the 48 I2C mentioned above,
>> complete with output of "info qtree".
>
> Clone
>
>   https://github.com/legoater/qemu/commits/aspeed-11.0
>
> Download
>
>   https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.08/ast2700-default-obmc.tar.gz
>
> And run :
>
>   qemu-system-aarch64 -M ast2700a1-evb -m 8G -smp 4 -net nic,macaddr=,netdev=net0 -netdev user,id=net0,hostfwd=::2207-:22 -drive file=path/to/ast2700-default/image-bmc,format=raw,if=mtd -nographic -serial mon:stdio -snapshot
>
> Today, to attach an I2C device on one of the Aspeed SoC I2C buses :
>
>   -device tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test
>
> The Aspeed SoC I2C bus names follow the "aspeed.i2c.bus.X" format.
> This is the model typename. The 2 new IO expander models attached
> to the Aspeed SoC have an extra 16 I2C buses each. These buses use
> an "ioexpX.Y" name, as proposed in the aspeed-next branch.
>
> Attaching a device to one of the IO expanders I2C buses would be :
>
>   -device tmp105,bus=ioexp0.1,address=0x4d,id=tmp-test
>
> See the qtree below.

Thank you!

Machine ast2700a1-evb has QOM objects

    /machine/soc/i2c/ (aspeed.i2c-ast2700)
      /bus[N] (aspeed.i2c.bus)    for N in 0..15
        /aspeed.i2c.bus.N (i2c-bus)

in both master and aspeed-11.0.

Object aspeed.i2c-ast2700 is a sysbus device that contains 16
aspeed.i2c.bus objects as children bus[N] for N in 0..15.  Each object
has a property "bus-id" with value N.

Object aspeed.i2c.bus is a sysbus device that contains an i2c-bus object
as child aspeed.i2c.bus.N.

Aside: why parent TYPE_SYS_BUS_DEVICE, and not TYPE_DEVICE?  It doesn't
actually plug into a sysbus...

Object i2c-bus is a qbus.

In master, object aspeed.i2c.bus computes the child name by appending .N
to its own type name TYPE_ASPEED_I2C_BUS.

In aspeed-11.0, it takes it from its property "bus-name".  The property
is set by its QOM parent aspeed.i2c-ast2700, and computed by appending
.N to the value of its property "bus-label".  It defaults the property
to TYPE_ASPEED_I2C_BUS.

Since nothing sets this property in this case, we get the same child
name.

aspeed-11.0 additionally has QOM objects

    /machine/soc/ioexp[M] (aspeed.ast1700)    for M in 0..1
        /ioexp-i2c[0] (aspeed.i2c-ast2700)
          /bus[N] (aspeed.i2c.bus)    for N in 0..15
            /ioexpM.N (i2c-bus)

Object aspeed.ast1700 is a sysbus device that contains an
aspeed.i2c-ast2700 as child "ioexp-i2c[0]".  It has a property
"board-idx" with value M.

Aside: we only ever create one aspeed.i2c-ast2700 child.  Why [0]?

Aside^2: I tried to strangle the "[*]" feature in the crib, but failed.
Has been a minor thorn in my side ever since.

aspeed.ast1700 set its child's (aspeed.i2c-ast2700) property "bus-label"
to "ioexpM".  This makes the child set the grandchild's (aspeed.i2c.bus)
property "bus-name" to "ioexpM.N", which makes the grandchild name the
great-grandchild (i2c-bus) "ioexpM.N".

This naming business is complicated, and I had a hard time ferreting it
out.  As far as I can tell, it's all in service of -device bus=...
Let's examine how that works.

We want to be able to plug i2c devices into any of these 48 i2c buses
with -device / device_add.  To do that, we need to select a bus with the
"bus" argument.

In a world saner than the one we live in, the value of "bus" would be a
QOM path or qdev ID, where qdev ID is shorthand for the QOM path
/machine/peripheral/ID.

For instance, the first i2c bus could then be selected with absolute QOM
path "/machine/soc/i2c/bus[0]" or partial QOM paths "soc/i2c/bus[0]" or
"i2c/bus[0]".  Partial QOM paths are a convenience feature that is
virtually unknown (and possibly ill-advised): you can omit leading path
components as long as there's no ambiguity.

However, in the world we live in, the value of bus is not a QOM path,
it's a path in the qtree.  Why?  qdev and -device / device_add predate
QOM.

If the path starts with "/", it's anchored at the main system bus.

Else, it's anchored at a bus whose name is the first path component.  If
there's more than one such bus, we pick the first one we find.  This is
a misfeature.

Remaining path components, if any, pick a path in the qtree from that
anchor towards leaves.  Note that the child of a qbus is always a qdev,
and the child of a qdev always a qbus.

This must ultimately resolve to a qbus of the appropriate type.

Picking a qdev child of a qbus works like this:

* If a child with a (user-specified) qdev ID equal to the path component
  exists, pick it.  Since qdev IDs are unique, there can only be one.

* Else, if children whose QOM type name equals the path component
  exists, pick the first one.

* Else, if children whose qdev alias equals the path component exists,
  pick the first one.

Picking the first one is again a misfeature.

Picking a qbus child is simpler: we pick the first child whose bus name
equals the path component.

Bus names are defined as follows:

* Whatever creates the bus may set its name.

* Else, if the qbus's parent qdev has an ID, the bus name is ID.N, where
  N counts up from 0 within that qdev.

* Else, the bus name is TYPE.N, where TYPE is the parent qdev's QOM type
  name, and N counts up from 0 within that bus class.

The only case where this is actually works is picking the N-th bus child
provided by a qdev with an ID: use bus=ID.N (a partial tree path of just
one component).  Anything else is unfit for purpose, except in special
cases, e.g. when the machine can have just one device of a certain type.

This mess is harmless for user-created devices: just specify the ID.
It's awful for onboard devices, which cannot have an ID.

This is a qdev design flaw.  It's not specific to I2C or Aspeed, as
Philippe suspected.

To illustrate it further, let's have a look at the qtree of machine
ast2700a1-evb.  Output of "info qtree" in master:

    bus: main-system-bus
      type System
      [...]
      dev: aspeed.i2c.bus, id ""    for N in 0..15
        [...]
        bus: aspeed.i2c.bus.N
          type i2c-bus
          [...]
      
In aspeed-11.0, we additionally have

      dev: aspeed.i2c.bus, id ""    for M in 0..1, N in 0..15
        [...]
        bus: ioexpM.N
          type i2c-bus
          [...]

The i2c-buses all have unique names: aspeed.i2c-bus.N and ioexpM.N.  We
can point to any of them with a partial qtree path of just one
component: bus=NAME where NAME is one of these unique names works, and
there is no ambiguity.

The buses have unique names only because device code takes pains to make
them configurable by parent devices, and the parent devices cooperate to
configure them so the resulting bus names are unique.

This is a lot of complexity to work around this qdev design flaw for
just one special instance.

Can we instead remedy the design flaw once and for all?

Here't the obvious stupid idea: give -device / device_add the means to
pick a bus by QOM path.

Thoughts?



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-20 10:36         ` Markus Armbruster
@ 2026-01-22 10:42           ` Cédric Le Goater
  2026-01-22 13:02             ` Markus Armbruster
  0 siblings, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2026-01-22 10:42 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Philippe Mathieu-Daudé, Kane Chen, Peter Maydell, Steven Lee,
	Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley,
	open list:ASPEED BMCs, open list:All patches CC here, troy_lee,
	Paolo Bonzini, Daniel P. Berrange, Eduardo Habkost

On 1/20/26 11:36, Markus Armbruster wrote:
> Cc: QOM/qdev maintainers
> 
> Cédric Le Goater <clg@kaod.org> writes:
> 
>> Hello,
>>
>> On 1/19/26 15:25, Markus Armbruster wrote:
>>> Cédric Le Goater <clg@kaod.org> writes:
>>>
>>>> On 1/15/26 20:47, Philippe Mathieu-Daudé wrote:
>>>>> Hi,
>>>>> Cc'ing Markus.
>>>>> On 12/1/26 09:30, Kane Chen via qemu development wrote:
>>>>>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>>>>>
>>>>>> Currently, the Aspeed I2C controller uses a static naming convention
>>>>>> for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
>>>>>> object name conflicts in machine models that incorporate multiple I2C
>>>>>> controllers, such as an Aspeed SoC paired with an external IO expander
>>>>>> or a co-processor like the AST1700.
>>>>>>
>>>>> Is this a side-effect of Problem 4: 'The /machine/unattached/ orphanage'
>>>>> described here?
>>>>> https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/
> 
> No, but ...
> 
>>>>> This problem isn't specific to I2C nor Aspeed.
> 
> ... yes, indeed.  Details below.
> 
>>>> See the discussion here :
>>>>
>>>>     https://lore.kernel.org/qemu-devel/006fa26f-6b84-4e82-b6e1-7d1353579441@kaod.org/
>>>>
>>>> The Aspeed SoC has 3*16 I2C buses attached on 3 different I2C
>>>> controllers plus the I2C/I3C buses. We need to find a way to
>>>> distinguish these groups at the QEMU machine level to be able
>>>> to add devices on the right bus when using the command line.
>>>>
>>>> Suggestions welcome !
>>>
>>> Please show me how to start a QEMU with the 48 I2C mentioned above,
>>> complete with output of "info qtree".
>>
>> Clone
>>
>>    https://github.com/legoater/qemu/commits/aspeed-11.0
>>
>> Download
>>
>>    https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.08/ast2700-default-obmc.tar.gz
>>
>> And run :
>>
>>    qemu-system-aarch64 -M ast2700a1-evb -m 8G -smp 4 -net nic,macaddr=,netdev=net0 -netdev user,id=net0,hostfwd=::2207-:22 -drive file=path/to/ast2700-default/image-bmc,format=raw,if=mtd -nographic -serial mon:stdio -snapshot
>>
>> Today, to attach an I2C device on one of the Aspeed SoC I2C buses :
>>
>>    -device tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test
>>
>> The Aspeed SoC I2C bus names follow the "aspeed.i2c.bus.X" format.
>> This is the model typename. The 2 new IO expander models attached
>> to the Aspeed SoC have an extra 16 I2C buses each. These buses use
>> an "ioexpX.Y" name, as proposed in the aspeed-next branch.
>>
>> Attaching a device to one of the IO expanders I2C buses would be :
>>
>>    -device tmp105,bus=ioexp0.1,address=0x4d,id=tmp-test
>>
>> See the qtree below.
> 
> Thank you!

Thank you for taking the time of analyzing the code and history.

> 
> Machine ast2700a1-evb has QOM objects
> 
>      /machine/soc/i2c/ (aspeed.i2c-ast2700)
>        /bus[N] (aspeed.i2c.bus)    for N in 0..15
>          /aspeed.i2c.bus.N (i2c-bus)
> 
> in both master and aspeed-11.0.

Yes, that doesn't change because I would prefer not to break the current
user interface. The "i2c.X" bus name would have been a better choice.
I didn't anticipated that 10y ago when I proposed this model.


> Object aspeed.i2c-ast2700 is a sysbus device that contains 16
> aspeed.i2c.bus objects as children bus[N] for N in 0..15.  Each object
> has a property "bus-id" with value N.
> 
> Object aspeed.i2c.bus is a sysbus device that contains an i2c-bus object
> as child aspeed.i2c.bus.N.
> 
> Aside: why parent TYPE_SYS_BUS_DEVICE, and not TYPE_DEVICE?  It doesn't
> actually plug into a sysbus...

The AspeedI2CBus model has a couple of MRs and an IRQ. I guess that's why.
  
> Object i2c-bus is a qbus.
> 
> In master, object aspeed.i2c.bus computes the child name by appending .N
> to its own type name TYPE_ASPEED_I2C_BUS.
> 
> In aspeed-11.0, it takes it from its property "bus-name".  The property
> is set by its QOM parent aspeed.i2c-ast2700, and computed by appending
> .N to the value of its property "bus-label".  It defaults the property
> to TYPE_ASPEED_I2C_BUS.
> 
> Since nothing sets this property in this case, we get the same child
> name.
> 
> aspeed-11.0 additionally has QOM objects
> 
>      /machine/soc/ioexp[M] (aspeed.ast1700)    for M in 0..1
>          /ioexp-i2c[0] (aspeed.i2c-ast2700)
>            /bus[N] (aspeed.i2c.bus)    for N in 0..15
>              /ioexpM.N (i2c-bus)
> 
> Object aspeed.ast1700 is a sysbus device that contains an
> aspeed.i2c-ast2700 as child "ioexp-i2c[0]".  It has a property
> "board-idx" with value M.
> 
> Aside: we only ever create one aspeed.i2c-ast2700 child.  Why [0]?

Right. We can drop [*] in aspeed_ast1700_instance_init() - PATCH 15.

  
> Aside^2: I tried to strangle the "[*]" feature in the crib, but failed.
> Has been a minor thorn in my side ever since.
> 
> aspeed.ast1700 set its child's (aspeed.i2c-ast2700) property "bus-label"
> to "ioexpM".  This makes the child set the grandchild's (aspeed.i2c.bus)
> property "bus-name" to "ioexpM.N", which makes the grandchild name the
> great-grandchild (i2c-bus) "ioexpM.N".
> 
> This naming business is complicated, and I had a hard time ferreting it
> out.  As far as I can tell, it's all in service of -device bus=...

yes.

That's why I regret not letting QEMU taking care of the naming with :

    s->bus = i2c_init_bus(dev, NULL);

This would break the user interface though. This is still an option.

> Let's examine how that works.
> 
> We want to be able to plug i2c devices into any of these 48 i2c buses
> with -device / device_add.  To do that, we need to select a bus with the
> "bus" argument.
> 
> In a world saner than the one we live in, the value of "bus" would be a
> QOM path or qdev ID, where qdev ID is shorthand for the QOM path
> /machine/peripheral/ID.
>
> For instance, the first i2c bus could then be selected with absolute QOM
> path "/machine/soc/i2c/bus[0]" or partial QOM paths "soc/i2c/bus[0]" or
> "i2c/bus[0]".  Partial QOM paths are a convenience feature that is
> virtually unknown (and possibly ill-advised): you can omit leading path
> components as long as there's no ambiguity.
> 
> However, in the world we live in, the value of bus is not a QOM path,
> it's a path in the qtree.  Why?  qdev and -device / device_add predate
> QOM.
> 
> If the path starts with "/", it's anchored at the main system bus.
> 
> Else, it's anchored at a bus whose name is the first path component.  If
> there's more than one such bus, we pick the first one we find.  This is
> a misfeature.
> 
> Remaining path components, if any, pick a path in the qtree from that
> anchor towards leaves.  Note that the child of a qbus is always a qdev,
> and the child of a qdev always a qbus.
> 
> This must ultimately resolve to a qbus of the appropriate type.
> 
> Picking a qdev child of a qbus works like this:
> 
> * If a child with a (user-specified) qdev ID equal to the path component
>    exists, pick it.  Since qdev IDs are unique, there can only be one.
> 
> * Else, if children whose QOM type name equals the path component
>    exists, pick the first one.
> 
> * Else, if children whose qdev alias equals the path component exists,
>    pick the first one.
> 
> Picking the first one is again a misfeature.
> 
> Picking a qbus child is simpler: we pick the first child whose bus name
> equals the path component.
> 
> Bus names are defined as follows:
> 
> * Whatever creates the bus may set its name.
> 
> * Else, if the qbus's parent qdev has an ID, the bus name is ID.N, where
>    N counts up from 0 within that qdev.
> 
> * Else, the bus name is TYPE.N, where TYPE is the parent qdev's QOM type
>    name, and N counts up from 0 within that bus class.
> 
> The only case where this is actually works is picking the N-th bus child
> provided by a qdev with an ID: use bus=ID.N (a partial tree path of just
> one component).  Anything else is unfit for purpose, except in special
> cases, e.g. when the machine can have just one device of a certain type.
> 
> This mess is harmless for user-created devices: just specify the ID.
> It's awful for onboard devices, which cannot have an ID.
> 
> This is a qdev design flaw.  It's not specific to I2C or Aspeed, as
> Philippe suspected.
> 
> To illustrate it further, let's have a look at the qtree of machine
> ast2700a1-evb.  Output of "info qtree" in master:
> 
>      bus: main-system-bus
>        type System
>        [...]
>        dev: aspeed.i2c.bus, id ""    for N in 0..15
>          [...]
>          bus: aspeed.i2c.bus.N
>            type i2c-bus
>            [...]
>        
> In aspeed-11.0, we additionally have
> 
>        dev: aspeed.i2c.bus, id ""    for M in 0..1, N in 0..15
>          [...]
>          bus: ioexpM.N
>            type i2c-bus
>            [...]
> 
> The i2c-buses all have unique names: aspeed.i2c-bus.N and ioexpM.N.  We
> can point to any of them with a partial qtree path of just one
> component: bus=NAME where NAME is one of these unique names works, and
> there is no ambiguity.
> 
> The buses have unique names only because device code takes pains to make
> them configurable by parent devices, and the parent devices cooperate to
> configure them so the resulting bus names are unique.
> 
> This is a lot of complexity to work around this qdev design flaw for
> just one special instance.
> 
> Can we instead remedy the design flaw once and for all?
> 
> Here't the obvious stupid idea: give -device / device_add the means to
> pick a bus by QOM path.
> 
> Thoughts?
> 

Could we support both methods ? I mean looking up the bus by the
qtree name and by the QOM path ?

Thanks,

C.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-22 10:42           ` Cédric Le Goater
@ 2026-01-22 13:02             ` Markus Armbruster
  2026-01-22 18:58               ` Cédric Le Goater
  0 siblings, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2026-01-22 13:02 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Philippe Mathieu-Daudé, Kane Chen, Peter Maydell, Steven Lee,
	Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley,
	open list:ASPEED BMCs, open list:All patches CC here, troy_lee,
	Paolo Bonzini, Daniel P. Berrange, Eduardo Habkost

Cédric Le Goater <clg@kaod.org> writes:

> On 1/20/26 11:36, Markus Armbruster wrote:
>> Cc: QOM/qdev maintainers
>> Cédric Le Goater <clg@kaod.org> writes:
>> 
>>> Hello,
>>>
>>> On 1/19/26 15:25, Markus Armbruster wrote:
>>>> Cédric Le Goater <clg@kaod.org> writes:
>>>>
>>>>> On 1/15/26 20:47, Philippe Mathieu-Daudé wrote:
>>>>>> Hi,
>>>>>> Cc'ing Markus.
>>>>>> On 12/1/26 09:30, Kane Chen via qemu development wrote:
>>>>>>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>>>>>>
>>>>>>> Currently, the Aspeed I2C controller uses a static naming convention
>>>>>>> for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
>>>>>>> object name conflicts in machine models that incorporate multiple I2C
>>>>>>> controllers, such as an Aspeed SoC paired with an external IO expander
>>>>>>> or a co-processor like the AST1700.
>>>>>>>
>>>>>> Is this a side-effect of Problem 4: 'The /machine/unattached/ orphanage'
>>>>>> described here?
>>>>>> https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/
>> 
>> No, but ...
>> 
>>>>>> This problem isn't specific to I2C nor Aspeed.
>> 
>> ... yes, indeed.  Details below.
>> 
>>>>> See the discussion here :
>>>>>
>>>>>     https://lore.kernel.org/qemu-devel/006fa26f-6b84-4e82-b6e1-7d1353579441@kaod.org/
>>>>>
>>>>> The Aspeed SoC has 3*16 I2C buses attached on 3 different I2C
>>>>> controllers plus the I2C/I3C buses. We need to find a way to
>>>>> distinguish these groups at the QEMU machine level to be able
>>>>> to add devices on the right bus when using the command line.
>>>>>
>>>>> Suggestions welcome !
>>>>
>>>> Please show me how to start a QEMU with the 48 I2C mentioned above,
>>>> complete with output of "info qtree".
>>>
>>> Clone
>>>
>>>    https://github.com/legoater/qemu/commits/aspeed-11.0
>>>
>>> Download
>>>
>>>    https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.08/ast2700-default-obmc.tar.gz
>>>
>>> And run :
>>>
>>>    qemu-system-aarch64 -M ast2700a1-evb -m 8G -smp 4 -net nic,macaddr=,netdev=net0 -netdev user,id=net0,hostfwd=::2207-:22 -drive file=path/to/ast2700-default/image-bmc,format=raw,if=mtd -nographic -serial mon:stdio -snapshot
>>>
>>> Today, to attach an I2C device on one of the Aspeed SoC I2C buses :
>>>
>>>    -device tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test
>>>
>>> The Aspeed SoC I2C bus names follow the "aspeed.i2c.bus.X" format.
>>> This is the model typename. The 2 new IO expander models attached
>>> to the Aspeed SoC have an extra 16 I2C buses each. These buses use
>>> an "ioexpX.Y" name, as proposed in the aspeed-next branch.
>>>
>>> Attaching a device to one of the IO expanders I2C buses would be :
>>>
>>>    -device tmp105,bus=ioexp0.1,address=0x4d,id=tmp-test
>>>
>>> See the qtree below.
>> 
>> Thank you!
>
> Thank you for taking the time of analyzing the code and history.
>
>> Machine ast2700a1-evb has QOM objects
>>
>>     /machine/soc/i2c/ (aspeed.i2c-ast2700)
>>       /bus[N] (aspeed.i2c.bus)    for N in 0..15
>>         /aspeed.i2c.bus.N (i2c-bus)
>>
>> in both master and aspeed-11.0.
>
> Yes, that doesn't change because I would prefer not to break the current
> user interface. The "i2c.X" bus name would have been a better choice.
> I didn't anticipated that 10y ago when I proposed this model.

We make mistakes, we learn :)

>> Object aspeed.i2c-ast2700 is a sysbus device that contains 16
>> aspeed.i2c.bus objects as children bus[N] for N in 0..15.  Each object
>> has a property "bus-id" with value N.
>>
>> Object aspeed.i2c.bus is a sysbus device that contains an i2c-bus object
>> as child aspeed.i2c.bus.N.
>>
>> Aside: why parent TYPE_SYS_BUS_DEVICE, and not TYPE_DEVICE?  It doesn't
>> actually plug into a sysbus...
>
> The AspeedI2CBus model has a couple of MRs and an IRQ. I guess that's why.

Sysbus was a questionable idea from the start.

Qdev was built around the design assumption "each device plugs into a
bus provided by a device".  This isn't not how real hardware works, but
simplifications can be useful.  However, this one broke down right away:
most onboard devices and many other devices don't plug into a
recognizable bus.  To make the assumption work, Paul Brook invented the
catch all "system bus".  Various infrastructure was then tied to the
system bus over time, because it needed to be tied to something, system
bus is something, therefore it needed to be tied to the system bus.

The design assumption is long gone.  We still create system bus devices
just to be able to use infrastructure.  Blerch.

End of digression.

>> Object i2c-bus is a qbus.
>>
>> In master, object aspeed.i2c.bus computes the child name by appending .N
>> to its own type name TYPE_ASPEED_I2C_BUS.
>>
>> In aspeed-11.0, it takes it from its property "bus-name".  The property
>> is set by its QOM parent aspeed.i2c-ast2700, and computed by appending
>> .N to the value of its property "bus-label".  It defaults the property
>> to TYPE_ASPEED_I2C_BUS.
>>
>> Since nothing sets this property in this case, we get the same child
>> name.
>>
>> aspeed-11.0 additionally has QOM objects
>>
>>     /machine/soc/ioexp[M] (aspeed.ast1700)    for M in 0..1
>>         /ioexp-i2c[0] (aspeed.i2c-ast2700)
>>           /bus[N] (aspeed.i2c.bus)    for N in 0..15
>>             /ioexpM.N (i2c-bus)
>>
>> Object aspeed.ast1700 is a sysbus device that contains an
>> aspeed.i2c-ast2700 as child "ioexp-i2c[0]".  It has a property
>> "board-idx" with value M.
>>
>> Aside: we only ever create one aspeed.i2c-ast2700 child.  Why [0]?
>
> Right. We can drop [*] in aspeed_ast1700_instance_init() - PATCH 15.
>
>> Aside^2: I tried to strangle the "[*]" feature in the crib, but failed.
>> Has been a minor thorn in my side ever since.
>>
>> aspeed.ast1700 set its child's (aspeed.i2c-ast2700) property "bus-label"
>> to "ioexpM".  This makes the child set the grandchild's (aspeed.i2c.bus)
>> property "bus-name" to "ioexpM.N", which makes the grandchild name the
>> great-grandchild (i2c-bus) "ioexpM.N".
>>
>> This naming business is complicated, and I had a hard time ferreting it
>> out.  As far as I can tell, it's all in service of -device bus=...
>
> yes.
>
> That's why I regret not letting QEMU taking care of the naming with :
>
>    s->bus = i2c_init_bus(dev, NULL);
>
> This would break the user interface though. This is still an option.

Since the devices providing these i2c buses have no qdev ID, these buses
would then be named i2c.N, where N counts up from 0.  I think.  See "Bus
names are defined as follows" below.

Good enough?

>> Let's examine how that works.
>>
>> We want to be able to plug i2c devices into any of these 48 i2c buses
>> with -device / device_add.  To do that, we need to select a bus with the
>> "bus" argument.
>>
>> In a world saner than the one we live in, the value of "bus" would be a
>> QOM path or qdev ID, where qdev ID is shorthand for the QOM path
>> /machine/peripheral/ID.

Nonsense.

"QOM path or qdev ID" is how the @id argument of device_del and
device_sync_config work.  A qdev ID resolves to a qdev.  Here, we need
to resolve to a qbus.

Instead, "QOM path or qbus ID".  Except the concept "qbus ID" does not
exist.  All we have is qbus names, which aren't unique (we screwed that
up).

>> For instance, the first i2c bus could then be selected with absolute QOM
>> path "/machine/soc/i2c/bus[0]" or partial QOM paths "soc/i2c/bus[0]" or
>> "i2c/bus[0]".  Partial QOM paths are a convenience feature that is
>> virtually unknown (and possibly ill-advised): you can omit leading path
>> components as long as there's no ambiguity.
>>
>> However, in the world we live in, the value of bus is not a QOM path,
>> it's a path in the qtree.  Why?  qdev and -device / device_add predate
>> QOM.
>>
>> If the path starts with "/", it's anchored at the main system bus.
>>
>> Else, it's anchored at a bus whose name is the first path component.  If
>> there's more than one such bus, we pick the first one we find.  This is
>> a misfeature.
>>
>> Remaining path components, if any, pick a path in the qtree from that
>> anchor towards leaves.  Note that the child of a qbus is always a qdev,
>> and the child of a qdev always a qbus.
>>
>> This must ultimately resolve to a qbus of the appropriate type.
>>
>> Picking a qdev child of a qbus works like this:
>>
>> * If a child with a (user-specified) qdev ID equal to the path component
>>   exists, pick it.  Since qdev IDs are unique, there can only be one.
>>
>> * Else, if children whose QOM type name equals the path component
>>   exists, pick the first one.
>>
>> * Else, if children whose qdev alias equals the path component exists,
>>   pick the first one.
>>
>> Picking the first one is again a misfeature.
>>
>> Picking a qbus child is simpler: we pick the first child whose bus name
>> equals the path component.
>>
>> Bus names are defined as follows:
>>
>> * Whatever creates the bus may set its name.
>>
>> * Else, if the qbus's parent qdev has an ID, the bus name is ID.N, where
>>   N counts up from 0 within that qdev.
>>
>> * Else, the bus name is TYPE.N, where TYPE is the parent qdev's QOM type
>>   name, and N counts up from 0 within that bus class.

The qbus's QOM type name, of course.

>> The only case where this is actually works is picking the N-th bus child
>> provided by a qdev with an ID: use bus=ID.N (a partial tree path of just
>> one component).  Anything else is unfit for purpose, except in special
>> cases, e.g. when the machine can have just one device of a certain type.
>>
>> This mess is harmless for user-created devices: just specify the ID.
>> It's awful for onboard devices, which cannot have an ID.
>>
>> This is a qdev design flaw.  It's not specific to I2C or Aspeed, as
>> Philippe suspected.
>>
>> To illustrate it further, let's have a look at the qtree of machine
>> ast2700a1-evb.  Output of "info qtree" in master:
>>
>>     bus: main-system-bus
>>       type System
>>       [...]
>>       dev: aspeed.i2c.bus, id ""    for N in 0..15
>>         [...]
>>         bus: aspeed.i2c.bus.N
>>           type i2c-bus
>>           [...]
>>       
>> In aspeed-11.0, we additionally have
>>
>>       dev: aspeed.i2c.bus, id ""    for M in 0..1, N in 0..15
>>         [...]
>>         bus: ioexpM.N
>>           type i2c-bus
>>           [...]
>>
>> The i2c-buses all have unique names: aspeed.i2c-bus.N and ioexpM.N.  We
>> can point to any of them with a partial qtree path of just one
>> component: bus=NAME where NAME is one of these unique names works, and
>> there is no ambiguity.
>>
>> The buses have unique names only because device code takes pains to make
>> them configurable by parent devices, and the parent devices cooperate to
>> configure them so the resulting bus names are unique.
>>
>> This is a lot of complexity to work around this qdev design flaw for
>> just one special instance.
>>
>> Can we instead remedy the design flaw once and for all?
>>
>> Here't the obvious stupid idea: give -device / device_add the means to
>> pick a bus by QOM path.
>>
>> Thoughts?
>
> Could we support both methods ? I mean looking up the bus by the
> qtree name and by the QOM path ?

We can't just change "bus".  Management applications and human users
rely on it.

Except we might be able to break aspects that aren't actually used in
anger.  Is anyone using '/' in qtree paths in anger?  I sure hope nobody
does, because that way is madness.  But it's hard to be reasonably sure.

If we can break '/' in the value of "bus", we could overload "bus": make
it a QOM path or a qbus ID, similar to how device_del's @id is a QOM
path or a qdev ID.  We'd have to define "qbus ID".

Instead, we could perhaps add an argument "parent" to specify the QOM
parent object, mutually exclusive with "bus".  For devices that plug
into a certain kind of bus, the parent object must be an instance of
that bus.

> Thanks,
>
> C.

You're welcome!



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-22 13:02             ` Markus Armbruster
@ 2026-01-22 18:58               ` Cédric Le Goater
  2026-01-23  7:50                 ` Markus Armbruster
  0 siblings, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2026-01-22 18:58 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Philippe Mathieu-Daudé, Kane Chen, Peter Maydell, Steven Lee,
	Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley,
	open list:ASPEED BMCs, open list:All patches CC here, troy_lee,
	Paolo Bonzini, Daniel P. Berrange, Eduardo Habkost

On 1/22/26 14:02, Markus Armbruster wrote:
> Cédric Le Goater <clg@kaod.org> writes:
> 
>> On 1/20/26 11:36, Markus Armbruster wrote:
>>> Cc: QOM/qdev maintainers
>>> Cédric Le Goater <clg@kaod.org> writes:
>>>
>>>> Hello,
>>>>
>>>> On 1/19/26 15:25, Markus Armbruster wrote:
>>>>> Cédric Le Goater <clg@kaod.org> writes:
>>>>>
>>>>>> On 1/15/26 20:47, Philippe Mathieu-Daudé wrote:
>>>>>>> Hi,
>>>>>>> Cc'ing Markus.
>>>>>>> On 12/1/26 09:30, Kane Chen via qemu development wrote:
>>>>>>>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>>>>>>>
>>>>>>>> Currently, the Aspeed I2C controller uses a static naming convention
>>>>>>>> for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
>>>>>>>> object name conflicts in machine models that incorporate multiple I2C
>>>>>>>> controllers, such as an Aspeed SoC paired with an external IO expander
>>>>>>>> or a co-processor like the AST1700.
>>>>>>>>
>>>>>>> Is this a side-effect of Problem 4: 'The /machine/unattached/ orphanage'
>>>>>>> described here?
>>>>>>> https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/
>>>
>>> No, but ...
>>>
>>>>>>> This problem isn't specific to I2C nor Aspeed.
>>>
>>> ... yes, indeed.  Details below.
>>>
>>>>>> See the discussion here :
>>>>>>
>>>>>>      https://lore.kernel.org/qemu-devel/006fa26f-6b84-4e82-b6e1-7d1353579441@kaod.org/
>>>>>>
>>>>>> The Aspeed SoC has 3*16 I2C buses attached on 3 different I2C
>>>>>> controllers plus the I2C/I3C buses. We need to find a way to
>>>>>> distinguish these groups at the QEMU machine level to be able
>>>>>> to add devices on the right bus when using the command line.
>>>>>>
>>>>>> Suggestions welcome !
>>>>>
>>>>> Please show me how to start a QEMU with the 48 I2C mentioned above,
>>>>> complete with output of "info qtree".
>>>>
>>>> Clone
>>>>
>>>>     https://github.com/legoater/qemu/commits/aspeed-11.0
>>>>
>>>> Download
>>>>
>>>>     https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.08/ast2700-default-obmc.tar.gz
>>>>
>>>> And run :
>>>>
>>>>     qemu-system-aarch64 -M ast2700a1-evb -m 8G -smp 4 -net nic,macaddr=,netdev=net0 -netdev user,id=net0,hostfwd=::2207-:22 -drive file=path/to/ast2700-default/image-bmc,format=raw,if=mtd -nographic -serial mon:stdio -snapshot
>>>>
>>>> Today, to attach an I2C device on one of the Aspeed SoC I2C buses :
>>>>
>>>>     -device tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test
>>>>
>>>> The Aspeed SoC I2C bus names follow the "aspeed.i2c.bus.X" format.
>>>> This is the model typename. The 2 new IO expander models attached
>>>> to the Aspeed SoC have an extra 16 I2C buses each. These buses use
>>>> an "ioexpX.Y" name, as proposed in the aspeed-next branch.
>>>>
>>>> Attaching a device to one of the IO expanders I2C buses would be :
>>>>
>>>>     -device tmp105,bus=ioexp0.1,address=0x4d,id=tmp-test
>>>>
>>>> See the qtree below.
>>>
>>> Thank you!
>>
>> Thank you for taking the time of analyzing the code and history.
>>
>>> Machine ast2700a1-evb has QOM objects
>>>
>>>      /machine/soc/i2c/ (aspeed.i2c-ast2700)
>>>        /bus[N] (aspeed.i2c.bus)    for N in 0..15
>>>          /aspeed.i2c.bus.N (i2c-bus)
>>>
>>> in both master and aspeed-11.0.
>>
>> Yes, that doesn't change because I would prefer not to break the current
>> user interface. The "i2c.X" bus name would have been a better choice.
>> I didn't anticipated that 10y ago when I proposed this model.
> 
> We make mistakes, we learn :)
> 
>>> Object aspeed.i2c-ast2700 is a sysbus device that contains 16
>>> aspeed.i2c.bus objects as children bus[N] for N in 0..15.  Each object
>>> has a property "bus-id" with value N.
>>>
>>> Object aspeed.i2c.bus is a sysbus device that contains an i2c-bus object
>>> as child aspeed.i2c.bus.N.
>>>
>>> Aside: why parent TYPE_SYS_BUS_DEVICE, and not TYPE_DEVICE?  It doesn't
>>> actually plug into a sysbus...
>>
>> The AspeedI2CBus model has a couple of MRs and an IRQ. I guess that's why.
> 
> Sysbus was a questionable idea from the start.
> 
> Qdev was built around the design assumption "each device plugs into a
> bus provided by a device".  This isn't not how real hardware works, but
> simplifications can be useful.  However, this one broke down right away:
> most onboard devices and many other devices don't plug into a
> recognizable bus.  To make the assumption work, Paul Brook invented the
> catch all "system bus".  Various infrastructure was then tied to the
> system bus over time, because it needed to be tied to something, system
> bus is something, therefore it needed to be tied to the system bus.
> 
> The design assumption is long gone.  We still create system bus devices
> just to be able to use infrastructure.  Blerch.
> 
> End of digression.
> 
>>> Object i2c-bus is a qbus.
>>>
>>> In master, object aspeed.i2c.bus computes the child name by appending .N
>>> to its own type name TYPE_ASPEED_I2C_BUS.
>>>
>>> In aspeed-11.0, it takes it from its property "bus-name".  The property
>>> is set by its QOM parent aspeed.i2c-ast2700, and computed by appending
>>> .N to the value of its property "bus-label".  It defaults the property
>>> to TYPE_ASPEED_I2C_BUS.
>>>
>>> Since nothing sets this property in this case, we get the same child
>>> name.
>>>
>>> aspeed-11.0 additionally has QOM objects
>>>
>>>      /machine/soc/ioexp[M] (aspeed.ast1700)    for M in 0..1
>>>          /ioexp-i2c[0] (aspeed.i2c-ast2700)
>>>            /bus[N] (aspeed.i2c.bus)    for N in 0..15
>>>              /ioexpM.N (i2c-bus)
>>>
>>> Object aspeed.ast1700 is a sysbus device that contains an
>>> aspeed.i2c-ast2700 as child "ioexp-i2c[0]".  It has a property
>>> "board-idx" with value M.
>>>
>>> Aside: we only ever create one aspeed.i2c-ast2700 child.  Why [0]?
>>
>> Right. We can drop [*] in aspeed_ast1700_instance_init() - PATCH 15.
>>
>>> Aside^2: I tried to strangle the "[*]" feature in the crib, but failed.
>>> Has been a minor thorn in my side ever since.
>>>
>>> aspeed.ast1700 set its child's (aspeed.i2c-ast2700) property "bus-label"
>>> to "ioexpM".  This makes the child set the grandchild's (aspeed.i2c.bus)
>>> property "bus-name" to "ioexpM.N", which makes the grandchild name the
>>> great-grandchild (i2c-bus) "ioexpM.N".
>>>
>>> This naming business is complicated, and I had a hard time ferreting it
>>> out.  As far as I can tell, it's all in service of -device bus=...
>>
>> yes.
>>
>> That's why I regret not letting QEMU taking care of the naming with :
>>
>>     s->bus = i2c_init_bus(dev, NULL);
>>
>> This would break the user interface though. This is still an option.
> 
> Since the devices providing these i2c buses have no qdev ID, these buses
> would then be named i2c.N, where N counts up from 0.  I think.  See "Bus
> names are defined as follows" below.
> 
> Good enough?

Good enough to avoid the bus naming conflict, not good enough
to easily identify a bus in the machine topology and it's also
breaking the user interface ... too many cons to be a good
choice.

> 
>>> Let's examine how that works.
>>>
>>> We want to be able to plug i2c devices into any of these 48 i2c buses
>>> with -device / device_add.  To do that, we need to select a bus with the
>>> "bus" argument.
>>>
>>> In a world saner than the one we live in, the value of "bus" would be a
>>> QOM path or qdev ID, where qdev ID is shorthand for the QOM path
>>> /machine/peripheral/ID.
> 
> Nonsense.
> 
> "QOM path or qdev ID" is how the @id argument of device_del and
> device_sync_config work.  A qdev ID resolves to a qdev.  Here, we need
> to resolve to a qbus.
> 
> Instead, "QOM path or qbus ID".  Except the concept "qbus ID" does not
> exist.  All we have is qbus names, which aren't unique (we screwed that
> up).
> 
>>> For instance, the first i2c bus could then be selected with absolute QOM
>>> path "/machine/soc/i2c/bus[0]" or partial QOM paths "soc/i2c/bus[0]" or
>>> "i2c/bus[0]".  Partial QOM paths are a convenience feature that is
>>> virtually unknown (and possibly ill-advised): you can omit leading path
>>> components as long as there's no ambiguity.
>>>
>>> However, in the world we live in, the value of bus is not a QOM path,
>>> it's a path in the qtree.  Why?  qdev and -device / device_add predate
>>> QOM.
>>>
>>> If the path starts with "/", it's anchored at the main system bus.
>>>
>>> Else, it's anchored at a bus whose name is the first path component.  If
>>> there's more than one such bus, we pick the first one we find.  This is
>>> a misfeature.
>>>
>>> Remaining path components, if any, pick a path in the qtree from that
>>> anchor towards leaves.  Note that the child of a qbus is always a qdev,
>>> and the child of a qdev always a qbus.
>>>
>>> This must ultimately resolve to a qbus of the appropriate type.
>>>
>>> Picking a qdev child of a qbus works like this:
>>>
>>> * If a child with a (user-specified) qdev ID equal to the path component
>>>    exists, pick it.  Since qdev IDs are unique, there can only be one.
>>>
>>> * Else, if children whose QOM type name equals the path component
>>>    exists, pick the first one.
>>>
>>> * Else, if children whose qdev alias equals the path component exists,
>>>    pick the first one.
>>>
>>> Picking the first one is again a misfeature.
>>>
>>> Picking a qbus child is simpler: we pick the first child whose bus name
>>> equals the path component.
>>>
>>> Bus names are defined as follows:
>>>
>>> * Whatever creates the bus may set its name.
>>>
>>> * Else, if the qbus's parent qdev has an ID, the bus name is ID.N, where
>>>    N counts up from 0 within that qdev.
>>>
>>> * Else, the bus name is TYPE.N, where TYPE is the parent qdev's QOM type
>>>    name, and N counts up from 0 within that bus class.
> 
> The qbus's QOM type name, of course.
> 
>>> The only case where this is actually works is picking the N-th bus child
>>> provided by a qdev with an ID: use bus=ID.N (a partial tree path of just
>>> one component).  Anything else is unfit for purpose, except in special
>>> cases, e.g. when the machine can have just one device of a certain type.
>>>
>>> This mess is harmless for user-created devices: just specify the ID.
>>> It's awful for onboard devices, which cannot have an ID.
>>>
>>> This is a qdev design flaw.  It's not specific to I2C or Aspeed, as
>>> Philippe suspected.
>>>
>>> To illustrate it further, let's have a look at the qtree of machine
>>> ast2700a1-evb.  Output of "info qtree" in master:
>>>
>>>      bus: main-system-bus
>>>        type System
>>>        [...]
>>>        dev: aspeed.i2c.bus, id ""    for N in 0..15
>>>          [...]
>>>          bus: aspeed.i2c.bus.N
>>>            type i2c-bus
>>>            [...]
>>>        
>>> In aspeed-11.0, we additionally have
>>>
>>>        dev: aspeed.i2c.bus, id ""    for M in 0..1, N in 0..15
>>>          [...]
>>>          bus: ioexpM.N
>>>            type i2c-bus
>>>            [...]
>>>
>>> The i2c-buses all have unique names: aspeed.i2c-bus.N and ioexpM.N.  We
>>> can point to any of them with a partial qtree path of just one
>>> component: bus=NAME where NAME is one of these unique names works, and
>>> there is no ambiguity.
>>>
>>> The buses have unique names only because device code takes pains to make
>>> them configurable by parent devices, and the parent devices cooperate to
>>> configure them so the resulting bus names are unique.
>>>
>>> This is a lot of complexity to work around this qdev design flaw for
>>> just one special instance.
>>>
>>> Can we instead remedy the design flaw once and for all?
>>>
>>> Here't the obvious stupid idea: give -device / device_add the means to
>>> pick a bus by QOM path.
>>>
>>> Thoughts?
>>
>> Could we support both methods ? I mean looking up the bus by the
>> qtree name and by the QOM path ?
> 
> We can't just change "bus".  Management applications and human users
> rely on it.
> 
> Except we might be able to break aspects that aren't actually used in
> anger.  Is anyone using '/' in qtree paths in anger?  I sure hope nobody
> does, because that way is madness.  But it's hard to be reasonably sure.

Could we add a prefix to the bus name argument to signify a change of
namespace for the lookup ? like 'qom:' or 'machine:' ?

Thanks !

C.

> If we can break '/' in the value of "bus", we could overload "bus": make
> it a QOM path or a qbus ID, similar to how device_del's @id is a QOM
> path or a qdev ID.  We'd have to define "qbus ID".
> 
> Instead, we could perhaps add an argument "parent" to specify the QOM
> parent object, mutually exclusive with "bus".  For devices that plug
> into a certain kind of bus, the parent object must be an instance of
> that bus.
> 
>> Thanks,
>>
>> C.
> 
> You're welcome!
> 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-22 18:58               ` Cédric Le Goater
@ 2026-01-23  7:50                 ` Markus Armbruster
  2026-01-26  8:51                   ` Cédric Le Goater
  0 siblings, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2026-01-23  7:50 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Philippe Mathieu-Daudé, Kane Chen, Peter Maydell, Steven Lee,
	Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley,
	open list:ASPEED BMCs, open list:All patches CC here, troy_lee,
	Paolo Bonzini, Daniel P. Berrange, Eduardo Habkost

Cédric Le Goater <clg@kaod.org> writes:

> On 1/22/26 14:02, Markus Armbruster wrote:
>> Cédric Le Goater <clg@kaod.org> writes:
>> 
>>> On 1/20/26 11:36, Markus Armbruster wrote:
>>>> Cc: QOM/qdev maintainers
>>>> Cédric Le Goater <clg@kaod.org> writes:
>>>>
>>>>> Hello,
>>>>>
>>>>> On 1/19/26 15:25, Markus Armbruster wrote:
>>>>>> Cédric Le Goater <clg@kaod.org> writes:
>>>>>>
>>>>>>> On 1/15/26 20:47, Philippe Mathieu-Daudé wrote:
>>>>>>>> Hi,
>>>>>>>> Cc'ing Markus.
>>>>>>>> On 12/1/26 09:30, Kane Chen via qemu development wrote:
>>>>>>>>> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>>>>>>>>>
>>>>>>>>> Currently, the Aspeed I2C controller uses a static naming convention
>>>>>>>>> for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to
>>>>>>>>> object name conflicts in machine models that incorporate multiple I2C
>>>>>>>>> controllers, such as an Aspeed SoC paired with an external IO expander
>>>>>>>>> or a co-processor like the AST1700.
>>>>>>>>>
>>>>>>>> Is this a side-effect of Problem 4: 'The /machine/unattached/ orphanage'
>>>>>>>> described here?
>>>>>>>> https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/
>>>>
>>>> No, but ...
>>>>
>>>>>>>> This problem isn't specific to I2C nor Aspeed.
>>>>
>>>> ... yes, indeed.  Details below.
>>>>
>>>>>>> See the discussion here :
>>>>>>>
>>>>>>>      https://lore.kernel.org/qemu-devel/006fa26f-6b84-4e82-b6e1-7d1353579441@kaod.org/
>>>>>>>
>>>>>>> The Aspeed SoC has 3*16 I2C buses attached on 3 different I2C
>>>>>>> controllers plus the I2C/I3C buses. We need to find a way to
>>>>>>> distinguish these groups at the QEMU machine level to be able
>>>>>>> to add devices on the right bus when using the command line.
>>>>>>>
>>>>>>> Suggestions welcome !
>>>>>>
>>>>>> Please show me how to start a QEMU with the 48 I2C mentioned above,
>>>>>> complete with output of "info qtree".
>>>>>
>>>>> Clone
>>>>>
>>>>>     https://github.com/legoater/qemu/commits/aspeed-11.0
>>>>>
>>>>> Download
>>>>>
>>>>>     https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.08/ast2700-default-obmc.tar.gz
>>>>>
>>>>> And run :
>>>>>
>>>>>     qemu-system-aarch64 -M ast2700a1-evb -m 8G -smp 4 -net nic,macaddr=,netdev=net0 -netdev user,id=net0,hostfwd=::2207-:22 -drive file=path/to/ast2700-default/image-bmc,format=raw,if=mtd -nographic -serial mon:stdio -snapshot
>>>>>
>>>>> Today, to attach an I2C device on one of the Aspeed SoC I2C buses :
>>>>>
>>>>>     -device tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test
>>>>>
>>>>> The Aspeed SoC I2C bus names follow the "aspeed.i2c.bus.X" format.
>>>>> This is the model typename. The 2 new IO expander models attached
>>>>> to the Aspeed SoC have an extra 16 I2C buses each. These buses use
>>>>> an "ioexpX.Y" name, as proposed in the aspeed-next branch.
>>>>>
>>>>> Attaching a device to one of the IO expanders I2C buses would be :
>>>>>
>>>>>     -device tmp105,bus=ioexp0.1,address=0x4d,id=tmp-test
>>>>>
>>>>> See the qtree below.
>>>>
>>>> Thank you!
>>>
>>> Thank you for taking the time of analyzing the code and history.
>>>
>>>> Machine ast2700a1-evb has QOM objects
>>>>
>>>>      /machine/soc/i2c/ (aspeed.i2c-ast2700)
>>>>        /bus[N] (aspeed.i2c.bus)    for N in 0..15
>>>>          /aspeed.i2c.bus.N (i2c-bus)
>>>>
>>>> in both master and aspeed-11.0.
>>>
>>> Yes, that doesn't change because I would prefer not to break the current
>>> user interface. The "i2c.X" bus name would have been a better choice.
>>> I didn't anticipated that 10y ago when I proposed this model.
>> 
>> We make mistakes, we learn :)
>> 
>>>> Object aspeed.i2c-ast2700 is a sysbus device that contains 16
>>>> aspeed.i2c.bus objects as children bus[N] for N in 0..15.  Each object
>>>> has a property "bus-id" with value N.
>>>>
>>>> Object aspeed.i2c.bus is a sysbus device that contains an i2c-bus object
>>>> as child aspeed.i2c.bus.N.
>>>>
>>>> Aside: why parent TYPE_SYS_BUS_DEVICE, and not TYPE_DEVICE?  It doesn't
>>>> actually plug into a sysbus...
>>>
>>> The AspeedI2CBus model has a couple of MRs and an IRQ. I guess that's why.
>> 
>> Sysbus was a questionable idea from the start.
>> 
>> Qdev was built around the design assumption "each device plugs into a
>> bus provided by a device".  This isn't not how real hardware works, but
>> simplifications can be useful.  However, this one broke down right away:
>> most onboard devices and many other devices don't plug into a
>> recognizable bus.  To make the assumption work, Paul Brook invented the
>> catch all "system bus".  Various infrastructure was then tied to the
>> system bus over time, because it needed to be tied to something, system
>> bus is something, therefore it needed to be tied to the system bus.
>> 
>> The design assumption is long gone.  We still create system bus devices
>> just to be able to use infrastructure.  Blerch.
>> 
>> End of digression.
>> 
>>>> Object i2c-bus is a qbus.
>>>>
>>>> In master, object aspeed.i2c.bus computes the child name by appending .N
>>>> to its own type name TYPE_ASPEED_I2C_BUS.
>>>>
>>>> In aspeed-11.0, it takes it from its property "bus-name".  The property
>>>> is set by its QOM parent aspeed.i2c-ast2700, and computed by appending
>>>> .N to the value of its property "bus-label".  It defaults the property
>>>> to TYPE_ASPEED_I2C_BUS.
>>>>
>>>> Since nothing sets this property in this case, we get the same child
>>>> name.
>>>>
>>>> aspeed-11.0 additionally has QOM objects
>>>>
>>>>      /machine/soc/ioexp[M] (aspeed.ast1700)    for M in 0..1
>>>>          /ioexp-i2c[0] (aspeed.i2c-ast2700)
>>>>            /bus[N] (aspeed.i2c.bus)    for N in 0..15
>>>>              /ioexpM.N (i2c-bus)
>>>>
>>>> Object aspeed.ast1700 is a sysbus device that contains an
>>>> aspeed.i2c-ast2700 as child "ioexp-i2c[0]".  It has a property
>>>> "board-idx" with value M.
>>>>
>>>> Aside: we only ever create one aspeed.i2c-ast2700 child.  Why [0]?
>>>
>>> Right. We can drop [*] in aspeed_ast1700_instance_init() - PATCH 15.
>>>
>>>> Aside^2: I tried to strangle the "[*]" feature in the crib, but failed.
>>>> Has been a minor thorn in my side ever since.
>>>>
>>>> aspeed.ast1700 set its child's (aspeed.i2c-ast2700) property "bus-label"
>>>> to "ioexpM".  This makes the child set the grandchild's (aspeed.i2c.bus)
>>>> property "bus-name" to "ioexpM.N", which makes the grandchild name the
>>>> great-grandchild (i2c-bus) "ioexpM.N".
>>>>
>>>> This naming business is complicated, and I had a hard time ferreting it
>>>> out.  As far as I can tell, it's all in service of -device bus=...
>>>
>>> yes.
>>>
>>> That's why I regret not letting QEMU taking care of the naming with :
>>>
>>>     s->bus = i2c_init_bus(dev, NULL);
>>>
>>> This would break the user interface though. This is still an option.
>> 
>> Since the devices providing these i2c buses have no qdev ID, these buses
>> would then be named i2c.N, where N counts up from 0.  I think.  See "Bus
>> names are defined as follows" below.
>> 
>> Good enough?
>
> Good enough to avoid the bus naming conflict, not good enough
> to easily identify a bus in the machine topology and it's also
> breaking the user interface ... too many cons to be a good
> choice.

How is it breaking the user interface?

Mind, I didn't mean to propose changing existing bus names, i.e. the bus
names "aspeed.i2c.bus.N" of the i2c bus objects at
/machine/soc/i2c/bus[N]/aspeed.i2c_init_bus.N.  Only the bus names of
the new i2c bus objects at
/machine/soc/ioexp[M]/ioexp-i2c[0]/bus[N]/NEW-BUS-OBJECT.

>>>> Let's examine how that works.
>>>>
>>>> We want to be able to plug i2c devices into any of these 48 i2c buses
>>>> with -device / device_add.  To do that, we need to select a bus with the
>>>> "bus" argument.
>>>>
>>>> In a world saner than the one we live in, the value of "bus" would be a
>>>> QOM path or qdev ID, where qdev ID is shorthand for the QOM path
>>>> /machine/peripheral/ID.
>> 
>> Nonsense.
>> 
>> "QOM path or qdev ID" is how the @id argument of device_del and
>> device_sync_config work.  A qdev ID resolves to a qdev.  Here, we need
>> to resolve to a qbus.
>> 
>> Instead, "QOM path or qbus ID".  Except the concept "qbus ID" does not
>> exist.  All we have is qbus names, which aren't unique (we screwed that
>> up).
>> 
>>>> For instance, the first i2c bus could then be selected with absolute QOM
>>>> path "/machine/soc/i2c/bus[0]" or partial QOM paths "soc/i2c/bus[0]" or
>>>> "i2c/bus[0]".  Partial QOM paths are a convenience feature that is
>>>> virtually unknown (and possibly ill-advised): you can omit leading path
>>>> components as long as there's no ambiguity.
>>>>
>>>> However, in the world we live in, the value of bus is not a QOM path,
>>>> it's a path in the qtree.  Why?  qdev and -device / device_add predate
>>>> QOM.
>>>>
>>>> If the path starts with "/", it's anchored at the main system bus.
>>>>
>>>> Else, it's anchored at a bus whose name is the first path component.  If
>>>> there's more than one such bus, we pick the first one we find.  This is
>>>> a misfeature.
>>>>
>>>> Remaining path components, if any, pick a path in the qtree from that
>>>> anchor towards leaves.  Note that the child of a qbus is always a qdev,
>>>> and the child of a qdev always a qbus.
>>>>
>>>> This must ultimately resolve to a qbus of the appropriate type.
>>>>
>>>> Picking a qdev child of a qbus works like this:
>>>>
>>>> * If a child with a (user-specified) qdev ID equal to the path component
>>>>   exists, pick it.  Since qdev IDs are unique, there can only be one.
>>>>
>>>> * Else, if children whose QOM type name equals the path component
>>>>   exists, pick the first one.
>>>>
>>>> * Else, if children whose qdev alias equals the path component exists,
>>>>   pick the first one.
>>>>
>>>> Picking the first one is again a misfeature.
>>>>
>>>> Picking a qbus child is simpler: we pick the first child whose bus name
>>>> equals the path component.
>>>>
>>>> Bus names are defined as follows:
>>>>
>>>> * Whatever creates the bus may set its name.
>>>>
>>>> * Else, if the qbus's parent qdev has an ID, the bus name is ID.N, where
>>>>   N counts up from 0 within that qdev.
>>>>
>>>> * Else, the bus name is TYPE.N, where TYPE is the parent qdev's QOM type
>>>>   name, and N counts up from 0 within that bus class.
>> 
>> The qbus's QOM type name, of course.
>> 
>>>> The only case where this is actually works is picking the N-th bus child
>>>> provided by a qdev with an ID: use bus=ID.N (a partial tree path of just
>>>> one component).  Anything else is unfit for purpose, except in special
>>>> cases, e.g. when the machine can have just one device of a certain type.
>>>>
>>>> This mess is harmless for user-created devices: just specify the ID.
>>>> It's awful for onboard devices, which cannot have an ID.
>>>>
>>>> This is a qdev design flaw.  It's not specific to I2C or Aspeed, as
>>>> Philippe suspected.
>>>>
>>>> To illustrate it further, let's have a look at the qtree of machine
>>>> ast2700a1-evb.  Output of "info qtree" in master:
>>>>
>>>>      bus: main-system-bus
>>>>        type System
>>>>        [...]
>>>>        dev: aspeed.i2c.bus, id ""    for N in 0..15
>>>>          [...]
>>>>          bus: aspeed.i2c.bus.N
>>>>            type i2c-bus
>>>>            [...]
>>>>        In aspeed-11.0, we additionally have
>>>>
>>>>        dev: aspeed.i2c.bus, id ""    for M in 0..1, N in 0..15
>>>>          [...]
>>>>          bus: ioexpM.N
>>>>            type i2c-bus
>>>>            [...]
>>>>
>>>> The i2c-buses all have unique names: aspeed.i2c-bus.N and ioexpM.N.  We
>>>> can point to any of them with a partial qtree path of just one
>>>> component: bus=NAME where NAME is one of these unique names works, and
>>>> there is no ambiguity.
>>>>
>>>> The buses have unique names only because device code takes pains to make
>>>> them configurable by parent devices, and the parent devices cooperate to
>>>> configure them so the resulting bus names are unique.
>>>>
>>>> This is a lot of complexity to work around this qdev design flaw for
>>>> just one special instance.
>>>>
>>>> Can we instead remedy the design flaw once and for all?
>>>>
>>>> Here't the obvious stupid idea: give -device / device_add the means to
>>>> pick a bus by QOM path.
>>>>
>>>> Thoughts?
>>>
>>> Could we support both methods ? I mean looking up the bus by the
>>> qtree name and by the QOM path ?
>> 
>> We can't just change "bus".  Management applications and human users
>> rely on it.
>> 
>> Except we might be able to break aspects that aren't actually used in
>> anger.  Is anyone using '/' in qtree paths in anger?  I sure hope nobody
>> does, because that way is madness.  But it's hard to be reasonably sure.
>
> Could we add a prefix to the bus name argument to signify a change of
> namespace for the lookup ? like 'qom:' or 'machine:' ?

Hmm.

The value of "bus" is a path in the qtree.  It either starts with '/'
(main system bus) or a bus name.  Recall that a bus name can be

* set by whatever creates the bus

* ID.N, where ID is the parent qdev's ID.

* TYPE.N, where TYPE is the bus's QOM type name.

TYPE.N start with "qom:" or "machine:", because QOM type names cannot
create ':' since commit b447378e121 (qom/object: Limit type names to
alphanumerical and some few special characters).

ID.N shouldn't start with "qom:" or "machine:", because

* Users cannot pick qdev IDs containing ':' since commit b560a9ab9be
  (qemu-option: Reject anti-social IDs).  Except they can, because we
  regressed in 9.2 I'll post a patch.

* Code should not pick such qdev IDs (but it totally could).

Likewise, code setting bus names containing ':' would be unwise (but it
totally could).

I'm not sure we want to rely on this particular house of cards.

As so often, the combination of ill-advised convenience features and
equally ill-advised loose syntax bites us in the butt.

>
> Thanks !
>
> C.
>
>> If we can break '/' in the value of "bus", we could overload "bus": make
>> it a QOM path or a qbus ID, similar to how device_del's @id is a QOM
>> path or a qdev ID.  We'd have to define "qbus ID".
>> 
>> Instead, we could perhaps add an argument "parent" to specify the QOM
>> parent object, mutually exclusive with "bus".  For devices that plug
>> into a certain kind of bus, the parent object must be an instance of
>> that bus.
>> 
>>> Thanks,
>>>
>>> C.
>> 
>> You're welcome!



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming
  2026-01-23  7:50                 ` Markus Armbruster
@ 2026-01-26  8:51                   ` Cédric Le Goater
  0 siblings, 0 replies; 21+ messages in thread
From: Cédric Le Goater @ 2026-01-26  8:51 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Philippe Mathieu-Daudé, Kane Chen, Peter Maydell, Steven Lee,
	Troy Lee, Jamin Lin, Andrew Jeffery, Joel Stanley,
	open list:ASPEED BMCs, open list:All patches CC here, troy_lee,
	Paolo Bonzini, Daniel P. Berrange, Eduardo Habkost

>>>> That's why I regret not letting QEMU taking care of the naming with :
>>>>
>>>>      s->bus = i2c_init_bus(dev, NULL);
>>>>
>>>> This would break the user interface though. This is still an option.
>>>
>>> Since the devices providing these i2c buses have no qdev ID, these buses
>>> would then be named i2c.N, where N counts up from 0.  I think.  See "Bus
>>> names are defined as follows" below.
>>>
>>> Good enough?
>>
>> Good enough to avoid the bus naming conflict, not good enough
>> to easily identify a bus in the machine topology and it's also
>> breaking the user interface ... too many cons to be a good
>> choice.
> 
> How is it breaking the user interface?

Sorry I might have misunderstood your comment.
  
> Mind, I didn't mean to propose changing existing bus names, i.e. the bus
> names "aspeed.i2c.bus.N" of the i2c bus objects at
> /machine/soc/i2c/bus[N]/aspeed.i2c_init_bus.N.  Only the bus names of
> the new i2c bus objects at
> /machine/soc/ioexp[M]/ioexp-i2c[0]/bus[N]/NEW-BUS-OBJECT.

Yeah that's fine. This is would be a good addition if possible.

Thanks,

C.


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-01-26  8:52 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-12  8:30 [PATCH v1 0/1] hw/i2c/aspeed: Introduce 'bus-label' to customize bus naming Kane Chen via
2026-01-12  8:30 ` Kane Chen via qemu development
2026-01-12  8:30 ` [PATCH v1 1/1] " Kane Chen via
2026-01-12  8:30   ` Kane Chen via qemu development
2026-01-14 23:11   ` Nabih Estefan
2026-01-15  2:55     ` Kane Chen
2026-01-15  7:18     ` Cédric Le Goater
2026-01-15 16:37       ` Nabih Estefan
2026-01-15  7:15   ` Cédric Le Goater
2026-01-15  7:19     ` Kane Chen
2026-01-15 17:19       ` Cédric Le Goater
2026-01-15 19:47 ` [PATCH v1 0/1] " Philippe Mathieu-Daudé
2026-01-16  5:16   ` Cédric Le Goater
2026-01-19 14:25     ` Markus Armbruster
2026-01-19 17:38       ` Cédric Le Goater
2026-01-20 10:36         ` Markus Armbruster
2026-01-22 10:42           ` Cédric Le Goater
2026-01-22 13:02             ` Markus Armbruster
2026-01-22 18:58               ` Cédric Le Goater
2026-01-23  7:50                 ` Markus Armbruster
2026-01-26  8:51                   ` Cédric Le Goater

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.