soc/amd/stoneyridge: Simplify and fix SMBUS code

Solve issues left from Change-Id Ib88a868e654ad127be70ecc506f6b90b784f8d1b
Unify code: smbus.c to have the actual execution code, sm.c and smbus_spd.c
call functions within smbus.c.

Fix some functions that wrongly use SMBHSTCTRL as the register for the
data being transfered. The correct register is SMBHSTDAT0.

Include file smbus.h should only be used by sm.c, smbus.c and smbus_spd.c.

BUG=b:62200225

Change-Id: Ibd55560c95b6752652a4f255b04198e7a4e77d05
Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
Reviewed-on: https://review.coreboot.org/21887
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
Reviewed-by: Marc Jones <marc@marcjonesconsulting.com>

5 files changed, 74 insertions, 133 deletions

diff --git a/src/soc/amd/stoneyridge/include/soc/smbus.h b/src/soc/amd/stoneyridge/include/soc/smbus.h
index db3e9b2e5f..1bb4346186 100644
--- a/src/soc/amd/stoneyridge/include/soc/smbus.h
+++ b/src/soc/amd/stoneyridge/include/soc/smbus.h
@@ -27,8 +27,10 @@
 #define   SMBHST_STAT_BUSY		0x01
 #define   SMBHST_STAT_CLEAR		0xff
 #define   SMBHST_STAT_NOERROR		0x02
+#define   SMBHST_STAT_VAL_BITS		0x1f
+#define   SMBHST_STAT_ERROR_BITS	0x1c
 
-#define   SMBSLVSTAT			0x1
+#define SMBSLVSTAT			0x1
 #define   SMBSLV_STAT_ALERT		0x20
 #define   SMBSLV_STAT_SHADOW2		0x10
 #define   SMBSLV_STAT_SHADOW1		0x08
@@ -60,6 +62,9 @@
 #define SMBSLVDAT			0xc
 #define SMBTIMING			0xe
 
+#define SMB_ASF_IO_BASE			0x01
+#define SMB_SPEED_400KHZ		(66000000 / (400000 * 4))
+
 #define AX_INDXC			0
 #define AX_INDXP			2
 #define AXCFG				4
@@ -86,10 +91,10 @@
 #define rcindxp_reg(reg, port, mask, val)	\
 	alink_rc_indx((RC_INDXP), (reg), (port), (mask), (val))
 
-int do_smbus_read_byte(u32 smbus_io_base, u32 device, u32 address);
-int do_smbus_write_byte(u32 smbus_io_base, u32 device, u32 address, u8 val);
-int do_smbus_recv_byte(u32 smbus_io_base, u32 device);
-int do_smbus_send_byte(u32 smbus_io_base, u32 device, u8 val);
+int do_smbus_read_byte(u16 smbus_io_base, u8 device, u8 address);
+int do_smbus_write_byte(u16 smbus_io_base, u8 device, u8 address, u8 val);
+int do_smbus_recv_byte(u16 smbus_io_base, u8 device);
+int do_smbus_send_byte(u16 smbus_io_base, u8 device, u8 val);
 void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port, u32 mask, u32 val);
 void alink_ab_indx(u32 reg_space, u32 reg_addr, u32 mask, u32 val);
 void alink_ax_indx(u32 space /*c or p? */, u32 axindc, u32 mask, u32 val);
diff --git a/src/soc/amd/stoneyridge/sm.c b/src/soc/amd/stoneyridge/sm.c
index 0c31a3ed8e..9456cbffcf 100644
--- a/src/soc/amd/stoneyridge/sm.c
+++ b/src/soc/amd/stoneyridge/sm.c
@@ -48,7 +48,7 @@ static void sm_init(device_t dev)
 
 static int lsmbus_recv_byte(device_t dev)
 {
-	u32 device;
+	u8 device;
 	struct resource *res;
 	struct bus *pbus;
 
@@ -62,7 +62,7 @@ static int lsmbus_recv_byte(device_t dev)
 
 static int lsmbus_send_byte(device_t dev, u8 val)
 {
-	u32 device;
+	u8 device;
 	struct resource *res;
 	struct bus *pbus;
 
@@ -76,7 +76,7 @@ static int lsmbus_send_byte(device_t dev, u8 val)
 
 static int lsmbus_read_byte(device_t dev, u8 address)
 {
-	u32 device;
+	u8 device;
 	struct resource *res;
 	struct bus *pbus;
 
@@ -90,7 +90,7 @@ static int lsmbus_read_byte(device_t dev, u8 address)
 
 static int lsmbus_write_byte(device_t dev, u8 address, u8 val)
 {
-	u32 device;
+	u8 device;
 	struct resource *res;
 	struct bus *pbus;
 
@@ -108,20 +108,12 @@ static struct smbus_bus_operations lops_smbus_bus = {
 	.write_byte = lsmbus_write_byte,
 };
 
-static void sm_read_resources(device_t dev)
-{
-}
-
-static void sm_set_resources(struct device *dev)
-{
-}
-
 static struct pci_operations lops_pci = {
 	.set_subsystem = pci_dev_set_subsystem,
 };
 static struct device_operations smbus_ops = {
-	.read_resources = sm_read_resources,
-	.set_resources = sm_set_resources,
+	.read_resources = DEVICE_NOOP,
+	.set_resources = DEVICE_NOOP,
 	.enable_resources = pci_dev_enable_resources,
 	.init = sm_init,
 	.scan_bus = scan_smbus,
diff --git a/src/soc/amd/stoneyridge/smbus.c b/src/soc/amd/stoneyridge/smbus.c
index b216f1ee9a..919a52edd4 100644
--- a/src/soc/amd/stoneyridge/smbus.c
+++ b/src/soc/amd/stoneyridge/smbus.c
@@ -17,14 +17,14 @@
 #include <stdint.h>
 #include <soc/smbus.h>
 
-static int smbus_wait_until_ready(u32 smbus_io_base)
+static int smbus_wait_until_ready(u16 smbus_io_base)
 {
 	u32 loops;
 	loops = SMBUS_TIMEOUT;
 	do {
 		u8 val;
 		val = inb(smbus_io_base + SMBHSTSTAT);
-		val &= 0x1f;
+		val &= SMBHST_STAT_VAL_BITS;
 		if (val == 0) {	/* ready now */
 			return 0;
 		}
@@ -33,7 +33,7 @@ static int smbus_wait_until_ready(u32 smbus_io_base)
 	return -2;		/* time out */
 }
 
-static int smbus_wait_until_done(u32 smbus_io_base)
+static int smbus_wait_until_done(u16 smbus_io_base)
 {
 	u32 loops;
 	loops = SMBUS_TIMEOUT;
@@ -41,10 +41,10 @@ static int smbus_wait_until_done(u32 smbus_io_base)
 		u8 val;
 
 		val = inb(smbus_io_base + SMBHSTSTAT);
-		val &= 0x1f;	/* mask off reserved bits */
-		if (val & 0x1c)
+		val &= SMBHST_STAT_VAL_BITS;	/* mask off reserved bits */
+		if (val & SMBHST_STAT_ERROR_BITS)
 			return -5;	/* error */
-		if (val == 0x02) {
+		if (val == SMBHST_STAT_NOERROR) {
 			outb(val, smbus_io_base + SMBHSTSTAT); /* clear sts */
 			return 0;
 		}
@@ -52,7 +52,7 @@ static int smbus_wait_until_done(u32 smbus_io_base)
 	return -3;		/* timeout */
 }
 
-int do_smbus_recv_byte(u32 smbus_io_base, u32 device)
+int do_smbus_recv_byte(u16 smbus_io_base, u8 device)
 {
 	u8 byte;
 
@@ -63,8 +63,8 @@ int do_smbus_recv_byte(u32 smbus_io_base, u32 device)
 	outb(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHSTADDR);
 
 	byte = inb(smbus_io_base + SMBHSTCTRL);
-	byte &= 0xe3;		/* Clear [4:2] */
-	byte |= (1 << 2) | (1 << 6); /* Byte data R/W cmd, start the command */
+	byte &= ~SMBHST_CTRL_MODE_BITS;			/* Clear [4:2] */
+	byte |= SMBHST_CTRL_STRT | SMBHST_CTRL_BTE_RW;	/* set mode, start */
 	outb(byte, smbus_io_base + SMBHSTCTRL);
 
 	/* poll for transaction completion */
@@ -72,12 +72,12 @@ int do_smbus_recv_byte(u32 smbus_io_base, u32 device)
 		return -3;	/* timeout or error */
 
 	/* read results of transaction */
-	byte = inb(smbus_io_base + SMBHSTCMD);
+	byte = inb(smbus_io_base + SMBHSTDAT0);
 
 	return byte;
 }
 
-int do_smbus_send_byte(u32 smbus_io_base, u32 device, u8 val)
+int do_smbus_send_byte(u16 smbus_io_base, u8 device, u8 val)
 {
 	u8 byte;
 
@@ -85,14 +85,14 @@ int do_smbus_send_byte(u32 smbus_io_base, u32 device, u8 val)
 		return -2;	/* not ready */
 
 	/* set the command... */
-	outb(val, smbus_io_base + SMBHSTCMD);
+	outb(val, smbus_io_base + SMBHSTDAT0);
 
 	/* set the device I'm talking to */
 	outb(((device & 0x7f) << 1) | 0, smbus_io_base + SMBHSTADDR);
 
 	byte = inb(smbus_io_base + SMBHSTCTRL);
-	byte &= 0xe3;		/* Clear [4:2] */
-	byte |= (1 << 2) | (1 << 6);	/* Byte data R/W cmd, start command */
+	byte &= ~SMBHST_CTRL_MODE_BITS;			/* Clear [4:2] */
+	byte |= SMBHST_CTRL_STRT | SMBHST_CTRL_BTE_RW;	/* set mode, start */
 	outb(byte, smbus_io_base + SMBHSTCTRL);
 
 	/* poll for transaction completion */
@@ -102,8 +102,7 @@ int do_smbus_send_byte(u32 smbus_io_base, u32 device, u8 val)
 	return 0;
 }
 
-int do_smbus_read_byte(u32 smbus_io_base, u32 device,
-			      u32 address)
+int do_smbus_read_byte(u16 smbus_io_base, u8 device, u8 address)
 {
 	u8 byte;
 
@@ -117,8 +116,8 @@ int do_smbus_read_byte(u32 smbus_io_base, u32 device,
 	outb(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHSTADDR);
 
 	byte = inb(smbus_io_base + SMBHSTCTRL);
-	byte &= 0xe3;		/* Clear [4:2] */
-	byte |= (1 << 3) | (1 << 6);	/* Byte data R/W cmd, start command */
+	byte &= ~SMBHST_CTRL_MODE_BITS;			/* Clear [4:2] */
+	byte |= SMBHST_CTRL_STRT | SMBHST_CTRL_BDT_RW;	/* set mode, start */
 	outb(byte, smbus_io_base + SMBHSTCTRL);
 
 	/* poll for transaction completion */
@@ -131,8 +130,7 @@ int do_smbus_read_byte(u32 smbus_io_base, u32 device,
 	return byte;
 }
 
-int do_smbus_write_byte(u32 smbus_io_base, u32 device,
-			       u32 address, u8 val)
+int do_smbus_write_byte(u16 smbus_io_base, u8 device, u8 address, u8 val)
 {
 	u8 byte;
 
@@ -149,8 +147,8 @@ int do_smbus_write_byte(u32 smbus_io_base, u32 device,
 	outb(val, smbus_io_base + SMBHSTDAT0);
 
 	byte = inb(smbus_io_base + SMBHSTCTRL);
-	byte &= 0xe3;		/* Clear [4:2] */
-	byte |= (1 << 3) | (1 << 6);	/* Byte data R/W cmd, start command */
+	byte &= ~SMBHST_CTRL_MODE_BITS;			/* Clear [4:2] */
+	byte |= SMBHST_CTRL_STRT | SMBHST_CTRL_BDT_RW;	/* set mode, start */
 	outb(byte, smbus_io_base + SMBHSTCTRL);
 
 	/* poll for transaction completion */
@@ -160,8 +158,7 @@ int do_smbus_write_byte(u32 smbus_io_base, u32 device,
 	return 0;
 }
 
-void alink_ab_indx(u32 reg_space, u32 reg_addr,
-			  u32 mask, u32 val)
+void alink_ab_indx(u32 reg_space, u32 reg_addr, u32 mask, u32 val)
 {
 	u32 tmp;
 
@@ -185,8 +182,7 @@ void alink_ab_indx(u32 reg_space, u32 reg_addr,
 	outl(0, AB_INDX);
 }
 
-void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port,
-			  u32 mask, u32 val)
+void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port, u32 mask, u32 val)
 {
 	u32 tmp;
 
@@ -210,7 +206,8 @@ void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port,
 	outl(0, AB_INDX);
 }
 
-/* space = 0: AX_INDXC, AX_DATAC
+/*
+ * space = 0: AX_INDXC, AX_DATAC
  * space = 1: AX_INDXP, AX_DATAP
  */
 void alink_ax_indx(u32 space /*c or p? */, u32 axindc, u32 mask, u32 val)
diff --git a/src/soc/amd/stoneyridge/smbus_spd.c b/src/soc/amd/stoneyridge/smbus_spd.c
index 605f8da78e..a2763357f4 100644
--- a/src/soc/amd/stoneyridge/smbus_spd.c
+++ b/src/soc/amd/stoneyridge/smbus_spd.c
@@ -24,101 +24,49 @@
 #include <dimmSpd.h>
 
 /*
- * readSmbusByteData - read a single SPD byte from any offset
- */
-static int readSmbusByteData(int iobase, int address, char *buffer, int offset)
-{
-	unsigned int status;
-	UINT64 limit;
-
-	address |= 1; // set read bit
-
-	__outbyte(iobase + SMBHSTSTAT, SMBHST_STAT_CLEAR);
-	__outbyte(iobase + SMBSLVSTAT, SMBSLV_STAT_CLEAR);
-	__outbyte(iobase + SMBHSTCMD, offset);		// offset in eeprom
-	__outbyte(iobase + SMBHSTADDR, address);	// slave addr & read bit
-	__outbyte(iobase + SMBHSTCTRL, SMBHST_CTRL_STRT + SMBHST_CTRL_BDT_RW);
-
-	// time limit to avoid hanging for unexpected error status
-	limit = __rdtsc() + 2000000000 / 10;
-	for (;;) {
-		status = __inbyte(iobase + SMBHSTSTAT);
-		if (__rdtsc() > limit)
-			break;
-		if ((status & SMBHST_STAT_INTERRUPT) == 0)
-			continue;	// SMBusInterrupt not set, keep waiting
-		if ((status & SMBHST_STAT_BUSY) == SMBHST_STAT_BUSY)
-			continue;	// HostBusy set, keep waiting
-		break;
-	}
-
-	buffer[0] = __inbyte(iobase + SMBHSTDAT0);
-	if (status == SMBHST_STAT_NOERROR)
-		status = 0;		// done with no errors
-	return status;
-}
-
-/*
- * readSmbusByte - read a single SPD byte from the default offset
- *                 this function is faster function readSmbusByteData
- */
-static int readSmbusByte(int iobase, int address, char *buffer)
-{
-	unsigned int status;
-	UINT64 limit;
-
-	__outbyte(iobase + SMBHSTSTAT, SMBHST_STAT_CLEAR);
-	__outbyte(iobase + SMBHSTCTRL, SMBHST_CTRL_STRT + SMBHST_CTRL_BTE_RW);
-
-	// time limit to avoid hanging for unexpected error status
-	limit = __rdtsc() + 2000000000 / 10;
-	for (;;) {
-		status = __inbyte(iobase + SMBHSTSTAT);
-		if (__rdtsc() > limit)
-			break;
-		if ((status & SMBHST_STAT_INTERRUPT) == 0)
-			continue;	// SMBusInterrupt not set, keep waiting
-		if ((status & SMBHST_STAT_BUSY) == SMBHST_STAT_BUSY)
-			continue;	// HostBusy set, keep waiting
-		break;
-	}
-
-	buffer[0] = __inbyte(iobase + SMBHSTDAT0);
-	if (status == SMBHST_STAT_NOERROR)
-		status = 0;		// done with no errors
-	return status;
-}
-
-/*
  * readspd - Read one or more SPD bytes from a DIMM.
  *           Start with offset zero and read sequentially.
  *           Optimization relies on autoincrement to avoid
  *           sending offset for every byte.
  *          Reads 128 bytes in 7-8 ms at 400 KHz.
  */
-static int readspd(int iobase, int SmbusSlaveAddress, char *buffer, int count)
+static int readspd(uint16_t iobase, int SmbusSlaveAddress,
+			char *buffer, size_t count)
 {
-	int index, error;
+	u8 dev_addr;
+	size_t index;
+	int error;
+	char *pbuf = buffer;
 
 	printk(BIOS_SPEW, "-------------READING SPD-----------\n");
 	printk(BIOS_SPEW, "iobase: 0x%08X, SmbusSlave: 0x%08X, count: %d\n",
-					iobase, SmbusSlaveAddress, count);
+					iobase, SmbusSlaveAddress, (int)count);
 
-	/* read the first byte using offset zero */
-	error = readSmbusByteData(iobase, SmbusSlaveAddress, buffer, 0);
+	/*
+	 * Convert received device address to the format accepted by
+	 * do_smbus_read_byte and do_smbus_recv_byte.
+	 */
+	dev_addr = (SmbusSlaveAddress >> 1);
 
-	if (error) {
+	/* Read the first SPD byte */
+	error = do_smbus_read_byte(iobase, dev_addr, 0);
+	if (error < 0) {
 		printk(BIOS_ERR, "-------------SPD READ ERROR-----------\n");
 		return error;
+	} else {
+		*pbuf = (char) error;
+		pbuf++;
 	}
 
-	/* read the remaining bytes using auto-increment for speed */
-	for (index = 1; index < count; index++) {
-		error = readSmbusByte(iobase, SmbusSlaveAddress,
-					&buffer[index]);
-		if (error) {
+	/* Read the remaining SPD bytes using do_smbus_recv_byte for speed */
+	for (index = 1 ; index < count ; index++) {
+		error = do_smbus_recv_byte(iobase, dev_addr);
+		if (error < 0) {
 			printk(BIOS_ERR, "-------------SPD READ ERROR-----------\n");
 			return error;
+		} else {
+			*pbuf = (char) error;
+			pbuf++;
 		}
 	}
 	printk(BIOS_SPEW, "\n");
@@ -129,22 +77,22 @@ static int readspd(int iobase, int SmbusSlaveAddress, char *buffer, int count)
 
 static void writePmReg(int reg, int data)
 {
-	__outbyte(PM_INDEX, reg);
-	__outbyte(PM_DATA, data);
+	outb(reg, PM_INDEX);
+	outb(data, PM_DATA);
 }
 
-static void setupFch(int ioBase)
+static void setupFch(uint16_t ioBase)
 {
-	/* register 0x2c and 0x2d are not defined in public datasheet */
-	writePmReg(0x2d, ioBase >> 8);
-	writePmReg(0x2c, ioBase | 1);
-	/* set SMBus clock to 400 KHz */
-	__outbyte(ioBase + SMBTIMING, 66000000 / 400000 / 4);
+	writePmReg(SMB_ASF_IO_BASE, ioBase >> 8);
+	outb(SMB_SPEED_400KHZ, ioBase + SMBTIMING);
+	/* Clear all SMBUS status bits */
+	outb(SMBHST_STAT_CLEAR, ioBase + SMBHSTSTAT);
+	outb(SMBSLV_STAT_CLEAR, ioBase + SMBSLVSTAT);
 }
 
 int sb_readSpd(int spdAddress, char *buf, size_t len)
 {
-	int ioBase = SMB_BASE_ADDR;
+	uint16_t ioBase = SMB_BASE_ADDR;
 	setupFch(ioBase);
 	return readspd(ioBase, spdAddress, buf, len);
 }
diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c
index b947be123f..8ea53012a2 100644
--- a/src/soc/amd/stoneyridge/southbridge.c
+++ b/src/soc/amd/stoneyridge/southbridge.c
@@ -26,7 +26,6 @@
 #include <cbmem.h>
 #include <amd_pci_util.h>
 #include <soc/southbridge.h>
-#include <soc/smbus.h>
 #include <soc/smi.h>
 #include <fchec.h>