Skip to contentSkip to conversation

i2c: octeon: thunderx: Recovery fixes and improvements

PATCH In review Jan Glauber 3 participants 6 messages Sep 23, 2016 – Sep 26, 2016

Subsystems (2)
I2C subsystem host drivers Cavium I2C driver
lore
Share & tools
lore

Jan Glauber

author
#0 lore raw

v2 cover i2c: octeon: thunderx: Recovery fixes and improvements

Hi Wolfram,

here are the remaining two patches with iopoll.h usage and a timed wait before entering the recovery on a failed bus check.

thanks,
Jan

---------------------

Jan Glauber (2):
i2c: octeon: thunderx: Check bus state before starting a transaction
i2c: octeon: thunderx: Limit register access retries

 drivers/i2c/busses/i2c-octeon-core.c | 33 ++++++++++++++++++++++++++++++++-
 drivers/i2c/busses/i2c-octeon-core.h | 27 ++++++++++++++++-----------
 2 files changed, 48 insertions(+), 12 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v2 1/2 i2c: octeon: thunderx: Check bus state before starting a transaction

Add an additional status check before starting a transaction. If the check fails wait for some time to tolerate multi-master mode. After the timeout expires trigger the recovery.

Signed-off-by Jan Glauber <[email protected]>
---
 drivers/i2c/busses/i2c-octeon-core.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
drivers/i2c/busses/i2c-octeon-core.c +29 -0
index 5e63b17..f526511 100644
@@ -630,6 +630,31 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
 	return ret;
 }
 
+static int octeon_i2c_check_bus(struct octeon_i2c *i2c)
+{
+	u64 end = get_jiffies_64() + i2c->adap.timeout;
+	int stat, lines;
+
+	while (time_before64(get_jiffies_64(), end)) {
+		stat = octeon_i2c_stat_read(i2c);
+
+		/* get I2C line state */
+		lines = octeon_i2c_read_int(i2c) & (TWSI_INT_SCL | TWSI_INT_SDA);
+
+		if (stat == STAT_IDLE && lines == (TWSI_INT_SCL | TWSI_INT_SDA))
+			return 0;
+
+		if (stat == STAT_LOST_ARB_38 || stat == STAT_LOST_ARB_68 ||
+		    stat == STAT_LOST_ARB_78 || stat == STAT_LOST_ARB_B0)
+			break;
+
+		usleep_range(I2C_OCTEON_EVENT_WAIT / 2, I2C_OCTEON_EVENT_WAIT);
+	}
+
+	/* bus check failed, try to recover */
+	return octeon_i2c_recovery(i2c);
+}
+
 /**
  * octeon_i2c_xfer - The driver's master_xfer function
  * @adap: Pointer to the i2c_adapter structure

@@ -643,6 +668,10 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	struct octeon_i2c *i2c = i2c_get_adapdata(adap);
 	int i, ret = 0;
 
+	ret = octeon_i2c_check_bus(i2c);
+	if (ret)
+		goto out;
+
 	if (num == 1) {
 		if (msgs[0].len > 0 && msgs[0].len <= 8) {
 			if (msgs[0].flags & I2C_M_RD)
Signature · 1 line
1.9.1
#2 lore raw

Re: [PATCH v2 1/2] i2c: octeon: thunderx: Check bus state before starting a transaction

Quoted 5 lines · Jan Glauber

Add an additional status check before starting a transaction. If the check fails wait for some time to tolerate multi-master mode. After the timeout expires trigger the recovery.

Signed-off-by Jan Glauber <[email protected]>

Need to think more about it, needs to wait for next cycle.

#3 lore raw

Re: [PATCH v2 1/2] i2c: octeon: thunderx: Check bus state before starting a transaction

Quoted 9 lines · Wolfram Sang
Quoted 5 lines · Jan Glauber

Add an additional status check before starting a transaction. If the check fails wait for some time to tolerate multi-master mode. After the timeout expires trigger the recovery.

Signed-off-by Jan Glauber <[email protected]>

Need to think more about it, needs to wait for next cycle.

OK, please share your thoughts when you get to it.

--Jan

author
#4 lore raw

PATCH v2 2/2 i2c: octeon: thunderx: Limit register access retries

In reply to Jan Glauber

Do not infinitely retry register readq and writeq operations in order to not lock up the CPU in case the TWSI gets stuck.

Return -ETIMEDOUT in case of a failed data read. For all other cases just return so subsequent operations will fail and trigger the recovery.

Signed-off-by Jan Glauber <[email protected]>
---
 drivers/i2c/busses/i2c-octeon-core.c |  4 +++-
 drivers/i2c/busses/i2c-octeon-core.h | 27 ++++++++++++++++-----------
 2 files changed, 19 insertions(+), 12 deletions(-)
drivers/i2c/busses/i2c-octeon-core.c +3 -1
index f526511..1dfc4b8 100644
@@ -381,7 +381,9 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target,
 		if (result)
 			return result;
 
-		data[i] = octeon_i2c_data_read(i2c);
+		data[i] = octeon_i2c_data_read(i2c, &result);
+		if (result)
+			return result;
 		if (recv_len && i == 0) {
 			if (data[i] > I2C_SMBUS_BLOCK_MAX + 1)
 				return -EPROTO;
drivers/i2c/busses/i2c-octeon-core.h +16 -11
index 87151ea..1db7c83 100644
@@ -5,6 +5,7 @@
 #include <linux/i2c.h>
 #include <linux/i2c-smbus.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/pci.h>
 

@@ -144,9 +145,9 @@ static inline void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8
 	u64 tmp;
 
 	__raw_writeq(SW_TWSI_V | eop_reg | data, i2c->twsi_base + SW_TWSI(i2c));
-	do {
-		tmp = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
-	} while ((tmp & SW_TWSI_V) != 0);
+
+	readq_poll_timeout(i2c->twsi_base + SW_TWSI(i2c), tmp, tmp & SW_TWSI_V,
+			   I2C_OCTEON_EVENT_WAIT, i2c->adap.timeout);
 }
 
 #define octeon_i2c_ctl_write(i2c, val)					\

@@ -163,24 +164,28 @@ static inline void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8
  *
  * The I2C core registers are accessed indirectly via the SW_TWSI CSR.
  */
-static inline u8 octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg)
+static inline int octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg,
+				      int *error)
 {
 	u64 tmp;
+	int ret;
 
 	__raw_writeq(SW_TWSI_V | eop_reg | SW_TWSI_R, i2c->twsi_base + SW_TWSI(i2c));
-	do {
-		tmp = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
-	} while ((tmp & SW_TWSI_V) != 0);
 
+	ret = readq_poll_timeout(i2c->twsi_base + SW_TWSI(i2c), tmp,
+				 tmp & SW_TWSI_V, I2C_OCTEON_EVENT_WAIT,
+				 i2c->adap.timeout);
+	if (error)
+		*error = ret;
 	return tmp & 0xFF;
 }
 
 #define octeon_i2c_ctl_read(i2c)					\
-	octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_CTL)
-#define octeon_i2c_data_read(i2c)					\
-	octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_DATA)
+	octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_CTL, NULL)
+#define octeon_i2c_data_read(i2c, error)				\
+	octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_DATA, error)
 #define octeon_i2c_stat_read(i2c)					\
-	octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_STAT)
+	octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_STAT, NULL)
 
 /**
  * octeon_i2c_read_int - read the TWSI_INT register
Signature · 1 line
1.9.1
#5 lore raw

Re: [PATCH v2 2/2] i2c: octeon: thunderx: Limit register access retries

Quoted 8 lines · Jan Glauber

Do not infinitely retry register readq and writeq operations in order to not lock up the CPU in case the TWSI gets stuck.

Return -ETIMEDOUT in case of a failed data read. For all other cases just return so subsequent operations will fail and trigger the recovery.

Signed-off-by Jan Glauber <[email protected]>

Deleted the last 4 words from commit message (previous recovery patch needs more discussion) and applied to for-next, thanks!