Re: [PATCH v7 4/6] tpm: tpm_tis_spi: Support cr50 devices

2019-10-06 Thread Jarkko Sakkinen
On Fri, Sep 20, 2019 at 11:32:38AM -0700, Stephen Boyd wrote:
> From: Andrey Pronin 
> 
> Add TPM2.0 PTP FIFO compatible SPI interface for chips with Cr50
> firmware. The firmware running on the currently supported H1 Secure
> Microcontroller requires a special driver to handle its specifics:
> 
>  - need to ensure a certain delay between SPI transactions, or else
>the chip may miss some part of the next transaction
>  - if there is no SPI activity for some time, it may go to sleep,
>and needs to be waken up before sending further commands
>  - access to vendor-specific registers
> 
> Cr50 firmware has a requirement to wait for the TPM to wakeup before
> sending commands over the SPI bus. Otherwise, the firmware could be in
> deep sleep and not respond. The method to wait for the device to wakeup
> is slightly different than the usual flow control mechanism described in
> the TCG SPI spec. Add a completion to tpm_tis_spi_transfer() before we
> start a SPI transfer so we can keep track of the last time the TPM
> driver accessed the SPI bus to support the flow control mechanism.
> 
> Split the cr50 logic off into a different file to keep it out of the
> normal code flow of the existing SPI driver while making it all part of
> the same module when the code is optionally compiled into the same
> module. Export a new function, tpm_tis_spi_init(), and the associated
> read/write/transfer APIs so that we can do this. Make the cr50 code wrap
> the tpm_tis_spi_phy struct with its own struct to override the behavior
> of tpm_tis_spi_transfer() by supplying a custom flow control hook. This
> shares the most code between the core driver and the cr50 support
> without combining everything into the core driver or exporting module
> symbols.
> 
> Signed-off-by: Andrey Pronin 
> Cc: Andrey Pronin 
> Cc: Duncan Laurie 
> Cc: Jason Gunthorpe 
> Cc: Arnd Bergmann 
> Cc: Greg Kroah-Hartman 
> Cc: Guenter Roeck 
> Cc: Alexander Steffen 
> Cc: Heiko Stuebner 

First, I apologize for such a long latency (two weeks).

I think this looks legit now.

Reviewed-by: Jarkko Sakkinen 

/Jarkko


[PATCH v7 4/6] tpm: tpm_tis_spi: Support cr50 devices

2019-09-20 Thread Stephen Boyd
From: Andrey Pronin 

Add TPM2.0 PTP FIFO compatible SPI interface for chips with Cr50
firmware. The firmware running on the currently supported H1 Secure
Microcontroller requires a special driver to handle its specifics:

 - need to ensure a certain delay between SPI transactions, or else
   the chip may miss some part of the next transaction
 - if there is no SPI activity for some time, it may go to sleep,
   and needs to be waken up before sending further commands
 - access to vendor-specific registers

Cr50 firmware has a requirement to wait for the TPM to wakeup before
sending commands over the SPI bus. Otherwise, the firmware could be in
deep sleep and not respond. The method to wait for the device to wakeup
is slightly different than the usual flow control mechanism described in
the TCG SPI spec. Add a completion to tpm_tis_spi_transfer() before we
start a SPI transfer so we can keep track of the last time the TPM
driver accessed the SPI bus to support the flow control mechanism.

Split the cr50 logic off into a different file to keep it out of the
normal code flow of the existing SPI driver while making it all part of
the same module when the code is optionally compiled into the same
module. Export a new function, tpm_tis_spi_init(), and the associated
read/write/transfer APIs so that we can do this. Make the cr50 code wrap
the tpm_tis_spi_phy struct with its own struct to override the behavior
of tpm_tis_spi_transfer() by supplying a custom flow control hook. This
shares the most code between the core driver and the cr50 support
without combining everything into the core driver or exporting module
symbols.

Signed-off-by: Andrey Pronin 
Cc: Andrey Pronin 
Cc: Duncan Laurie 
Cc: Jason Gunthorpe 
Cc: Arnd Bergmann 
Cc: Greg Kroah-Hartman 
Cc: Guenter Roeck 
Cc: Alexander Steffen 
Cc: Heiko Stuebner 
[swb...@chromium.org: Replace boilerplate with SPDX tag, drop
suspended bit and remove ifdef checks in cr50.h, migrate to functions
exported in tpm_tis_spi.h, combine into one module instead of two]
Signed-off-by: Stephen Boyd 
---
 drivers/char/tpm/Kconfig|   7 +
 drivers/char/tpm/Makefile   |   4 +-
 drivers/char/tpm/tpm_tis_spi.c  |  78 ---
 drivers/char/tpm/tpm_tis_spi.h  |  53 +
 drivers/char/tpm/tpm_tis_spi_cr50.c | 321 
 5 files changed, 431 insertions(+), 32 deletions(-)
 create mode 100644 drivers/char/tpm/tpm_tis_spi.h
 create mode 100644 drivers/char/tpm/tpm_tis_spi_cr50.c

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 88a3c06fc153..ecbffae14941 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -67,6 +67,13 @@ config TCG_TIS_SPI
  within Linux. To compile this driver as a module, choose  M here;
  the module will be called tpm_tis_spi.
 
+config TCG_TIS_SPI_CR50
+   bool "Cr50 SPI Interface"
+   depends on TCG_TIS_SPI
+   ---help---
+ If you have a H1 secure module running Cr50 firmware on SPI bus,
+ say Yes and it will be accessible from within Linux.
+
 config TCG_TIS_I2C_ATMEL
tristate "TPM Interface Specification 1.2 Interface (I2C - Atmel)"
depends on I2C
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a01c4cab902a..c96439f11c85 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -21,7 +21,9 @@ tpm-$(CONFIG_EFI) += eventlog/efi.o
 tpm-$(CONFIG_OF) += eventlog/of.o
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o
-obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
+obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi_mod.o
+tpm_tis_spi_mod-y := tpm_tis_spi.o
+tpm_tis_spi_mod-$(CONFIG_TCG_TIS_SPI_CR50) += tpm_tis_spi_cr50.o
 obj-$(CONFIG_TCG_TIS_I2C_ATMEL) += tpm_i2c_atmel.o
 obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o
 obj-$(CONFIG_TCG_TIS_I2C_NUVOTON) += tpm_i2c_nuvoton.o
diff --git a/drivers/char/tpm/tpm_tis_spi.c b/drivers/char/tpm/tpm_tis_spi.c
index b3ed85671dd8..5e4253e7c080 100644
--- a/drivers/char/tpm/tpm_tis_spi.c
+++ b/drivers/char/tpm/tpm_tis_spi.c
@@ -20,6 +20,7 @@
  * Dorn and Kyleen Hall and Jarko Sakkinnen.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -31,27 +32,16 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include "tpm.h"
 #include "tpm_tis_core.h"
+#include "tpm_tis_spi.h"
 
 #define MAX_SPI_FRAMESIZE 64
 
-struct tpm_tis_spi_phy {
-   struct tpm_tis_data priv;
-   struct spi_device *spi_device;
-   int (*flow_control)(struct tpm_tis_spi_phy *phy,
-   struct spi_transfer *xfer);
-   u8 *iobuf;
-};
-
-static inline struct tpm_tis_spi_phy *to_tpm_tis_spi_phy(struct tpm_tis_data 
*data)
-{
-   return container_of(data, struct tpm_tis_spi_phy, priv);
-}
-
 /*
  * TCG SPI flow control is documented in section 6.4 of the spec[1]. In short,
  * keep trying to read from the device until MISO goes high indicating the
@@ -87,8 +77,8 @@ static int