[PATCH v2] tpm: tpm_tis: Add optional delay after relinquish

[PATCH v2] tpm: tpm_tis: Add optional delay after relinquish

[Jim Broadus]

Some TPMs fail to grant locality when requested immediately after being
relinquished. In this case, the TPM_ACCESS_REQUEST_USE bit of the
TPM_ACCESS register is cleared immediately without setting
TPM_ACCESS_ACTIVE_LOCALITY.

This issue can be seen at boot since tpm_chip_start, called right
after locality is relinquished, fails. This causes the probe to fail:

tpm_tis MSFT0101:00: probe with driver tpm_tis failed with error -1

This occurs on some older Dell Latitudes and maybe others. To work
around this, add a "settle" boolean param to tpm_tis. When this is
enabled, a delay is added after locality is relinquished.

Signed-off-by: Jim Broadus <jbroadus@gmail.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 7 +++++++
 drivers/char/tpm/tpm_tis.c                      | 7 +++++++
 drivers/char/tpm/tpm_tis_core.c                 | 3 +++
 drivers/char/tpm/tpm_tis_core.h                 | 1 +
 4 files changed, 18 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 4d0f545fb3ec..5b7111033fbb 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -7651,6 +7651,13 @@ Kernel parameters
 			defined by Trusted Computing Group (TCG) see
 			https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/
 
+	tpm_tis.settle= [HW,TPM]
+			Format: <bool>
+			When enabled, this adds a delay after locality is
+			relinquished. Some TPMs will fail to grant locality if
+			requested immediately after being relinquished. This
+			causes the probe to fail.
+
 	tp_printk	[FTRACE]
 			Have the tracepoints sent to printk as well as the
 			tracing ring buffer. This is useful for early boot up
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 9aa230a63616..8ac0ea78570e 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -101,6 +101,10 @@ module_param(force, bool, 0444);
 MODULE_PARM_DESC(force, "Force device probe rather than using ACPI entry");
 #endif
 
+static bool settle;
+module_param(settle, bool, 0444);
+MODULE_PARM_DESC(settle, "Add settle time after relinquish");
+
 #if defined(CONFIG_PNP) && defined(CONFIG_ACPI)
 static int has_hid(struct acpi_device *dev, const char *hid)
 {
@@ -242,6 +246,9 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info)
 	if (itpm || is_itpm(ACPI_COMPANION(dev)))
 		set_bit(TPM_TIS_ITPM_WORKAROUND, &phy->priv.flags);
 
+	if (settle)
+		set_bit(TPM_TIS_SETTLE_AFTER_RELINQUISH, &phy->priv.flags);
+
 	return tpm_tis_core_init(dev, &phy->priv, irq, &tpm_tcg,
 				 ACPI_HANDLE(dev));
 }
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 21d79ad3b164..fbeee085098e 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -171,6 +171,9 @@ static int __tpm_tis_relinquish_locality(struct tpm_tis_data *priv, int l)
 {
 	tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
 
+	if (test_bit(TPM_TIS_SETTLE_AFTER_RELINQUISH, &priv->flags))
+		tpm_msleep(TPM_TIMEOUT);
+
 	return 0;
 }
 
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 6c3aa480396b..413cac5e0f31 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -90,6 +90,7 @@ enum tpm_tis_flags {
 	TPM_TIS_DEFAULT_CANCELLATION	= 2,
 	TPM_TIS_IRQ_TESTED		= 3,
 	TPM_TIS_STATUS_VALID_RETRY	= 4,
+	TPM_TIS_SETTLE_AFTER_RELINQUISH	= 5,
 };
 
 struct tpm_tis_data {
-- 
2.54.0

[PATCH v3 0/2] tpm: tpm_tis: Fix probe of some older TPMs

[Jim Broadus]

Changes v3:
- Renamed the series from "Add optional delay after relinquish" to
  "Fix probe of some older TPMs"
- Addressed review comment, saving entire did_vid in priv structure.
- Use did_vid value to identify problem TPM and apply quirk.
Changes v2:
- Move delay from tpm_tis_relinquish_locality to
  __tpm_tis_relinquish_locality.

Link to v2: https://lore.kernel.org/all/20260519154530.6386-1-jbroadus@gmail.com/

Jim Broadus (2):
  tpm: tpm_tis: store entire did_vid
  tpm: tpm_tis: Add settle time for some TPMs

 drivers/char/tpm/tpm_tis_core.c | 28 ++++++++++++++++++++--------
 drivers/char/tpm/tpm_tis_core.h |  3 ++-
 2 files changed, 22 insertions(+), 9 deletions(-)

-- 
2.54.0

[PATCH v3 1/2] tpm: tpm_tis: store entire did_vid

[Jim Broadus]

The entire 32 bit did_vid is read from the device, but only the 16 bit
vendor id portion was stored in the tpm_tis_data structure. Storing the
entire value allows the device id to be used to handle quirks. Printing
the vid and did in the error case also helps identify problem devices.

Signed-off-by: Jim Broadus <jbroadus@gmail.com>
---
 drivers/char/tpm/tpm_tis_core.c | 22 ++++++++++++++--------
 drivers/char/tpm/tpm_tis_core.h |  2 +-
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 21d79ad3b164..d0b87f5daa3e 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -792,9 +792,10 @@ static int probe_itpm(struct tpm_chip *chip)
 static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+	u16 vendor_id = priv->did_vid;
 
 	if (!test_bit(TPM_TIS_DEFAULT_CANCELLATION, &priv->flags)) {
-		switch (priv->manufacturer_id) {
+		switch (vendor_id) {
 		case TPM_VID_WINBOND:
 			return ((status == TPM_STS_VALID) ||
 				(status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)));
@@ -1115,7 +1116,8 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		      const struct tpm_tis_phy_ops *phy_ops,
 		      acpi_handle acpi_dev_handle)
 {
-	u32 vendor;
+	u16 vendor_id;
+	u16 device_id;
 	u32 intfcaps;
 	u32 intmask;
 	u32 clkrun_val;
@@ -1148,19 +1150,20 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 
 	dev_set_drvdata(&chip->dev, priv);
 
-	rc = tpm_tis_read32(priv, TPM_DID_VID(0), &vendor);
+	rc = tpm_tis_read32(priv, TPM_DID_VID(0), &priv->did_vid);
 	if (rc < 0)
 		return rc;
 
-	priv->manufacturer_id = vendor;
+	vendor_id = priv->did_vid;
+	device_id = priv->did_vid >> 16;
 
-	if (priv->manufacturer_id == TPM_VID_ATML &&
+	if (vendor_id == TPM_VID_ATML &&
 		!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
 		priv->timeout_min = TIS_TIMEOUT_MIN_ATML;
 		priv->timeout_max = TIS_TIMEOUT_MAX_ATML;
 	}
 
-	if (priv->manufacturer_id == TPM_VID_IFX)
+	if (vendor_id == TPM_VID_IFX)
 		set_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags);
 
 	if (is_bsw()) {
@@ -1247,9 +1250,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 	if (rc < 0)
 		goto out_err;
 
-	dev_info(dev, "%s TPM (device-id 0x%X, rev-id %d)\n",
+	dev_info(dev, "%s TPM (vendor-id 0x%X, device-id 0x%X, rev-id %d)\n",
 		 (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2",
-		 vendor >> 16, rid);
+		 vendor_id, device_id, rid);
 
 	probe = probe_itpm(chip);
 	if (probe < 0) {
@@ -1315,6 +1318,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 
 	return 0;
 out_err:
+	dev_err(dev, "TPM vid 0x%X, did 0x%X init failed with error %d\n",
+		vendor_id, device_id, rc);
+
 	if (chip->ops->clk_enable != NULL)
 		chip->ops->clk_enable(chip, false);
 
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 6c3aa480396b..f2c77844062a 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -94,7 +94,7 @@ enum tpm_tis_flags {
 
 struct tpm_tis_data {
 	struct tpm_chip *chip;
-	u16 manufacturer_id;
+	u32 did_vid;
 	struct mutex locality_count_mutex;
 	unsigned int locality_count;
 	int locality;
-- 
2.54.0

[PATCH v3 2/2] tpm: tpm_tis: Add settle time for some TPMs

[Jim Broadus]

Some TPMs fail to grant locality when requested immediately after being
relinquished. In this case, the TPM_ACCESS_REQUEST_USE bit of the
TPM_ACCESS register is cleared immediately without setting
TPM_ACCESS_ACTIVE_LOCALITY.

This issue can be seen at boot since tpm_chip_start, called right
after locality is relinquished, will fail. This causes the probe to
fail:

tpm_tis MSFT0101:00: probe with driver tpm_tis failed with error -1

This occurs on some older Dell Latitudes. For the Nuvoton TPM used in
these machines, add a delay after locality is relinquished.

Signed-off-by: Jim Broadus <jbroadus@gmail.com>
---
 drivers/char/tpm/tpm_tis_core.c | 6 ++++++
 drivers/char/tpm/tpm_tis_core.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index d0b87f5daa3e..e0ee98e93ede 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -171,6 +171,9 @@ static int __tpm_tis_relinquish_locality(struct tpm_tis_data *priv, int l)
 {
 	tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
 
+	if (test_bit(TPM_TIS_SETTLE_AFTER_RELINQUISH, &priv->flags))
+		tpm_msleep(TPM_TIMEOUT);
+
 	return 0;
 }
 
@@ -1166,6 +1169,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 	if (vendor_id == TPM_VID_IFX)
 		set_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags);
 
+	if (vendor_id == TPM_VID_WINBOND && device_id == 0x00FE)
+		set_bit(TPM_TIS_SETTLE_AFTER_RELINQUISH, &priv->flags);
+
 	if (is_bsw()) {
 		priv->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR,
 					ILB_REMAP_SIZE);
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index f2c77844062a..aa6d78898ef3 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -90,6 +90,7 @@ enum tpm_tis_flags {
 	TPM_TIS_DEFAULT_CANCELLATION	= 2,
 	TPM_TIS_IRQ_TESTED		= 3,
 	TPM_TIS_STATUS_VALID_RETRY	= 4,
+	TPM_TIS_SETTLE_AFTER_RELINQUISH	= 5,
 };
 
 struct tpm_tis_data {
-- 
2.54.0