If the system starts up with the TPM chip still in reset the probe routine
of the tpm-tis driver fails with the following error message:
"tpm_tis_spi: probe of spiX.Y failed with error -110"
The reason for this error is a missing response to the first command sent
to the chip (TPM_DID_VID) and a following timeout.
With the SLB9670 this issue can be triggered by setting up a pin
configuration in device tree with the reset line setting to
BCM2835_PUD_DOWN (note that the reset signal for the SLB9670 is active
low).
This patchset adds support to set the chip out of reset from within the TPM
driver.
For this reason two new callbacks are introduced that can optionally be
implemented by a tpm tis driver:
int (*unset_reset) (struct tpm_tis_data *data);
int (*set_reset) (struct tpm_tis_data *data);
Function "unset_reset" is called directly before the first TPM command is
issued. Function "set_reset" is called at system shutdown directly after
the TPM2 shutdown command.
Both callbacks are added to the set of tpm_tis_phy_ops functions. Patch 5
of this series provides the implementations for the SLB9670.
Patch 1:
Extend struct tpm_tis_phy_ops by the optional callbacks "set_reset" and
"unset_reset". If defined call "set_reset" before the first TPM command
is sent and "unset_reset" at system shutdown after the TPM2 shutdown
command.
Patch 2:
Document the property "reset-gpios" as an optional property which can be
used to specify the TPM chips reset gpio.
Patch 3:
If available get the reset gpio and store it in the tpm_tis_data
structure.
Patch 4:
Declare functions tpm_tis_spi_flow_control, tpm_tis_spi_read_bytes and
tpm_tis_spi_write_bytes as extern. This is in preparation of the next
patch in which a custom probe function for the SLB9670 is implemented
that is used to define its own set of tpm_tis_phy_ops.
Patch 5:
Implement the "set_reset" and "unset_reset" callbacks for the SLB9670 and
assign it in the probe function. The SLB9670 specific parts are moved
into an own file to separate it from the generic code (for now I opted
not to use a kernel config option for the SLB9670 code as it is used in
case of the SPI CR50).
This series has been tested with a SLB9670.
Lino Sanfilippo (5):
tpm: add functions to set and unset the tpm chips reset state
dt-bindings: tpm: document reset gpio property
tpm: tpm_tis: get optionally defined reset gpio
tpm: tpm_tis: make functions available for external linkage
tpm: tpm_tis_spi_slb_9670: implement set_reset and unset_reset
functions
.../bindings/security/tpm/tpm_tis_spi.txt | 2 +
drivers/char/tpm/Makefile | 1 +
drivers/char/tpm/tpm-chip.c | 5 ++
drivers/char/tpm/tpm_tis_core.c | 23 ++++++
drivers/char/tpm/tpm_tis_core.h | 3 +
drivers/char/tpm/tpm_tis_spi.h | 9 ++
drivers/char/tpm/tpm_tis_spi_main.c | 16 ++--
drivers/char/tpm/tpm_tis_spi_slb9670.c | 82 +++++++++++++++++++
8 files changed, 133 insertions(+), 8 deletions(-)
create mode 100644 drivers/char/tpm/tpm_tis_spi_slb9670.c
base-commit: ed4643521e6af8ab8ed1e467630a85884d2696cf
--
2.35.1
Provide implementations for the tpm phy operations set_reset and
unset_reset. Taking the chip out of reset requires a certain sequence of
line assertions and deassertions with respective minimum wait intervals:
deassert RST
wait at least 60 ms
assert RST
wait at least 2 usecs
deassert RST
wait at least 60 ms
assert RST
wait at least 2 usecs
deassert RST
wait at least 60 ms before issuing the first TPM command
According to the Infineon SLB 9670VQ2.0 datasheet this sequence is needed
to avoid triggering the chips defense modes in which it protects itself
from dictionary attacks in conjunction with resets.
Since the generic probe function tpm_tis_spi_probe only sets the
non-optional phy ops provide a custom version in which also set_reset and
unset_reset are assigned. Move the implementation of these functions into a
new file to separate the SLB9670 specific code from the generic code.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
drivers/char/tpm/Makefile | 1 +
drivers/char/tpm/tpm_tis_spi.h | 2 +
drivers/char/tpm/tpm_tis_spi_main.c | 4 +-
drivers/char/tpm/tpm_tis_spi_slb9670.c | 82 ++++++++++++++++++++++++++
4 files changed, 87 insertions(+), 2 deletions(-)
create mode 100644 drivers/char/tpm/tpm_tis_spi_slb9670.c
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 66d39ea6bd10..22c82eb4e382 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_TCG_TIS_SYNQUACER) += tpm_tis_synquacer.o
obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
tpm_tis_spi-y := tpm_tis_spi_main.o
+tpm_tis_spi-y += tpm_tis_spi_slb9670.o
tpm_tis_spi-$(CONFIG_TCG_TIS_SPI_CR50) += tpm_tis_spi_cr50.o
obj-$(CONFIG_TCG_TIS_I2C_CR50) += tpm_tis_i2c_cr50.o
diff --git a/drivers/char/tpm/tpm_tis_spi.h b/drivers/char/tpm/tpm_tis_spi.h
index 8f4331d8a4dd..b90848832da4 100644
--- a/drivers/char/tpm/tpm_tis_spi.h
+++ b/drivers/char/tpm/tpm_tis_spi.h
@@ -51,6 +51,8 @@ static inline int cr50_spi_probe(struct spi_device *spi)
}
#endif
+extern int slb9670_spi_probe(struct spi_device *spi);
+
#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_TCG_TIS_SPI_CR50)
extern int tpm_tis_spi_resume(struct device *dev);
#else
diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
index b2d13b844659..50da1f6eeaea 100644
--- a/drivers/char/tpm/tpm_tis_spi_main.c
+++ b/drivers/char/tpm/tpm_tis_spi_main.c
@@ -264,7 +264,7 @@ static void tpm_tis_spi_remove(struct spi_device *dev)
static const struct spi_device_id tpm_tis_spi_id[] = {
{ "st33htpm-spi", (unsigned long)tpm_tis_spi_probe },
- { "slb9670", (unsigned long)tpm_tis_spi_probe },
+ { "slb9670", (unsigned long)slb9670_spi_probe },
{ "tpm_tis_spi", (unsigned long)tpm_tis_spi_probe },
{ "tpm_tis-spi", (unsigned long)tpm_tis_spi_probe },
{ "cr50", (unsigned long)cr50_spi_probe },
@@ -274,7 +274,7 @@ MODULE_DEVICE_TABLE(spi, tpm_tis_spi_id);
static const struct of_device_id of_tis_spi_match[] = {
{ .compatible = "st,st33htpm-spi", .data = tpm_tis_spi_probe },
- { .compatible = "infineon,slb9670", .data = tpm_tis_spi_probe },
+ { .compatible = "infineon,slb9670", .data = slb9670_spi_probe },
{ .compatible = "tcg,tpm_tis-spi", .data = tpm_tis_spi_probe },
{ .compatible = "google,cr50", .data = cr50_spi_probe },
{}
diff --git a/drivers/char/tpm/tpm_tis_spi_slb9670.c b/drivers/char/tpm/tpm_tis_spi_slb9670.c
new file mode 100644
index 000000000000..ba9cd54e8bff
--- /dev/null
+++ b/drivers/char/tpm/tpm_tis_spi_slb9670.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * tpm_tis_spi_slb9670.c
+ *
+ * Copyright (C) 2022 KUNBUS GmbH
+ *
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/delay.h>
+
+#include "tpm_tis_core.h"
+#include "tpm_tis_spi.h"
+
+/*
+ * Time intervals used in the reset sequence.
+ * RSTIN: minimum time to hold the reset line deasserted.
+ * WRST: minimum time to hold the reset line asserted.
+ */
+#define SLB9670_TIME_RSTIN 60 /* time in ms */
+#define SLB9670_TIME_WRST 2 /* time in usecs */
+
+int slb9670_spi_unset_reset(struct tpm_tis_data *data)
+{
+ /*
+ * Perform the reset sequence: we have to deassert and assert the reset
+ * line two times and wait the respective time intervals.
+ * After a last wait interval of RSTIN the chip is ready to receive the
+ * first command.
+ */
+ gpiod_set_value(data->reset_gpio, 0);
+ msleep(SLB9670_TIME_RSTIN);
+ gpiod_set_value(data->reset_gpio, 1);
+ udelay(SLB9670_TIME_WRST);
+ gpiod_set_value(data->reset_gpio, 0);
+ msleep(SLB9670_TIME_RSTIN);
+ gpiod_set_value(data->reset_gpio, 1);
+ udelay(SLB9670_TIME_WRST);
+ gpiod_set_value(data->reset_gpio, 0);
+ msleep(SLB9670_TIME_RSTIN);
+
+ return 0;
+}
+
+int slb9670_spi_set_reset(struct tpm_tis_data *data)
+{
+ gpiod_set_value(data->reset_gpio, 1);
+ return 0;
+}
+
+static const struct tpm_tis_phy_ops slb9670_spi_phy_ops = {
+ .read_bytes = tpm_tis_spi_read_bytes,
+ .write_bytes = tpm_tis_spi_write_bytes,
+ .read16 = tpm_tis_spi_read16,
+ .read32 = tpm_tis_spi_read32,
+ .write32 = tpm_tis_spi_write32,
+ .set_reset = slb9670_spi_set_reset,
+ .unset_reset = slb9670_spi_unset_reset,
+};
+
+int slb9670_spi_probe(struct spi_device *spi)
+{
+ struct tpm_tis_spi_phy *phy;
+ int irq;
+
+ phy = devm_kzalloc(&spi->dev, sizeof(struct tpm_tis_spi_phy),
+ GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ phy->flow_control = tpm_tis_spi_flow_control;
+
+ /* If the SPI device has an IRQ then use that */
+ if (spi->irq > 0)
+ irq = spi->irq;
+ else
+ irq = -1;
+
+ init_completion(&phy->ready);
+ return tpm_tis_spi_init(spi, phy, irq, &slb9670_spi_phy_ops);
+}
--
2.35.1
Add a despription for the property "reset-gpios" which can be used to
specify the gpio connected to the tpm chips reset pin. Also add an example
for how to use it within the tpm node.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
Documentation/devicetree/bindings/security/tpm/tpm_tis_spi.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/security/tpm/tpm_tis_spi.txt b/Documentation/devicetree/bindings/security/tpm/tpm_tis_spi.txt
index b800667da92b..0090a1948ca5 100644
--- a/Documentation/devicetree/bindings/security/tpm/tpm_tis_spi.txt
+++ b/Documentation/devicetree/bindings/security/tpm/tpm_tis_spi.txt
@@ -8,6 +8,7 @@ Required properties:
Optional SoC Specific Properties:
- pinctrl-names: Contains only one value - "default".
- pintctrl-0: Specifies the pin control groups used for this controller.
+- reset-gpios: Specifies the reset GPIO.
Example (for ARM-based BeagleBoard xM with TPM_TIS on SPI4):
@@ -19,5 +20,6 @@ Example (for ARM-based BeagleBoard xM with TPM_TIS on SPI4):
compatible = "tcg,tpm_tis-spi";
spi-max-frequency = <10000000>;
+ reset-gpios = <&gpio 3 GPIO_ACTIVE_LOW>;
};
};
--
2.35.1
Get the optionally specified reset gpio. This property can be set with the
con-id "reset-gpios".
Signed-off-by: Lino Sanfilippo <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 20 ++++++++++++++++++++
drivers/char/tpm/tpm_tis_core.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 11e5e045f3a7..89bfee3cfb18 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -24,6 +24,7 @@
#include <linux/wait.h>
#include <linux/acpi.h>
#include <linux/freezer.h>
+#include <linux/gpio/consumer.h>
#include "tpm.h"
#include "tpm_tis_core.h"
@@ -919,6 +920,21 @@ static const struct tpm_class_ops tpm_tis = {
.clk_enable = tpm_tis_clkrun_enable,
};
+/*
+ * Retrieve the reset GPIO if it is defined.
+ */
+static int tpm_tis_get_reset_gpio(struct device *dev, struct tpm_tis_data *data)
+{
+ data->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(data->reset_gpio))
+ return PTR_ERR(data->reset_gpio);
+
+ if (data->reset_gpio)
+ gpiod_set_consumer_name(data->reset_gpio, "TPM reset");
+
+ return 0;
+}
+
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)
@@ -952,6 +968,10 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
dev_set_drvdata(&chip->dev, priv);
+ rc = tpm_tis_get_reset_gpio(dev, priv);
+ if (rc)
+ return rc;
+
if (priv->phy_ops->unset_reset)
priv->phy_ops->unset_reset(priv);
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index f1a67445a5c5..502816d91353 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -94,6 +94,7 @@ struct tpm_tis_data {
int irq;
bool irq_tested;
unsigned long flags;
+ struct gpio_desc *reset_gpio;
void __iomem *ilb_base_addr;
u16 clkrun_enabled;
wait_queue_head_t int_queue;
--
2.35.1
On Thu, Apr 07, 2022 at 01:18:49PM +0200, Lino Sanfilippo wrote:
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_tis_spi_slb9670.c
[...]
> +int slb9670_spi_unset_reset(struct tpm_tis_data *data)
[...]
> +int slb9670_spi_set_reset(struct tpm_tis_data *data)
[...]
> +static const struct tpm_tis_phy_ops slb9670_spi_phy_ops = {
> + .read_bytes = tpm_tis_spi_read_bytes,
> + .write_bytes = tpm_tis_spi_write_bytes,
> + .read16 = tpm_tis_spi_read16,
> + .read32 = tpm_tis_spi_read32,
> + .write32 = tpm_tis_spi_write32,
> + .set_reset = slb9670_spi_set_reset,
> + .unset_reset = slb9670_spi_unset_reset,
> +};
0-day is complaining that slb9670_spi_set_reset() / slb9670_spi_unset_reset()
are not declared static:
https://lore.kernel.org/all/[email protected]/
Thanks,
Lukas
On 10.04.22 at 19:18, Lukas Wunner wrote:
> On Thu, Apr 07, 2022 at 01:18:49PM +0200, Lino Sanfilippo wrote:
>> --- /dev/null
>> +++ b/drivers/char/tpm/tpm_tis_spi_slb9670.c
> [...]
>> +int slb9670_spi_unset_reset(struct tpm_tis_data *data)
> [...]
>> +int slb9670_spi_set_reset(struct tpm_tis_data *data)
> [...]
>> +static const struct tpm_tis_phy_ops slb9670_spi_phy_ops = {
>> + .read_bytes = tpm_tis_spi_read_bytes,
>> + .write_bytes = tpm_tis_spi_write_bytes,
>> + .read16 = tpm_tis_spi_read16,
>> + .read32 = tpm_tis_spi_read32,
>> + .write32 = tpm_tis_spi_write32,
>> + .set_reset = slb9670_spi_set_reset,
>> + .unset_reset = slb9670_spi_unset_reset,
>> +};
>
> 0-day is complaining that slb9670_spi_set_reset() / slb9670_spi_unset_reset()
> are not declared static:
>
> https://lore.kernel.org/all/[email protected]/
>
> Thanks,
>
> Lukas
>
Right, I will fix this in the next version, thanks!
Regards,
Lino