Re: [PATCH v3 14/18] hw/arm/aspeed: attach I2C device to AST1700 model

2025-12-10 Thread Nabih Estefan
On Sun, Dec 7, 2025 at 11:49 PM Kane Chen via  wrote:
>
> From: Kane-Chen-AS 
>
> 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 
> ---
>  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 | 49 ++---
>  hw/i2c/aspeed_i2c.c | 19 +++--
>  6 files changed, 85 insertions(+), 6 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 cebd8c21c8..602ce3924d 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 1cb3cc4f7c..bd677727f5 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]   =  0x00C0,
>  [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]  =  0x0400,
> @@ -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 402799416f..7433d365a3 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,
> 

[PATCH v3 14/18] hw/arm/aspeed: attach I2C device to AST1700 model

2025-12-07 Thread Kane Chen via
From: Kane-Chen-AS 

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 
---
 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 | 49 ++---
 hw/i2c/aspeed_i2c.c | 19 +++--
 6 files changed, 85 insertions(+), 6 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 cebd8c21c8..602ce3924d 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 1cb3cc4f7c..bd677727f5 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]   =  0x00C0,
 [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]  =  0x0400,
@@ -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 402799416f..7433d365a3 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_I