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@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(); + } }
/*
-
Richard Spiegel (Code Review)