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

From: John Keeping <john@metanate.com>
To: Ondrej Jirman <megous@megous.com>
Cc: Heiko Stuebner <heiko@sntech.de>, Wolfram Sang <wsa@kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	"moderated list:ARM/Rockchip SoC support"
	<linux-arm-kernel@lists.infradead.org>,
	 "open list:ARM/Rockchip SoC support"
	<linux-rockchip@lists.infradead.org>,
	"open list:I2C SUBSYSTEM HOST DRIVERS"
	<linux-i2c@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag
Date: Thu, 18 Nov 2021 18:37:33 +0000	[thread overview]
Message-ID: <YZadbTw0q4NA2XWs@donbot> (raw)
In-Reply-To: <20210924111528.2924251-1-megous@megous.com>

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)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel