| Message ID | 20230331010131.1412571-1-komlodi@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | i3c: aspeed: Add I3C support | expand |
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? 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. > 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 > >
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 >> >> >
Hi Cédric, > > 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. [...] > 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. The differences (at least from the view of the current Linux driver implementation) are very minor; unless we want to be errata-compatible, you could use the dw driver directly, plus the ast2600-specific global register space. Cheers, Jeremy
Hi all, On Sun, Apr 2, 2023 at 1:11 AM Jeremy Kerr <jk@codeconstruct.com.au> wrote: > > Hi Cédric, > > > > 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. > > [...] > > > 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. > > The differences (at least from the view of the current Linux driver > implementation) are very minor; unless we want to be errata-compatible, > you could use the dw driver directly, plus the ast2600-specific global > register space. > This is my understanding as well from an outside look. From a QEMU standpoint I could split off the dwc portion into a dwc_i3c model, which the aspeed_i3c portion inherits from. I can do that in a v2 if that sounds good with everyone. Thanks, Joe > Cheers, > > > Jeremy
Hi Joe, > > > Jeremy, how different is it ? Could we introduce properties or sub > > > classes, to support both. > > > > The differences (at least from the view of the current Linux driver > > implementation) are very minor; unless we want to be errata-compatible, > > you could use the dw driver directly, plus the ast2600-specific global > > register space. > > > > This is my understanding as well from an outside look. > From a QEMU standpoint I could split off the dwc portion into a > dwc_i3c model, which the aspeed_i3c portion inherits from. I can do > that in a v2 if that sounds good with everyone. I'm not a qemu maintainer, but for the record: I'm fine with the current approach. I don't have access to any of the non-aspeed dw documentation, so verifying what should go into the dw model vs. what is ast2600-specific has been a bit tricky. If someone needs a non-aspeed dw model, and has a bit of documentation about the underlying dw hardware, it should be easy enough to split back out. Maybe just make sure any "known" divergences - like the IBI PEC behaviour - are well commented. That said, if you're keen to do the split dw+aspeed models, that's also good :) Cheers, Jeremy
On Wed, 5 Apr 2023 at 02:07, Jeremy Kerr <jk@codeconstruct.com.au> wrote: > > Hi Joe, > > > > > Jeremy, how different is it ? Could we introduce properties or sub > > > > classes, to support both. > > > > > > The differences (at least from the view of the current Linux driver > > > implementation) are very minor; unless we want to be errata-compatible, > > > you could use the dw driver directly, plus the ast2600-specific global > > > register space. > > > > > > > This is my understanding as well from an outside look. > > From a QEMU standpoint I could split off the dwc portion into a > > dwc_i3c model, which the aspeed_i3c portion inherits from. I can do > > that in a v2 if that sounds good with everyone. > > I'm not a qemu maintainer, but for the record: I'm fine with the current > approach. I don't have access to any of the non-aspeed dw documentation, > so verifying what should go into the dw model vs. what is > ast2600-specific has been a bit tricky. > > If someone needs a non-aspeed dw model, and has a bit of documentation > about the underlying dw hardware, it should be easy enough to split back > out. Maybe just make sure any "known" divergences - like the IBI PEC > behaviour - are well commented. > > That said, if you're keen to do the split dw+aspeed models, that's also > good :) I think the split makes sense. It will help other platforms who have the designware part add it to their system in the future. Joe, when you send out the patches again please be sure to use get_maintianers.pl to send it to the appropriate reviewers. Thanks for sharing your work! Cheers, Joel