skylake: Generate ACPI timing values for I2C devices

Have the Skylake SOC generate ACPI timing values 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 forumla, 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.

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@chromium.org>
Reviewed-on: https://review.coreboot.org/15291
Tested-by: build bot (Jenkins)
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

6 files changed, 100 insertions, 40 deletions

diff --git a/src/soc/intel/common/lpss_i2c.c b/src/soc/intel/common/lpss_i2c.c
index bb684e2f35..a9bbbb32a7 100644
--- a/src/soc/intel/common/lpss_i2c.c
+++ b/src/soc/intel/common/lpss_i2c.c
@@ -20,6 +20,7 @@
 #include <console/console.h>
 #include <device/device.h>
 #include <device/i2c.h>
+#include <string.h>
 #include <timer.h>
 #include "lpss_i2c.h"
 
@@ -305,7 +306,17 @@ int platform_i2c_transfer(unsigned bus, struct i2c_seg *segments, int count)
 	return 0;
 }
 
-void lpss_i2c_acpi_write_speed_config(
+/*
+ * Write ACPI object to describe speed configuration.
+ *
+ * ACPI Object: Name ("xxxx", Package () { scl_lcnt, scl_hcnt, sda_hold }
+ *
+ * SSCN: I2C_SPEED_STANDARD
+ * FMCN: I2C_SPEED_FAST
+ * FPCN: I2C_SPEED_FAST_PLUS
+ * HSCN: I2C_SPEED_HIGH
+ */
+static void lpss_i2c_acpi_write_speed_config(
 	const struct lpss_i2c_speed_config *config)
 {
 	if (!config)
@@ -330,6 +341,37 @@ void lpss_i2c_acpi_write_speed_config(
 	acpigen_pop_len();
 }
 
+void lpss_i2c_acpi_fill_ssdt(const struct lpss_i2c_speed_config *override)
+{
+	const struct lpss_i2c_speed_config *sptr;
+	struct lpss_i2c_speed_config sgen;
+	enum i2c_speed speeds[LPSS_I2C_SPEED_CONFIG_COUNT] = {
+		I2C_SPEED_STANDARD,
+		I2C_SPEED_FAST,
+		I2C_SPEED_FAST_PLUS,
+		I2C_SPEED_HIGH,
+	};
+	int i;
+
+	/* 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 = override; 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);
+	}
+}
+
 int lpss_i2c_set_speed_config(unsigned bus,
 			      const struct lpss_i2c_speed_config *config)
 {
diff --git a/src/soc/intel/common/lpss_i2c.h b/src/soc/intel/common/lpss_i2c.h
index 1ca23b59e8..3ec92134c7 100644
--- a/src/soc/intel/common/lpss_i2c.h
+++ b/src/soc/intel/common/lpss_i2c.h
@@ -85,17 +85,10 @@ int lpss_i2c_set_speed_config(unsigned bus,
 			      const struct lpss_i2c_speed_config *config);
 
 /*
- * Write ACPI object to describe speed configuration.
- *
- * ACPI Object: Name ("xxxx", Package () { scl_lcnt, scl_hcnt, sda_hold }
- *
- * SSCN: I2C_SPEED_STANDARD
- * FMCN: I2C_SPEED_FAST
- * FPCN: I2C_SPEED_FAST_PLUS
- * HSCN: I2C_SPEED_HIGH
+ * Generate I2C timing information into the SSDT for the OS driver to consume,
+ * optionally applying override values provided by the caller.
  */
-void lpss_i2c_acpi_write_speed_config(
-	const struct lpss_i2c_speed_config *config);
+void lpss_i2c_acpi_fill_ssdt(const struct lpss_i2c_speed_config *override);
 
 /*
  * Set I2C bus speed for this controller.
diff --git a/src/soc/intel/skylake/acpi/serialio.asl b/src/soc/intel/skylake/acpi/serialio.asl
index f274be83eb..849679be62 100644
--- a/src/soc/intel/skylake/acpi/serialio.asl
+++ b/src/soc/intel/skylake/acpi/serialio.asl
@@ -20,55 +20,36 @@ 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)
diff --git a/src/soc/intel/skylake/chip.h b/src/soc/intel/skylake/chip.h
index fe03e59451..eec6f1ed17 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 64d39cddb2..eebe665d94 100644
--- a/src/soc/intel/skylake/i2c.c
+++ b/src/soc/intel/skylake/i2c.c
@@ -13,9 +13,11 @@
  * 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>
@@ -54,26 +56,53 @@ 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;
+	const struct lpss_i2c_speed_config *sptr;
+	enum i2c_speed speed;
+	int i, bus = i2c_dev_to_bus(dev);
+
+	if (!config || bus < 0)
+		return;
+
+	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;
 	int bus = i2c_dev_to_bus(dev);
 
 	if (!config || bus < 0)
 		return;
 
-	lpss_i2c_init(bus, config->i2c[bus].speed ? : I2C_SPEED_FAST);
+	acpigen_write_scope(acpi_device_path(dev));
+	lpss_i2c_acpi_fill_ssdt(config->i2c[bus].speed_config);
+	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 64e692420e..3d2e9945c0 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)