From 1b04aa25917e4bad63f1befa378660f01af1b811 Mon Sep 17 00:00:00 2001 From: Arthur Heymans Date: Fri, 4 Aug 2017 14:28:50 +0200 Subject: [PATCH] sb/intel/common: Fix SMBus block commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clear LAST_BYTE flag at beginning of block commands. For reads, slave device determines in the message format how many bytes it has to transfer out, host firmware only dictates the maximum buffer length. Return SMBUS_ERROR if only partial message was received. For writes, return SMBUS_ERROR if length > 32. For writes, fix off-by-one error reading memory one byte past the buffer end. Change-Id: If9e5d24255a0a03039b52d2863bc278d0eefc311 Signed-off-by: Arthur Heymans Signed-off-by: Kyösti Mälkki Reviewed-on: https://review.coreboot.org/20879 Tested-by: build bot (Jenkins) Reviewed-by: Nico Huber --- src/southbridge/intel/common/smbus.c | 58 +++++++++++++++++++++------- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/src/southbridge/intel/common/smbus.c b/src/southbridge/intel/common/smbus.c index cac9ba7943..ee58aab995 100644 --- a/src/southbridge/intel/common/smbus.c +++ b/src/southbridge/intel/common/smbus.c @@ -17,6 +17,7 @@ #include #include +#include #include "smbus.h" @@ -193,14 +194,17 @@ int do_smbus_write_byte(unsigned int smbus_base, u8 device, } int do_smbus_block_read(unsigned int smbus_base, u8 device, u8 cmd, - unsigned int bytes, u8 *buf) + unsigned int max_bytes, u8 *buf) { u8 status; + int slave_bytes; int bytes_read = 0; unsigned int loops = SMBUS_TIMEOUT; if (smbus_wait_until_ready(smbus_base) < 0) return SMBUS_WAIT_UNTIL_READY_TIMEOUT; + max_bytes = MIN(32, max_bytes); + /* Set up transaction */ /* Disable interrupts */ outb(inb(smbus_base + SMBHSTCTL) & ~SMBHSTCNT_INTREN, @@ -210,11 +214,15 @@ int do_smbus_block_read(unsigned int smbus_base, u8 device, u8 cmd, /* Set the command/address... */ outb(cmd & 0xff, smbus_base + SMBHSTCMD); /* Set up for a block data read */ - outb((inb(smbus_base + SMBHSTCTL) & 0xe3) | I801_BLOCK_DATA, + outb((inb(smbus_base + SMBHSTCTL) & 0xc3) | I801_BLOCK_DATA, (smbus_base + SMBHSTCTL)); /* Clear any lingering errors, so the transaction will run */ outb(inb(smbus_base + SMBHSTSTAT), smbus_base + SMBHSTSTAT); + /* Reset number of bytes to transfer so we notice later it + * was really updated with the transaction. */ + outb(0, smbus_base + SMBHSTDAT0); + /* Start the command */ outb((inb(smbus_base + SMBHSTCTL) | SMBHSTCNT_START), smbus_base + SMBHSTCTL); @@ -233,28 +241,39 @@ int do_smbus_block_read(unsigned int smbus_base, u8 device, u8 cmd, return SMBUS_ERROR; if (status & SMBHSTSTS_BYTE_DONE) { /* Byte done */ - *buf = inb(smbus_base + SMBBLKDAT); - buf++; - bytes_read++; - if (--bytes == 1) { - /* indicate that next byte is the last one */ - outb(inb(smbus_base + SMBHSTCTL) - | SMBHSTCNT_LAST_BYTE, - smbus_base + SMBHSTCTL); + + if (bytes_read < max_bytes) { + *buf = inb(smbus_base + SMBBLKDAT); + buf++; + bytes_read++; } + + /* Engine internally completes the transaction + * and clears HOST_BUSY flag once the byte count + * from slave is reached. + */ outb(status, smbus_base + SMBHSTSTAT); } } while ((status & SMBHSTSTS_HOST_BUSY) && loops); + /* Post-check we received complete message. */ + slave_bytes = inb(smbus_base + SMBHSTDAT0); + if (bytes_read < slave_bytes) + return SMBUS_ERROR; + return bytes_read; } int do_smbus_block_write(unsigned int smbus_base, u8 device, u8 cmd, - unsigned int bytes, const u8 *buf) + const unsigned int bytes, const u8 *buf) { u8 status; + int bytes_sent = 0; unsigned int loops = SMBUS_TIMEOUT; + if (bytes > 32) + return SMBUS_ERROR; + if (smbus_wait_until_ready(smbus_base) < 0) return SMBUS_WAIT_UNTIL_READY_TIMEOUT; @@ -267,7 +286,7 @@ int do_smbus_block_write(unsigned int smbus_base, u8 device, u8 cmd, /* Set the command/address... */ outb(cmd & 0xff, smbus_base + SMBHSTCMD); /* Set up for a block data write */ - outb((inb(smbus_base + SMBHSTCTL) & 0xe3) | I801_BLOCK_DATA, + outb((inb(smbus_base + SMBHSTCTL) & 0xc3) | I801_BLOCK_DATA, (smbus_base + SMBHSTCTL)); /* Clear any lingering errors, so the transaction will run */ outb(inb(smbus_base + SMBHSTSTAT), smbus_base + SMBHSTSTAT); @@ -275,8 +294,10 @@ int do_smbus_block_write(unsigned int smbus_base, u8 device, u8 cmd, /* set number of bytes to transfer */ outb(bytes, smbus_base + SMBHSTDAT0); + /* Send first byte from buffer, bytes_sent increments after + * hardware acknowledges it. + */ outb(*buf++, smbus_base + SMBBLKDAT); - bytes--; /* Start the command */ outb((inb(smbus_base + SMBHSTCTL) | SMBHSTCNT_START), @@ -296,12 +317,19 @@ int do_smbus_block_write(unsigned int smbus_base, u8 device, u8 cmd, return SMBUS_ERROR; if (status & SMBHSTSTS_BYTE_DONE) { - outb(*buf++, smbus_base + SMBBLKDAT); + bytes_sent++; + if (bytes_sent < bytes) + outb(*buf++, smbus_base + SMBBLKDAT); + + /* Engine internally completes the transaction + * and clears HOST_BUSY flag once the byte count + * has been reached. + */ outb(status, smbus_base + SMBHSTSTAT); } } while ((status & SMBHSTSTS_HOST_BUSY) && loops); - return 0; + return bytes_sent; } /* Only since ICH5 */