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

Joe Komlodi posted 16 patches 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230331010131.1412571-1-komlodi@google.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Cédric Le Goater" <clg@kaod.org>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Laurent Vivier <lvivier@redhat.com>
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
[PATCH 00/16] i3c: aspeed: Add I3C support
Posted by Joe Komlodi 1 year, 1 month ago
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

 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
Re: [PATCH 00/16] i3c: aspeed: Add I3C support
Posted by Ben Dooks 1 year, 1 month ago
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
> 
> 

-- 
Ben Dooks, ben@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.
Re: [PATCH 00/16] i3c: aspeed: Add I3C support
Posted by Cédric Le Goater 1 year, 1 month ago
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
>>
>>
>
Re: [PATCH 00/16] i3c: aspeed: Add I3C support
Posted by Jeremy Kerr 1 year, 1 month ago
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
Re: [PATCH 00/16] i3c: aspeed: Add I3C support
Posted by Joe Komlodi 1 year, 1 month ago
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
Re: [PATCH 00/16] i3c: aspeed: Add I3C support
Posted by Jeremy Kerr 1 year, 1 month ago
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
Re: [PATCH 00/16] i3c: aspeed: Add I3C support
Posted by Joel Stanley 1 year, 1 month ago
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