[coreboot-gerrit] Change in ...coreboot[master]: sb/intel/common: SMBus execute_command()

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/21119 )

Change subject: sb/intel/common: SMBus execute_command()
......................................................................

sb/intel/common: SMBus execute_command()

Implement the common start of transaction.

Fixes a problem where smbus_wait_until_active()
can miss SMBHSTSTS_HOST_BUSY being set, if
transaction completes very fast. Or if we are
single-stepping or executing under SerialIce
emulation.

Change-Id: Icb27d7d6a1c54968950ca292dbae05415f97e461
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/21119
Tested-by: build bot (Jenkins) 
Reviewed-by: Arthur Heymans 
---
M src/southbridge/intel/common/smbus.c
1 file changed, 37 insertions(+), 40 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Arthur Heymans: Looks good to me, approved

diff --git a/src/southbridge/intel/common/smbus.c b/src/southbridge/intel/common/smbus.c
index b2a78c9..fb7f8e5 100644
--- a/src/southbridge/intel/common/smbus.c
+++ b/src/southbridge/intel/common/smbus.c
@@ -101,19 +101,31 @@
    return 0;
 }

-static int smbus_wait_until_active(u16 smbus_base)
+static int execute_command(unsigned int smbus_base)
 {
-	unsigned long loops;
-	loops = SMBUS_TIMEOUT;
+	unsigned int loops = SMBUS_TIMEOUT;
+	u8 status;
+
+	/* Start the command. */
+	outb((inb(smbus_base + SMBHSTCTL) | SMBHSTCNT_START),
+	     smbus_base + SMBHSTCTL);
+
+	/* Poll for it to start. */
    do {
-		unsigned char val;
    	smbus_delay();
-		val = inb(smbus_base + SMBHSTSTAT);
-		if ((val & SMBHSTSTS_HOST_BUSY)) {
-			break;
-		}
-	} while (--loops);
-	return loops ? 0 : -1;
+
+		/* If we poll too slow, we could miss HOST_BUSY flag
+		 * set and detect INTR or x_ERR flags instead here.
+		 */
+		status = inb(smbus_base + SMBHSTSTAT);
+		status &= ~(SMBHSTSTS_SMBALERT_STS | SMBHSTSTS_INUSE_STS);
+	} while (--loops && status == 0);
+
+	if (loops == 0)
+		return recover_master(smbus_base,
+				      SMBUS_WAIT_UNTIL_ACTIVE_TIMEOUT);
+
+	return 0;
 }

static int smbus_wait_until_done(u16 smbus_base)
@@ -149,12 +161,9 @@
    outb(0, smbus_base + SMBHSTDAT0);

/* Start the command */
-	outb((inb(smbus_base + SMBHSTCTL) | SMBHSTCNT_START),
-	     smbus_base + SMBHSTCTL);
-
-	/* poll for it to start */
-	if (smbus_wait_until_active(smbus_base) < 0)
-		return SMBUS_WAIT_UNTIL_ACTIVE_TIMEOUT;
+	ret = execute_command(smbus_base);
+	if (ret < 0)
+		return ret;

/* Poll for transaction completion */
    if (smbus_wait_until_done(smbus_base) < 0)
@@ -190,12 +199,9 @@
    outb(data, smbus_base + SMBHSTDAT0);

/* Start the command */
-	outb((inb(smbus_base + SMBHSTCTL) | SMBHSTCNT_START),
-	     smbus_base + SMBHSTCTL);
-
-	/* poll for it to start */
-	if (smbus_wait_until_active(smbus_base) < 0)
-		return SMBUS_WAIT_UNTIL_ACTIVE_TIMEOUT;
+	ret = execute_command(smbus_base);
+	if (ret < 0)
+		return ret;

/* Poll for transaction completion */
    if (smbus_wait_until_done(smbus_base) < 0)
@@ -236,12 +242,9 @@
    outb(0, smbus_base + SMBHSTDAT0);

/* Start the command */
-	outb((inb(smbus_base + SMBHSTCTL) | SMBHSTCNT_START),
-	     smbus_base + SMBHSTCTL);
-
-	/* poll for it to start */
-	if (smbus_wait_until_active(smbus_base) < 0)
-		return SMBUS_WAIT_UNTIL_ACTIVE_TIMEOUT;
+	ret = execute_command(smbus_base);
+	if (ret < 0)
+		return ret;

/* Poll for transaction completion */
    do {
@@ -306,12 +309,9 @@
    outb(*buf++, smbus_base + SMBBLKDAT);

/* Start the command */
-	outb((inb(smbus_base + SMBHSTCTL) | SMBHSTCNT_START),
-	     smbus_base + SMBHSTCTL);
-
-	/* poll for it to start */
-	if (smbus_wait_until_active(smbus_base) < 0)
-		return SMBUS_WAIT_UNTIL_ACTIVE_TIMEOUT;
+	ret = execute_command(smbus_base);
+	if (ret < 0)
+		return ret;

/* Poll for transaction completion */
    do {
@@ -367,12 +367,9 @@
    outb(offset, smbus_base + SMBHSTDAT1);

/* Start the command */
-	outb((inb(smbus_base + SMBHSTCTL) | SMBHSTCNT_START),
-	     smbus_base + SMBHSTCTL);
-
-	/* poll for it to start */
-	if (smbus_wait_until_active(smbus_base) < 0)
-		return SMBUS_WAIT_UNTIL_ACTIVE_TIMEOUT;
+	ret = execute_command(smbus_base);
+	if (ret < 0)
+		return ret;

/* Poll for transaction completion */
    do {