From: Kane-Chen-AS <kane_chen@aspeedtech.com>
Connect the I2C controller to the AST1700 model by mapping its MMIO
region and wiring its interrupt line.
This patch also adds a bus_label property to distinguish I2C buses on
the BMC from those on external boards. This prevents user-specified
I2C devices from being attached to the wrong bus when provided via CLI.
Signed-off-by: Kane-Chen-AS <kane_chen@aspeedtech.com>
---
include/hw/arm/aspeed_ast1700.h | 2 ++
include/hw/arm/aspeed_soc.h | 2 ++
include/hw/i2c/aspeed_i2c.h | 1 +
hw/arm/aspeed_ast1700.c | 18 ++++++++++++
hw/arm/aspeed_ast27x0.c | 51 +++++++++++++++++++++++++++++++--
hw/i2c/aspeed_i2c.c | 19 ++++++++++--
6 files changed, 88 insertions(+), 5 deletions(-)
diff --git a/include/hw/arm/aspeed_ast1700.h b/include/hw/arm/aspeed_ast1700.h
index 7ea6ff4c1a..d4b7abee7d 100644
--- a/include/hw/arm/aspeed_ast1700.h
+++ b/include/hw/arm/aspeed_ast1700.h
@@ -12,6 +12,7 @@
#include "hw/misc/aspeed_scu.h"
#include "hw/adc/aspeed_adc.h"
#include "hw/gpio/aspeed_gpio.h"
+#include "hw/i2c/aspeed_i2c.h"
#include "hw/misc/aspeed_ltpi.h"
#include "hw/ssi/aspeed_smc.h"
#include "hw/char/serial-mm.h"
@@ -34,6 +35,7 @@ struct AspeedAST1700SoCState {
AspeedADCState adc;
AspeedSCUState scu;
AspeedGPIOState gpio;
+ AspeedI2CState i2c;
};
#endif /* ASPEED_AST1700_H */
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index b051d0eb3a..4ea2521041 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -290,6 +290,8 @@ enum {
ASPEED_DEV_LTPI_CTRL2,
ASPEED_DEV_LTPI_IO0,
ASPEED_DEV_LTPI_IO1,
+ ASPEED_DEV_IOEXP0_I2C,
+ ASPEED_DEV_IOEXP1_I2C,
ASPEED_DEV_IOEXP0_INTCIO,
ASPEED_DEV_IOEXP1_INTCIO,
};
diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 2daacc10ce..babbad5ed9 100644
--- a/include/hw/i2c/aspeed_i2c.h
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -269,6 +269,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/arm/aspeed_ast1700.c b/hw/arm/aspeed_ast1700.c
index 9a6019908e..fad3b86e8d 100644
--- a/hw/arm/aspeed_ast1700.c
+++ b/hw/arm/aspeed_ast1700.c
@@ -22,6 +22,7 @@ enum {
ASPEED_AST1700_DEV_ADC,
ASPEED_AST1700_DEV_SCU,
ASPEED_AST1700_DEV_GPIO,
+ ASPEED_AST1700_DEV_I2C,
ASPEED_AST1700_DEV_UART12,
ASPEED_AST1700_DEV_LTPI_CTRL,
ASPEED_AST1700_DEV_SPI0_MEM,
@@ -33,6 +34,7 @@ static const hwaddr aspeed_ast1700_io_memmap[] = {
[ASPEED_AST1700_DEV_ADC] = 0x00C00000,
[ASPEED_AST1700_DEV_SCU] = 0x00C02000,
[ASPEED_AST1700_DEV_GPIO] = 0x00C0B000,
+ [ASPEED_AST1700_DEV_I2C] = 0x00C0F000,
[ASPEED_AST1700_DEV_UART12] = 0x00C33B00,
[ASPEED_AST1700_DEV_LTPI_CTRL] = 0x00C34000,
[ASPEED_AST1700_DEV_SPI0_MEM] = 0x04000000,
@@ -108,6 +110,18 @@ static void aspeed_ast1700_realize(DeviceState *dev, Error **errp)
aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_GPIO],
sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->gpio), 0));
+ /* I2C */
+ snprintf(dev_name, sizeof(dev_name), "ioexp%d", s->board_idx);
+ qdev_prop_set_string(DEVICE(&s->i2c), "bus-label", dev_name);
+ object_property_set_link(OBJECT(&s->i2c), "dram",
+ OBJECT(&s->iomem), errp);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c), errp)) {
+ return;
+ }
+ memory_region_add_subregion(&s->iomem,
+ aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_I2C],
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c), 0));
+
/* LTPI controller */
if (!sysbus_realize(SYS_BUS_DEVICE(&s->ltpi), errp)) {
return;
@@ -141,6 +155,10 @@ static void aspeed_ast1700_instance_init(Object *obj)
object_initialize_child(obj, "ioexp-gpio[*]", &s->gpio,
"aspeed.gpio-ast2700");
+ /* I2C */
+ object_initialize_child(obj, "ioexp-i2c[*]", &s->i2c,
+ "aspeed.i2c-ast2700");
+
/* LTPI controller */
object_initialize_child(obj, "ltpi-ctrl",
&s->ltpi, TYPE_ASPEED_LTPI);
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index d998326536..ca3adf9a50 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -205,6 +205,8 @@ static const int aspeed_soc_ast2700a1_irqmap[] = {
[ASPEED_DEV_ETH3] = 196,
[ASPEED_DEV_PECI] = 197,
[ASPEED_DEV_SDHCI] = 197,
+ [ASPEED_DEV_IOEXP0_I2C] = 198,
+ [ASPEED_DEV_IOEXP1_I2C] = 200,
};
/* GICINT 128 */
@@ -267,6 +269,18 @@ static const int ast2700_gic133_gic197_intcmap[] = {
[ASPEED_DEV_PECI] = 4,
};
+/* Primary AST1700 Interrupts */
+/* A1: GICINT 198 */
+static const int ast2700_gic198_intcmap[] = {
+ [ASPEED_DEV_IOEXP0_I2C] = 0, /* 0 - 15 */
+};
+
+/* Secondary AST1700 Interrupts */
+/* A1: GINTC 200 */
+static const int ast2700_gic200_intcmap[] = {
+ [ASPEED_DEV_IOEXP1_I2C] = 0, /* 0 - 15 */
+};
+
/* GICINT 128 ~ 136 */
/* GICINT 192 ~ 201 */
struct gic_intc_irq_info {
@@ -283,9 +297,9 @@ static const struct gic_intc_irq_info ast2700_gic_intcmap[] = {
{195, 1, 3, ast2700_gic131_gic195_intcmap},
{196, 1, 4, ast2700_gic132_gic196_intcmap},
{197, 1, 5, ast2700_gic133_gic197_intcmap},
- {198, 1, 6, NULL},
+ {198, 2, 0, ast2700_gic198_intcmap},
{199, 1, 7, NULL},
- {200, 1, 8, NULL},
+ {200, 3, 0, ast2700_gic200_intcmap},
{201, 1, 9, NULL},
{128, 0, 1, ast2700_gic128_gic192_intcmap},
{129, 0, 2, NULL},
@@ -327,14 +341,23 @@ static qemu_irq aspeed_soc_ast2700_get_irq_index(AspeedSoCState *s, int dev,
int or_idx;
int idx;
int i;
+ OrIRQState *porgates;
for (i = 0; i < ARRAY_SIZE(ast2700_gic_intcmap); i++) {
if (sc->irqmap[dev] == ast2700_gic_intcmap[i].irq) {
assert(ast2700_gic_intcmap[i].ptr);
or_idx = ast2700_gic_intcmap[i].orgate_idx;
idx = ast2700_gic_intcmap[i].intc_idx;
- return qdev_get_gpio_in(DEVICE(&a->intc[idx].orgates[or_idx]),
+ if (idx < ASPEED_INTC_NUM) {
+ porgates = &a->intc[idx].orgates[or_idx];
+ return qdev_get_gpio_in(DEVICE(porgates),
+ ast2700_gic_intcmap[i].ptr[dev] + index);
+ } else {
+ idx -= ASPEED_INTC_NUM;
+ porgates = &a->intcioexp[idx].orgates[or_idx];
+ return qdev_get_gpio_in(DEVICE(porgates),
ast2700_gic_intcmap[i].ptr[dev] + index);
+ }
}
}
@@ -1098,6 +1121,8 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
/* IO Expander */
for (i = 0; i < sc->ioexp_num; i++) {
+ AspeedI2CClass *i2c_ctl;
+
qdev_prop_set_uint8(DEVICE(&s->ioexp[i]), "board-idx", i);
if (!sysbus_realize(SYS_BUS_DEVICE(&s->ioexp[i]), errp)) {
return;
@@ -1128,6 +1153,26 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->ioexp[i].gpio), 0,
aspeed_soc_ast2700_get_irq(s, ASPEED_DEV_GPIO));
+ /* I2C */
+ i2c_ctl = ASPEED_I2C_GET_CLASS(&s->ioexp[i].i2c);
+ for (int j = 0; j < i2c_ctl->num_busses; j++) {
+ /*
+ * For I2C on AST1700:
+ * I2C bus interrupts are connected to the OR gate from bit 0 to bit
+ * 15, and the OR gate output pin is connected to the input pin of
+ * GICINT192 of IO expander Interrupt controller (INTC2/3). Then,
+ * the output pin is connected to the INTC (CPU Die) input pin, and
+ * its output pin is connected to the GIC.
+ *
+ * I2C bus 0 is connected to the OR gate at bit 0.
+ * I2C bus 15 is connected to the OR gate at bit 15.
+ */
+ irq = aspeed_soc_ast2700_get_irq_index(s,
+ ASPEED_DEV_IOEXP0_I2C + i,
+ j);
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->ioexp[i].i2c.busses[j]),
+ 0, irq);
+ }
}
aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->dpmcu),
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 83fb906bdc..ca84068bb4 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -1261,6 +1261,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)
@@ -1421,14 +1422,28 @@ 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 *name = NULL;
+ g_autofree char *pool_name = NULL;
if (!s->controller) {
error_setg(errp, TYPE_ASPEED_I2C_BUS ": 'controller' link not set");
return;
}
+ /*
+ * I2C bus naming:
+ * - Empty bus_label -> BMC internal controller, use default name.
+ * - Non-empty bus_label -> external/addon controller, prefix with label
+ * to avoid conflicts and show bus origin.
+ */
+ if (!s->controller->bus_label || (strlen(s->controller->bus_label) == 0)) {
+ name = g_strdup_printf(TYPE_ASPEED_I2C_BUS ".%d", s->id);
+ } else {
+ name = g_strdup_printf("aspeed.%s.i2c.bus.%d",
+ s->controller->bus_label, s->id);
+ }
+ pool_name = g_strdup_printf("%s.pool", name);
+
aic = ASPEED_I2C_GET_CLASS(s->controller);
sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
--
2.43.0Hello Kane,
I agree we need a way to distinguish the I2C buses belonging to
the Aspeed Soc from those of the Aspeed IO Expander, so devices
can be attached on the correct bus.
Let's proceed with your proposal. Below are some refinements.
[ ... ]
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -1261,6 +1261,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)
> @@ -1421,14 +1422,28 @@ 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);
I would prefer if you added a new property to AspeedI2CBus :
DEFINE_PROP_STRING("name", AspeedI2CBus, name),
which would be set in aspeed_i2c_realize() :
/* default value */
if (!s->bus_label) {
s->bus_label = g_strdup(TYPE_ASPEED_I2C_BUS);
}
g_autofree char *name = g_strdup_printf("%s.%d", s->bus_label, i);
if (!object_property_set_str(bus, "bus-name", name, errp)) {
return;
}
The naming should be a higher level decision, made at the SoC level.
So, aspeed_ast1700_realize() should be adjusted when the "bus-label"
property is set.
I don't think we need to keep the "aspeed.%s.i2c.bus" prefix. How about
removing "aspeed" (which is redudant anyhow on an Aspeed machine) ?
"ioexp%d.i2c.bus"
Also, in aspeed_i2c_bus_realize(), please make sure AspeedI2CBus::name
is set, like s->controller. This could be an assert too.
Thanks,
C.
> - g_autofree char *pool_name = g_strdup_printf("%s.pool", name);
> + g_autofree char *name = NULL;
> + g_autofree char *pool_name = NULL;
>
> if (!s->controller) {
> error_setg(errp, TYPE_ASPEED_I2C_BUS ": 'controller' link not set");
> return;
> }
>
> + /*
> + * I2C bus naming:
> + * - Empty bus_label -> BMC internal controller, use default name.
> + * - Non-empty bus_label -> external/addon controller, prefix with label
> + * to avoid conflicts and show bus origin.
> + */
> + if (!s->controller->bus_label || (strlen(s->controller->bus_label) == 0)) {
> + name = g_strdup_printf(TYPE_ASPEED_I2C_BUS ".%d", s->id);
> + } else {
> + name = g_strdup_printf("aspeed.%s.i2c.bus.%d",
> + s->controller->bus_label, s->id);
> + }
> + pool_name = g_strdup_printf("%s.pool", name);
> +
> aic = ASPEED_I2C_GET_CLASS(s->controller);
>
> sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
On 1/7/26 15:53, Cédric Le Goater wrote:
> Hello Kane,
>
> I agree we need a way to distinguish the I2C buses belonging to
> the Aspeed Soc from those of the Aspeed IO Expander, so devices
> can be attached on the correct bus.
>
> Let's proceed with your proposal. Below are some refinements.
>
> [ ... ]
>
>> --- a/hw/i2c/aspeed_i2c.c
>> +++ b/hw/i2c/aspeed_i2c.c
>> @@ -1261,6 +1261,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)
>> @@ -1421,14 +1422,28 @@ 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);
>
>
> I would prefer if you added a new property to AspeedI2CBus :
>
> DEFINE_PROP_STRING("name", AspeedI2CBus, name),
>
> which would be set in aspeed_i2c_realize() :
>
> /* default value */
> if (!s->bus_label) {
> s->bus_label = g_strdup(TYPE_ASPEED_I2C_BUS);
> }
>
> g_autofree char *name = g_strdup_printf("%s.%d", s->bus_label, i);
>
> if (!object_property_set_str(bus, "bus-name", name, errp)) {
> return;
> }
The above changes may be included in a separate prerequisite patch.
Thanks,
C.
> The naming should be a higher level decision, made at the SoC level.
> So, aspeed_ast1700_realize() should be adjusted when the "bus-label"
> property is set.
>
> I don't think we need to keep the "aspeed.%s.i2c.bus" prefix. How about
> removing "aspeed" (which is redudant anyhow on an Aspeed machine) ?
>
> "ioexp%d.i2c.bus"
>
> Also, in aspeed_i2c_bus_realize(), please make sure AspeedI2CBus::name
> is set, like s->controller. This could be an assert too.
>
> Thanks,
>
> C.
>
>
>> - g_autofree char *pool_name = g_strdup_printf("%s.pool", name);
>> + g_autofree char *name = NULL;
>> + g_autofree char *pool_name = NULL;
>> if (!s->controller) {
>> error_setg(errp, TYPE_ASPEED_I2C_BUS ": 'controller' link not set");
>> return;
>> }
>> + /*
>> + * I2C bus naming:
>> + * - Empty bus_label -> BMC internal controller, use default name.
>> + * - Non-empty bus_label -> external/addon controller, prefix with label
>> + * to avoid conflicts and show bus origin.
>> + */
>> + if (!s->controller->bus_label || (strlen(s->controller->bus_label) == 0)) {
>> + name = g_strdup_printf(TYPE_ASPEED_I2C_BUS ".%d", s->id);
>> + } else {
>> + name = g_strdup_printf("aspeed.%s.i2c.bus.%d",
>> + s->controller->bus_label, s->id);
>> + }
>> + pool_name = g_strdup_printf("%s.pool", name);
>> +
>> aic = ASPEED_I2C_GET_CLASS(s->controller);
>> sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
>
> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Wednesday, January 7, 2026 11:35 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>; nabihestefan@google.com
> Subject: Re: [PATCH v4 14/19] hw/arm/aspeed: attach I2C device to AST1700
> model
>
> On 1/7/26 15:53, Cédric Le Goater wrote:
> > Hello Kane,
> >
> > I agree we need a way to distinguish the I2C buses belonging to the
> > Aspeed Soc from those of the Aspeed IO Expander, so devices can be
> > attached on the correct bus.
> >
> > Let's proceed with your proposal. Below are some refinements.
> >
> > [ ... ]
> >
> >> --- a/hw/i2c/aspeed_i2c.c
> >> +++ b/hw/i2c/aspeed_i2c.c
> >> @@ -1261,6 +1261,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) @@ -1421,14 +1422,28 @@ 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);
> >
> >
> > I would prefer if you added a new property to AspeedI2CBus :
> >
> > DEFINE_PROP_STRING("name", AspeedI2CBus, name),
> >
> > which would be set in aspeed_i2c_realize() :
> >
> > /* default value */
> > if (!s->bus_label) {
> > s->bus_label = g_strdup(TYPE_ASPEED_I2C_BUS);
> > }
> >
> > g_autofree char *name = g_strdup_printf("%s.%d", s->bus_label,
> > i);
> >
> > if (!object_property_set_str(bus, "bus-name", name, errp)) {
> > return;
> > }
>
> The above changes may be included in a separate prerequisite patch.
>
> Thanks,
>
> C.
>
>
> > The naming should be a higher level decision, made at the SoC level.
> > So, aspeed_ast1700_realize() should be adjusted when the "bus-label"
> > property is set.
> >
> > I don't think we need to keep the "aspeed.%s.i2c.bus" prefix. How
> > about removing "aspeed" (which is redudant anyhow on an Aspeed
> machine) ?
> >
> > "ioexp%d.i2c.bus"
> >
> > Also, in aspeed_i2c_bus_realize(), please make sure AspeedI2CBus::name
> > is set, like s->controller. This could be an assert too.
> >
> > Thanks,
> >
> > C.
> >
> >
> >> - g_autofree char *pool_name = g_strdup_printf("%s.pool", name);
> >> + g_autofree char *name = NULL;
> >> + g_autofree char *pool_name = NULL;
> >> if (!s->controller) {
> >> error_setg(errp, TYPE_ASPEED_I2C_BUS ": 'controller' link
> >> not set");
> >> return;
> >> }
> >> + /*
> >> + * I2C bus naming:
> >> + * - Empty bus_label -> BMC internal controller, use default
> name.
> >> + * - Non-empty bus_label -> external/addon controller, prefix
> >> +with label
> >> + * to avoid conflicts and show bus origin.
> >> + */
> >> + if (!s->controller->bus_label ||
> >> +(strlen(s->controller->bus_label) == 0)) {
> >> + name = g_strdup_printf(TYPE_ASPEED_I2C_BUS ".%d", s->id);
> >> + } else {
> >> + name = g_strdup_printf("aspeed.%s.i2c.bus.%d",
> >> + s->controller->bus_label,
> s->id);
> >> + }
> >> + pool_name = g_strdup_printf("%s.pool", name);
> >> +
> >> aic = ASPEED_I2C_GET_CLASS(s->controller);
> >> sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
> >
Hi Cédric,
Thanks for your suggestions. I have refactored the bus naming logic to align with
your comments. The decision making for the bus name has been moved up to the
SoC level, and the redundant "aspeed" prefix has been removed.
Here is a summary of the changes:
1. Added a bus-label property to AspeedAST1700SoCState. This allows the top-level
SoC (e.g., AST2700) to define the label during its initialization or realize phase.
2. The bus-label is passed from aspeed_ast1700_realize to the I2C controller
(AspeedI2CState).
3. In aspeed_i2c_realize, the controller generates unique names using the bus-label.
These names are passed to the AspeedI2CBus through a new bus-name property
during the initialization of the buses.
With these changes, the new object hierarchies and bus names are as follows:
BMC: /i2c/bus[0]/aspeed.i2c.bus.0
IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/ioexp0.0
IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/ioexp1.0
I have also verified that this naming convention does not require changes to existing
test scripts, and all functional tests passed successfully.
If you have no further concerns regarding this approach, I will submit the updated patch
series.
Best Regards,
Kane
Hi Kane, > Thanks for your suggestions. I have refactored the bus naming logic to align with > your comments. The decision making for the bus name has been moved up to the > SoC level, and the redundant "aspeed" prefix has been removed. > > Here is a summary of the changes: > 1. Added a bus-label property to AspeedAST1700SoCState. This allows the top-level > SoC (e.g., AST2700) to define the label during its initialization or realize phase. > 2. The bus-label is passed from aspeed_ast1700_realize to the I2C controller > (AspeedI2CState). > 3. In aspeed_i2c_realize, the controller generates unique names using the bus-label. > These names are passed to the AspeedI2CBus through a new bus-name property > during the initialization of the buses. > > With these changes, the new object hierarchies and bus names are as follows: > BMC: /i2c/bus[0]/aspeed.i2c.bus.0 > IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/ioexp0.0 > IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/ioexp1.0 The names in the object hierarchy should not have changed, only the bus names exposed to the user are impacted. > I have also verified that this naming convention does not require changes to existing > test scripts, and all functional tests passed successfully. > > If you have no further concerns regarding this approach, I will submit the updated patch > series. Please separate the bus-label change from the rest. I am expecting a functional test case too, maybe we should update the sdk version to v10.00 first ? Thanks, C.
> -----Original Message----- > From: Cédric Le Goater <clg@kaod.org> > Sent: Thursday, January 8, 2026 6:24 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>; nabihestefan@google.com > Subject: Re: [PATCH v4 14/19] hw/arm/aspeed: attach I2C device to AST1700 > model > > Hi Kane, > > > Thanks for your suggestions. I have refactored the bus naming logic to > > align with your comments. The decision making for the bus name has > > been moved up to the SoC level, and the redundant "aspeed" prefix has > been removed. > > > > Here is a summary of the changes: > > 1. Added a bus-label property to AspeedAST1700SoCState. This allows the > top-level > > SoC (e.g., AST2700) to define the label during its initialization or realize > phase. > > 2. The bus-label is passed from aspeed_ast1700_realize to the I2C controller > > (AspeedI2CState). > > 3. In aspeed_i2c_realize, the controller generates unique names using the > bus-label. > > These names are passed to the AspeedI2CBus through a new bus-name > property > > during the initialization of the buses. > > > > With these changes, the new object hierarchies and bus names are as > follows: > > BMC: /i2c/bus[0]/aspeed.i2c.bus.0 > > IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/ioexp0.0 > > IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/ioexp1.0 > > The names in the object hierarchy should not have changed, only the bus > names exposed to the user are impacted. Sorry, I may not fully understand your point here, so I'd like to double-check. From my understanding, the object hierarchy itself has not been changed, and only the user-visible bus names are affected. Below is the current object hierarchy without my changes: BMC: /i2c/bus[0]/aspeed.i2c.bus.0 IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/aspeed.i2c.bus.0 IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/aspeed.i2c.bus.0 I believe this matches your comment that the object hierarchy remains the same, while the bus naming logic is adjusted. Please let me know if you were expecting a different result here, or if there is still something I should change. > > > I have also verified that this naming convention does not require > > changes to existing test scripts, and all functional tests passed successfully. > > > > If you have no further concerns regarding this approach, I will submit > > the updated patch series. > > Please separate the bus-label change from the rest. I am expecting a > functional test case too, maybe we should update the sdk version to v10.00 > first ? Regarding the functional test case: currently, our BMC releases do not enable AST1700 by default. I tested the image on other platforms instead. I noticed that the AST2700 DCSCM image includes a DTB with AST1700 enabled, but I ran into an image size issue. On AST2700 EVB, the BMC image size is 128 MB, while on AST2700 DCSCM it is 64 MB. This prevents directly loading the DCSCM image on the EVB. As a workaround, I concatenated the DCSCM image twice to match the 128 MB size, which allowed the image to boot and proceed with further testing. However, this feels like an unexpected and non-ideal approach for testing. Do you have any suggestions on how you would prefer this situation to be handled? Thanks for your guidance. > > Thanks, > > C. > Best Regards, Kane
On 1/9/26 10:59, Kane Chen wrote: >> -----Original Message----- >> From: Cédric Le Goater <clg@kaod.org> >> Sent: Thursday, January 8, 2026 6:24 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>; nabihestefan@google.com >> Subject: Re: [PATCH v4 14/19] hw/arm/aspeed: attach I2C device to AST1700 >> model >> >> Hi Kane, >> >>> Thanks for your suggestions. I have refactored the bus naming logic to >>> align with your comments. The decision making for the bus name has >>> been moved up to the SoC level, and the redundant "aspeed" prefix has >> been removed. >>> >>> Here is a summary of the changes: >>> 1. Added a bus-label property to AspeedAST1700SoCState. This allows the >> top-level >>> SoC (e.g., AST2700) to define the label during its initialization or realize >> phase. >>> 2. The bus-label is passed from aspeed_ast1700_realize to the I2C controller >>> (AspeedI2CState). >>> 3. In aspeed_i2c_realize, the controller generates unique names using the >> bus-label. >>> These names are passed to the AspeedI2CBus through a new bus-name >> property >>> during the initialization of the buses. >>> >>> With these changes, the new object hierarchies and bus names are as >> follows: >>> BMC: /i2c/bus[0]/aspeed.i2c.bus.0 >>> IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/ioexp0.0 >>> IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/ioexp1.0 >> >> The names in the object hierarchy should not have changed, only the bus >> names exposed to the user are impacted. > > Sorry, I may not fully understand your point here, so I'd like to double-check. > From my understanding, the object hierarchy itself has not been changed, and > only the user-visible bus names are affected. Below is the current object hierarchy > without my changes: > > BMC: /i2c/bus[0]/aspeed.i2c.bus.0 > IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/aspeed.i2c.bus.0 > IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/aspeed.i2c.bus.0 ah yes. The "child<i2c-bus>" objects are really deep in the hierarchy. I think it is fine. > > I believe this matches your comment that the object hierarchy remains the same, > while the bus naming logic is adjusted. Please let me know if you were expecting > a different result here, or if there is still something I should change. > >> >>> I have also verified that this naming convention does not require >>> changes to existing test scripts, and all functional tests passed successfully. >>> >>> If you have no further concerns regarding this approach, I will submit >>> the updated patch series. >> >> Please separate the bus-label change from the rest. I am expecting a >> functional test case too, maybe we should update the sdk version to v10.00 >> first ? > > Regarding the functional test case: currently, our BMC releases do not enable AST1700 > by default. I tested the image on other platforms instead. I noticed that the AST2700 > DCSCM image includes a DTB with AST1700 enabled, but I ran into an image size issue. > > On AST2700 EVB, the BMC image size is 128 MB, while on AST2700 DCSCM it is 64 MB. > This prevents directly loading the DCSCM image on the EVB. As a workaround, I concatenated > the DCSCM image twice to match the 128 MB size, which allowed the image to boot and > proceed with further testing. However, this feels like an unexpected and non-ideal > approach for testing. Did you try using the "fmc-model" machine option ? > Do you have any suggestions on how you would prefer this situation to be handled? We could add a new "ast2700-dc-scm" machine too, if it make sense. Thanks, C.
> -----Original Message----- > From: Cédric Le Goater <clg@kaod.org> > Sent: Friday, January 9, 2026 9:26 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>; nabihestefan@google.com > Subject: Re: [PATCH v4 14/19] hw/arm/aspeed: attach I2C device to AST1700 > model > > On 1/9/26 10:59, Kane Chen wrote: > >> -----Original Message----- > >> From: Cédric Le Goater <clg@kaod.org> > >> Sent: Thursday, January 8, 2026 6:24 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>; nabihestefan@google.com > >> Subject: Re: [PATCH v4 14/19] hw/arm/aspeed: attach I2C device to > >> AST1700 model > >> > >> Hi Kane, > >> > >>> Thanks for your suggestions. I have refactored the bus naming logic > >>> to align with your comments. The decision making for the bus name > >>> has been moved up to the SoC level, and the redundant "aspeed" > >>> prefix has > >> been removed. > >>> > >>> Here is a summary of the changes: > >>> 1. Added a bus-label property to AspeedAST1700SoCState. This allows > >>> the > >> top-level > >>> SoC (e.g., AST2700) to define the label during its > >>> initialization or realize > >> phase. > >>> 2. The bus-label is passed from aspeed_ast1700_realize to the I2C > controller > >>> (AspeedI2CState). > >>> 3. In aspeed_i2c_realize, the controller generates unique names > >>> using the > >> bus-label. > >>> These names are passed to the AspeedI2CBus through a new > >>> bus-name > >> property > >>> during the initialization of the buses. > >>> > >>> With these changes, the new object hierarchies and bus names are as > >> follows: > >>> BMC: /i2c/bus[0]/aspeed.i2c.bus.0 > >>> IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/ioexp0.0 > >>> IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/ioexp1.0 > >> > >> The names in the object hierarchy should not have changed, only the > >> bus names exposed to the user are impacted. > > > > Sorry, I may not fully understand your point here, so I'd like to double-check. > > From my understanding, the object hierarchy itself has not been > > changed, and only the user-visible bus names are affected. Below is > > the current object hierarchy without my changes: > > > > BMC: /i2c/bus[0]/aspeed.i2c.bus.0 > > IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/aspeed.i2c.bus.0 > > IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/aspeed.i2c.bus.0 > > ah yes. The "child<i2c-bus>" objects are really deep in the hierarchy. > I think it is fine. > > > > > > I believe this matches your comment that the object hierarchy remains > > the same, while the bus naming logic is adjusted. Please let me know > > if you were expecting a different result here, or if there is still something I > should change. > > > >> > >>> I have also verified that this naming convention does not require > >>> changes to existing test scripts, and all functional tests passed > successfully. > >>> > >>> If you have no further concerns regarding this approach, I will > >>> submit the updated patch series. > >> > >> Please separate the bus-label change from the rest. I am expecting a > >> functional test case too, maybe we should update the sdk version to > >> v10.00 first ? > > > > Regarding the functional test case: currently, our BMC releases do not > > enable AST1700 by default. I tested the image on other platforms > > instead. I noticed that the AST2700 DCSCM image includes a DTB with > AST1700 enabled, but I ran into an image size issue. > > > > On AST2700 EVB, the BMC image size is 128 MB, while on AST2700 DCSCM > it is 64 MB. > > This prevents directly loading the DCSCM image on the EVB. As a > > workaround, I concatenated the DCSCM image twice to match the 128 MB > > size, which allowed the image to boot and proceed with further > > testing. However, this feels like an unexpected and non-ideal approach for > testing. > Did you try using the "fmc-model" machine option ? > > > Do you have any suggestions on how you would prefer this situation to be > handled? > > We could add a new "ast2700-dc-scm" machine too, if it make sense. > > Thanks, > > C. Hi Cédric, Thanks for your suggestion. After updating the fmc-model, I have successfully loaded the AST2700 DCSCM image using the ast2700a1-evb machine type. Regarding the I2C test case for the AST1700 (IO expander): would you prefer me to append this to the end of the current AST1700 patch series, or should I submit it as a separate patch series? Best Regards, Kane
Hello Kane, > Thanks for your suggestion. After updating the fmc-model, I have successfully > loaded the AST2700 DCSCM image using the ast2700a1-evb machine type. > > Regarding the I2C test case for the AST1700 (IO expander): would you prefer me > to append this to the end of the current AST1700 patch series, or should I submit > it as a separate patch series? If it is ready, please append the functional test at the end of v5. Thanks, C.
> Subject: Re: [PATCH v4 14/19] hw/arm/aspeed: attach I2C device to AST1700 > model > > Hi Kane, > > > Thanks for your suggestions. I have refactored the bus naming logic to > > align with your comments. The decision making for the bus name has > > been moved up to the SoC level, and the redundant "aspeed" prefix has been > removed. > > > > Here is a summary of the changes: > > 1. Added a bus-label property to AspeedAST1700SoCState. This allows the > top-level > > SoC (e.g., AST2700) to define the label during its initialization or realize > phase. > > 2. The bus-label is passed from aspeed_ast1700_realize to the I2C controller > > (AspeedI2CState). > > 3. In aspeed_i2c_realize, the controller generates unique names using the > bus-label. > > These names are passed to the AspeedI2CBus through a new bus-name > property > > during the initialization of the buses. > > > > With these changes, the new object hierarchies and bus names are as > follows: > > BMC: /i2c/bus[0]/aspeed.i2c.bus.0 > > IOEXP0 (LTPI0): /ioexp[0]/ioexp-i2c[0]/bus[0]/ioexp0.0 > > IOEXP1 (LTPI1): /ioexp[1]/ioexp-i2c[0]/bus[0]/ioexp1.0 > > The names in the object hierarchy should not have changed, only the bus > names exposed to the user are impacted. > > > I have also verified that this naming convention does not require > > changes to existing test scripts, and all functional tests passed successfully. > > > > If you have no further concerns regarding this approach, I will submit > > the updated patch series. > > Please separate the bus-label change from the rest. I am expecting a functional > test case too, maybe we should update the sdk version to v10.00 first ? > + Hao Hi Cédric, Nabih, Hao ASPEED changed the boot image format starting from SDK v10.00. As a result, vbootrom can no longer boot correctly, and the manual loader command also needs to be updated. Specifically, the U-Boot FIT image has been replaced with a legacy U-Boot raw image, so the secondary solution’s manual loader command must be adjusted accordingly. I have created a new pull request to google/vbootrom and am currently waiting for review: https://github.com/google/vbootrom/pull/12 I plan to update the functional test SDK v10.00 after this PR is merged. Additionally, ASPEED will release SDK v11.00 on January 26, so I am also considering updating directly to v11.00 instead. Nabih, Hao Could you please help review this PR when you have time? Thanks in advance for your help. Jamin > Thanks, > > C. >
Hello Jamin, > ASPEED changed the boot image format starting from SDK v10.00. As a result, vbootrom can no longer boot correctly, and the manual loader command also needs to be updated. OK. See commit d63961f957ff for the update. > Specifically, the U-Boot FIT image has been replaced with a legacy U-Boot raw image, so the secondary solution’s manual loader command must be adjusted accordingly. > > I have created a new pull request to google/vbootrom and am currently waiting for review: > https://github.com/google/vbootrom/pull/12 > > I plan to update the functional test SDK v10.00 after this PR is merged. Sure. The functional test can come later. > Additionally, ASPEED will release SDK v11.00 on January 26, so I am also considering updating directly to v11.00 instead. Fine. Let's plan the updates for QEMU 11.0. Thanks, C. > > Nabih, Hao > Could you please help review this PR when you have time? > > Thanks in advance for your help. > Jamin
On 12/24/25 02:41, Kane Chen via wrote:
> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>
> Connect the I2C controller to the AST1700 model by mapping its MMIO
> region and wiring its interrupt line.
>
> This patch also adds a bus_label property to distinguish I2C buses on
> the BMC from those on external boards. This prevents user-specified
> I2C devices from being attached to the wrong bus when provided via CLI.
This patch does 2 things. Please split the changes and provide an
example of the bus name in the commit log.
I think we should add documentation for the IO expander and mention
the I2C bus names too.
Thanks,
C.
>
> Signed-off-by: Kane-Chen-AS <kane_chen@aspeedtech.com>
> ---
> include/hw/arm/aspeed_ast1700.h | 2 ++
> include/hw/arm/aspeed_soc.h | 2 ++
> include/hw/i2c/aspeed_i2c.h | 1 +
> hw/arm/aspeed_ast1700.c | 18 ++++++++++++
> hw/arm/aspeed_ast27x0.c | 51 +++++++++++++++++++++++++++++++--
> hw/i2c/aspeed_i2c.c | 19 ++++++++++--
> 6 files changed, 88 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/arm/aspeed_ast1700.h b/include/hw/arm/aspeed_ast1700.h
> index 7ea6ff4c1a..d4b7abee7d 100644
> --- a/include/hw/arm/aspeed_ast1700.h
> +++ b/include/hw/arm/aspeed_ast1700.h
> @@ -12,6 +12,7 @@
> #include "hw/misc/aspeed_scu.h"
> #include "hw/adc/aspeed_adc.h"
> #include "hw/gpio/aspeed_gpio.h"
> +#include "hw/i2c/aspeed_i2c.h"
> #include "hw/misc/aspeed_ltpi.h"
> #include "hw/ssi/aspeed_smc.h"
> #include "hw/char/serial-mm.h"
> @@ -34,6 +35,7 @@ struct AspeedAST1700SoCState {
> AspeedADCState adc;
> AspeedSCUState scu;
> AspeedGPIOState gpio;
> + AspeedI2CState i2c;
> };
>
> #endif /* ASPEED_AST1700_H */
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index b051d0eb3a..4ea2521041 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -290,6 +290,8 @@ enum {
> ASPEED_DEV_LTPI_CTRL2,
> ASPEED_DEV_LTPI_IO0,
> ASPEED_DEV_LTPI_IO1,
> + ASPEED_DEV_IOEXP0_I2C,
> + ASPEED_DEV_IOEXP1_I2C,
> ASPEED_DEV_IOEXP0_INTCIO,
> ASPEED_DEV_IOEXP1_INTCIO,
> };
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> index 2daacc10ce..babbad5ed9 100644
> --- a/include/hw/i2c/aspeed_i2c.h
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -269,6 +269,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/arm/aspeed_ast1700.c b/hw/arm/aspeed_ast1700.c
> index 9a6019908e..fad3b86e8d 100644
> --- a/hw/arm/aspeed_ast1700.c
> +++ b/hw/arm/aspeed_ast1700.c
> @@ -22,6 +22,7 @@ enum {
> ASPEED_AST1700_DEV_ADC,
> ASPEED_AST1700_DEV_SCU,
> ASPEED_AST1700_DEV_GPIO,
> + ASPEED_AST1700_DEV_I2C,
> ASPEED_AST1700_DEV_UART12,
> ASPEED_AST1700_DEV_LTPI_CTRL,
> ASPEED_AST1700_DEV_SPI0_MEM,
> @@ -33,6 +34,7 @@ static const hwaddr aspeed_ast1700_io_memmap[] = {
> [ASPEED_AST1700_DEV_ADC] = 0x00C00000,
> [ASPEED_AST1700_DEV_SCU] = 0x00C02000,
> [ASPEED_AST1700_DEV_GPIO] = 0x00C0B000,
> + [ASPEED_AST1700_DEV_I2C] = 0x00C0F000,
> [ASPEED_AST1700_DEV_UART12] = 0x00C33B00,
> [ASPEED_AST1700_DEV_LTPI_CTRL] = 0x00C34000,
> [ASPEED_AST1700_DEV_SPI0_MEM] = 0x04000000,
> @@ -108,6 +110,18 @@ static void aspeed_ast1700_realize(DeviceState *dev, Error **errp)
> aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_GPIO],
> sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->gpio), 0));
>
> + /* I2C */
> + snprintf(dev_name, sizeof(dev_name), "ioexp%d", s->board_idx);
> + qdev_prop_set_string(DEVICE(&s->i2c), "bus-label", dev_name);
> + object_property_set_link(OBJECT(&s->i2c), "dram",
> + OBJECT(&s->iomem), errp);
> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c), errp)) {
> + return;
> + }
> + memory_region_add_subregion(&s->iomem,
> + aspeed_ast1700_io_memmap[ASPEED_AST1700_DEV_I2C],
> + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c), 0));
> +
> /* LTPI controller */
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->ltpi), errp)) {
> return;
> @@ -141,6 +155,10 @@ static void aspeed_ast1700_instance_init(Object *obj)
> object_initialize_child(obj, "ioexp-gpio[*]", &s->gpio,
> "aspeed.gpio-ast2700");
>
> + /* I2C */
> + object_initialize_child(obj, "ioexp-i2c[*]", &s->i2c,
> + "aspeed.i2c-ast2700");
> +
> /* LTPI controller */
> object_initialize_child(obj, "ltpi-ctrl",
> &s->ltpi, TYPE_ASPEED_LTPI);
> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
> index d998326536..ca3adf9a50 100644
> --- a/hw/arm/aspeed_ast27x0.c
> +++ b/hw/arm/aspeed_ast27x0.c
> @@ -205,6 +205,8 @@ static const int aspeed_soc_ast2700a1_irqmap[] = {
> [ASPEED_DEV_ETH3] = 196,
> [ASPEED_DEV_PECI] = 197,
> [ASPEED_DEV_SDHCI] = 197,
> + [ASPEED_DEV_IOEXP0_I2C] = 198,
> + [ASPEED_DEV_IOEXP1_I2C] = 200,
> };
>
> /* GICINT 128 */
> @@ -267,6 +269,18 @@ static const int ast2700_gic133_gic197_intcmap[] = {
> [ASPEED_DEV_PECI] = 4,
> };
>
> +/* Primary AST1700 Interrupts */
> +/* A1: GICINT 198 */
> +static const int ast2700_gic198_intcmap[] = {
> + [ASPEED_DEV_IOEXP0_I2C] = 0, /* 0 - 15 */
> +};
> +
> +/* Secondary AST1700 Interrupts */
> +/* A1: GINTC 200 */
> +static const int ast2700_gic200_intcmap[] = {
> + [ASPEED_DEV_IOEXP1_I2C] = 0, /* 0 - 15 */
> +};
> +
> /* GICINT 128 ~ 136 */
> /* GICINT 192 ~ 201 */
> struct gic_intc_irq_info {
> @@ -283,9 +297,9 @@ static const struct gic_intc_irq_info ast2700_gic_intcmap[] = {
> {195, 1, 3, ast2700_gic131_gic195_intcmap},
> {196, 1, 4, ast2700_gic132_gic196_intcmap},
> {197, 1, 5, ast2700_gic133_gic197_intcmap},
> - {198, 1, 6, NULL},
> + {198, 2, 0, ast2700_gic198_intcmap},
> {199, 1, 7, NULL},
> - {200, 1, 8, NULL},
> + {200, 3, 0, ast2700_gic200_intcmap},
> {201, 1, 9, NULL},
> {128, 0, 1, ast2700_gic128_gic192_intcmap},
> {129, 0, 2, NULL},
> @@ -327,14 +341,23 @@ static qemu_irq aspeed_soc_ast2700_get_irq_index(AspeedSoCState *s, int dev,
> int or_idx;
> int idx;
> int i;
> + OrIRQState *porgates;
>
> for (i = 0; i < ARRAY_SIZE(ast2700_gic_intcmap); i++) {
> if (sc->irqmap[dev] == ast2700_gic_intcmap[i].irq) {
> assert(ast2700_gic_intcmap[i].ptr);
> or_idx = ast2700_gic_intcmap[i].orgate_idx;
> idx = ast2700_gic_intcmap[i].intc_idx;
> - return qdev_get_gpio_in(DEVICE(&a->intc[idx].orgates[or_idx]),
> + if (idx < ASPEED_INTC_NUM) {
> + porgates = &a->intc[idx].orgates[or_idx];
> + return qdev_get_gpio_in(DEVICE(porgates),
> + ast2700_gic_intcmap[i].ptr[dev] + index);
> + } else {
> + idx -= ASPEED_INTC_NUM;
> + porgates = &a->intcioexp[idx].orgates[or_idx];
> + return qdev_get_gpio_in(DEVICE(porgates),
> ast2700_gic_intcmap[i].ptr[dev] + index);
> + }
> }
> }
>
> @@ -1098,6 +1121,8 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
>
> /* IO Expander */
> for (i = 0; i < sc->ioexp_num; i++) {
> + AspeedI2CClass *i2c_ctl;
> +
> qdev_prop_set_uint8(DEVICE(&s->ioexp[i]), "board-idx", i);
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->ioexp[i]), errp)) {
> return;
> @@ -1128,6 +1153,26 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->ioexp[i].gpio), 0,
> aspeed_soc_ast2700_get_irq(s, ASPEED_DEV_GPIO));
>
> + /* I2C */
> + i2c_ctl = ASPEED_I2C_GET_CLASS(&s->ioexp[i].i2c);
> + for (int j = 0; j < i2c_ctl->num_busses; j++) {
> + /*
> + * For I2C on AST1700:
> + * I2C bus interrupts are connected to the OR gate from bit 0 to bit
> + * 15, and the OR gate output pin is connected to the input pin of
> + * GICINT192 of IO expander Interrupt controller (INTC2/3). Then,
> + * the output pin is connected to the INTC (CPU Die) input pin, and
> + * its output pin is connected to the GIC.
> + *
> + * I2C bus 0 is connected to the OR gate at bit 0.
> + * I2C bus 15 is connected to the OR gate at bit 15.
> + */
> + irq = aspeed_soc_ast2700_get_irq_index(s,
> + ASPEED_DEV_IOEXP0_I2C + i,
> + j);
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->ioexp[i].i2c.busses[j]),
> + 0, irq);
> + }
> }
>
> aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->dpmcu),
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 83fb906bdc..ca84068bb4 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -1261,6 +1261,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)
> @@ -1421,14 +1422,28 @@ 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 *name = NULL;
> + g_autofree char *pool_name = NULL;
>
> if (!s->controller) {
> error_setg(errp, TYPE_ASPEED_I2C_BUS ": 'controller' link not set");
> return;
> }
>
> + /*
> + * I2C bus naming:
> + * - Empty bus_label -> BMC internal controller, use default name.
> + * - Non-empty bus_label -> external/addon controller, prefix with label
> + * to avoid conflicts and show bus origin.
> + */
> + if (!s->controller->bus_label || (strlen(s->controller->bus_label) == 0)) {
> + name = g_strdup_printf(TYPE_ASPEED_I2C_BUS ".%d", s->id);
> + } else {
> + name = g_strdup_printf("aspeed.%s.i2c.bus.%d",
> + s->controller->bus_label, s->id);
> + }
> + pool_name = g_strdup_printf("%s.pool", name);
> +
> aic = ASPEED_I2C_GET_CLASS(s->controller);
>
> sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
© 2016 - 2026 Red Hat, Inc.