Change in coreboot[master]: soc/intel/broadwell: Drop old forked version of SMBUS support
Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38142 )
Change subject: soc/intel/broadwell: Drop old forked version of SMBUS support ......................................................................
soc/intel/broadwell: Drop old forked version of SMBUS support
Change-Id: Icbd54b5671ea2a94aea5db4642698ef679540625 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/Makefile.inc M src/soc/intel/broadwell/include/soc/romstage.h M src/soc/intel/broadwell/include/soc/smbus.h M src/soc/intel/broadwell/romstage/smbus.c M src/soc/intel/broadwell/smbus.c D src/soc/intel/broadwell/smbus_common.c 7 files changed, 2 insertions(+), 162 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/38142/1
diff --git a/src/soc/intel/broadwell/Kconfig b/src/soc/intel/broadwell/Kconfig index f01777f..20219d3 100644 --- a/src/soc/intel/broadwell/Kconfig +++ b/src/soc/intel/broadwell/Kconfig @@ -20,6 +20,7 @@ select HAVE_SMI_HANDLER select SOUTHBRIDGE_INTEL_COMMON_RESET select SOUTHBRIDGE_INTEL_COMMON_RTC + select SOUTHBRIDGE_INTEL_COMMON_SMBUS select SOUTHBRIDGE_INTEL_COMMON_SPI_ICH9 select HAVE_USBDEBUG select IOAPIC diff --git a/src/soc/intel/broadwell/Makefile.inc b/src/soc/intel/broadwell/Makefile.inc index 03aa3fb..be854b2 100644 --- a/src/soc/intel/broadwell/Makefile.inc +++ b/src/soc/intel/broadwell/Makefile.inc @@ -53,8 +53,6 @@ ramstage-y += sata.c ramstage-y += serialio.c ramstage-y += smbus.c -ramstage-y += smbus_common.c -romstage-y += smbus_common.c ramstage-y += smi.c smm-y += smihandler.c ramstage-y += smmrelocate.c diff --git a/src/soc/intel/broadwell/include/soc/romstage.h b/src/soc/intel/broadwell/include/soc/romstage.h index 7d0270f..4631652 100644 --- a/src/soc/intel/broadwell/include/soc/romstage.h +++ b/src/soc/intel/broadwell/include/soc/romstage.h @@ -43,6 +43,5 @@ void intel_early_me_status(void);
void enable_smbus(void); -int smbus_read_byte(unsigned int device, unsigned int address);
#endif diff --git a/src/soc/intel/broadwell/include/soc/smbus.h b/src/soc/intel/broadwell/include/soc/smbus.h index 4d9d3e1..40aaf43 100644 --- a/src/soc/intel/broadwell/include/soc/smbus.h +++ b/src/soc/intel/broadwell/include/soc/smbus.h @@ -40,9 +40,4 @@ #define SMBUS_TIMEOUT (10 * 1000 * 100) #define SMBUS_SLAVE_ADDR 0x24
-int do_smbus_read_byte(unsigned int smbus_base, unsigned int device, - unsigned int address); -int do_smbus_write_byte(unsigned int smbus_base, unsigned int device, - unsigned int address, unsigned int data); - #endif diff --git a/src/soc/intel/broadwell/romstage/smbus.c b/src/soc/intel/broadwell/romstage/smbus.c index 48ba684..dd5d030 100644 --- a/src/soc/intel/broadwell/romstage/smbus.c +++ b/src/soc/intel/broadwell/romstage/smbus.c @@ -40,8 +40,3 @@ { reg_script_run_on_dev(PCH_DEV_SMBUS, smbus_init_script); } - -int smbus_read_byte(unsigned int device, unsigned int address) -{ - return do_smbus_read_byte(SMBUS_BASE_ADDRESS, device, address); -} diff --git a/src/soc/intel/broadwell/smbus.c b/src/soc/intel/broadwell/smbus.c index 68d3983..35fbc24 100644 --- a/src/soc/intel/broadwell/smbus.c +++ b/src/soc/intel/broadwell/smbus.c @@ -24,6 +24,7 @@ #include <soc/iomap.h> #include <soc/ramstage.h> #include <soc/smbus.h> +#include <southbridge/intel/common/smbus.h>
static void pch_smbus_init(struct device *dev) { diff --git a/src/soc/intel/broadwell/smbus_common.c b/src/soc/intel/broadwell/smbus_common.c deleted file mode 100644 index 99ed55b..0000000 --- a/src/soc/intel/broadwell/smbus_common.c +++ /dev/null @@ -1,149 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2005 Yinghai Lu yinghailu@gmail.com - * Copyright (C) 2008-2009 coresystems GmbH - * Copyright (C) 2014 Google Inc. - * - * 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 - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#include <arch/io.h> -#include <console/console.h> -#include <device/device.h> -#include <device/path.h> -#include <device/smbus_def.h> -#include <device/pci.h> -#include <soc/ramstage.h> -#include <soc/smbus.h> - -static void smbus_delay(void) -{ - inb(0x80); -} - -static int smbus_wait_until_ready(u16 smbus_base) -{ - unsigned int loops = SMBUS_TIMEOUT; - unsigned char byte; - do { - smbus_delay(); - if (--loops == 0) - break; - byte = inb(smbus_base + SMBHSTSTAT); - } while (byte & 1); - return loops ? 0 : -1; -} - -static int smbus_wait_until_done(u16 smbus_base) -{ - unsigned int loops = SMBUS_TIMEOUT; - unsigned char byte; - do { - smbus_delay(); - if (--loops == 0) - break; - byte = inb(smbus_base + SMBHSTSTAT); - } while ((byte & 1) || (byte & ~((1 << 6) | (1 << 0))) == 0); - return loops ? 0 : -1; -} - -int do_smbus_read_byte(unsigned int smbus_base, unsigned int device, - unsigned int address) -{ - unsigned char global_status_register; - unsigned char byte; - - if (smbus_wait_until_ready(smbus_base) < 0) - return SMBUS_WAIT_UNTIL_READY_TIMEOUT; - - /* Setup transaction */ - /* Disable interrupts */ - outb(inb(smbus_base + SMBHSTCTL) & (~1), smbus_base + SMBHSTCTL); - /* Set the device I'm talking to */ - outb(((device & 0x7f) << 1) | 1, smbus_base + SMBXMITADD); - /* Set the command/address... */ - outb(address & 0xff, smbus_base + SMBHSTCMD); - /* Set up for a byte data read */ - outb((inb(smbus_base + SMBHSTCTL) & 0xe3) | (0x2 << 2), - (smbus_base + SMBHSTCTL)); - /* Clear any lingering errors, so the transaction will run */ - outb(inb(smbus_base + SMBHSTSTAT), smbus_base + SMBHSTSTAT); - - /* Clear the data byte... */ - outb(0, smbus_base + SMBHSTDAT0); - - /* Start the command */ - outb((inb(smbus_base + SMBHSTCTL) | 0x40), - smbus_base + SMBHSTCTL); - - /* Poll for transaction completion */ - if (smbus_wait_until_done(smbus_base) < 0) - return SMBUS_WAIT_UNTIL_DONE_TIMEOUT; - - global_status_register = inb(smbus_base + SMBHSTSTAT); - - /* Ignore the "In Use" status... */ - global_status_register &= ~(3 << 5); - - /* Read results of transaction */ - byte = inb(smbus_base + SMBHSTDAT0); - if (global_status_register != (1 << 1)) - return SMBUS_ERROR; - return byte; -} - -int do_smbus_write_byte(unsigned int smbus_base, unsigned int device, - unsigned int address, unsigned int data) -{ - unsigned char global_status_register; - - if (smbus_wait_until_ready(smbus_base) < 0) - return SMBUS_WAIT_UNTIL_READY_TIMEOUT; - - /* Setup transaction */ - /* Disable interrupts */ - outb(inb(smbus_base + SMBHSTCTL) & (~1), smbus_base + SMBHSTCTL); - /* Set the device I'm talking to */ - outb(((device & 0x7f) << 1) & ~0x01, smbus_base + SMBXMITADD); - /* Set the command/address... */ - outb(address & 0xff, smbus_base + SMBHSTCMD); - /* Set up for a byte data read */ - outb((inb(smbus_base + SMBHSTCTL) & 0xe3) | (0x2 << 2), - (smbus_base + SMBHSTCTL)); - /* Clear any lingering errors, so the transaction will run */ - outb(inb(smbus_base + SMBHSTSTAT), smbus_base + SMBHSTSTAT); - - /* Clear the data byte... */ - outb(data, smbus_base + SMBHSTDAT0); - - /* Start the command */ - outb((inb(smbus_base + SMBHSTCTL) | 0x40), - smbus_base + SMBHSTCTL); - - /* Poll for transaction completion */ - if (smbus_wait_until_done(smbus_base) < 0) { - printk(BIOS_ERR, "SMBUS transaction timeout\n"); - return SMBUS_WAIT_UNTIL_DONE_TIMEOUT; - } - - global_status_register = inb(smbus_base + SMBHSTSTAT); - - /* Ignore the "In Use" status... */ - global_status_register &= ~(3 << 5); - - /* Read results of transaction */ - if (global_status_register != (1 << 1)) { - printk(BIOS_ERR, "SMBUS transaction error\n"); - return SMBUS_ERROR; - } - - return 0; -}
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38142 )
Change subject: soc/intel/broadwell: Drop old forked version of SMBUS support ......................................................................
Patch Set 2: Code-Review+1
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38142 )
Change subject: soc/intel/broadwell: Drop old forked version of SMBUS support ......................................................................
Patch Set 2:
I'd add a note to the commit message that we're switching to using the sb/common implementation, since it's not immediately obvious, and as stated implies that we're simply dropping an unused component, not replacing it.
same for identical change on soc/common
Hello Patrick Rudolph, Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38142
to look at the new patch set (#3).
Change subject: soc/intel/broadwell: Drop old forked version of SMBUS support ......................................................................
soc/intel/broadwell: Drop old forked version of SMBUS support
Switch to use the more recent version in sb/intel/common.
Change-Id: Icbd54b5671ea2a94aea5db4642698ef679540625 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/Makefile.inc M src/soc/intel/broadwell/include/soc/romstage.h M src/soc/intel/broadwell/include/soc/smbus.h M src/soc/intel/broadwell/romstage/smbus.c M src/soc/intel/broadwell/smbus.c D src/soc/intel/broadwell/smbus_common.c 7 files changed, 2 insertions(+), 162 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/38142/3
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38142 )
Change subject: soc/intel/broadwell: Drop old forked version of SMBUS support ......................................................................
Patch Set 3: Code-Review+2
Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38142 )
Change subject: soc/intel/broadwell: Drop old forked version of SMBUS support ......................................................................
soc/intel/broadwell: Drop old forked version of SMBUS support
Switch to use the more recent version in sb/intel/common.
Change-Id: Icbd54b5671ea2a94aea5db4642698ef679540625 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38142 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/Makefile.inc M src/soc/intel/broadwell/include/soc/romstage.h M src/soc/intel/broadwell/include/soc/smbus.h M src/soc/intel/broadwell/romstage/smbus.c M src/soc/intel/broadwell/smbus.c D src/soc/intel/broadwell/smbus_common.c 7 files changed, 2 insertions(+), 162 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/broadwell/Kconfig b/src/soc/intel/broadwell/Kconfig index f01777f..20219d3 100644 --- a/src/soc/intel/broadwell/Kconfig +++ b/src/soc/intel/broadwell/Kconfig @@ -20,6 +20,7 @@ select HAVE_SMI_HANDLER select SOUTHBRIDGE_INTEL_COMMON_RESET select SOUTHBRIDGE_INTEL_COMMON_RTC + select SOUTHBRIDGE_INTEL_COMMON_SMBUS select SOUTHBRIDGE_INTEL_COMMON_SPI_ICH9 select HAVE_USBDEBUG select IOAPIC diff --git a/src/soc/intel/broadwell/Makefile.inc b/src/soc/intel/broadwell/Makefile.inc index 03aa3fb..be854b2 100644 --- a/src/soc/intel/broadwell/Makefile.inc +++ b/src/soc/intel/broadwell/Makefile.inc @@ -53,8 +53,6 @@ ramstage-y += sata.c ramstage-y += serialio.c ramstage-y += smbus.c -ramstage-y += smbus_common.c -romstage-y += smbus_common.c ramstage-y += smi.c smm-y += smihandler.c ramstage-y += smmrelocate.c diff --git a/src/soc/intel/broadwell/include/soc/romstage.h b/src/soc/intel/broadwell/include/soc/romstage.h index 7d0270f..4631652 100644 --- a/src/soc/intel/broadwell/include/soc/romstage.h +++ b/src/soc/intel/broadwell/include/soc/romstage.h @@ -43,6 +43,5 @@ void intel_early_me_status(void);
void enable_smbus(void); -int smbus_read_byte(unsigned int device, unsigned int address);
#endif diff --git a/src/soc/intel/broadwell/include/soc/smbus.h b/src/soc/intel/broadwell/include/soc/smbus.h index 4d9d3e1..40aaf43 100644 --- a/src/soc/intel/broadwell/include/soc/smbus.h +++ b/src/soc/intel/broadwell/include/soc/smbus.h @@ -40,9 +40,4 @@ #define SMBUS_TIMEOUT (10 * 1000 * 100) #define SMBUS_SLAVE_ADDR 0x24
-int do_smbus_read_byte(unsigned int smbus_base, unsigned int device, - unsigned int address); -int do_smbus_write_byte(unsigned int smbus_base, unsigned int device, - unsigned int address, unsigned int data); - #endif diff --git a/src/soc/intel/broadwell/romstage/smbus.c b/src/soc/intel/broadwell/romstage/smbus.c index 48ba684..dd5d030 100644 --- a/src/soc/intel/broadwell/romstage/smbus.c +++ b/src/soc/intel/broadwell/romstage/smbus.c @@ -40,8 +40,3 @@ { reg_script_run_on_dev(PCH_DEV_SMBUS, smbus_init_script); } - -int smbus_read_byte(unsigned int device, unsigned int address) -{ - return do_smbus_read_byte(SMBUS_BASE_ADDRESS, device, address); -} diff --git a/src/soc/intel/broadwell/smbus.c b/src/soc/intel/broadwell/smbus.c index 68d3983..35fbc24 100644 --- a/src/soc/intel/broadwell/smbus.c +++ b/src/soc/intel/broadwell/smbus.c @@ -24,6 +24,7 @@ #include <soc/iomap.h> #include <soc/ramstage.h> #include <soc/smbus.h> +#include <southbridge/intel/common/smbus.h>
static void pch_smbus_init(struct device *dev) { diff --git a/src/soc/intel/broadwell/smbus_common.c b/src/soc/intel/broadwell/smbus_common.c deleted file mode 100644 index 99ed55b..0000000 --- a/src/soc/intel/broadwell/smbus_common.c +++ /dev/null @@ -1,149 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2005 Yinghai Lu yinghailu@gmail.com - * Copyright (C) 2008-2009 coresystems GmbH - * Copyright (C) 2014 Google Inc. - * - * 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 - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#include <arch/io.h> -#include <console/console.h> -#include <device/device.h> -#include <device/path.h> -#include <device/smbus_def.h> -#include <device/pci.h> -#include <soc/ramstage.h> -#include <soc/smbus.h> - -static void smbus_delay(void) -{ - inb(0x80); -} - -static int smbus_wait_until_ready(u16 smbus_base) -{ - unsigned int loops = SMBUS_TIMEOUT; - unsigned char byte; - do { - smbus_delay(); - if (--loops == 0) - break; - byte = inb(smbus_base + SMBHSTSTAT); - } while (byte & 1); - return loops ? 0 : -1; -} - -static int smbus_wait_until_done(u16 smbus_base) -{ - unsigned int loops = SMBUS_TIMEOUT; - unsigned char byte; - do { - smbus_delay(); - if (--loops == 0) - break; - byte = inb(smbus_base + SMBHSTSTAT); - } while ((byte & 1) || (byte & ~((1 << 6) | (1 << 0))) == 0); - return loops ? 0 : -1; -} - -int do_smbus_read_byte(unsigned int smbus_base, unsigned int device, - unsigned int address) -{ - unsigned char global_status_register; - unsigned char byte; - - if (smbus_wait_until_ready(smbus_base) < 0) - return SMBUS_WAIT_UNTIL_READY_TIMEOUT; - - /* Setup transaction */ - /* Disable interrupts */ - outb(inb(smbus_base + SMBHSTCTL) & (~1), smbus_base + SMBHSTCTL); - /* Set the device I'm talking to */ - outb(((device & 0x7f) << 1) | 1, smbus_base + SMBXMITADD); - /* Set the command/address... */ - outb(address & 0xff, smbus_base + SMBHSTCMD); - /* Set up for a byte data read */ - outb((inb(smbus_base + SMBHSTCTL) & 0xe3) | (0x2 << 2), - (smbus_base + SMBHSTCTL)); - /* Clear any lingering errors, so the transaction will run */ - outb(inb(smbus_base + SMBHSTSTAT), smbus_base + SMBHSTSTAT); - - /* Clear the data byte... */ - outb(0, smbus_base + SMBHSTDAT0); - - /* Start the command */ - outb((inb(smbus_base + SMBHSTCTL) | 0x40), - smbus_base + SMBHSTCTL); - - /* Poll for transaction completion */ - if (smbus_wait_until_done(smbus_base) < 0) - return SMBUS_WAIT_UNTIL_DONE_TIMEOUT; - - global_status_register = inb(smbus_base + SMBHSTSTAT); - - /* Ignore the "In Use" status... */ - global_status_register &= ~(3 << 5); - - /* Read results of transaction */ - byte = inb(smbus_base + SMBHSTDAT0); - if (global_status_register != (1 << 1)) - return SMBUS_ERROR; - return byte; -} - -int do_smbus_write_byte(unsigned int smbus_base, unsigned int device, - unsigned int address, unsigned int data) -{ - unsigned char global_status_register; - - if (smbus_wait_until_ready(smbus_base) < 0) - return SMBUS_WAIT_UNTIL_READY_TIMEOUT; - - /* Setup transaction */ - /* Disable interrupts */ - outb(inb(smbus_base + SMBHSTCTL) & (~1), smbus_base + SMBHSTCTL); - /* Set the device I'm talking to */ - outb(((device & 0x7f) << 1) & ~0x01, smbus_base + SMBXMITADD); - /* Set the command/address... */ - outb(address & 0xff, smbus_base + SMBHSTCMD); - /* Set up for a byte data read */ - outb((inb(smbus_base + SMBHSTCTL) & 0xe3) | (0x2 << 2), - (smbus_base + SMBHSTCTL)); - /* Clear any lingering errors, so the transaction will run */ - outb(inb(smbus_base + SMBHSTSTAT), smbus_base + SMBHSTSTAT); - - /* Clear the data byte... */ - outb(data, smbus_base + SMBHSTDAT0); - - /* Start the command */ - outb((inb(smbus_base + SMBHSTCTL) | 0x40), - smbus_base + SMBHSTCTL); - - /* Poll for transaction completion */ - if (smbus_wait_until_done(smbus_base) < 0) { - printk(BIOS_ERR, "SMBUS transaction timeout\n"); - return SMBUS_WAIT_UNTIL_DONE_TIMEOUT; - } - - global_status_register = inb(smbus_base + SMBHSTSTAT); - - /* Ignore the "In Use" status... */ - global_status_register &= ~(3 << 5); - - /* Read results of transaction */ - if (global_status_register != (1 << 1)) { - printk(BIOS_ERR, "SMBUS transaction error\n"); - return SMBUS_ERROR; - } - - return 0; -}
-
Angel Pons (Code Review) -
Kyösti Mälkki (Code Review)
-
Matt DeVillier (Code Review)
-
Paul Menzel (Code Review)