On Nov 16 16:58, Cédric Le Goater wrote: > On 11/16/22 09:43, Klaus Jensen wrote: > > From: Klaus Jensen > > > > It is not given that the current master will release the bus after a > > transfer ends. Only schedule a pending master if the bus is idle. > > > > Fixes: 37fa5ca42623 ("hw/i2c: support multiple masters") > > Signed-off-by: Klaus Jensen > > --- > > hw/i2c/aspeed_i2c.c | 2 ++ > > hw/i2c/core.c | 37 ++++++++++++++++++++++--------------- > > include/hw/i2c/i2c.h | 2 ++ > > 3 files changed, 26 insertions(+), 15 deletions(-) > > > > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c > > index c166fd20fa11..1f071a3811f7 100644 > > --- a/hw/i2c/aspeed_i2c.c > > +++ b/hw/i2c/aspeed_i2c.c > > @@ -550,6 +550,8 @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) > > } > > SHARED_ARRAY_FIELD_DP32(bus->regs, reg_cmd, M_STOP_CMD, 0); > > aspeed_i2c_set_state(bus, I2CD_IDLE); > > + > > + i2c_schedule_pending_master(bus->bus); > > Shouldn't it be i2c_bus_release() ? > The reason for having both i2c_bus_release() and i2c_schedule_pending_master() is that i2c_bus_release() sort of pairs with i2c_bus_master(). They either set or clear the bus->bh member. In the current design, the controller (in this case the Aspeed I2C) is an "implicit" master (it does not have a bottom half driving it), so there is no bus->bh to clear. I should (and will) write some documentation on the asynchronous API.