On 11/17/22 07:40, Klaus Jensen wrote:
On Nov 16 16:58, Cédric Le Goater wrote:
On 11/16/22 09:43, Klaus Jensen wrote:
From: Klaus Jensen <k.jen...@samsung.com>
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 <k.jen...@samsung.com>
---
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.
I found the routine names confusing. Thanks for the clarification.
Maybe we could do this rename :
i2c_bus_release() -> i2c_bus_release_and_clear()
i2c_schedule_pending_master() -> i2c_bus_release()
and keep i2c_schedule_pending_master() internal the I2C core subsystem.
C.
