* [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag
@ 2021-09-24 11:15 Ondrej Jirman
2021-11-18 18:37 ` John Keeping
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ondrej Jirman @ 2021-09-24 11:15 UTC (permalink / raw)
To: Heiko Stuebner, Wolfram Sang, Krzysztof Kozlowski, Ondrej Jirman,
moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support,
open list:I2C SUBSYSTEM HOST DRIVERS, open list
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>
---
Re-sending, because originally, maintainers script did not pick up rockchip
mailing list, to send this patch to.
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)
--
2.33.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag
2021-09-24 11:15 [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag Ondrej Jirman
@ 2021-11-18 18:37 ` John Keeping
2021-11-29 9:37 ` Wolfram Sang
2021-11-30 21:37 ` Wolfram Sang
2 siblings, 0 replies; 6+ messages in thread
From: John Keeping @ 2021-11-18 18:37 UTC (permalink / raw)
To: Ondrej Jirman
Cc: Heiko Stuebner, Wolfram Sang, Krzysztof Kozlowski,
moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support,
open list:I2C SUBSYSTEM HOST DRIVERS, open list
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag
2021-09-24 11:15 [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag Ondrej Jirman
2021-11-18 18:37 ` John Keeping
@ 2021-11-29 9:37 ` Wolfram Sang
2021-11-30 15:54 ` Ondřej Jirman
2021-11-30 21:37 ` Wolfram Sang
2 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2021-11-29 9:37 UTC (permalink / raw)
To: Ondrej Jirman
Cc: Heiko Stuebner, Krzysztof Kozlowski,
moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support,
open list:I2C SUBSYSTEM HOST DRIVERS, open list
[-- Attachment #1.1: Type: text/plain, Size: 327 bytes --]
> This causes I2C transfer being aborted in polled mode from a stop completion
> handler:
I wonder why this only happens in polling mode? The question behind that
is: is it really a spurious irq from the HW or is it maybe a race in the
driver? Because polling uses the same interrupt handler but just
periodically polls it.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag
2021-11-29 9:37 ` Wolfram Sang
@ 2021-11-30 15:54 ` Ondřej Jirman
2021-11-30 21:36 ` Wolfram Sang
0 siblings, 1 reply; 6+ messages in thread
From: Ondřej Jirman @ 2021-11-30 15:54 UTC (permalink / raw)
To: Wolfram Sang, Heiko Stuebner, Krzysztof Kozlowski,
moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support,
open list:I2C SUBSYSTEM HOST DRIVERS, open list
On Mon, Nov 29, 2021 at 10:37:43AM +0100, Wolfram Sang wrote:
>
> > This causes I2C transfer being aborted in polled mode from a stop completion
> > handler:
>
> I wonder why this only happens in polling mode? The question behind that
> is: is it really a spurious irq from the HW or is it maybe a race in the
> driver? Because polling uses the same interrupt handler but just
> periodically polls it.
Spurious interupt happens in both modes.
Abort only happens in polled mode, becase polling executes the irq handler
before the stop condition interrupt fires. In that case rk3x_i2c_irq will
execute rk3x_i2c_handle_stop, which will terminate the transfer, becasuse
it's executed without STOP completion interrupt flag being set.
In interrupt mode, the interrupt handler alwyas executes after the stop
condition, so the spurious START interrupt is ignored in this case.
kind regards,
o.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag
2021-11-30 15:54 ` Ondřej Jirman
@ 2021-11-30 21:36 ` Wolfram Sang
0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2021-11-30 21:36 UTC (permalink / raw)
To: Ondřej Jirman, Heiko Stuebner, Krzysztof Kozlowski,
moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support,
open list:I2C SUBSYSTEM HOST DRIVERS, open list
[-- Attachment #1.1: Type: text/plain, Size: 83 bytes --]
> Spurious interupt happens in both modes.
Thanks for the detailed explanation!
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag
2021-09-24 11:15 [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag Ondrej Jirman
2021-11-18 18:37 ` John Keeping
2021-11-29 9:37 ` Wolfram Sang
@ 2021-11-30 21:37 ` Wolfram Sang
2 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2021-11-30 21:37 UTC (permalink / raw)
To: Ondrej Jirman
Cc: Heiko Stuebner, Krzysztof Kozlowski,
moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support,
open list:I2C SUBSYSTEM HOST DRIVERS, open list
[-- Attachment #1.1: Type: text/plain, Size: 1087 bytes --]
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>
Applied to for-current, thanks!
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-30 21:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24 11:15 [RESEND PATCH] i2c: rk3x: Handle a spurious start completion interrupt flag Ondrej Jirman
2021-11-18 18:37 ` John Keeping
2021-11-29 9:37 ` Wolfram Sang
2021-11-30 15:54 ` Ondřej Jirman
2021-11-30 21:36 ` Wolfram Sang
2021-11-30 21:37 ` Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).