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

2013-05-11 Thread Simon Glass
Hi Dirk,

On Tue, Apr 30, 2013 at 6:54 AM,  dirk.eib...@gdsys.cc wrote:
 From: Dirk Eibach eib...@gdsys.de

 Add support for Atmel TPM devices with two wire interface.

 Signed-off-by: Dirk Eibach dirk.eib...@gdsys.cc
 Signed-off-by: Reinhard Pfau reinhard.p...@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 000..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);
 +   

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

2013-04-30 Thread dirk . eibach
From: Dirk Eibach eib...@gdsys.de

Add support for Atmel TPM devices with two wire interface.

Signed-off-by: Dirk Eibach dirk.eib...@gdsys.cc
Signed-off-by: Reinhard Pfau reinhard.p...@gdsys.cc
---
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 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.
+
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
 
 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 000..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
+
+
+/*
+ * 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?! */
+   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) {
+