[coreboot-gerrit] Patch set updated for coreboot: skylake: Generate ACPI for I2C devices

Duncan Laurie (dlaurie@chromium.org) gerrit at coreboot.org
Tue Jun 28 20:37:54 CEST 2016


Duncan Laurie (dlaurie at chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15291

-gerrit

commit 2281ccb0919561d1a4539c78a48d4d3b6a12325e
Author: Duncan Laurie <dlaurie at chromium.org>
Date:   Tue Jun 21 10:41:19 2016 -0700

    skylake: Generate ACPI for I2C devices
    
    Have the Skylake SOC generate ACPI Devices for the enabled I2C
    controllers instead of passing it in the DSDT with static timings.
    
    The timing values are generated from the controller clock speed and
    are more accurate than the hardcoded values that were in the ASL which
    were originally copied from Broadwell where the controller is running
    at a different clock speed...
    
    Additionally it is now possible for a board to override the values
    using devicetree.cb.  If zero is passed in for SCL HCNT or LCNT then
    the kernel will generate its own timing using the same formula, but if
    the SDA hold time value is zero the kernel will NOT generate a correct
    value and the SDA hold time may be incorrect.
    
    The logic in the SSDT generator is a bit more complicated than I wanted
    but the flexibility for board-level override is important as there may
    be extra load on an I2C bus and a board may need to tune these values
    to get stable I2C operation on that bus.
    
    This was tested on the Chell platform to ensure all the I2C devices on
    the board are still operational with these new timing values.
    
    Change-Id: I4feb3df9e083592792f8fadd7105e081a984a906
    Signed-off-by: Duncan Laurie <dlaurie at chromium.org>
---
 src/soc/intel/skylake/acpi/serialio.asl | 55 ---------------------
 src/soc/intel/skylake/chip.h            |  3 ++
 src/soc/intel/skylake/i2c.c             | 84 +++++++++++++++++++++++++++++----
 src/soc/intel/skylake/romstage/i2c.c    | 14 +++++-
 4 files changed, 90 insertions(+), 66 deletions(-)

diff --git a/src/soc/intel/skylake/acpi/serialio.asl b/src/soc/intel/skylake/acpi/serialio.asl
index f274be8..4f74320 100644
--- a/src/soc/intel/skylake/acpi/serialio.asl
+++ b/src/soc/intel/skylake/acpi/serialio.asl
@@ -16,61 +16,6 @@
 
 /* Intel Serial IO Devices */
 
-Device (I2C0)
-{
-	Name (_ADR, 0x00150000)
-	Name (_DDN, "Serial IO I2C Controller 0")
-
-	Name (SSCN, Package () { 432, 507, 30 })
-	Name (FMCN, Package () { 72, 160, 30 })
-}
-
-Device (I2C1)
-{
-	Name (_ADR, 0x00150001)
-	Name (_DDN, "Serial IO I2C Controller 1")
-
-	Name (SSCN, Package () { 528, 640, 30 })
-	Name (FMCN, Package () { 128, 160, 30 })
-	Name (FPCN, Package () { 48, 64, 30})
-}
-
-Device (I2C2)
-{
-	Name (_ADR, 0x00150002)
-	Name (_DDN, "Serial IO I2C Controller 2")
-
-	Name (SSCN, Package () { 432, 507, 30 })
-	Name (FMCN, Package () { 72, 160, 30 })
-}
-
-Device (I2C3)
-{
-	Name (_ADR, 0x00150003)
-	Name (_DDN, "Serial IO I2C Controller 3")
-
-	Name (SSCN, Package () { 432, 507, 30 })
-	Name (FMCN, Package () { 72, 160, 30 })
-}
-
-Device (I2C4)
-{
-	Name (_ADR, 0x00190002)
-	Name (_DDN, "Serial IO I2C Controller 4")
-
-	Name (SSCN, Package () { 432, 507, 30 })
-	Name (FMCN, Package () { 72, 160, 30 })
-}
-
-Device (I2C5)
-{
-	Name (_ADR, 0x00190002)
-	Name (_DDN, "Serial IO I2C Controller 5")
-
-	Name (SSCN, Package () { 432, 507, 30 })
-	Name (FMCN, Package () { 72, 160, 30 })
-}
-
 Device (SPI0)
 {
 	Name (_ADR, 0x001E0002)
diff --git a/src/soc/intel/skylake/chip.h b/src/soc/intel/skylake/chip.h
index fe03e59..eec6f1e 100644
--- a/src/soc/intel/skylake/chip.h
+++ b/src/soc/intel/skylake/chip.h
@@ -24,6 +24,7 @@
 #include <stdint.h>
 #include <soc/gpio_defs.h>
 #include <soc/gpe.h>
+#include <soc/intel/common/lpss_i2c.h>
 #include <soc/pci_devs.h>
 #include <soc/pmc.h>
 #include <soc/serialio.h>
@@ -44,6 +45,8 @@ struct skylake_i2c_config {
 	enum i2c_speed speed;
 	/* Bus should be enabled prior to ramstage with temporary base */
 	int early_init;
+	/* Custom bus speed configuration { scl_lcnt, scl_hcnt, sda_hold } */
+	struct lpss_i2c_speed_config speed_config[LPSS_I2C_SPEED_CONFIG_COUNT];
 };
 
 struct soc_intel_skylake_config {
diff --git a/src/soc/intel/skylake/i2c.c b/src/soc/intel/skylake/i2c.c
index 64d39cd..8377b50 100644
--- a/src/soc/intel/skylake/i2c.c
+++ b/src/soc/intel/skylake/i2c.c
@@ -13,12 +13,15 @@
  * GNU General Public License for more details.
  */
 
+#include <arch/acpigen.h>
 #include <device/device.h>
 #include <device/i2c.h>
 #include <device/pci.h>
+#include <device/pci_def.h>
 #include <device/pci_ids.h>
 #include <soc/intel/common/lpss_i2c.h>
 #include <soc/ramstage.h>
+#include <string.h>
 
 uintptr_t lpss_i2c_base_address(unsigned bus)
 {
@@ -54,26 +57,87 @@ static int i2c_dev_to_bus(struct device *dev)
 static void i2c_dev_init(struct device *dev)
 {
 	struct soc_intel_skylake_config *config = dev->chip_info;
-	int bus = i2c_dev_to_bus(dev);
+	const struct lpss_i2c_speed_config *sptr;
+	enum i2c_speed speed;
+	int i, bus = i2c_dev_to_bus(dev);
 
 	if (!config || bus < 0)
 		return;
 
-	lpss_i2c_init(bus, config->i2c[bus].speed ? : I2C_SPEED_FAST);
+	speed = config->i2c[bus].speed ? : I2C_SPEED_FAST;
+	lpss_i2c_init(bus, speed);
+
+	/* Apply custom speed config if it has been set by the board */
+	for (i = 0; i < LPSS_I2C_SPEED_CONFIG_COUNT; i++) {
+		sptr = &config->i2c[bus].speed_config[i];
+		if (sptr->speed == speed) {
+			lpss_i2c_set_speed_config(bus, sptr);
+			break;
+		}
+	}
+}
+
+/* Generate ACPI I2C device objects */
+static void i2c_fill_ssdt(struct device *dev)
+{
+	struct soc_intel_skylake_config *config = dev->chip_info;
+	struct lpss_i2c_speed_config *sptr, sgen;
+	uint32_t pci_address;
+	int i, bus = i2c_dev_to_bus(dev);
+	enum i2c_speed speeds[LPSS_I2C_SPEED_CONFIG_COUNT] = {
+		I2C_SPEED_STANDARD,
+		I2C_SPEED_FAST,
+		I2C_SPEED_FAST_PLUS,
+		I2C_SPEED_HIGH,
+	};
+
+	if (!config || bus < 0)
+		return;
+
+	acpigen_write_scope(acpi_device_scope(dev));
+	acpigen_write_device(acpi_device_name(dev));
+
+	/* Build ACPI PCI address */
+	pci_address = PCI_SLOT(dev->path.pci.devfn) << 16;
+	pci_address |= PCI_FUNC(dev->path.pci.devfn);
+	acpigen_write_name_dword("_ADR", pci_address);
+
+	/* Report timing values for the OS driver */
+	for (i = 0; i < LPSS_I2C_SPEED_CONFIG_COUNT; i++) {
+		/* Generate speed config for default case */
+		if (lpss_i2c_gen_speed_config(speeds[i], &sgen) < 0)
+			continue;
+
+		/* Apply board specific override for this speed if found */
+		for (sptr = config->i2c[bus].speed_config;
+		     sptr && sptr->speed; sptr++) {
+			if (sptr->speed == speeds[i]) {
+				memcpy(&sgen, sptr, sizeof(sgen));
+				break;
+			}
+		}
+
+		/* Generate ACPI based on selected speed config */
+		lpss_i2c_acpi_write_speed_config(&sgen);
+	}
+
+	acpigen_pop_len();
+	acpigen_pop_len();
 }
 
 static struct i2c_bus_operations i2c_bus_ops = {
-	.dev_to_bus		= &i2c_dev_to_bus,
+	.dev_to_bus			= &i2c_dev_to_bus,
 };
 
 static struct device_operations i2c_dev_ops = {
-	.read_resources		= &pci_dev_read_resources,
-	.set_resources		= &pci_dev_set_resources,
-	.enable_resources	= &pci_dev_enable_resources,
-	.scan_bus		= &scan_smbus,
-	.ops_pci		= &soc_pci_ops,
-	.ops_i2c_bus		= &i2c_bus_ops,
-	.init			= &i2c_dev_init,
+	.read_resources			= &pci_dev_read_resources,
+	.set_resources			= &pci_dev_set_resources,
+	.enable_resources		= &pci_dev_enable_resources,
+	.scan_bus			= &scan_smbus,
+	.ops_pci			= &soc_pci_ops,
+	.ops_i2c_bus			= &i2c_bus_ops,
+	.init				= &i2c_dev_init,
+	.acpi_fill_ssdt_generator	= &i2c_fill_ssdt,
 };
 
 static const unsigned short pci_device_ids[] = {
diff --git a/src/soc/intel/skylake/romstage/i2c.c b/src/soc/intel/skylake/romstage/i2c.c
index 64e6924..3d2e994 100644
--- a/src/soc/intel/skylake/romstage/i2c.c
+++ b/src/soc/intel/skylake/romstage/i2c.c
@@ -46,6 +46,8 @@ static void i2c_early_init_bus(unsigned bus)
 {
 	ROMSTAGE_CONST struct soc_intel_skylake_config *config;
 	ROMSTAGE_CONST struct device *tree_dev;
+	const struct lpss_i2c_speed_config *sptr;
+	enum i2c_speed speed;
 	pci_devfn_t dev;
 	unsigned devfn;
 	uintptr_t base;
@@ -84,7 +86,17 @@ static void i2c_early_init_bus(unsigned bus)
 	write32(reg, value);
 
 	/* Initialize the controller */
-	lpss_i2c_init(bus, config->i2c[bus].speed ? : I2C_SPEED_FAST);
+	speed = config->i2c[bus].speed ? : I2C_SPEED_FAST;
+	lpss_i2c_init(bus, speed);
+
+	/* Apply custom speed config if it has been set by the board */
+	for (value = 0; value < LPSS_I2C_SPEED_CONFIG_COUNT; value++) {
+		sptr = &config->i2c[bus].speed_config[value];
+		if (sptr->speed == speed) {
+			lpss_i2c_set_speed_config(bus, sptr);
+			break;
+		}
+	}
 }
 
 void i2c_early_init(void)



More information about the coreboot-gerrit mailing list