Change in coreboot[master]: southbridge/intel/common/: Mask smbus_read_* return value

[EricR Lai (Code Review)]

EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40574 )

Change subject: southbridge/intel/common/: Mask smbus_read_* return value
......................................................................

southbridge/intel/common/: Mask smbus_read_* return value

Mask smbus_read_* return value to the correct size.

BUG=b:154445630
TEST=Check return value is not whole integer.

Signed-off-by: Eric Lai 
Change-Id: I0426eb7ee267842a9048317f11116e2a2411fccb
---
M src/southbridge/intel/common/smbus.c
1 file changed, 2 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/40574/1

diff --git a/src/southbridge/intel/common/smbus.c b/src/southbridge/intel/common/smbus.c
index 17ac5110..aa25b0f 100644
--- a/src/southbridge/intel/common/smbus.c
+++ b/src/southbridge/intel/common/smbus.c
@@ -347,12 +347,12 @@

int do_smbus_read_byte(uintptr_t base, u8 device, u8 address)
 {
-	return smbus_read_cmd(base, I801_BYTE_DATA, device, address);
+	return smbus_read_cmd(base, I801_BYTE_DATA, device, address) & 0xff;
 }

int do_smbus_read_word(uintptr_t base, u8 device, u8 address)
 {
-	return smbus_read_cmd(base, I801_WORD_DATA, device, address);
+	return smbus_read_cmd(base, I801_WORD_DATA, device, address) & 0xffff;
 }

int do_smbus_write_byte(uintptr_t base, u8 device, u8 address, u8 data)

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40574 )

Change subject: southbridge/intel/common/: Mask smbus_read_* return value
......................................................................

Patch Set 1:

@Furquan, mask the smbus return value here.

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40574 )

Change subject: southbridge/intel/common/: Mask smbus_read_* return value
......................................................................

Patch Set 1: Code-Review-2

That makes it impossible to detect transmission errors. Please do proper error handling on the call side.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40574 )

Change subject: southbridge/intel/common/: Mask smbus_read_* return value
......................................................................

Patch Set 1:

@Furquanm looks like this was doesn't work. Any idea of this?

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40574 )

Change subject: southbridge/intel/common/: Mask smbus_read_* return value
......................................................................

Patch Set 1:
> Patch Set 1: Code-Review-2
>
> That makes it impossible to detect transmission errors. Please do proper error handling on the call side.

What exactly is the expectation for return value of do_smbus_read_byte()? It's not really documented. Digging through the call sequences, it looks like do_smbus_read_byte() would return CB_ERR(-1) if SMBus transaction fails. At minimum, the header file should be updated to reflect this.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40574 )

Change subject: southbridge/intel/common/: Mask smbus_read_* return value
......................................................................

Patch Set 1:
> Patch Set 1:
>
> @Furquanm looks like this was doesn't work. Any idea of this?

Posted comment on https://review.coreboot.org/c/coreboot/+/40558.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40574 )

Change subject: southbridge/intel/common/: Mask smbus_read_* return value
......................................................................

Patch Set 1:

okay, abandon this.

[EricR Lai (Code Review)]

EricR Lai has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/40574 )

Change subject: southbridge/intel/common/: Mask smbus_read_* return value
......................................................................

Abandoned

infeasible