From: Lino Sanfilippo <[email protected]>
This series enables IRQ support for the TPM TIS core. For this reason a
number of bugfixes around the interrupt handling are required (patches 1 to
4).
Patches 5 and 6 take into account that according to the TPM Interface
Specification stsValid and commandRead interrupts might not be supported
by the hardware. For this reason the supported interrupts are first queried
and stored (patch 5). Then wait_for_tpm_stat() is adjusted to not wait for
status changes that are not reported by interrupts (patch 6).
Patch 7 addresses the issue with concurrent locality handling:
Since the interrupt handler writes the interrupt status registers it needs
to hold the locality. However it runs concurrently to the thread which
triggered the interrupt (e.g. by reading or writing data to the TPM). So
it must take care when claiming and releasing the locality itself,
because it may race with the concurrent running thread which also claims
and releases the locality.
To avoid that both interrupt and concurrent running thread interfere with
each other a locality counter is used which guarantees that at any time
the locality is held as long as it is required by one of both execution
paths.
Patch 8 implements the request of a threaded interrupt handler. This is
needed since SPI uses a mutex for data transmission and since we access the
interrupt status register via SPI in the irq handler we need a sleepable
context.
In the last version of this series this was only done for SPI. But since
with patch 9 grabbing and releasing the locality includes using a mutex
which protects a locality counter the threaded interrupt is always
needed.
Changes in v5:
- improve commit message of patch 1 as requested by Jarko
- drop patch that makes locality handling simpler by only claiming it at
driver startup and releasing it at driver shutdown (requested by Jarko)
- drop patch that moves the interrupt test from tpm_tis_send()
to tmp_tis_probe_irq_single() as requested by Jarko
- add patch to make locality handling threadsafe so that it can also be
done by the irq handler
- separate logical changes into own patches
- always request threaded interrupt handler
Changes in v4:
- only request threaded irq in case of SPI as requested by Jarko.
- reimplement patch 2 to limit locality handling changes to the TIS core.
- separate fixes from cleanups as requested by Jarko.
- rephrase commit messages
Changes in v3:
- fixed compiler error reported by kernel test robot
- rephrased commit message as suggested by Jarko Sakkinen
- added Reviewed-by tag
Changes in v2:
- rebase against 5.12
- free irq on error path
Lino Sanfilippo (10):
tpm, tpm_tis: Avoid cache incoherency in test for interrupts
tpm, tpm_tis: Claim locality before writing TPM_INT_ENABLE register
tpm, tpm_tis: Disable interrupts if tpm_tis_probe_irq() failed
tpm, tmp_tis: Claim locality before writing interrupt registers
tpm, tpm_tis: Store result of interrupt capability query
tpm, tpm_tis: Only handle supported interrupts in wait_for_tpm_stat()
tmp, tmp_tis: Implement usage counter for locality
tpm, tpm_tis: Request threaded interrupt handler
tpm, tpm_tis: Claim locality in interrupt handler
tpm, tpm_tis: Enable interrupt test
drivers/char/tpm/tpm_tis_core.c | 217 ++++++++++++++++++++++----------
drivers/char/tpm/tpm_tis_core.h | 9 +-
2 files changed, 156 insertions(+), 70 deletions(-)
base-commit: 4b0986a3613c92f4ec1bdc7f60ec66fea135991f
--
2.36.1
From: Lino Sanfilippo <[email protected]>
In disable_interrupts() the TPM_GLOBAL_INT_ENABLE bit is unset in the
TPM_INT_ENABLE register to shut the interrupts off. However modifying the
register is only possible with a held locality. So claim the locality
before disable_interrupts() is called.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 6f2cf75add8b..ee6b48c55ac9 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1084,7 +1084,11 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
dev_err(&chip->dev, FW_BUG
"TPM interrupt not working, polling instead\n");
+ rc = request_locality(chip, 0);
+ if (rc < 0)
+ goto out_err;
disable_interrupts(chip);
+ release_locality(chip, 0);
}
} else {
tpm_tis_probe_irq(chip, intmask);
--
2.36.1
From: Lino Sanfilippo <[email protected]>
The test for interrupts in tpm_tis_send() is skipped if the flag
TPM_CHIP_FLAG_IRQ is not set. Since the current code never sets the flag
initially the test is never executed.
Fix this by setting the flag in tpm_tis_gen_interrupt() right after
interrupts have been enabled.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index e5edf745fb23..be229c173f10 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -774,11 +774,16 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
if (ret < 0)
return ret;
+ chip->flags |= TPM_CHIP_FLAG_IRQ;
+
if (chip->flags & TPM_CHIP_FLAG_TPM2)
ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
else
ret = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0);
+ if (ret)
+ chip->flags &= ~TPM_CHIP_FLAG_IRQ;
+
release_locality(chip, 0);
return ret;
--
2.36.1
From: Lino Sanfilippo <[email protected]>
The interrupt handler that sets the boolean variable irq_tested may run on
another CPU as the thread that checks irq_tested as part of the irq test in
tmp_tis_send().
Since nothing guarantees cache coherency between CPUs for unsynchronized
accesses to boolean variables the testing thread might not perceive the
value change done in the interrupt handler.
Avoid this issue by using a bitfield instead of a boolean variable and by
accessing this field with the bit manipulating functions that provide cache
coherency.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 13 +++++++------
drivers/char/tpm/tpm_tis_core.h | 6 +++++-
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index dc56b976d816..6f2cf75add8b 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -470,7 +470,8 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
int rc, irq;
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
- if (!(chip->flags & TPM_CHIP_FLAG_IRQ) || priv->irq_tested)
+ if (!(chip->flags & TPM_CHIP_FLAG_IRQ) ||
+ test_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags))
return tpm_tis_send_main(chip, buf, len);
/* Verify receipt of the expected IRQ */
@@ -480,11 +481,11 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
rc = tpm_tis_send_main(chip, buf, len);
priv->irq = irq;
chip->flags |= TPM_CHIP_FLAG_IRQ;
- if (!priv->irq_tested)
+ if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags))
tpm_msleep(1);
- if (!priv->irq_tested)
+ if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags))
disable_interrupts(chip);
- priv->irq_tested = true;
+ set_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags);
return rc;
}
@@ -693,7 +694,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
if (interrupt == 0)
return IRQ_NONE;
- priv->irq_tested = true;
+ set_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags);
if (interrupt & TPM_INTF_DATA_AVAIL_INT)
wake_up_interruptible(&priv->read_queue);
if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT)
@@ -779,7 +780,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
if (rc < 0)
return rc;
- priv->irq_tested = false;
+ clear_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags);
/* Generate an interrupt by having the core call through to
* tpm_tis_send
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 3be24f221e32..0f29d0b68c3e 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -88,11 +88,15 @@ enum tpm_tis_flags {
TPM_TIS_INVALID_STATUS = BIT(1),
};
+enum tpm_tis_irqtest_flags {
+ TPM_TIS_IRQ_TESTED = BIT(0),
+};
+
struct tpm_tis_data {
u16 manufacturer_id;
int locality;
int irq;
- bool irq_tested;
+ unsigned long irqtest_flags;
unsigned long flags;
void __iomem *ilb_base_addr;
u16 clkrun_enabled;
--
2.36.1
From: Lino Sanfilippo <[email protected]>
According to the TPM Interface Specification (TIS) support for "stsValid"
and "commandReady" interrupts is only optional.
This has to be taken into account when handling the interrupts in functions
like wait_for_tpm_stat(). So query the set of supported interrupts and
store it in a global variable so that it can be accessed later.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 73 +++++++++++++++++----------------
drivers/char/tpm/tpm_tis_core.h | 1 +
2 files changed, 38 insertions(+), 36 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 718525fcadc0..2f03fefa1706 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1007,8 +1007,39 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
if (rc < 0)
goto out_err;
- intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
- TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
+ /* Figure out the capabilities */
+ rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
+ if (rc < 0)
+ goto out_err;
+
+ dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
+ intfcaps);
+ if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
+ dev_dbg(dev, "\tBurst Count Static\n");
+ if (intfcaps & TPM_INTF_CMD_READY_INT) {
+ intmask |= TPM_INTF_CMD_READY_INT;
+ dev_dbg(dev, "\tCommand Ready Int Support\n");
+ }
+ if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
+ dev_dbg(dev, "\tInterrupt Edge Falling\n");
+ if (intfcaps & TPM_INTF_INT_EDGE_RISING)
+ dev_dbg(dev, "\tInterrupt Edge Rising\n");
+ if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
+ dev_dbg(dev, "\tInterrupt Level Low\n");
+ if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
+ dev_dbg(dev, "\tInterrupt Level High\n");
+ if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT)
+ intmask |= TPM_INTF_LOCALITY_CHANGE_INT;
+ dev_dbg(dev, "\tLocality Change Int Support\n");
+ if (intfcaps & TPM_INTF_STS_VALID_INT) {
+ intmask |= TPM_INTF_STS_VALID_INT;
+ dev_dbg(dev, "\tSts Valid Int Support\n");
+ }
+ if (intfcaps & TPM_INTF_DATA_AVAIL_INT) {
+ intmask |= TPM_INTF_DATA_AVAIL_INT;
+ dev_dbg(dev, "\tData Avail Int Support\n");
+ }
+
intmask &= ~TPM_GLOBAL_INT_ENABLE;
rc = request_locality(chip, 0);
@@ -1042,32 +1073,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
goto out_err;
}
- /* Figure out the capabilities */
- rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
- if (rc < 0)
- goto out_err;
-
- dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
- intfcaps);
- if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
- dev_dbg(dev, "\tBurst Count Static\n");
- if (intfcaps & TPM_INTF_CMD_READY_INT)
- dev_dbg(dev, "\tCommand Ready Int Support\n");
- if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
- dev_dbg(dev, "\tInterrupt Edge Falling\n");
- if (intfcaps & TPM_INTF_INT_EDGE_RISING)
- dev_dbg(dev, "\tInterrupt Edge Rising\n");
- if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
- dev_dbg(dev, "\tInterrupt Level Low\n");
- if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
- dev_dbg(dev, "\tInterrupt Level High\n");
- if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT)
- dev_dbg(dev, "\tLocality Change Int Support\n");
- if (intfcaps & TPM_INTF_STS_VALID_INT)
- dev_dbg(dev, "\tSts Valid Int Support\n");
- if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
- dev_dbg(dev, "\tData Avail Int Support\n");
-
/* INTERRUPT Setup */
init_waitqueue_head(&priv->read_queue);
init_waitqueue_head(&priv->int_queue);
@@ -1098,7 +1103,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
else
tpm_tis_probe_irq(chip, intmask);
- if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
+ if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ priv->irqs_in_use = intmask;
+ } else {
dev_err(&chip->dev, FW_BUG
"TPM interrupt not working, polling instead\n");
@@ -1145,13 +1152,7 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
if (rc < 0)
goto out;
- rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
- if (rc < 0)
- goto out;
-
- intmask |= TPM_INTF_CMD_READY_INT
- | TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_DATA_AVAIL_INT
- | TPM_INTF_STS_VALID_INT | TPM_GLOBAL_INT_ENABLE;
+ intmask = priv->irqs_in_use | TPM_GLOBAL_INT_ENABLE;
tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 0f29d0b68c3e..8e02faa4079d 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -96,6 +96,7 @@ struct tpm_tis_data {
u16 manufacturer_id;
int locality;
int irq;
+ unsigned int irqs_in_use;
unsigned long irqtest_flags;
unsigned long flags;
void __iomem *ilb_base_addr;
--
2.36.1
From: Lino Sanfilippo <[email protected]>
The TIS interrupt handler at least has to read and write the interrupt
status register. In case of SPI both operations result in a call to
tpm_tis_spi_transfer() which uses the bus_lock_mutex of the spi device
and thus must only be called from a sleepable context.
To ensure this request a threaded interrupt handler.
Fixes: 1a339b658d9d ("tpm_tis_spi: Pass the SPI IRQ down to the driver")
Signed-off-by: Lino Sanfilippo <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 0ef74979bc2c..8b5aa4fdbe92 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -794,8 +794,11 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
int rc;
u32 int_status;
- if (devm_request_irq(chip->dev.parent, irq, tis_int_handler, flags,
- dev_name(&chip->dev), chip) != 0) {
+
+ rc = devm_request_threaded_irq(chip->dev.parent, irq, NULL,
+ tis_int_handler, IRQF_ONESHOT | flags,
+ dev_name(&chip->dev), chip);
+ if (rc) {
dev_info(&chip->dev, "Unable to request irq: %d for probe\n",
irq);
return -1;
--
2.36.1
From: Lino Sanfilippo <[email protected]>
According to the TPM Interface Specification (TIS) interrupts for
"stsValid" and "commandReady" might not be supported.
Take this into account and only wait for interrupts which are actually in
use in wait_for_tpm_stat(). After that process all the remaining status
changes by polling the status register.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 46 ++++++++++++++++++++++++---------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 2f03fefa1706..028bec44362d 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -53,41 +53,63 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
long rc;
u8 status;
bool canceled = false;
+ u8 active_irqs = 0;
+ int ret = 0;
/* check current status */
status = chip->ops->status(chip);
if ((status & mask) == mask)
return 0;
- stop = jiffies + timeout;
+ /* check what status changes can be handled by irqs */
+ if (priv->irqs_in_use & TPM_INTF_STS_VALID_INT)
+ active_irqs |= TPM_STS_VALID;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (priv->irqs_in_use & TPM_INTF_DATA_AVAIL_INT)
+ active_irqs |= TPM_STS_DATA_AVAIL;
+
+ if (priv->irqs_in_use & TPM_INTF_CMD_READY_INT)
+ active_irqs |= TPM_STS_COMMAND_READY;
+
+ active_irqs &= mask;
+
+ stop = jiffies + timeout;
+ /* process status changes with irq support */
+ if (active_irqs) {
+ ret = -ETIME;
again:
timeout = stop - jiffies;
if ((long)timeout <= 0)
return -ETIME;
rc = wait_event_interruptible_timeout(*queue,
- wait_for_tpm_stat_cond(chip, mask, check_cancel,
+ wait_for_tpm_stat_cond(chip, active_irqs, check_cancel,
&canceled),
timeout);
if (rc > 0) {
if (canceled)
return -ECANCELED;
- return 0;
+ ret = 0;
}
if (rc == -ERESTARTSYS && freezing(current)) {
clear_thread_flag(TIF_SIGPENDING);
goto again;
}
- } else {
- do {
- usleep_range(priv->timeout_min,
- priv->timeout_max);
- status = chip->ops->status(chip);
- if ((status & mask) == mask)
- return 0;
- } while (time_before(jiffies, stop));
}
+
+ if (ret)
+ return ret;
+
+ mask &= ~active_irqs;
+ if (!mask) /* all done */
+ return 0;
+ /* process status changes without irq support */
+ do {
+ status = chip->ops->status(chip);
+ if ((status & mask) == mask)
+ return 0;
+ usleep_range(priv->timeout_min,
+ priv->timeout_max);
+ } while (time_before(jiffies, stop));
return -ETIME;
}
--
2.36.1
From: Lino Sanfilippo <[email protected]>
Implement a usage counter for the (default) locality used by the TPM TIS
driver:
Request the locality from the TPM if it has not been claimed yet, otherwise
only increment the counter. Also release the locality if the counter is 0
otherwise only decrement the counter. Ensure thread-safety by protecting
the counter with a mutex.
This allows to request and release the locality from a thread and the
interrupt handler at the same time without the danger to interfere with
each other.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 30 ++++++++++++++++++++++++++++--
drivers/char/tpm/tpm_tis_core.h | 2 ++
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 028bec44362d..0ef74979bc2c 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -158,16 +158,26 @@ static bool check_locality(struct tpm_chip *chip, int l)
return false;
}
+static int release_locality_locked(struct tpm_tis_data *priv, int l)
+{
+ tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
+ return 0;
+}
+
static int release_locality(struct tpm_chip *chip, int l)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
- tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
+ mutex_lock(&priv->locality_count_mutex);
+ priv->locality_count--;
+ if (priv->locality_count == 0)
+ release_locality_locked(priv, l);
+ mutex_unlock(&priv->locality_count_mutex);
return 0;
}
-static int request_locality(struct tpm_chip *chip, int l)
+static int request_locality_locked(struct tpm_chip *chip, int l)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
unsigned long stop, timeout;
@@ -208,6 +218,20 @@ static int request_locality(struct tpm_chip *chip, int l)
return -1;
}
+static int request_locality(struct tpm_chip *chip, int l)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ int ret = 0;
+
+ mutex_lock(&priv->locality_count_mutex);
+ if (priv->locality_count == 0)
+ ret = request_locality_locked(chip, l);
+ if (!ret)
+ priv->locality_count++;
+ mutex_unlock(&priv->locality_count_mutex);
+ return ret;
+}
+
static u8 tpm_tis_status(struct tpm_chip *chip)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
@@ -987,6 +1011,8 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
priv->timeout_min = TPM_TIMEOUT_USECS_MIN;
priv->timeout_max = TPM_TIMEOUT_USECS_MAX;
priv->phy_ops = phy_ops;
+ priv->locality_count = 0;
+ mutex_init(&priv->locality_count_mutex);
dev_set_drvdata(&chip->dev, priv);
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 8e02faa4079d..e1871c482da2 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -94,6 +94,8 @@ enum tpm_tis_irqtest_flags {
struct tpm_tis_data {
u16 manufacturer_id;
+ struct mutex locality_count_mutex;
+ unsigned int locality_count;
int locality;
int irq;
unsigned int irqs_in_use;
--
2.36.1
On Fri, Jun 10, 2022 at 01:08:37PM +0200, [email protected] wrote:
> From: Lino Sanfilippo <[email protected]>
>
> The interrupt handler that sets the boolean variable irq_tested may run on
> another CPU as the thread that checks irq_tested as part of the irq test in
> tmp_tis_send().
>
> Since nothing guarantees cache coherency between CPUs for unsynchronized
> accesses to boolean variables the testing thread might not perceive the
> value change done in the interrupt handler.
>
> Avoid this issue by using a bitfield instead of a boolean variable and by
> accessing this field with the bit manipulating functions that provide cache
> coherency.
>
> Signed-off-by: Lino Sanfilippo <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_core.c | 13 +++++++------
> drivers/char/tpm/tpm_tis_core.h | 6 +++++-
> 2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index dc56b976d816..6f2cf75add8b 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -470,7 +470,8 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
> int rc, irq;
> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>
> - if (!(chip->flags & TPM_CHIP_FLAG_IRQ) || priv->irq_tested)
> + if (!(chip->flags & TPM_CHIP_FLAG_IRQ) ||
> + test_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags))
> return tpm_tis_send_main(chip, buf, len);
>
> /* Verify receipt of the expected IRQ */
> @@ -480,11 +481,11 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
> rc = tpm_tis_send_main(chip, buf, len);
> priv->irq = irq;
> chip->flags |= TPM_CHIP_FLAG_IRQ;
> - if (!priv->irq_tested)
> + if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags))
> tpm_msleep(1);
> - if (!priv->irq_tested)
> + if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags))
> disable_interrupts(chip);
> - priv->irq_tested = true;
> + set_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags);
> return rc;
> }
>
> @@ -693,7 +694,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
> if (interrupt == 0)
> return IRQ_NONE;
>
> - priv->irq_tested = true;
> + set_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags);
> if (interrupt & TPM_INTF_DATA_AVAIL_INT)
> wake_up_interruptible(&priv->read_queue);
> if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT)
> @@ -779,7 +780,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
> if (rc < 0)
> return rc;
>
> - priv->irq_tested = false;
> + clear_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags);
>
> /* Generate an interrupt by having the core call through to
> * tpm_tis_send
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 3be24f221e32..0f29d0b68c3e 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -88,11 +88,15 @@ enum tpm_tis_flags {
> TPM_TIS_INVALID_STATUS = BIT(1),
> };
>
> +enum tpm_tis_irqtest_flags {
> + TPM_TIS_IRQ_TESTED = BIT(0),
> +};
> +
> struct tpm_tis_data {
> u16 manufacturer_id;
> int locality;
> int irq;
> - bool irq_tested;
> + unsigned long irqtest_flags;
> unsigned long flags;
> void __iomem *ilb_base_addr;
> u16 clkrun_enabled;
> --
> 2.36.1
>
Otherwise looks fine, but please add TPM_TIS_IRQ_TESTED to 'flags', and
convert existing sites to use set_bit() and and test_bit().
BR, Jarkko
On Fri, Jun 10, 2022 at 01:08:38PM +0200, [email protected] wrote:
> From: Lino Sanfilippo <[email protected]>
>
> In disable_interrupts() the TPM_GLOBAL_INT_ENABLE bit is unset in the
> TPM_INT_ENABLE register to shut the interrupts off. However modifying the
> register is only possible with a held locality. So claim the locality
> before disable_interrupts() is called.
>
> Signed-off-by: Lino Sanfilippo <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 6f2cf75add8b..ee6b48c55ac9 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -1084,7 +1084,11 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
> dev_err(&chip->dev, FW_BUG
> "TPM interrupt not working, polling instead\n");
>
> + rc = request_locality(chip, 0);
> + if (rc < 0)
> + goto out_err;
> disable_interrupts(chip);
> + release_locality(chip, 0);
> }
> } else {
> tpm_tis_probe_irq(chip, intmask);
> --
> 2.36.1
>
Reviewed-by: Jarkko Sakkinen <[email protected]>
BR, Jarkko
On Fri, Jun 10, 2022 at 01:08:42PM +0200, [email protected] wrote:
> From: Lino Sanfilippo <[email protected]>
>
> According to the TPM Interface Specification (TIS) interrupts for
> "stsValid" and "commandReady" might not be supported.
>
> Take this into account and only wait for interrupts which are actually in
> use in wait_for_tpm_stat(). After that process all the remaining status
> changes by polling the status register.
>
> Signed-off-by: Lino Sanfilippo <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_core.c | 46 ++++++++++++++++++++++++---------
> 1 file changed, 34 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 2f03fefa1706..028bec44362d 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -53,41 +53,63 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
> long rc;
> u8 status;
> bool canceled = false;
> + u8 active_irqs = 0;
Something like sts_mask would be a better name. Calling it
'active_irqs' is a bit confusing.
> + int ret = 0;
>
> /* check current status */
> status = chip->ops->status(chip);
> if ((status & mask) == mask)
> return 0;
>
> - stop = jiffies + timeout;
> + /* check what status changes can be handled by irqs */
> + if (priv->irqs_in_use & TPM_INTF_STS_VALID_INT)
> + active_irqs |= TPM_STS_VALID;
>
> - if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> + if (priv->irqs_in_use & TPM_INTF_DATA_AVAIL_INT)
> + active_irqs |= TPM_STS_DATA_AVAIL;
> +
> + if (priv->irqs_in_use & TPM_INTF_CMD_READY_INT)
> + active_irqs |= TPM_STS_COMMAND_READY;
> +
> + active_irqs &= mask;
> +
> + stop = jiffies + timeout;
> + /* process status changes with irq support */
> + if (active_irqs) {
> + ret = -ETIME;
> again:
> timeout = stop - jiffies;
> if ((long)timeout <= 0)
> return -ETIME;
> rc = wait_event_interruptible_timeout(*queue,
> - wait_for_tpm_stat_cond(chip, mask, check_cancel,
> + wait_for_tpm_stat_cond(chip, active_irqs, check_cancel,
> &canceled),
> timeout);
> if (rc > 0) {
> if (canceled)
> return -ECANCELED;
> - return 0;
> + ret = 0;
> }
> if (rc == -ERESTARTSYS && freezing(current)) {
> clear_thread_flag(TIF_SIGPENDING);
> goto again;
> }
> - } else {
> - do {
> - usleep_range(priv->timeout_min,
> - priv->timeout_max);
> - status = chip->ops->status(chip);
> - if ((status & mask) == mask)
> - return 0;
> - } while (time_before(jiffies, stop));
> }
> +
> + if (ret)
> + return ret;
> +
> + mask &= ~active_irqs;
> + if (!mask) /* all done */
> + return 0;
> + /* process status changes without irq support */
> + do {
> + status = chip->ops->status(chip);
> + if ((status & mask) == mask)
> + return 0;
> + usleep_range(priv->timeout_min,
> + priv->timeout_max);
> + } while (time_before(jiffies, stop));
> return -ETIME;
> }
>
> --
> 2.36.1
>
BR, Jarkko
On Fri, Jun 10, 2022 at 01:08:41PM +0200, [email protected] wrote:
> From: Lino Sanfilippo <[email protected]>
>
> According to the TPM Interface Specification (TIS) support for "stsValid"
> and "commandReady" interrupts is only optional.
> This has to be taken into account when handling the interrupts in functions
> like wait_for_tpm_stat(). So query the set of supported interrupts and
> store it in a global variable so that it can be accessed later.
>
> Signed-off-by: Lino Sanfilippo <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_core.c | 73 +++++++++++++++++----------------
> drivers/char/tpm/tpm_tis_core.h | 1 +
> 2 files changed, 38 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 718525fcadc0..2f03fefa1706 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -1007,8 +1007,39 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
> if (rc < 0)
> goto out_err;
>
> - intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
> - TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
> + /* Figure out the capabilities */
> + rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
> + if (rc < 0)
> + goto out_err;
> +
> + dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
> + intfcaps);
> + if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
> + dev_dbg(dev, "\tBurst Count Static\n");
> + if (intfcaps & TPM_INTF_CMD_READY_INT) {
> + intmask |= TPM_INTF_CMD_READY_INT;
> + dev_dbg(dev, "\tCommand Ready Int Support\n");
> + }
> + if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
> + dev_dbg(dev, "\tInterrupt Edge Falling\n");
> + if (intfcaps & TPM_INTF_INT_EDGE_RISING)
> + dev_dbg(dev, "\tInterrupt Edge Rising\n");
> + if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
> + dev_dbg(dev, "\tInterrupt Level Low\n");
> + if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
> + dev_dbg(dev, "\tInterrupt Level High\n");
> + if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT)
> + intmask |= TPM_INTF_LOCALITY_CHANGE_INT;
> + dev_dbg(dev, "\tLocality Change Int Support\n");
> + if (intfcaps & TPM_INTF_STS_VALID_INT) {
> + intmask |= TPM_INTF_STS_VALID_INT;
> + dev_dbg(dev, "\tSts Valid Int Support\n");
> + }
> + if (intfcaps & TPM_INTF_DATA_AVAIL_INT) {
> + intmask |= TPM_INTF_DATA_AVAIL_INT;
> + dev_dbg(dev, "\tData Avail Int Support\n");
> + }
> +
> intmask &= ~TPM_GLOBAL_INT_ENABLE;
>
> rc = request_locality(chip, 0);
> @@ -1042,32 +1073,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
> goto out_err;
> }
>
> - /* Figure out the capabilities */
> - rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
> - if (rc < 0)
> - goto out_err;
> -
> - dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
> - intfcaps);
> - if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
> - dev_dbg(dev, "\tBurst Count Static\n");
> - if (intfcaps & TPM_INTF_CMD_READY_INT)
> - dev_dbg(dev, "\tCommand Ready Int Support\n");
> - if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
> - dev_dbg(dev, "\tInterrupt Edge Falling\n");
> - if (intfcaps & TPM_INTF_INT_EDGE_RISING)
> - dev_dbg(dev, "\tInterrupt Edge Rising\n");
> - if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
> - dev_dbg(dev, "\tInterrupt Level Low\n");
> - if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
> - dev_dbg(dev, "\tInterrupt Level High\n");
> - if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT)
> - dev_dbg(dev, "\tLocality Change Int Support\n");
> - if (intfcaps & TPM_INTF_STS_VALID_INT)
> - dev_dbg(dev, "\tSts Valid Int Support\n");
> - if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
> - dev_dbg(dev, "\tData Avail Int Support\n");
> -
> /* INTERRUPT Setup */
> init_waitqueue_head(&priv->read_queue);
> init_waitqueue_head(&priv->int_queue);
> @@ -1098,7 +1103,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
> else
> tpm_tis_probe_irq(chip, intmask);
>
> - if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
> + if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> + priv->irqs_in_use = intmask;
> + } else {
> dev_err(&chip->dev, FW_BUG
> "TPM interrupt not working, polling instead\n");
>
> @@ -1145,13 +1152,7 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
> if (rc < 0)
> goto out;
>
> - rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
> - if (rc < 0)
> - goto out;
> -
> - intmask |= TPM_INTF_CMD_READY_INT
> - | TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_DATA_AVAIL_INT
> - | TPM_INTF_STS_VALID_INT | TPM_GLOBAL_INT_ENABLE;
> + intmask = priv->irqs_in_use | TPM_GLOBAL_INT_ENABLE;
>
> tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
>
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 0f29d0b68c3e..8e02faa4079d 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -96,6 +96,7 @@ struct tpm_tis_data {
> u16 manufacturer_id;
> int locality;
> int irq;
> + unsigned int irqs_in_use;
> unsigned long irqtest_flags;
> unsigned long flags;
> void __iomem *ilb_base_addr;
> --
> 2.36.1
>
int_mask would be imho a better name.
Can you squash this and the following patch? It's good to slice in
small patches but here I think it would make sense to tie the field
to use case, in order for it to make sense.
BR, Jarkko
On Fri, Jun 10, 2022 at 01:08:46PM +0200, [email protected] wrote:
> From: Lino Sanfilippo <[email protected]>
>
> The test for interrupts in tpm_tis_send() is skipped if the flag
> TPM_CHIP_FLAG_IRQ is not set. Since the current code never sets the flag
> initially the test is never executed.
>
> Fix this by setting the flag in tpm_tis_gen_interrupt() right after
> interrupts have been enabled.
>
> Signed-off-by: Lino Sanfilippo <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_core.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index e5edf745fb23..be229c173f10 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -774,11 +774,16 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
> if (ret < 0)
> return ret;
>
> + chip->flags |= TPM_CHIP_FLAG_IRQ;
> +
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
> else
> ret = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0);
>
> + if (ret)
> + chip->flags &= ~TPM_CHIP_FLAG_IRQ;
> +
> release_locality(chip, 0);
>
> return ret;
> --
> 2.36.1
>
James, do you have a chance to test this on your side? Thanks.
BR, Jarkko
On Fri, Jun 10, 2022 at 01:08:43PM +0200, [email protected] wrote:
> From: Lino Sanfilippo <[email protected]>
>
> Implement a usage counter for the (default) locality used by the TPM TIS
> driver:
> Request the locality from the TPM if it has not been claimed yet, otherwise
> only increment the counter. Also release the locality if the counter is 0
> otherwise only decrement the counter. Ensure thread-safety by protecting
> the counter with a mutex.
>
> This allows to request and release the locality from a thread and the
> interrupt handler at the same time without the danger to interfere with
> each other.
>
> Signed-off-by: Lino Sanfilippo <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_core.c | 30 ++++++++++++++++++++++++++++--
> drivers/char/tpm/tpm_tis_core.h | 2 ++
> 2 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 028bec44362d..0ef74979bc2c 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -158,16 +158,26 @@ static bool check_locality(struct tpm_chip *chip, int l)
> return false;
> }
>
> +static int release_locality_locked(struct tpm_tis_data *priv, int l)
> +{
> + tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
nit: empty line here
Also it would not hurt to prefix it with tpm_tis.
This is for simple and practical reasons, like grepping. I don't
mind if you do that also for other functions (if you want to).
> + return 0;
> +}
> +
> static int release_locality(struct tpm_chip *chip, int l)
> {
> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>
> - tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
> + mutex_lock(&priv->locality_count_mutex);
> + priv->locality_count--;
> + if (priv->locality_count == 0)
> + release_locality_locked(priv, l);
> + mutex_unlock(&priv->locality_count_mutex);
>
> return 0;
> }
>
> -static int request_locality(struct tpm_chip *chip, int l)
> +static int request_locality_locked(struct tpm_chip *chip, int l)
> {
> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> unsigned long stop, timeout;
> @@ -208,6 +218,20 @@ static int request_locality(struct tpm_chip *chip, int l)
> return -1;
> }
>
> +static int request_locality(struct tpm_chip *chip, int l)
> +{
> + struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> + int ret = 0;
> +
> + mutex_lock(&priv->locality_count_mutex);
> + if (priv->locality_count == 0)
> + ret = request_locality_locked(chip, l);
> + if (!ret)
> + priv->locality_count++;
> + mutex_unlock(&priv->locality_count_mutex);
> + return ret;
> +}
> +
> static u8 tpm_tis_status(struct tpm_chip *chip)
> {
> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> @@ -987,6 +1011,8 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
> priv->timeout_min = TPM_TIMEOUT_USECS_MIN;
> priv->timeout_max = TPM_TIMEOUT_USECS_MAX;
> priv->phy_ops = phy_ops;
> + priv->locality_count = 0;
> + mutex_init(&priv->locality_count_mutex);
>
> dev_set_drvdata(&chip->dev, priv);
>
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index 8e02faa4079d..e1871c482da2 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -94,6 +94,8 @@ enum tpm_tis_irqtest_flags {
>
> struct tpm_tis_data {
> u16 manufacturer_id;
> + struct mutex locality_count_mutex;
> + unsigned int locality_count;
> int locality;
> int irq;
> unsigned int irqs_in_use;
> --
> 2.36.1
>
BR, Jarkko
On Fri, Jun 10, 2022 at 01:08:44PM +0200, [email protected] wrote:
> From: Lino Sanfilippo <[email protected]>
>
> The TIS interrupt handler at least has to read and write the interrupt
> status register. In case of SPI both operations result in a call to
> tpm_tis_spi_transfer() which uses the bus_lock_mutex of the spi device
> and thus must only be called from a sleepable context.
>
> To ensure this request a threaded interrupt handler.
>
> Fixes: 1a339b658d9d ("tpm_tis_spi: Pass the SPI IRQ down to the driver")
> Signed-off-by: Lino Sanfilippo <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_core.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 0ef74979bc2c..8b5aa4fdbe92 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -794,8 +794,11 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
> int rc;
> u32 int_status;
>
> - if (devm_request_irq(chip->dev.parent, irq, tis_int_handler, flags,
> - dev_name(&chip->dev), chip) != 0) {
> +
> + rc = devm_request_threaded_irq(chip->dev.parent, irq, NULL,
> + tis_int_handler, IRQF_ONESHOT | flags,
> + dev_name(&chip->dev), chip);
> + if (rc) {
> dev_info(&chip->dev, "Unable to request irq: %d for probe\n",
> irq);
> return -1;
> --
> 2.36.1
>
I actually would not add fixes tag to this because given that
interrupt support is quite unusable, this should not cause any
harm.
The code change itself is fine.
BR, Jarkko
On Wed, 2022-06-15 at 21:26 +0300, Jarkko Sakkinen wrote:
> On Fri, Jun 10, 2022 at 01:08:46PM +0200, [email protected] wrote:
> > From: Lino Sanfilippo <[email protected]>
> >
> > The test for interrupts in tpm_tis_send() is skipped if the flag
> > TPM_CHIP_FLAG_IRQ is not set. Since the current code never sets the flag
> > initially the test is never executed.
> >
> > Fix this by setting the flag in tpm_tis_gen_interrupt() right after
> > interrupts have been enabled.
> >
> > Signed-off-by: Lino Sanfilippo <[email protected]>
> > ---
> > drivers/char/tpm/tpm_tis_core.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/char/tpm/tpm_tis_core.c
> > b/drivers/char/tpm/tpm_tis_core.c
> > index e5edf745fb23..be229c173f10 100644
> > --- a/drivers/char/tpm/tpm_tis_core.c
> > +++ b/drivers/char/tpm/tpm_tis_core.c
> > @@ -774,11 +774,16 @@ static int tpm_tis_gen_interrupt(struct tpm_chip
> > *chip)
> > if (ret < 0)
> > return ret;
> >
> > + chip->flags |= TPM_CHIP_FLAG_IRQ;
> > +
> > if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
> > else
> > ret = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
> > desc, 0);
> >
> > + if (ret)
> > + chip->flags &= ~TPM_CHIP_FLAG_IRQ;
> > +
> > release_locality(chip, 0);
> >
> > return ret;
> > --
> > 2.36.1
> >
>
> James, do you have a chance to test this on your side? Thanks.
>
> BR, Jarkko
Hi guys,
for me this series causes boot problems - somehow feels like an interrupt
storm...
Not sure yet, which commit is causing that. I'll do more testing tomorrow.
@Tim could you test on any of your devices, please?
BR
Michael
On 15.06.22 at 20:21, Jarkko Sakkinen wrote:
> On Fri, Jun 10, 2022 at 01:08:42PM +0200, [email protected] wrote:
>> From: Lino Sanfilippo <[email protected]>
>>
>> According to the TPM Interface Specification (TIS) interrupts for
>> "stsValid" and "commandReady" might not be supported.
>>
>> Take this into account and only wait for interrupts which are actually in
>> use in wait_for_tpm_stat(). After that process all the remaining status
>> changes by polling the status register.
>>
>> Signed-off-by: Lino Sanfilippo <[email protected]>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 46 ++++++++++++++++++++++++---------
>> 1 file changed, 34 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index 2f03fefa1706..028bec44362d 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -53,41 +53,63 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
>> long rc;
>> u8 status;
>> bool canceled = false;
>> + u8 active_irqs = 0;
>
> Something like sts_mask would be a better name. Calling it
> 'active_irqs' is a bit confusing.
Ok.
Regards,
Lino
On 15.06.22 at 20:23, Jarkko Sakkinen wrote:
> On Fri, Jun 10, 2022 at 01:08:43PM +0200, [email protected] wrote:
>> From: Lino Sanfilippo <[email protected]>
>>
>> Implement a usage counter for the (default) locality used by the TPM TIS
>> driver:
>> Request the locality from the TPM if it has not been claimed yet, otherwise
>> only increment the counter. Also release the locality if the counter is 0
>> otherwise only decrement the counter. Ensure thread-safety by protecting
>> the counter with a mutex.
>>
>> This allows to request and release the locality from a thread and the
>> interrupt handler at the same time without the danger to interfere with
>> each other.
>>
>> Signed-off-by: Lino Sanfilippo <[email protected]>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 30 ++++++++++++++++++++++++++++--
>> drivers/char/tpm/tpm_tis_core.h | 2 ++
>> 2 files changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index 028bec44362d..0ef74979bc2c 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -158,16 +158,26 @@ static bool check_locality(struct tpm_chip *chip, int l)
>> return false;
>> }
>>
>> +static int release_locality_locked(struct tpm_tis_data *priv, int l)
>> +{
>> + tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
>
> nit: empty line here
>
> Also it would not hurt to prefix it with tpm_tis.
>
> This is for simple and practical reasons, like grepping. I don't
> mind if you do that also for other functions (if you want to).
>
Ok, will do so.
Regards,
Lino
Hi,
On 15.06.22 at 16:32, Jarkko Sakkinen wrote:
> On Fri, Jun 10, 2022 at 01:08:37PM +0200, [email protected] wrote:
>> From: Lino Sanfilippo <[email protected]>
>>
>> The interrupt handler that sets the boolean variable irq_tested may run on
>> another CPU as the thread that checks irq_tested as part of the irq test in
>> tmp_tis_send().
>>
>> Since nothing guarantees cache coherency between CPUs for unsynchronized
>> accesses to boolean variables the testing thread might not perceive the
>> value change done in the interrupt handler.
>>
>> Avoid this issue by using a bitfield instead of a boolean variable and by
>> accessing this field with the bit manipulating functions that provide cache
>> coherency.
>>
>> Signed-off-by: Lino Sanfilippo <[email protected]>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 13 +++++++------
>> drivers/char/tpm/tpm_tis_core.h | 6 +++++-
>> 2 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index dc56b976d816..6f2cf75add8b 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -470,7 +470,8 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
>> int rc, irq;
>> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>>
>> - if (!(chip->flags & TPM_CHIP_FLAG_IRQ) || priv->irq_tested)
>> + if (!(chip->flags & TPM_CHIP_FLAG_IRQ) ||
>> + test_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags))
>> return tpm_tis_send_main(chip, buf, len);
>>
>> /* Verify receipt of the expected IRQ */
>> @@ -480,11 +481,11 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
>> rc = tpm_tis_send_main(chip, buf, len);
>> priv->irq = irq;
>> chip->flags |= TPM_CHIP_FLAG_IRQ;
>> - if (!priv->irq_tested)
>> + if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags))
>> tpm_msleep(1);
>> - if (!priv->irq_tested)
>> + if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags))
>> disable_interrupts(chip);
>> - priv->irq_tested = true;
>> + set_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags);
>> return rc;
>> }
>>
>> @@ -693,7 +694,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
>> if (interrupt == 0)
>> return IRQ_NONE;
>>
>> - priv->irq_tested = true;
>> + set_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags);
>> if (interrupt & TPM_INTF_DATA_AVAIL_INT)
>> wake_up_interruptible(&priv->read_queue);
>> if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT)
>> @@ -779,7 +780,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
>> if (rc < 0)
>> return rc;
>>
>> - priv->irq_tested = false;
>> + clear_bit(TPM_TIS_IRQ_TESTED, &priv->irqtest_flags);
>>
>> /* Generate an interrupt by having the core call through to
>> * tpm_tis_send
>> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
>> index 3be24f221e32..0f29d0b68c3e 100644
>> --- a/drivers/char/tpm/tpm_tis_core.h
>> +++ b/drivers/char/tpm/tpm_tis_core.h
>> @@ -88,11 +88,15 @@ enum tpm_tis_flags {
>> TPM_TIS_INVALID_STATUS = BIT(1),
>> };
>>
>> +enum tpm_tis_irqtest_flags {
>> + TPM_TIS_IRQ_TESTED = BIT(0),
>> +};
>> +
>> struct tpm_tis_data {
>> u16 manufacturer_id;
>> int locality;
>> int irq;
>> - bool irq_tested;
>> + unsigned long irqtest_flags;
>> unsigned long flags;
>> void __iomem *ilb_base_addr;
>> u16 clkrun_enabled;
>> --
>> 2.36.1
>>
>
> Otherwise looks fine, but please add TPM_TIS_IRQ_TESTED to 'flags', and
> convert existing sites to use set_bit() and and test_bit().
>
> BR, Jarkko
>
Ok, will do.
Regards,
Lino
On 15.06.22 at 20:18, Jarkko Sakkinen wrote:
> On Fri, Jun 10, 2022 at 01:08:41PM +0200, [email protected] wrote:
>> From: Lino Sanfilippo <[email protected]>
>>
>> According to the TPM Interface Specification (TIS) support for "stsValid"
>> and "commandReady" interrupts is only optional.
>> This has to be taken into account when handling the interrupts in functions
>> like wait_for_tpm_stat(). So query the set of supported interrupts and
>> store it in a global variable so that it can be accessed later.
>>
>> Signed-off-by: Lino Sanfilippo <[email protected]>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 73 +++++++++++++++++----------------
>> drivers/char/tpm/tpm_tis_core.h | 1 +
>> 2 files changed, 38 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index 718525fcadc0..2f03fefa1706 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -1007,8 +1007,39 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
>> if (rc < 0)
>> goto out_err;
>>
>> - intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
>> - TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
>> + /* Figure out the capabilities */
>> + rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
>> + if (rc < 0)
>> + goto out_err;
>> +
>> + dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
>> + intfcaps);
>> + if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
>> + dev_dbg(dev, "\tBurst Count Static\n");
>> + if (intfcaps & TPM_INTF_CMD_READY_INT) {
>> + intmask |= TPM_INTF_CMD_READY_INT;
>> + dev_dbg(dev, "\tCommand Ready Int Support\n");
>> + }
>> + if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
>> + dev_dbg(dev, "\tInterrupt Edge Falling\n");
>> + if (intfcaps & TPM_INTF_INT_EDGE_RISING)
>> + dev_dbg(dev, "\tInterrupt Edge Rising\n");
>> + if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
>> + dev_dbg(dev, "\tInterrupt Level Low\n");
>> + if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
>> + dev_dbg(dev, "\tInterrupt Level High\n");
>> + if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT)
>> + intmask |= TPM_INTF_LOCALITY_CHANGE_INT;
>> + dev_dbg(dev, "\tLocality Change Int Support\n");
>> + if (intfcaps & TPM_INTF_STS_VALID_INT) {
>> + intmask |= TPM_INTF_STS_VALID_INT;
>> + dev_dbg(dev, "\tSts Valid Int Support\n");
>> + }
>> + if (intfcaps & TPM_INTF_DATA_AVAIL_INT) {
>> + intmask |= TPM_INTF_DATA_AVAIL_INT;
>> + dev_dbg(dev, "\tData Avail Int Support\n");
>> + }
>> +
>> intmask &= ~TPM_GLOBAL_INT_ENABLE;
>>
>> rc = request_locality(chip, 0);
>> @@ -1042,32 +1073,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
>> goto out_err;
>> }
>>
>> - /* Figure out the capabilities */
>> - rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
>> - if (rc < 0)
>> - goto out_err;
>> -
>> - dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
>> - intfcaps);
>> - if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
>> - dev_dbg(dev, "\tBurst Count Static\n");
>> - if (intfcaps & TPM_INTF_CMD_READY_INT)
>> - dev_dbg(dev, "\tCommand Ready Int Support\n");
>> - if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
>> - dev_dbg(dev, "\tInterrupt Edge Falling\n");
>> - if (intfcaps & TPM_INTF_INT_EDGE_RISING)
>> - dev_dbg(dev, "\tInterrupt Edge Rising\n");
>> - if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
>> - dev_dbg(dev, "\tInterrupt Level Low\n");
>> - if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
>> - dev_dbg(dev, "\tInterrupt Level High\n");
>> - if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT)
>> - dev_dbg(dev, "\tLocality Change Int Support\n");
>> - if (intfcaps & TPM_INTF_STS_VALID_INT)
>> - dev_dbg(dev, "\tSts Valid Int Support\n");
>> - if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
>> - dev_dbg(dev, "\tData Avail Int Support\n");
>> -
>> /* INTERRUPT Setup */
>> init_waitqueue_head(&priv->read_queue);
>> init_waitqueue_head(&priv->int_queue);
>> @@ -1098,7 +1103,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
>> else
>> tpm_tis_probe_irq(chip, intmask);
>>
>> - if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
>> + if (chip->flags & TPM_CHIP_FLAG_IRQ) {
>> + priv->irqs_in_use = intmask;
>> + } else {
>> dev_err(&chip->dev, FW_BUG
>> "TPM interrupt not working, polling instead\n");
>>
>> @@ -1145,13 +1152,7 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
>> if (rc < 0)
>> goto out;
>>
>> - rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
>> - if (rc < 0)
>> - goto out;
>> -
>> - intmask |= TPM_INTF_CMD_READY_INT
>> - | TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_DATA_AVAIL_INT
>> - | TPM_INTF_STS_VALID_INT | TPM_GLOBAL_INT_ENABLE;
>> + intmask = priv->irqs_in_use | TPM_GLOBAL_INT_ENABLE;
>>
>> tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
>> index 0f29d0b68c3e..8e02faa4079d 100644
>> --- a/drivers/char/tpm/tpm_tis_core.h
>> +++ b/drivers/char/tpm/tpm_tis_core.h
>> @@ -96,6 +96,7 @@ struct tpm_tis_data {
>> u16 manufacturer_id;
>> int locality;
>> int irq;
>> + unsigned int irqs_in_use;
>> unsigned long irqtest_flags;
>> unsigned long flags;
>> void __iomem *ilb_base_addr;
>> --
>> 2.36.1
>>
>
> int_mask would be imho a better name.
>
> Can you squash this and the following patch? It's good to slice in
> small patches but here I think it would make sense to tie the field
> to use case, in order for it to make sense.
>
Sure, will do so.
Regards,
Lino
Hi Michael,
On 15.06.22 at 23:54, Michael Niewöhner wrote:
>
> Hi guys,
>
> for me this series causes boot problems - somehow feels like an interrupt
> storm...
Thanks for this info. Which hardware do you use?
> Not sure yet, which commit is causing that
> @Tim could you test on any of your devices, please?
>
> BR
> Michael
>
Regards,
Lino
Hi Lino,
On Thu, 2022-06-16 at 01:30 +0200, Lino Sanfilippo wrote:
>
> Hi Michael,
>
> On 15.06.22 at 23:54, Michael Niewöhner wrote:
>
> >
> > Hi guys,
> >
> > for me this series causes boot problems - somehow feels like an interrupt
> > storm...
>
>
> Thanks for this info. Which hardware do you use?
>
> > Not sure yet, which commit is causing that
> > @Tim could you test on any of your devices, please?
> >
> > BR
> > Michael
> >
>
> Regards,
> Lino
looks like something was wrong with the devices firmware... I flashed a fresh
image and everything is totally fine now - TPM gets detected without the
"interrupts not working" error! :-)
Test device: Clevo L140MU, FW v1.07.12, TPM 2.0 Infineon SLB9670 (SPI)
BR
Michael
Hi Michael,
On 16.06.22 at 15:03, Michael Niewöhner wrote:
> Hi Lino,
>
> On Thu, 2022-06-16 at 01:30 +0200, Lino Sanfilippo wrote:
>>
>> Hi Michael,
>>
>> On 15.06.22 at 23:54, Michael Niewöhner wrote:
>>
>>>
>>> Hi guys,
>>>
>>> for me this series causes boot problems - somehow feels like an interrupt
>>> storm...
>>
>>
>> Thanks for this info. Which hardware do you use?
>>
>>> Not sure yet, which commit is causing that
>>> @Tim could you test on any of your devices, please?
>>>
>>> BR
>>> Michael
>>>
>>
>> Regards,
>> Lino
>
> looks like something was wrong with the devices firmware... I flashed a fresh
> image and everything is totally fine now - TPM gets detected without the
> "interrupts not working" error! :-)
>
> Test device: Clevo L140MU, FW v1.07.12, TPM 2.0 Infineon SLB9670 (SPI)
>
Good to hear that everything is fine now, thanks a lot for testing!
Best regards,
Lino
On Wed, 2022-06-15 at 21:26 +0300, Jarkko Sakkinen wrote:
> On Fri, Jun 10, 2022 at 01:08:46PM +0200, [email protected] wrote:
> > From: Lino Sanfilippo <[email protected]>
> >
> > The test for interrupts in tpm_tis_send() is skipped if the flag
> > TPM_CHIP_FLAG_IRQ is not set. Since the current code never sets the flag
> > initially the test is never executed.
> >
> > Fix this by setting the flag in tpm_tis_gen_interrupt() right after
> > interrupts have been enabled.
> >
> > Signed-off-by: Lino Sanfilippo <[email protected]>
> > ---
> > drivers/char/tpm/tpm_tis_core.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/char/tpm/tpm_tis_core.c
> > b/drivers/char/tpm/tpm_tis_core.c
> > index e5edf745fb23..be229c173f10 100644
> > --- a/drivers/char/tpm/tpm_tis_core.c
> > +++ b/drivers/char/tpm/tpm_tis_core.c
> > @@ -774,11 +774,16 @@ static int tpm_tis_gen_interrupt(struct tpm_chip
> > *chip)
> > if (ret < 0)
> > return ret;
> >
> > + chip->flags |= TPM_CHIP_FLAG_IRQ;
> > +
> > if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
> > else
> > ret = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
> > desc, 0);
> >
> > + if (ret)
> > + chip->flags &= ~TPM_CHIP_FLAG_IRQ;
> > +
> > release_locality(chip, 0);
> >
> > return ret;
> > --
> > 2.36.1
> >
>
> James, do you have a chance to test this on your side? Thanks.
>
> BR, Jarkko
Tested-by: Michael Niewöhner <[email protected]>
On Thu, Jun 16, 2022 at 03:04:30PM +0200, Michael Niew?hner wrote:
> On Wed, 2022-06-15 at 21:26 +0300, Jarkko Sakkinen wrote:
> > On Fri, Jun 10, 2022 at 01:08:46PM +0200, [email protected]?wrote:
> > > From: Lino Sanfilippo <[email protected]>
> > >
> > > The test for interrupts in tpm_tis_send() is skipped if the flag
> > > TPM_CHIP_FLAG_IRQ is not set. Since the current code never sets the flag
> > > initially the test is never executed.
> > >
> > > Fix this by setting the flag in tpm_tis_gen_interrupt() right after
> > > interrupts have been enabled.
> > >
> > > Signed-off-by: Lino Sanfilippo <[email protected]>
> > > ---
> > > ?drivers/char/tpm/tpm_tis_core.c | 5 +++++
> > > ?1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/char/tpm/tpm_tis_core.c
> > > b/drivers/char/tpm/tpm_tis_core.c
> > > index e5edf745fb23..be229c173f10 100644
> > > --- a/drivers/char/tpm/tpm_tis_core.c
> > > +++ b/drivers/char/tpm/tpm_tis_core.c
> > > @@ -774,11 +774,16 @@ static int tpm_tis_gen_interrupt(struct tpm_chip
> > > *chip)
> > > ????????if (ret < 0)
> > > ????????????????return ret;
> > > ?
> > > +???????chip->flags |= TPM_CHIP_FLAG_IRQ;
> > > +
> > > ????????if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > > ????????????????ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
> > > ????????else
> > > ????????????????ret = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
> > > desc, 0);
> > > ?
> > > +???????if (ret)
> > > +???????????????chip->flags &= ~TPM_CHIP_FLAG_IRQ;
> > > +
> > > ????????release_locality(chip, 0);
> > > ?
> > > ????????return ret;
> > > --
> > > 2.36.1
> > >
> >
> > James, do you have a chance to test this on your side? Thanks.
> >
> > BR, Jarkko
>
> Tested-by: Michael Niew?hner <[email protected]>
Thank you.
BR, Jarkko