Re: [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag

John Keeping
2021-11-18T18:38:29Z

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@megous.com>
> ---

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@metanate.com>

> 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)