[coreboot-gerrit] Change in coreboot[master]: soc/amd/stoneyridge: SMBUS code simplify
Richard Spiegel (Code Review)
gerrit at coreboot.org
Thu Oct 5 02:55:24 CEST 2017
Richard Spiegel has uploaded this change for review. ( https://review.coreboot.org/21887
Change subject: soc/amd/stoneyridge: SMBUS code simplify
......................................................................
soc/amd/stoneyridge: SMBUS code simplify
Solve issues left from review 19722. Unify code: smbus.c to have the actual
execution code, sm.c and smbus_spd.c call functions within smbus.c. 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 at silverbackltd.com>
---
M src/soc/amd/stoneyridge/include/soc/smbus.h
M src/soc/amd/stoneyridge/sm.c
M src/soc/amd/stoneyridge/smbus.c
M src/soc/amd/stoneyridge/smbus_spd.c
M src/soc/amd/stoneyridge/southbridge.c
5 files changed, 92 insertions(+), 153 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/21887/1
diff --git a/src/soc/amd/stoneyridge/include/soc/smbus.h b/src/soc/amd/stoneyridge/include/soc/smbus.h
index 727f72e..e1b4c5f 100644
--- a/src/soc/amd/stoneyridge/include/soc/smbus.h
+++ b/src/soc/amd/stoneyridge/include/soc/smbus.h
@@ -18,57 +18,60 @@
#include <stdint.h>
-#define SMBHSTSTAT 0x0
-#define SMBHST_STAT_FAILED 0x10
-#define SMBHST_STAT_COLLISION 0x08
+#define SMBHSTSTAT 0x0
+#define SMBHST_STAT_FAILED 0x10
+#define SMBHST_STAT_COLLISION 0x08
#define SMBHST_STAT_ERROR 0x04
-#define SMBHST_STAT_INTERRUPT 0x02
+#define SMBHST_STAT_INTERRUPT 0x02
#define SMBHST_STAT_BUSY 0x01
#define SMBHST_STAT_CLEAR 0xff
-#define SMBHST_STAT_NOERROR 0x02
+#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
-#define SMBSLV_STAT_SLV_STS 0x04
-#define SMBSLV_STAT_SLV_INIT 0x02
-#define SMBSLV_STAT_SLV_BUSY 0x01
+#define SMBSLV_STAT_SHADOW2 0x10
+#define SMBSLV_STAT_SHADOW1 0x08
+#define SMBSLV_STAT_SLV_STS 0x04
+#define SMBSLV_STAT_SLV_INIT 0x02
+#define SMBSLV_STAT_SLV_BUSY 0x01
#define SMBSLV_STAT_CLEAR 0x1f
-#define SMBHSTCTRL 0x2
+#define SMBHSTCTRL 0x2
#define SMBHST_CTRL_RST 0x80
#define SMBHST_CTRL_STRT 0x40
-#define SMBHST_CTRL_QCK_RW 0x00
-#define SMBHST_CTRL_BTE_RW 0x04
-#define SMBHST_CTRL_BDT_RW 0x08
-#define SMBHST_CTRL_WDT_RW 0x0c
-#define SMBHST_CTRL_BLK_RW 0x14
+#define SMBHST_CTRL_QCK_RW 0x00
+#define SMBHST_CTRL_BTE_RW 0x04
+#define SMBHST_CTRL_BDT_RW 0x08
+#define SMBHST_CTRL_WDT_RW 0x0c
+#define SMBHST_CTRL_BLK_RW 0x14
#define SMBHST_CTRL_KILL 0x02
#define SMBHST_CTRL_IEN 0x01
+#define SMBHST_CTRL_MODE_BITS 0x1c
-#define SMBHSTCMD 0x3
-#define SMBHSTADDR 0x4
-#define SMBHSTDAT0 0x5
-#define SMBHSTDAT1 0x6
-#define SMBHSTBLKDAT 0x7
-#define SMBSLVCTRL 0x8
-#define SMBSLVCMD_SHADOW 0x9
-#define SMBSLVEVT 0xa
-#define SMBSLVDAT 0xc
-#define SMBTIMING 0xe
+#define SMBHSTCMD 0x3
+#define SMBHSTADDR 0x4
+#define SMBHSTDAT0 0x5
+#define SMBHSTDAT1 0x6
+#define SMBHSTBLKDAT 0x7
+#define SMBSLVCTRL 0x8
+#define SMBSLVCMD_SHADOW 0x9
+#define SMBSLVEVT 0xa
+#define SMBSLVDAT 0xc
+#define SMBTIMING 0xe
-#define SMB_BASE_ADDR 0xb00
+#define SMB_BASE_ADDR 0xb00
-#define AX_INDXC 0
-#define AX_INDXP 2
-#define AXCFG 4
-#define ABCFG 6
-#define RC_INDXC 1
-#define RC_INDXP 3
+#define AX_INDXC 0
+#define AX_INDXP 2
+#define AXCFG 4
+#define ABCFG 6
+#define RC_INDXC 1
+#define RC_INDXP 3
-#define AB_INDX 0xcd8
-#define AB_DATA (AB_INDX+4)
+#define AB_INDX 0xcd8
+#define AB_DATA (AB_INDX+4)
/*
* Between 1-10 seconds, We should never timeout normally
diff --git a/src/soc/amd/stoneyridge/sm.c b/src/soc/amd/stoneyridge/sm.c
index 0c31a3e..3898a1f 100644
--- a/src/soc/amd/stoneyridge/sm.c
+++ b/src/soc/amd/stoneyridge/sm.c
@@ -108,20 +108,12 @@
.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 b216f1e..855f86c 100644
--- a/src/soc/amd/stoneyridge/smbus.c
+++ b/src/soc/amd/stoneyridge/smbus.c
@@ -24,7 +24,7 @@
do {
u8 val;
val = inb(smbus_io_base + SMBHSTSTAT);
- val &= 0x1f;
+ val &= SMBHST_STAT_VAL_BITS;
if (val == 0) { /* ready now */
return 0;
}
@@ -41,10 +41,10 @@
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;
}
@@ -63,8 +63,8 @@
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,7 +72,7 @@
return -3; /* timeout or error */
/* read results of transaction */
- byte = inb(smbus_io_base + SMBHSTCMD);
+ byte = inb(smbus_io_base + SMBHSTDAT0);
return byte;
}
@@ -91,8 +91,8 @@
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 @@
return 0;
}
-int do_smbus_read_byte(u32 smbus_io_base, u32 device,
- u32 address)
+int do_smbus_read_byte(u32 smbus_io_base, u32 device, u32 address)
{
u8 byte;
@@ -117,8 +116,8 @@
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 @@
return byte;
}
-int do_smbus_write_byte(u32 smbus_io_base, u32 device,
- u32 address, u8 val)
+int do_smbus_write_byte(u32 smbus_io_base, u32 device, u32 address, u8 val)
{
u8 byte;
@@ -149,8 +147,8 @@
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 @@
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 @@
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 @@
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 167929c..eb04a3e 100644
--- a/src/soc/amd/stoneyridge/smbus_spd.c
+++ b/src/soc/amd/stoneyridge/smbus_spd.c
@@ -2,6 +2,7 @@
* This file is part of the coreboot project.
*
* Copyright (C) 2012 Advanced Micro Devices, Inc.
+ * Copyright (C) 2017 Silverback, Ltd.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -13,82 +14,16 @@
* GNU General Public License for more details.
*/
+#include <io.h>
#include <device/pci_def.h>
#include <device/device.h>
-/* warning: Porting.h includes an open #pragma pack(1) */
#include <Porting.h>
#include <AGESA.h>
#include <amdlib.h>
#include <soc/southbridge.h>
#include <soc/smbus.h>
#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.
@@ -99,27 +34,40 @@
*/
static int readspd(int iobase, int SmbusSlaveAddress, char *buffer, int count)
{
+ u32 smbus_base, dev_addr;
int index, 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);
- /* read the first byte using offset zero */
- error = readSmbusByteData(iobase, SmbusSlaveAddress, buffer, 0);
+ /*
+ * Convert received parameters to the format accepted by
+ * do_smbus_read_byte and do_smbus_recv_byte.
+ */
+ dev_addr = (u32)((SmbusSlaveAddress & 0xfe) >> 1);
+ smbus_base = (u32) iobase;
- if (error) {
+ /* Read the first SPD byte */
+ error = do_smbus_read_byte(smbus_base, dev_addr, 0);
+ if (error < 0) {
printk(BIOS_ERR, "-------------SPD READ ERROR-----------\n");
return error;
+ } else {
+ *pbuf = (char) (error & 0xff);
+ pbuf++;
}
- /* read the remaining bytes using auto-increment for speed */
+ /* Read the remaining SPD bytes using do_smbus_recv_byte for speed */
for (index = 1 ; index < count ; index++) {
- error = readSmbusByte(iobase, SmbusSlaveAddress,
- &buffer[index]);
- if (error) {
+ error = do_smbus_recv_byte(smbus_base, dev_addr);
+ if (error < 0) {
printk(BIOS_ERR, "-------------SPD READ ERROR-----------\n");
return error;
+ } else {
+ *pbuf = (char) (error & 0xff);
+ pbuf++;
}
}
printk(BIOS_SPEW, "\n");
@@ -130,8 +78,8 @@
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)
@@ -141,6 +89,9 @@
writePmReg(0x2c, ioBase | 1);
/* set SMBus clock to 400 KHz */
__outbyte(ioBase + SMBTIMING, 66000000 / 400000 / 4);
+ /* 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)
diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c
index 5e36100..895e45a 100644
--- a/src/soc/amd/stoneyridge/southbridge.c
+++ b/src/soc/amd/stoneyridge/southbridge.c
@@ -26,11 +26,8 @@
#include <cbmem.h>
#include <amd_pci_util.h>
#include <soc/southbridge.h>
-#include <soc/smbus.h>
#include <soc/smi.h>
-#if IS_ENABLED(CONFIG_STONEYRIDGE_IMC_FWM)
#include <fchec.h>
-#endif
int acpi_get_sleep_type(void)
@@ -98,12 +95,11 @@
void southbridge_final(void *chip_info)
{
-#if IS_ENABLED(CONFIG_STONEYRIDGE_IMC_FWM)
- agesawrapper_fchecfancontrolservice();
-#if !IS_ENABLED(CONFIG_ACPI_ENABLE_THERMAL_ZONE)
- enable_imc_thermal_zone();
-#endif
-#endif
+ if (IS_ENABLED(CONFIG_STONEYRIDGE_IMC_FWM)) {
+ agesawrapper_fchecfancontrolservice();
+ if (IS_ENABLED(CONFIG_ACPI_ENABLE_THERMAL_ZONE))
+ enable_imc_thermal_zone();
+ }
}
/*
--
To view, visit https://review.coreboot.org/21887
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibd55560c95b6752652a4f255b04198e7a4e77d05
Gerrit-Change-Number: 21887
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Spiegel <richard.spiegel at silverbackltd.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20171005/542e86bf/attachment-0001.html>
More information about the coreboot-gerrit
mailing list