[U-Boot] [PATCH v5 5/7] Add Atmel I2C tpm

Simon Glass sjg at chromium.org
Sat May 11 23:00:40 CEST 2013


Hi Dirk,

On Tue, Apr 30, 2013 at 6:54 AM,  <dirk.eibach at gdsys.cc> wrote:
> From: Dirk Eibach <eibach at gdsys.de>
>
> Add support for Atmel TPM devices with two wire interface.
>
> Signed-off-by: Dirk Eibach <dirk.eibach at gdsys.cc>
> Signed-off-by: Reinhard Pfau <reinhard.pfau at gdsys.cc>
> ---
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None

Can you please rebase this patch on top of the existing pending patches?

http://patchwork.ozlabs.org/patch/236211/
http://patchwork.ozlabs.org/patch/236212/
http://patchwork.ozlabs.org/patch/236214/
http://patchwork.ozlabs.org/patch/236215/
http://patchwork.ozlabs.org/patch/236213/

If it helps I have put them in branch 'tpm' of u-boot-x86.git.

Also a few minor comments below:

>
>  README                      |    6 ++
>  drivers/tpm/Makefile        |    1 +
>  drivers/tpm/atmel_twi_tpm.c |  119 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 126 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/tpm/atmel_twi_tpm.c
>
> diff --git a/README b/README
> index 58b2ee5..2053931 100644
> --- a/README
> +++ b/README
> @@ -1201,6 +1201,12 @@ The following options need to be configured:
>                         If this option is set, the driver enables cache flush.
>
>  - TPM Support:
> +               Only one TPM device per system is supported at this time.
> +               So enable only one of the supported devices.
> +
> +               CONFIG_ATMEL_TWI_TPM
> +               Support for Atmel TWI TPM device. Requires I2C support.
> +

Needs rebase, also suggest CONFIG_TPM_ATMEL_TWI since you will see
that we have decided to make all the drivers start with CONFIG_TPM.

>                 CONFIG_GENERIC_LPC_TPM
>                 Support for generic parallel port TPM devices. Only one device
>                 per system is supported at this time.
> diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
> index e8c159c..5c8f246 100644
> --- a/drivers/tpm/Makefile
> +++ b/drivers/tpm/Makefile
> @@ -28,6 +28,7 @@ $(shell mkdir -p $(obj)slb9635_i2c)
>  COBJS-$(CONFIG_GENERIC_LPC_TPM) = generic_lpc_tpm.o
>  COBJS-$(CONFIG_INFINEON_TPM_I2C) += tis_i2c.o slb9635_i2c/tpm.o
>  COBJS-$(CONFIG_INFINEON_TPM_I2C) += slb9635_i2c/tpm_tis_i2c.o
> +COBJS-$(CONFIG_ATMEL_TWI_TPM) = atmel_twi_tpm.o

You should put this at the top since A comes before G.
>
>  COBJS  := $(COBJS-y)
>  SRCS   := $(COBJS:.o=.c)
> diff --git a/drivers/tpm/atmel_twi_tpm.c b/drivers/tpm/atmel_twi_tpm.c
> new file mode 100644
> index 0000000..1fa626f
> --- /dev/null
> +++ b/drivers/tpm/atmel_twi_tpm.c
> @@ -0,0 +1,119 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <tpm.h>
> +#include <i2c.h>
> +#include <asm/unaligned.h>
> +
> +#undef DEBUG_ATMEL_TWI_TPM
> +#ifdef DEBUG
> +#define DEBUG_ATMEL_TWI_TPM
> +#endif

Can this file not just use #ifdef DEBUG, and even debug() ?

> +
> +
> +/*
> + * tis_init()
> + *
> + * Initialize the TPM device. Returns 0 on success or -1 on
> + * failure (in case device probing did not succeed).
> + */
> +int tis_init(void)
> +{
> +       return 0;
> +}
> +
> +/*
> + * tis_open()
> + *
> + * Requests access to locality 0 for the caller. After all commands have been
> + * completed the caller is supposed to call tis_close().
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +int tis_open(void)
> +{
> +       return 0;
> +}
> +
> +/*
> + * tis_close()
> + *
> + * terminate the currect session with the TPM by releasing the locked
> + * locality. Returns 0 on success of -1 on failure (in case lock
> + * removal did not succeed).
> + */
> +int tis_close(void)
> +{
> +       return 0;
> +}
> +
> +/*
> + * tis_sendrecv()
> + *
> + * Send the requested data to the TPM and then try to get its response
> + *
> + * @sendbuf - buffer of the data to send
> + * @send_size size of the data to send
> + * @recvbuf - memory to save the response to
> + * @recv_len - pointer to the size of the response buffer
> + *
> + * Returns 0 on success (and places the number of response bytes at recv_len)
> + * or -1 on failure.
> + */
> +int tis_sendrecv(const uint8_t *sendbuf, size_t send_size, uint8_t *recvbuf,
> +                       size_t *recv_len)
> +{
> +       int res;
> +
> +#ifdef DEBUG_ATMEL_TWI_TPM
> +       memset(recvbuf, 0xcc, *recv_len);
> +       printf("send to TPM (%d bytes, recv_len=%d):\n", send_size, *recv_len);
> +       print_buffer(0, (void *)sendbuf, 1, send_size, 0);
> +#endif
> +
> +       res = i2c_write(0x29, 0, 0, (uchar *)sendbuf, send_size);
> +       if (res) {
> +               printf("i2c_write returned %d\n", res);
> +               return -1;
> +       }
> +
> +       mdelay(1);
> +       /* TODO timeout for the loop?! */

Who is going to do this timeout? It does seem like a good idea, something like:

start = get_timer(0);
while (...)
   if (get_timer(start) > TIMEOUT_MS)
       ...do something..


> +       while ((res = i2c_read(0x29, 0, 0, recvbuf, 10)))
> +               udelay(100);
> +       if (!res) {
> +               *recv_len = get_unaligned_be32(recvbuf + 2);
> +               if (*recv_len > 10)
> +                       res = i2c_read(0x29, 0, 0, recvbuf, *recv_len);
> +       }
> +       if (res) {
> +               printf("i2c_read returned %d (rlen=%d)\n", res, *recv_len);
> +#ifdef DEBUG_ATMEL_TWI_TPM
> +               print_buffer(0, recvbuf, 1, *recv_len, 0);
> +#endif
> +       }
> +
> +#ifdef DEBUG_ATMEL_TWI_TPM
> +       if (!res) {
> +               printf("read from TPM (%d bytes):\n", *recv_len);
> +               print_buffer(0, recvbuf, 1, *recv_len, 0);
> +       }
> +#endif
> +
> +       return res;
> +}
> --
> 1.7.2.5
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Regards,
Simon


More information about the U-Boot mailing list