Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1443884 > unrolled thread

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

Started byAndrey Pronin <apronin@chromium.org>
First post2016-07-15 04:30 +0200
Last post2016-07-25 19:10 +0200
Articles 20 on this page of 27 — 7 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/2] tpm: add driver for cr50 on SPI Andrey Pronin <apronin@chromium.org> - 2016-07-15 04:30 +0200
    [PATCH 1/2] tpm: devicetree: document properties for cr50 Andrey Pronin <apronin@chromium.org> - 2016-07-15 04:30 +0200
      Re: [PATCH 1/2] tpm: devicetree: document properties for cr50 Guenter Roeck <groeck@google.com> - 2016-07-15 06:10 +0200
        Re: [PATCH 1/2] tpm: devicetree: document properties for cr50 Andrey Pronin <apronin@chromium.org> - 2016-07-15 19:40 +0200
          Re: [PATCH 1/2] tpm: devicetree: document properties for cr50 Guenter Roeck <groeck@google.com> - 2016-07-15 20:30 +0200
      Re: [PATCH 1/2] tpm: devicetree: document properties for cr50 Rob Herring <robh@kernel.org> - 2016-07-17 15:30 +0200
    [PATCH 2/2] tpm: add driver for cr50 on SPI Andrey Pronin <apronin@chromium.org> - 2016-07-15 04:30 +0200
      Re: [PATCH 2/2] tpm: add driver for cr50 on SPI Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-07-15 05:40 +0200
        Re: [PATCH 2/2] tpm: add driver for cr50 on SPI Andrey Pronin <apronin@chromium.org> - 2016-07-15 05:50 +0200
          Re: [PATCH 2/2] tpm: add driver for cr50 on SPI Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-07-19 15:00 +0200
            Re: [PATCH 2/2] tpm: add driver for cr50 on SPI Andrey Pronin <apronin@chromium.org> - 2016-07-20 02:30 +0200
              Re: [PATCH 2/2] tpm: add driver for cr50 on SPI Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-07-20 19:10 +0200
                Re: [PATCH 2/2] tpm: add driver for cr50 on SPI Andrey Pronin <apronin@chromium.org> - 2016-07-21 20:20 +0200
                  Re: [PATCH 2/2] tpm: add driver for cr50 on SPI Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-07-21 23:10 +0200
    Re: [PATCH 0/2] tpm: add driver for cr50 on SPI Peter Huewe <peterhuewe@gmx.de> - 2016-07-15 04:30 +0200
      Re: [PATCH 0/2] tpm: add driver for cr50 on SPI Andrey Pronin <apronin@chromium.org> - 2016-07-15 05:00 +0200
        Re: [PATCH 0/2] tpm: add driver for cr50 on SPI Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-07-15 05:30 +0200
          Re: [PATCH 0/2] tpm: add driver for cr50 on SPI Andrey Pronin <apronin@chromium.org> - 2016-07-15 19:20 +0200
    Re: [PATCH 0/2] tpm: add driver for cr50 on SPI Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-07-19 15:00 +0200
    [PATCH v2 1/2] tpm: devicetree: document properties for cr50 Andrey Pronin <apronin@chromium.org> - 2016-07-20 05:50 +0200
      Re: [PATCH v2 1/2] tpm: devicetree: document properties for cr50 Rob Herring <robh@kernel.org> - 2016-07-20 21:10 +0200
        Re: [PATCH v2 1/2] tpm: devicetree: document properties for cr50 Andrey Pronin <apronin@chromium.org> - 2016-07-20 21:50 +0200
          Re: [PATCH v2 1/2] tpm: devicetree: document properties for cr50 Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-07-20 22:00 +0200
          Re: [PATCH v2 1/2] tpm: devicetree: document properties for cr50 Rob Herring <robh@kernel.org> - 2016-07-21 23:10 +0200
    [PATCH v2 0/2] tpm: add driver for cr50 on SPI Andrey Pronin <apronin@chromium.org> - 2016-07-20 05:50 +0200
      [PATCH v2 2/2] tpm: add driver for cr50 on SPI Andrey Pronin <apronin@chromium.org> - 2016-07-20 05:50 +0200
      Aw: [PATCH v2 0/2] tpm: add driver for cr50 on SPI "Peter Huewe" <PeterHuewe@gmx.de> - 2016-07-25 19:10 +0200

Page 1 of 2  [1] 2  Next page →


#1443884 — [PATCH 0/2] tpm: add driver for cr50 on SPI

FromAndrey Pronin <apronin@chromium.org>
Date2016-07-15 04:30 +0200
Subject[PATCH 0/2] tpm: add driver for cr50 on SPI
Message-ID<rV133-14j-7@gated-at.bofh.it>
This patchset adds a TCG TPM2.0 PTP FIFO compliant interface for
Cr50 chip on SPI.

Depends on the following patches by Andrey Pronin <apronin@chromium.org>
that add new members to phy_ops in tpm_tis_core:
 - tpm: support driver-specific sysfs attrs in tpm_tis_core
 - tpm_tis_core: add optional max xfer size check

Andrey Pronin (2):
  tpm: devicetree: document properties for cr50
  tpm: add driver for cr50 on SPI

 .../devicetree/bindings/security/tpm/cr50_spi.txt  |  30 ++
 drivers/char/tpm/Kconfig                           |   9 +
 drivers/char/tpm/Makefile                          |   1 +
 drivers/char/tpm/cr50_spi.c                        | 409 +++++++++++++++++++++
 4 files changed, 449 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
 create mode 100644 drivers/char/tpm/cr50_spi.c

-- 
2.6.6

[toc] | [next] | [standalone]


#1443886 — [PATCH 1/2] tpm: devicetree: document properties for cr50

FromAndrey Pronin <apronin@chromium.org>
Date2016-07-15 04:30 +0200
Subject[PATCH 1/2] tpm: devicetree: document properties for cr50
Message-ID<rV133-14j-11@gated-at.bofh.it>
In reply to#1443884
Add TPM2.0-compatible interface to Cr50. Document its properties
in devicetree.

Signed-off-by: Andrey Pronin <apronin@chromium.org>
---
 .../devicetree/bindings/security/tpm/cr50_spi.txt  | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/cr50_spi.txt

diff --git a/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt b/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
new file mode 100644
index 0000000..1b05e51
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
@@ -0,0 +1,30 @@
+* Cr50 Chip on SPI.
+
+TCG PTP FIFO Compliant Interface to Cr50 on SPI bus.
+
+Required properties:
+- compatible: Should be "google,cr50_spi".
+- spi-max-frequency: Maximum SPI frequency.
+
+Optional properties:
+- access-delay-msec: Required delay between subsequent transactions on SPI.
+- sleep-delay-msec: Time after the last SPI activity, after which the chip
+  may go to sleep.
+- wake-start-delay-msec: Time after initiating wake up before the chip is
+  ready to accept commands over SPI.
+
+Example:
+
+&spi0 {
+        status = "okay";
+
+        cr50@0 {
+                compatible = "google,cr50_spi";
+                reg = <0>;
+                spi-max-frequency = <800000>;
+
+                access-delay-msec = <2>;
+                sleep-delay-msec = <1000>;
+                wake-start-delay-msec = <60>;
+        };
+};
-- 
2.6.6

[toc] | [prev] | [next] | [standalone]


#1443928 — Re: [PATCH 1/2] tpm: devicetree: document properties for cr50

FromGuenter Roeck <groeck@google.com>
Date2016-07-15 06:10 +0200
SubjectRe: [PATCH 1/2] tpm: devicetree: document properties for cr50
Message-ID<rV2BP-25M-3@gated-at.bofh.it>
In reply to#1443886
On Thu, Jul 14, 2016 at 7:20 PM, Andrey Pronin <apronin@chromium.org> wrote:
> Add TPM2.0-compatible interface to Cr50. Document its properties
> in devicetree.
>
> Signed-off-by: Andrey Pronin <apronin@chromium.org>
> ---
>  .../devicetree/bindings/security/tpm/cr50_spi.txt  | 30 ++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
>
> diff --git a/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt b/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
> new file mode 100644
> index 0000000..1b05e51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
> @@ -0,0 +1,30 @@
> +* Cr50 Chip on SPI.
> +
> +TCG PTP FIFO Compliant Interface to Cr50 on SPI bus.
> +
> +Required properties:
> +- compatible: Should be "google,cr50_spi".

google,cr50, maybe ? The "_spi" seems redundant.

Also, I agree with comments from others - the term cr50 really needs
an explanation (Google thinks that it is a motor bike, a scanner, or a
coffee roaster).

Thanks,
Guenter

> +- spi-max-frequency: Maximum SPI frequency.
> +
> +Optional properties:
> +- access-delay-msec: Required delay between subsequent transactions on SPI.
> +- sleep-delay-msec: Time after the last SPI activity, after which the chip
> +  may go to sleep.
> +- wake-start-delay-msec: Time after initiating wake up before the chip is
> +  ready to accept commands over SPI.
> +
> +Example:
> +
> +&spi0 {
> +        status = "okay";
> +
> +        cr50@0 {
> +                compatible = "google,cr50_spi";
> +                reg = <0>;
> +                spi-max-frequency = <800000>;
> +
> +                access-delay-msec = <2>;
> +                sleep-delay-msec = <1000>;
> +                wake-start-delay-msec = <60>;
> +        };
> +};
> --
> 2.6.6
>

[toc] | [prev] | [next] | [standalone]


#1444472 — Re: [PATCH 1/2] tpm: devicetree: document properties for cr50

FromAndrey Pronin <apronin@chromium.org>
Date2016-07-15 19:40 +0200
SubjectRe: [PATCH 1/2] tpm: devicetree: document properties for cr50
Message-ID<rVffH-1iY-9@gated-at.bofh.it>
In reply to#1443928
On Thu, Jul 14, 2016 at 09:05:53PM -0700, Guenter Roeck wrote:
> On Thu, Jul 14, 2016 at 7:20 PM, Andrey Pronin <apronin@chromium.org> wrote:
> > +
> > +Required properties:
> > +- compatible: Should be "google,cr50_spi".
> 
> google,cr50, maybe ? The "_spi" seems redundant.
>

I believe "_spi" is warranted. It's the driver that handles the SPI
interface for Cr50 specifically.
But if the same firmware ever talks through a different interface (say,
I2C), this driver will not be compatible.

> Also, I agree with comments from others - the term cr50 really needs
> an explanation (Google thinks that it is a motor bike, a scanner, or a
> coffee roaster).
> 

Yes, will add a better description of what it is. My original one was
too brief and imprecise at the same time.

[toc] | [prev] | [next] | [standalone]


#1444500 — Re: [PATCH 1/2] tpm: devicetree: document properties for cr50

FromGuenter Roeck <groeck@google.com>
Date2016-07-15 20:30 +0200
SubjectRe: [PATCH 1/2] tpm: devicetree: document properties for cr50
Message-ID<rVg25-1P7-1@gated-at.bofh.it>
In reply to#1444472
On Fri, Jul 15, 2016 at 10:31 AM, Andrey Pronin <apronin@chromium.org> wrote:
> On Thu, Jul 14, 2016 at 09:05:53PM -0700, Guenter Roeck wrote:
>> On Thu, Jul 14, 2016 at 7:20 PM, Andrey Pronin <apronin@chromium.org> wrote:
>> > +
>> > +Required properties:
>> > +- compatible: Should be "google,cr50_spi".
>>
>> google,cr50, maybe ? The "_spi" seems redundant.
>>
>
> I believe "_spi" is warranted. It's the driver that handles the SPI
> interface for Cr50 specifically.
> But if the same firmware ever talks through a different interface (say,
> I2C), this driver will not be compatible.
>
I meant in the context of it being attached to a spi device, which
implies that it is connected through a spi bus.

Guenter

>> Also, I agree with comments from others - the term cr50 really needs
>> an explanation (Google thinks that it is a motor bike, a scanner, or a
>> coffee roaster).
>>
>
> Yes, will add a better description of what it is. My original one was
> too brief and imprecise at the same time.
>

[toc] | [prev] | [next] | [standalone]


#1445050 — Re: [PATCH 1/2] tpm: devicetree: document properties for cr50

FromRob Herring <robh@kernel.org>
Date2016-07-17 15:30 +0200
SubjectRe: [PATCH 1/2] tpm: devicetree: document properties for cr50
Message-ID<rVUiS-1ks-5@gated-at.bofh.it>
In reply to#1443886
On Thu, Jul 14, 2016 at 07:20:17PM -0700, Andrey Pronin wrote:
> Add TPM2.0-compatible interface to Cr50. Document its properties
> in devicetree.
> 
> Signed-off-by: Andrey Pronin <apronin@chromium.org>
> ---
>  .../devicetree/bindings/security/tpm/cr50_spi.txt  | 30 ++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
> 
> diff --git a/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt b/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
> new file mode 100644
> index 0000000..1b05e51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
> @@ -0,0 +1,30 @@
> +* Cr50 Chip on SPI.
> +
> +TCG PTP FIFO Compliant Interface to Cr50 on SPI bus.
> +
> +Required properties:
> +- compatible: Should be "google,cr50_spi".

I agree with dropping '_spi'. The interface is defined by the parent 
device.

> +- spi-max-frequency: Maximum SPI frequency.
> +
> +Optional properties:
> +- access-delay-msec: Required delay between subsequent transactions on SPI.

There may be a standard property for this...

> +- sleep-delay-msec: Time after the last SPI activity, after which the chip
> +  may go to sleep.
> +- wake-start-delay-msec: Time after initiating wake up before the chip is
> +  ready to accept commands over SPI.

Use the standard unit '-ms' instead of '-msec'.

Do these times really vary much and need to be in DT?

[toc] | [prev] | [next] | [standalone]


#1443887 — [PATCH 2/2] tpm: add driver for cr50 on SPI

FromAndrey Pronin <apronin@chromium.org>
Date2016-07-15 04:30 +0200
Subject[PATCH 2/2] tpm: add driver for cr50 on SPI
Message-ID<rV133-14j-15@gated-at.bofh.it>
In reply to#1443884
Add TCG TPM2.0 PTP FIFO compatible interface for Cr50 chip on SPI bus.

Signed-off-by: Andrey Pronin <apronin@chromium.org>
---
 drivers/char/tpm/Kconfig    |   9 +
 drivers/char/tpm/Makefile   |   1 +
 drivers/char/tpm/cr50_spi.c | 409 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 419 insertions(+)
 create mode 100644 drivers/char/tpm/cr50_spi.c

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 9faa0b1..6773a06 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 Compliant Interface over SPI for Cr50"
+	depends on SPI
+	select TCG_TIS_CORE
+	---help---
+	  If you have a Cr50 chip 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 0000000..0672434
--- /dev/null
+++ b/drivers/char/tpm/cr50_spi.c
@@ -0,0 +1,409 @@
+/*
+ * 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 compliant interface over SPI
+ * for Cr50 devices.
+ * It is based on cr50_spi driver by Peter Huewe and Christophe Ricard.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/wait.h>
+#include <linux/freezer.h>
+
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/gpio.h>
+#include <linux/of_irq.h>
+#include <linux/of_gpio.h>
+#include <linux/tpm.h>
+#include "tpm.h"
+#include "tpm_tis_core.h"
+
+#define MAX_SPI_FRAMESIZE	64
+
+/* Cr50 default 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_DFT_SLEEP_DELAY_MSEC		1000
+#define CR50_DFT_WAKE_START_DELAY_MSEC		60
+#define CR50_DFT_ACCESS_DELAY_MSEC		2
+
+#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;
+};
+
+static inline 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 inline 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 bits/bytes on the bus. In such case, wake it up
+ * by asserting CS and give it time to start up.
+ */
+static inline bool cr50_needs_waking(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 probably timeout or read
+	 * incorrect value from TPM_STS and just retry the operation.
+	 */
+	return !time_in_range_open(jiffies,
+				   phy->last_access_jiffies,
+				   phy->wake_after_jiffies);
+}
+static inline void cr50_wake_if_needed(struct cr50_spi_phy *phy)
+{
+	if (cr50_needs_waking(phy)) {
+		/* assert CS, wait 1 msec, deassert CS */
+		struct spi_transfer spi_cs_wake = { .delay_usecs = 1000 };
+
+		spi_sync_transfer(phy->spi_device, &spi_cs_wake, 1);
+		/* wait for it to fully wake */
+		msleep(phy->wake_start_delay_msec);
+	}
+	/* Reset the time when we need to wake Cr50 again */
+	phy->wake_after_jiffies = jiffies + phy->sleep_delay_jiffies;
+}
+
+/* Flow control: clock the bus and wait for cr50 to set LSB before
+ * sending/receiving data. TCG PTP spec allows it to happen during
+ * the last byte of header, but cr50 never does that in practice,
+ * and earlier versions had a bug when it was set too early, so don't
+ * check for it during header transfer.
+ */
+static int cr50_spi_flow_control(struct cr50_spi_phy *phy)
+{
+	unsigned long timeout_jiffies =
+		jiffies + msecs_to_jiffies(TPM_RETRY * TPM_TIMEOUT_RETRY);
+	struct spi_message m;
+	int ret;
+	u8 rx = 0;
+	struct spi_transfer spi_xfer = {
+		.rx_buf = &rx,
+		.len = 1,
+		.cs_change = 1,
+	};
+
+	do {
+		spi_message_init(&m);
+		spi_message_add_tail(&spi_xfer, &m);
+		ret = spi_sync_locked(phy->spi_device, &m);
+		if (ret < 0)
+			return ret;
+		if (time_after(jiffies, timeout_jiffies))
+			return -EBUSY;
+	} while (!(rx & 0x01));
+	return 0;
+}
+
+static int cr50_spi_xfer_bytes(struct tpm_tis_data *data, u32 addr,
+			       u16 len, u8 *buf, bool do_write)
+{
+	struct cr50_spi_phy *phy = to_cr50_spi_phy(data);
+	struct spi_message m;
+	u8 tx_buf[4];
+	u8 rx_buf[4];
+	struct spi_transfer spi_xfer = {
+		.tx_buf = tx_buf,
+		.rx_buf = rx_buf,
+		.len = 4,
+		.cs_change = 1,
+	};
+	int ret;
+
+	if (len > MAX_SPI_FRAMESIZE)
+		return -EINVAL;
+
+	/* Do this outside of spi_bus_lock in case cr50 is not the
+	 * only device on that spi bus.
+	 */
+	mutex_lock(&phy->time_track_mutex);
+	cr50_ensure_access_delay(phy);
+	cr50_wake_if_needed(phy);
+	mutex_unlock(&phy->time_track_mutex);
+
+	tx_buf[0] = (do_write ? 0x00 : 0x80) | (len - 1);
+	tx_buf[1] = 0xD4;
+	tx_buf[2] = (addr >> 8) & 0xFF;
+	tx_buf[3] = addr & 0xFF;
+
+	spi_message_init(&m);
+	spi_message_add_tail(&spi_xfer, &m);
+
+	spi_bus_lock(phy->spi_device->master);
+	ret = spi_sync_locked(phy->spi_device, &m);
+	if (ret < 0)
+		goto exit;
+
+	ret = cr50_spi_flow_control(phy);
+	if (ret < 0)
+		goto exit;
+
+	spi_xfer.cs_change = 0;
+	spi_xfer.len = len;
+	if (do_write) {
+		spi_xfer.tx_buf = buf;
+		spi_xfer.rx_buf = NULL;
+	} else {
+		spi_xfer.tx_buf = NULL;
+		spi_xfer.rx_buf = buf;
+	}
+
+	spi_message_init(&m);
+	spi_message_add_tail(&spi_xfer, &m);
+	ret = spi_sync_locked(phy->spi_device, &m);
+
+exit:
+	spi_bus_unlock(phy->spi_device->master);
+
+	mutex_lock(&phy->time_track_mutex);
+	phy->last_access_jiffies = jiffies;
+	mutex_unlock(&phy->time_track_mutex);
+
+	return ret;
+}
+
+static int cr50_spi_read_bytes(struct tpm_tis_data *data, u32 addr,
+			       u16 len, u8 *result)
+{
+	return cr50_spi_xfer_bytes(data, addr, len, result, false);
+}
+
+static int cr50_spi_write_bytes(struct tpm_tis_data *data, u32 addr,
+				u16 len, u8 *value)
+{
+	return cr50_spi_xfer_bytes(data, addr, len, value, true);
+}
+
+static int cr50_spi_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
+{
+	int rc;
+
+	rc = data->phy_ops->read_bytes(data, addr, sizeof(u16), (u8 *)result);
+	if (!rc)
+		*result = le16_to_cpu(*result);
+	return rc;
+}
+
+static int cr50_spi_read32(struct tpm_tis_data *data, u32 addr, u32 *result)
+{
+	int rc;
+
+	rc = data->phy_ops->read_bytes(data, addr, sizeof(u32), (u8 *)result);
+	if (!rc)
+		*result = le32_to_cpu(*result);
+	return rc;
+}
+
+static int cr50_spi_write32(struct tpm_tis_data *data, u32 addr, u32 value)
+{
+	value = cpu_to_le32(value);
+	return data->phy_ops->write_bytes(data, addr, sizeof(u32),
+					   (u8 *)&value);
+}
+
+static void cr50_get_fw_version(struct tpm_tis_data *data, char *fw_ver)
+{
+	int i, len = 0;
+	char fw_ver_block[4];
+
+	/* Write anything to TPM_CR50_FW_VER to start from the beg of string */
+	tpm_tis_write8(data, TPM_CR50_FW_VER(data->locality), 0);
+
+	/* Read the string, 4 bytes at a time, until we get '\0' */
+	do {
+		tpm_tis_read_bytes(data,
+			TPM_CR50_FW_VER(data->locality), 4, fw_ver_block);
+		for (i = 0; i < 4 && fw_ver_block[i]; )
+			fw_ver[len++] = fw_ver_block[i++];
+	} while (i == 4 && len < TPM_CR50_MAX_FW_VER_LEN);
+	fw_ver[len] = 0;
+}
+
+static ssize_t fw_version_show(struct device *dev,
+			       struct device_attribute *attr,
+			       char *buf)
+{
+	char fw_ver[TPM_CR50_MAX_FW_VER_LEN+1];
+	struct tpm_tis_data *data = dev_get_drvdata(dev);
+
+	cr50_get_fw_version(data, fw_ver);
+	return sprintf(buf, "%s\n", fw_ver);
+}
+static DEVICE_ATTR_RO(fw_version);
+
+static struct attribute *cr50_attrs[] = {
+	&dev_attr_fw_version.attr,
+	NULL,
+};
+
+static const struct attribute_group cr50_attr_group = {
+	.attrs = cr50_attrs,
+};
+
+static const struct tpm_tis_phy_ops cr50_spi_phy_ops = {
+	.read_bytes = cr50_spi_read_bytes,
+	.write_bytes = cr50_spi_write_bytes,
+	.read16 = cr50_spi_read16,
+	.read32 = cr50_spi_read32,
+	.write32 = cr50_spi_write32,
+	.attr_group = &cr50_attr_group,
+	.max_xfer_size = MAX_SPI_FRAMESIZE,
+};
+
+static int cr50_of_property_read_u32_optional(struct spi_device *dev,
+					      const char *name,
+					      u32 default_value,
+					      u32 *value)
+{
+	struct device_node *np = dev->dev.of_node;
+	int rc;
+
+	if (of_find_property(np, name, NULL)) {
+		rc = of_property_read_u32(np, name, value);
+		if (rc < 0) {
+			dev_err(&dev->dev,
+				"invalid '%s' property (%d)\n",
+				name, rc);
+			return rc;
+		}
+	} else {
+		*value = default_value;
+	}
+
+	dev_dbg(&dev->dev, "%s = %u\n", name, *value);
+	return 0;
+}
+
+static int cr50_spi_probe(struct spi_device *dev)
+{
+	char fw_ver[TPM_CR50_MAX_FW_VER_LEN+1];
+	struct cr50_spi_phy *phy;
+	int rc;
+	u32 value;
+
+	phy = devm_kzalloc(&dev->dev, sizeof(struct cr50_spi_phy),
+			   GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	phy->spi_device = dev;
+
+	/* Cr50 timing properties.
+	 */
+	rc = cr50_of_property_read_u32_optional(dev, "access-delay-msec",
+						CR50_DFT_ACCESS_DELAY_MSEC,
+						&value);
+	if (rc < 0)
+		return rc;
+	phy->access_delay_jiffies = msecs_to_jiffies(value);
+
+	rc = cr50_of_property_read_u32_optional(dev, "sleep-delay-msec",
+						CR50_DFT_SLEEP_DELAY_MSEC,
+						&value);
+	if (rc < 0)
+		return rc;
+	phy->sleep_delay_jiffies = msecs_to_jiffies(value);
+
+	rc = cr50_of_property_read_u32_optional(dev, "wake-start-delay-msec",
+						CR50_DFT_WAKE_START_DELAY_MSEC,
+						&value);
+	if (rc < 0)
+		return rc;
+	phy->wake_start_delay_msec = value;
+
+	mutex_init(&phy->time_track_mutex);
+	phy->wake_after_jiffies = jiffies;
+	phy->last_access_jiffies = jiffies;
+
+	rc = tpm_tis_core_init(&dev->dev, &phy->priv, -1, &cr50_spi_phy_ops,
+				 NULL);
+	if (rc < 0)
+		return rc;
+
+	cr50_get_fw_version(&phy->priv, fw_ver);
+	dev_info(&dev->dev, "Cr50 firmware version: %s\n", fw_ver);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(cr50_pm, tpm_pm_suspend, tpm_tis_resume);
+
+static int cr50_spi_remove(struct spi_device *dev)
+{
+	struct tpm_chip *chip = spi_get_drvdata(dev);
+
+	tpm_chip_unregister(chip);
+	tpm_tis_remove(chip);
+	return 0;
+}
+
+static const struct spi_device_id cr50_spi_id[] = {
+	{"cr50_spi", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(spi, cr50_spi_id);
+
+static const struct of_device_id of_cr50_spi_match[] = {
+	{ .compatible = "google,cr50_spi", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_cr50_spi_match);
+
+static struct spi_driver cr50_spi_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "cr50_spi",
+		.pm = &cr50_pm,
+		.of_match_table = of_match_ptr(of_cr50_spi_match),
+	},
+	.probe = cr50_spi_probe,
+	.remove = cr50_spi_remove,
+	.id_table = cr50_spi_id,
+};
+module_spi_driver(cr50_spi_driver);
+
+MODULE_DESCRIPTION("Cr50 TCG PTP FIFO SPI driver");
+MODULE_LICENSE("GPL v2");
-- 
2.6.6

[toc] | [prev] | [next] | [standalone]


#1443920 — Re: [PATCH 2/2] tpm: add driver for cr50 on SPI

FromJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date2016-07-15 05:40 +0200
SubjectRe: [PATCH 2/2] tpm: add driver for cr50 on SPI
Message-ID<rV28N-1GW-9@gated-at.bofh.it>
In reply to#1443887
On Thu, Jul 14, 2016 at 07:20:18PM -0700, Andrey Pronin wrote:

> +static int cr50_spi_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
> +{
> +	int rc;
> +
> +	rc = data->phy_ops->read_bytes(data, addr, sizeof(u16), (u8 *)result);
> +	if (!rc)
> +		*result = le16_to_cpu(*result);
> +	return rc;
> +}

I thought we had core support for this pattern?

Christophe ?

Please change this so this code isn't duplicated.

Jason

[toc] | [prev] | [next] | [standalone]


#1443926 — Re: [PATCH 2/2] tpm: add driver for cr50 on SPI

FromAndrey Pronin <apronin@chromium.org>
Date2016-07-15 05:50 +0200
SubjectRe: [PATCH 2/2] tpm: add driver for cr50 on SPI
Message-ID<rV2it-1Kg-5@gated-at.bofh.it>
In reply to#1443920
On Thu, Jul 14, 2016 at 09:32:36PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 14, 2016 at 07:20:18PM -0700, Andrey Pronin wrote:
> 
> > +static int cr50_spi_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
> > +{
> > +	int rc;
> > +
> > +	rc = data->phy_ops->read_bytes(data, addr, sizeof(u16), (u8 *)result);
> > +	if (!rc)
> > +		*result = le16_to_cpu(*result);
> > +	return rc;
> > +}
> 
> I thought we had core support for this pattern?
> 
> Christophe ?
> 
> Please change this so this code isn't duplicated.
> 
> Jason
>
Hmm, didn't see the support. Would be great if there is.
The pattern itself is copied from tpm_tis_spi as is.
read_bytes/write_bytes were de-dup'ed as they used a lot of common code
(even more for this driver than for tpm_tis_spi).
But as for _readNN/_writeNN, there're only three of these functions,
so it din't seem too bad.

[toc] | [prev] | [next] | [standalone]


#1446426 — Re: [PATCH 2/2] tpm: add driver for cr50 on SPI

FromJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date2016-07-19 15:00 +0200
SubjectRe: [PATCH 2/2] tpm: add driver for cr50 on SPI
Message-ID<rWCN1-47Q-41@gated-at.bofh.it>
In reply to#1443926
On Thu, Jul 14, 2016 at 08:44:44PM -0700, Andrey Pronin wrote:
> On Thu, Jul 14, 2016 at 09:32:36PM -0600, Jason Gunthorpe wrote:
> > On Thu, Jul 14, 2016 at 07:20:18PM -0700, Andrey Pronin wrote:
> > 
> > > +static int cr50_spi_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
> > > +{
> > > +	int rc;
> > > +
> > > +	rc = data->phy_ops->read_bytes(data, addr, sizeof(u16), (u8 *)result);
> > > +	if (!rc)
> > > +		*result = le16_to_cpu(*result);
> > > +	return rc;
> > > +}
> > 
> > I thought we had core support for this pattern?
> > 
> > Christophe ?
> > 
> > Please change this so this code isn't duplicated.
> > 
> > Jason
> >
> Hmm, didn't see the support. Would be great if there is.
> The pattern itself is copied from tpm_tis_spi as is.
> read_bytes/write_bytes were de-dup'ed as they used a lot of common code
> (even more for this driver than for tpm_tis_spi).
> But as for _readNN/_writeNN, there're only three of these functions,
> so it din't seem too bad.

If there isn't, please add a separate commit before this that adds an
inline function to tpm_tis_core.h.

/Jarkko

[toc] | [prev] | [next] | [standalone]


#1446834 — Re: [PATCH 2/2] tpm: add driver for cr50 on SPI

FromAndrey Pronin <apronin@chromium.org>
Date2016-07-20 02:30 +0200
SubjectRe: [PATCH 2/2] tpm: add driver for cr50 on SPI
Message-ID<rWNyF-2yx-11@gated-at.bofh.it>
In reply to#1446426
On Tue, Jul 19, 2016 at 03:55:27PM +0300, Jarkko Sakkinen wrote:
> On Thu, Jul 14, 2016 at 08:44:44PM -0700, Andrey Pronin wrote:
> > On Thu, Jul 14, 2016 at 09:32:36PM -0600, Jason Gunthorpe wrote:
> > > On Thu, Jul 14, 2016 at 07:20:18PM -0700, Andrey Pronin wrote:
> > > 
> > > > +static int cr50_spi_read16(struct tpm_tis_data *data, u32 addr, u16 *result)
> > > > +{
> > > > +	int rc;
> > > > +
> > > > +	rc = data->phy_ops->read_bytes(data, addr, sizeof(u16), (u8 *)result);
> > > > +	if (!rc)
> > > > +		*result = le16_to_cpu(*result);
> > > > +	return rc;
> > > > +}
> > > 
> > > I thought we had core support for this pattern?
> > > 
> > > Christophe ?
> > > 
> > > Please change this so this code isn't duplicated.
> > > 
> > > Jason
> > >
> > Hmm, didn't see the support. Would be great if there is.
> > The pattern itself is copied from tpm_tis_spi as is.
> > read_bytes/write_bytes were de-dup'ed as they used a lot of common code
> > (even more for this driver than for tpm_tis_spi).
> > But as for _readNN/_writeNN, there're only three of these functions,
> > so it din't seem too bad.
> 
> If there isn't, please add a separate commit before this that adds an
> inline function to tpm_tis_core.h.
>
tpm_tis_core.h currently has access functions defined like:
static inline int tpm_tis_read16(struct tpm_tis_data *data, u32 addr,
                                 u16 *result)
{
        return data->phy_ops->read16(data, addr, result);
}

And it's up to the drivers implementing phy_ops to do (or not)
byte-swapping as necessary before/after IO ops. For example,
tpm_tis.c in its phy_ops->read16 simply does ioread16(), while
tpm_tis_spi.c (and cr50_spi.c) does phy_ops->read_bytes()
followed by le16_to_cpu().

Still, I can create add'l inline functions:
static inline int tpm_tis_read_le16(struct tpm_tis_data *data,
                                    u32 addr, u16 *result)
{
	int rc;

	rc = data->phy_ops->read_bytes(data, addr,
				       sizeof(u16), (u8 *)result);
	if (!rc)
		*result = le16_to_cpu(*result);
	return rc;
}
and re-use them both in cr50_spi.c and tpm_tis_spi.c

The only two things that bother me with such approach are
(1) whatever names I pick for the new set of functions, they
    will be similar to and thus might be confused with the
    original tpm_tis_read/writeXX;
(2) these functions are phy-specific, so possibly it's better
    to create tpm_tis_spi.h and put them there with proper
    name prefixes. And then use in tpm_tis_spi and cr50_spi.

Any preferences on what I should better do?

Andrey

[toc] | [prev] | [next] | [standalone]


#1447369 — Re: [PATCH 2/2] tpm: add driver for cr50 on SPI

FromJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date2016-07-20 19:10 +0200
SubjectRe: [PATCH 2/2] tpm: add driver for cr50 on SPI
Message-ID<rX3aq-4fX-33@gated-at.bofh.it>
In reply to#1446834
On Tue, Jul 19, 2016 at 05:24:11PM -0700, Andrey Pronin wrote:

> The only two things that bother me with such approach are
> (1) whatever names I pick for the new set of functions, they
>     will be similar to and thus might be confused with the
>     original tpm_tis_read/writeXX;

tpm_tis_helper_read16 ?

> (2) these functions are phy-specific, so possibly it's better
>     to create tpm_tis_spi.h and put them there with proper
>     name prefixes. And then use in tpm_tis_spi and cr50_spi.

No, they are generic to any tis phy that implements read only through
read_bytes.

(Honestly, I'm not sure we made the best choice here having phy
 functions for all the versions, we are not that performance
 sensitive, just getting rid of everything but read_bytes from the
 phy_ops would probably also be a reasonable thing to do.)
 
Jason

[toc] | [prev] | [next] | [standalone]


#1448038 — Re: [PATCH 2/2] tpm: add driver for cr50 on SPI

FromAndrey Pronin <apronin@chromium.org>
Date2016-07-21 20:20 +0200
SubjectRe: [PATCH 2/2] tpm: add driver for cr50 on SPI
Message-ID<rXqJI-2G4-3@gated-at.bofh.it>
In reply to#1447369
On Wed, Jul 20, 2016 at 11:03:36AM -0600, Jason Gunthorpe wrote:
> On Tue, Jul 19, 2016 at 05:24:11PM -0700, Andrey Pronin wrote:
> 
> > The only two things that bother me with such approach are
> > (1) whatever names I pick for the new set of functions, they
> >     will be similar to and thus might be confused with the
> >     original tpm_tis_read/writeXX;
> 
> tpm_tis_helper_read16 ?
> 
> > (2) these functions are phy-specific, so possibly it's better
> >     to create tpm_tis_spi.h and put them there with proper
> >     name prefixes. And then use in tpm_tis_spi and cr50_spi.
> 
> No, they are generic to any tis phy that implements read only through
> read_bytes.
> 
> (Honestly, I'm not sure we made the best choice here having phy
>  functions for all the versions, we are not that performance
>  sensitive, just getting rid of everything but read_bytes from the
>  phy_ops would probably also be a reasonable thing to do.)
> 

One thing we can do is re-implement functions tpm_tis_read/writeXX
to use phy-specific implementations of read16, read32, write32 if they
are provided. But if those function pointers are left NULL in phy_ops,
fallback to using read/write_bytes and byte-swapping.

I.e., instead of:

static inline int tpm_tis_read16(struct tpm_tis_data *data, u32 addr,
				 u16 *result)
{
	return data->phy_ops->read16(data, addr, result);
}

do the following:

static inline int tpm_tis_read16(struct tpm_tis_data *data, u32 addr,
				 u16 *result)
{
        int rc;

	if (data->phy_ops->read16)
		return data->phy_ops->read16(data, addr, result);

	rc = data->phy_ops->read_bytes(data, addr,
				       sizeof(u16), (u8 *)result);
	if (!rc)
		*result = le16_to_cpu(*result);
	return rc;
}

If you like the idea, I'll submit it as a separate patch.

Andrey

[toc] | [prev] | [next] | [standalone]


#1448145 — Re: [PATCH 2/2] tpm: add driver for cr50 on SPI

FromJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date2016-07-21 23:10 +0200
SubjectRe: [PATCH 2/2] tpm: add driver for cr50 on SPI
Message-ID<rXtoe-4vY-27@gated-at.bofh.it>
In reply to#1448038
On Thu, Jul 21, 2016 at 11:10:47AM -0700, Andrey Pronin wrote:
> On Wed, Jul 20, 2016 at 11:03:36AM -0600, Jason Gunthorpe wrote:
> > On Tue, Jul 19, 2016 at 05:24:11PM -0700, Andrey Pronin wrote:
> > 
> > > The only two things that bother me with such approach are
> > > (1) whatever names I pick for the new set of functions, they
> > >     will be similar to and thus might be confused with the
> > >     original tpm_tis_read/writeXX;
> > 
> > tpm_tis_helper_read16 ?
> > 
> > > (2) these functions are phy-specific, so possibly it's better
> > >     to create tpm_tis_spi.h and put them there with proper
> > >     name prefixes. And then use in tpm_tis_spi and cr50_spi.
> > 
> > No, they are generic to any tis phy that implements read only through
> > read_bytes.
> > 
> > (Honestly, I'm not sure we made the best choice here having phy
> >  functions for all the versions, we are not that performance
> >  sensitive, just getting rid of everything but read_bytes from the
> >  phy_ops would probably also be a reasonable thing to do.)
> > 
> 
> One thing we can do is re-implement functions tpm_tis_read/writeXX
> to use phy-specific implementations of read16, read32, write32 if they
> are provided. But if those function pointers are left NULL in phy_ops,
> fallback to using read/write_bytes and byte-swapping.

I was thinking of just getting rid of phy_ops->read16 entirely and
only use read_bytes at the ops layer.

Jason

[toc] | [prev] | [next] | [standalone]


#1443889

FromPeter Huewe <peterhuewe@gmx.de>
Date2016-07-15 04:30 +0200
Message-ID<rV133-14j-13@gated-at.bofh.it>
In reply to#1443884

Am 14. Juli 2016 19:20:16 GMT-07:00, schrieb Andrey Pronin <apronin@chromium.org>:
>This patchset adds a TCG TPM2.0 PTP FIFO compliant interface for
>Cr50 chip on SPI.
>
>Depends on the following patches by Andrey Pronin
><apronin@chromium.org>
>that add new members to phy_ops in tpm_tis_core:
> - tpm: support driver-specific sysfs attrs in tpm_tis_core
> - tpm_tis_core: add optional max xfer size check
>
>Andrey Pronin (2):
>  tpm: devicetree: document properties for cr50
>  tpm: add driver for cr50 on SPI
>
> .../devicetree/bindings/security/tpm/cr50_spi.txt  |  30 ++
> drivers/char/tpm/Kconfig                           |   9 +
> drivers/char/tpm/Makefile                          |   1 +
>drivers/char/tpm/cr50_spi.c                        | 409
>+++++++++++++++++++++
> 4 files changed, 449 insertions(+)
>create mode 100644
>Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
> create mode 100644 drivers/char/tpm/cr50_spi.c


Hi, 
can you explain a bit more about this device? And why it needs a special driver and cannot be handled by tpm_tis_spi if its tcg compliant?

Peter
-- 
Sent from my mobile

[toc] | [prev] | [next] | [standalone]


#1443901

FromAndrey Pronin <apronin@chromium.org>
Date2016-07-15 05:00 +0200
Message-ID<rV1wf-1ey-3@gated-at.bofh.it>
In reply to#1443889
On Thu, Jul 14, 2016 at 07:28:55PM -0700, Peter Huewe wrote:
> Am 14. Juli 2016 19:20:16 GMT-07:00, schrieb Andrey Pronin <apronin@chromium.org>:
> >This patchset adds a TCG TPM2.0 PTP FIFO compliant interface for
> >Cr50 chip on SPI.
> >
> >Depends on the following patches by Andrey Pronin
> ><apronin@chromium.org>
> >that add new members to phy_ops in tpm_tis_core:
> > - tpm: support driver-specific sysfs attrs in tpm_tis_core
> > - tpm_tis_core: add optional max xfer size check
> >
> >Andrey Pronin (2):
> >  tpm: devicetree: document properties for cr50
> >  tpm: add driver for cr50 on SPI
> >
> > .../devicetree/bindings/security/tpm/cr50_spi.txt  |  30 ++
> > drivers/char/tpm/Kconfig                           |   9 +
> > drivers/char/tpm/Makefile                          |   1 +
> >drivers/char/tpm/cr50_spi.c                        | 409
> >+++++++++++++++++++++
> > 4 files changed, 449 insertions(+)
> >create mode 100644
> >Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
> > create mode 100644 drivers/char/tpm/cr50_spi.c
> 
> 
> Hi, 
> can you explain a bit more about this device? And why it needs a special driver and cannot be handled by tpm_tis_spi if its tcg compliant?
> 
> Peter
> -- 
> Sent from my mobile

Hi Peter,

Yes, it has a TCG-compliant interface, however, there are several things
specific to this device:
 - need to ensure a certain delay between spi transactions, or else
   the chip can miss several first bytes.
 - if there is no spi activity for this chip, it may go to sleep, and
   needs to be waken up before sending further commands.
 - it has some vendor-specific registers accessible from spi bus.

All that combined to me seemed to be enough justification to add a
device-specific driver rather than adding vendor-specific code to
tpm_tis_spi in multiple places.

Plus, where it seemed appropriate, I added additional hooks to
tpm_tis_core (device-specific sysfs attributes, capping burstcnt
in case of chip error) to support this chip specifics.

Best regards,
Andrey

[toc] | [prev] | [next] | [standalone]


#1443917

FromJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date2016-07-15 05:30 +0200
Message-ID<rV1Z7-1DV-11@gated-at.bofh.it>
In reply to#1443901
On Thu, Jul 14, 2016 at 07:50:26PM -0700, Andrey Pronin wrote:
> Yes, it has a TCG-compliant interface, however, there are several things
> specific to this device:
>  - need to ensure a certain delay between spi transactions, or else
>    the chip can miss several first bytes.
>  - if there is no spi activity for this chip, it may go to sleep, and
>    needs to be waken up before sending further commands.
>  - it has some vendor-specific registers accessible from spi bus.

This all needs to be commented clearly in the driver..

The chip sounds pretty broken...

Jason

[toc] | [prev] | [next] | [standalone]


#1444464

FromAndrey Pronin <apronin@chromium.org>
Date2016-07-15 19:20 +0200
Message-ID<rVeWl-1cu-11@gated-at.bofh.it>
In reply to#1443917
On Thu, Jul 14, 2016 at 09:28:14PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 14, 2016 at 07:50:26PM -0700, Andrey Pronin wrote:
> > Yes, it has a TCG-compliant interface, however, there are several things
> > specific to this device:
> >  - need to ensure a certain delay between spi transactions, or else
> >    the chip can miss several first bytes.
> >  - if there is no spi activity for this chip, it may go to sleep, and
> >    needs to be waken up before sending further commands.
> >  - it has some vendor-specific registers accessible from spi bus.
> 
> This all needs to be commented clearly in the driver..
> 

Will provide more details in the patch description. The driver code
has comments about those cases in the code: see cr50_ensure_access_delay
and cr50_needs_waking, for example.

[toc] | [prev] | [next] | [standalone]


#1446423

FromJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date2016-07-19 15:00 +0200
Message-ID<rWCN1-47Q-29@gated-at.bofh.it>
In reply to#1443884
On Thu, Jul 14, 2016 at 07:20:16PM -0700, Andrey Pronin wrote:
> This patchset adds a TCG TPM2.0 PTP FIFO compliant interface for
> Cr50 chip on SPI.
> 
> Depends on the following patches by Andrey Pronin <apronin@chromium.org>
> that add new members to phy_ops in tpm_tis_core:
>  - tpm: support driver-specific sysfs attrs in tpm_tis_core
>  - tpm_tis_core: add optional max xfer size check
> 
> Andrey Pronin (2):
>   tpm: devicetree: document properties for cr50
>   tpm: add driver for cr50 on SPI

I wonder who coul peer test this? I do not possess this hardware.

/Jarkko

[toc] | [prev] | [next] | [standalone]


#1446925 — [PATCH v2 1/2] tpm: devicetree: document properties for cr50

FromAndrey Pronin <apronin@chromium.org>
Date2016-07-20 05:50 +0200
Subject[PATCH v2 1/2] tpm: devicetree: document properties for cr50
Message-ID<rWQGd-4wj-3@gated-at.bofh.it>
In reply to#1443884
Add TPM2.0 PTP FIFO compatible SPI interface for chips with Cr50
firmware. Several timing-related properties that may differ from
one firmware version to another are added to devicetree.
Document these properties.

Signed-off-by: Andrey Pronin <apronin@chromium.org>
---
 .../devicetree/bindings/security/tpm/cr50_spi.txt  | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/cr50_spi.txt

diff --git a/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt b/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
new file mode 100644
index 0000000..f212b6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/cr50_spi.txt
@@ -0,0 +1,32 @@
+* H1 Secure Microcontroller with Cr50 Firmware on SPI Bus.
+
+H1 Secure Microcontroller running Cr50 firmware provides several
+functions, including TPM-like functionality. It communicates over
+SPI using the FIFO protocol described in the PTP Spec, section 6.
+
+Required properties:
+- compatible: Should be "google,cr50".
+- spi-max-frequency: Maximum SPI frequency.
+
+Optional properties:
+- access-delay-ms: Required delay between subsequent transactions on SPI.
+- sleep-delay-ms: Time after the last SPI activity, after which the chip
+  may go to sleep.
+- wake-start-delay-ms: Time after initiating wake up before the chip is
+  ready to accept commands over SPI.
+
+Example:
+
+&spi0 {
+        status = "okay";
+
+        cr50@0 {
+                compatible = "google,cr50";
+                reg = <0>;
+                spi-max-frequency = <800000>;
+
+                access-delay-ms = <2>;
+                sleep-delay-ms = <1000>;
+                wake-start-delay-ms = <60>;
+        };
+};
-- 
2.6.6

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web