From: Peter Delevoryas <pdel@fb.com>
To: Klaus Jensen <its@irrelevant.dk>
Cc: "Cameron Esfahani via" <qemu-devel@nongnu.org>,
"Peter Maydell" <peter.maydell@linaro.org>,
"Arun Kumar Kashinath Agasar" <arun.kka@samsung.com>,
"Corey Minyard" <cminyard@mvista.com>,
"Andrew Jeffery" <andrew@aj.id.au>,
"Klaus Jensen" <k.jensen@samsung.com>,
"Cédric Le Goater" <clg@kaod.org>,
"qemu-arm@nongnu.org" <qemu-arm@nongnu.org>,
"Joel Stanley" <joel@jms.id.au>,
"Padmakar Kalghatgi" <p.kalghatgi@samsung.com>,
"Matt Johnston" <matt@codeconstruct.com.au>,
"Jeremy Kerr" <jk@ozlabs.org>
Subject: Re: [RFC PATCH 0/4] hw/i2c: i2c slave mode support
Date: Tue, 5 Apr 2022 20:52:32 +0000 [thread overview]
Message-ID: <CA9A7D12-EA42-450B-B378-92D53D3D22EF@fb.com> (raw)
In-Reply-To: <20220331165737.1073520-1-its@irrelevant.dk>
> On Mar 31, 2022, at 9:57 AM, 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.
This is great, I’m attempting to use your changes right now for the same thing (MCTP).
>
> 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.
>
> 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
When I try this with my system, it seems like the i2c-echo device takes over
the bus but never echoes the data to the EEPROM. Am I missing something to
make this work? It seems like the “i2c_send_async” calls aren’t happening,
which must be because the bottom half isn’t being scheduled, right? After
the i2c_do_start_transfer, how is the bottom half supposed to be scheduled
again? Is the slave receiving (the EEPROM) supposed to call i2c_ack or something?
root@bmc-oob:~# echo 24c02 0x1064 > /sys/bus/i2c/devices/i2c-8/new_device
[ 135.559719] at24 8-1064: 256 byte 24c02 EEPROM, writable, 1 bytes/write
[ 135.562661] i2c i2c-8: new_device: Instantiated device 24c02 at 0x64
root@bmc-oob:~# i2cset -y 8 0x42 0x64 0x00 0xaa i
i2c_echo_event: start send
i2c_echo_send: data[0] = 0x64
i2c_echo_send: data[1] = 0x00
i2c_echo_send: data[2] = 0xaa
i2c_echo_event: scheduling bottom-half
i2c_echo_bh: attempting to gain mastery of bus
i2c_echo_bh: starting a send to address 0x64
root@bmc-oob:~# hexdump -C /sys/bus/i2c/devices/8-1064/eeprom
00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00000100
Thanks again for this, it’s exactly what I needed.
>
> [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
>
> --
> 2.35.1
>
>
next prev parent reply other threads:[~2022-04-05 20:54 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-31 16:57 [RFC PATCH 0/4] hw/i2c: i2c slave mode support Klaus Jensen
2022-03-31 16:57 ` [RFC PATCH 1/4] hw/i2c: support multiple masters Klaus Jensen
2022-03-31 16:57 ` [RFC PATCH 2/4] hw/i2c: add async send Klaus Jensen
2022-03-31 16:57 ` [RFC PATCH 3/4] hw/i2c: add slave mode for aspeed_i2c Klaus Jensen
2022-03-31 20:44 ` Philippe Mathieu-Daudé
2022-04-01 6:30 ` Klaus Jensen
2022-04-06 6:14 ` Cédric Le Goater
2022-04-06 7:40 ` Klaus Jensen
2022-04-06 8:52 ` Cédric Le Goater
2022-04-06 9:16 ` Klaus Jensen
2022-04-06 9:44 ` Cédric Le Goater
2022-03-31 16:57 ` [RFC PATCH 4/4] hw/misc: add a toy i2c echo device Klaus Jensen
2022-03-31 20:32 ` [RFC PATCH 0/4] hw/i2c: i2c slave mode support Corey Minyard
2022-04-01 6:29 ` Klaus Jensen
2022-04-01 8:58 ` Damien Hedde
2022-04-01 9:05 ` Klaus Jensen
2022-04-01 13:06 ` Corey Minyard
2022-04-05 20:52 ` Peter Delevoryas [this message]
2022-04-06 6:07 ` Klaus Jensen
2022-04-06 17:03 ` Peter Delevoryas
2022-04-06 18:41 ` Klaus Jensen
2022-04-06 22:06 ` Peter Delevoryas
2022-05-06 14:07 ` Jonathan Cameron via
2022-05-06 16:49 ` Cédric Le Goater
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA9A7D12-EA42-450B-B378-92D53D3D22EF@fb.com \
--to=pdel@fb.com \
--cc=andrew@aj.id.au \
--cc=arun.kka@samsung.com \
--cc=clg@kaod.org \
--cc=cminyard@mvista.com \
--cc=its@irrelevant.dk \
--cc=jk@ozlabs.org \
--cc=joel@jms.id.au \
--cc=k.jensen@samsung.com \
--cc=matt@codeconstruct.com.au \
--cc=p.kalghatgi@samsung.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).