Re: [RFC PATCH 0/4] hw/i2c: i2c slave mode support

[Jonathan Cameron]

On Thu, 31 Mar 2022 18:57:33 +0200
Klaus Jensen <its@irrelevant.dk> wrote:

> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Hi all,
> 
> This RFC series adds I2C "slave mode" support for the Aspeed I2C
> controller as well as the necessary infrastructure in the i2c core to
> support this.
> 
> Background
> ~~~~~~~~~~
> We are working on an emulated NVM Express Management Interface[1] for
> testing and validation purposes. NVMe-MI is based on the MCTP
> protocol[2] which may use a variety of underlying transports. The one we
> are interested in is I2C[3].
> 
> The first general trickery here is that all MCTP transactions are based
> on the SMBus Block Write bus protocol[4]. This means that the slave must
> be able to master the bus to communicate. As you know, hw/i2c/core.c
> currently does not support this use case.
> 
> The second issue is how to interact with these mastering devices. Jeremy
> and Matt (CC'ed) have been working on an MCTP stack for the Linux Kernel
> (already upstream) and an I2C binding driver[5] is currently under
> review. This binding driver relies on I2C slave mode support in the I2C
> controller.

Hi Klaus,

Just thought I'd mention I'm also interested in MCTP over I2C emulation
for a couple of projects:

1) DMTF SPDM - mostly as a second transport for the kernel stack alongside
   PCI DOE.
2) CXL FM-API - adding support for the Fabric Manager interfaces
   on emulated CXL switches which is also typically carried over
   MCTP.

I was thinking of emulating a MCTP over PCI VDM but this has saved me
going to the effort of doing that for now at least :)

I have hacked a really really basic MCTP device together using this
series and it all seems to be working with the kernel stack (subject to a
few kernel driver bugs that I'll report / send fixes for next week).
I'm cheating all over the place so far, (lots of hard coded values) but
would be interested in a more flexible solution that might perhaps
share infrastructure with your NVMe-MI work.

Thanks,

Jonathan


> 
> This series
> ~~~~~~~~~~~
> Patch 1 adds support for multiple masters in the i2c core, allowing
> slaves to master the bus and safely issue i2c_send/recv(). Patch 2 adds
> an asynchronous send i2c_send_async(I2CBus *, uint8) on the bus that
> must be paired with an explicit ack using i2c_ack(I2CBus *).
> 
> Patch 3 adds the slave mode functionality to the emulated Aspeed I2C
> controller. The implementation is probably buggy since I had to rely on
> the implementation of the kernel driver to reverse engineer the behavior
> of the controller slave mode (I do not have access to a spec sheet for
> the Aspeed, but maybe someone can help me out with that?).
> 
> Finally, patch 4 adds an example device using this new API. The device
> is a simple "echo" device that upon being sent a set of bytes uses the
> first byte as the address of the slave to echo to.
> 
> With this combined I am able to boot up Linux on an emulated Aspeed 2600
> evaluation board and have the i2c echo device write into a Linux slave
> EEPROM. Assuming the echo device is on address 0x42:
> 
>   # echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-15/new_device
>   i2c i2c-15: new_device: Instantiated device slave-24c02 at 0x64
>   # i2cset -y 15 0x42 0x64 0x00 0xaa i
>   # hexdump /sys/bus/i2c/devices/15-1064/slave-eeprom
>   0000000 ffaa ffff ffff ffff ffff ffff ffff ffff
>   0000010 ffff ffff ffff ffff ffff ffff ffff ffff
>   *
>   0000100
> 
>   [1]: https://nvmexpress.org/developers/nvme-mi-specification/
>   [2]: 
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0236_1.3.1.pdf
>   [3]: 
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0237_1.2.0.pdf
>   [4]: http://www.smbus.org/specs/SMBus_3_1_20180319.pdf
>   [5]: 
> https://lore.kernel.org/linux-i2c/20220218055106.1944485-1-matt@codeconstruct.com.au/
> 
> Klaus Jensen (4):
>   hw/i2c: support multiple masters
>   hw/i2c: add async send
>   hw/i2c: add slave mode for aspeed_i2c
>   hw/misc: add a toy i2c echo device
> 
>  hw/i2c/aspeed_i2c.c         |  95 +++++++++++++++++++++---
>  hw/i2c/core.c               |  57 +++++++++++++-
>  hw/i2c/trace-events         |   2 +-
>  hw/misc/i2c-echo.c          | 144 ++++++++++++++++++++++++++++++++++++
>  hw/misc/meson.build         |   2 +
>  include/hw/i2c/aspeed_i2c.h |   8 ++
>  include/hw/i2c/i2c.h        |  19 +++++
>  7 files changed, 316 insertions(+), 11 deletions(-)
>  create mode 100644 hw/misc/i2c-echo.c
>