Re: [PATCH 2/2] hw/arm/virt: Add DesignWare I2C controller

[Jonathan Cameron via <qemu-arm@nongnu.org>]

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