Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48219 )
Change subject: soc/amd/common/smbus: remove misleading definition ......................................................................
soc/amd/common/smbus: remove misleading definition
SMBHST_STAT_NOERROR was a redefinition of SMBHST_STAT_INTERRUPT that was used in smbus_wait_until_done. Remove the misleading bit definition that also didn't correspond with the register definitions and replace it with the definition of the actual bit that gets checked. Also add a comment that the code actually checks the IRQ status flag to see if the last command is already completed.
Change-Id: I1a58fe0d58d3887dd2e83320e977a57e271685b3 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/soc/amd/common/block/include/amdblocks/smbus.h M src/soc/amd/common/block/smbus/smbus.c 2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/48219/1
diff --git a/src/soc/amd/common/block/include/amdblocks/smbus.h b/src/soc/amd/common/block/include/amdblocks/smbus.h index 773daf7..73f63b0 100644 --- a/src/soc/amd/common/block/include/amdblocks/smbus.h +++ b/src/soc/amd/common/block/include/amdblocks/smbus.h @@ -11,7 +11,6 @@ #define SMBHST_STAT_INTERRUPT (1 << 1) #define SMBHST_STAT_BUSY (1 << 0) #define SMBHST_STAT_CLEAR 0xff -#define SMBHST_STAT_NOERROR (1 << 1) /* TODO: this one looks odd */ #define SMBHST_STAT_VAL_BITS 0x1f #define SMBHST_STAT_ERROR_BITS 0x1c
diff --git a/src/soc/amd/common/block/smbus/smbus.c b/src/soc/amd/common/block/smbus/smbus.c index e94adf5..4fb68d4 100644 --- a/src/soc/amd/common/block/smbus/smbus.c +++ b/src/soc/amd/common/block/smbus/smbus.c @@ -69,7 +69,8 @@ val &= SMBHST_STAT_VAL_BITS; /* mask off reserved bits */ if (val & SMBHST_STAT_ERROR_BITS) return -5; /* error */ - if (val == SMBHST_STAT_NOERROR) { + /* check IRQ status bit to see if the last host command is completed */ + if (val == SMBHST_STAT_INTERRUPT) { controller_write8(mmio, SMBHSTSTAT, val); /* clr sts */ return 0; }
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48219 )
Change subject: soc/amd/common/smbus: remove misleading definition ......................................................................
Patch Set 1: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48219 )
Change subject: soc/amd/common/smbus: remove misleading definition ......................................................................
soc/amd/common/smbus: remove misleading definition
SMBHST_STAT_NOERROR was a redefinition of SMBHST_STAT_INTERRUPT that was used in smbus_wait_until_done. Remove the misleading bit definition that also didn't correspond with the register definitions and replace it with the definition of the actual bit that gets checked. Also add a comment that the code actually checks the IRQ status flag to see if the last command is already completed.
Change-Id: I1a58fe0d58d3887dd2e83320e977a57e271685b3 Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/48219 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/common/block/include/amdblocks/smbus.h M src/soc/amd/common/block/smbus/smbus.c 2 files changed, 2 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved
diff --git a/src/soc/amd/common/block/include/amdblocks/smbus.h b/src/soc/amd/common/block/include/amdblocks/smbus.h index 773daf7..73f63b0 100644 --- a/src/soc/amd/common/block/include/amdblocks/smbus.h +++ b/src/soc/amd/common/block/include/amdblocks/smbus.h @@ -11,7 +11,6 @@ #define SMBHST_STAT_INTERRUPT (1 << 1) #define SMBHST_STAT_BUSY (1 << 0) #define SMBHST_STAT_CLEAR 0xff -#define SMBHST_STAT_NOERROR (1 << 1) /* TODO: this one looks odd */ #define SMBHST_STAT_VAL_BITS 0x1f #define SMBHST_STAT_ERROR_BITS 0x1c
diff --git a/src/soc/amd/common/block/smbus/smbus.c b/src/soc/amd/common/block/smbus/smbus.c index e94adf5..4fb68d4 100644 --- a/src/soc/amd/common/block/smbus/smbus.c +++ b/src/soc/amd/common/block/smbus/smbus.c @@ -69,7 +69,8 @@ val &= SMBHST_STAT_VAL_BITS; /* mask off reserved bits */ if (val & SMBHST_STAT_ERROR_BITS) return -5; /* error */ - if (val == SMBHST_STAT_NOERROR) { + /* check IRQ status bit to see if the last host command is completed */ + if (val == SMBHST_STAT_INTERRUPT) { controller_write8(mmio, SMBHSTSTAT, val); /* clr sts */ return 0; }
-
Felix Held (Code Review) -
Marshall Dawson (Code Review)