On Fri, Sep 24, 2021 at 01:15:27PM +0200, Ondrej Jirman wrote: > In a typical read transfer, start completion flag is being set after > read finishes (notice ipd bit 4 being set): > > trasnfer poll=0 > i2c start > rk3x-i2c fdd40000.i2c: IRQ: state 1, ipd: 10 > i2c read > rk3x-i2c fdd40000.i2c: IRQ: state 2, ipd: 1b > i2c stop > rk3x-i2c fdd40000.i2c: IRQ: state 4, ipd: 33 > > This causes I2C transfer being aborted in polled mode from a stop completion > handler: > > trasnfer poll=1 > i2c start > rk3x-i2c fdd40000.i2c: IRQ: state 1, ipd: 10 > i2c read > rk3x-i2c fdd40000.i2c: IRQ: state 2, ipd: 0 > rk3x-i2c fdd40000.i2c: IRQ: state 2, ipd: 1b > i2c stop > rk3x-i2c fdd40000.i2c: IRQ: state 4, ipd: 13 > i2c stop > rk3x-i2c fdd40000.i2c: unexpected irq in STOP: 0x10 > > Clearing the START flag after read fixes the issue without any obvious > side effects. > > This issue was dicovered on RK3566 when adding support for powering > off the RK817 PMIC. > > Signed-off-by: Ondrej Jirman <megous@xxxxxxxxxx> > --- I haven't seen the issue described here, so I can't test whether this fix works, but the explanation makes sense, so: Reviewed-by: John Keeping <john@xxxxxxxxxxxx> > drivers/i2c/busses/i2c-rk3x.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c > index 819ab4ee517e..02ddb237f69a 100644 > --- a/drivers/i2c/busses/i2c-rk3x.c > +++ b/drivers/i2c/busses/i2c-rk3x.c > @@ -423,8 +423,8 @@ static void rk3x_i2c_handle_read(struct rk3x_i2c *i2c, unsigned int ipd) > if (!(ipd & REG_INT_MBRF)) > return; > > - /* ack interrupt */ > - i2c_writel(i2c, REG_INT_MBRF, REG_IPD); > + /* ack interrupt (read also produces a spurious START flag, clear it too) */ > + i2c_writel(i2c, REG_INT_MBRF | REG_INT_START, REG_IPD); > > /* Can only handle a maximum of 32 bytes at a time */ > if (len > 32)
