[Skiboot] [PATCH 1/4] i2c: Add i2c_run_req() to crank the state machine for a request

Stewart Smith stewart at linux.vnet.ibm.com
Wed Nov 23 16:45:59 AEDT 2016


Doing everything asynchronously is brilliant, it's exactly what we
want to do.

Except... the tpm driver wants to do things synchronously, which isn't
so cool.

For reasons that are not yet completely known, we spend an awful lot of
time in the main thread *not* running pollers (potentially seconds), which
doesn't bode well for I2C timeouts.

Since the TPM measure is done in a secondary thread, we do *not* run pollers
there either (as of 323c8aeb54bd4e0b9004091fcbb4a9daeda2f576 - which is
roughly as of skiboot 2.1.1).

But we still need to crank the i2c state machine, so we introduce a call
to do just that. It will return how long the poll interval should be, so
that we can time_wait() for a more appropriate time for whatever i2c
implementation is sitting behind things.

Without this, it was "easy" to get to a situation where the i2c state machine
wasn't cranked at all, and you'd hit the i2c timeout (for the issued operation)
before the poller to crank i2c was ever called.

Signed-off-by: Stewart Smith <stewart at linux.vnet.ibm.com>
---
 hw/p8-i2c.c                        | 18 ++++++++++++++++++
 include/i2c.h                      |  8 ++++++++
 libstb/drivers/tpm_i2c_interface.c | 14 +++++++++-----
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/hw/p8-i2c.c b/hw/p8-i2c.c
index 4e29432..12a4d1a 100644
--- a/hw/p8-i2c.c
+++ b/hw/p8-i2c.c
@@ -1064,6 +1064,23 @@ static void p8_i2c_set_request_timeout(struct i2c_request *req,
 	request->timeout = msecs_to_tb(duration);
 }
 
+static uint64_t p8_i2c_run_request(struct i2c_request *req)
+{
+	struct i2c_bus *bus = req->bus;
+	struct p8_i2c_master_port *port =
+		container_of(bus, struct p8_i2c_master_port, bus);
+	struct p8_i2c_master *master = port->master;
+	uint64_t poll_interval = 0;
+
+	lock(&master->lock);
+	p8_i2c_check_status(master);
+	p8_i2c_check_work(master);
+	poll_interval = master->poll_interval;
+	unlock(&master->lock);
+
+	return poll_interval;
+}
+
 static inline uint32_t p8_i2c_get_bit_rate_divisor(uint32_t lb_freq,
 						   uint32_t bus_speed)
 {
@@ -1399,6 +1416,7 @@ static void p8_i2c_init_one(struct dt_node *i2cm, enum p8_i2c_master_type type)
 		port->bus.alloc_req = p8_i2c_alloc_request;
 		port->bus.free_req = p8_i2c_free_request;
 		port->bus.set_req_timeout = p8_i2c_set_request_timeout;
+		port->bus.run_req = p8_i2c_run_request;
 		i2c_add_bus(&port->bus);
 
 		/* Add OPAL properties to the bus node */
diff --git a/include/i2c.h b/include/i2c.h
index 83c6ec5..a10a179 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -28,6 +28,7 @@ struct i2c_bus {
 	void			(*free_req)(struct i2c_request *req);
 	void			(*set_req_timeout)(struct i2c_request *req,
 						   uint64_t duration);
+	uint64_t		(*run_req)(struct i2c_request *req);
 };
 
 /*
@@ -89,6 +90,13 @@ static inline void i2c_set_req_timeout(struct i2c_request *req,
 		req->bus->set_req_timeout(req, duration);
 }
 
+static inline uint64_t i2c_run_req(struct i2c_request *req)
+{
+	if (req->bus->run_req)
+		return req->bus->run_req(req);
+	return 0;
+}
+
 /* P8 implementation details */
 extern void p8_i2c_init(void);
 extern void p8_i2c_interrupt(uint32_t chip_id);
diff --git a/libstb/drivers/tpm_i2c_interface.c b/libstb/drivers/tpm_i2c_interface.c
index ba0c5a8..4d136fd 100644
--- a/libstb/drivers/tpm_i2c_interface.c
+++ b/libstb/drivers/tpm_i2c_interface.c
@@ -57,6 +57,7 @@ int tpm_i2c_request_send(int tpm_bus_id, int tpm_dev_addr, int read_write,
 	int rc, waited, retries, timeout;
 	struct i2c_request *req;
 	struct i2c_bus *bus;
+	uint64_t time_to_wait = 0;
 
 	bus = i2c_find_bus_by_id(tpm_bus_id);
 	if (!bus) {
@@ -106,9 +107,12 @@ int tpm_i2c_request_send(int tpm_bus_id, int tpm_dev_addr, int read_write,
 		i2c_queue_req(req);
 
 		do {
-			time_wait_ms(REQ_COMPLETE_POLLING);
-			waited += REQ_COMPLETE_POLLING;
-		} while (waited < timeout && rc == 1);
+			time_to_wait = i2c_run_req(req);
+			if (!time_to_wait)
+				time_to_wait = REQ_COMPLETE_POLLING;
+			time_wait(time_to_wait);
+			waited += time_to_wait;
+		} while (tb_to_msecs(waited) < timeout && rc == 1);
 
 		if (rc == OPAL_I2C_NACK_RCVD)
 			continue;
@@ -117,10 +121,10 @@ int tpm_i2c_request_send(int tpm_bus_id, int tpm_dev_addr, int read_write,
 			break;
 	}
 
-	DBG("%s tpm req op=%x offset=%x buf=%016llx buflen=%d delay=%d/%d,"
+	DBG("%s tpm req op=%x offset=%x buf=%016llx buflen=%d delay=%lu/%d,"
 	    "rc=%d\n",
 	    (rc) ? "!!!!" : "----", req->op, req->offset,
-	    *(uint64_t*) buf, req->rw_len, waited, timeout, rc);
+	    *(uint64_t*) buf, req->rw_len, tb_to_msecs(waited), timeout, rc);
 
 	i2c_free_req(req);
 	if (rc)
-- 
2.7.4



More information about the Skiboot mailing list