Re: [PATCH v4 2/2] tpm: add driver for cr50 on SPI

[Jarkko Sakkinen]

On Thu, Jul 28, 2016 at 06:55:14PM -0700, Andrey Pronin wrote:
> 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.
> 
> Signed-off-by: Andrey Pronin 
> ---
>  drivers/char/tpm/Kconfig|   9 ++
>  drivers/char/tpm/Makefile   |   1 +
>  drivers/char/tpm/cr50_spi.c | 350 
> 
>  3 files changed, 360 insertions(+)
>  create mode 100644 drivers/char/tpm/cr50_spi.c
> 
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 9faa0b1..3320acc 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -100,6 +100,15 @@ config TCG_ATMEL
> will be accessible from within Linux.  To compile this driver 
> as a module, choose M here; the module will be called tpm_atmel.
>  
> +config TCG_CR50_SPI
> + tristate "TCG PTP FIFO Interface over SPI - Chips with Cr50 Firmware"
> + depends on SPI
> + select TCG_TIS_CORE
> + ---help---
> +   If you have a chip running Cr50 firmware on SPI bus, say Yes and it
> +   will be accessible from within Linux. To compile this driver as a
> +   module, choose M here; the module will be called cr50_spi.
> +
>  config TCG_INFINEON
>   tristate "Infineon Technologies TPM Interface"
>   depends on PNP
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index a385fb8..b346306 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o
>  obj-$(CONFIG_TCG_TIS_I2C_NUVOTON) += tpm_i2c_nuvoton.o
>  obj-$(CONFIG_TCG_NSC) += tpm_nsc.o
>  obj-$(CONFIG_TCG_ATMEL) += tpm_atmel.o
> +obj-$(CONFIG_TCG_CR50_SPI) += cr50_spi.o
>  obj-$(CONFIG_TCG_INFINEON) += tpm_infineon.o
>  obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
>  obj-$(CONFIG_TCG_TIS_ST33ZP24) += st33zp24/
> diff --git a/drivers/char/tpm/cr50_spi.c b/drivers/char/tpm/cr50_spi.c
> new file mode 100644
> index 000..9cc1620
> --- /dev/null
> +++ b/drivers/char/tpm/cr50_spi.c
> @@ -0,0 +1,350 @@
> +/*
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2, as published 
> by
> + * the Free Software Foundation.
> + *
> + * This device driver implements a TCG PTP FIFO interface over SPI for chips
> + * with Cr50 firmware.
> + * It is based on tpm_tis_spi driver by Peter Huewe and Christophe Ricard.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include "tpm.h"
> +#include "tpm_tis_core.h"
> +
> +/*
> + * Cr50 timing constants:
> + * - can go to sleep not earlier than after CR50_SLEEP_DELAY_MSEC
> + * - needs up to CR50_WAKE_START_DELAY_MSEC to wake after sleep
> + * - requires at least CR50_ACCESS_DELAY_MSEC between transactions
> + */
> +#define CR50_SLEEP_DELAY_MSEC1000
> +#define CR50_WAKE_START_DELAY_MSEC   60
> +#define CR50_ACCESS_DELAY_MSEC   2
> +
> +#define MAX_SPI_FRAMESIZE64
> +
> +#define TPM_CR50_FW_VER(l)   (0x0F90 | ((l) << 12))
> +#define TPM_CR50_MAX_FW_VER_LEN  64
> +
> +struct cr50_spi_phy {
> + struct tpm_tis_data priv;
> + struct spi_device *spi_device;
> +
> + struct mutex time_track_mutex;
> + unsigned long last_access_jiffies;
> + unsigned long wake_after_jiffies;
> +
> + unsigned long access_delay_jiffies;
> + unsigned long sleep_delay_jiffies;
> + unsigned int wake_start_delay_msec;
> +
> + u8 tx_buf[MAX_SPI_FRAMESIZE] cacheline_aligned;
> + u8 rx_buf[MAX_SPI_FRAMESIZE] cacheline_aligned;
> +};
> +
> +static struct cr50_spi_phy *to_cr50_spi_phy(struct tpm_tis_data *data)
> +{
> + return container_of(data, struct cr50_spi_phy, priv);
> +}
> +
> +/*
> + * Cr50 needs to have at least some delay between consecutive
> + * transactions. Make sure we wait.
> + */

"How to format kernel-doc comments" [1]

[1] https://www.kernel.org/doc/Documentation/kernel-documentation.rst

/Jarkko

> +static void cr50_ensure_access_delay(struct cr50_spi_phy *phy)
> +{
> + /*
> +  * Note: There is a small chance, if Cr50 is not accessed in a few days,
> +  * that time_in_range will not provide the correct result after the wrap
> +  * around for jiffies. In this case, we'll have an unneeded short delay,
> +  * which is fine.
> +  */
> + unsigned long 

Re: [PATCH v4 2/2] tpm: add driver for cr50 on SPI

[Jarkko Sakkinen]

On Thu, Jul 28, 2016 at 06:55:14PM -0700, Andrey Pronin wrote:
> 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.
> 
> Signed-off-by: Andrey Pronin 
> ---
>  drivers/char/tpm/Kconfig|   9 ++
>  drivers/char/tpm/Makefile   |   1 +
>  drivers/char/tpm/cr50_spi.c | 350 
> 
>  3 files changed, 360 insertions(+)
>  create mode 100644 drivers/char/tpm/cr50_spi.c
> 
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 9faa0b1..3320acc 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -100,6 +100,15 @@ config TCG_ATMEL
> will be accessible from within Linux.  To compile this driver 
> as a module, choose M here; the module will be called tpm_atmel.
>  
> +config TCG_CR50_SPI
> + tristate "TCG PTP FIFO Interface over SPI - Chips with Cr50 Firmware"
> + depends on SPI
> + select TCG_TIS_CORE
> + ---help---
> +   If you have a chip running Cr50 firmware on SPI bus, say Yes and it
> +   will be accessible from within Linux. To compile this driver as a
> +   module, choose M here; the module will be called cr50_spi.
> +
>  config TCG_INFINEON
>   tristate "Infineon Technologies TPM Interface"
>   depends on PNP
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index a385fb8..b346306 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o
>  obj-$(CONFIG_TCG_TIS_I2C_NUVOTON) += tpm_i2c_nuvoton.o
>  obj-$(CONFIG_TCG_NSC) += tpm_nsc.o
>  obj-$(CONFIG_TCG_ATMEL) += tpm_atmel.o
> +obj-$(CONFIG_TCG_CR50_SPI) += cr50_spi.o
>  obj-$(CONFIG_TCG_INFINEON) += tpm_infineon.o
>  obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
>  obj-$(CONFIG_TCG_TIS_ST33ZP24) += st33zp24/
> diff --git a/drivers/char/tpm/cr50_spi.c b/drivers/char/tpm/cr50_spi.c
> new file mode 100644
> index 000..9cc1620
> --- /dev/null
> +++ b/drivers/char/tpm/cr50_spi.c
> @@ -0,0 +1,350 @@
> +/*
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2, as published 
> by
> + * the Free Software Foundation.
> + *
> + * This device driver implements a TCG PTP FIFO interface over SPI for chips
> + * with Cr50 firmware.
> + * It is based on tpm_tis_spi driver by Peter Huewe and Christophe Ricard.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include "tpm.h"
> +#include "tpm_tis_core.h"
> +
> +/*
> + * Cr50 timing constants:
> + * - can go to sleep not earlier than after CR50_SLEEP_DELAY_MSEC
> + * - needs up to CR50_WAKE_START_DELAY_MSEC to wake after sleep
> + * - requires at least CR50_ACCESS_DELAY_MSEC between transactions
> + */
> +#define CR50_SLEEP_DELAY_MSEC1000
> +#define CR50_WAKE_START_DELAY_MSEC   60
> +#define CR50_ACCESS_DELAY_MSEC   2
> +
> +#define MAX_SPI_FRAMESIZE64
> +
> +#define TPM_CR50_FW_VER(l)   (0x0F90 | ((l) << 12))
> +#define TPM_CR50_MAX_FW_VER_LEN  64
> +
> +struct cr50_spi_phy {
> + struct tpm_tis_data priv;
> + struct spi_device *spi_device;
> +
> + struct mutex time_track_mutex;
> + unsigned long last_access_jiffies;
> + unsigned long wake_after_jiffies;
> +
> + unsigned long access_delay_jiffies;
> + unsigned long sleep_delay_jiffies;
> + unsigned int wake_start_delay_msec;
> +
> + u8 tx_buf[MAX_SPI_FRAMESIZE] cacheline_aligned;
> + u8 rx_buf[MAX_SPI_FRAMESIZE] cacheline_aligned;
> +};
> +
> +static struct cr50_spi_phy *to_cr50_spi_phy(struct tpm_tis_data *data)
> +{
> + return container_of(data, struct cr50_spi_phy, priv);
> +}
> +
> +/*
> + * Cr50 needs to have at least some delay between consecutive
> + * transactions. Make sure we wait.
> + */

"How to format kernel-doc comments" [1]

[1] https://www.kernel.org/doc/Documentation/kernel-documentation.rst

/Jarkko

> +static void cr50_ensure_access_delay(struct cr50_spi_phy *phy)
> +{
> + /*
> +  * Note: There is a small chance, if Cr50 is not accessed in a few days,
> +  * that time_in_range will not provide the correct result after the wrap
> +  * around for jiffies. In this case, we'll have an unneeded short delay,
> +  * which is fine.
> +  */
> + unsigned long allowed_access =
> + 

[PATCH v4 2/2] tpm: add driver for cr50 on SPI

[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.

Signed-off-by: Andrey Pronin 
---
 drivers/char/tpm/Kconfig|   9 ++
 drivers/char/tpm/Makefile   |   1 +
 drivers/char/tpm/cr50_spi.c | 350 
 3 files changed, 360 insertions(+)
 create mode 100644 drivers/char/tpm/cr50_spi.c

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 9faa0b1..3320acc 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -100,6 +100,15 @@ config TCG_ATMEL
  will be accessible from within Linux.  To compile this driver 
  as a module, choose M here; the module will be called tpm_atmel.
 
+config TCG_CR50_SPI
+   tristate "TCG PTP FIFO Interface over SPI - Chips with Cr50 Firmware"
+   depends on SPI
+   select TCG_TIS_CORE
+   ---help---
+ If you have a chip running Cr50 firmware on SPI bus, say Yes and it
+ will be accessible from within Linux. To compile this driver as a
+ module, choose M here; the module will be called cr50_spi.
+
 config TCG_INFINEON
tristate "Infineon Technologies TPM Interface"
depends on PNP
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a385fb8..b346306 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o
 obj-$(CONFIG_TCG_TIS_I2C_NUVOTON) += tpm_i2c_nuvoton.o
 obj-$(CONFIG_TCG_NSC) += tpm_nsc.o
 obj-$(CONFIG_TCG_ATMEL) += tpm_atmel.o
+obj-$(CONFIG_TCG_CR50_SPI) += cr50_spi.o
 obj-$(CONFIG_TCG_INFINEON) += tpm_infineon.o
 obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
 obj-$(CONFIG_TCG_TIS_ST33ZP24) += st33zp24/
diff --git a/drivers/char/tpm/cr50_spi.c b/drivers/char/tpm/cr50_spi.c
new file mode 100644
index 000..9cc1620
--- /dev/null
+++ b/drivers/char/tpm/cr50_spi.c
@@ -0,0 +1,350 @@
+/*
+ * Copyright (C) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2, as published by
+ * the Free Software Foundation.
+ *
+ * This device driver implements a TCG PTP FIFO interface over SPI for chips
+ * with Cr50 firmware.
+ * It is based on tpm_tis_spi driver by Peter Huewe and Christophe Ricard.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include "tpm.h"
+#include "tpm_tis_core.h"
+
+/*
+ * Cr50 timing constants:
+ * - can go to sleep not earlier than after CR50_SLEEP_DELAY_MSEC
+ * - needs up to CR50_WAKE_START_DELAY_MSEC to wake after sleep
+ * - requires at least CR50_ACCESS_DELAY_MSEC between transactions
+ */
+#define CR50_SLEEP_DELAY_MSEC  1000
+#define CR50_WAKE_START_DELAY_MSEC 60
+#define CR50_ACCESS_DELAY_MSEC 2
+
+#define MAX_SPI_FRAMESIZE  64
+
+#define TPM_CR50_FW_VER(l) (0x0F90 | ((l) << 12))
+#define TPM_CR50_MAX_FW_VER_LEN64
+
+struct cr50_spi_phy {
+   struct tpm_tis_data priv;
+   struct spi_device *spi_device;
+
+   struct mutex time_track_mutex;
+   unsigned long last_access_jiffies;
+   unsigned long wake_after_jiffies;
+
+   unsigned long access_delay_jiffies;
+   unsigned long sleep_delay_jiffies;
+   unsigned int wake_start_delay_msec;
+
+   u8 tx_buf[MAX_SPI_FRAMESIZE] cacheline_aligned;
+   u8 rx_buf[MAX_SPI_FRAMESIZE] cacheline_aligned;
+};
+
+static struct cr50_spi_phy *to_cr50_spi_phy(struct tpm_tis_data *data)
+{
+   return container_of(data, struct cr50_spi_phy, priv);
+}
+
+/*
+ * Cr50 needs to have at least some delay between consecutive
+ * transactions. Make sure we wait.
+ */
+static void cr50_ensure_access_delay(struct cr50_spi_phy *phy)
+{
+   /*
+* Note: There is a small chance, if Cr50 is not accessed in a few days,
+* that time_in_range will not provide the correct result after the wrap
+* around for jiffies. In this case, we'll have an unneeded short delay,
+* which is fine.
+*/
+   unsigned long allowed_access =
+   phy->last_access_jiffies + phy->access_delay_jiffies;
+   unsigned long time_now = jiffies;
+
+   if (time_in_range_open(time_now,
+  phy->last_access_jiffies, allowed_access))
+   mdelay(jiffies_to_msecs(allowed_access - time_now));
+}
+
+/*
+ * Cr50 might go to sleep if there is no SPI activity for some time and
+ * miss 

[PATCH v4 2/2] tpm: add driver for cr50 on SPI

[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.

Signed-off-by: Andrey Pronin 
---
 drivers/char/tpm/Kconfig|   9 ++
 drivers/char/tpm/Makefile   |   1 +
 drivers/char/tpm/cr50_spi.c | 350 
 3 files changed, 360 insertions(+)
 create mode 100644 drivers/char/tpm/cr50_spi.c

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 9faa0b1..3320acc 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -100,6 +100,15 @@ config TCG_ATMEL
  will be accessible from within Linux.  To compile this driver 
  as a module, choose M here; the module will be called tpm_atmel.
 
+config TCG_CR50_SPI
+   tristate "TCG PTP FIFO Interface over SPI - Chips with Cr50 Firmware"
+   depends on SPI
+   select TCG_TIS_CORE
+   ---help---
+ If you have a chip running Cr50 firmware on SPI bus, say Yes and it
+ will be accessible from within Linux. To compile this driver as a
+ module, choose M here; the module will be called cr50_spi.
+
 config TCG_INFINEON
tristate "Infineon Technologies TPM Interface"
depends on PNP
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a385fb8..b346306 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_TCG_TIS_I2C_INFINEON) += tpm_i2c_infineon.o
 obj-$(CONFIG_TCG_TIS_I2C_NUVOTON) += tpm_i2c_nuvoton.o
 obj-$(CONFIG_TCG_NSC) += tpm_nsc.o
 obj-$(CONFIG_TCG_ATMEL) += tpm_atmel.o
+obj-$(CONFIG_TCG_CR50_SPI) += cr50_spi.o
 obj-$(CONFIG_TCG_INFINEON) += tpm_infineon.o
 obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
 obj-$(CONFIG_TCG_TIS_ST33ZP24) += st33zp24/
diff --git a/drivers/char/tpm/cr50_spi.c b/drivers/char/tpm/cr50_spi.c
new file mode 100644
index 000..9cc1620
--- /dev/null
+++ b/drivers/char/tpm/cr50_spi.c
@@ -0,0 +1,350 @@
+/*
+ * Copyright (C) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2, as published by
+ * the Free Software Foundation.
+ *
+ * This device driver implements a TCG PTP FIFO interface over SPI for chips
+ * with Cr50 firmware.
+ * It is based on tpm_tis_spi driver by Peter Huewe and Christophe Ricard.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include "tpm.h"
+#include "tpm_tis_core.h"
+
+/*
+ * Cr50 timing constants:
+ * - can go to sleep not earlier than after CR50_SLEEP_DELAY_MSEC
+ * - needs up to CR50_WAKE_START_DELAY_MSEC to wake after sleep
+ * - requires at least CR50_ACCESS_DELAY_MSEC between transactions
+ */
+#define CR50_SLEEP_DELAY_MSEC  1000
+#define CR50_WAKE_START_DELAY_MSEC 60
+#define CR50_ACCESS_DELAY_MSEC 2
+
+#define MAX_SPI_FRAMESIZE  64
+
+#define TPM_CR50_FW_VER(l) (0x0F90 | ((l) << 12))
+#define TPM_CR50_MAX_FW_VER_LEN64
+
+struct cr50_spi_phy {
+   struct tpm_tis_data priv;
+   struct spi_device *spi_device;
+
+   struct mutex time_track_mutex;
+   unsigned long last_access_jiffies;
+   unsigned long wake_after_jiffies;
+
+   unsigned long access_delay_jiffies;
+   unsigned long sleep_delay_jiffies;
+   unsigned int wake_start_delay_msec;
+
+   u8 tx_buf[MAX_SPI_FRAMESIZE] cacheline_aligned;
+   u8 rx_buf[MAX_SPI_FRAMESIZE] cacheline_aligned;
+};
+
+static struct cr50_spi_phy *to_cr50_spi_phy(struct tpm_tis_data *data)
+{
+   return container_of(data, struct cr50_spi_phy, priv);
+}
+
+/*
+ * Cr50 needs to have at least some delay between consecutive
+ * transactions. Make sure we wait.
+ */
+static void cr50_ensure_access_delay(struct cr50_spi_phy *phy)
+{
+   /*
+* Note: There is a small chance, if Cr50 is not accessed in a few days,
+* that time_in_range will not provide the correct result after the wrap
+* around for jiffies. In this case, we'll have an unneeded short delay,
+* which is fine.
+*/
+   unsigned long allowed_access =
+   phy->last_access_jiffies + phy->access_delay_jiffies;
+   unsigned long time_now = jiffies;
+
+   if (time_in_range_open(time_now,
+  phy->last_access_jiffies, allowed_access))
+   mdelay(jiffies_to_msecs(allowed_access - time_now));
+}
+
+/*
+ * Cr50 might go to sleep if there is no SPI activity for some time and
+ * miss the first few