Re: [PATCH 00/16] i3c: aspeed: Add I3C support

[Cédric Le Goater]

Hello,

On 4/1/23 19:28, Ben Dooks wrote:
> On Fri, Mar 31, 2023 at 01:01:15AM +0000, Joe Komlodi wrote:
> > Hi all,
> >
> > This series adds I3C bus support to QEMU and adds more functionality to the
> > Aspeed I3C controller.
> >
> > This implementation is a basic implementation that introduces IBIs
> > (including hot-join), CCCs, and SDR data transfer. As-is, it doesnt support
> > multi-controller buses or HDR transfers.
> >
> > First we add the I3C bus and controller model. With that added we
> > gradually extend the functionality of the Aspeed I3C controller so it
> > can do transfers.
> >
> > With that added, we add 2 targets. The first target is a mock I3C
> > target. It's intended to be a very simple target just to verify that I3C
> > is working on the guest. Internally, we've used it on Linux to verify
> > that i3C devices can be probed and can send/receive data and send IBIs.
> >
> > The second target is a remote target. The intention of this is to be
> > able to communicate to a target that exists outside of QEMU.
> >
> > Lastly we add hotplugging support. The hotplugging doesn't do anything too
> > complicated, it just adds the device attempting to hotplug to the bus. It is
> > the device's responsibility to hot-join and go through the DAA process to
> > participate on the bus.
> >
> > Thanks!
> > Joe
> >
> > Joe Komlodi (16):
> >    hw/misc/aspeed_i3c: Move to i3c directory
> >    hw/i3c: Add bus support
> >    hw/i3c/aspeed_i3c: Add more register fields
> >    hw/i3c/aspeed_i3c: Add more reset values
> >    hw/i3c/aspeed_i3c: Add register RO field masks
> >    hw/i3c/aspeed_i3c: Treat more registers as read-as-zero
> >    hw/i3c/aspeed_i3c: Use 32 bits on MMIO writes
> >    hw/i3c/aspeed_i3c: Add IRQ MMIO behavior
> >    hw/i3c/aspeed_i3c: Add data TX and RX
> >    hw/i3c/aspeed_i3c: Add IBI handling
> >    hw/i3c/aspeed_i3c: Add ctrl MMIO handling
> >    hw/i3c/aspeed_i3c: Add controller resets
> >    hw/i3c: Add Mock target
> >    hw/i3c: remote_i3c: Add model
> >    qtest: remote_i3c: Add remote I3C qtest
> >    hw/i3c: Add hotplug support
>
> Isn't this the designware i3c ip block, and as such could we name
> it so? 

Currently, QEMU only has a model for a dummy Aspeed I3C variant so this is a
great addition.

> I was going to send an i2c only version of this but it seems you've beaten me 
> to it and got the i3c core going.

I gave the model a try adding a sensor on the legacy bus of an ast2600-evb
machine :

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

Looks good. Next step would be to add some real I3C device model.


According to recent work on the kernel, it is indeed based on designware I3C :

  https://lore.kernel.org/all/20230331091501.3800299-1-jk@codeconstruct.com.au/

Jeremy, how different is it ? Could we introduce properties or sub classes,
to support both.


Thanks,


C.

> >   hw/Kconfig                    |    1 +
> >   hw/arm/Kconfig                |    2 +
> >   hw/i3c/Kconfig                |   17 +
> >   hw/i3c/aspeed_i3c.c           | 2044 +++++++++++++++++++++++++++++++++
> >   hw/i3c/core.c                 |  646 +++++++++++
> >   hw/i3c/meson.build            |    6 +
> >   hw/i3c/mock-target.c          |  314 +++++
> >   hw/i3c/remote-i3c.c           |  469 ++++++++
> >   hw/i3c/trace-events           |   52 +
> >   hw/i3c/trace.h                |    1 +
> >   hw/meson.build                |    1 +
> >   hw/misc/aspeed_i3c.c          |  384 -------
> >   hw/misc/meson.build           |    1 -
> >   hw/misc/trace-events          |    6 -
> >   include/hw/arm/aspeed_soc.h   |    2 +-
> >   include/hw/i3c/aspeed_i3c.h   |  207 ++++
> >   include/hw/i3c/i3c.h          |  275 +++++
> >   include/hw/i3c/mock-target.h  |   60 +
> >   include/hw/i3c/remote-i3c.h   |   72 ++
> >   include/hw/misc/aspeed_i3c.h  |   48 -
> >   meson.build                   |    1 +
> >   tests/qtest/meson.build       |    1 +
> >   tests/qtest/remote-i3c-test.c |  610 ++++++++++
> >   23 files changed, 4780 insertions(+), 440 deletions(-)
> >   create mode 100644 hw/i3c/Kconfig
> >   create mode 100644 hw/i3c/aspeed_i3c.c
> >   create mode 100644 hw/i3c/core.c
> >   create mode 100644 hw/i3c/meson.build
> >   create mode 100644 hw/i3c/mock-target.c
> >   create mode 100644 hw/i3c/remote-i3c.c
> >   create mode 100644 hw/i3c/trace-events
> >   create mode 100644 hw/i3c/trace.h
> >   delete mode 100644 hw/misc/aspeed_i3c.c
> >   create mode 100644 include/hw/i3c/aspeed_i3c.h
> >   create mode 100644 include/hw/i3c/i3c.h
> >   create mode 100644 include/hw/i3c/mock-target.h
> >   create mode 100644 include/hw/i3c/remote-i3c.h
> >   delete mode 100644 include/hw/misc/aspeed_i3c.h
> >   create mode 100644 tests/qtest/remote-i3c-test.c
> >
> > --
> > 2.40.0.348.gf938b09366-goog