linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Support TPM Reset GPIO
@ 2022-04-07 11:18 Lino Sanfilippo
  2022-04-07 11:18 ` [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state Lino Sanfilippo
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Lino Sanfilippo @ 2022-04-07 11:18 UTC (permalink / raw)
  To: peterhuewe, jarkko, jgg, =robh+dt
  Cc: devicetree, linux-integrity, linux-kernel, stefanb,
	p.rosenberger, lukas, Lino Sanfilippo

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state
  2022-04-07 11:18 [PATCH 0/5] Support TPM Reset GPIO Lino Sanfilippo
@ 2022-04-07 11:18 ` Lino Sanfilippo
  2022-04-07 14:25   ` Jason Gunthorpe
  2022-04-07 11:18 ` [PATCH 2/5] dt-bindings: tpm: document reset gpio property Lino Sanfilippo
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Lino Sanfilippo @ 2022-04-07 11:18 UTC (permalink / raw)
  To: peterhuewe, jarkko, jgg, =robh+dt
  Cc: devicetree, linux-integrity, linux-kernel, stefanb,
	p.rosenberger, lukas, Lino Sanfilippo

Currently it is not possible to set the tpm chips reset state from within
the driver. This is problematic if the chip is still in reset after the
system comes up. This may e.g. happen if the reset line is pulled into
reset state by a pin configuration in the device tree.

To handle this case extend tpm_tis_phy_ops by the two functions "set_reset"
and "unset_reset" which may optionally be defined by a tpm driver.
If defined call "unset_reset" at chip startup before the first tpm command
is issued. Also if defined call "set_reset" at chip shutdown after the tpm2
shutdown command has been sent.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/char/tpm/tpm-chip.c     | 5 +++++
 drivers/char/tpm/tpm_tis_core.c | 3 +++
 drivers/char/tpm/tpm_tis_core.h | 2 ++
 3 files changed, 10 insertions(+)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 783d65fc71f0..c1b79ba9159d 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -24,6 +24,7 @@
 #include <linux/tpm_eventlog.h>
 #include <linux/hw_random.h>
 #include "tpm.h"
+#include "tpm_tis_core.h"
 
 DEFINE_IDR(dev_nums_idr);
 static DEFINE_MUTEX(idr_lock);
@@ -286,6 +287,7 @@ static void tpm_dev_release(struct device *dev)
 static int tpm_class_shutdown(struct device *dev)
 {
 	struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
+	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 
 	down_write(&chip->ops_sem);
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
@@ -297,6 +299,9 @@ static int tpm_class_shutdown(struct device *dev)
 	chip->ops = NULL;
 	up_write(&chip->ops_sem);
 
+	if (priv->phy_ops->set_reset)
+		priv->phy_ops->set_reset(priv);
+
 	return 0;
 }
 
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index dc56b976d816..11e5e045f3a7 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -952,6 +952,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 
 	dev_set_drvdata(&chip->dev, priv);
 
+	if (priv->phy_ops->unset_reset)
+		priv->phy_ops->unset_reset(priv);
+
 	rc = tpm_tis_read32(priv, TPM_DID_VID(0), &vendor);
 	if (rc < 0)
 		return rc;
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 3be24f221e32..f1a67445a5c5 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -105,6 +105,8 @@ struct tpm_tis_data {
 };
 
 struct tpm_tis_phy_ops {
+	int (*set_reset)(struct tpm_tis_data *data);
+	int (*unset_reset)(struct tpm_tis_data *data);
 	int (*read_bytes)(struct tpm_tis_data *data, u32 addr, u16 len,
 			  u8 *result);
 	int (*write_bytes)(struct tpm_tis_data *data, u32 addr, u16 len,
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/5] dt-bindings: tpm: document reset gpio property
  2022-04-07 11:18 [PATCH 0/5] Support TPM Reset GPIO Lino Sanfilippo
  2022-04-07 11:18 ` [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state Lino Sanfilippo
@ 2022-04-07 11:18 ` Lino Sanfilippo
  2022-04-07 11:18 ` [PATCH 3/5] tpm: tpm_tis: get optionally defined reset gpio Lino Sanfilippo
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Lino Sanfilippo @ 2022-04-07 11:18 UTC (permalink / raw)
  To: peterhuewe, jarkko, jgg, =robh+dt
  Cc: devicetree, linux-integrity, linux-kernel, stefanb,
	p.rosenberger, lukas, Lino Sanfilippo

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 <LinoSanfilippo@gmx.de>
---
 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/5] tpm: tpm_tis: get optionally defined reset gpio
  2022-04-07 11:18 [PATCH 0/5] Support TPM Reset GPIO Lino Sanfilippo
  2022-04-07 11:18 ` [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state Lino Sanfilippo
  2022-04-07 11:18 ` [PATCH 2/5] dt-bindings: tpm: document reset gpio property Lino Sanfilippo
@ 2022-04-07 11:18 ` Lino Sanfilippo
  2022-04-07 11:18 ` [PATCH 4/5] tpm: tpm_tis: make functions available for external linkage Lino Sanfilippo
  2022-04-07 11:18 ` [PATCH 5/5] tpm: tpm_tis_spi_slb_9670: implement set_reset and unset_reset functions Lino Sanfilippo
  4 siblings, 0 replies; 13+ messages in thread
From: Lino Sanfilippo @ 2022-04-07 11:18 UTC (permalink / raw)
  To: peterhuewe, jarkko, jgg, =robh+dt
  Cc: devicetree, linux-integrity, linux-kernel, stefanb,
	p.rosenberger, lukas, Lino Sanfilippo

Get the optionally specified reset gpio. This property can be set with the
con-id "reset-gpios".

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/5] tpm: tpm_tis: make functions available for external linkage
  2022-04-07 11:18 [PATCH 0/5] Support TPM Reset GPIO Lino Sanfilippo
                   ` (2 preceding siblings ...)
  2022-04-07 11:18 ` [PATCH 3/5] tpm: tpm_tis: get optionally defined reset gpio Lino Sanfilippo
@ 2022-04-07 11:18 ` Lino Sanfilippo
  2022-04-07 11:18 ` [PATCH 5/5] tpm: tpm_tis_spi_slb_9670: implement set_reset and unset_reset functions Lino Sanfilippo
  4 siblings, 0 replies; 13+ messages in thread
From: Lino Sanfilippo @ 2022-04-07 11:18 UTC (permalink / raw)
  To: peterhuewe, jarkko, jgg, =robh+dt
  Cc: devicetree, linux-integrity, linux-kernel, stefanb,
	p.rosenberger, lukas, Lino Sanfilippo

The tpm_tis_phy_ops functions tpm_tis_spi_read16, tpm_tis_spi_read32 and
tpm_tis_spi_write32 are already declared extern to be used outside of
tpm_tim_spi_main.c.

Do the same with tpm_tis_spi_read_bytes and tpm_tis_spi_write_bytes to
allow the external access to the complete set of tpm_tis_phy_ops.

This is in preparation of the assignment of SLB9670 specific phy ops in a
separate file.

Furthermore declare tpm_tis_spi_flow_control extern, so that we can use it
in a SLB9670 specific probe function in the same file.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/char/tpm/tpm_tis_spi.h      |  7 +++++++
 drivers/char/tpm/tpm_tis_spi_main.c | 12 ++++++------
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_spi.h b/drivers/char/tpm/tpm_tis_spi.h
index bba73979c368..8f4331d8a4dd 100644
--- a/drivers/char/tpm/tpm_tis_spi.h
+++ b/drivers/char/tpm/tpm_tis_spi.h
@@ -25,6 +25,9 @@ static inline struct tpm_tis_spi_phy *to_tpm_tis_spi_phy(struct tpm_tis_data *da
 	return container_of(data, struct tpm_tis_spi_phy, priv);
 }
 
+extern int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy,
+				    struct spi_transfer *spi_xfer);
+
 extern int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy,
 			    int irq, const struct tpm_tis_phy_ops *phy_ops);
 
@@ -34,6 +37,10 @@ extern int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
 extern int tpm_tis_spi_read16(struct tpm_tis_data *data, u32 addr, u16 *result);
 extern int tpm_tis_spi_read32(struct tpm_tis_data *data, u32 addr, u32 *result);
 extern int tpm_tis_spi_write32(struct tpm_tis_data *data, u32 addr, u32 value);
+extern int tpm_tis_spi_read_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
+				  u8 *result);
+extern int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
+				   const u8 *value);
 
 #ifdef CONFIG_TCG_TIS_SPI_CR50
 extern int cr50_spi_probe(struct spi_device *spi);
diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
index 184396b3af50..b2d13b844659 100644
--- a/drivers/char/tpm/tpm_tis_spi_main.c
+++ b/drivers/char/tpm/tpm_tis_spi_main.c
@@ -45,8 +45,8 @@
  *
  * [1] https://trustedcomputinggroup.org/resource/pc-client-platform-tpm-profile-ptp-specification/
  */
-static int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy,
-				    struct spi_transfer *spi_xfer)
+int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy,
+			     struct spi_transfer *spi_xfer)
 {
 	struct spi_message m;
 	int ret, i;
@@ -140,14 +140,14 @@ int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
 	return ret;
 }
 
-static int tpm_tis_spi_read_bytes(struct tpm_tis_data *data, u32 addr,
-				  u16 len, u8 *result)
+int tpm_tis_spi_read_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
+			   u8 *result)
 {
 	return tpm_tis_spi_transfer(data, addr, len, result, NULL);
 }
 
-static int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr,
-				   u16 len, const u8 *value)
+int tpm_tis_spi_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
+			    const u8 *value)
 {
 	return tpm_tis_spi_transfer(data, addr, len, NULL, value);
 }
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/5] tpm: tpm_tis_spi_slb_9670: implement set_reset and unset_reset functions
  2022-04-07 11:18 [PATCH 0/5] Support TPM Reset GPIO Lino Sanfilippo
                   ` (3 preceding siblings ...)
  2022-04-07 11:18 ` [PATCH 4/5] tpm: tpm_tis: make functions available for external linkage Lino Sanfilippo
@ 2022-04-07 11:18 ` Lino Sanfilippo
  2022-04-10 17:18   ` Lukas Wunner
  4 siblings, 1 reply; 13+ messages in thread
From: Lino Sanfilippo @ 2022-04-07 11:18 UTC (permalink / raw)
  To: peterhuewe, jarkko, jgg, =robh+dt
  Cc: devicetree, linux-integrity, linux-kernel, stefanb,
	p.rosenberger, lukas, Lino Sanfilippo

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 <LinoSanfilippo@gmx.de>
---
 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state
  2022-04-07 11:18 ` [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state Lino Sanfilippo
@ 2022-04-07 14:25   ` Jason Gunthorpe
  2022-04-10  9:03     ` Lino Sanfilippo
  2022-04-10 17:11     ` Lukas Wunner
  0 siblings, 2 replies; 13+ messages in thread
From: Jason Gunthorpe @ 2022-04-07 14:25 UTC (permalink / raw)
  To: Lino Sanfilippo
  Cc: peterhuewe, jarkko, =robh+dt, devicetree, linux-integrity,
	linux-kernel, stefanb, p.rosenberger, lukas

On Thu, Apr 07, 2022 at 01:18:45PM +0200, Lino Sanfilippo wrote:
> Currently it is not possible to set the tpm chips reset state from within
> the driver. This is problematic if the chip is still in reset after the
> system comes up. This may e.g. happen if the reset line is pulled into
> reset state by a pin configuration in the device tree.

This kind of system is badly misdesigned.

TPM PCRs fundementally cannot work if the TPM reset line is under
software control.

Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state
  2022-04-07 14:25   ` Jason Gunthorpe
@ 2022-04-10  9:03     ` Lino Sanfilippo
  2022-04-10 17:11     ` Lukas Wunner
  1 sibling, 0 replies; 13+ messages in thread
From: Lino Sanfilippo @ 2022-04-10  9:03 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: peterhuewe, jarkko, robh+dt, devicetree, linux-integrity,
	linux-kernel, stefanb, p.rosenberger, lukas


Hi,

On 07.04.22 at 16:25, Jason Gunthorpe wrote:
> On Thu, Apr 07, 2022 at 01:18:45PM +0200, Lino Sanfilippo wrote:
>> Currently it is not possible to set the tpm chips reset state from within
>> the driver. This is problematic if the chip is still in reset after the
>> system comes up. This may e.g. happen if the reset line is pulled into
>> reset state by a pin configuration in the device tree.
>
> This kind of system is badly misdesigned.
>
> TPM PCRs fundementally cannot work if the TPM reset line is under
> software control.
>
> Jason
>


you may be right about the misdesign, but as a matter of fact
there are systems which have the TPMs reset line connected to a GPIO and not
the system reset. For those systems we should provide a way to let at least the
driver put the TPM out of reset (note that on those systems the TPM reset can
be triggered by software anyway by asserting/deasserting the GPIO line).


Regards,
Lino

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state
  2022-04-07 14:25   ` Jason Gunthorpe
  2022-04-10  9:03     ` Lino Sanfilippo
@ 2022-04-10 17:11     ` Lukas Wunner
  2022-04-11 11:47       ` Jason Gunthorpe
  1 sibling, 1 reply; 13+ messages in thread
From: Lukas Wunner @ 2022-04-10 17:11 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Lino Sanfilippo, peterhuewe, jarkko, =robh+dt, devicetree,
	linux-integrity, linux-kernel, stefanb, p.rosenberger

On Thu, Apr 07, 2022 at 11:25:26AM -0300, Jason Gunthorpe wrote:
> On Thu, Apr 07, 2022 at 01:18:45PM +0200, Lino Sanfilippo wrote:
> > Currently it is not possible to set the tpm chips reset state from within
> > the driver. This is problematic if the chip is still in reset after the
> > system comes up. This may e.g. happen if the reset line is pulled into
> > reset state by a pin configuration in the device tree.
> 
> This kind of system is badly misdesigned.
> 
> TPM PCRs fundementally cannot work if the TPM reset line is under
> software control.

Not every system which incorporates a TPM wants to use or is even capable
of measuring software state of any kind or perform secure boot.

Those systems may merely want to use the TPM to store key material.

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/5] tpm: tpm_tis_spi_slb_9670: implement set_reset and unset_reset functions
  2022-04-07 11:18 ` [PATCH 5/5] tpm: tpm_tis_spi_slb_9670: implement set_reset and unset_reset functions Lino Sanfilippo
@ 2022-04-10 17:18   ` Lukas Wunner
  2022-04-10 19:44     ` Lino Sanfilippo
  0 siblings, 1 reply; 13+ messages in thread
From: Lukas Wunner @ 2022-04-10 17:18 UTC (permalink / raw)
  To: Lino Sanfilippo
  Cc: peterhuewe, jarkko, jgg, =robh+dt, devicetree, linux-integrity,
	linux-kernel, stefanb, p.rosenberger

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/202204081357.8SfjQosI-lkp@intel.com/

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/5] tpm: tpm_tis_spi_slb_9670: implement set_reset and unset_reset functions
  2022-04-10 17:18   ` Lukas Wunner
@ 2022-04-10 19:44     ` Lino Sanfilippo
  0 siblings, 0 replies; 13+ messages in thread
From: Lino Sanfilippo @ 2022-04-10 19:44 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: peterhuewe, jarkko, jgg, =robh+dt, devicetree, linux-integrity,
	linux-kernel, stefanb, p.rosenberger

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/202204081357.8SfjQosI-lkp@intel.com/
>
> Thanks,
>
> Lukas
>

Right, I will fix this in the next version, thanks!

Regards,
Lino


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state
  2022-04-10 17:11     ` Lukas Wunner
@ 2022-04-11 11:47       ` Jason Gunthorpe
  2022-04-14 12:13         ` Jarkko Sakkinen
  0 siblings, 1 reply; 13+ messages in thread
From: Jason Gunthorpe @ 2022-04-11 11:47 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Lino Sanfilippo, peterhuewe, jarkko, =robh+dt, devicetree,
	linux-integrity, linux-kernel, stefanb, p.rosenberger

On Sun, Apr 10, 2022 at 07:11:23PM +0200, Lukas Wunner wrote:
> On Thu, Apr 07, 2022 at 11:25:26AM -0300, Jason Gunthorpe wrote:
> > On Thu, Apr 07, 2022 at 01:18:45PM +0200, Lino Sanfilippo wrote:
> > > Currently it is not possible to set the tpm chips reset state from within
> > > the driver. This is problematic if the chip is still in reset after the
> > > system comes up. This may e.g. happen if the reset line is pulled into
> > > reset state by a pin configuration in the device tree.
> > 
> > This kind of system is badly misdesigned.
> > 
> > TPM PCRs fundementally cannot work if the TPM reset line is under
> > software control.
> 
> Not every system which incorporates a TPM wants to use or is even capable
> of measuring software state of any kind or perform secure boot.
> 
> Those systems may merely want to use the TPM to store key material.

Then maybe the TPM driver should make it clear somehow that the PCRs
don't work in these systems.

It is really dangerous to add capabilities like this that should
never, ever be used in sanely designed systems.

Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state
  2022-04-11 11:47       ` Jason Gunthorpe
@ 2022-04-14 12:13         ` Jarkko Sakkinen
  0 siblings, 0 replies; 13+ messages in thread
From: Jarkko Sakkinen @ 2022-04-14 12:13 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Lukas Wunner, Lino Sanfilippo, peterhuewe, devicetree,
	linux-integrity, linux-kernel, stefanb, p.rosenberger

On Mon, Apr 11, 2022 at 08:47:41AM -0300, Jason Gunthorpe wrote:
> On Sun, Apr 10, 2022 at 07:11:23PM +0200, Lukas Wunner wrote:
> > On Thu, Apr 07, 2022 at 11:25:26AM -0300, Jason Gunthorpe wrote:
> > > On Thu, Apr 07, 2022 at 01:18:45PM +0200, Lino Sanfilippo wrote:
> > > > Currently it is not possible to set the tpm chips reset state from within
> > > > the driver. This is problematic if the chip is still in reset after the
> > > > system comes up. This may e.g. happen if the reset line is pulled into
> > > > reset state by a pin configuration in the device tree.
> > > 
> > > This kind of system is badly misdesigned.
> > > 
> > > TPM PCRs fundementally cannot work if the TPM reset line is under
> > > software control.
> > 
> > Not every system which incorporates a TPM wants to use or is even capable
> > of measuring software state of any kind or perform secure boot.
> > 
> > Those systems may merely want to use the TPM to store key material.
> 
> Then maybe the TPM driver should make it clear somehow that the PCRs
> don't work in these systems.
> 
> It is really dangerous to add capabilities like this that should
> never, ever be used in sanely designed systems.
> 
> Jason

I agree. That niche should do the bad things with oot patches.

BR, Jarkko

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-04-14 12:14 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-07 11:18 [PATCH 0/5] Support TPM Reset GPIO Lino Sanfilippo
2022-04-07 11:18 ` [PATCH 1/5] tpm: add functions to set and unset the tpm chips reset state Lino Sanfilippo
2022-04-07 14:25   ` Jason Gunthorpe
2022-04-10  9:03     ` Lino Sanfilippo
2022-04-10 17:11     ` Lukas Wunner
2022-04-11 11:47       ` Jason Gunthorpe
2022-04-14 12:13         ` Jarkko Sakkinen
2022-04-07 11:18 ` [PATCH 2/5] dt-bindings: tpm: document reset gpio property Lino Sanfilippo
2022-04-07 11:18 ` [PATCH 3/5] tpm: tpm_tis: get optionally defined reset gpio Lino Sanfilippo
2022-04-07 11:18 ` [PATCH 4/5] tpm: tpm_tis: make functions available for external linkage Lino Sanfilippo
2022-04-07 11:18 ` [PATCH 5/5] tpm: tpm_tis_spi_slb_9670: implement set_reset and unset_reset functions Lino Sanfilippo
2022-04-10 17:18   ` Lukas Wunner
2022-04-10 19:44     ` Lino Sanfilippo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).