From: Robert Hancock <robert.hancock@calian.com> To: Michal Simek <michal.simek@amd.com>, Andi Shyti <andi.shyti@kernel.org>, Shubhrajyoti Datta <shubhraj@xilinx.com>, Wolfram Sang <wsa@kernel.org>, Marek Vasut <marex@denx.de> Cc: linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, Robert Hancock <robert.hancock@calian.com> Subject: [PATCH] i2c: xiic: Don't try to handle more interrupt events after error Date: Tue, 6 Jun 2023 12:25:58 -0600 [thread overview] Message-ID: <20230606182558.1301413-1-robert.hancock@calian.com> (raw) In xiic_process, it is possible that error events such as arbitration lost or TX error can be raised in conjunction with other interrupt flags such as TX FIFO empty or bus not busy. Error events result in the controller being reset and the error returned to the calling request, but the function could potentially try to keep handling the other events, such as by writing more messages into the TX FIFO. Since the transaction has already failed, this is not helpful and will just cause issues. This problem has been present ever since: commit 7f9906bd7f72 ("i2c: xiic: Service all interrupts in isr") which allowed non-error events to be handled after errors, but became more obvious after: commit 743e227a8959 ("i2c: xiic: Defer xiic_wakeup() and __xiic_start_xfer() in xiic_process()") which reworked the code to add a WARN_ON which triggers if both the xfer_more and wakeup_req flags were set, since this combination is not supposed to happen, but was occurring in this scenario. Skip further interrupt handling after error flags are detected to avoid this problem. Fixes: 7f9906bd7f72 ("i2c: xiic: Service all interrupts in isr") Signed-off-by: Robert Hancock <robert.hancock@calian.com> --- drivers/i2c/busses/i2c-xiic.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 8a3d9817cb41..ee6edc963dea 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -721,6 +721,8 @@ static irqreturn_t xiic_process(int irq, void *dev_id) wakeup_req = 1; wakeup_code = STATE_ERROR; } + /* don't try to handle other events */ + goto out; } if (pend & XIIC_INTR_RX_FULL_MASK) { /* Receive register/FIFO is full */ -- 2.40.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Robert Hancock <robert.hancock@calian.com> To: Michal Simek <michal.simek@amd.com>, Andi Shyti <andi.shyti@kernel.org>, Shubhrajyoti Datta <shubhraj@xilinx.com>, Wolfram Sang <wsa@kernel.org>, Marek Vasut <marex@denx.de> Cc: linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, Robert Hancock <robert.hancock@calian.com> Subject: [PATCH] i2c: xiic: Don't try to handle more interrupt events after error Date: Tue, 6 Jun 2023 12:25:58 -0600 [thread overview] Message-ID: <20230606182558.1301413-1-robert.hancock@calian.com> (raw) In xiic_process, it is possible that error events such as arbitration lost or TX error can be raised in conjunction with other interrupt flags such as TX FIFO empty or bus not busy. Error events result in the controller being reset and the error returned to the calling request, but the function could potentially try to keep handling the other events, such as by writing more messages into the TX FIFO. Since the transaction has already failed, this is not helpful and will just cause issues. This problem has been present ever since: commit 7f9906bd7f72 ("i2c: xiic: Service all interrupts in isr") which allowed non-error events to be handled after errors, but became more obvious after: commit 743e227a8959 ("i2c: xiic: Defer xiic_wakeup() and __xiic_start_xfer() in xiic_process()") which reworked the code to add a WARN_ON which triggers if both the xfer_more and wakeup_req flags were set, since this combination is not supposed to happen, but was occurring in this scenario. Skip further interrupt handling after error flags are detected to avoid this problem. Fixes: 7f9906bd7f72 ("i2c: xiic: Service all interrupts in isr") Signed-off-by: Robert Hancock <robert.hancock@calian.com> --- drivers/i2c/busses/i2c-xiic.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 8a3d9817cb41..ee6edc963dea 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -721,6 +721,8 @@ static irqreturn_t xiic_process(int irq, void *dev_id) wakeup_req = 1; wakeup_code = STATE_ERROR; } + /* don't try to handle other events */ + goto out; } if (pend & XIIC_INTR_RX_FULL_MASK) { /* Receive register/FIFO is full */ -- 2.40.1
next reply other threads:[~2023-06-06 18:27 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-06-06 18:25 Robert Hancock [this message] 2023-06-06 18:25 ` [PATCH] i2c: xiic: Don't try to handle more interrupt events after error Robert Hancock 2023-06-06 19:24 ` Andi Shyti 2023-06-06 19:24 ` Andi Shyti 2023-06-06 19:40 ` Robert Hancock 2023-06-06 19:40 ` Robert Hancock 2023-06-06 21:20 ` Andi Shyti 2023-06-06 21:20 ` Andi Shyti 2023-06-09 15:32 ` wsa 2023-06-09 15:32 ` wsa 2023-06-09 21:25 ` Andi Shyti 2023-06-09 21:25 ` Andi Shyti 2023-06-27 16:13 ` Robert Hancock 2023-06-27 16:13 ` Robert Hancock 2023-06-27 21:25 ` wsa 2023-06-27 21:25 ` wsa 2023-07-06 19:33 ` Wolfram Sang 2023-07-06 19:33 ` Wolfram Sang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230606182558.1301413-1-robert.hancock@calian.com \ --to=robert.hancock@calian.com \ --cc=andi.shyti@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-i2c@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marex@denx.de \ --cc=michal.simek@amd.com \ --cc=shubhraj@xilinx.com \ --cc=wsa@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.