diff mbox

p8-i2c reset things manually in some error conditions

Message ID 1480645766-15080-1-git-send-email-stewart@linux.vnet.ibm.com
State Accepted
Headers show

Commit Message

Stewart Smith Dec. 2, 2016, 2:29 a.m. UTC
It appears that our reset code wasn't entirely correct, and what we're
meant to do is reset each port and wait for command complete. In the
event where that fails, we can then bitbang things to recover to a state
where at least the i2c engine isn't in a weird state.

Practically, this means that "i2cdetect -y 10; i2cdetect -y 10" (where 10
is the bus where a TPM is attached, typically p8e1p2) doesn't hard lock
the machine (things are still bad and you won't reboot successfully, but
it's *better*).

one downside to this patch is that we spend a *long* time in OPAL (tens
of ms) when doing the reset. This is something that we really need to fix,
as it's not at all nice. The full fix for this though will involve changing
a decent chunk of the p8-i2c code, as we don't want to write *any* registers
while doing this extended reset (while existing code checks status a bit
later).

Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
---
 hw/p8-i2c.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 157 insertions(+), 22 deletions(-)

Comments

Stewart Smith Dec. 2, 2016, 3:56 a.m. UTC | #1
Stewart Smith <stewart@linux.vnet.ibm.com> writes:
> It appears that our reset code wasn't entirely correct, and what we're
> meant to do is reset each port and wait for command complete. In the
> event where that fails, we can then bitbang things to recover to a state
> where at least the i2c engine isn't in a weird state.
>
> Practically, this means that "i2cdetect -y 10; i2cdetect -y 10" (where 10
> is the bus where a TPM is attached, typically p8e1p2) doesn't hard lock
> the machine (things are still bad and you won't reboot successfully, but
> it's *better*).
>
> one downside to this patch is that we spend a *long* time in OPAL (tens
> of ms) when doing the reset. This is something that we really need to fix,
> as it's not at all nice. The full fix for this though will involve changing
> a decent chunk of the p8-i2c code, as we don't want to write *any* registers
> while doing this extended reset (while existing code checks status a bit
> later).
>
> Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
> ---
>  hw/p8-i2c.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 157 insertions(+), 22 deletions(-)

Merged to master as of
and to 5.4.x as of 2924f7fd96564ed139927ee404699547c2350d5e
diff mbox

Patch

diff --git a/hw/p8-i2c.c b/hw/p8-i2c.c
index faaf6ede3373..221663596606 100644
--- a/hw/p8-i2c.c
+++ b/hw/p8-i2c.c
@@ -171,7 +171,11 @@  DEFINE_LOG_ENTRY(OPAL_RC_I2C_RESET, OPAL_INPUT_OUTPUT_ERR_EVT, OPAL_I2C,
 #define I2C_RESIDUAL_BACK_END		PPC_BITMASK(16, 31)
 
 /* Port busy register */
-#define I2C_PORT_BUYS_REG		0xe
+#define I2C_PORT_BUSY_REG		0xe
+#define I2C_SET_S_SCL_REG		0xd
+#define I2C_RESET_S_SCL_REG		0xf
+#define I2C_SET_S_SDA_REG		0x10
+#define I2C_RESET_S_SDA_REG		0x11
 
 enum p8_i2c_master_type {
 	I2C_POWER8,
@@ -207,6 +211,7 @@  struct p8_i2c_master {
 	struct timer		sensor_cache;
 	uint8_t			recovery_pass;
 	struct list_node	link;
+	struct list_head	ports;
 };
 
 struct p8_i2c_master_port {
@@ -214,6 +219,7 @@  struct p8_i2c_master_port {
 	struct p8_i2c_master	*master;
 	uint32_t		port_num;
 	uint32_t		bit_rate_div;	/* Divisor to set bus speed*/
+	struct list_node	link;
 };
 
 struct p8_i2c_request {
@@ -463,6 +469,153 @@  static void p8_i2c_translate_error(struct i2c_request *req, uint64_t status)
 		req->result = OPAL_I2C_TIMEOUT;
 }
 
+static void p8_i2c_force_reset(struct p8_i2c_master *master)
+{
+	struct p8_i2c_master_port *p;
+	uint64_t mode;
+	int rc;
+
+	/* Reset the i2c engine */
+	rc = xscom_write(master->chip_id, master->xscom_base +
+			 I2C_RESET_I2C_REG, 0);
+	if (rc) {
+		log_simple_error(&e_info(OPAL_RC_I2C_RESET), "I2C: Failed "
+				 "to reset the i2c engine\n");
+		return;
+	}
+	time_wait_us_nopoll(10);
+	/* Reset port busy */
+	rc = xscom_write(master->chip_id, master->xscom_base +
+			 I2C_PORT_BUSY_REG, 0x8000000000000000ULL);
+	if (rc) {
+		log_simple_error(&e_info(OPAL_RC_I2C_RESET), "I2C: Failed "
+				 "to reset port busy on i2c engine\n");
+		return;
+	}
+	time_wait_us_nopoll(10);
+	list_for_each(&master->ports, p, link) {
+		mode = 0;
+		mode = SETFIELD(I2C_MODE_PORT_NUM, mode, p->port_num);
+		mode = SETFIELD(I2C_MODE_BIT_RATE_DIV, mode, p->bit_rate_div);
+		mode |= I2C_MODE_DIAGNOSTIC;
+		rc = xscom_write(master->chip_id,
+				 master->xscom_base + I2C_MODE_REG,
+				 mode);
+		if (rc)
+			prlog(PR_ERR, "I2C: Failed to write the MODE_REG\n");
+
+		time_wait_us_nopoll(10);
+		rc = xscom_write(master->chip_id,
+				 master->xscom_base + I2C_RESET_S_SCL_REG,
+				 0);
+		if (rc)
+			prlog(PR_ERR, "I2C: Failed to reset S_SCL\n");
+
+		time_wait_us_nopoll(10);
+		rc = xscom_write(master->chip_id,
+				 master->xscom_base + I2C_SET_S_SCL_REG,
+				 0);
+		if (rc)
+			prlog(PR_ERR, "I2C: Failed to set S_SCL\n");
+
+		/* Manually reset */
+		time_wait_us_nopoll(10);
+		rc = xscom_write(master->chip_id,
+				 master->xscom_base + I2C_RESET_S_SCL_REG,
+				 0);
+		if (rc)
+			prlog(PR_ERR, "I2C: sendStop: fail reset S_SCL\n");
+
+		time_wait_us_nopoll(10);
+		rc = xscom_write(master->chip_id,
+				 master->xscom_base + I2C_RESET_S_SDA_REG,
+				 0);
+		if (rc)
+			prlog(PR_ERR, "I2C: sendStop: fail reset S_SDA\n");
+
+		time_wait_us_nopoll(10);
+		rc = xscom_write(master->chip_id,
+				 master->xscom_base + I2C_SET_S_SCL_REG,
+				 0);
+		if (rc)
+			prlog(PR_ERR, "I2C: sendStop: fail set S_SCL\n");
+
+		time_wait_us_nopoll(10);
+		rc = xscom_write(master->chip_id,
+				 master->xscom_base + I2C_SET_S_SDA_REG,
+				 0);
+		if (rc)
+			prlog(PR_ERR, "I2C: sendStop: fail set 2 S_SDA\n");
+
+		mode ^= I2C_MODE_DIAGNOSTIC;
+		time_wait_us_nopoll(10);
+		rc = xscom_write(master->chip_id,
+				 master->xscom_base + I2C_MODE_REG,
+				 mode);
+		if (rc)
+			prlog(PR_ERR, "I2C: Failed to write the MODE_REG\n");
+	}
+}
+
+static int p8_i2c_reset_engine(struct p8_i2c_master *master)
+{
+	struct p8_i2c_master_port *p;
+	int reset_loops;
+	int rc;
+	uint64_t status;
+
+	list_for_each(&master->ports, p, link) {
+		/*
+		 * Reset each port by issuing a STOP command to slave.
+		 *
+		 * Reprogram the mode register with 'enhanced bit' set
+		 */
+		rc = p8_i2c_prog_mode(p, true);
+		if (rc) {
+			log_simple_error(&e_info(OPAL_RC_I2C_RESET),
+					 "I2C: Failed to program the MODE_REG\n");
+			return -1;
+		}
+
+		/* Send an immediate stop */
+		master->state = state_error;
+		rc = xscom_write(master->chip_id, master->xscom_base +
+				 I2C_CMD_REG, I2C_CMD_WITH_STOP);
+		if (rc) {
+			log_simple_error(&e_info(OPAL_RC_I2C_RESET),
+					 "I2C: Failed to issue immediate STOP\n");
+			return -1;
+		}
+
+		/* Wait for COMMAND COMPLETE */
+		reset_loops = 0;
+		do {
+			rc = xscom_read(master->chip_id,
+					master->xscom_base + I2C_STAT_REG,
+					&status);
+			if (rc) {
+				log_simple_error(&e_info(OPAL_RC_I2C_TRANSFER),
+						 "I2C: Failed to read the STAT_REG\n");
+				return -1;
+			}
+			if (! (status & I2C_STAT_CMD_COMP)) {
+				time_wait_ms(10);
+				if (reset_loops++ == 5) {
+					prlog(PR_WARNING, "I2C: Retrying reset, with force!\n");
+					p8_i2c_force_reset(master);
+					continue;
+				}
+				if (reset_loops == 10) {
+					log_simple_error(&e_info(OPAL_RC_I2C_TRANSFER),
+							 "I2C: Failed to recover i2c engine\n");
+					break;
+				}
+			}
+		} while (! (status & I2C_STAT_CMD_COMP));
+	}
+	return 0;
+}
+
 static void p8_i2c_status_error(struct p8_i2c_master_port *port,
 				struct i2c_request *req,
 				uint64_t status)
@@ -495,30 +648,10 @@  static void p8_i2c_status_error(struct p8_i2c_master_port *port,
 		 */
 		p8_i2c_complete_request(master, req, req->result);
 	} else {
-		/*
-		 * Reset the bus by issuing a STOP command to slave.
-		 *
-		 * Reprogram the mode register with 'enhanced bit' set
-		 */
-		rc = p8_i2c_prog_mode(port, true);
-		if (rc) {
-			log_simple_error(&e_info(OPAL_RC_I2C_RESET), "I2C: "
-					 "Failed to program the MODE_REG\n");
+		if (p8_i2c_reset_engine(master))
 			goto exit;
-		}
-
 		/* Enable the interrupt */
 		p8_i2c_enable_irqs(master);
-
-		/* Send an immediate stop */
-		master->state = state_error;
-		rc = xscom_write(master->chip_id, master->xscom_base +
-				 I2C_CMD_REG, I2C_CMD_WITH_STOP);
-		if (rc) {
-			log_simple_error(&e_info(OPAL_RC_I2C_RESET), "I2C: "
-					 "Failed to issue immediate STOP\n");
-			goto exit;
-		}
 	}
 	return;
 
@@ -1363,6 +1496,7 @@  static void p8_i2c_init_one(struct dt_node *i2cm, enum p8_i2c_master_type type)
 
 	master->fifo_size = GETFIELD(I2C_EXTD_STAT_FIFO_SIZE, ex_stat);
 	list_head_init(&master->req_list);
+	list_head_init(&master->ports);
 
 	/* Check if interrupt is usable */
 	master->irq_ok = p8_i2c_has_irqs(master);
@@ -1422,6 +1556,7 @@  static void p8_i2c_init_one(struct dt_node *i2cm, enum p8_i2c_master_type type)
 		port->bus.set_req_timeout = p8_i2c_set_request_timeout;
 		port->bus.run_req = p8_i2c_run_request;
 		i2c_add_bus(&port->bus);
+		list_add_tail(&master->ports, &port->link);
 
 		/* Add OPAL properties to the bus node */
 		p8_i2c_add_bus_prop(port);