[coreboot-gerrit] New patch to review for coreboot: intel/fsp_baytrail: Fix I2C abort logic

[Ben Gardner (gardner.ben@gmail.com)]

Ben Gardner (gardner.ben at gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/14160

-gerrit

commit 2d6743ece3b3c5021568602e0acd8e9d77c1b09a
Author: Ben Gardner <gardner.ben at gmail.com>
Date:   Wed Mar 23 09:40:37 2016 -0500

    intel/fsp_baytrail: Fix I2C abort logic
    
    A call to i2c_read() for a non-existent address followed by an i2c_read()
    to a valid address results in a false abort status for the 2nd call.
    
    i2c_read(1, 0x40, 0, buf, sizeof(buf)) => 0x2000000 (I2C_ERR_TIMEOUT)
    i2c_read(1, 0x74, 0, buf, sizeof(buf)) => 0x4000000 (I2C_ERR_ABORT)
    
    Because the abort status register is cleared on read and wait_tx_fifo()
    reads it twice, the returned status does not contain the abort status.
    Fixing that changed the 2nd read to reflect the abort status.
    
    i2c_read(1, 0x40, 0, buf, sizeof(buf)) => 0x2000000 (I2C_ERR_TIMEOUT)
    i2c_read(1, 0x74, 0, buf, sizeof(buf)) => 0x4000001 (I2C_ERR_ABORT)
    
    Bit 0 indicates that the address was not acknowledged by any slave.
    That's the abort status from the previous transaction.
    So I added a read of the abort status before starting a transaction in
    both i2c_read() and i2c_write().
    
    i2c_read(1, 0x40, 0, buf, sizeof(buf)) => 0x2000000 (I2C_ERR_TIMEOUT)
    i2c_read(1, 0x74, 0, buf, sizeof(buf)) => 0 (I2C_SUCCESS)
    
    Tested on a Bay Trail E3845 SoC.
    
    Change-Id: I39e4ff4206587267b6fceef58f4a567bf162fbbe
    Signed-off-by: Ben Gardner <gardner.ben at gmail.com>
---
 src/soc/intel/fsp_baytrail/i2c.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/soc/intel/fsp_baytrail/i2c.c b/src/soc/intel/fsp_baytrail/i2c.c
index c6c8f65..38ac451 100644
--- a/src/soc/intel/fsp_baytrail/i2c.c
+++ b/src/soc/intel/fsp_baytrail/i2c.c
@@ -25,12 +25,13 @@
  */
 static int wait_tx_fifo(char *base_adr) {
 	int i;
+	u32 as;
 
-	if (read32(base_adr + I2C_ABORT_SOURCE) & 0x1ffff) {
+	as = read32(base_adr + I2C_ABORT_SOURCE) & 0x1ffff;
+	if (as) {
 		/* Reading back I2C_CLR_TX_ABRT resets abort lock on TX FIFO */
-		i = *((volatile unsigned int *)(base_adr + I2C_CLR_TX_ABRT));
-		return I2C_ERR_ABORT |
-		 (*((unsigned int *)(base_adr + I2C_ABORT_SOURCE)) & 0x1ffff);
+		i = read32(base_adr + I2C_CLR_TX_ABRT);
+		return I2C_ERR_ABORT | as;
 	}
 
 	/* Wait here for a free slot in TX-FIFO */
@@ -49,11 +50,13 @@ static int wait_tx_fifo(char *base_adr) {
  */
 static int wait_rx_fifo(char *base_adr) {
 	int i;
-	if (read32(base_adr + I2C_ABORT_SOURCE) & 0x1ffff) {
+	u32 as;
+
+	as = read32(base_adr + I2C_ABORT_SOURCE) & 0x1ffff;
+	if (as) {
 		/* Reading back I2C_CLR_TX_ABRT resets abort lock on TX FIFO */
-		i = *((volatile unsigned int *)(base_adr + I2C_CLR_TX_ABRT));
-		return I2C_ERR_ABORT |
-		 (*((unsigned int *)(base_adr + I2C_ABORT_SOURCE)) & 0x1ffff);
+		i = read32(base_adr + I2C_CLR_TX_ABRT);
+		return I2C_ERR_ABORT | as;
 	}
 
 	/* Wait here for a received entry in RX-FIFO */
@@ -176,6 +179,10 @@ int i2c_read(unsigned bus, unsigned chip, unsigned addr,
 	stat = wait_for_idle(base_ptr);
 	if (stat != I2C_SUCCESS)
 		return stat;
+
+	/* clear any abort status from a previous transaction */
+	read32(base_ptr + I2C_CLR_TX_ABRT);
+
 	/* Now we can program the desired slave address and start transfer */
 	*((unsigned int *)(base_ptr + I2C_TARGET_ADR)) = (chip & 0xff);
 	/* Send address inside slave to read from */
@@ -231,6 +238,10 @@ int i2c_write(unsigned bus, unsigned chip, unsigned addr,
 	if (stat) {
 		return stat;
 	}
+
+	/* clear any abort status from a previous transaction */
+	read32(base_ptr + I2C_CLR_TX_ABRT);
+
 	/* Program slave address to use for this transfer */
 	*((unsigned int *)(base_ptr + I2C_TARGET_ADR)) = (chip & 0xff);