Hi Jean-Luc,
On 1 April 2014 05:48, Jean-Luc BLANC <stmicroelectronics....@gmail.com>wrote:
> This driver add support to STMicroelectronics ST33ZP24 SPI TPM.
>
> Signed-off-by: Jean-Luc BLANC <jean-luc.bl...@st.com>
> ---
> README | 12 +
> drivers/tpm/Makefile | 1 +
> drivers/tpm/tpm_spi_stm_st33.c | 671
> ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 684 insertions(+)
> create mode 100644 drivers/tpm/tpm_spi_stm_st33.c
>
> diff --git a/README b/README
> index aea82be..e04866d 100644
> --- a/README
> +++ b/README
> @@ -1322,6 +1322,18 @@ The following options need to be configured:
> Define this to enable authorized functions in the TPM
> library.
> Requires CONFIG_TPM and CONFIG_SHA1.
>
> + CONFIG_TPM_ST_SPI
> + Support SPI STMicroelectronics TPM. Require SPI support
> +
> + TPM0_SPI_MAX_SPEED
> + Define SPI frequency for TPM, 10000000 Hz max
> +
> + TPM0_SPI_BUS_NUM
> + Define SPI Bus ID connected to TPM
> +
> + TPM0_SPI_CS
> + Define SPI Chip Select ID connected to TPM
> +
> - USB Support:
> At the moment only the UHCI host controller is
> supported (PIP405, MIP405, MPC5200); define
> diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
> index 150570e..1ee707e 100644
> --- a/drivers/tpm/Makefile
> +++ b/drivers/tpm/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_TPM_TIS_I2C) += tpm.o
> obj-$(CONFIG_TPM_TIS_I2C) += tpm_tis_i2c.o
> obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
> obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o
> +obj-$(CONFIG_TPM_ST_SPI) += tpm_spi_stm_st33.o
> diff --git a/drivers/tpm/tpm_spi_stm_st33.c
> b/drivers/tpm/tpm_spi_stm_st33.c
> new file mode 100644
> index 0000000..78a4e54
> --- /dev/null
> +++ b/drivers/tpm/tpm_spi_stm_st33.c
> @@ -0,0 +1,671 @@
> +/*
> + * STMicroelectronics TPM SPI UBOOT Linux driver for TPM ST33ZP24
> + * Copyright (C) 2014 STMicroelectronics
> +
>
Missing *
> + *
> + * Description: Device driver for ST33ZP24 SPI TPM TCG.
> + *
> + * This device driver implements the TPM interface as defined in
> + * the TCG TPM Interface Spec version 1.21, revision 1.0 and the
> + * STMicroelectronics SPI Protocol Stack Specification version 1.2.0.
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + *
> + * @Author: Jean-Luc BLANC <jean-luc.bl...@st.com>
> + *
> + * @File: tpm_spi_stm_st33.c
> + */
> +
> +#include <common.h>
> +#include <spi.h>
> +#include <linux/types.h>
> +#include <tpm.h>
> +#include <errno.h>
> +#include <asm/unaligned.h>
> +
> +#define TPM_ACCESS (0x0)
> +#define TPM_STS (0x18)
> +#define TPM_HASH_END (0x20)
> +#define TPM_DATA_FIFO (0x24)
> +#define TPM_HASH_DATA (0x24)
> +#define TPM_HASH_START (0x28)
> +#define TPM_INTF_CAPABILITY (0x14)
> +#define TPM_INT_STATUS (0x10)
> +#define TPM_INT_ENABLE (0x08)
>
We shouldn't need brackets around these constants here, and below.
> +
> +#define TPM_DUMMY_BYTE 0x00
> +#define TPM_WRITE_DIRECTION 0x80
> +#define TPM_HEADER_SIZE 10
> +#define TPM_BUFSIZE 2048
> +
> +#define LOCALITY0 0
> +#define LOCALITY1 1
> +#define LOCALITY2 2
> +#define LOCALITY3 3
> +#define LOCALITY4 4
> +
> +/* Index of Count field in TPM response buffer */
> +#define TPM_RSP_SIZE_BYTE 2
> +
> +#define MAX_NUMBER_TPM_ONBOARD 2
> +
> +#define SPI_WRITE_HEADER_SIZE 4
> +
> +struct tpm_chip {
>
Extra space.
> + int latency;
> + int is_open;
> + bool bchipf;
> + int locality;
> + u8 buf[TPM_BUFSIZE];
> + unsigned long timeout_a, timeout_b, timeout_c, timeout_d; /* msec
> */
> + unsigned long duration; /* msec */
> + struct spi_slave *tpm_dev_spi_info;
>
Can we have comments on these fields? You can do something like this above
the struct:
/**
* @latency: Some description
...
+};
> +
> +struct tpm_chip tpm_st33_spi_board_info[1];
> +
> +struct tpm_chip *active_tpm;
> +
> +/* Error value returned on various TPM driver errors. */
> +#define TPM_DRIVER_ERR (1)
>
I think it would be better to use -EIO or something like that from errno.h,
rather than creating your own error.
> +
> +/* Maximum command duration */
> +#define TPM_MAX_COMMAND_DURATION 120000
>
Perhaps an _MS suffix to indicate the units are ms?
> +
> +#define min_t(type, x, y) ({ \
> + type __min1 = (x); \
> + type __min2 = (y); \
> + __min1 < __min2 ? __min1 : __min2; })
>
Can we use min()?
> +
> +enum stm33zp24_access {
> + TPM_ACCESS_VALID = 0x80,
> + TPM_ACCESS_ACTIVE_LOCALITY = 0x20,
> + TPM_ACCESS_REQUEST_PENDING = 0x04,
> + TPM_ACCESS_REQUEST_USE = 0x02,
> +};
> +
> +enum stm33zp24_status {
> + TPM_STS_VALID = 0x80,
> + TPM_STS_COMMAND_READY = 0x40,
> + TPM_STS_GO = 0x20,
> + TPM_STS_DATA_AVAIL = 0x10,
> + TPM_STS_DATA_EXPECT = 0x08,
> +};
> +
> +enum stm33zp24_int_flags {
> + TPM_GLOBAL_INT_ENABLE = 0x80,
> + TPM_INTF_CMD_READY_INT = 0x80,
> + TPM_INTF_FIFO_AVALAIBLE_INT = 0x40,
> + TPM_INTF_WAKE_UP_READY_INT = 0x20,
> + TPM_INTF_LOC4SOFTRELEASE_INT = 0x08,
> + TPM_INTF_LOCALITY_CHANGE_INT = 0x04,
> + TPM_INTF_STS_VALID_INT = 0x02,
> + TPM_INTF_DATA_AVAIL_INT = 0x01,
> +};
> +
> +enum tis_defaults {
> + TIS_SHORT_TIMEOUT = 750, /* ms */
> + TIS_LONG_TIMEOUT = 2000, /* 2 sec */
>
Again _MS suffix might be nice.
> +};
> +
> +
> +/*
> + * spi_write8_reg
> + * Send byte to the TIS register according to the ST33ZP24 SPI protocol.
>
I think it is supposed to be:
/**
* spi_write8_reg() - Send byte to the TIS register according to the
ST33ZP24 SPI protocol.
*
* Some extra description if you want it.
*
* @tpm: description
* @tpm_register: etc...
...
*/
Granted, I myself used your style for quite a while, but I believe the new
standard for U-Boot is Docbook rather than doxygen. Marek is the expert on
this.
> + * @param: tpm, the chip description
> + * @param: tpm_register, the tpm tis register where the data should be
> written
> + * @param: tpm_data, the tpm_data to write inside the tpm_register
> + * @param: tpm_size, The length of the data
> + * @return: should be zero if success else a negative error code.
> + */
> +static int spi_write8_reg(struct tpm_chip *tpm, u8 tpm_register,
> + const u8 *tpm_data, u16 tpm_size)
> +{
> + u8 data = 0;
> + int total_length = 0, nbr_dummy_bytes = 0;
> + int value = 0;
> + u8 tx_buffer[TPM_BUFSIZE + SPI_WRITE_HEADER_SIZE];
> +
> + data = TPM_WRITE_DIRECTION | tpm->locality;
> + memcpy(tx_buffer + total_length, &data, sizeof(data));
> + total_length++;
> + data = tpm_register;
> + memcpy(tx_buffer + total_length, &data, sizeof(data));
>
sizeof(data) is 1, so why not just:
tx_buffer[total_length++] = tpm_register;
in these cases?
> + total_length++;
> +
> + if (tpm_size > 0 && ((tpm_register == TPM_DATA_FIFO)
> + || (tpm_register == TPM_HASH_DATA))) {
> + tx_buffer[total_length++] = tpm_size >> 8;
> + tx_buffer[total_length++] = tpm_size;
> + }
> + memcpy(tx_buffer + total_length, tpm_data, tpm_size);
> + total_length += tpm_size;
> + nbr_dummy_bytes = tpm->latency + 1;
> + memset(tx_buffer + total_length, TPM_DUMMY_BYTE, nbr_dummy_bytes);
> +
> + spi_claim_bus(tpm->tpm_dev_spi_info);
>
Error check
> + value = spi_xfer(tpm->tpm_dev_spi_info,
> + (total_length + nbr_dummy_bytes)*8, tx_buffer,
> + tx_buffer, SPI_XFER_BEGIN | SPI_XFER_END);
> + spi_release_bus(tpm->tpm_dev_spi_info);
> +
> + return value;
>
I think ret would be a better name than value, but it's up to you.
> +} /* spi_write8_reg() */
>
Don't need this comment.
> +
> +/*
> + * spi_read8_reg
> + * Recv byte from the TIS register according to the ST33ZP24 SPI protocol.
> + * @param: tpm, the chip description
> + * @param: tpm_loc, the locality to read register from
> + * @param: tpm_register, the tpm tis register where the data should be
> read
> + * @param: tpm_data, the TPM response
> + * @param: tpm_size, tpm TPM response size to read.
> + * @return: should be zero if success else a negative error code.
> + */
> +static u8 spi_read8_reg(struct tpm_chip *tpm, u8 tpm_loc, u8 tpm_register,
> + u8 *tpm_data, u16 tpm_size)
> +{
> + u8 data = 0;
> + int total_length = 0, nbr_dummy_bytes;
> + int value = 0;
> + u8 *data_buffer;
> +
> + data_buffer = tpm_data;
> + /* SPI read message is : locality & direction */
> + data = tpm_loc;
> + memcpy(data_buffer + total_length, &data, sizeof(data));
> + total_length++;
> + /* + TPM target register */
> + data = tpm_register;
> + memcpy(data_buffer + total_length, &data, sizeof(data));
> + total_length++;
> + /* + TPM latency (2B) + Status byte (1B) + Nb to read (tpm_size) */
> + nbr_dummy_bytes = tpm->latency + 1 + tpm_size;
> + memset(&data_buffer[total_length], TPM_DUMMY_BYTE,
> nbr_dummy_bytes);
> +
> + spi_claim_bus(tpm->tpm_dev_spi_info);
> + value = spi_xfer(tpm->tpm_dev_spi_info,
> + (total_length + nbr_dummy_bytes)*8,
> + data_buffer, tpm_data, SPI_XFER_BEGIN |
> SPI_XFER_END);
> + spi_release_bus(tpm->tpm_dev_spi_info);
> +
> + if (tpm_size > 0 && value == 0) {
> + if (tpm_data[tpm->latency + 2] == 0x5A)
> + memcpy(tpm_data,
> + tpm_data + total_length + nbr_dummy_bytes -
> tpm_size,
> + tpm_size);
>
Should line up with tpm->latency above, not with the memcpy(). Did you use
'patman' to prepare the patches? It should warn about this sort of thing.
> + else {
> + printf("Error in the TPM command, TPM status byte
> = ");
> + printf("%x\n", tpm_data[tpm->latency + tpm_size +
> 1]);
> + value = -TPM_DRIVER_ERR;
> + }
> + }
> + return value;
> +} /* spi_read8_reg() */
> +
> +/*
> + * tpm_stm_spi_cancel_or_command_ready, cancel command or move TPM in
> + * Command Ready state
> + * @param: chip, the tpm chip description as specified in
> + * driver/char/tpm/tpm.h.
> + */
> +static void tpm_stm_spi_cancel_or_command_ready(struct tpm_chip *chip)
> +{
> + u8 data = TPM_STS_COMMAND_READY;
> +
> + spi_write8_reg(chip, TPM_STS, &data, 1);
> +} /* tpm_stm_spi_cancel() */
> +
> +/*
> + * tpm_stm_spi_status return the TPM_STS register
> + * @param: chip, the tpm chip description
> + * @return: the TPM_STS register value.
> + */
> +static u8 tpm_stm_spi_status(struct tpm_chip *chip)
> +{
> + spi_read8_reg(chip, active_tpm->locality, TPM_STS,
> active_tpm->buf, 1);
> + return active_tpm->buf[0];
> +} /* tpm_stm_spi_status() */
> +
> +/*
> + * check_locality if the locality is active
> + * @param: chip, the tpm chip description
> + * @return: the active locality or -1 if no locality active.
> + */
> +static int check_locality(struct tpm_chip *chip)
> +{
> + u8 status;
> + int ret, loc_to_check = 0;
> +
> + do {
> + status = spi_read8_reg(chip, loc_to_check, TPM_ACCESS,
> + active_tpm->buf, 1);
> + if ((status == 0) && (active_tpm->buf[0] &
> + (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) ==
> + (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID))
> + break;
> + loc_to_check++;
> + } while (loc_to_check < 5);
>
I think 5 should be an enum / define.
> + if (loc_to_check == 5)
> + ret = -TPM_DRIVER_ERR;
> + else
> + ret = loc_to_check;
> + return ret;
> +} /* check_locality() */
> +
> +/*
> + * request_locality request the TPM locality
> + * @param: chip, the chip description
> + * @return: the active locality or -TPM_DRIVER_ERR.
> + */
> +static int request_locality(struct tpm_chip *chip)
> +{
> + unsigned long start, stop;
> + int rc;
> + u8 data = 0;
> +
> + /* Check locality */
> + if (check_locality(chip) == chip->locality)
> + return chip->locality;
> +
> + /* Request locality */
> + data = TPM_ACCESS_REQUEST_USE;
> + rc = spi_write8_reg(chip, TPM_ACCESS, &data, 1);
> + if (rc < 0)
> + goto end;
> +
> + /* wait for locality activated */
> + start = get_timer(0);
> + stop = chip->timeout_a;
> + do {
> + if (check_locality(chip) == chip->locality)
> + return chip->locality;
> + } while (get_timer(start) < stop);
> + rc = -TPM_DRIVER_ERR;
> +end:
> + return rc;
> +} /* request_locality() */
> +
> +/*
> + * release_locality release the active locality
> + * @param: chip, the tpm chip description.
> + * @return: should be zero if success else a negative error code.
> + */
> +static int release_locality(struct tpm_chip *chip)
> +{
> + u8 data = 0;
> +
> + data = TPM_ACCESS_ACTIVE_LOCALITY;
> + return spi_write8_reg(chip, TPM_ACCESS, &data, 1);
> +} /* release_locality()*/
> +
> +/*
> + * get_burstcount return the burstcount address 0x19 0x1A
> + * @param: chip, the chip description
> + * @return: the burstcount.
> + */
> +static int get_burstcount(struct tpm_chip *chip)
> +{
> + unsigned long start, stop;
> + u32 burstcnt;
> + u8 tpm_reg;
> + long status = 0;
> + int ret;
> +
> + /* wait for burstcount */
> + start = get_timer(0);
> + stop = chip->timeout_d;
> + do {
> + tpm_reg = TPM_STS + 1;
> + status = spi_read8_reg(chip, active_tpm->locality, tpm_reg,
> + active_tpm->buf, 1);
> + if (status < 0)
> + return -EBUSY;
> +
> + burstcnt = active_tpm->buf[0];
> + status = spi_read8_reg(chip, active_tpm->locality,
> ++tpm_reg,
> + active_tpm->buf, 1);
> + if (status < 0)
> + return -EBUSY;
> +
> + burstcnt |= active_tpm->buf[0] << 8;
> + if (burstcnt) {
> + ret = burstcnt;
> + goto end;
> + }
> + } while (get_timer(start) < stop);
> + ret = -TPM_DRIVER_ERR;
>
-ETIMEDOUT ?
> +end:
> + return ret;
> +} /* get_burstcount() */
> +
> +/*
> + * wait_for_stat wait for a TPM_STS value
> + * @param: chip, the tpm chip description
> + * @param: mask, the value mask to wait
> + * @param: timeout, the timeoutH
>
This parameter doesn't seem to be used.
> + * @param: queue, the wait queue.
> + * @return: 0 if success, -ETIME if timeout is reached.
> + */
> +static int wait_for_stat(struct tpm_chip *chip, u8 mask, unsigned long
> timeout)
> +{
> + unsigned long start, stop;
> + u8 status;
> +
> + /* check current status */
> + status = tpm_stm_spi_status(chip);
> + if ((status & mask) == mask)
> + return 0;
> +
> + start = get_timer(0);
> + stop = chip->timeout_d;
> + do {
> + status = tpm_stm_spi_status(chip);
> + if ((status & mask) == mask)
> + return 0;
> + } while (get_timer(start) < stop);
> +return -ETIME;
> +} /* wait_for_stat() */
> +
> +
> +/*
> + * recv_data receive data
> + * @param: chip, the tpm chip description
> + * @param: buf, the buffer where the data are received
> + * @param: count, the number of data to receive
> + * @return: number of byte read on success, minus error code otherwise.
> + */
> +static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
> +{
> + int size = 0, burstcnt, len;
> + long status = 0;
> +
> + while (size < count &&
> + wait_for_stat(chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> + chip->timeout_c) == 0) {
> + burstcnt = get_burstcount(chip);
> + len = min_t(int, burstcnt, count - size);
> + status = spi_read8_reg(chip, active_tpm->locality,
> + TPM_DATA_FIFO, buf + size, len);
> + if (status < 0)
> + return status;
> + size += len;
> + }
> +return size;
> +} /* recv_data() */
> +
> +
> +/*
> + * tpm_stm_spi_send send TPM commands through the SPI bus.
> + * @param: chip, the tpm chip description
> + * @param: buf, the tpm command buffer
> + * @param: len, the tpm command size
> + * @return: 0 if success else the negative error code.
> + */
> +static int tpm_stm_spi_send(struct tpm_chip *chip, const unsigned char
> *buf,
> + size_t len)
> +{
> + u32 burstcnt = 0, i, size = 0;
> + u8 data = 0;
> + long status = 0, ret = 0;
> +
> + if (chip == NULL)
> + return -EINVAL;
> + if (len < TPM_HEADER_SIZE)
> + return -EINVAL;
> +
> + ret = request_locality(chip);
> + if (ret < 0)
> + return ret;
> +
> + status = tpm_stm_spi_status(chip);
> + if ((status & TPM_STS_COMMAND_READY) == 0) {
> + tpm_stm_spi_cancel_or_command_ready(chip);
> + if (wait_for_stat(chip, TPM_STS_COMMAND_READY,
> + chip->timeout_b) < 0) {
> + ret = -ETIME;
> + goto out_err;
> + }
> + }
> +
> + for (i = 0; i < len - 1;) {
> + burstcnt = get_burstcount(chip);
> + size = min_t(int, len - i - 1, burstcnt);
> + ret = spi_write8_reg(chip, TPM_DATA_FIFO, buf, size);
> + if (ret < 0)
> + goto out_err;
> + i += size;
> + }
> +
> + status = tpm_stm_spi_status(chip);
> + if ((status & TPM_STS_DATA_EXPECT) == 0) {
> + ret = -EIO;
> + goto out_err;
> + }
> +
> + /* write last byte */
> + spi_write8_reg(chip, TPM_DATA_FIFO, buf + len - 1, 1);
> +
> + status = tpm_stm_spi_status(chip);
> + if ((status & TPM_STS_DATA_EXPECT) != 0) {
> + ret = -EIO;
> + goto out_err;
> + }
> +
> + /* go and do it */
> + data = TPM_STS_GO;
> + ret = spi_write8_reg(chip, TPM_STS, &data, 1);
> + if (ret < 0)
> + goto out_err;
> +
> + return len;
> +out_err:
> + tpm_stm_spi_cancel_or_command_ready(chip);
> + release_locality(chip);
> + return ret;
> +}
> +
> +/*
> + * tpm_stm_spi_send_hash send TPM locality 4 hash datas through the SPI
> bus
> + * to update the PCR[17].
> + * @param: chip, the tpm_chip description.
> + * @param: buf, the data buffer to send.
> + * @param: len, the number of bytes to send.
> + * @return: 0 in case of success else the negative error code.
> + */
> +static int tpm_stm_spi_send_hash(struct tpm_chip *chip, const uint8_t
> *buf,
> + size_t len)
> +{
> + int ret = 0;
> + u8 data;
> +
> + if (chip == NULL)
> + return -EBUSY;
> +
> + release_locality(chip);
> + chip->locality = LOCALITY4;
> + if (request_locality(chip) != LOCALITY4) {
> + printf("Failed to select locality 4, hash abort\n");
> + return -TPM_DRIVER_ERR;
> + }
> +
> + data = TPM_DUMMY_BYTE;
> + ret = spi_write8_reg(chip, TPM_HASH_START, &data, 1);
> + if (ret != 0)
> + goto end;
> + ret = spi_write8_reg(chip, TPM_DATA_FIFO, buf, len);
> + if (ret != 0)
> + goto end;
> + ret = spi_write8_reg(chip, TPM_HASH_END, &data, 1);
> + if (ret != 0)
> + goto end;
> +
> +end:
> + release_locality(chip);
> + chip->locality = LOCALITY0;
> + ret |= request_locality(chip);
>
What is happening here with ret? It looks like it can be an error (in which
case |= seems dodgy)?
> + return ret;
> +} /* tpm_stm_spi_send_hash */
> +
> +
> +/*
> + * tpm_stm_spi_recv received TPM response through the SPI bus.
> + * @param: chip, the tpm chip description
> + * @param: buf, the tpm command buffer
> + * @param: len, the tpm command size
> +* @return: 0 if success else the negative error code.
> + */
> +static int tpm_stm_spi_recv(struct tpm_chip *chip, unsigned char *buf,
> + size_t count)
> +{
> + int size = 0;
> + int expected;
> + u8 rx_buffer[TPM_BUFSIZE];
> +
> + if (chip == NULL)
> + return -EINVAL;
> + if (count < TPM_HEADER_SIZE) {
> + size = -EIO;
> + goto out;
> + }
> +
> + size = recv_data(chip, buf, TPM_HEADER_SIZE);
> +
> + /* read first 10 bytes, including tag, paramsize, and result */
> + if (size < TPM_HEADER_SIZE) {
> + printf("TPM error, unable to read header\n");
> + goto out;
> + }
> + memcpy(rx_buffer, buf, TPM_HEADER_SIZE);
> + expected = get_unaligned_be32(rx_buffer + TPM_RSP_SIZE_BYTE);
> + if (expected > count) {
> + size = -EIO;
> + goto out;
> + }
> +
> + size += recv_data(chip, buf, expected - TPM_HEADER_SIZE);
> + if (size < expected) {
> + printf("TPM error, unable to read remaining bytes of
> result\n");
> + size = -ETIME;
> + goto out;
> + }
> + memcpy(rx_buffer+TPM_HEADER_SIZE, buf, expected - TPM_HEADER_SIZE);
> + memcpy(buf, rx_buffer, expected);
> +out:
> + tpm_stm_spi_cancel_or_command_ready(chip);
> + release_locality(chip);
> + return size;
> +}
> +
> +/*
> + * tis_init() setup the SPI bus and check TPM(s) presence on it
> + * @return: 0 on success (the device is found or was found during an
> earlier
> + * invocation) or -ENODEV if the device is not found.
> + * Upon exit, TPM0 is the one active if present.
> + */
> +int tis_init(void)
> +{
> + int rc = 0;
> + struct spi_slave *slave;
> +
> + slave = spi_setup_slave(TPM0_SPI_BUS_NUM, TPM0_SPI_CS,
> + TPM0_SPI_MAX_SPEED, SPI_MODE_0);
> + if (slave != NULL) {
>
Perhaps:
if (!slave)
return -ENOENT;
> + active_tpm = &tpm_st33_spi_board_info[0];
> + active_tpm->timeout_a = TIS_SHORT_TIMEOUT;
> + active_tpm->timeout_b = TIS_LONG_TIMEOUT;
> + active_tpm->timeout_c = TIS_SHORT_TIMEOUT;
> + active_tpm->timeout_d = TIS_SHORT_TIMEOUT;
> + active_tpm->locality = LOCALITY0;
> + active_tpm->duration = TPM_MAX_COMMAND_DURATION;
> + active_tpm->tpm_dev_spi_info = slave;
> + active_tpm->latency = 2;
> + if (spi_read8_reg(active_tpm, active_tpm->locality,
> + TPM_ACCESS, active_tpm->buf, 1) != 0) {
> + rc = -TPM_DRIVER_ERR;
> + active_tpm->is_open = 0;
>
Is this assignment needed? Maybe just do return -EIO?
> + goto out_err;
> + }
> + active_tpm->is_open = 1;
> + printf("ST33ZP24 SPI TPM from STMicroelectronics found\n");
> + }
> +out_err:
> + return rc;
> +} /* tis_init() */
> +
> +/*
> + * tis_sendrecv() send the requested data to the TPM and then try read
> response
> + * @param: sendbuf - buffer of the data to send
> + * @param: send_size size of the data to send
> + * @param: recvbuf - memory to save the response to
> + * @param: recv_len - pointer to the size of the response buffer
> + * @return: 0 on success (and places the number of response bytes at
> recv_len)
> + * or -TPM_DRIVER_ERR on failure.
> + */
> +int tis_sendrecv(const uint8_t *sendbuf, size_t sbuf_size,
> + uint8_t *recvbuf, size_t *rbuf_len)
> +{
> + int len;
> +
> + if (active_tpm->is_open == 0)
> + return -TPM_DRIVER_ERR;
> +
> + if (sizeof(active_tpm->buf) < sbuf_size)
> + return -TPM_DRIVER_ERR;
> + len = tpm_stm_spi_send(active_tpm, sendbuf, sbuf_size);
> +
> + if (len < sbuf_size) {
> + printf("TPM error, command not fully transmitted");
> + printf(", only %d sent where expect %d\n", len, sbuf_size);
> + return -TPM_DRIVER_ERR;
> + }
> + if (wait_for_stat(active_tpm, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> + active_tpm->timeout_c) != 0)
> + return -TPM_DRIVER_ERR;
> +
> + len = tpm_stm_spi_recv(active_tpm, active_tpm->buf,
> + sizeof(active_tpm->buf));
> + if (len < 10) {
> + *rbuf_len = 0;
> + return -TPM_DRIVER_ERR;
> + }
> + if (recvbuf != NULL) {
> + memcpy(recvbuf, active_tpm->buf, len);
> + *rbuf_len = len;
> + } else {
> + printf("recvbuf is NULL, drop the TPM answer\n");
>
Not nice to printf() in drivers - should you return -EINVAL in this case?
or debug()?
> + }
> +
> + return 0;
> +} /* tis_sendrecv() */
> +
> +/*
> + * tis_open() requests access to locality 0. After all commands have been
> + * completed the caller is supposed to call tis_close().
> + * @param: chip_number, the tpm chip to activate (0 or 1)
> + * @return: 0 on success, -TPM_DRIVER_ERR if an error occur
> + */
> +int tis_open(void)
> +{
> + if (tis_close())
> + return -TPM_DRIVER_ERR;
> + /* now request access to locality. */
> + if (request_locality(active_tpm) != 0) {
> + printf("%s:%d - failed to lock locality 0\n",
> + __FILE__, __LINE__);
>
debug()
> + return -TPM_DRIVER_ERR;
> + }
> + return 0;
> +} /* tis_open() */
> +
> +/*
> + * tis_close() terminate the current session with the TPM by releasing the
> + * locked locality.
> + * @return: Returns 0 on success or TPM_DRIVER_ERR on failure (in case
> lock
> + * removal did not succeed).
> + */
> +int tis_close(void)
> +{
> + return release_locality(active_tpm);
> +} /* tis_close() */
> +
> --
> 1.7.9.5
>
>
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
