[U-Boot] [PATCH v3 17/25] tpm: add support for TPMv2.x SPI modules

[Simon Glass <sjg@chromium.org>]

Hi Miquel,

On 2 May 2018 at 02:59, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Add the tpm2_tis_spi driver that should support any TPMv2 compliant
> (SPI) module.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/tpm/Kconfig        |  10 +
>  drivers/tpm/Makefile       |   2 +
>  drivers/tpm/tpm2_tis_spi.c | 678 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 690 insertions(+)
>  create mode 100644 drivers/tpm/tpm2_tis_spi.c
>
> diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
> index 01967ffd35..6661dcc1e3 100644
> --- a/drivers/tpm/Kconfig
> +++ b/drivers/tpm/Kconfig
> @@ -141,6 +141,16 @@ config TPM_V2
>
>  if TPM_V2 && !TPM_V1
>
> +config TPM2_TIS_SPI
> +       bool "Enable support for TPMv2.x SPI chips"
> +       depends on TPM_V2 && DM_SPI
> +       select TPM_DRIVER_SELECTED
> +       help
> +         This driver supports TPMv2.x devices connected on the SPI bus.
> +         The usual TPM operations and the 'tpm' command can be used to talk
> +         to the device using the standard TPM Interface Specification (TIS)
> +         protocol.
> +
>  endif # TPM_V2
>
>  endmenu
> diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
> index c42a93f267..2c88b64659 100644
> --- a/drivers/tpm/Makefile
> +++ b/drivers/tpm/Makefile
> @@ -11,3 +11,5 @@ obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
>  obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o
>  obj-$(CONFIG_TPM_ST33ZP24_I2C) += tpm_tis_st33zp24_i2c.o
>  obj-$(CONFIG_TPM_ST33ZP24_SPI) += tpm_tis_st33zp24_spi.o
> +
> +obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_spi.o
> diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c
> new file mode 100644
> index 0000000000..cfef5b8c24
> --- /dev/null
> +++ b/drivers/tpm/tpm2_tis_spi.c
> @@ -0,0 +1,678 @@
> +/*
> + * Author:
> + * Miquel Raynal <miquel.raynal@bootlin.com>
> + *
> + * Description:
> + * SPI-level driver for TCG/TIS TPM (trusted platform module).
> + * Specifications at www.trustedcomputinggroup.org
> + *
> + * This device driver implements the TPM interface as defined in
> + * the TCG SPI protocol stack version 2.0.
> + *
> + * It is based on the U-Boot driver tpm_tis_infineon_i2c.c.
> + *
> + * SPDX-License-Identifier:    GPL-2.0
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <log.h>
> +#include <spi.h>
> +#include <tpm-v2.h>
> +#include <linux/errno.h>
> +#include <linux/compiler.h>
> +#include <linux/types.h>
> +#include <linux/unaligned/be_byteshift.h>
> +
> +#include "tpm_tis.h"
> +#include "tpm_internal.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define TPM_ACCESS(l)                  (0x0000 | ((l) << 12))
> +#define TPM_INT_ENABLE(l)               (0x0008 | ((l) << 12))
> +#define TPM_STS(l)                     (0x0018 | ((l) << 12))
> +#define TPM_DATA_FIFO(l)               (0x0024 | ((l) << 12))
> +#define TPM_DID_VID(l)                 (0x0F00 | ((l) << 12))
> +#define TPM_RID(l)                     (0x0F04 | ((l) << 12))
> +
> +#define MAX_SPI_FRAMESIZE 64
> +
> +/* Number of wait states to wait for */
> +#define TPM_WAIT_STATES 100
> +
> +/*
> + * tpm_tis_spi_read() - read from TPM register
> + * @addr: register address to read from
> + * @buffer: provided by caller
> + * @len: number of bytes to read
> + *
> + * Read len bytes from TPM register and put them into
> + * buffer (little-endian format, i.e. first byte is put into buffer[0]).
> + *
> + * NOTE: TPM is big-endian for multi-byte values. Multi-byte
> + * values have to be swapped.
> + *
> + * Return -EIO on error, 0 on success.
> + */
> +static int tpm_tis_spi_xfer(struct udevice *dev, u32 addr, const u8 *out,
> +                           u8 *in, u16 len)
> +{
> +       struct spi_slave *slave = dev_get_parent_priv(dev);
> +       int transfer_len, ret;
> +       u8 tx_buf[MAX_SPI_FRAMESIZE];
> +       u8 rx_buf[MAX_SPI_FRAMESIZE];
> +
> +       if (in && out) {
> +               log(LOGC_NONE, LOGL_ERR, "%s: can't do full duplex\n",
> +                   __func__);
> +               return -EINVAL;
> +       }
> +
> +       ret = spi_claim_bus(slave);
> +       if (ret < 0) {
> +               log(LOGC_NONE, LOGL_ERR, "%s: could not claim bus\n", __func__);
> +               return ret;
> +       }
> +
> +       while (len) {
> +               /* Request */
> +               transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
> +               tx_buf[0] = (in ? BIT(7) : 0) | (transfer_len - 1);
> +               tx_buf[1] = 0xD4;
> +               tx_buf[2] = addr >> 8;
> +               tx_buf[3] = addr;
> +
> +               ret = spi_xfer(slave, 4 * 8, tx_buf, rx_buf, SPI_XFER_BEGIN);
> +               if (ret < 0) {
> +                       log(LOGC_NONE, LOGL_ERR,
> +                           "%s: spi request transfer failed (err: %d)\n",
> +                           __func__, ret);
> +                       goto release_bus;
> +               }
> +
> +               /* Wait state */
> +               if (!(rx_buf[3] & 0x1)) {
> +                       int i;
> +
> +                       rx_buf[0] = 0;

I don't think you need this?

> +                       for (i = 0; i < TPM_WAIT_STATES; i++) {
> +                               ret = spi_xfer(slave, 1 * 8, NULL, rx_buf, 0);
> +                               if (ret < 0) {
> +                                       log(LOGC_NONE, LOGL_ERR,
> +                                           "%s: wait state failed: %d\n",
> +                                           __func__, ret);
> +                                       goto release_bus;
> +                               }
> +
> +                               if (rx_buf[0] & 0x1)
> +                                       break;
> +                       }
> +
> +                       if (i == TPM_WAIT_STATES) {
> +                               log(LOGC_NONE, LOGL_ERR,
> +                                   "%s: timeout on wait state\n", __func__);
> +                               ret = -ETIMEDOUT;
> +                               goto release_bus;
> +                       }
> +               }
> +
> +               /* Read/Write */
> +               if (out) {
> +                       memcpy(tx_buf, out, transfer_len);
> +                       out += transfer_len;
> +               }
> +
> +               ret = spi_xfer(slave, transfer_len * 8,
> +                              out ? tx_buf : NULL,
> +                              in ? rx_buf : NULL,
> +                              SPI_XFER_END);
> +               if (ret < 0) {
> +                       log(LOGC_NONE, LOGL_ERR,
> +                           "%s: spi read transfer failed (err: %d)\n",
> +                           __func__, ret);
> +                       goto release_bus;
> +               }
> +
> +               if (in) {
> +                       memcpy(in, rx_buf, transfer_len);
> +                       in += transfer_len;
> +               }
> +
> +               len -= transfer_len;
> +       }
> +
> +release_bus:
> +       /* If an error occurred, release the chip by deasserting the CS */
> +       if (ret < 0)
> +               spi_xfer(slave, 0, NULL, NULL, SPI_XFER_END);
> +
> +       spi_release_bus(slave);
> +
> +       return ret;
> +}
> +
> +static int tpm_tis_spi_read(struct udevice *dev, u16 addr, u8 *in, u16 len)
> +{
> +       return tpm_tis_spi_xfer(dev, addr, NULL, in, len);
> +}
> +
> +static __maybe_unused int tpm_tis_spi_read16(struct udevice *dev, u32 addr,
> +                                            u16 *result)

Why is __maybe_unused needed in this file?

> +{
> +       __le16 result_le;
> +       int ret;
> +
> +       ret = tpm_tis_spi_read(dev, addr, (u8 *)&result_le, sizeof(u16));
> +       if (!ret)
> +               *result = le16_to_cpu(result_le);
> +
> +       return ret;
> +}
> +
> +static __maybe_unused int tpm_tis_spi_read32(struct udevice *dev, u32 addr,
> +                                            u32 *result)
> +{
> +       __le32 result_le;
> +       int ret;
> +
> +       ret = tpm_tis_spi_read(dev, addr, (u8 *)&result_le, sizeof(u32));
> +       if (!ret)
> +               *result = le32_to_cpu(result_le);

Does this assume host endianness? Will it work on a big-endian machine?

[..]

> +static int tpm_tis_spi_probe(struct udevice *dev)
> +{
> +       struct tpm_chip *chip = dev_get_priv(dev);
> +       int ret;
> +
> +       /* Ensure a minimum amount of time elapsed since reset */
> +       mdelay(30);

This seems bad. Why is this needed? Where does the number come from?
Can we instead check the time since reset somehow?

Regards,
Simon