[coreboot-gerrit] Change in coreboot[master]: drivers/i2c/tpm/cr50: Use tis_plat_irq_status for Cr50 irq s...
Daniel Kurtz (Code Review)
gerrit at coreboot.org
Wed Apr 19 12:59:02 CEST 2017
Daniel Kurtz has uploaded a new change for review. ( https://review.coreboot.org/19363 )
Change subject: drivers/i2c/tpm/cr50: Use tis_plat_irq_status for Cr50 irq status
......................................................................
drivers/i2c/tpm/cr50: Use tis_plat_irq_status for Cr50 irq status
The Cr50 TPM uses a irq to provide a "status" signal used for hand-shaking
the completion of commands. Real IRQs are not supported in firmware,
however firmware can still poll interrupt status registers for the same
effect.
Commit 94cc485338a3 ("drivers/i2c/tpm/cr50: Support interrupts for status")
added support for the Cr50 driver on X86 platforms to use a KConfig file
to supply an irq which it would poll using acpi_get_gpe. If the irq is
not supplied, the Cr50 driver inserts a 20 ms wait.
Unfortunately this doesn't work so well when using the i2c connected Cr50
on ARM platforms. Luckily, a more generic implementation to allow a
mainboard to supply a Cr50 irq status polling function was solved for SPI
connected Cr50s by commit 19e3d335bddb ("drivers/spi/tpm: using tpm irq to
sync tpm transaction").
Let's refactor the i2c c50 drive to use this same approach, and refactor
the Cr50 integration for the only two mainboards that currently use i2c
Cr50, reef and eve.
This essentially reverts these two commits:
48f708d199 drivers/i2c/tpm/cr50: Initialize IRQ status handler before probe
94cc485338 drivers/i2c/tpm/cr50: Support interrupts for status
And ports this commit to i2c/tpm/cr50:
19e3d335bd drivers/spi/tpm: using tpm irq to sync tpm transaction
As a side effect the tpm_vendor_specific irq field goes back to its
original usage as the "TPM 1.2 command complete" interrupt, instead of
being repurposed to hold the flow control irq.
Signed-off-by: Daniel Kurtz <djkurtz at chromium.org>
BRANCH=none
BUG=b:36786804
TEST=Boot reef w/ serial enabled firmware, verify verstage sees
"cr50 TPM" and does not complain about lack of tis_plat_irq_status().
TEST=Boot eve w/ serial enabled firmware, verify verstage sees
"cr50 TPM" and does not complain about lack of tis_plat_irq_status().
Change-Id: I004329eae1d8aabda51c46b8504bf210484782b4
---
M src/drivers/i2c/tpm/Kconfig
M src/drivers/i2c/tpm/cr50.c
M src/drivers/i2c/tpm/tpm.h
M src/mainboard/google/eve/Kconfig
M src/mainboard/google/eve/chromeos.c
M src/mainboard/google/reef/Kconfig
M src/mainboard/google/reef/chromeos.c
7 files changed, 37 insertions(+), 55 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/19363/1
diff --git a/src/drivers/i2c/tpm/Kconfig b/src/drivers/i2c/tpm/Kconfig
index 32e5fb4..289b783 100644
--- a/src/drivers/i2c/tpm/Kconfig
+++ b/src/drivers/i2c/tpm/Kconfig
@@ -44,11 +44,6 @@
default 0x2 # FIXME, workaround for Kconfig BS
depends on I2C_TPM
-config DRIVER_TPM_I2C_IRQ
- int "IRQ or GPE to use for TPM interrupt"
- default -1
- depends on I2C_TPM
-
config DRIVER_I2C_TPM_ACPI
bool "Generate I2C TPM ACPI device"
default y if ARCH_X86 && I2C_TPM
diff --git a/src/drivers/i2c/tpm/cr50.c b/src/drivers/i2c/tpm/cr50.c
index ce30bf1..076fcc9 100644
--- a/src/drivers/i2c/tpm/cr50.c
+++ b/src/drivers/i2c/tpm/cr50.c
@@ -42,10 +42,6 @@
#include <tpm.h>
#include "tpm.h"
-#if IS_ENABLED(CONFIG_ARCH_X86)
-#include <arch/acpi.h>
-#endif
-
#define CR50_MAX_BUFSIZE 63
#define CR50_TIMEOUT_LONG_MS 2000 /* Long timeout while waiting for TPM */
#define CR50_TIMEOUT_SHORT_MS 2 /* Short timeout during transactions */
@@ -61,31 +57,31 @@
static struct tpm_inf_dev g_tpm_dev CAR_GLOBAL;
+__attribute__ ((weak)) int tis_plat_irq_status(void)
+{
+ static int warning_displayed CAR_GLOBAL;
+
+ if (!car_get_var(warning_displayed)) {
+ printk(BIOS_WARNING, "WARNING: tis_plat_irq_status() not implemented, wasting 20ms to wait on Cr50!\n");
+ car_set_var(warning_displayed, 1);
+ }
+ mdelay(CR50_TIMEOUT_NOIRQ_MS);
+
+ return 1;
+}
+
/* Wait for interrupt to indicate the TPM is ready */
static int cr50_i2c_wait_tpm_ready(struct tpm_chip *chip)
{
struct stopwatch sw;
- if (!chip->vendor.irq_status) {
- /* Fixed delay if interrupt not supported */
- mdelay(CR50_TIMEOUT_NOIRQ_MS);
- return 0;
- }
-
stopwatch_init_msecs_expire(&sw, CR50_TIMEOUT_IRQ_MS);
- while (!chip->vendor.irq_status(chip->vendor.irq))
+ while (!tis_plat_irq_status())
if (stopwatch_expired(&sw))
return -1;
return 0;
-}
-
-/* Clear pending interrupts */
-static void cr50_i2c_clear_tpm_irq(struct tpm_chip *chip)
-{
- if (chip->vendor.irq_status)
- chip->vendor.irq_status(chip->vendor.irq);
}
/*
@@ -111,7 +107,7 @@
return -1;
/* Clear interrupt before starting transaction */
- cr50_i2c_clear_tpm_irq(chip);
+ tis_plat_irq_status();
/* Send the register address byte to the TPM */
if (i2c_write_raw(tpm_dev->bus, tpm_dev->addr, &addr, 1)) {
@@ -161,7 +157,7 @@
memcpy(tpm_dev->buf + 1, buffer, len);
/* Clear interrupt before starting transaction */
- cr50_i2c_clear_tpm_irq(chip);
+ tis_plat_irq_status();
/* Send write request buffer with address */
if (i2c_write_raw(tpm_dev->bus, tpm_dev->addr, tpm_dev->buf, len + 1)) {
@@ -418,25 +414,6 @@
chip->vendor.recv = &cr50_i2c_tis_recv;
chip->vendor.send = &cr50_i2c_tis_send;
chip->vendor.cancel = &cr50_i2c_tis_ready;
- chip->vendor.irq = CONFIG_DRIVER_TPM_I2C_IRQ;
-
- /*
- * Interrupts are not supported this early in firmware,
- * use use an arch-specific method to query for interrupt status.
- */
- if (chip->vendor.irq > 0) {
-#if IS_ENABLED(CONFIG_ARCH_X86)
- /* Query GPE status for interrupt */
- chip->vendor.irq_status = &acpi_get_gpe;
-#else
- chip->vendor.irq = -1;
-#endif
- }
-
- if (chip->vendor.irq <= 0)
- printk(BIOS_WARNING,
- "%s: No IRQ, will use %ums delay for TPM ready\n",
- __func__, CR50_TIMEOUT_NOIRQ_MS);
}
int tpm_vendor_probe(unsigned int bus, uint32_t addr)
@@ -491,6 +468,9 @@
cr50_vendor_init(chip);
+ /* Disable TPM 1.2-style command complete interrupt (not supported) */
+ chip->vendor.irq = 0;
+
if (request_locality(chip, 0) != 0)
return -1;
@@ -503,8 +483,8 @@
goto out_err;
}
- printk(BIOS_DEBUG, "cr50 TPM 2.0 (i2c %u:0x%02x irq %d id 0x%x)\n",
- bus, dev_addr, chip->vendor.irq, vendor >> 16);
+ printk(BIOS_DEBUG, "cr50 TPM 2.0 (i2c %u:0x%02x id 0x%x)\n",
+ bus, dev_addr, vendor >> 16);
chip->is_open = 1;
return 0;
diff --git a/src/drivers/i2c/tpm/tpm.h b/src/drivers/i2c/tpm/tpm.h
index e511e0d..35cf397 100644
--- a/src/drivers/i2c/tpm/tpm.h
+++ b/src/drivers/i2c/tpm/tpm.h
@@ -59,7 +59,6 @@
uint8_t req_complete_val;
uint8_t req_canceled;
int irq;
- int (*irq_status)(int irq);
int (*recv)(struct tpm_chip *, uint8_t *, size_t);
int (*send)(struct tpm_chip *, uint8_t *, size_t);
void (*cancel)(struct tpm_chip *);
diff --git a/src/mainboard/google/eve/Kconfig b/src/mainboard/google/eve/Kconfig
index ccffae5..974c7f3 100644
--- a/src/mainboard/google/eve/Kconfig
+++ b/src/mainboard/google/eve/Kconfig
@@ -40,10 +40,6 @@
hex
default 0x50
-config DRIVER_TPM_I2C_IRQ
- int
- default 64 # GPE0_DW2_00 (GPP_E0)
-
config GBB_HWID
string
depends on CHROMEOS
diff --git a/src/mainboard/google/eve/chromeos.c b/src/mainboard/google/eve/chromeos.c
index a77a23e..a66530e 100644
--- a/src/mainboard/google/eve/chromeos.c
+++ b/src/mainboard/google/eve/chromeos.c
@@ -14,9 +14,12 @@
* GNU General Public License for more details.
*/
+#include <arch/acpi.h>
#include <rules.h>
#include <gpio.h>
+#include <soc/gpe.h>
#include <soc/gpio.h>
+#include <tpm.h>
#include <vendorcode/google/chromeos/chromeos.h>
#include "gpio.h"
@@ -54,3 +57,8 @@
{
chromeos_acpi_gpio_generate(cros_gpios, ARRAY_SIZE(cros_gpios));
}
+
+int tis_plat_irq_status(void)
+{
+ return acpi_get_gpe(GPE0_DW2_00); // (GPP_E0)
+}
diff --git a/src/mainboard/google/reef/Kconfig b/src/mainboard/google/reef/Kconfig
index c4bf212..c64f059 100644
--- a/src/mainboard/google/reef/Kconfig
+++ b/src/mainboard/google/reef/Kconfig
@@ -33,10 +33,6 @@
hex
default 0x50
-config DRIVER_TPM_I2C_IRQ
- int
- default 60 # GPE0_DW1_28
-
config VBOOT
select EC_GOOGLE_CHROMEEC_SWITCHES
select HAS_RECOVERY_MRC_CACHE
diff --git a/src/mainboard/google/reef/chromeos.c b/src/mainboard/google/reef/chromeos.c
index 7b7f2b8..ceb0509 100644
--- a/src/mainboard/google/reef/chromeos.c
+++ b/src/mainboard/google/reef/chromeos.c
@@ -13,11 +13,14 @@
* GNU General Public License for more details.
*/
+#include <arch/acpi.h>
#include <baseboard/variants.h>
#include <boot/coreboot_tables.h>
#include <gpio.h>
#include <vendorcode/google/chromeos/chromeos.h>
+#include <soc/gpe.h>
#include <soc/gpio.h>
+#include <tpm.h>
#include <variant/gpio.h>
void fill_lb_gpios(struct lb_gpios *gpios)
@@ -48,3 +51,8 @@
gpios = variant_cros_gpios(&num);
chromeos_acpi_gpio_generate(gpios, num);
}
+
+int tis_plat_irq_status(void)
+{
+ return acpi_get_gpe(GPE0_DW1_28);
+}
--
To view, visit https://review.coreboot.org/19363
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I004329eae1d8aabda51c46b8504bf210484782b4
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurtz <djkurtz at google.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz at chromium.org>
More information about the coreboot-gerrit
mailing list