- Fix some copy bugs and thinkos in the i440bx SMbus

read code.  SBbus reads to RAM now work. Yah!  
- Rename the register constants to something I can look at 
more easily.
- Make the logic flow match the flow from V1 assembly 
- #if 0 out other SMbus functions that are still broken.



git-svn-id: svn://svn.coreboot.org/coreboot/trunk@2353 2b7e53f0-3cfb-0310-b3e9-8179ed1497e1

2 files changed, 72 insertions, 55 deletions

diff --git a/src/southbridge/intel/i440bx/i440bx_early_smbus.c b/src/southbridge/intel/i440bx/i440bx_early_smbus.c
index a3db7b88c0..172414e3d4 100644
--- a/src/southbridge/intel/i440bx/i440bx_early_smbus.c
+++ b/src/southbridge/intel/i440bx/i440bx_early_smbus.c
@@ -11,7 +11,7 @@ static void enable_smbus(void)
 	}
 	uint8_t enable;
 	print_spew("SMBus controller enabled\r\n");
-	pci_write_config32(dev, 0x90, SMBUS_IO_BASE );
+	pci_write_config32(dev, 0x90, SMBUS_IO_BASE | 1 );
 	// Enable and set SMBBus 
 	// 0x01 Interrupt to SMI# 
 	// (0x4<<1)|1 set interrupt to IRQ9
@@ -21,25 +21,33 @@ static void enable_smbus(void)
 	pci_write_config16(dev, 0x04, 1);
 	
 	/* clear any lingering errors, so the transaction will run */
-	outb(0x1e, SMBUS_IO_BASE + SMBGSTATUS);
+	outb(0x1e, SMBUS_IO_BASE + SMBHST_STATUS);
 }
 
-static int smbus_recv_byte(unsigned device)
-{
-	return do_smbus_recv_byte(SMBUS_IO_BASE, device);
-}
 
-static int smbus_send_byte(unsigned device, unsigned char val)
-{
-	return do_smbus_send_byte(SMBUS_IO_BASE, device, val);
-}
 
 static int smbus_read_byte(unsigned device, unsigned address)
 {
 	return do_smbus_read_byte(SMBUS_IO_BASE, device, address);
 }
 
+
+// The following functions are broken.  Do no use until you
+// have fixed the low level code to do the right thing.
+//
+#if 0
 static int smbus_write_byte(unsigned device, unsigned address, unsigned char val)
 {
 	return do_smbus_write_byte(SMBUS_IO_BASE, device, address, val);
 }
+
+static int smbus_recv_byte(unsigned device)
+{
+	return do_smbus_recv_byte(SMBUS_IO_BASE, device);
+}
+
+static int smbus_send_byte(unsigned device, unsigned char val)
+{
+	return do_smbus_send_byte(SMBUS_IO_BASE, device, val);
+}
+#endif
diff --git a/src/southbridge/intel/i440bx/i440bx_smbus.h b/src/southbridge/intel/i440bx/i440bx_smbus.h
index 9a100c0f8b..e1893c5cb1 100644
--- a/src/southbridge/intel/i440bx/i440bx_smbus.h
+++ b/src/southbridge/intel/i440bx/i440bx_smbus.h
@@ -1,13 +1,14 @@
 #include <device/smbus_def.h>
 
-#define SMBGSTATUS 0x0
-#define SMBGCTL    0x2
-#define SMBHSTCMD  0x3
-#define SMBHSTADDR 0x4
-#define SMBHSTDAT  0x5
+#define SMBHST_STATUS 	0x0
+#define SMBHST_CTL    	0x2
+#define SMBHST_CMD  	0x3
+#define SMBHST_ADDR 	0x4
+#define SMBHST_DAT  	0x5
 
 #define SMBUS_TIMEOUT (100*1000*10)
 #define SMBUS_STATUS_MASK 0x1e
+#define SMBUS_ERROR_FLAG (1<<2)
 
 static inline void smbus_delay(void)
 {
@@ -26,14 +27,16 @@ static int smbus_wait_until_ready(unsigned smbus_io_base)
 	do {
 		unsigned char val;
 		smbus_delay();
-		val = inb(smbus_io_base + SMBGSTATUS);
+		val = inb(smbus_io_base + SMBHST_STATUS);
 		if ((val & 0x1) == 0) {
 			break;
 		}
+#if 0		
 		if(loops == (SMBUS_TIMEOUT / 2)) {
-			outw(inw(smbus_io_base + SMBGSTATUS), 
-				smbus_io_base + SMBGSTATUS);
+			outw(inw(smbus_io_base + SMBHST_STATUS), 
+				smbus_io_base + SMBHST_STATUS);
 		}
+#endif
 	} while(--loops);
 	return loops?0:SMBUS_WAIT_UNTIL_READY_TIMEOUT;
 }
@@ -46,7 +49,7 @@ static int smbus_wait_until_done(unsigned smbus_io_base)
 		unsigned short val;
 		smbus_delay();
 		
-		val = inb(smbus_io_base + SMBGSTATUS);
+		val = inb(smbus_io_base + SMBHST_STATUS);
 		// Make sure the command is done
 		if ((val & 0x1) != 0) { 
 			continue;
@@ -71,23 +74,23 @@ static int do_smbus_recv_byte(unsigned smbus_io_base, unsigned device)
 	
 	/* setup transaction */
 	/* disable interrupts */
-	outw(inw(smbus_io_base + SMBGCTL) & ~((1<<10)|(1<<9)|(1<<8)|(1<<4)), smbus_io_base + SMBGCTL);
+	outw(inw(smbus_io_base + SMBHST_CTL) & ~((1<<10)|(1<<9)|(1<<8)|(1<<4)), smbus_io_base + SMBHST_CTL);
 	/* set the device I'm talking too */
-	outw(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHSTADDR);
+	outw(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHST_ADDR);
 	/* set the command/address... */
-	outb(0, smbus_io_base + SMBHSTCMD);
+	outb(0, smbus_io_base + SMBHST_CMD);
 	/* set up for a send byte */
-	outw((inw(smbus_io_base + SMBGCTL) & ~7) | (0x1), smbus_io_base + SMBGCTL);
+	outw((inw(smbus_io_base + SMBHST_CTL) & ~7) | (0x1), smbus_io_base + SMBHST_CTL);
 
 	/* clear any lingering errors, so the transaction will run */
 	/* Do I need to write the bits to a 1 to clear an error? */
-	outw(inw(smbus_io_base + SMBGSTATUS), smbus_io_base + SMBGSTATUS);
+	outw(inw(smbus_io_base + SMBHST_STATUS), smbus_io_base + SMBHST_STATUS);
 
 	/* set the data word...*/
-	outw(0, smbus_io_base + SMBHSTDAT);
+	outw(0, smbus_io_base + SMBHST_DAT);
 
 	/* start the command */
-	outw((inw(smbus_io_base + SMBGCTL) | (1 << 3)), smbus_io_base + SMBGCTL);
+	outw((inw(smbus_io_base + SMBHST_CTL) | (1 << 3)), smbus_io_base + SMBHST_CTL);
 
 
 	/* poll for transaction completion */
@@ -95,10 +98,10 @@ static int do_smbus_recv_byte(unsigned smbus_io_base, unsigned device)
 		return SMBUS_WAIT_UNTIL_DONE_TIMEOUT;
 	}
 
-	global_status_register = inw(smbus_io_base + SMBGSTATUS);
+	global_status_register = inw(smbus_io_base + SMBHST_STATUS);
 
 	/* read results of transaction */
-	byte = inb(smbus_io_base + SMBHSTDAT) & 0xff;
+	byte = inb(smbus_io_base + SMBHST_DAT) & 0xff;
 
 	// Check for any result other than a command completion
 	if ((global_status_register & SMBUS_STATUS_MASK) != (1 << 1)) {
@@ -117,30 +120,30 @@ static int do_smbus_send_byte(unsigned smbus_io_base, unsigned device, unsigned
 	
 	/* setup transaction */
 	/* disable interrupts */
-	outw(inw(smbus_io_base + SMBGCTL) & ~((1<<10)|(1<<9)|(1<<8)|(1<<4)), smbus_io_base + SMBGCTL);
+	outw(inw(smbus_io_base + SMBHST_CTL) & ~((1<<10)|(1<<9)|(1<<8)|(1<<4)), smbus_io_base + SMBHST_CTL);
 	/* set the device I'm talking too */
-	outw(((device & 0x7f) << 1) | 0, smbus_io_base + SMBHSTADDR);
+	outw(((device & 0x7f) << 1) | 0, smbus_io_base + SMBHST_ADDR);
 	/* set the command/address... */
-	outb(0, smbus_io_base + SMBHSTCMD);
+	outb(0, smbus_io_base + SMBHST_CMD);
 	/* set up for a send byte */
-	outw((inw(smbus_io_base + SMBGCTL) & ~7) | (0x1), smbus_io_base + SMBGCTL);
+	outw((inw(smbus_io_base + SMBHST_CTL) & ~7) | (0x1), smbus_io_base + SMBHST_CTL);
 
 	/* clear any lingering errors, so the transaction will run */
 	/* Do I need to write the bits to a 1 to clear an error? */
-	outw(inw(smbus_io_base + SMBGSTATUS), smbus_io_base + SMBGSTATUS);
+	outw(inw(smbus_io_base + SMBHST_STATUS), smbus_io_base + SMBHST_STATUS);
 
 	/* set the data word...*/
-	outw(value, smbus_io_base + SMBHSTDAT);
+	outw(value, smbus_io_base + SMBHST_DAT);
 
 	/* start the command */
-	outw((inw(smbus_io_base + SMBGCTL) | (1 << 3)), smbus_io_base + SMBGCTL);
+	outw((inw(smbus_io_base + SMBHST_CTL) | (1 << 3)), smbus_io_base + SMBHST_CTL);
 
 
 	/* poll for transaction completion */
 	if (smbus_wait_until_done(smbus_io_base) < 0) {
 		return SMBUS_WAIT_UNTIL_DONE_TIMEOUT;
 	}
-	global_status_register = inw(smbus_io_base + SMBGSTATUS);
+	global_status_register = inw(smbus_io_base + SMBHST_STATUS);
 
 	if ((global_status_register & SMBUS_STATUS_MASK) != (1 << 4)) {
 		return SMBUS_ERROR;
@@ -151,7 +154,7 @@ static int do_smbus_send_byte(unsigned smbus_io_base, unsigned device, unsigned
 
 static int do_smbus_read_byte(unsigned smbus_io_base, unsigned device, unsigned address)
 {
-	unsigned global_status_register;
+	unsigned status_register;
 	unsigned byte;
 
 	if (smbus_wait_until_ready(smbus_io_base) < 0) {
@@ -161,30 +164,36 @@ static int do_smbus_read_byte(unsigned smbus_io_base, unsigned device, unsigned
 	/* setup transaction */
 
 	/* clear any lingering errors, so the transaction will run */
-	outb(0x1e, smbus_io_base + SMBGSTATUS);
+	outb(0x1e, smbus_io_base + SMBHST_STATUS);
 
 	/* set the device I'm talking too */
-	outb(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHSTADDR);
+	outb(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHST_ADDR);
+
 	/* set the command/address... */
-	outb(address & 0xFF, smbus_io_base + SMBHSTCMD);
+	outb(address & 0xff, smbus_io_base + SMBHST_CMD);
 
 	/* clear the data word...*/
-	outb(0, smbus_io_base + SMBHSTDAT);
+	outb(0, smbus_io_base + SMBHST_DAT);
 
 	/* start a byte read with interrupts disabled */
-	outb( (0x02 << 2)|(1<<6), smbus_io_base + SMBGCTL);
+	outb( (0x02 << 2)|(1<<6), smbus_io_base + SMBHST_CTL);
 
 	/* poll for transaction completion */
 	if (smbus_wait_until_done(smbus_io_base) < 0) {
 		return SMBUS_WAIT_UNTIL_DONE_TIMEOUT;
 	}
 
-	global_status_register = inw(smbus_io_base + SMBGSTATUS);
+	status_register = inw(smbus_io_base + SMBHST_STATUS);
 
 	/* read results of transaction */
-	byte = inw(smbus_io_base + SMBHSTDAT) & 0xff;
-
-	if ((global_status_register & SMBUS_STATUS_MASK) != (1 << 4)) {
+	byte = inw(smbus_io_base + SMBHST_DAT) & 0xff;
+
+	if (status_register & 0x04) {
+#if 0
+ 		print_debug("Read fail ");
+		print_debug_hex16(status_register);
+		print_debug("\r\n");
+#endif
 		return SMBUS_ERROR;
 	}
 	return byte;
@@ -200,29 +209,29 @@ static int do_smbus_write_byte(unsigned smbus_io_base, unsigned device, unsigned
 
 	/* setup transaction */
 	/* disable interrupts */
-	outw(inw(smbus_io_base + SMBGCTL) & ~((1<<10)|(1<<9)|(1<<8)|(1<<4)), smbus_io_base + SMBGCTL);
+	outw(inw(smbus_io_base + SMBHST_CTL) & ~((1<<10)|(1<<9)|(1<<8)|(1<<4)), smbus_io_base + SMBHST_CTL);
 	/* set the device I'm talking too */
-	outw(((device & 0x7f) << 1) | 0, smbus_io_base + SMBHSTADDR);
-	outb(address & 0xFF, smbus_io_base + SMBHSTCMD);
+	outw(((device & 0x7f) << 1) | 0, smbus_io_base + SMBHST_ADDR);
+	outb(address & 0xFF, smbus_io_base + SMBHST_CMD);
 	/* set up for a byte data write */ /* FIXME */
-	outw((inw(smbus_io_base + SMBGCTL) & ~7) | (0x2), smbus_io_base + SMBGCTL);
+	outw((inw(smbus_io_base + SMBHST_CTL) & ~7) | (0x2), smbus_io_base + SMBHST_CTL);
 	/* clear any lingering errors, so the transaction will run */
 	/* Do I need to write the bits to a 1 to clear an error? */
-	outw(inw(smbus_io_base + SMBGSTATUS), smbus_io_base + SMBGSTATUS);
+	outw(inw(smbus_io_base + SMBHST_STATUS), smbus_io_base + SMBHST_STATUS);
 
 	/* write the data word...*/
-	outw(val, smbus_io_base + SMBHSTDAT);
+	outw(val, smbus_io_base + SMBHST_DAT);
 
 	/* start the command */
-	outw((inw(smbus_io_base + SMBGCTL) | (1 << 3)), smbus_io_base + SMBGCTL);
+	outw((inw(smbus_io_base + SMBHST_CTL) | (1 << 3)), smbus_io_base + SMBHST_CTL);
 
 	/* poll for transaction completion */
 	if (smbus_wait_until_done(smbus_io_base) < 0) {
 		return SMBUS_WAIT_UNTIL_DONE_TIMEOUT;
 	}
-	global_status_register = inw(smbus_io_base + SMBGSTATUS);
+	global_status_register = inw(smbus_io_base + SMBHST_STATUS);
 
-	if ((global_status_register & SMBUS_STATUS_MASK) != (1 << 4)) {
+	if ((global_status_register & SMBUS_STATUS_MASK) != (1 << 1)) {
 		return SMBUS_ERROR;
 	}
 	return 0;