From: Alano Song <AlanoSong@163.com>
Add DesignWare I2C controller onto virt board,
and also an at24c eeprom for r/w operation.
Add these two devices into arm virt acpi table.
Confirmed with i2c-tools under v6.18 linux driver.
Signed-off-by: Alano Song <AlanoSong@163.com>
---
hw/arm/Kconfig | 1 +
hw/arm/virt-acpi-build.c | 32 ++++++++++++++++++++++++++++++++
hw/arm/virt.c | 38 +++++++++++++++++++++++++++++++++++++-
include/hw/arm/virt.h | 1 +
4 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 97d747e206..f23c063474 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -36,6 +36,7 @@ config ARM_VIRT
select VIRTIO_MEM_SUPPORTED
select ACPI_CXL
select ACPI_HMAT
+ select DW_I2C
config CUBIEBOARD
bool
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 03b4342574..3d06356169 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -100,6 +100,34 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
aml_append(scope, dev);
}
+static void acpi_dsdt_add_i2c(Aml *scope, const MemMapEntry *i2c_memmap,
+ uint32_t i2c_irq)
+{
+ Aml *i2c_dev, *eprm_dev, *crs;
+
+ i2c_dev = aml_device("I2C0");
+ aml_append(i2c_dev, aml_name_decl("_HID", aml_string("INT3433")));
+ aml_append(i2c_dev, aml_name_decl("_UID", aml_int(0)));
+
+ crs = aml_resource_template();
+ aml_append(crs, aml_memory32_fixed(i2c_memmap->base,
+ i2c_memmap->size, AML_READ_WRITE));
+ aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
+ AML_EXCLUSIVE, &i2c_irq, 1));
+ aml_append(i2c_dev, aml_name_decl("_CRS", crs));
+
+ eprm_dev = aml_device("EPRM");
+ aml_append(eprm_dev, aml_name_decl("_HID", aml_string("INT3499")));
+ aml_append(eprm_dev, aml_name_decl("_UID", aml_int(0)));
+
+ crs = aml_resource_template();
+ aml_append(crs, aml_i2c_serial_bus_device(0x50, "^"));
+ aml_append(eprm_dev, aml_name_decl("_CRS", crs));
+
+ aml_append(i2c_dev, eprm_dev);
+ aml_append(scope, i2c_dev);
+}
+
static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
{
Aml *dev, *crs;
@@ -1037,6 +1065,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
acpi_dsdt_add_uart(scope, &memmap[VIRT_UART1],
(irqmap[VIRT_UART1] + ARM_SPI_BASE), 1);
}
+
+ acpi_dsdt_add_i2c(scope, &memmap[VIRT_I2C],
+ irqmap[VIRT_I2C] + ARM_SPI_BASE);
+
if (vmc->acpi_expose_flash) {
acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
}
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index fd0e28f030..8fd37126d1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -38,6 +38,8 @@
#include "hw/arm/boot.h"
#include "hw/arm/primecell.h"
#include "hw/arm/virt.h"
+#include "hw/i2c/dw_i2c.h"
+#include "hw/nvram/eeprom_at24c.h"
#include "hw/arm/machines-qom.h"
#include "hw/block/flash.h"
#include "hw/display/ramfb.h"
@@ -193,7 +195,8 @@ static const MemMapEntry base_memmap[] = {
[VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN},
[VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
[VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
- [VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
+ [VIRT_I2C] = { 0x090c0000, 0x00001000 },
+ [VIRT_ACPI_PCIHP] = { 0x090d0000, ACPI_PCIHP_SIZE },
[VIRT_MMIO] = { 0x0a000000, 0x00000200 },
/* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
[VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
@@ -245,6 +248,7 @@ static const int a15irqmap[] = {
[VIRT_GPIO] = 7,
[VIRT_UART1] = 8,
[VIRT_ACPI_GED] = 9,
+ [VIRT_I2C] = 10,
[VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
[VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
[VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
@@ -1016,6 +1020,36 @@ static void create_uart(const VirtMachineState *vms, int uart,
g_free(nodename);
}
+static void create_i2c(const VirtMachineState *vms, int i2c)
+{
+ char *nodename = NULL;
+ hwaddr base = vms->memmap[i2c].base;
+ hwaddr size = vms->memmap[i2c].size;
+ int irq = vms->irqmap[i2c];
+ MachineState *ms = MACHINE(vms);
+ DeviceState *dev = sysbus_create_simple(TYPE_DW_I2C, base,
+ qdev_get_gpio_in(vms->gic, irq));
+ DWI2CState *s = DW_I2C(dev);
+
+ nodename = g_strdup_printf("/dw-i2c@%" PRIx64, base);
+ qemu_fdt_add_subnode(ms->fdt, nodename);
+ qemu_fdt_setprop_string(ms->fdt, nodename, "compatible",
+ "snps,designware-i2c");
+ qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
+ 2, base, 2, size);
+ qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
+ GIC_FDT_IRQ_TYPE_SPI, irq,
+ GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+
+ if (s && s->bus) {
+ at24c_eeprom_init(s->bus, 0x50, 256);
+ } else {
+ fprintf(stderr, "Warning: DW I2C created but bus not available\n");
+ }
+
+ g_free(nodename);
+}
+
static void create_rtc(const VirtMachineState *vms)
{
char *nodename;
@@ -2493,6 +2527,8 @@ static void machvirt_init(MachineState *machine)
create_uart(vms, VIRT_UART1, secure_sysmem, serial_hd(1), true);
}
+ create_i2c(vms, VIRT_I2C);
+
if (vms->secure) {
create_secure_ram(vms, secure_sysmem, secure_tag_sysmem);
}
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 8694aaa4e2..911beea7fd 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -75,6 +75,7 @@ enum {
VIRT_PLATFORM_BUS,
VIRT_GPIO,
VIRT_UART1,
+ VIRT_I2C,
VIRT_SECURE_MEM,
VIRT_SECURE_GPIO,
VIRT_PCDIMM_ACPI,
--
2.43.0On Tue, 6 Jan 2026 21:12:53 +0800
AlanoSong@163.com wrote:
> From: Alano Song <AlanoSong@163.com>
>
> Add DesignWare I2C controller onto virt board,
> and also an at24c eeprom for r/w operation.
>
> Add these two devices into arm virt acpi table.
>
> Confirmed with i2c-tools under v6.18 linux driver.
>
Hi Alano,
> Signed-off-by: Alano Song <AlanoSong@163.com>
Perhaps a silly question but why do you want this on arm/virt?
I've been carrying a backed up version of the aspeed i2c but for
that we are using it with MCTP (I'm guessing this one isn't capable
enough) and devices on that are inherently discoverable unlike
normal I2C devices. Even so I don't plan to upstream that as for
the CXL fabric stuff I can use MCTP over USB instead and don't
need to change arm/virt at all.
I'm not sure how useful an eeprom is beyond verifying your control emulation,
but perhaps that's all that is intended?
> ---
> hw/arm/Kconfig | 1 +
> hw/arm/virt-acpi-build.c | 32 ++++++++++++++++++++++++++++++++
> hw/arm/virt.c | 38 +++++++++++++++++++++++++++++++++++++-
> include/hw/arm/virt.h | 1 +
> 4 files changed, 71 insertions(+), 1 deletion(-)
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 03b4342574..3d06356169 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -100,6 +100,34 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> aml_append(scope, dev);
> }
>
An example of the AML blob would be useful in the patch description (or
the one updating the ACPI test tables).
I'd also expect a lot of bios test failures given you didn't change the
data files. Run make check-qtest and see what blow up.
> +static void acpi_dsdt_add_i2c(Aml *scope, const MemMapEntry *i2c_memmap,
> + uint32_t i2c_irq)
> +{
> + Aml *i2c_dev, *eprm_dev, *crs;
> +
> + i2c_dev = aml_device("I2C0");
> + aml_append(i2c_dev, aml_name_decl("_HID", aml_string("INT3433")));
That seems to be a valid intel PNP ID, but please add a reference to where it
came from (I'll guess the kernel driver rather than some document?)
> + aml_append(i2c_dev, aml_name_decl("_UID", aml_int(0)));
> +
> + crs = aml_resource_template();
> + aml_append(crs, aml_memory32_fixed(i2c_memmap->base,
> + i2c_memmap->size, AML_READ_WRITE));
> + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> + AML_EXCLUSIVE, &i2c_irq, 1));
> + aml_append(i2c_dev, aml_name_decl("_CRS", crs));
> +
> + eprm_dev = aml_device("EPRM");
> + aml_append(eprm_dev, aml_name_decl("_HID", aml_string("INT3499")));
Likewise, a reference for this PNP format ACPI ID would be good.
> + aml_append(eprm_dev, aml_name_decl("_UID", aml_int(0)));
> +
> + crs = aml_resource_template();
> + aml_append(crs, aml_i2c_serial_bus_device(0x50, "^"));
This is the bit that made me ask for a blob in the patch description.
I have very little idea what that actually does in AML :( (the "^"
in particular)
> + aml_append(eprm_dev, aml_name_decl("_CRS", crs));
> +
> + aml_append(i2c_dev, eprm_dev);
> + aml_append(scope, i2c_dev);
> +}
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index fd0e28f030..8fd37126d1 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -38,6 +38,8 @@
> #include "hw/arm/boot.h"
> #include "hw/arm/primecell.h"
> #include "hw/arm/virt.h"
> +#include "hw/i2c/dw_i2c.h"
> +#include "hw/nvram/eeprom_at24c.h"
> #include "hw/arm/machines-qom.h"
> #include "hw/block/flash.h"
> #include "hw/display/ramfb.h"
> @@ -193,7 +195,8 @@ static const MemMapEntry base_memmap[] = {
> [VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN},
> [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
> [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
> - [VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
> + [VIRT_I2C] = { 0x090c0000, 0x00001000 },
> + [VIRT_ACPI_PCIHP] = { 0x090d0000, ACPI_PCIHP_SIZE },
This breaks VM migration. You need to put your new memory in a hole
in the memory map, not move things.
> [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
> [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
> @@ -245,6 +248,7 @@ static const int a15irqmap[] = {
> [VIRT_GPIO] = 7,
> [VIRT_UART1] = 8,
> [VIRT_ACPI_GED] = 9,
> + [VIRT_I2C] = 10,
> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
> [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
> [VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
> @@ -1016,6 +1020,36 @@ static void create_uart(const VirtMachineState *vms, int uart,
> g_free(nodename);
> }
>
> +static void create_i2c(const VirtMachineState *vms, int i2c)
> +{
> + char *nodename = NULL;
Always overridden so don't set initial value. Maybe use a g_autofree
to avoid having to tidy it up on exit from the function.
> + hwaddr base = vms->memmap[i2c].base;
> + hwaddr size = vms->memmap[i2c].size;
> + int irq = vms->irqmap[i2c];
> + MachineState *ms = MACHINE(vms);
> + DeviceState *dev = sysbus_create_simple(TYPE_DW_I2C, base,
> + qdev_get_gpio_in(vms->gic, irq));
> + DWI2CState *s = DW_I2C(dev);
> +
> + nodename = g_strdup_printf("/dw-i2c@%" PRIx64, base);
> + qemu_fdt_add_subnode(ms->fdt, nodename);
> + qemu_fdt_setprop_string(ms->fdt, nodename, "compatible",
> + "snps,designware-i2c");
> + qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
> + 2, base, 2, size);
> + qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
> + GIC_FDT_IRQ_TYPE_SPI, irq,
> + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> +
> + if (s && s->bus) {
> + at24c_eeprom_init(s->bus, 0x50, 256);
> + } else {
> + fprintf(stderr, "Warning: DW I2C created but bus not available\n");
> + }
> +
> + g_free(nodename);
> +}
At 2026-01-06 23:45:22, "Jonathan Cameron via" <qemu-devel@nongnu.org> wrote:
>On Tue, 6 Jan 2026 21:12:53 +0800
>AlanoSong@163.com wrote:
>
>> From: Alano Song <AlanoSong@163.com>
>>
>> Add DesignWare I2C controller onto virt board,
>> and also an at24c eeprom for r/w operation.
>>
>> Add these two devices into arm virt acpi table.
>>
>> Confirmed with i2c-tools under v6.18 linux driver.
>>
>Hi Alano,
>
>> Signed-off-by: Alano Song <AlanoSong@163.com>
>
>Perhaps a silly question but why do you want this on arm/virt?
>
>I've been carrying a backed up version of the aspeed i2c but for
>that we are using it with MCTP (I'm guessing this one isn't capable
>enough) and devices on that are inherently discoverable unlike
>normal I2C devices. Even so I don't plan to upstream that as for
>the CXL fabric stuff I can use MCTP over USB instead and don't
>need to change arm/virt at all.
>
Cause we are emulating our soc chip on qemu, and our first choice of
machine board is arm/virt.
But we unfortunately found there is no DesignWare I2C model and no
I2C device in arm/virt.
Someone else may encounter this problem as well, so I decide to solve it.
>I'm not sure how useful an eeprom is beyond verifying your control emulation,
>but perhaps that's all that is intended?
>
>
Yes you are right, single I2C controller cannot be verified, so I
add an eeprom to work with it.
>> ---
>> hw/arm/Kconfig | 1 +
>> hw/arm/virt-acpi-build.c | 32 ++++++++++++++++++++++++++++++++
>> hw/arm/virt.c | 38 +++++++++++++++++++++++++++++++++++++-
>> include/hw/arm/virt.h | 1 +
>> 4 files changed, 71 insertions(+), 1 deletion(-)
>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 03b4342574..3d06356169 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -100,6 +100,34 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>> aml_append(scope, dev);
>> }
>>
>
>An example of the AML blob would be useful in the patch description (or
>the one updating the ACPI test tables).
>I'd also expect a lot of bios test failures given you didn't change the
>data files. Run make check-qtest and see what blow up.
>
Your suggestion is quiet right, I will add comment of ACPI table in
patch v2 description.
And I make check-qtest, you are right, I will fix the test failure leaded by
this patch in patch v2.
>> +static void acpi_dsdt_add_i2c(Aml *scope, const MemMapEntry *i2c_memmap,
>> + uint32_t i2c_irq)
>> +{
>> + Aml *i2c_dev, *eprm_dev, *crs;
>> +
>> + i2c_dev = aml_device("I2C0");
>> + aml_append(i2c_dev, aml_name_decl("_HID", aml_string("INT3433")));
>
>That seems to be a valid intel PNP ID, but please add a reference to where it
>came from (I'll guess the kernel driver rather than some document?)
>
Yes, you are right, this PNP ID comes from kernel
driver(i2c-designware-platdrv.c/dw_i2c_acpi_match)
it's just for testing convenience under linux kernel.
I will add comment for it.
>> + aml_append(i2c_dev, aml_name_decl("_UID", aml_int(0)));
>> +
>> + crs = aml_resource_template();
>> + aml_append(crs, aml_memory32_fixed(i2c_memmap->base,
>> + i2c_memmap->size, AML_READ_WRITE));
>> + aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
>> + AML_EXCLUSIVE, &i2c_irq, 1));
>> + aml_append(i2c_dev, aml_name_decl("_CRS", crs));
>> +
>> + eprm_dev = aml_device("EPRM");
>> + aml_append(eprm_dev, aml_name_decl("_HID", aml_string("INT3499")));
>
>Likewise, a reference for this PNP format ACPI ID would be good.
>
Okay, I will add comment for it :)
>> + aml_append(eprm_dev, aml_name_decl("_UID", aml_int(0)));
>> +
>> + crs = aml_resource_template();
>> + aml_append(crs, aml_i2c_serial_bus_device(0x50, "^"));
>
>This is the bit that made me ask for a blob in the patch description.
>I have very little idea what that actually does in AML :( (the "^"
>in particular)
>
Here, "^" represents the father device of eeprom ("EPRM"),
just the I2C controller device ("I2C0").
I will add description for it in patch v2.
>> + aml_append(eprm_dev, aml_name_decl("_CRS", crs));
>> +
>> + aml_append(i2c_dev, eprm_dev);
>> + aml_append(scope, i2c_dev);
>> +}
>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index fd0e28f030..8fd37126d1 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -38,6 +38,8 @@
>> #include "hw/arm/boot.h"
>> #include "hw/arm/primecell.h"
>> #include "hw/arm/virt.h"
>> +#include "hw/i2c/dw_i2c.h"
>> +#include "hw/nvram/eeprom_at24c.h"
>> #include "hw/arm/machines-qom.h"
>> #include "hw/block/flash.h"
>> #include "hw/display/ramfb.h"
>> @@ -193,7 +195,8 @@ static const MemMapEntry base_memmap[] = {
>> [VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN},
>> [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
>> [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
>> - [VIRT_ACPI_PCIHP] = { 0x090c0000, ACPI_PCIHP_SIZE },
>> + [VIRT_I2C] = { 0x090c0000, 0x00001000 },
>> + [VIRT_ACPI_PCIHP] = { 0x090d0000, ACPI_PCIHP_SIZE },
>
>This breaks VM migration. You need to put your new memory in a hole
>in the memory map, not move things.
>
Okay, I will refine this part code :)
>> [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
>> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>> [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
>> @@ -245,6 +248,7 @@ static const int a15irqmap[] = {
>> [VIRT_GPIO] = 7,
>> [VIRT_UART1] = 8,
>> [VIRT_ACPI_GED] = 9,
>> + [VIRT_I2C] = 10,
>> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>> [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
>> [VIRT_SMMU] = 74, /* ...to 74 + NUM_SMMU_IRQS - 1 */
>> @@ -1016,6 +1020,36 @@ static void create_uart(const VirtMachineState *vms, int uart,
>> g_free(nodename);
>> }
>>
>> +static void create_i2c(const VirtMachineState *vms, int i2c)
>> +{
>> + char *nodename = NULL;
>
>Always overridden so don't set initial value. Maybe use a g_autofree
>to avoid having to tidy it up on exit from the function.
>
Thanks for your advice, I will refine this part code :)
>> + hwaddr base = vms->memmap[i2c].base;
>> + hwaddr size = vms->memmap[i2c].size;
>> + int irq = vms->irqmap[i2c];
>> + MachineState *ms = MACHINE(vms);
>> + DeviceState *dev = sysbus_create_simple(TYPE_DW_I2C, base,
>> + qdev_get_gpio_in(vms->gic, irq));
>> + DWI2CState *s = DW_I2C(dev);
>> +
>> + nodename = g_strdup_printf("/dw-i2c@%" PRIx64, base);
>> + qemu_fdt_add_subnode(ms->fdt, nodename);
>> + qemu_fdt_setprop_string(ms->fdt, nodename, "compatible",
>> + "snps,designware-i2c");
>> + qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
>> + 2, base, 2, size);
>> + qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
>> + GIC_FDT_IRQ_TYPE_SPI, irq,
>> + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
>> +
>> + if (s && s->bus) {
>> + at24c_eeprom_init(s->bus, 0x50, 256);
>> + } else {
>> + fprintf(stderr, "Warning: DW I2C created but bus not available\n");
>> + }
>> +
>> + g_free(nodename);
>> +}
>
On Wed, 7 Jan 2026 15:07:10 +0800 (CST)
"Alano Song" <alanosong@163.com> wrote:
> At 2026-01-06 23:45:22, "Jonathan Cameron via" <qemu-devel@nongnu.org> wrote:
> >On Tue, 6 Jan 2026 21:12:53 +0800
> >AlanoSong@163.com wrote:
> >
> >> From: Alano Song <AlanoSong@163.com>
> >>
> >> Add DesignWare I2C controller onto virt board,
> >> and also an at24c eeprom for r/w operation.
> >>
> >> Add these two devices into arm virt acpi table.
> >>
> >> Confirmed with i2c-tools under v6.18 linux driver.
> >>
> >Hi Alano,
> >
> >> Signed-off-by: Alano Song <AlanoSong@163.com>
> >
> >Perhaps a silly question but why do you want this on arm/virt?
> >
> >I've been carrying a backed up version of the aspeed i2c but for
> >that we are using it with MCTP (I'm guessing this one isn't capable
> >enough) and devices on that are inherently discoverable unlike
> >normal I2C devices. Even so I don't plan to upstream that as for
> >the CXL fabric stuff I can use MCTP over USB instead and don't
> >need to change arm/virt at all.
> >
>
>
> Cause we are emulating our soc chip on qemu, and our first choice of
> machine board is arm/virt.
Are you planning to ultimately upstream support for emulating your SoC?
If you do, is a new emulated board perhaps appropriate?
> But we unfortunately found there is no DesignWare I2C model and no
> I2C device in arm/virt.
>
> Someone else may encounter this problem as well, so I decide to solve it.
It's definitely good to have emulation of this fairly common IP.
>
>
> >I'm not sure how useful an eeprom is beyond verifying your control emulation,
> >but perhaps that's all that is intended?
> >
>
> >
>
>
> Yes you are right, single I2C controller cannot be verified, so I
> add an eeprom to work with it.
Within arm virt one concern is that this unconditionally enabled
so attacks on emulation code are a real possibility (in VM usecases)
>
>
> >> ---
> >> +static void acpi_dsdt_add_i2c(Aml *scope, const MemMapEntry *i2c_memmap,
> >> + uint32_t i2c_irq)
> >> +{
> >> + Aml *i2c_dev, *eprm_dev, *crs;
> >> +
> >> + i2c_dev = aml_device("I2C0");
> >> + aml_append(i2c_dev, aml_name_decl("_HID", aml_string("INT3433")));
> >
> >That seems to be a valid intel PNP ID, but please add a reference to where it
> >came from (I'll guess the kernel driver rather than some document?)
>
> >
>
>
> Yes, you are right, this PNP ID comes from kernel
> driver(i2c-designware-platdrv.c/dw_i2c_acpi_match)
> it's just for testing convenience under linux kernel.
If you are ultimately emulating a real SoC I'd suggest using
an ID issued by they vendor of the board or the SoC. They will need
to have a suitable ACPI or PNP ID though (and so be a member of
UEFI.org) That will allow for any quirks in the kernel driver
(and you'd need to emulate them as well). Who knows what slight
differences there might be with the Intel version of the IP.
>
>
> >> + aml_append(eprm_dev, aml_name_decl("_UID", aml_int(0)));
> >> +
> >> + crs = aml_resource_template();
> >> + aml_append(crs, aml_i2c_serial_bus_device(0x50, "^"));
> >
> >This is the bit that made me ask for a blob in the patch description.
> >I have very little idea what that actually does in AML :( (the "^"
> >in particular)
>
> >
>
>
> Here, "^" represents the father device of eeprom ("EPRM"),
> just the I2C controller device ("I2C0").
> I will add description for it in patch v2.
Ah. Ok. I didn't know about that and couldn't find it in the ACPI
spec :( A more specific reference like you suggest is fine.
Jonathan
© 2016 - 2026 Red Hat, Inc.