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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YZadbTw0q4NA2XWs@donbot>
Date:   Thu, 18 Nov 2021 18:37:33 +0000
From:   John Keeping <john@...anate.com>
To:     Ondrej Jirman <megous@...ous.com>
Cc:     Heiko Stuebner <heiko@...ech.de>, Wolfram Sang <wsa@...nel.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        "moderated list:ARM/Rockchip SoC support" 
        <linux-arm-kernel@...ts.infradead.org>,
        "open list:ARM/Rockchip SoC support" 
        <linux-rockchip@...ts.infradead.org>,
        "open list:I2C SUBSYSTEM HOST DRIVERS" <linux-i2c@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH] i2c: rk3x: Handle a spurious start completion
 interrupt flag

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@...ous.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@...anate.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)