[Skiboot] [PATCH] hw/p8-i2c: Fix OCC locking

Oliver O'Halloran oohall at gmail.com
Thu Aug 24 00:11:02 AEST 2017


There's a few issues with the Host<->OCC I2C bus handshaking. First up,
skiboot is currently examining the wrong bit when checking if the OCC
is currently using the bus. Secondly, when we need to wait for the OCC
to release the bus we are scheduling a recovery timer to run zero
timebase ticks after the current moment so the next poller run will
always time out the current request. The is also an issue with the
order of operations in the recovery timeout handler which can cause
an assertion failure when the cause of the recovery is a timeout
waiting for the OCC.

This patch addresses all these issues and sets the recovery timeout to
10ms.

Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
---
 hw/p8-i2c.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/hw/p8-i2c.c b/hw/p8-i2c.c
index f4666e24953e..69d0984e65cb 100644
--- a/hw/p8-i2c.c
+++ b/hw/p8-i2c.c
@@ -1063,8 +1063,11 @@ static int occ_i2c_lock(struct p8_i2c_master *master)
 
 	busflag = PPC_BIT(16 + (master->engine_id - 1) * 2);
 
-	DBG("occflags = %llx (locks = %.6llx)\n", (u64)occflags,
-	    GETFIELD(PPC_BITMASK(16, 22), occflags));
+	DBG("I2C: c%de%d: occflags = %llx (locks = %x:%x:%x)\n",
+		master->chip_id, master->engine_id, (u64) occflags,
+		(u32) GETFIELD(PPC_BITMASK(16, 17), occflags),
+		(u32) GETFIELD(PPC_BITMASK(18, 19), occflags),
+		(u32) GETFIELD(PPC_BITMASK(20, 21), occflags));
 
 	rc = xscom_write(master->chip_id, OCCFLG_SET, busflag);
 	if (rc) {
@@ -1073,8 +1076,11 @@ static int occ_i2c_lock(struct p8_i2c_master *master)
 	}
 
 	/* If the OCC also has this bus locked then wait for IRQ */
-	if (occflags & (busflag << 1))
+	if (occflags & (busflag >> 1)) {
+		prerror("I2C: c%de%d: Master in use by OCC\n",
+			master->chip_id, master->engine_id);
 		return 1;
+	}
 
 	master->occ_lock_acquired = true;
 
@@ -1161,7 +1167,7 @@ static int p8_i2c_start_request(struct p8_i2c_master *master,
 	if (rc > 0) {
 		/* Wait for OCC IRQ */
 		master->state = state_occache_dis;
-		schedule_timer(&master->recovery, rc);
+		schedule_timer(&master->recovery, msecs_to_tb(10));
 		return 0;
 	}
 
@@ -1298,11 +1304,13 @@ void p9_i2c_bus_owner_change(u32 chip_id)
 			continue;
 		}
 
-		/* Run the state machine */
-		p8_i2c_check_status(master);
+		/* clear the existing wait timer */
+		cancel_timer(&master->recovery);
 
-		/* Check for new work */
+		/* start the request now that we own the master */
+		master->state = state_idle;
 		p8_i2c_check_work(master);
+		p8_i2c_check_status(master);
 
 		unlock(&master->lock);
 	}
-- 
2.9.5



More information about the Skiboot mailing list