From: Sven Peter via B4 Relay <devnull+sven.svenpeter.dev@kernel.org>
To: Janne Grunau <j@jannau.net>,
Alyssa Rosenzweig <alyssa@rosenzweig.io>,
Madhavan Srinivasan <maddy@linux.ibm.com>,
Michael Ellerman <mpe@ellerman.id.au>,
Nicholas Piggin <npiggin@gmail.com>,
Christophe Leroy <christophe.leroy@csgroup.eu>,
Naveen N Rao <naveen@kernel.org>,
Andi Shyti <andi.shyti@kernel.org>, Neal Gompa <neal@gompa.dev>,
Hector Martin <marcan@marcan.st>
Cc: linuxppc-dev@lists.ozlabs.org, asahi@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org,
linux-kernel@vger.kernel.org, Sven Peter <sven@svenpeter.dev>
Subject: [PATCH v2 4/6] i2c: pasemi: Improve error recovery
Date: Tue, 15 Apr 2025 15:36:58 +0000 [thread overview]
Message-ID: <20250415-pasemi-fixes-v2-4-c543bf53151a@svenpeter.dev> (raw)
In-Reply-To: <20250415-pasemi-fixes-v2-0-c543bf53151a@svenpeter.dev>
From: Hector Martin <marcan@marcan.st>
The hardware (supposedly) has a 25ms timeout for clock stretching
and the driver uses 100ms which should be plenty. The error
reocvery itself is however lacking.
Add handling for all the missing error condition, and better recovery in
pasemi_smb_clear(). Also move the timeout to a #define since it's used
in multiple places now.
Signed-off-by: Hector Martin <marcan@marcan.st>
[sven: adjusted commit message after splitting the commit into two]
Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
drivers/i2c/busses/i2c-pasemi-core.c | 75 +++++++++++++++++++++++++++++++-----
1 file changed, 65 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/i2c-pasemi-core.c b/drivers/i2c/busses/i2c-pasemi-core.c
index 9b611dbdfef23e78a4ea75ac0311938d52b6ba96..2f31f039bedfd7f78d6572fe925125b1a6d0724b 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -9,6 +9,7 @@
#include <linux/delay.h>
#include <linux/i2c.h>
#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
@@ -52,6 +53,12 @@
#define CTL_UJM BIT(8)
#define CTL_CLK_M GENMASK(7, 0)
+/*
+ * The hardware (supposedly) has a 25ms timeout for clock stretching, thus
+ * use 100ms here which should be plenty.
+ */
+#define TRANSFER_TIMEOUT_MS 100
+
static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
{
dev_dbg(smbus->dev, "smbus write reg %x val %08x\n", reg, val);
@@ -80,24 +87,44 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
reinit_completion(&smbus->irq_completion);
}
-static void pasemi_smb_clear(struct pasemi_smbus *smbus)
+static int pasemi_smb_clear(struct pasemi_smbus *smbus)
{
unsigned int status;
+ int ret;
- status = reg_read(smbus, REG_SMSTA);
+ /* First wait for the bus to go idle */
+ ret = readx_poll_timeout(ioread32, smbus->ioaddr + REG_SMSTA,
+ status, !(status & (SMSTA_XIP | SMSTA_JAM)),
+ USEC_PER_MSEC, USEC_PER_MSEC * TRANSFER_TIMEOUT_MS);
+
+ if (ret < 0) {
+ dev_err(smbus->dev, "Bus is still stuck (status 0x%08x)\n", status);
+ return -EIO;
+ }
+
+ /* If any badness happened or there is data in the FIFOs, reset the FIFOs */
+ if ((status & (SMSTA_MRNE | SMSTA_JMD | SMSTA_MTO | SMSTA_TOM | SMSTA_MTN | SMSTA_MTA)) ||
+ !(status & SMSTA_MTE))
+ pasemi_reset(smbus);
+
+ /* Clear the flags */
reg_write(smbus, REG_SMSTA, status);
+
+ return 0;
}
static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
{
- int timeout = 100;
+ int timeout = TRANSFER_TIMEOUT_MS;
int ret;
unsigned int status;
if (smbus->use_irq) {
reinit_completion(&smbus->irq_completion);
- reg_write(smbus, REG_IMASK, SMSTA_XEN | SMSTA_MTN);
- ret = wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(100));
+ /* XEN should be set when a transaction terminates, whether due to error or not */
+ reg_write(smbus, REG_IMASK, SMSTA_XEN);
+ ret = wait_for_completion_timeout(&smbus->irq_completion,
+ msecs_to_jiffies(timeout));
reg_write(smbus, REG_IMASK, 0);
status = reg_read(smbus, REG_SMSTA);
@@ -123,9 +150,35 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
}
}
+ /* Controller timeout? */
+ if (status & SMSTA_TOM) {
+ dev_warn(smbus->dev, "Controller timeout, status 0x%08x\n", status);
+ return -EIO;
+ }
+
+ /* Peripheral timeout? */
+ if (status & SMSTA_MTO) {
+ dev_warn(smbus->dev, "Peripheral timeout, status 0x%08x\n", status);
+ return -ETIME;
+ }
+
+ /* Still stuck in a transaction? */
+ if (status & SMSTA_XIP) {
+ dev_warn(smbus->dev, "Bus stuck, status 0x%08x\n", status);
+ return -EIO;
+ }
+
+ /* Arbitration loss? */
+ if (status & SMSTA_MTA) {
+ dev_warn(smbus->dev, "Arbitration loss, status 0x%08x\n", status);
+ return -EBUSY;
+ }
+
/* Got NACK? */
- if (status & SMSTA_MTN)
+ if (status & SMSTA_MTN) {
+ dev_warn(smbus->dev, "NACK, status 0x%08x\n", status);
return -ENXIO;
+ }
/* Clear XEN */
reg_write(smbus, REG_SMSTA, SMSTA_XEN);
@@ -187,9 +240,9 @@ static int pasemi_i2c_xfer(struct i2c_adapter *adapter,
struct pasemi_smbus *smbus = adapter->algo_data;
int ret, i;
- pasemi_smb_clear(smbus);
-
- ret = 0;
+ ret = pasemi_smb_clear(smbus);
+ if (ret)
+ return ret;
for (i = 0; i < num && !ret; i++)
ret = pasemi_i2c_xfer_msg(adapter, &msgs[i], (i == (num - 1)));
@@ -210,7 +263,9 @@ static int pasemi_smb_xfer(struct i2c_adapter *adapter,
addr <<= 1;
read_flag = read_write == I2C_SMBUS_READ;
- pasemi_smb_clear(smbus);
+ err = pasemi_smb_clear(smbus);
+ if (err)
+ return err;
switch (size) {
case I2C_SMBUS_QUICK:
--
2.34.1
next prev parent reply other threads:[~2025-04-15 15:37 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-15 15:36 [PATCH v2 0/6] Apple/PASemi i2c error recovery fixes Sven Peter via B4 Relay
2025-04-15 15:36 ` [PATCH v2 1/6] i2c: pasemi: Use correct bits.h include Sven Peter via B4 Relay
2025-04-15 17:10 ` Alyssa Rosenzweig
2025-04-15 15:36 ` [PATCH v2 2/6] i2c: pasemi: Sort includes alphabetically Sven Peter via B4 Relay
2025-04-15 17:11 ` Alyssa Rosenzweig
2025-04-15 15:36 ` [PATCH v2 3/6] i2c: pasemi: Improve timeout handling Sven Peter via B4 Relay
2025-04-15 17:11 ` Alyssa Rosenzweig
2025-04-17 12:57 ` Andi Shyti
2025-04-15 15:36 ` Sven Peter via B4 Relay [this message]
2025-04-15 17:12 ` [PATCH v2 4/6] i2c: pasemi: Improve error recovery Alyssa Rosenzweig
2025-04-17 13:07 ` Andi Shyti
2025-04-27 11:29 ` Sven Peter
2025-04-15 15:36 ` [PATCH v2 5/6] i2c: pasemi: Enable the unjam machine Sven Peter via B4 Relay
2025-04-15 15:37 ` [PATCH v2 6/6] i2c: pasemi: Log bus reset causes Sven Peter via B4 Relay
2025-04-17 13:10 ` Andi Shyti
2025-04-16 1:38 ` [PATCH v2 0/6] Apple/PASemi i2c error recovery fixes Neal Gompa
2025-04-17 13:16 ` Andi Shyti
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=20250415-pasemi-fixes-v2-4-c543bf53151a@svenpeter.dev \
--to=devnull+sven.svenpeter.dev@kernel.org \
--cc=alyssa@rosenzweig.io \
--cc=andi.shyti@kernel.org \
--cc=asahi@lists.linux.dev \
--cc=christophe.leroy@csgroup.eu \
--cc=j@jannau.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=maddy@linux.ibm.com \
--cc=marcan@marcan.st \
--cc=mpe@ellerman.id.au \
--cc=naveen@kernel.org \
--cc=neal@gompa.dev \
--cc=npiggin@gmail.com \
--cc=sven@svenpeter.dev \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox