On Fri, Jul 15, 2022 at 09:49:58AM +0200, Klaus Jensen wrote: > On Jul 14 20:06, Peter Delevoryas wrote: > > Hey Cedric, Klaus, and Corey, > > > > Hi Peter, > > Regardless of the issues you are facing its awesome to see this being > put to work like this!
Haha yeah, well, _all_ the designs at Meta (fb) rely significantly on multi-master i2c. I think I've been trying to get this working for months now, but we're really close! If I can just get the i2c layer working, then proper IPMB and MCTP testing between BMC and BIC firmware will be much easier. There's some part defects that have a very low frequency of occurrence, and the patches for those defects rely on a BMC <-> BIC <-> <device> chain of IPMB messages. With QEMU, we could test those patches much more thoroughly, because we can inject the part-defect behavior. > > > So I realized something about the current state of multi-master i2c: > > > > We can't do transfers between two Aspeed I2C controllers, e.g. AST1030 <-> > > AST2600. I'm looking into this case in the new fby35 machine (which isn't > > even > > merged yet, just in Cedric's pull request) > > > > This is because the AspeedI2CBusSlave is only designed to receive through > > i2c_send_async(). But the AspeedI2CBus master-mode transfers use i2c_send(). > > > > So, the AST2600 can't send data to the AST1030. And the AST1030 can't reply > > to > > the AST2600. > > > > (By the way, another small issue: AspeedI2CBusSlave expects the parent of > > its > > parent to be its AspeedI2CBus, but that's not true if multiple SoC's are > > sharing > > an I2CBus. But that's easy to resolve, I'll send a patch for that soon). > > > > I'm wondering how best to resolve the multi-SoC send-async issue, while > > retaining the ability to send synchronously to non-SoC slave devices. > > > > I think there's only one way, as far as I can see: > > > > - Force the Aspeed I2C Controller to master the I2C bus before starting a > > master > > transfer. Even for synchronous transfers. > > > > This shouldn't be a big problem, we can still do synchronous transfers, we > > just > > have to wait for the bus to be free before starting the transfer. > > > > - If the I2C slave targets for a master2slave transfer support async_send, > > then > > use async_send. This requires refactoring aspeed_i2c_bus_send into a state > > machine to send data asynchronously. > > > > In other words, don't try to do a synchronous transfer to an SoC. > > > > But, of course, we can keep doing synchronous transfers from SoC -> sensor > > or > > sensor -> SoC. > > > > Yeah, hmm. This is tricky because callers of bus_send expects the > transfer to be "resolved" immediately. Per design, the asynchronous send > requires the device mastering the bus to itself be asynchronous (like > the i2c-echo device I added as an example). Understood: I was ommitting other necessary changes. Yes, we would need to async-ify all the way up the chain to the register read/write. > > However, looking at aspeed_i2c_bus_handle_cmd (which is the caller of > bus_send), it should be possible to accept bus_send to "yield" as you > sketch below and not raise any interrupt. And yes, it would be required > in bus_send to call i2c_bus_master to register a BH which can then > raise the interrupt upon i2c_ack(). Yep, that's what I was thinking of. I think I would actually call i2c_bus_master in aspeed_i2c_bus_handle_cmd or higher though, because I would only call i2c_bus_master once until the STOP command is issued (or the DMA/pool transfer is complete). But yeah, I think we're on the same page.
