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

From: Klaus Jensen <its@irrelevant.dk>
To: Corey Minyard <minyard@acm.org>
Cc: "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>,
	qemu-devel@nongnu.org, "Jeremy Kerr" <jk@ozlabs.org>,
	qemu-arm@nongnu.org, "Cédric Le Goater" <clg@kaod.org>,
	"Padmakar Kalghatgi" <p.kalghatgi@samsung.com>,
	"Matt Johnston" <matt@codeconstruct.com.au>,
	"Joel Stanley" <joel@jms.id.au>
Subject: Re: [RFC PATCH 0/4] hw/i2c: i2c slave mode support
Date: Fri, 1 Apr 2022 08:29:03 +0200	[thread overview]
Message-ID: <Ykabr0wQe9VLISu2@apples> (raw)
In-Reply-To: <20220331203205.GB29333@minyard.net>

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

On Mar 31 15:32, Corey Minyard wrote:
> On Thu, Mar 31, 2022 at 06:57:33PM +0200, Klaus Jensen 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.
> 
> I've been wondering when this would happen :).  I had put some thought
> into how this would work, but hadn't come up with anything good.
> 
> The big disadvantage of this is you are adding an interface that is
> incompatible with the current masters and slaves.  So you are using the
> same I2C bus, but slaves written this way cannot talk to existing
> masters, and masters written this way cannot talk to existing slave.
> You could adapt the masters to be able to work either way, and I suppose
> some slaves that could do it could have both an async send and a normal
> send. 

Would it make sense to introduce a QOM Interface to differentiate
between the slave/master types?

> But you could not adapt a slave device for the Aspeed to do both.

Exactly, the Aspeed must be able to defer the ack, so it cannot
implement send(). Even if it buffered up the write, I don't think it
would be correct to Ack the transfer until the host has Acked it.

> But that said, I don't know of a better way to handle this.
> 
> You don't have the ability to nack a byte in what you have currently.
> That's probably something that will be needed.

True. Didn't consider that. Since the ack is basically defined as the
scheduling of the bh, I guess I have to come up with something where I
can also pass a "return value".

> 
> This is obviously not something useful by itself.  How do you plan to
> tie this in to something else that would use it?
> 

This is specifically for implementing an NVMe-MI device which uses MCTP
transactions (in which both requests and replies are master->slave
transfers). I just wanted to get a feel for how you maintaines would
envision this begin done before posting that. The NVMe-MI device will
function exactly like the example i2c echo device (i.e. receive an MCTP
transaction using the normal i2c slave interface, parse the
transaction/request, master the bus and start a new transfer).

Thanks for your comments Corey!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]