Skip to content

Commit 41ebeb9

Browse files
marcanjannau
authored andcommitted
i2c: pasemi: Improve timeout handling and error recovery
The hardware (supposedly) has a 25ms timeout for clock stretching, but the driver uses a 10ms timeout, which is too low (and actually gets hit with the tipd controllers on Apple Silicon machines sporadically). Increase the timeout to 100ms, which should be plenty, and then add handling for all the missing error condition, and better recovery in pasemi_smb_clear(). Since this needs a bunch more bit defines, take the change to switch to bitfield.h macros, which is much more readable. Signed-off-by: Hector Martin <marcan@marcan.st>
1 parent 1dd9433 commit 41ebeb9

1 file changed

Lines changed: 54 additions & 12 deletions

File tree

drivers/i2c/busses/i2c-pasemi-core.c

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@
5252
#define CTL_UJM BIT(8)
5353
#define CTL_CLK_M GENMASK(7, 0)
5454

55+
#define TRANSFER_TIMEOUT_MS 100
56+
5557
static inline void reg_write(struct pasemi_smbus *smbus, int reg, int val)
5658
{
5759
dev_dbg(smbus->dev, "smbus write reg %x val %08x\n", reg, val);
@@ -80,23 +82,45 @@ static void pasemi_reset(struct pasemi_smbus *smbus)
8082
reinit_completion(&smbus->irq_completion);
8183
}
8284

83-
static void pasemi_smb_clear(struct pasemi_smbus *smbus)
85+
static int pasemi_smb_clear(struct pasemi_smbus *smbus)
8486
{
8587
unsigned int status;
88+
int timeout = TRANSFER_TIMEOUT_MS;
8689

8790
status = reg_read(smbus, REG_SMSTA);
91+
92+
/* First wait for the bus to go idle */
93+
while ((status & (SMSTA_XIP | SMSTA_JAM)) && timeout--) {
94+
msleep(1);
95+
status = reg_read(smbus, REG_SMSTA);
96+
}
97+
98+
if (timeout < 0) {
99+
dev_warn(smbus->dev, "Bus is still stuck (status 0x%08x)\n", status);
100+
return -EIO;
101+
}
102+
103+
/* If any badness happened or there is data in the FIFOs, reset the FIFOs */
104+
if ((status & (SMSTA_MRNE | SMSTA_JMD | SMSTA_MTO | SMSTA_TOM | SMSTA_MTN | SMSTA_MTA)) ||
105+
!(status & SMSTA_MTE))
106+
pasemi_reset(smbus);
107+
108+
/* Clear the flags */
88109
reg_write(smbus, REG_SMSTA, status);
110+
111+
return 0;
89112
}
90113

91114
static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
92115
{
93-
int timeout = 100;
116+
int timeout = TRANSFER_TIMEOUT_MS;
94117
unsigned int status;
95118

96119
if (smbus->use_irq) {
97120
reinit_completion(&smbus->irq_completion);
98-
reg_write(smbus, REG_IMASK, SMSTA_XEN | SMSTA_MTN);
99-
wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(100));
121+
/* XEN should be set when a transaction terminates, whether due to error or not */
122+
reg_write(smbus, REG_IMASK, SMSTA_XEN);
123+
wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(timeout));
100124
reg_write(smbus, REG_IMASK, 0);
101125
status = reg_read(smbus, REG_SMSTA);
102126
} else {
@@ -107,16 +131,32 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
107131
}
108132
}
109133

110-
/* Got NACK? */
111-
if (status & SMSTA_MTN)
112-
return -ENXIO;
134+
/* Controller timeout? */
135+
if (status & SMSTA_TOM) {
136+
dev_warn(smbus->dev, "Controller timeout, status 0x%08x\n", status);
137+
return -EIO;
138+
}
113139

114-
if (timeout < 0) {
115-
dev_warn(smbus->dev, "Timeout, status 0x%08x\n", status);
116-
reg_write(smbus, REG_SMSTA, status);
140+
/* Peripheral timeout? */
141+
if (status & SMSTA_MTO) {
142+
dev_warn(smbus->dev, "Peripheral timeout, status 0x%08x\n", status);
117143
return -ETIME;
118144
}
119145

146+
/* Still stuck in a transaction? */
147+
if (status & SMSTA_XIP) {
148+
dev_warn(smbus->dev, "Bus stuck, status 0x%08x\n", status);
149+
return -EIO;
150+
}
151+
152+
/* Arbitration loss? */
153+
if (status & SMSTA_MTA)
154+
return -EBUSY;
155+
156+
/* Got NACK? */
157+
if (status & SMSTA_MTN)
158+
return -ENXIO;
159+
120160
/* Clear XEN */
121161
reg_write(smbus, REG_SMSTA, SMSTA_XEN);
122162

@@ -177,7 +217,8 @@ static int pasemi_i2c_xfer(struct i2c_adapter *adapter,
177217
struct pasemi_smbus *smbus = adapter->algo_data;
178218
int ret, i;
179219

180-
pasemi_smb_clear(smbus);
220+
if (pasemi_smb_clear(smbus))
221+
return -EIO;
181222

182223
ret = 0;
183224

@@ -200,7 +241,8 @@ static int pasemi_smb_xfer(struct i2c_adapter *adapter,
200241
addr <<= 1;
201242
read_flag = read_write == I2C_SMBUS_READ;
202243

203-
pasemi_smb_clear(smbus);
244+
if (pasemi_smb_clear(smbus))
245+
return -EIO;
204246

205247
switch (size) {
206248
case I2C_SMBUS_QUICK:

0 commit comments

Comments
 (0)