Re: [PATCH maybe-7.2 1/3] hw/i2c: only schedule pending master when bus is idle

[Klaus Jensen <its@irrelevant.dk>]

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

On Nov 16 16:58, Cédric Le Goater wrote:
> On 11/16/22 09:43, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@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.jensen@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.

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