Change in ...coreboot[master]: sb/intel/common: Fix SMBUs block commands

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31622

Change subject: sb/intel/common: Fix SMBUs block commands
......................................................................

sb/intel/common: Fix SMBUs block commands

Fix regression after commit
  c38d543 sb/intel/common: SMBus complete_command()

When evaluating HSTSTS register, BYTE_DONE bit must
be excluded from transaction completion and error criteria.

Change-Id: I49cc43d1fa58250988cc41b2ca747b9f1d7586d6
Signed-off-by: Kyösti Mälkki 
---
M src/southbridge/intel/common/smbus.c
1 file changed, 7 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/31622/1

diff --git a/src/southbridge/intel/common/smbus.c b/src/southbridge/intel/common/smbus.c
index a842a61..9ae01ad 100644
--- a/src/southbridge/intel/common/smbus.c
+++ b/src/southbridge/intel/common/smbus.c
@@ -74,7 +74,10 @@
 {
    if (status & SMBHSTSTS_HOST_BUSY)
    	return 0;
-	status &= ~(SMBHSTSTS_SMBALERT_STS | SMBHSTSTS_INUSE_STS);
+
+	/* These status bits do not imply completion of transaction. */
+	status &= ~(SMBHSTSTS_BYTE_DONE | SMBHSTSTS_INUSE_STS |
+		    SMBHSTSTS_SMBALERT_STS);
    return status != 0;
 }

@@ -89,8 +92,9 @@

static int cb_err_from_stat(u8 status)
 {
-	/* Ignore the "In Use" status... */
-	status &= ~(SMBHSTSTS_SMBALERT_STS | SMBHSTSTS_INUSE_STS);
+	/* These status bits do not imply errors. */
+	status &= ~(SMBHSTSTS_BYTE_DONE | SMBHSTSTS_INUSE_STS |
+		    SMBHSTSTS_SMBALERT_STS);

if (status == SMBHSTSTS_INTR)
    	return 0;

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31622 )

Change subject: sb/intel/common: Fix SMBUs block commands
......................................................................

Patch Set 1: Code-Review+1

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31622 )

Change subject: sb/intel/common: Fix SMBUs block commands
......................................................................

Patch Set 1:

untested so far

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31622 )

Change subject: sb/intel/common: Fix SMBUs block commands
......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/#/c/31622/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31622/1//COMMIT_MSG@7 
PS1, Line 7: SMBUs
SMBus

Change in ...coreboot[master]: sb/intel/common: Fix SMBus block commands

[Kyösti Mälkki (Code Review)]

Hello Patrick Rudolph, Arthur Heymans, Paul Menzel, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31622

to look at the new patch set (#2).

Change subject: sb/intel/common: Fix SMBus block commands
......................................................................

sb/intel/common: Fix SMBus block commands

Fix regression after commit
  c38d543 sb/intel/common: SMBus complete_command()

When evaluating HSTSTS register, BYTE_DONE bit must
be excluded from transaction completion and error criteria.

Change-Id: I49cc43d1fa58250988cc41b2ca747b9f1d7586d6
Signed-off-by: Kyösti Mälkki 
---
M src/southbridge/intel/common/smbus.c
1 file changed, 7 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/31622/2

Change in ...coreboot[master]: sb/intel/common: Fix SMBus block commands

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31622 )

Change subject: sb/intel/common: Fix SMBus block commands
......................................................................

Patch Set 2: Code-Review+2

TESTED with i2c_eeprom_read on Thinkpad X200, ich9 doing a dummy read on the SPD after the raminit.

Change in ...coreboot[master]: sb/intel/common: Fix SMBus block commands

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31622 )

Change subject: sb/intel/common: Fix SMBus block commands
......................................................................

sb/intel/common: Fix SMBus block commands

Fix regression after commit
  c38d543 sb/intel/common: SMBus complete_command()

When evaluating HSTSTS register, BYTE_DONE bit must
be excluded from transaction completion and error criteria.

Change-Id: I49cc43d1fa58250988cc41b2ca747b9f1d7586d6
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/31622
Reviewed-by: Arthur Heymans 
Reviewed-by: Paul Menzel 
Reviewed-by: Nico Huber 
Tested-by: build bot (Jenkins) 
---
M src/southbridge/intel/common/smbus.c
1 file changed, 7 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, but someone else must approve
  Paul Menzel: Looks good to me, but someone else must approve
  Arthur Heymans: Looks good to me, approved

diff --git a/src/southbridge/intel/common/smbus.c b/src/southbridge/intel/common/smbus.c
index a842a61..9ae01ad 100644
--- a/src/southbridge/intel/common/smbus.c
+++ b/src/southbridge/intel/common/smbus.c
@@ -74,7 +74,10 @@
 {
    if (status & SMBHSTSTS_HOST_BUSY)
    	return 0;
-	status &= ~(SMBHSTSTS_SMBALERT_STS | SMBHSTSTS_INUSE_STS);
+
+	/* These status bits do not imply completion of transaction. */
+	status &= ~(SMBHSTSTS_BYTE_DONE | SMBHSTSTS_INUSE_STS |
+		    SMBHSTSTS_SMBALERT_STS);
    return status != 0;
 }

@@ -89,8 +92,9 @@

static int cb_err_from_stat(u8 status)
 {
-	/* Ignore the "In Use" status... */
-	status &= ~(SMBHSTSTS_SMBALERT_STS | SMBHSTSTS_INUSE_STS);
+	/* These status bits do not imply errors. */
+	status &= ~(SMBHSTSTS_BYTE_DONE | SMBHSTSTS_INUSE_STS |
+		    SMBHSTSTS_SMBALERT_STS);

if (status == SMBHSTSTS_INTR)
    	return 0;