Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1443884 > unrolled thread
| Started by | Andrey Pronin <apronin@chromium.org> |
|---|---|
| First post | 2016-07-15 04:30 +0200 |
| Last post | 2016-07-25 19:10 +0200 |
| Articles | 20 on this page of 27 — 7 participants |
Back to article view | Back to linux.kernel
[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 →
| From | Andrey Pronin <apronin@chromium.org> |
|---|---|
| Date | 2016-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]
| From | Andrey Pronin <apronin@chromium.org> |
|---|---|
| Date | 2016-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]
| From | Guenter Roeck <groeck@google.com> |
|---|---|
| Date | 2016-07-15 06:10 +0200 |
| Subject | Re: [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]
| From | Andrey Pronin <apronin@chromium.org> |
|---|---|
| Date | 2016-07-15 19:40 +0200 |
| Subject | Re: [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]
| From | Guenter Roeck <groeck@google.com> |
|---|---|
| Date | 2016-07-15 20:30 +0200 |
| Subject | Re: [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]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2016-07-17 15:30 +0200 |
| Subject | Re: [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]
| From | Andrey Pronin <apronin@chromium.org> |
|---|---|
| Date | 2016-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]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2016-07-15 05:40 +0200 |
| Subject | Re: [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]
| From | Andrey Pronin <apronin@chromium.org> |
|---|---|
| Date | 2016-07-15 05:50 +0200 |
| Subject | Re: [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]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2016-07-19 15:00 +0200 |
| Subject | Re: [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]
| From | Andrey Pronin <apronin@chromium.org> |
|---|---|
| Date | 2016-07-20 02:30 +0200 |
| Subject | Re: [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]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2016-07-20 19:10 +0200 |
| Subject | Re: [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]
| From | Andrey Pronin <apronin@chromium.org> |
|---|---|
| Date | 2016-07-21 20:20 +0200 |
| Subject | Re: [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]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2016-07-21 23:10 +0200 |
| Subject | Re: [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]
| From | Peter Huewe <peterhuewe@gmx.de> |
|---|---|
| Date | 2016-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]
| From | Andrey Pronin <apronin@chromium.org> |
|---|---|
| Date | 2016-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]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2016-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]
| From | Andrey Pronin <apronin@chromium.org> |
|---|---|
| Date | 2016-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]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2016-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]
| From | Andrey Pronin <apronin@chromium.org> |
|---|---|
| Date | 2016-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