U-boot Archive mirror
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs
@ 2015-08-09 13:19 Christophe Ricard
  2015-08-09 13:19 ` [U-Boot] [PATCH 1/3] tpm: Move tpm_tis_i2c to tpm_i2c_infineon Christophe Ricard
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Christophe Ricard @ 2015-08-09 13:19 UTC (permalink / raw
  To: u-boot

Hi,

This patch serie introduce TPM driver model allowing to instantiate a TPM
using U_BOOT_DEVICE macro for platform_data or device tree.

As an information, there is no TCG transport protocol official specification
for i2c for TPM1.2. The TPM uclass allows to support different kind of bus
(LPC, I2C, SPI) keeping the TPM command duration in common.

Also, this serie introduce TPM1.2 from STMicroelectronics ST33ZP24 with
I2C and SPI support. It has been ported from existing Linux drivers.

This has been tested on Beagleboard xM.

Best Regards
Christophe


Christophe Ricard (3):
  tpm: Move tpm_tis_i2c to tpm_i2c_infineon
  tpm: Initial work to introduce TPM driver model
  tpm: Add st33zp24 tpm with i2c and spi phy

 README                                            |  23 +-
 drivers/tpm/Makefile                              |   5 +-
 drivers/tpm/st33zp24/Makefile                     |   7 +
 drivers/tpm/st33zp24/i2c.c                        | 226 +++++++++++
 drivers/tpm/st33zp24/spi.c                        | 286 ++++++++++++++
 drivers/tpm/st33zp24/st33zp24.c                   | 454 ++++++++++++++++++++++
 drivers/tpm/st33zp24/st33zp24.h                   |  29 ++
 drivers/tpm/tpm.c                                 | 275 +++----------
 drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} | 271 ++++++++-----
 drivers/tpm/tpm_private.h                         |  23 +-
 include/dm/platform_data/st33zp24_i2c.h           |  28 ++
 include/dm/platform_data/st33zp24_spi.h           |  30 ++
 include/dm/platform_data/tpm_i2c_infineon.h       |  23 ++
 include/dm/uclass-id.h                            |   1 +
 include/fdtdec.h                                  |   5 +-
 lib/fdtdec.c                                      |   2 +
 16 files changed, 1351 insertions(+), 337 deletions(-)
 create mode 100644 drivers/tpm/st33zp24/Makefile
 create mode 100644 drivers/tpm/st33zp24/i2c.c
 create mode 100644 drivers/tpm/st33zp24/spi.c
 create mode 100644 drivers/tpm/st33zp24/st33zp24.c
 create mode 100644 drivers/tpm/st33zp24/st33zp24.h
 rename drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} (70%)
 create mode 100644 include/dm/platform_data/st33zp24_i2c.h
 create mode 100644 include/dm/platform_data/st33zp24_spi.h
 create mode 100644 include/dm/platform_data/tpm_i2c_infineon.h

-- 
2.1.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 1/3] tpm: Move tpm_tis_i2c to tpm_i2c_infineon
  2015-08-09 13:19 [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs Christophe Ricard
@ 2015-08-09 13:19 ` Christophe Ricard
  2015-08-13 15:55   ` Simon Glass
  2015-08-09 13:19 ` [U-Boot] [PATCH 2/3] tpm: Initial work to introduce TPM driver model Christophe Ricard
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Christophe Ricard @ 2015-08-09 13:19 UTC (permalink / raw
  To: u-boot

As there is no TCG specification or recommendation for i2c TPM 1.2, move
tpm_tis_i2c driver to tpm_i2c_infineon. Other tpm vendors like atmel or
stmicroelectronics may have a different transport protocol for i2c.

Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
---

 README                                            | 4 ++--
 drivers/tpm/Makefile                              | 2 +-
 drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} | 0
 3 files changed, 3 insertions(+), 3 deletions(-)
 rename drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} (100%)

diff --git a/README b/README
index 1bcb63c..a563aa1 100644
--- a/README
+++ b/README
@@ -1492,8 +1492,8 @@ The following options need to be configured:
 		CONFIG_TPM
 		Support TPM devices.
 
-		CONFIG_TPM_TIS_I2C
-		Support for i2c bus TPM devices. Only one device
+		CONFIG_TPM_I2C_INFINEON
+		Support for infineon i2c bus TPM devices. Only one device
 		per system is supported at this time.
 
 			CONFIG_TPM_TIS_I2C_BUS_NUMBER
diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
index 150570e..fea246f 100644
--- a/drivers/tpm/Makefile
+++ b/drivers/tpm/Makefile
@@ -6,6 +6,6 @@
 # TODO: Merge tpm_tis_lpc.c with tpm.c
 obj-$(CONFIG_TPM_ATMEL_TWI) += tpm_atmel_twi.o
 obj-$(CONFIG_TPM_TIS_I2C) += tpm.o
-obj-$(CONFIG_TPM_TIS_I2C) += tpm_tis_i2c.o
+obj-$(CONFIG_TPM_INFINEON_I2C) += tpm_i2c_infineon.o
 obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
 obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o
diff --git a/drivers/tpm/tpm_tis_i2c.c b/drivers/tpm/tpm_i2c_infineon.c
similarity index 100%
rename from drivers/tpm/tpm_tis_i2c.c
rename to drivers/tpm/tpm_i2c_infineon.c
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/3] tpm: Initial work to introduce TPM driver model
  2015-08-09 13:19 [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs Christophe Ricard
  2015-08-09 13:19 ` [U-Boot] [PATCH 1/3] tpm: Move tpm_tis_i2c to tpm_i2c_infineon Christophe Ricard
@ 2015-08-09 13:19 ` Christophe Ricard
  2015-08-13 15:55   ` Simon Glass
  2015-08-09 13:19 ` [U-Boot] [PATCH 3/3] tpm: Add st33zp24 tpm with i2c and spi phy Christophe Ricard
  2015-08-09 13:28 ` [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs Simon Glass
  3 siblings, 1 reply; 17+ messages in thread
From: Christophe Ricard @ 2015-08-09 13:19 UTC (permalink / raw
  To: u-boot

drivers/tpm/tpm.c is a TPM core driver port from Linux.
So far in u-boot only infineon i2c driver is using it but it could fit
for others...

Introduce a new tpm uclass so that every TPM driver can register against it and
and take benefit of common functions and data such as tpm_transmit,
tpm_register_hardware & tpm_remove_hardware.
Finally tis_init, tis_open, tis_close, tis_sendrecv are using ops allowing
to introduce proprietary instructions.
Also this patch convert tpm_i2c_infineon for using this tpm uclass.

Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
---

 README                                      |   8 +-
 drivers/tpm/Makefile                        |   2 +-
 drivers/tpm/tpm.c                           | 275 +++++++---------------------
 drivers/tpm/tpm_i2c_infineon.c              | 271 ++++++++++++++++-----------
 drivers/tpm/tpm_private.h                   |  23 ++-
 include/dm/platform_data/tpm_i2c_infineon.h |  23 +++
 include/dm/uclass-id.h                      |   1 +
 7 files changed, 270 insertions(+), 333 deletions(-)
 create mode 100644 include/dm/platform_data/tpm_i2c_infineon.h

diff --git a/README b/README
index a563aa1..506ff6c 100644
--- a/README
+++ b/README
@@ -1489,19 +1489,13 @@ The following options need to be configured:
 		Support for PWM modul on the imx6.
 
 - TPM Support:
-		CONFIG_TPM
+		CONFIG_DM_TPM
 		Support TPM devices.
 
 		CONFIG_TPM_I2C_INFINEON
 		Support for infineon i2c bus TPM devices. Only one device
 		per system is supported at this time.
 
-			CONFIG_TPM_TIS_I2C_BUS_NUMBER
-			Define the the i2c bus number for the TPM device
-
-			CONFIG_TPM_TIS_I2C_SLAVE_ADDRESS
-			Define the TPM's address on the i2c bus
-
 			CONFIG_TPM_TIS_I2C_BURST_LIMITATION
 			Define the burst count bytes upper limit
 
diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
index fea246f..bd2cd6d 100644
--- a/drivers/tpm/Makefile
+++ b/drivers/tpm/Makefile
@@ -5,7 +5,7 @@
 
 # TODO: Merge tpm_tis_lpc.c with tpm.c
 obj-$(CONFIG_TPM_ATMEL_TWI) += tpm_atmel_twi.o
-obj-$(CONFIG_TPM_TIS_I2C) += tpm.o
+obj-$(CONFIG_DM_TPM) += tpm.o
 obj-$(CONFIG_TPM_INFINEON_I2C) += tpm_i2c_infineon.o
 obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
 obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o
diff --git a/drivers/tpm/tpm.c b/drivers/tpm/tpm.c
index a650892..caf208d 100644
--- a/drivers/tpm/tpm.c
+++ b/drivers/tpm/tpm.c
@@ -36,8 +36,6 @@
 #include <common.h>
 #include <dm.h>
 #include <linux/compiler.h>
-#include <fdtdec.h>
-#include <i2c.h>
 #include <tpm.h>
 #include <asm-generic/errno.h>
 #include <linux/types.h>
@@ -47,21 +45,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-/* TPM configuration */
-struct tpm {
-#ifdef CONFIG_DM_I2C
-	struct udevice *dev;
-#else
-	int i2c_bus;
-	int slave_addr;
-	int old_bus;
-#endif
-	char inited;
-} tpm;
-
-/* Global structure for tpm chip data */
-static struct tpm_chip g_chip;
-
 enum tpm_duration {
 	TPM_SHORT = 0,
 	TPM_MEDIUM = 1,
@@ -375,14 +358,12 @@ static unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
 		return duration;
 }
 
-static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz)
+static ssize_t tpm_transmit(struct tpm_chip *chip, const unsigned char *buf, size_t bufsiz)
 {
 	int rc;
 	u32 count, ordinal;
 	unsigned long start, stop;
 
-	struct tpm_chip *chip = &g_chip;
-
 	/* switch endianess: big->little */
 	count = get_unaligned_be32(buf + TPM_CMD_COUNT_BYTE);
 	ordinal = get_unaligned_be32(buf + TPM_CMD_ORDINAL_BYTE);
@@ -441,233 +422,102 @@ out:
 	return rc;
 }
 
-#ifdef CONFIG_DM_I2C
-static int tpm_open_dev(struct udevice *dev)
-{
-	int rc;
-
-	debug("%s: start\n", __func__);
-	if (g_chip.is_open)
-		return -EBUSY;
-	rc = tpm_vendor_init_dev(dev);
-	if (rc < 0)
-		g_chip.is_open = 0;
-	return rc;
-}
-#else
-static int tpm_open(uint32_t dev_addr)
-{
-	int rc;
-
-	if (g_chip.is_open)
-		return -EBUSY;
-	rc = tpm_vendor_init(dev_addr);
-	if (rc < 0)
-		g_chip.is_open = 0;
-	return rc;
-}
-#endif
-static void tpm_close(void)
-{
-	if (g_chip.is_open) {
-		tpm_vendor_cleanup(&g_chip);
-		g_chip.is_open = 0;
-	}
-}
-
-static int tpm_select(void)
-{
-#ifndef CONFIG_DM_I2C
-	int ret;
-
-	tpm.old_bus = i2c_get_bus_num();
-	if (tpm.old_bus != tpm.i2c_bus) {
-		ret = i2c_set_bus_num(tpm.i2c_bus);
-		if (ret) {
-			debug("%s: Fail to set i2c bus %d\n", __func__,
-			      tpm.i2c_bus);
-			return -1;
-		}
-	}
-#endif
-	return 0;
-}
-
-static int tpm_deselect(void)
+void tpm_remove_hardware(struct udevice *dev)
 {
-#ifndef CONFIG_DM_I2C
-	int ret;
-
-	if (tpm.old_bus != i2c_get_bus_num()) {
-		ret = i2c_set_bus_num(tpm.old_bus);
-		if (ret) {
-			debug("%s: Fail to restore i2c bus %d\n",
-			      __func__, tpm.old_bus);
-			return -1;
-		}
-	}
-	tpm.old_bus = -1;
-#endif
-	return 0;
 }
 
-/**
- * Decode TPM configuration.
- *
- * @param dev	Returns a configuration of TPM device
- * @return 0 if ok, -1 on error
- */
-static int tpm_decode_config(struct tpm *dev)
+struct tpm_chip *tpm_register_hardware(struct udevice *dev,
+			const struct tpm_vendor_specific *entry)
 {
-	const void *blob = gd->fdt_blob;
-	int parent;
-	int node;
-
-	node = fdtdec_next_compatible(blob, 0, COMPAT_INFINEON_SLB9635_TPM);
-	if (node < 0) {
-		node = fdtdec_next_compatible(blob, 0,
-				COMPAT_INFINEON_SLB9645_TPM);
-	}
-	if (node < 0) {
-		debug("%s: Node not found\n", __func__);
-		return -1;
-	}
-	parent = fdt_parent_offset(blob, node);
-	if (parent < 0) {
-		debug("%s: Cannot find node parent\n", __func__);
-		return -1;
-	}
-#ifdef CONFIG_DM_I2C
-	struct udevice *bus;
-	int chip_addr;
-	int ret;
-
-	/*
-	 * TODO(sjg at chromium.org): Remove this when driver model supports
-	 * TPMs
-	 */
-	ret = uclass_get_device_by_of_offset(UCLASS_I2C, parent, &bus);
-	if (ret) {
-		debug("Cannot find bus for node '%s: ret=%d'\n",
-		      fdt_get_name(blob, parent, NULL), ret);
-		return ret;
-	}
-
-	chip_addr = fdtdec_get_int(blob, node, "reg", -1);
-	if (chip_addr == -1) {
-		debug("Cannot find reg property for node '%s: ret=%d'\n",
-		      fdt_get_name(blob, node, NULL), ret);
-		return ret;
-	}
-	/*
-	 * TODO(sjg@chromium.org): Older TPMs will need to use the older method
-	 * in iic_tpm_read() so the offset length needs to be 0 here.
-	 */
-	ret = i2c_get_chip(bus, chip_addr, 1, &dev->dev);
-	if (ret) {
-		debug("Cannot find device for node '%s: ret=%d'\n",
-		      fdt_get_name(blob, node, NULL), ret);
-		return ret;
-	}
-#else
-	int i2c_bus;
-
-	i2c_bus = i2c_get_bus_num_fdt(parent);
-	if (i2c_bus < 0)
-		return -1;
-	dev->i2c_bus = i2c_bus;
-	dev->slave_addr = fdtdec_get_addr(blob, node, "reg");
-#endif
-
-	return 0;
-}
-
-struct tpm_chip *tpm_register_hardware(const struct tpm_vendor_specific *entry)
-{
-	struct tpm_chip *chip;
+	struct tpm_chip *chip = dev_get_uclass_priv(dev);
 
 	/* Driver specific per-device data */
-	chip = &g_chip;
 	memcpy(&chip->vendor, entry, sizeof(struct tpm_vendor_specific));
-	chip->is_open = 1;
 
 	return chip;
 }
 
 int tis_init(void)
 {
-	if (tpm.inited)
-		return 0;
-
-	if (tpm_decode_config(&tpm))
-		return -1;
-
-	if (tpm_select())
-		return -1;
+	int ret;
+	struct udevice *dev;
+	const struct dm_tpm_ops *ops;
 
-#ifndef CONFIG_DM_I2C
-	/*
-	 * Probe TPM twice; the first probing might fail because TPM is asleep,
-	 * and the probing can wake up TPM.
-	 */
-	if (i2c_probe(tpm.slave_addr) && i2c_probe(tpm.slave_addr)) {
-		debug("%s: fail to probe i2c addr 0x%x\n", __func__,
-		      tpm.slave_addr);
-		return -1;
+	ret = uclass_get_device(UCLASS_TPM, 0, &dev);
+	if (ret) {
+		printf("TIS: Can't find any TPM\n");
+		return -EINVAL;
 	}
-#endif
-
-	tpm_deselect();
-	debug("%s: done\n", __func__);
 
-	tpm.inited = 1;
+	ops = device_get_ops(dev);
+	if (ops && ops->init)
+		return ops->init(dev);
 
 	return 0;
 }
 
 int tis_open(void)
 {
-	int rc;
-
-	if (!tpm.inited)
-		return -1;
+	int ret;
+	struct udevice *dev;
+	struct tpm_chip *chip;
+	const struct dm_tpm_ops *ops;
 
-	if (tpm_select())
-		return -1;
+	ret = uclass_get_device(UCLASS_TPM, 0, &dev);
+	if (ret) {
+		printf("TIS: Can't find any TPM\n");
+		return -EINVAL;
+	}
 
-#ifdef CONFIG_DM_I2C
-	rc = tpm_open_dev(tpm.dev);
-#else
-	rc = tpm_open(tpm.slave_addr);
-#endif
+	chip = dev_get_uclass_priv(dev);
+	chip->is_open = 1;
 
-	tpm_deselect();
+	ops = device_get_ops(dev);
+	if (ops && ops->open)
+		return ops->open(dev);
 
-	return rc;
+	return 0;
 }
 
 int tis_close(void)
 {
-	if (!tpm.inited)
-		return -1;
+	int ret;
+	struct udevice *dev;
+	struct tpm_chip *chip;
+	const struct dm_tpm_ops *ops;
 
-	if (tpm_select())
-		return -1;
+	ret = uclass_get_device(UCLASS_TPM, 0, &dev);
+	if (ret) {
+		printf("TIS: Can't find any TPM\n");
+		return -EINVAL;
+	}
 
-	tpm_close();
+	chip = dev_get_uclass_priv(dev);
+	chip->is_open = 0;
 
-	tpm_deselect();
+	ops = device_get_ops(dev);
+	if (ops && ops->close)
+		return ops->close(dev);
 
 	return 0;
 }
 
 int tis_sendrecv(const uint8_t *sendbuf, size_t sbuf_size,
-		uint8_t *recvbuf, size_t *rbuf_len)
+		 uint8_t *recvbuf, size_t *rbuf_len)
 {
-	int len;
+	struct udevice *dev;
+	struct tpm_chip *chip;
+	int len, ret;
 	uint8_t buf[4096];
 
-	if (!tpm.inited)
+	ret = uclass_get_device(UCLASS_TPM, 0, &dev);
+	if (ret) {
+		printf("TIS: Can't find any TPM\n");
+		return -EINVAL;
+	}
+
+	chip = dev_get_uclass_priv(dev);
+	if (!chip->is_open)
 		return -1;
 
 	if (sizeof(buf) < sbuf_size)
@@ -675,12 +525,7 @@ int tis_sendrecv(const uint8_t *sendbuf, size_t sbuf_size,
 
 	memcpy(buf, sendbuf, sbuf_size);
 
-	if (tpm_select())
-		return -1;
-
-	len = tpm_transmit(buf, sbuf_size);
-
-	tpm_deselect();
+	len = tpm_transmit(chip, buf, sbuf_size);
 
 	if (len < 10) {
 		*rbuf_len = 0;
@@ -692,3 +537,9 @@ int tis_sendrecv(const uint8_t *sendbuf, size_t sbuf_size,
 
 	return 0;
 }
+
+UCLASS_DRIVER(tpm) = {
+	.id             = UCLASS_TPM,
+	.name           = "tpm",
+	.per_device_auto_alloc_size = sizeof(struct tpm_chip),
+};
diff --git a/drivers/tpm/tpm_i2c_infineon.c b/drivers/tpm/tpm_i2c_infineon.c
index ee4dfea..b39ebd2 100644
--- a/drivers/tpm/tpm_i2c_infineon.c
+++ b/drivers/tpm/tpm_i2c_infineon.c
@@ -45,6 +45,7 @@
 #include <asm-generic/errno.h>
 #include <linux/types.h>
 #include <linux/unaligned/be_byteshift.h>
+#include <dm/platform_data/tpm_i2c_infineon.h>
 
 #include "tpm_private.h"
 
@@ -123,25 +124,16 @@ static const char * const chip_name[] = {
 
 /* Structure to store I2C TPM specific stuff */
 struct tpm_dev {
-#ifdef CONFIG_DM_I2C
-	struct udevice *dev;
-#else
-	uint addr;
-#endif
+	uint slave_addr;
+	uint i2c_bus;
+	uint old_bus;
 	u8 buf[TPM_DEV_BUFSIZE + sizeof(u8)];  /* Max buffer size + addr */
 	enum i2c_chip_type chip_type;
 };
 
-static struct tpm_dev tpm_dev = {
-#ifndef CONFIG_DM_I2C
-	.addr = TPM_I2C_ADDR
-#endif
-};
-
-static struct tpm_dev tpm_dev;
-
 /*
  * iic_tpm_read() - read from TPM register
+ * @chip: tpm chip to deal with
  * @addr: register address to read from
  * @buffer: provided by caller
  * @len: number of bytes to read
@@ -154,21 +146,18 @@ static struct tpm_dev tpm_dev;
  *
  * Return -EIO on error, 0 on success.
  */
-static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
+static int iic_tpm_read(struct tpm_chip *chip, u8 addr, u8 *buffer, size_t len)
 {
 	int rc;
 	int count;
 	uint32_t addrbuf = addr;
+	struct tpm_dev *tpm_dev = TPM_VPRIV(chip);
 
-	if ((tpm_dev.chip_type == SLB9635) || (tpm_dev.chip_type == UNKNOWN)) {
+	if ((tpm_dev->chip_type == SLB9635) || (tpm_dev->chip_type == UNKNOWN)) {
 		/* slb9635 protocol should work in both cases */
 		for (count = 0; count < MAX_COUNT; count++) {
-#ifdef CONFIG_DM_I2C
-			rc = dm_i2c_write(tpm_dev.dev, 0, (uchar *)&addrbuf, 1);
-#else
-			rc = i2c_write(tpm_dev.addr, 0, 0,
+			rc = i2c_write(tpm_dev->slave_addr, 0, 0,
 				       (uchar *)&addrbuf, 1);
-#endif
 			if (rc == 0)
 				break;  /* Success, break to skip sleep */
 			udelay(SLEEP_DURATION);
@@ -182,11 +171,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
 		 */
 		for (count = 0; count < MAX_COUNT; count++) {
 			udelay(SLEEP_DURATION);
-#ifdef CONFIG_DM_I2C
-			rc = dm_i2c_read(tpm_dev.dev, 0, buffer, len);
-#else
-			rc = i2c_read(tpm_dev.addr, 0, 0, buffer, len);
-#endif
+			rc = i2c_read(tpm_dev->slave_addr, 0, 0, buffer, len);
 			if (rc == 0)
 				break;  /* success, break to skip sleep */
 		}
@@ -199,11 +184,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
 		 * be safe on the safe side.
 		 */
 		for (count = 0; count < MAX_COUNT; count++) {
-#ifdef CONFIG_DM_I2C
-			rc = dm_i2c_read(tpm_dev.dev, addr, buffer, len);
-#else
-			rc = i2c_read(tpm_dev.addr, addr, 1, buffer, len);
-#endif
+			rc = i2c_read(tpm_dev->slave_addr, addr, 1, buffer, len);
 			if (rc == 0)
 				break;  /* break here to skip sleep */
 			udelay(SLEEP_DURATION);
@@ -218,26 +199,21 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
 	return 0;
 }
 
-static int iic_tpm_write_generic(u8 addr, u8 *buffer, size_t len,
+static int iic_tpm_write_generic(struct tpm_chip *chip, u8 addr, u8 *buffer, size_t len,
 		unsigned int sleep_time, u8 max_count)
 {
 	int rc = 0;
 	int count;
+	struct tpm_dev *tpm_dev = TPM_VPRIV(chip);
 
 	/* Prepare send buffer */
-#ifndef CONFIG_DM_I2C
-	tpm_dev.buf[0] = addr;
-	memcpy(&(tpm_dev.buf[1]), buffer, len);
-	buffer = tpm_dev.buf;
+	tpm_dev->buf[0] = addr;
+	memcpy(&(tpm_dev->buf[1]), buffer, len);
+	buffer = tpm_dev->buf;
 	len++;
-#endif
 
 	for (count = 0; count < max_count; count++) {
-#ifdef CONFIG_DM_I2C
-		rc = dm_i2c_write(tpm_dev.dev, addr, buffer, len);
-#else
-		rc = i2c_write(tpm_dev.addr, 0, 0, buffer, len);
-#endif
+		rc = i2c_write(tpm_dev->slave_addr, 0, 0, buffer, len);
 		if (rc == 0)
 			break;  /* Success, break to skip sleep */
 		udelay(sleep_time);
@@ -253,6 +229,7 @@ static int iic_tpm_write_generic(u8 addr, u8 *buffer, size_t len,
 
 /*
  * iic_tpm_write() - write to TPM register
+ * @chip: tpm chip to deal with
  * @addr: register address to write to
  * @buffer: containing data to be written
  * @len: number of bytes to write
@@ -267,9 +244,9 @@ static int iic_tpm_write_generic(u8 addr, u8 *buffer, size_t len,
  *
  * Return -EIO on error, 0 on success
  */
-static int iic_tpm_write(u8 addr, u8 *buffer, size_t len)
+static int iic_tpm_write(struct tpm_chip *chip, u8 addr, u8 *buffer, size_t len)
 {
-	return iic_tpm_write_generic(addr, buffer, len, SLEEP_DURATION,
+	return iic_tpm_write_generic(chip, addr, buffer, len, SLEEP_DURATION,
 			MAX_COUNT);
 }
 
@@ -277,9 +254,9 @@ static int iic_tpm_write(u8 addr, u8 *buffer, size_t len)
  * This function is needed especially for the cleanup situation after
  * sending TPM_READY
  */
-static int iic_tpm_write_long(u8 addr, u8 *buffer, size_t len)
+static int iic_tpm_write_long(struct tpm_chip *chip, u8 addr, u8 *buffer, size_t len)
 {
-	return iic_tpm_write_generic(addr, buffer, len, SLEEP_DURATION_LONG,
+	return iic_tpm_write_generic(chip, addr, buffer, len, SLEEP_DURATION_LONG,
 			MAX_COUNT_LONG);
 }
 
@@ -289,7 +266,7 @@ static int check_locality(struct tpm_chip *chip, int loc)
 	u8 buf;
 	int rc;
 
-	rc = iic_tpm_read(TPM_ACCESS(loc), &buf, 1);
+	rc = iic_tpm_read(chip, TPM_ACCESS(loc), &buf, 1);
 	if (rc < 0)
 		return rc;
 
@@ -306,12 +283,12 @@ static void release_locality(struct tpm_chip *chip, int loc, int force)
 	const u8 mask = TPM_ACCESS_REQUEST_PENDING | TPM_ACCESS_VALID;
 	u8 buf;
 
-	if (iic_tpm_read(TPM_ACCESS(loc), &buf, 1) < 0)
+	if (iic_tpm_read(chip, TPM_ACCESS(loc), &buf, 1) < 0)
 		return;
 
 	if (force || (buf & mask) == mask) {
 		buf = TPM_ACCESS_ACTIVE_LOCALITY;
-		iic_tpm_write(TPM_ACCESS(loc), &buf, 1);
+		iic_tpm_write(chip, TPM_ACCESS(loc), &buf, 1);
 	}
 }
 
@@ -324,7 +301,7 @@ static int request_locality(struct tpm_chip *chip, int loc)
 	if (check_locality(chip, loc) >= 0)
 		return loc;  /* We already have the locality */
 
-	rc = iic_tpm_write(TPM_ACCESS(loc), &buf, 1);
+	rc = iic_tpm_write(chip, TPM_ACCESS(loc), &buf, 1);
 	if (rc)
 		return rc;
 
@@ -340,18 +317,18 @@ static int request_locality(struct tpm_chip *chip, int loc)
 	return -1;
 }
 
-static u8 tpm_tis_i2c_status(struct tpm_chip *chip)
+static u8 tpm_i2c_tis_status(struct tpm_chip *chip)
 {
 	/* NOTE: Since i2c read may fail, return 0 in this case --> time-out */
 	u8 buf;
 
-	if (iic_tpm_read(TPM_STS(chip->vendor.locality), &buf, 1) < 0)
+	if (iic_tpm_read(chip, TPM_STS(chip->vendor.locality), &buf, 1) < 0)
 		return 0;
 	else
 		return buf;
 }
 
-static void tpm_tis_i2c_ready(struct tpm_chip *chip)
+static void tpm_i2c_tis_ready(struct tpm_chip *chip)
 {
 	int rc;
 
@@ -359,7 +336,7 @@ static void tpm_tis_i2c_ready(struct tpm_chip *chip)
 	u8 buf = TPM_STS_COMMAND_READY;
 
 	debug("%s\n", __func__);
-	rc = iic_tpm_write_long(TPM_STS(chip->vendor.locality), &buf, 1);
+	rc = iic_tpm_write_long(chip, TPM_STS(chip->vendor.locality), &buf, 1);
 	if (rc)
 		debug("%s: rc=%d\n", __func__, rc);
 }
@@ -377,7 +354,7 @@ static ssize_t get_burstcount(struct tpm_chip *chip)
 	do {
 		/* Note: STS is little endian */
 		addr = TPM_STS(chip->vendor.locality) + 1;
-		if (iic_tpm_read(addr, buf, 3) < 0)
+		if (iic_tpm_read(chip, addr, buf, 3) < 0)
 			burstcnt = 0;
 		else
 			burstcnt = (buf[2] << 16) + (buf[1] << 8) + buf[0];
@@ -396,7 +373,7 @@ static int wait_for_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 	unsigned long start, stop;
 
 	/* Check current status */
-	*status = tpm_tis_i2c_status(chip);
+	*status = tpm_i2c_tis_status(chip);
 	if ((*status & mask) == mask)
 		return 0;
 
@@ -404,7 +381,7 @@ static int wait_for_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 	stop = timeout;
 	do {
 		udelay(TPM_TIMEOUT * 1000);
-		*status = tpm_tis_i2c_status(chip);
+		*status = tpm_i2c_tis_status(chip);
 		if ((*status & mask) == mask)
 			return 0;
 	} while (get_timer(start) < stop);
@@ -429,7 +406,7 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
 		if (burstcnt > (count - size))
 			burstcnt = count - size;
 
-		rc = iic_tpm_read(TPM_DATA_FIFO(chip->vendor.locality),
+		rc = iic_tpm_read(chip, TPM_DATA_FIFO(chip->vendor.locality),
 				&(buf[size]), burstcnt);
 		if (rc == 0)
 			size += burstcnt;
@@ -438,7 +415,43 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
 	return size;
 }
 
-static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
+static int tpm_select(struct tpm_chip *chip)
+{
+	struct tpm_dev *tpm = TPM_VPRIV(chip);
+	int ret;
+
+	tpm->old_bus = i2c_get_bus_num();
+	if (tpm->old_bus != tpm->i2c_bus) {
+		ret = i2c_set_bus_num(tpm->i2c_bus);
+		if (ret) {
+			debug("%s: Fail to set i2c bus %d\n", __func__,
+			      tpm->i2c_bus);
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+static int tpm_deselect(struct tpm_chip *chip)
+{
+	struct tpm_dev *tpm = TPM_VPRIV(chip);
+	int ret;
+
+	if (tpm->old_bus != i2c_get_bus_num()) {
+		ret = i2c_set_bus_num(tpm->old_bus);
+		if (ret) {
+			debug("%s: Fail to restore i2c bus %d\n",
+			      __func__, tpm->old_bus);
+			return -1;
+		}
+	}
+	tpm->old_bus = -1;
+
+	return 0;
+}
+
+static int tpm_i2c_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 {
 	int size = 0;
 	int expected, status;
@@ -448,6 +461,10 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 		goto out;
 	}
 
+	size = tpm_select(chip);
+	if (size < 0)
+		return size;
+
 	/* Read first 10 bytes, including tag, paramsize, and result */
 	size = recv_data(chip, buf, TPM_HEADER_SIZE);
 	if (size < TPM_HEADER_SIZE) {
@@ -479,18 +496,18 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	}
 
 out:
-	tpm_tis_i2c_ready(chip);
+	tpm_i2c_tis_ready(chip);
 	/*
 	 * The TPM needs some time to clean up here,
 	 * so we sleep rather than keeping the bus busy
 	 */
 	udelay(2000);
 	release_locality(chip, chip->vendor.locality, 0);
-
+	tpm_deselect(chip);
 	return size;
 }
 
-static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
+static int tpm_i2c_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
 {
 	int rc, status;
 	size_t burstcnt;
@@ -502,12 +519,16 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	if (len > TPM_DEV_BUFSIZE)
 		return -E2BIG;  /* Command is too long for our tpm, sorry */
 
+	rc = tpm_select(chip);
+	if (rc < 0)
+		return rc;
+
 	if (request_locality(chip, 0) < 0)
 		return -EBUSY;
 
-	status = tpm_tis_i2c_status(chip);
+	status = tpm_i2c_tis_status(chip);
 	if ((status & TPM_STS_COMMAND_READY) == 0) {
-		tpm_tis_i2c_ready(chip);
+		tpm_i2c_tis_ready(chip);
 		if (wait_for_stat(chip, TPM_STS_COMMAND_READY,
 				  chip->vendor.timeout_b, &status) < 0) {
 			rc = -ETIME;
@@ -531,7 +552,7 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
 			burstcnt = CONFIG_TPM_TIS_I2C_BURST_LIMITATION;
 #endif /* CONFIG_TPM_TIS_I2C_BURST_LIMITATION */
 
-		rc = iic_tpm_write(TPM_DATA_FIFO(chip->vendor.locality),
+		rc = iic_tpm_write(chip, TPM_DATA_FIFO(chip->vendor.locality),
 				&(buf[count]), burstcnt);
 		if (rc == 0)
 			count += burstcnt;
@@ -554,29 +575,29 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	}
 
 	/* Go and do it */
-	iic_tpm_write(TPM_STS(chip->vendor.locality), &sts, 1);
+	iic_tpm_write(chip, TPM_STS(chip->vendor.locality), &sts, 1);
 	debug("done\n");
 
 	return len;
 
 out_err:
 	debug("%s: out_err\n", __func__);
-	tpm_tis_i2c_ready(chip);
+	tpm_i2c_tis_ready(chip);
 	/*
 	 * The TPM needs some time to clean up here,
 	 * so we sleep rather than keeping the bus busy
 	 */
 	udelay(2000);
 	release_locality(chip, chip->vendor.locality, 0);
-
+	tpm_deselect(chip);
 	return rc;
 }
 
-static struct tpm_vendor_specific tpm_tis_i2c = {
-	.status = tpm_tis_i2c_status,
-	.recv = tpm_tis_i2c_recv,
-	.send = tpm_tis_i2c_send,
-	.cancel = tpm_tis_i2c_ready,
+static struct tpm_vendor_specific tpm_i2c_tis = {
+	.status = tpm_i2c_tis_status,
+	.recv = tpm_i2c_tis_recv,
+	.send = tpm_i2c_tis_send,
+	.cancel = tpm_i2c_tis_ready,
 	.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = TPM_STS_COMMAND_READY,
@@ -597,17 +618,13 @@ static enum i2c_chip_type tpm_vendor_chip_type(void)
 	return UNKNOWN;
 }
 
-static int tpm_vendor_init_common(void)
+static int tpm_vendor_init_common(struct tpm_chip *chip)
 {
-	struct tpm_chip *chip;
 	u32 vendor;
 	u32 expected_did_vid;
+	struct tpm_dev *tpm_dev = TPM_VPRIV(chip);
 
-	tpm_dev.chip_type = tpm_vendor_chip_type();
-
-	chip = tpm_register_hardware(&tpm_tis_i2c);
-	if (chip < 0)
-		return -ENODEV;
+	tpm_dev->chip_type = tpm_vendor_chip_type();
 
 	/* Disable interrupts (not supported) */
 	chip->vendor.irq = 0;
@@ -622,12 +639,12 @@ static int tpm_vendor_init_common(void)
 		return  -ENODEV;
 
 	/* Read four bytes from DID_VID register */
-	if (iic_tpm_read(TPM_DID_VID(0), (uchar *)&vendor, 4) < 0) {
+	if (iic_tpm_read(chip, TPM_DID_VID(0), (uchar *)&vendor, 4) < 0) {
 		release_locality(chip, 0, 1);
 		return -EIO;
 	}
 
-	if (tpm_dev.chip_type == SLB9635) {
+	if (tpm_dev->chip_type == SLB9635) {
 		vendor = be32_to_cpu(vendor);
 		expected_did_vid = TPM_TIS_I2C_DID_VID_9635;
 	} else {
@@ -635,13 +652,13 @@ static int tpm_vendor_init_common(void)
 		expected_did_vid = TPM_TIS_I2C_DID_VID_9645;
 	}
 
-	if (tpm_dev.chip_type != UNKNOWN && vendor != expected_did_vid) {
+	if (tpm_dev->chip_type != UNKNOWN && vendor != expected_did_vid) {
 		error("Vendor id did not match! ID was %08x\n", vendor);
 		return -ENODEV;
 	}
 
 	debug("1.2 TPM (chip type %s device-id 0x%X)\n",
-	      chip_name[tpm_dev.chip_type], vendor >> 16);
+	      chip_name[tpm_dev->chip_type], vendor >> 16);
 
 	/*
 	 * A timeout query to TPM can be placed here.
@@ -651,33 +668,79 @@ static int tpm_vendor_init_common(void)
 	return 0;
 }
 
-#ifdef CONFIG_DM_I2C
-/* Initialisation of i2c tpm */
-int tpm_vendor_init_dev(struct udevice *dev)
-{
-	tpm_dev.dev = dev;
-	return tpm_vendor_init_common();
-}
-#else
-/* Initialisation of i2c tpm */
-int tpm_vendor_init(uint32_t dev_addr)
+static int tpm_i2c_tis_probe(struct udevice *dev)
 {
-	uint old_addr;
-	int rc = 0;
+	struct tpm_chip *chip;
+	struct tpm_dev *tpm_dev = dev_get_priv(dev);
+	struct tpm_i2c_tis_platdata *platdata = dev_get_platdata(dev);
+
+	chip = tpm_register_hardware(dev, &tpm_i2c_tis);
+	if (chip < 0)
+		return -ENODEV;
 
-	old_addr = tpm_dev.addr;
-	if (dev_addr != 0)
-		tpm_dev.addr = dev_addr;
+	TPM_VPRIV(chip) = tpm_dev;
 
-	rc = tpm_vendor_init_common();
-	if (rc)
-		tpm_dev.addr = old_addr;
+	tpm_dev->slave_addr = platdata->slave_addr;
+	tpm_dev->i2c_bus = platdata->i2c_bus;
 
-	return rc;
+	return tpm_vendor_init_common(chip);
 }
-#endif
 
-void tpm_vendor_cleanup(struct tpm_chip *chip)
+static int tpm_i2c_tis_remove(struct udevice *dev)
 {
+	struct tpm_chip *chip = dev_get_uclass_priv(dev);
+
 	release_locality(chip, chip->vendor.locality, 1);
+	return 0;
 }
+
+#ifdef CONFIG_CONTROL_OF
+static const struct udevice_id tpm_i2c_tis_ids[] = {
+	{ .compatible = "infineon,tpm_i2c_infineon"},
+	{ .compatible = "infineon,slb9635tt"},
+	{ .compatible = "infineon,slb9645tt"},
+	{},
+};
+
+static int tpm_i2c_tis_ofdata_to_platdata(struct udevice *dev)
+{
+	int parent, node, i2c_bus;
+	const void *blob = gd->fdt_blob;
+	struct tpm_tis_platdata *platdata = dev_get_platdata(dev);
+
+	node = fdtdec_next_compatible(blob, 0, COMPAT_INFINEON_SLB9635_TPM);
+	if (node < 0) {
+		node = fdtdec_next_compatible(blob, 0,
+				COMPAT_INFINEON_SLB9645_TPM);
+	}
+	if (node < 0) {
+		debug("%s: Node not found\n", __func__);
+		return -1;
+	}
+	parent = fdt_parent_offset(blob, node);
+	if (parent < 0) {
+		debug("%s: Cannot find node parent\n", __func__);
+		return -1;
+	}
+
+	i2c_bus = i2c_get_bus_num_fdt(parent);
+	if (i2c_bus < 0)
+		return -1;
+
+	platdata->i2c_bus = i2c_bus;
+	platdata->slave_addr = fdtdec_get_addr(blob, node, "reg");
+
+	return 0;
+}
+#endif
+
+U_BOOT_DRIVER(tpm_i2c_infineon) = {
+	.name   = "tpm_i2c_infineon",
+	.id     = UCLASS_TPM,
+	.of_match = of_match_ptr(tpm_i2c_tis_ids),
+	.ofdata_to_platdata = of_match_ptr(tpm_i2c_tis_ofdata_to_platdata),
+	.probe  = tpm_i2c_tis_probe,
+	.remove = tpm_i2c_tis_remove,
+	.priv_auto_alloc_size = sizeof(struct tpm_dev),
+	.platdata_auto_alloc_size = sizeof(struct tpm_i2c_tis_platdata),
+};
diff --git a/drivers/tpm/tpm_private.h b/drivers/tpm/tpm_private.h
index 8894c98..1f5f53f 100644
--- a/drivers/tpm/tpm_private.h
+++ b/drivers/tpm/tpm_private.h
@@ -52,6 +52,15 @@ enum tpm_timeout {
 
 struct tpm_chip;
 
+struct dm_tpm_ops {
+	int (*init)(struct udevice *);
+	int (*open)(struct udevice *);
+	int (*close)(struct udevice *);
+	int (*sendrecv)(struct udevice *,
+			const uint8_t *, size_t,
+			uint8_t *, size_t *);
+};
+
 struct tpm_vendor_specific {
 	const u8 req_complete_mask;
 	const u8 req_complete_val;
@@ -64,8 +73,11 @@ struct tpm_vendor_specific {
 	int locality;
 	unsigned long timeout_a, timeout_b, timeout_c, timeout_d;  /* msec */
 	unsigned long duration[3];  /* msec */
+	void *priv;
 };
 
+#define TPM_VPRIV(c)     ((c)->vendor.priv)
+
 struct tpm_chip {
 	int is_open;
 	struct tpm_vendor_specific vendor;
@@ -127,14 +139,7 @@ struct tpm_cmd_t {
 	union tpm_cmd_params params;
 } __packed;
 
-struct tpm_chip *tpm_register_hardware(const struct tpm_vendor_specific *);
-
-int tpm_vendor_init(uint32_t dev_addr);
-
-struct udevice;
-int tpm_vendor_init_dev(struct udevice *dev);
-
-void tpm_vendor_cleanup(struct tpm_chip *chip);
-
+struct tpm_chip *tpm_register_hardware(struct udevice *dev,
+				const struct tpm_vendor_specific *);
 
 #endif
diff --git a/include/dm/platform_data/tpm_i2c_infineon.h b/include/dm/platform_data/tpm_i2c_infineon.h
new file mode 100644
index 0000000..4f9d7e6
--- /dev/null
+++ b/include/dm/platform_data/tpm_i2c_infineon.h
@@ -0,0 +1,23 @@
+/*
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __TPM_TIS_I2C_H__
+#define __TPM_TIS_I2C_H__
+
+struct tpm_i2c_tis_platdata {
+	int i2c_bus;
+	uint8_t slave_addr;
+} __packed;
+
+#endif
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index c744044..031daf2 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -58,6 +58,7 @@ enum uclass_id {
 	UCLASS_USB_DEV_GENERIC,	/* USB generic device */
 	UCLASS_USB_HUB,		/* USB hub */
 	UCLASS_VIDEO_BRIDGE,	/* Video bridge, e.g. DisplayPort to LVDS */
+	UCLASS_TPM,		/* TPM */
 
 	UCLASS_COUNT,
 	UCLASS_INVALID = -1,
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/3] tpm: Add st33zp24 tpm with i2c and spi phy
  2015-08-09 13:19 [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs Christophe Ricard
  2015-08-09 13:19 ` [U-Boot] [PATCH 1/3] tpm: Move tpm_tis_i2c to tpm_i2c_infineon Christophe Ricard
  2015-08-09 13:19 ` [U-Boot] [PATCH 2/3] tpm: Initial work to introduce TPM driver model Christophe Ricard
@ 2015-08-09 13:19 ` Christophe Ricard
  2015-08-13 15:55   ` Simon Glass
  2015-08-09 13:28 ` [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs Simon Glass
  3 siblings, 1 reply; 17+ messages in thread
From: Christophe Ricard @ 2015-08-09 13:19 UTC (permalink / raw
  To: u-boot

Add TPM st33zp24 tpm with i2c and spi phy. This is a port from Linux.
This driver relies on tpm uclass.

Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
---

 README                                  |  11 +
 drivers/tpm/Makefile                    |   1 +
 drivers/tpm/st33zp24/Makefile           |   7 +
 drivers/tpm/st33zp24/i2c.c              | 226 ++++++++++++++++
 drivers/tpm/st33zp24/spi.c              | 286 ++++++++++++++++++++
 drivers/tpm/st33zp24/st33zp24.c         | 454 ++++++++++++++++++++++++++++++++
 drivers/tpm/st33zp24/st33zp24.h         |  29 ++
 include/dm/platform_data/st33zp24_i2c.h |  28 ++
 include/dm/platform_data/st33zp24_spi.h |  30 +++
 include/fdtdec.h                        |   5 +-
 lib/fdtdec.c                            |   2 +
 11 files changed, 1078 insertions(+), 1 deletion(-)
 create mode 100644 drivers/tpm/st33zp24/Makefile
 create mode 100644 drivers/tpm/st33zp24/i2c.c
 create mode 100644 drivers/tpm/st33zp24/spi.c
 create mode 100644 drivers/tpm/st33zp24/st33zp24.c
 create mode 100644 drivers/tpm/st33zp24/st33zp24.h
 create mode 100644 include/dm/platform_data/st33zp24_i2c.h
 create mode 100644 include/dm/platform_data/st33zp24_spi.h

diff --git a/README b/README
index 506ff6c..d3f9590 100644
--- a/README
+++ b/README
@@ -1499,6 +1499,17 @@ The following options need to be configured:
 			CONFIG_TPM_TIS_I2C_BURST_LIMITATION
 			Define the burst count bytes upper limit
 
+		CONFIG_TPM_ST33ZP24
+		Support for STMicroelectronics TPM devices. Requires DM_TPM support.
+
+			CONFIG_TPM_ST33ZP24_I2C
+			Support for STMicroelectronics ST33ZP24 I2C devices.
+			Requires TPM_ST33ZP24 and I2C.
+
+			CONFIG_TPM_ST33ZP24_SPI
+			Support for STMicroelectronics ST33ZP24 SPI devices.
+			Requires TPM_ST33ZP24 and SPI.
+
 		CONFIG_TPM_ATMEL_TWI
 		Support for Atmel TWI TPM device. Requires I2C support.
 
diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
index bd2cd6d..48bc5f3 100644
--- a/drivers/tpm/Makefile
+++ b/drivers/tpm/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_DM_TPM) += tpm.o
 obj-$(CONFIG_TPM_INFINEON_I2C) += tpm_i2c_infineon.o
 obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
 obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o
+obj-$(CONFIG_TPM_ST33ZP24) += st33zp24/
diff --git a/drivers/tpm/st33zp24/Makefile b/drivers/tpm/st33zp24/Makefile
new file mode 100644
index 0000000..ed28e8c
--- /dev/null
+++ b/drivers/tpm/st33zp24/Makefile
@@ -0,0 +1,7 @@
+#
+# Makefile for ST33ZP24 TPM 1.2 driver
+#
+
+obj-$(CONFIG_TPM_ST33ZP24) += st33zp24.o
+obj-$(CONFIG_TPM_ST33ZP24_I2C) += i2c.o
+obj-$(CONFIG_TPM_ST33ZP24_SPI) += spi.o
diff --git a/drivers/tpm/st33zp24/i2c.c b/drivers/tpm/st33zp24/i2c.c
new file mode 100644
index 0000000..204021a
--- /dev/null
+++ b/drivers/tpm/st33zp24/i2c.c
@@ -0,0 +1,226 @@
+/*
+ * STMicroelectronics TPM ST33ZP24 I2C phy for UBOOT
+ * Copyright (C) 2015  STMicroelectronics
+ *
+ * Description: Device driver for ST33ZP24 I2C 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 I2C Protocol Stack Specification version 1.2.0.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <i2c.h>
+#include <dm.h>
+#include <linux/types.h>
+#include <malloc.h>
+#include <tpm.h>
+#include <errno.h>
+#include <asm/unaligned.h>
+#include <dm/platform_data/st33zp24_i2c.h>
+
+#include "../tpm_private.h"
+#include "st33zp24.h"
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define TPM_DUMMY_BYTE	0xAA
+#define TPM_ST33ZP24_I2C_SLAVE_ADDR 0x13
+
+struct st33zp24_i2c_phy {
+	uint8_t slave_addr;
+	int i2c_bus;
+	int old_bus;
+	u8 buf[TPM_BUFSIZE + 1];
+} __packed;
+
+/*
+ * write8_reg
+ * Send byte to the TIS register according to the ST33ZP24 I2C protocol.
+ * @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: Number of byte written successfully else an error code.
+ */
+static int write8_reg(void *phy_id, u8 tpm_register, u8 *tpm_data, u16 tpm_size)
+{
+	struct st33zp24_i2c_phy *phy = phy_id;
+	int ret;
+	phy->buf[0] = tpm_register;
+	memcpy(phy->buf + 1, tpm_data, tpm_size);
+	ret = i2c_write(phy->slave_addr, 0, 0, phy->buf, tpm_size + 1);
+	if (!ret)
+		return tpm_size + 1;
+	return ret;
+} /* write8_reg() */
+
+/*
+* read8_reg
+* Recv byte from the TIS register according to the ST33ZP24 I2C protocol.
+* @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: Number of byte read successfully else an error code.
+*/
+static int read8_reg(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size)
+{
+	struct st33zp24_i2c_phy *phy = phy_id;
+	int status;
+	u8 data;
+
+	data = TPM_DUMMY_BYTE;
+	status = write8_reg(phy, tpm_register, &data, 1);
+	if (status == 2) {
+		status = i2c_read(phy->slave_addr, 0, 0, tpm_data, tpm_size);
+		if (!status)
+			return tpm_size;
+	}
+	return status;
+} /* read8_reg() */
+
+static int tpm_select(void *phy_id)
+{
+	int ret;
+	struct st33zp24_i2c_phy *phy = phy_id;
+
+	phy->old_bus = i2c_get_bus_num();
+	if (phy->old_bus != phy->i2c_bus) {
+		ret = i2c_set_bus_num(phy->i2c_bus);
+		if (ret) {
+			debug("%s: Fail to set i2c bus %d\n", __func__,
+			      phy->i2c_bus);
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+static int tpm_deselect(void *phy_id)
+{
+	int ret;
+	struct st33zp24_i2c_phy *phy = phy_id;
+
+	if (phy->old_bus != i2c_get_bus_num()) {
+		ret = i2c_set_bus_num(phy->old_bus);
+		if (ret) {
+			debug("%s: Fail to restore i2c bus %d\n",
+			      __func__, phy->old_bus);
+			return -1;
+		}
+	}
+	phy->old_bus = -1;
+
+	return 0;
+}
+
+/*
+ * st33zp24_i2c_send
+ * Send byte to the TIS register according to the ST33ZP24 I2C protocol.
+ * @param: phy_id, the phy 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: number of byte written successfully: should be one if success.
+ */
+static int st33zp24_i2c_send(void *phy_id, u8 tpm_register, u8 *tpm_data,
+			     int tpm_size)
+{
+	int rc;
+
+	tpm_select(phy_id);
+	rc = write8_reg(phy_id, tpm_register | TPM_WRITE_DIRECTION, tpm_data,
+			tpm_size);
+	tpm_deselect(phy_id);
+	return rc;
+}
+
+/*
+ * st33zp24_i2c_recv
+ * Recv byte from the TIS register according to the ST33ZP24 I2C protocol.
+ * @param: phy_id, the phy description
+ * @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: number of byte read successfully: should be one if success.
+ */
+static int st33zp24_i2c_recv(void *phy_id, u8 tpm_register, u8 *tpm_data,
+			     int tpm_size)
+{
+	int rc;
+
+	tpm_select(phy_id);
+	rc = read8_reg(phy_id, tpm_register, tpm_data, tpm_size);
+	tpm_deselect(phy_id);
+	return rc;
+}
+
+static const struct st33zp24_phy_ops i2c_phy_ops = {
+	.send = st33zp24_i2c_send,
+	.recv = st33zp24_i2c_recv,
+};
+
+static int st33zp24_i2c_probe(struct udevice *dev)
+{
+	struct st33zp24_i2c_platdata *platdata = dev_get_platdata(dev);
+	struct st33zp24_i2c_phy *i2c_phy = dev_get_priv(dev);
+
+	i2c_phy->slave_addr = platdata->slave_addr;
+	i2c_phy->i2c_bus = platdata->i2c_bus;
+
+	return st33zp24_init(dev, i2c_phy, &i2c_phy_ops);
+}
+
+static int st33zp24_i2c_remove(struct udevice *dev)
+{
+	return st33zp24_remove(dev);
+}
+
+#ifdef CONFIG_OF_CONTROL
+static const struct udevice_id st33zp24_i2c_ids[] = {
+	{ .compatible = "st,st33zp24-i2c" },
+	{ }
+};
+
+static int st33zp24_i2c_ofdata_to_platdata(struct udevice *dev)
+{
+	int node, parent, i2c_bus;
+	const void *blob = gd->fdt_blob;
+	struct st33zp24_i2c_platdata *platdata = dev_get_platdata(dev);
+
+	node = fdtdec_next_compatible(blob, 0,
+				COMPAT_STMICROELECTRONICS_ST33ZP24_I2C);
+	if (node < 0) {
+		debug("%s: Node not found\n", __func__);
+		return -1;
+	}
+
+	parent = fdt_parent_offset(blob, node);
+	if (parent < 0) {
+		debug("%s: Cannot find node parent\n", __func__);
+		return -1;
+	}
+
+	i2c_bus = i2c_get_bus_num_fdt(parent);
+	if (i2c_bus < 0)
+		return -1;
+
+	platdata->i2c_bus = i2c_bus;
+	platdata->slave_addr = fdtdec_get_addr(blob, node, "reg");
+
+	return 0;
+}
+#endif
+
+U_BOOT_DRIVER(st33zp24_i2c) = {
+	.name   = "st33zp24-i2c",
+	.id     = UCLASS_TPM,
+	.of_match = of_match_ptr(st33zp24_i2c_ids),
+	.ofdata_to_platdata = of_match_ptr(st33zp24_i2c_ofdata_to_platdata),
+	.probe  = st33zp24_i2c_probe,
+	.remove = st33zp24_i2c_remove,
+	.priv_auto_alloc_size = sizeof(struct st33zp24_i2c_phy),
+	.platdata_auto_alloc_size = sizeof(struct st33zp24_i2c_platdata),
+};
diff --git a/drivers/tpm/st33zp24/spi.c b/drivers/tpm/st33zp24/spi.c
new file mode 100644
index 0000000..312ea07
--- /dev/null
+++ b/drivers/tpm/st33zp24/spi.c
@@ -0,0 +1,286 @@
+/*
+ * STMicroelectronics TPM ST33ZP24 SPI phy for UBOOT
+ * Copyright (C) 2015  STMicroelectronics
+ *
+ * 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+
+ */
+
+#include <common.h>
+#include <spi.h>
+#include <dm.h>
+#include <linux/types.h>
+#include <tpm.h>
+#include <errno.h>
+#include <asm/unaligned.h>
+#include <dm/platform_data/st33zp24_spi.h>
+
+#include "../tpm_private.h"
+#include "st33zp24.h"
+
+#define TPM_DATA_FIFO				0x24
+#define TPM_INTF_CAPABILITY			0x14
+
+#define TPM_DUMMY_BYTE				0x00
+#define TPM_WRITE_DIRECTION			0x80
+
+#define MAX_SPI_LATENCY				15
+#define LOCALITY0				0
+
+#define ST33ZP24_OK					0x5A
+#define ST33ZP24_UNDEFINED_ERR				0x80
+#define ST33ZP24_BADLOCALITY				0x81
+#define ST33ZP24_TISREGISTER_UKNOWN			0x82
+#define ST33ZP24_LOCALITY_NOT_ACTIVATED			0x83
+#define ST33ZP24_HASH_END_BEFORE_HASH_START		0x84
+#define ST33ZP24_BAD_COMMAND_ORDER			0x85
+#define ST33ZP24_INCORECT_RECEIVED_LENGTH		0x86
+#define ST33ZP24_TPM_FIFO_OVERFLOW			0x89
+#define ST33ZP24_UNEXPECTED_READ_FIFO			0x8A
+#define ST33ZP24_UNEXPECTED_WRITE_FIFO			0x8B
+#define ST33ZP24_CMDRDY_SET_WHEN_PROCESSING_HASH_END	0x90
+#define ST33ZP24_DUMMY_BYTES				0x00
+
+/*
+ * TPM command can be up to 2048 byte, A TPM response can be up to
+ * 1024 byte.
+ * Between command and response, there are latency byte (up to 15
+ * usually on st33zp24 2 are enough).
+ *
+ * Overall when sending a command and expecting an answer we need if
+ * worst case:
+ * 2048 (for the TPM command) + 1024 (for the TPM answer).  We need
+ * some latency byte before the answer is available (max 15).
+ * We have 2048 + 1024 + 15.
+ */
+#define ST33ZP24_SPI_BUFFER_SIZE (TPM_BUFSIZE + (TPM_BUFSIZE / 2) +\
+				  MAX_SPI_LATENCY)
+
+struct st33zp24_spi_phy {
+	struct spi_slave *spi_slave;
+	int latency;
+
+	u8 tx_buf[ST33ZP24_SPI_BUFFER_SIZE];
+	u8 rx_buf[ST33ZP24_SPI_BUFFER_SIZE];
+};
+
+static int st33zp24_status_to_errno(u8 code)
+{
+	switch (code) {
+	case ST33ZP24_OK:
+		return 0;
+	case ST33ZP24_UNDEFINED_ERR:
+	case ST33ZP24_BADLOCALITY:
+	case ST33ZP24_TISREGISTER_UKNOWN:
+	case ST33ZP24_LOCALITY_NOT_ACTIVATED:
+	case ST33ZP24_HASH_END_BEFORE_HASH_START:
+	case ST33ZP24_BAD_COMMAND_ORDER:
+	case ST33ZP24_UNEXPECTED_READ_FIFO:
+	case ST33ZP24_UNEXPECTED_WRITE_FIFO:
+	case ST33ZP24_CMDRDY_SET_WHEN_PROCESSING_HASH_END:
+		return -EPROTO;
+	case ST33ZP24_INCORECT_RECEIVED_LENGTH:
+	case ST33ZP24_TPM_FIFO_OVERFLOW:
+		return -EMSGSIZE;
+	case ST33ZP24_DUMMY_BYTES:
+		return -ENOSYS;
+	}
+	return code;
+}
+
+/*
+ * st33zp24_spi_send
+ * Send byte to TPM register according to the ST33ZP24 SPI protocol.
+ * @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 st33zp24_spi_send(void *phy_id, u8 tpm_register, u8 *tpm_data,
+			     int tpm_size)
+{
+	int total_length = 0, ret;
+	struct st33zp24_spi_phy *phy = phy_id;
+
+	struct spi_slave *dev = phy->spi_slave;
+	u8 *tx_buf = (u8 *)phy->tx_buf;
+	u8 *rx_buf = phy->rx_buf;
+
+	tx_buf[total_length++] = TPM_WRITE_DIRECTION | LOCALITY0;
+	tx_buf[total_length++] = tpm_register;
+
+	if (tpm_size > 0 && tpm_register == TPM_DATA_FIFO) {
+		tx_buf[total_length++] = tpm_size >> 8;
+		tx_buf[total_length++] = tpm_size;
+	}
+	memcpy(tx_buf + total_length, tpm_data, tpm_size);
+	total_length += tpm_size;
+
+	memset(tx_buf + total_length, TPM_DUMMY_BYTE, phy->latency);
+
+	total_length += phy->latency;
+
+	spi_claim_bus(dev);
+	ret = spi_xfer(dev, total_length * 8, tx_buf, rx_buf,
+		       SPI_XFER_BEGIN | SPI_XFER_END);
+	spi_release_bus(dev);
+
+	if (ret == 0)
+		ret = rx_buf[total_length - 1];
+
+	return st33zp24_status_to_errno(ret);
+} /* st33zp24_spi_send() */
+
+/*
+ * 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 read8_reg(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size)
+{
+	int total_length = 0, nbr_dummy_bytes, ret;
+	struct st33zp24_spi_phy *phy = phy_id;
+	struct spi_slave *dev = phy->spi_slave;
+	u8 *tx_buf = (u8 *)phy->tx_buf;
+	u8 *rx_buf = phy->rx_buf;
+
+	/* Pre-Header */
+	tx_buf[total_length++] = LOCALITY0;
+	tx_buf[total_length++] = tpm_register;
+
+	nbr_dummy_bytes = phy->latency;
+	memset(&tx_buf[total_length], TPM_DUMMY_BYTE,
+	       nbr_dummy_bytes + tpm_size);
+	total_length += nbr_dummy_bytes + tpm_size;
+
+	spi_claim_bus(dev);
+	ret = spi_xfer(dev, total_length * 8, tx_buf, rx_buf,
+		       SPI_XFER_BEGIN | SPI_XFER_END);
+	spi_release_bus(dev);
+
+	if (tpm_size > 0 && ret == 0) {
+		ret = rx_buf[total_length - tpm_size - 1];
+		memcpy(tpm_data, rx_buf + total_length - tpm_size, tpm_size);
+	}
+	return ret;
+} /* read8_reg() */
+
+/*
+ * st33zp24_spi_recv
+ * Recv byte from the TIS register according to the ST33ZP24 SPI protocol.
+ * @param: phy_id, the phy description
+ * @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: number of byte read successfully: should be one if success.
+ */
+static int st33zp24_spi_recv(void *phy_id, u8 tpm_register, u8 *tpm_data,
+			     int tpm_size)
+{
+	int ret;
+
+	ret = read8_reg(phy_id, tpm_register, tpm_data, tpm_size);
+	if (!st33zp24_status_to_errno(ret))
+		return tpm_size;
+	return ret;
+} /* st33zp24_spi_recv() */
+
+static int evaluate_latency(void *phy_id)
+{
+	struct st33zp24_spi_phy *phy = phy_id;
+	int latency = 1, status = 0;
+	u8 data = 0;
+
+	while (!status && latency < MAX_SPI_LATENCY) {
+		phy->latency = latency;
+		status = read8_reg(phy_id, TPM_INTF_CAPABILITY, &data, 1);
+		latency++;
+	}
+	return latency - 1;
+} /* evaluate_latency() */
+
+static const struct st33zp24_phy_ops spi_phy_ops = {
+	.send = st33zp24_spi_send,
+	.recv = st33zp24_spi_recv,
+};
+
+static int st33zp24_spi_probe(struct udevice *dev)
+{
+	struct st33zp24_spi_platdata *platdata = dev_get_platdata(dev);
+	struct st33zp24_spi_phy *spi_phy = dev_get_priv(dev);
+
+	#ifndef CONFIG_OF_CONTROL
+	spi_phy->spi_slave = spi_setup_slave(platdata->bus_num,
+					     platdata->cs,
+					     platdata->max_speed,
+					     platdata->mode);
+	#endif
+
+	spi_phy->latency = evaluate_latency(spi_phy);
+	if (spi_phy->latency <= 0)
+		return -ENODEV;
+
+	return st33zp24_init(dev, spi_phy, &spi_phy_ops);
+} /* st33zp24_spi_probe() */
+
+static int st33zp24_spi_remove(struct udevice *dev)
+{
+	return st33zp24_remove(dev);
+}
+
+#ifdef CONFIG_OF_CONTROL
+static const struct udevice_id st33zp24_spi_ids[] = {
+	{ .compatible = "st,st33zp24-spi" },
+	{ }
+};
+
+static int st33zp24_spi_ofdata_to_platdata(struct udevice *dev)
+{
+	int node, parent;
+	int i2c_bus;
+	const void *blob = gd->fdt_blob;
+	struct st33zp24_spi_platdata *platdata = dev_get_platdata(dev);
+	struct st33zp24_spi_phy *spi_phy = dev_get_priv(dev);
+
+	node = fdtdec_next_compatible(blob, 0,
+				COMPAT_STMICROELECTRONICS_ST33ZP24_SPI);
+	if (node < 0) {
+		debug("%s: Node not found\n", __func__);
+		return -1;
+	}
+
+	parent = fdt_parent_offset(blob, node);
+	if (parent < 0) {
+		debug("%s: Cannot find node parent\n", __func__);
+		return -1;
+	}
+
+	spi_phy->spi_slave = spi_setup_slave_fdt(blob, node, parent);
+	if (!spi_phy->spi_slave)
+		return -1;
+
+	return 0;
+}
+#endif
+
+U_BOOT_DRIVER(st33zp24_spi) = {
+	.name   = "st33zp24-spi",
+	.id     = UCLASS_TPM,
+	.of_match = of_match_ptr(st33zp24_spi_ids),
+	.ofdata_to_platdata = of_match_ptr(st33zp24_spi_ofdata_to_platdata),
+	.probe  = st33zp24_spi_probe,
+	.remove = st33zp24_spi_remove,
+	.priv_auto_alloc_size = sizeof(struct st33zp24_spi_phy),
+	.platdata_auto_alloc_size = sizeof(struct st33zp24_spi_platdata),
+};
diff --git a/drivers/tpm/st33zp24/st33zp24.c b/drivers/tpm/st33zp24/st33zp24.c
new file mode 100644
index 0000000..862b98a
--- /dev/null
+++ b/drivers/tpm/st33zp24/st33zp24.c
@@ -0,0 +1,454 @@
+/*
+ * STMicroelectronics TPM ST33ZP24 UBOOT driver
+ *
+ * Copyright (C) 2015  STMicroelectronics
+ *
+ * Description: Device driver for ST33ZP24 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 Protocol Stack Specification version 1.2.0.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <linux/types.h>
+#include <linux/compat.h>
+#include <malloc.h>
+#include <tpm.h>
+#include <dm.h>
+#include <errno.h>
+#include <asm/unaligned.h>
+
+#include "st33zp24.h"
+#include "../tpm_private.h"
+
+#define TPM_ACCESS			0x0
+#define TPM_STS				0x18
+#define TPM_DATA_FIFO			0x24
+
+#define LOCALITY0			0
+
+enum st33zp24_access {
+	TPM_ACCESS_VALID = 0x80,
+	TPM_ACCESS_ACTIVE_LOCALITY = 0x20,
+	TPM_ACCESS_REQUEST_PENDING = 0x04,
+	TPM_ACCESS_REQUEST_USE = 0x02,
+};
+
+enum st33zp24_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 st33zp24_int_flags {
+	TPM_GLOBAL_INT_ENABLE = 0x80,
+	TPM_INTF_CMD_READY_INT = 0x080,
+	TPM_INTF_FIFO_AVALAIBLE_INT = 0x040,
+	TPM_INTF_WAKE_UP_READY_INT = 0x020,
+	TPM_INTF_LOCTPM_BUFSIZE4SOFTRELEASE_INT = 0x008,
+	TPM_INTF_LOCALITY_CHANGE_INT = 0x004,
+	TPM_INTF_STS_VALID_INT = 0x002,
+	TPM_INTF_DATA_AVAIL_INT = 0x001,
+};
+
+enum tis_defaults {
+	TIS_SHORT_TIMEOUT = 750,	/* ms */
+	TIS_LONG_TIMEOUT = 2000,	/* 2 sec */
+};
+
+struct st33zp24_dev {
+	void *phy_id;
+	const struct st33zp24_phy_ops *ops;
+};
+
+/*
+ * release_locality release the active locality
+ * @param: chip, the tpm chip description.
+ */
+static void release_locality(struct tpm_chip *chip)
+{
+	struct st33zp24_dev *tpm_dev;
+	u8 data = TPM_ACCESS_ACTIVE_LOCALITY;
+
+	tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
+
+	tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
+} /* release_locality() */
+
+/*
+ * check_locality if the locality is active
+ * @param: chip, the tpm chip description
+ * @return: the active locality or -EACCES.
+ */
+static int check_locality(struct tpm_chip *chip)
+{
+	struct st33zp24_dev *tpm_dev;
+	u8 data;
+	u8 status;
+
+	tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
+
+	status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
+	if (status && (data &
+		(TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) ==
+		(TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID))
+		return chip->vendor.locality;
+
+	return -EACCES;
+} /* check_locality() */
+
+/*
+ * request_locality request the TPM locality
+ * @param: chip, the chip description
+ * @return: the active locality or negative value.
+ */
+static int request_locality(struct tpm_chip *chip)
+{
+	unsigned long start, stop;
+	long ret;
+	struct st33zp24_dev *tpm_dev;
+	u8 data;
+
+	if (check_locality(chip) == chip->vendor.locality)
+		return chip->vendor.locality;
+
+	tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
+
+	data = TPM_ACCESS_REQUEST_USE;
+	ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
+	if (ret < 0)
+		return ret;
+
+	/* wait for locality activated */
+	start = get_timer(0);
+	stop = chip->vendor.timeout_a;
+	do {
+		if (check_locality(chip) >= 0)
+			return chip->vendor.locality;
+		udelay(TPM_TIMEOUT * 1000);
+	} while	 (get_timer(start) < stop);
+
+	return -EACCES;
+} /* request_locality() */
+
+/*
+ * st33zp24_status return the TPM_STS register
+ * @param: chip, the tpm chip description
+ * @return: the TPM_STS register value.
+ */
+static u8 st33zp24_status(struct tpm_chip *chip)
+{
+	struct st33zp24_dev *tpm_dev;
+	u8 data;
+
+	tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
+
+	tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1);
+	return data;
+} /* st33zp24_status() */
+
+/*
+ * get_burstcount return the burstcount address 0x19 0x1A
+ * @param: chip, the chip description
+ * return: the burstcount or -TPM_DRIVER_ERR in case of error.
+ */
+static int get_burstcount(struct tpm_chip *chip)
+{
+	unsigned long start, stop;
+	int burstcnt, status;
+	u8 tpm_reg, temp;
+	struct st33zp24_dev *tpm_dev;
+
+	tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
+
+	/* wait for burstcount */
+	start = get_timer(0);
+	stop = chip->vendor.timeout_d;
+	do {
+		tpm_reg = TPM_STS + 1;
+		status = tpm_dev->ops->recv(tpm_dev->phy_id, tpm_reg, &temp, 1);
+		if (status < 0)
+			return -EBUSY;
+
+		tpm_reg = TPM_STS + 2;
+		burstcnt = temp;
+		status = tpm_dev->ops->recv(tpm_dev->phy_id, tpm_reg, &temp, 1);
+		if (status < 0)
+			return -EBUSY;
+
+		burstcnt |= temp << 8;
+		if (burstcnt)
+			return burstcnt;
+		udelay(TIS_SHORT_TIMEOUT * 1000);
+	} while (get_timer(start) < stop);
+	return -EBUSY;
+} /* get_burstcount() */
+
+/*
+ * st33zp24_cancel, cancel the current command execution or
+ * set STS to COMMAND READY.
+ * @param: chip, tpm_chip description.
+ */
+static void st33zp24_cancel(struct tpm_chip *chip)
+{
+	struct st33zp24_dev *tpm_dev;
+	u8 data;
+
+	tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
+
+	data = TPM_STS_COMMAND_READY;
+	tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1);
+} /* st33zp24_cancel() */
+
+/*
+ * 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 timeout
+ * @param: status,
+ * @return: the tpm status, 0 if success, -ETIME if timeout is reached.
+ */
+static int wait_for_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
+			 int *status)
+{
+	unsigned long start, stop;
+
+	/* Check current status */
+	*status = st33zp24_status(chip);
+	if ((*status & mask) == mask)
+		return 0;
+
+	start = get_timer(0);
+	stop = timeout;
+	do {
+		udelay(TPM_TIMEOUT * 1000);
+		*status = st33zp24_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: the number of bytes read from TPM FIFO.
+ */
+static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
+{
+	int size = 0, burstcnt, len, ret, status;
+	struct st33zp24_dev *tpm_dev;
+
+	tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
+
+	while (size < count &&
+	       wait_for_stat(chip,
+			     TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+			     chip->vendor.timeout_c, &status) == 0) {
+		burstcnt = get_burstcount(chip);
+		if (burstcnt < 0)
+			return burstcnt;
+		len = min_t(int, burstcnt, count - size);
+		ret = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_DATA_FIFO,
+					 buf + size, len);
+		if (ret < 0)
+			return ret;
+
+		size += len;
+	}
+	return size;
+} /* recv_data() */
+
+/*
+ * st33zp24_recv received TPM response through TPM phy.
+ * @param: chip, tpm_chip description.
+ * @param: buf,	the buffer to store data.
+ * @param: count, the number of bytes that can received (sizeof buf).
+ * @return: Returns zero in case of success else -EIO.
+ */
+static int st33zp24_recv(struct tpm_chip *chip, unsigned char *buf,
+			 size_t count)
+{
+	int size = 0;
+	int expected;
+
+	if (!chip)
+		return -ENODEV;
+
+	if (count < TPM_HEADER_SIZE) {
+		size = -EIO;
+		goto out;
+	}
+
+	size = recv_data(chip, buf, TPM_HEADER_SIZE);
+	if (size < TPM_HEADER_SIZE) {
+		printf("TPM error, unable to read header\n");
+		goto out;
+	}
+
+	expected = get_unaligned_be32(buf + 2);
+	if (expected > count) {
+		size = -EIO;
+		goto out;
+	}
+
+	size += recv_data(chip, &buf[TPM_HEADER_SIZE],
+			  expected - TPM_HEADER_SIZE);
+	if (size < expected) {
+		printf("TPM error, unable to read remaining bytes of result\n");
+		size = -EIO;
+		goto out;
+	}
+
+out:
+	st33zp24_cancel(chip);
+	release_locality(chip);
+	return size;
+} /* st33zp24_recv() */
+
+/*
+ * st33zp24_send send TPM commands through TPM phy.
+ * @param: chip, tpm_chip description.
+ * @param: buf,	the buffer to send.
+ * @param: len, the number of bytes to send.
+ * @return: Returns zero in case of success else the negative error code.
+ */
+static int st33zp24_send(struct tpm_chip *chip, u8 *buf,
+			 size_t len)
+{
+	u32 i, size;
+	int burstcnt, ret, status;
+	u8 data, tpm_stat;
+	struct st33zp24_dev *tpm_dev;
+
+	if (!chip)
+		return -ENODEV;
+	if (len < TPM_HEADER_SIZE)
+		return -EIO;
+
+	tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
+
+	ret = request_locality(chip);
+	if (ret < 0)
+		return ret;
+
+	tpm_stat = st33zp24_status(chip);
+	if ((tpm_stat & TPM_STS_COMMAND_READY) == 0) {
+		st33zp24_cancel(chip);
+		if (wait_for_stat(chip, TPM_STS_COMMAND_READY,
+				  chip->vendor.timeout_b, &status) < 0) {
+			ret = -ETIME;
+			goto out_err;
+		}
+	}
+
+	for (i = 0; i < len - 1;) {
+		burstcnt = get_burstcount(chip);
+		if (burstcnt < 0)
+			return burstcnt;
+		size = min_t(int, len - i - 1, burstcnt);
+		ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_DATA_FIFO,
+					 buf + i, size);
+		if (ret < 0)
+			goto out_err;
+
+		i += size;
+	}
+
+	tpm_stat = st33zp24_status(chip);
+	if ((tpm_stat & TPM_STS_DATA_EXPECT) == 0) {
+		ret = -EIO;
+		goto out_err;
+	}
+
+	ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_DATA_FIFO,
+				 buf + len - 1, 1);
+	if (ret < 0)
+		goto out_err;
+
+	tpm_stat = st33zp24_status(chip);
+	if ((tpm_stat & TPM_STS_DATA_EXPECT) != 0) {
+		ret = -EIO;
+		goto out_err;
+	}
+
+	data = TPM_STS_GO;
+	ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1);
+	if (ret < 0)
+		goto out_err;
+
+	return len;
+
+out_err:
+	st33zp24_cancel(chip);
+	release_locality(chip);
+	return ret;
+} /* st33zp24_send() */
+
+static struct tpm_vendor_specific st33zp24_tpm = {
+	.recv = st33zp24_recv,
+	.send = st33zp24_send,
+	.cancel = st33zp24_cancel,
+	.status = st33zp24_status,
+	.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+	.req_canceled = TPM_STS_COMMAND_READY,
+};
+
+int st33zp24_init(struct udevice *dev, void *phy_id,
+		  const struct st33zp24_phy_ops *ops)
+{
+	int ret;
+	struct tpm_chip *chip;
+	struct st33zp24_dev *tpm_dev;
+
+	chip = tpm_register_hardware(dev, &st33zp24_tpm);
+	if (chip < 0) {
+		ret = -ENODEV;
+		goto out_err;
+	}
+
+	tpm_dev = (struct st33zp24_dev *)malloc(sizeof(struct st33zp24_dev));
+	if (!tpm_dev) {
+		ret = -ENODEV;
+		goto out_err;
+	}
+
+	/* Disable interrupts (not supported) */
+	chip->vendor.irq = 0;
+
+	/* Default timeouts */
+	chip->vendor.timeout_a = TIS_SHORT_TIMEOUT;
+	chip->vendor.timeout_b = TIS_LONG_TIMEOUT;
+	chip->vendor.timeout_c = TIS_SHORT_TIMEOUT;
+	chip->vendor.timeout_d = TIS_SHORT_TIMEOUT;
+
+	chip->vendor.locality = LOCALITY0;
+	TPM_VPRIV(chip) = tpm_dev;
+	tpm_dev->phy_id = phy_id;
+	tpm_dev->ops = ops;
+
+	printf("ST33ZP24 TPM from STMicroelectronics found\n");
+	return 0;
+
+out_err:
+	return ret;
+}
+
+int st33zp24_remove(struct udevice *dev)
+{
+	struct tpm_chip *chip = dev_get_uclass_priv(dev);
+	struct st33zp24_dev *tpm_dev = TPM_VPRIV(chip);
+
+	release_locality(chip);
+	free(tpm_dev);
+	return 0;
+}
diff --git a/drivers/tpm/st33zp24/st33zp24.h b/drivers/tpm/st33zp24/st33zp24.h
new file mode 100644
index 0000000..4be06cb
--- /dev/null
+++ b/drivers/tpm/st33zp24/st33zp24.h
@@ -0,0 +1,29 @@
+/*
+ * STMicroelectronics TPM UBOOT driver for TPM ST33ZP24
+ *
+ * Copyright (C) 2015  STMicroelectronics
+ *
+ * Description: Device driver for ST33ZP24 I2C 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 Protocol Stack Specification version 1.2.0.
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#ifndef __LOCAL_ST33ZP24_H__
+#define __LOCAL_ST33ZP24_H__
+
+#define TPM_WRITE_DIRECTION             0x80
+#define TPM_HEADER_SIZE			10
+
+struct st33zp24_phy_ops {
+	int (*send)(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size);
+	int (*recv)(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size);
+};
+
+int st33zp24_init(struct udevice *dev, void *phy_id,
+		  const struct st33zp24_phy_ops *ops);
+int st33zp24_remove(struct udevice *dev);
+#endif /* __LOCAL_ST33ZP24_H__ */
diff --git a/include/dm/platform_data/st33zp24_i2c.h b/include/dm/platform_data/st33zp24_i2c.h
new file mode 100644
index 0000000..e71c9e3
--- /dev/null
+++ b/include/dm/platform_data/st33zp24_i2c.h
@@ -0,0 +1,28 @@
+/*
+ * STMicroelectronics TPM Linux driver for TPM 1.2 ST33ZP24
+ * Copyright (C) 2009 - 2015  STMicroelectronics
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __ST33ZP24_I2C_H__
+#define __ST33ZP24_I2C_H__
+
+#define TPM_ST33ZP24_I2C		"st33zp24-i2c"
+
+struct st33zp24_i2c_platdata {
+	int i2c_bus;
+	uint8_t slave_addr;
+} __packed;
+
+#endif /* __ST33ZP24_I2C_H__ */
diff --git a/include/dm/platform_data/st33zp24_spi.h b/include/dm/platform_data/st33zp24_spi.h
new file mode 100644
index 0000000..82f560b
--- /dev/null
+++ b/include/dm/platform_data/st33zp24_spi.h
@@ -0,0 +1,30 @@
+/*
+ * STMicroelectronics TPM Linux driver for TPM 1.2 ST33ZP24
+ * Copyright (C) 2009 - 2015  STMicroelectronics
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __ST33ZP24_SPI_H__
+#define __ST33ZP24_SPI_H__
+
+#define TPM_ST33ZP24_SPI		"st33zp24-spi"
+
+struct st33zp24_spi_platdata {
+	int bus_num;
+	int cs;
+	int max_speed;
+	int mode;
+};
+
+#endif /* __ST33ZP24_SPI_H__ */
diff --git a/include/fdtdec.h b/include/fdtdec.h
index eac679e..9eb2b3d 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -182,7 +182,10 @@ enum fdt_compat_id {
 	COMPAT_INTEL_PCH,		/* Intel PCH */
 	COMPAT_INTEL_IRQ_ROUTER,	/* Intel Interrupt Router */
 	COMPAT_ALTERA_SOCFPGA_DWMAC,	/* SoCFPGA Ethernet controller */
-
+	COMPAT_STMICROELECTRONICS_ST33ZP24_I2C,
+					/* STMicroelectronics ST33ZP24 I2C TPM */
+	COMPAT_STMICROELECTRONICS_ST33ZP24_SPI,
+					/* STMicroelectronics ST33ZP24 SPI TPM */
 	COMPAT_COUNT,
 };
 
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index b201787..55c64a0 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -76,6 +76,8 @@ static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(COMPAT_INTEL_PCH, "intel,bd82x6x"),
 	COMPAT(COMPAT_INTEL_IRQ_ROUTER, "intel,irq-router"),
 	COMPAT(ALTERA_SOCFPGA_DWMAC, "altr,socfpga-stmmac"),
+	COMPAT(STMICROELECTRONICS_ST33ZP24_I2C, "st,st33zp24-i2c"),
+	COMPAT(STMICROELECTRONICS_ST33ZP24_SPI, "st,st33zp24-spi"),
 };
 
 const char *fdtdec_get_compatible(enum fdt_compat_id id)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs
  2015-08-09 13:19 [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs Christophe Ricard
                   ` (2 preceding siblings ...)
  2015-08-09 13:19 ` [U-Boot] [PATCH 3/3] tpm: Add st33zp24 tpm with i2c and spi phy Christophe Ricard
@ 2015-08-09 13:28 ` Simon Glass
  2015-08-09 14:19   ` Christophe Ricard
  3 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2015-08-09 13:28 UTC (permalink / raw
  To: u-boot

Hi Christophe,

On 9 August 2015 at 07:19, Christophe Ricard
<christophe.ricard@gmail.com> wrote:
>
> Hi,
>
> This patch serie introduce TPM driver model allowing to instantiate a TPM
> using U_BOOT_DEVICE macro for platform_data or device tree.
>
> As an information, there is no TCG transport protocol official specification
> for i2c for TPM1.2. The TPM uclass allows to support different kind of bus
> (LPC, I2C, SPI) keeping the TPM command duration in common.
>
> Also, this serie introduce TPM1.2 from STMicroelectronics ST33ZP24 with
> I2C and SPI support. It has been ported from existing Linux drivers.
>
> This has been tested on Beagleboard xM.

As it happens I have just been trying to clean up the TPM drivers for
Kconfig and driver model.

Please see u-boot-dm branch tpm-working. I'll hurry up and try to send
out some patches and update the wiki. I wonder if we can join our work
somehow?

Regards,
Simon

>
> Best Regards
> Christophe
>
>
> Christophe Ricard (3):
>   tpm: Move tpm_tis_i2c to tpm_i2c_infineon
>   tpm: Initial work to introduce TPM driver model
>   tpm: Add st33zp24 tpm with i2c and spi phy
>
>  README                                            |  23 +-
>  drivers/tpm/Makefile                              |   5 +-
>  drivers/tpm/st33zp24/Makefile                     |   7 +
>  drivers/tpm/st33zp24/i2c.c                        | 226 +++++++++++
>  drivers/tpm/st33zp24/spi.c                        | 286 ++++++++++++++
>  drivers/tpm/st33zp24/st33zp24.c                   | 454 ++++++++++++++++++++++
>  drivers/tpm/st33zp24/st33zp24.h                   |  29 ++
>  drivers/tpm/tpm.c                                 | 275 +++----------
>  drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} | 271 ++++++++-----
>  drivers/tpm/tpm_private.h                         |  23 +-
>  include/dm/platform_data/st33zp24_i2c.h           |  28 ++
>  include/dm/platform_data/st33zp24_spi.h           |  30 ++
>  include/dm/platform_data/tpm_i2c_infineon.h       |  23 ++
>  include/dm/uclass-id.h                            |   1 +
>  include/fdtdec.h                                  |   5 +-
>  lib/fdtdec.c                                      |   2 +
>  16 files changed, 1351 insertions(+), 337 deletions(-)
>  create mode 100644 drivers/tpm/st33zp24/Makefile
>  create mode 100644 drivers/tpm/st33zp24/i2c.c
>  create mode 100644 drivers/tpm/st33zp24/spi.c
>  create mode 100644 drivers/tpm/st33zp24/st33zp24.c
>  create mode 100644 drivers/tpm/st33zp24/st33zp24.h
>  rename drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} (70%)
>  create mode 100644 include/dm/platform_data/st33zp24_i2c.h
>  create mode 100644 include/dm/platform_data/st33zp24_spi.h
>  create mode 100644 include/dm/platform_data/tpm_i2c_infineon.h
>
> --
> 2.1.4
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs
  2015-08-09 13:28 ` [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs Simon Glass
@ 2015-08-09 14:19   ` Christophe Ricard
  2015-08-09 15:12     ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: Christophe Ricard @ 2015-08-09 14:19 UTC (permalink / raw
  To: u-boot

Hi Simon,
Thanks for pointing me to the right tree and branch. I missed that :-(.

I will go and look at your on-going work and try to fit our STM drivers.

I will be happy to co-work on this.

Best regards
Christophe

Le dim. 9 ao?t 2015 15:28, Simon Glass <sjg@chromium.org> a ?crit :

> Hi Christophe,
>
> On 9 August 2015 at 07:19, Christophe Ricard
> <christophe.ricard@gmail.com> wrote:
> >
> > Hi,
> >
> > This patch serie introduce TPM driver model allowing to instantiate a TPM
> > using U_BOOT_DEVICE macro for platform_data or device tree.
> >
> > As an information, there is no TCG transport protocol official
> specification
> > for i2c for TPM1.2. The TPM uclass allows to support different kind of
> bus
> > (LPC, I2C, SPI) keeping the TPM command duration in common.
> >
> > Also, this serie introduce TPM1.2 from STMicroelectronics ST33ZP24 with
> > I2C and SPI support. It has been ported from existing Linux drivers.
> >
> > This has been tested on Beagleboard xM.
>
> As it happens I have just been trying to clean up the TPM drivers for
> Kconfig and driver model.
>
> Please see u-boot-dm branch tpm-working. I'll hurry up and try to send
> out some patches and update the wiki. I wonder if we can join our work
> somehow?
>
> Regards,
> Simon
>
> >
> > Best Regards
> > Christophe
> >
> >
> > Christophe Ricard (3):
> >   tpm: Move tpm_tis_i2c to tpm_i2c_infineon
> >   tpm: Initial work to introduce TPM driver model
> >   tpm: Add st33zp24 tpm with i2c and spi phy
> >
> >  README                                            |  23 +-
> >  drivers/tpm/Makefile                              |   5 +-
> >  drivers/tpm/st33zp24/Makefile                     |   7 +
> >  drivers/tpm/st33zp24/i2c.c                        | 226 +++++++++++
> >  drivers/tpm/st33zp24/spi.c                        | 286 ++++++++++++++
> >  drivers/tpm/st33zp24/st33zp24.c                   | 454
> ++++++++++++++++++++++
> >  drivers/tpm/st33zp24/st33zp24.h                   |  29 ++
> >  drivers/tpm/tpm.c                                 | 275 +++----------
> >  drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} | 271 ++++++++-----
> >  drivers/tpm/tpm_private.h                         |  23 +-
> >  include/dm/platform_data/st33zp24_i2c.h           |  28 ++
> >  include/dm/platform_data/st33zp24_spi.h           |  30 ++
> >  include/dm/platform_data/tpm_i2c_infineon.h       |  23 ++
> >  include/dm/uclass-id.h                            |   1 +
> >  include/fdtdec.h                                  |   5 +-
> >  lib/fdtdec.c                                      |   2 +
> >  16 files changed, 1351 insertions(+), 337 deletions(-)
> >  create mode 100644 drivers/tpm/st33zp24/Makefile
> >  create mode 100644 drivers/tpm/st33zp24/i2c.c
> >  create mode 100644 drivers/tpm/st33zp24/spi.c
> >  create mode 100644 drivers/tpm/st33zp24/st33zp24.c
> >  create mode 100644 drivers/tpm/st33zp24/st33zp24.h
> >  rename drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} (70%)
> >  create mode 100644 include/dm/platform_data/st33zp24_i2c.h
> >  create mode 100644 include/dm/platform_data/st33zp24_spi.h
> >  create mode 100644 include/dm/platform_data/tpm_i2c_infineon.h
> >
> > --
> > 2.1.4
> >
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs
  2015-08-09 14:19   ` Christophe Ricard
@ 2015-08-09 15:12     ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2015-08-09 15:12 UTC (permalink / raw
  To: u-boot

Hi Chrstophe,

On 9 August 2015 at 08:19, Christophe Ricard
<christophe.ricard@gmail.com> wrote:
> Hi Simon,
> Thanks for pointing me to the right tree and branch. I missed that :-(.

Well I was hoping to send patches next week. I should have sent an
email to the list before starting.

>
> I will go and look at your on-going work and try to fit our STM drivers.
>
> I will be happy to co-work on this.

Great. I'll tidy this up and send out the patches early next week.

Regards,
Simon

>
> Best regards
> Christophe
>
>
> Le dim. 9 ao?t 2015 15:28, Simon Glass <sjg@chromium.org> a ?crit :
>>
>> Hi Christophe,
>>
>> On 9 August 2015 at 07:19, Christophe Ricard
>> <christophe.ricard@gmail.com> wrote:
>> >
>> > Hi,
>> >
>> > This patch serie introduce TPM driver model allowing to instantiate a
>> > TPM
>> > using U_BOOT_DEVICE macro for platform_data or device tree.
>> >
>> > As an information, there is no TCG transport protocol official
>> > specification
>> > for i2c for TPM1.2. The TPM uclass allows to support different kind of
>> > bus
>> > (LPC, I2C, SPI) keeping the TPM command duration in common.
>> >
>> > Also, this serie introduce TPM1.2 from STMicroelectronics ST33ZP24 with
>> > I2C and SPI support. It has been ported from existing Linux drivers.
>> >
>> > This has been tested on Beagleboard xM.
>>
>> As it happens I have just been trying to clean up the TPM drivers for
>> Kconfig and driver model.
>>
>> Please see u-boot-dm branch tpm-working. I'll hurry up and try to send
>> out some patches and update the wiki. I wonder if we can join our work
>> somehow?
>>
>> Regards,
>> Simon
>>
>> >
>> > Best Regards
>> > Christophe
>> >
>> >
>> > Christophe Ricard (3):
>> >   tpm: Move tpm_tis_i2c to tpm_i2c_infineon
>> >   tpm: Initial work to introduce TPM driver model
>> >   tpm: Add st33zp24 tpm with i2c and spi phy
>> >
>> >  README                                            |  23 +-
>> >  drivers/tpm/Makefile                              |   5 +-
>> >  drivers/tpm/st33zp24/Makefile                     |   7 +
>> >  drivers/tpm/st33zp24/i2c.c                        | 226 +++++++++++
>> >  drivers/tpm/st33zp24/spi.c                        | 286 ++++++++++++++
>> >  drivers/tpm/st33zp24/st33zp24.c                   | 454
>> > ++++++++++++++++++++++
>> >  drivers/tpm/st33zp24/st33zp24.h                   |  29 ++
>> >  drivers/tpm/tpm.c                                 | 275 +++----------
>> >  drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} | 271 ++++++++-----
>> >  drivers/tpm/tpm_private.h                         |  23 +-
>> >  include/dm/platform_data/st33zp24_i2c.h           |  28 ++
>> >  include/dm/platform_data/st33zp24_spi.h           |  30 ++
>> >  include/dm/platform_data/tpm_i2c_infineon.h       |  23 ++
>> >  include/dm/uclass-id.h                            |   1 +
>> >  include/fdtdec.h                                  |   5 +-
>> >  lib/fdtdec.c                                      |   2 +
>> >  16 files changed, 1351 insertions(+), 337 deletions(-)
>> >  create mode 100644 drivers/tpm/st33zp24/Makefile
>> >  create mode 100644 drivers/tpm/st33zp24/i2c.c
>> >  create mode 100644 drivers/tpm/st33zp24/spi.c
>> >  create mode 100644 drivers/tpm/st33zp24/st33zp24.c
>> >  create mode 100644 drivers/tpm/st33zp24/st33zp24.h
>> >  rename drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} (70%)
>> >  create mode 100644 include/dm/platform_data/st33zp24_i2c.h
>> >  create mode 100644 include/dm/platform_data/st33zp24_spi.h
>> >  create mode 100644 include/dm/platform_data/tpm_i2c_infineon.h
>> >
>> > --
>> > 2.1.4
>> >

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 1/3] tpm: Move tpm_tis_i2c to tpm_i2c_infineon
  2015-08-09 13:19 ` [U-Boot] [PATCH 1/3] tpm: Move tpm_tis_i2c to tpm_i2c_infineon Christophe Ricard
@ 2015-08-13 15:55   ` Simon Glass
  2015-08-30 22:45     ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2015-08-13 15:55 UTC (permalink / raw
  To: u-boot

On 9 August 2015 at 07:19, Christophe Ricard
<christophe.ricard@gmail.com> wrote:
> As there is no TCG specification or recommendation for i2c TPM 1.2, move
> tpm_tis_i2c driver to tpm_i2c_infineon. Other tpm vendors like atmel or
> stmicroelectronics may have a different transport protocol for i2c.
>
> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
> ---
>
>  README                                            | 4 ++--
>  drivers/tpm/Makefile                              | 2 +-
>  drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} | 0
>  3 files changed, 3 insertions(+), 3 deletions(-)
>  rename drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} (100%)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/3] tpm: Initial work to introduce TPM driver model
  2015-08-09 13:19 ` [U-Boot] [PATCH 2/3] tpm: Initial work to introduce TPM driver model Christophe Ricard
@ 2015-08-13 15:55   ` Simon Glass
  2015-08-13 20:37     ` Christophe Ricard
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2015-08-13 15:55 UTC (permalink / raw
  To: u-boot

Hi Christophe,

On 9 August 2015 at 07:19, Christophe Ricard
<christophe.ricard@gmail.com> wrote:
> drivers/tpm/tpm.c is a TPM core driver port from Linux.
> So far in u-boot only infineon i2c driver is using it but it could fit
> for others...
>
> Introduce a new tpm uclass so that every TPM driver can register against it and
> and take benefit of common functions and data such as tpm_transmit,
> tpm_register_hardware & tpm_remove_hardware.
> Finally tis_init, tis_open, tis_close, tis_sendrecv are using ops allowing
> to introduce proprietary instructions.
> Also this patch convert tpm_i2c_infineon for using this tpm uclass.
>
> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
> ---
>
>  README                                      |   8 +-
>  drivers/tpm/Makefile                        |   2 +-
>  drivers/tpm/tpm.c                           | 275 +++++++---------------------
>  drivers/tpm/tpm_i2c_infineon.c              | 271 ++++++++++++++++-----------
>  drivers/tpm/tpm_private.h                   |  23 ++-
>  include/dm/platform_data/tpm_i2c_infineon.h |  23 +++
>  include/dm/uclass-id.h                      |   1 +
>  7 files changed, 270 insertions(+), 333 deletions(-)
>  create mode 100644 include/dm/platform_data/tpm_i2c_infineon.h

There is a lot going on in this patch - in general I think it is
better to split things up so that each patch does one thing.

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/3] tpm: Add st33zp24 tpm with i2c and spi phy
  2015-08-09 13:19 ` [U-Boot] [PATCH 3/3] tpm: Add st33zp24 tpm with i2c and spi phy Christophe Ricard
@ 2015-08-13 15:55   ` Simon Glass
  2015-08-13 20:59     ` Christophe Ricard
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2015-08-13 15:55 UTC (permalink / raw
  To: u-boot

Hi Christophe,

On 9 August 2015 at 07:19, Christophe Ricard
<christophe.ricard@gmail.com> wrote:
> Add TPM st33zp24 tpm with i2c and spi phy. This is a port from Linux.
> This driver relies on tpm uclass.
>
> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
> ---
>
>  README                                  |  11 +
>  drivers/tpm/Makefile                    |   1 +
>  drivers/tpm/st33zp24/Makefile           |   7 +
>  drivers/tpm/st33zp24/i2c.c              | 226 ++++++++++++++++
>  drivers/tpm/st33zp24/spi.c              | 286 ++++++++++++++++++++
>  drivers/tpm/st33zp24/st33zp24.c         | 454 ++++++++++++++++++++++++++++++++
>  drivers/tpm/st33zp24/st33zp24.h         |  29 ++
>  include/dm/platform_data/st33zp24_i2c.h |  28 ++
>  include/dm/platform_data/st33zp24_spi.h |  30 +++
>  include/fdtdec.h                        |   5 +-
>  lib/fdtdec.c                            |   2 +
>  11 files changed, 1078 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/tpm/st33zp24/Makefile
>  create mode 100644 drivers/tpm/st33zp24/i2c.c
>  create mode 100644 drivers/tpm/st33zp24/spi.c
>  create mode 100644 drivers/tpm/st33zp24/st33zp24.c
>  create mode 100644 drivers/tpm/st33zp24/st33zp24.h
>  create mode 100644 include/dm/platform_data/st33zp24_i2c.h
>  create mode 100644 include/dm/platform_data/st33zp24_spi.h
>
> diff --git a/README b/README
> index 506ff6c..d3f9590 100644
> --- a/README
> +++ b/README
> @@ -1499,6 +1499,17 @@ The following options need to be configured:
>                         CONFIG_TPM_TIS_I2C_BURST_LIMITATION
>                         Define the burst count bytes upper limit
>
> +               CONFIG_TPM_ST33ZP24
> +               Support for STMicroelectronics TPM devices. Requires DM_TPM support.
> +
> +                       CONFIG_TPM_ST33ZP24_I2C
> +                       Support for STMicroelectronics ST33ZP24 I2C devices.
> +                       Requires TPM_ST33ZP24 and I2C.
> +
> +                       CONFIG_TPM_ST33ZP24_SPI
> +                       Support for STMicroelectronics ST33ZP24 SPI devices.
> +                       Requires TPM_ST33ZP24 and SPI.
> +

These can go in Kconfig

>                 CONFIG_TPM_ATMEL_TWI
>                 Support for Atmel TWI TPM device. Requires I2C support.
>
> diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
> index bd2cd6d..48bc5f3 100644
> --- a/drivers/tpm/Makefile
> +++ b/drivers/tpm/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_DM_TPM) += tpm.o
>  obj-$(CONFIG_TPM_INFINEON_I2C) += tpm_i2c_infineon.o
>  obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
>  obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o
> +obj-$(CONFIG_TPM_ST33ZP24) += st33zp24/
> diff --git a/drivers/tpm/st33zp24/Makefile b/drivers/tpm/st33zp24/Makefile
> new file mode 100644
> index 0000000..ed28e8c
> --- /dev/null
> +++ b/drivers/tpm/st33zp24/Makefile
> @@ -0,0 +1,7 @@
> +#
> +# Makefile for ST33ZP24 TPM 1.2 driver
> +#
> +
> +obj-$(CONFIG_TPM_ST33ZP24) += st33zp24.o
> +obj-$(CONFIG_TPM_ST33ZP24_I2C) += i2c.o
> +obj-$(CONFIG_TPM_ST33ZP24_SPI) += spi.o
> diff --git a/drivers/tpm/st33zp24/i2c.c b/drivers/tpm/st33zp24/i2c.c
> new file mode 100644
> index 0000000..204021a
> --- /dev/null
> +++ b/drivers/tpm/st33zp24/i2c.c
> @@ -0,0 +1,226 @@
> +/*
> + * STMicroelectronics TPM ST33ZP24 I2C phy for UBOOT
> + * Copyright (C) 2015  STMicroelectronics
> + *
> + * Description: Device driver for ST33ZP24 I2C 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 I2C Protocol Stack Specification version 1.2.0.
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <i2c.h>
> +#include <dm.h>
> +#include <linux/types.h>

That may not be needed, but if so should go after the dm/platform_data...

> +#include <malloc.h>
> +#include <tpm.h>
> +#include <errno.h>

Should go up under common.h

> +#include <asm/unaligned.h>
> +#include <dm/platform_data/st33zp24_i2c.h>
> +
> +#include "../tpm_private.h"
> +#include "st33zp24.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define TPM_DUMMY_BYTE 0xAA
> +#define TPM_ST33ZP24_I2C_SLAVE_ADDR 0x13
> +
> +struct st33zp24_i2c_phy {
> +       uint8_t slave_addr;
> +       int i2c_bus;
> +       int old_bus;
> +       u8 buf[TPM_BUFSIZE + 1];
> +} __packed;

Should not need address and bus - just use a struct udevice. Also, why __packed?

> +
> +/*
> + * write8_reg
> + * Send byte to the TIS register according to the ST33ZP24 I2C protocol.
> + * @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: Number of byte written successfully else an error code.
> + */
> +static int write8_reg(void *phy_id, u8 tpm_register, u8 *tpm_data, u16 tpm_size)
> +{
> +       struct st33zp24_i2c_phy *phy = phy_id;

Why not pass struct st33zp24_i2c_phy to this function?

In general it's best to avoid things like u16 for parameters - just
use uint or unsigned int. It creates unnecessary masking.

> +       int ret;
> +       phy->buf[0] = tpm_register;
> +       memcpy(phy->buf + 1, tpm_data, tpm_size);
> +       ret = i2c_write(phy->slave_addr, 0, 0, phy->buf, tpm_size + 1);
> +       if (!ret)
> +               return tpm_size + 1;
> +       return ret;
> +} /* write8_reg() */
> +
> +/*
> +* read8_reg
> +* Recv byte from the TIS register according to the ST33ZP24 I2C protocol.
> +* @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: Number of byte read successfully else an error code.
> +*/
> +static int read8_reg(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size)
> +{
> +       struct st33zp24_i2c_phy *phy = phy_id;
> +       int status;
> +       u8 data;
> +
> +       data = TPM_DUMMY_BYTE;
> +       status = write8_reg(phy, tpm_register, &data, 1);
> +       if (status == 2) {

What is 2? How about jumping out when you get errors:

if (status < 0)
  return status;

status = i2c_read()...
...
return 0;

> +               status = i2c_read(phy->slave_addr, 0, 0, tpm_data, tpm_size);
> +               if (!status)
> +                       return tpm_size;
> +       }
> +       return status;
> +} /* read8_reg() */
> +
> +static int tpm_select(void *phy_id)

This isn't needed with driver model.

> +{
> +       int ret;
> +       struct st33zp24_i2c_phy *phy = phy_id;
> +
> +       phy->old_bus = i2c_get_bus_num();
> +       if (phy->old_bus != phy->i2c_bus) {
> +               ret = i2c_set_bus_num(phy->i2c_bus);
> +               if (ret) {
> +                       debug("%s: Fail to set i2c bus %d\n", __func__,
> +                             phy->i2c_bus);
> +                       return -1;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static int tpm_deselect(void *phy_id)
> +{
> +       int ret;
> +       struct st33zp24_i2c_phy *phy = phy_id;
> +
> +       if (phy->old_bus != i2c_get_bus_num()) {
> +               ret = i2c_set_bus_num(phy->old_bus);
> +               if (ret) {
> +                       debug("%s: Fail to restore i2c bus %d\n",
> +                             __func__, phy->old_bus);
> +                       return -1;
> +               }
> +       }
> +       phy->old_bus = -1;
> +
> +       return 0;
> +}
> +
> +/*
> + * st33zp24_i2c_send
> + * Send byte to the TIS register according to the ST33ZP24 I2C protocol.
> + * @param: phy_id, the phy 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: number of byte written successfully: should be one if success.
> + */
> +static int st33zp24_i2c_send(void *phy_id, u8 tpm_register, u8 *tpm_data,
> +                            int tpm_size)
> +{
> +       int rc;
> +
> +       tpm_select(phy_id);
> +       rc = write8_reg(phy_id, tpm_register | TPM_WRITE_DIRECTION, tpm_data,
> +                       tpm_size);
> +       tpm_deselect(phy_id);
> +       return rc;
> +}
> +
> +/*
> + * st33zp24_i2c_recv
> + * Recv byte from the TIS register according to the ST33ZP24 I2C protocol.
> + * @param: phy_id, the phy description
> + * @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: number of byte read successfully: should be one if success.
> + */
> +static int st33zp24_i2c_recv(void *phy_id, u8 tpm_register, u8 *tpm_data,
> +                            int tpm_size)
> +{
> +       int rc;
> +
> +       tpm_select(phy_id);
> +       rc = read8_reg(phy_id, tpm_register, tpm_data, tpm_size);
> +       tpm_deselect(phy_id);
> +       return rc;
> +}
> +
> +static const struct st33zp24_phy_ops i2c_phy_ops = {
> +       .send = st33zp24_i2c_send,
> +       .recv = st33zp24_i2c_recv,

We should avoid creating new structures with function pointers, unless
they are a uclass. What is the purpose of this? Is it for allowing
either I2C or SPI access?

> +};
> +
> +static int st33zp24_i2c_probe(struct udevice *dev)
> +{
> +       struct st33zp24_i2c_platdata *platdata = dev_get_platdata(dev);
> +       struct st33zp24_i2c_phy *i2c_phy = dev_get_priv(dev);
> +
> +       i2c_phy->slave_addr = platdata->slave_addr;
> +       i2c_phy->i2c_bus = platdata->i2c_bus;

Can you not just use the device? dm_i2c_read() doesn't need a bus / address.

> +
> +       return st33zp24_init(dev, i2c_phy, &i2c_phy_ops);
> +}
> +
> +static int st33zp24_i2c_remove(struct udevice *dev)
> +{
> +       return st33zp24_remove(dev);
> +}
> +
> +#ifdef CONFIG_OF_CONTROL
> +static const struct udevice_id st33zp24_i2c_ids[] = {
> +       { .compatible = "st,st33zp24-i2c" },
> +       { }
> +};
> +
> +static int st33zp24_i2c_ofdata_to_platdata(struct udevice *dev)
> +{
> +       int node, parent, i2c_bus;
> +       const void *blob = gd->fdt_blob;
> +       struct st33zp24_i2c_platdata *platdata = dev_get_platdata(dev);
> +
> +       node = fdtdec_next_compatible(blob, 0,
> +                               COMPAT_STMICROELECTRONICS_ST33ZP24_I2C);

You should be able to use dev->of_offset here.

> +       if (node < 0) {
> +               debug("%s: Node not found\n", __func__);
> +               return -1;
> +       }
> +
> +       parent = fdt_parent_offset(blob, node);
> +       if (parent < 0) {
> +               debug("%s: Cannot find node parent\n", __func__);
> +               return -1;
> +       }
> +
> +       i2c_bus = i2c_get_bus_num_fdt(parent);
> +       if (i2c_bus < 0)
> +               return -1;
> +
> +       platdata->i2c_bus = i2c_bus;
> +       platdata->slave_addr = fdtdec_get_addr(blob, node, "reg");

Really not needed. Just use dm_i2c_read()

> +
> +       return 0;
> +}
> +#endif
> +
> +U_BOOT_DRIVER(st33zp24_i2c) = {
> +       .name   = "st33zp24-i2c",
> +       .id     = UCLASS_TPM,
> +       .of_match = of_match_ptr(st33zp24_i2c_ids),
> +       .ofdata_to_platdata = of_match_ptr(st33zp24_i2c_ofdata_to_platdata),
> +       .probe  = st33zp24_i2c_probe,
> +       .remove = st33zp24_i2c_remove,
> +       .priv_auto_alloc_size = sizeof(struct st33zp24_i2c_phy),
> +       .platdata_auto_alloc_size = sizeof(struct st33zp24_i2c_platdata),
> +};
> diff --git a/drivers/tpm/st33zp24/spi.c b/drivers/tpm/st33zp24/spi.c
> new file mode 100644
> index 0000000..312ea07
> --- /dev/null
> +++ b/drivers/tpm/st33zp24/spi.c
> @@ -0,0 +1,286 @@
> +/*
> + * STMicroelectronics TPM ST33ZP24 SPI phy for UBOOT
> + * Copyright (C) 2015  STMicroelectronics
> + *
> + * 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+
> + */
> +
> +#include <common.h>
> +#include <spi.h>
> +#include <dm.h>
> +#include <linux/types.h>
> +#include <tpm.h>
> +#include <errno.h>
> +#include <asm/unaligned.h>
> +#include <dm/platform_data/st33zp24_spi.h>
> +
> +#include "../tpm_private.h"
> +#include "st33zp24.h"
> +
> +#define TPM_DATA_FIFO                          0x24
> +#define TPM_INTF_CAPABILITY                    0x14
> +
> +#define TPM_DUMMY_BYTE                         0x00
> +#define TPM_WRITE_DIRECTION                    0x80
> +
> +#define MAX_SPI_LATENCY                                15
> +#define LOCALITY0                              0
> +
> +#define ST33ZP24_OK                                    0x5A
> +#define ST33ZP24_UNDEFINED_ERR                         0x80
> +#define ST33ZP24_BADLOCALITY                           0x81
> +#define ST33ZP24_TISREGISTER_UKNOWN                    0x82
> +#define ST33ZP24_LOCALITY_NOT_ACTIVATED                        0x83
> +#define ST33ZP24_HASH_END_BEFORE_HASH_START            0x84
> +#define ST33ZP24_BAD_COMMAND_ORDER                     0x85
> +#define ST33ZP24_INCORECT_RECEIVED_LENGTH              0x86
> +#define ST33ZP24_TPM_FIFO_OVERFLOW                     0x89
> +#define ST33ZP24_UNEXPECTED_READ_FIFO                  0x8A
> +#define ST33ZP24_UNEXPECTED_WRITE_FIFO                 0x8B
> +#define ST33ZP24_CMDRDY_SET_WHEN_PROCESSING_HASH_END   0x90
> +#define ST33ZP24_DUMMY_BYTES                           0x00
> +
> +/*
> + * TPM command can be up to 2048 byte, A TPM response can be up to
> + * 1024 byte.
> + * Between command and response, there are latency byte (up to 15
> + * usually on st33zp24 2 are enough).
> + *
> + * Overall when sending a command and expecting an answer we need if
> + * worst case:
> + * 2048 (for the TPM command) + 1024 (for the TPM answer).  We need
> + * some latency byte before the answer is available (max 15).
> + * We have 2048 + 1024 + 15.
> + */
> +#define ST33ZP24_SPI_BUFFER_SIZE (TPM_BUFSIZE + (TPM_BUFSIZE / 2) +\
> +                                 MAX_SPI_LATENCY)
> +
> +struct st33zp24_spi_phy {
> +       struct spi_slave *spi_slave;

Better to use struct udevice for new code.

> +       int latency;
> +
> +       u8 tx_buf[ST33ZP24_SPI_BUFFER_SIZE];
> +       u8 rx_buf[ST33ZP24_SPI_BUFFER_SIZE];
> +};
> +
> +static int st33zp24_status_to_errno(u8 code)
> +{
> +       switch (code) {
> +       case ST33ZP24_OK:
> +               return 0;
> +       case ST33ZP24_UNDEFINED_ERR:
> +       case ST33ZP24_BADLOCALITY:
> +       case ST33ZP24_TISREGISTER_UKNOWN:
> +       case ST33ZP24_LOCALITY_NOT_ACTIVATED:
> +       case ST33ZP24_HASH_END_BEFORE_HASH_START:
> +       case ST33ZP24_BAD_COMMAND_ORDER:
> +       case ST33ZP24_UNEXPECTED_READ_FIFO:
> +       case ST33ZP24_UNEXPECTED_WRITE_FIFO:
> +       case ST33ZP24_CMDRDY_SET_WHEN_PROCESSING_HASH_END:
> +               return -EPROTO;
> +       case ST33ZP24_INCORECT_RECEIVED_LENGTH:
> +       case ST33ZP24_TPM_FIFO_OVERFLOW:
> +               return -EMSGSIZE;
> +       case ST33ZP24_DUMMY_BYTES:
> +               return -ENOSYS;
> +       }
> +       return code;
> +}
> +
> +/*
> + * st33zp24_spi_send
> + * Send byte to TPM register according to the ST33ZP24 SPI protocol.
> + * @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 st33zp24_spi_send(void *phy_id, u8 tpm_register, u8 *tpm_data,
> +                            int tpm_size)
> +{
> +       int total_length = 0, ret;
> +       struct st33zp24_spi_phy *phy = phy_id;
> +
> +       struct spi_slave *dev = phy->spi_slave;
> +       u8 *tx_buf = (u8 *)phy->tx_buf;
> +       u8 *rx_buf = phy->rx_buf;
> +
> +       tx_buf[total_length++] = TPM_WRITE_DIRECTION | LOCALITY0;
> +       tx_buf[total_length++] = tpm_register;
> +
> +       if (tpm_size > 0 && tpm_register == TPM_DATA_FIFO) {
> +               tx_buf[total_length++] = tpm_size >> 8;
> +               tx_buf[total_length++] = tpm_size;
> +       }
> +       memcpy(tx_buf + total_length, tpm_data, tpm_size);
> +       total_length += tpm_size;
> +
> +       memset(tx_buf + total_length, TPM_DUMMY_BYTE, phy->latency);
> +
> +       total_length += phy->latency;
> +
> +       spi_claim_bus(dev);
> +       ret = spi_xfer(dev, total_length * 8, tx_buf, rx_buf,
> +                      SPI_XFER_BEGIN | SPI_XFER_END);
> +       spi_release_bus(dev);
> +
> +       if (ret == 0)
> +               ret = rx_buf[total_length - 1];
> +
> +       return st33zp24_status_to_errno(ret);
> +} /* st33zp24_spi_send() */
> +
> +/*
> + * 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 read8_reg(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size)
> +{
> +       int total_length = 0, nbr_dummy_bytes, ret;
> +       struct st33zp24_spi_phy *phy = phy_id;
> +       struct spi_slave *dev = phy->spi_slave;
> +       u8 *tx_buf = (u8 *)phy->tx_buf;
> +       u8 *rx_buf = phy->rx_buf;
> +
> +       /* Pre-Header */
> +       tx_buf[total_length++] = LOCALITY0;
> +       tx_buf[total_length++] = tpm_register;
> +
> +       nbr_dummy_bytes = phy->latency;
> +       memset(&tx_buf[total_length], TPM_DUMMY_BYTE,
> +              nbr_dummy_bytes + tpm_size);
> +       total_length += nbr_dummy_bytes + tpm_size;
> +
> +       spi_claim_bus(dev);

Error checking?

> +       ret = spi_xfer(dev, total_length * 8, tx_buf, rx_buf,
> +                      SPI_XFER_BEGIN | SPI_XFER_END);
> +       spi_release_bus(dev);
> +
> +       if (tpm_size > 0 && ret == 0) {
> +               ret = rx_buf[total_length - tpm_size - 1];
> +               memcpy(tpm_data, rx_buf + total_length - tpm_size, tpm_size);
> +       }
> +       return ret;
> +} /* read8_reg() */
> +
> +/*
> + * st33zp24_spi_recv
> + * Recv byte from the TIS register according to the ST33ZP24 SPI protocol.
> + * @param: phy_id, the phy description
> + * @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: number of byte read successfully: should be one if success.
> + */
> +static int st33zp24_spi_recv(void *phy_id, u8 tpm_register, u8 *tpm_data,
> +                            int tpm_size)
> +{
> +       int ret;
> +
> +       ret = read8_reg(phy_id, tpm_register, tpm_data, tpm_size);
> +       if (!st33zp24_status_to_errno(ret))
> +               return tpm_size;
> +       return ret;
> +} /* st33zp24_spi_recv() */
> +
> +static int evaluate_latency(void *phy_id)
> +{
> +       struct st33zp24_spi_phy *phy = phy_id;
> +       int latency = 1, status = 0;
> +       u8 data = 0;
> +
> +       while (!status && latency < MAX_SPI_LATENCY) {
> +               phy->latency = latency;
> +               status = read8_reg(phy_id, TPM_INTF_CAPABILITY, &data, 1);
> +               latency++;
> +       }
> +       return latency - 1;
> +} /* evaluate_latency() */
> +
> +static const struct st33zp24_phy_ops spi_phy_ops = {
> +       .send = st33zp24_spi_send,
> +       .recv = st33zp24_spi_recv,
> +};

Again I'm not sure of the benefit of this indirection.

> +
> +static int st33zp24_spi_probe(struct udevice *dev)
> +{
> +       struct st33zp24_spi_platdata *platdata = dev_get_platdata(dev);
> +       struct st33zp24_spi_phy *spi_phy = dev_get_priv(dev);
> +
> +       #ifndef CONFIG_OF_CONTROL

Do we need to support this case?

> +       spi_phy->spi_slave = spi_setup_slave(platdata->bus_num,
> +                                            platdata->cs,
> +                                            platdata->max_speed,
> +                                            platdata->mode);
> +       #endif
> +
> +       spi_phy->latency = evaluate_latency(spi_phy);
> +       if (spi_phy->latency <= 0)
> +               return -ENODEV;
> +
> +       return st33zp24_init(dev, spi_phy, &spi_phy_ops);
> +} /* st33zp24_spi_probe() */
> +
> +static int st33zp24_spi_remove(struct udevice *dev)
> +{
> +       return st33zp24_remove(dev);
> +}
> +
> +#ifdef CONFIG_OF_CONTROL
> +static const struct udevice_id st33zp24_spi_ids[] = {
> +       { .compatible = "st,st33zp24-spi" },
> +       { }
> +};
> +
> +static int st33zp24_spi_ofdata_to_platdata(struct udevice *dev)
> +{
> +       int node, parent;
> +       int i2c_bus;
> +       const void *blob = gd->fdt_blob;
> +       struct st33zp24_spi_platdata *platdata = dev_get_platdata(dev);
> +       struct st33zp24_spi_phy *spi_phy = dev_get_priv(dev);
> +
> +       node = fdtdec_next_compatible(blob, 0,
> +                               COMPAT_STMICROELECTRONICS_ST33ZP24_SPI);

See comments for the I2C case.

> +       if (node < 0) {
> +               debug("%s: Node not found\n", __func__);
> +               return -1;
> +       }
> +
> +       parent = fdt_parent_offset(blob, node);
> +       if (parent < 0) {
> +               debug("%s: Cannot find node parent\n", __func__);
> +               return -1;
> +       }
> +
> +       spi_phy->spi_slave = spi_setup_slave_fdt(blob, node, parent);
> +       if (!spi_phy->spi_slave)
> +               return -1;
> +
> +       return 0;
> +}
> +#endif
> +
> +U_BOOT_DRIVER(st33zp24_spi) = {
> +       .name   = "st33zp24-spi",
> +       .id     = UCLASS_TPM,
> +       .of_match = of_match_ptr(st33zp24_spi_ids),
> +       .ofdata_to_platdata = of_match_ptr(st33zp24_spi_ofdata_to_platdata),
> +       .probe  = st33zp24_spi_probe,
> +       .remove = st33zp24_spi_remove,
> +       .priv_auto_alloc_size = sizeof(struct st33zp24_spi_phy),
> +       .platdata_auto_alloc_size = sizeof(struct st33zp24_spi_platdata),
> +};
> diff --git a/drivers/tpm/st33zp24/st33zp24.c b/drivers/tpm/st33zp24/st33zp24.c
> new file mode 100644
> index 0000000..862b98a
> --- /dev/null
> +++ b/drivers/tpm/st33zp24/st33zp24.c
> @@ -0,0 +1,454 @@
> +/*
> + * STMicroelectronics TPM ST33ZP24 UBOOT driver
> + *
> + * Copyright (C) 2015  STMicroelectronics
> + *
> + * Description: Device driver for ST33ZP24 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 Protocol Stack Specification version 1.2.0.
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <linux/types.h>
> +#include <linux/compat.h>
> +#include <malloc.h>
> +#include <tpm.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <asm/unaligned.h>
> +
> +#include "st33zp24.h"
> +#include "../tpm_private.h"
> +
> +#define TPM_ACCESS                     0x0
> +#define TPM_STS                                0x18
> +#define TPM_DATA_FIFO                  0x24
> +
> +#define LOCALITY0                      0
> +
> +enum st33zp24_access {
> +       TPM_ACCESS_VALID = 0x80,
> +       TPM_ACCESS_ACTIVE_LOCALITY = 0x20,
> +       TPM_ACCESS_REQUEST_PENDING = 0x04,
> +       TPM_ACCESS_REQUEST_USE = 0x02,
> +};
> +
> +enum st33zp24_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 st33zp24_int_flags {
> +       TPM_GLOBAL_INT_ENABLE = 0x80,
> +       TPM_INTF_CMD_READY_INT = 0x080,
> +       TPM_INTF_FIFO_AVALAIBLE_INT = 0x040,
> +       TPM_INTF_WAKE_UP_READY_INT = 0x020,
> +       TPM_INTF_LOCTPM_BUFSIZE4SOFTRELEASE_INT = 0x008,
> +       TPM_INTF_LOCALITY_CHANGE_INT = 0x004,
> +       TPM_INTF_STS_VALID_INT = 0x002,
> +       TPM_INTF_DATA_AVAIL_INT = 0x001,
> +};
> +
> +enum tis_defaults {
> +       TIS_SHORT_TIMEOUT = 750,        /* ms */
> +       TIS_LONG_TIMEOUT = 2000,        /* 2 sec */
> +};
> +
> +struct st33zp24_dev {
> +       void *phy_id;
> +       const struct st33zp24_phy_ops *ops;
> +};
> +
> +/*
> + * release_locality release the active locality
> + * @param: chip, the tpm chip description.
> + */
> +static void release_locality(struct tpm_chip *chip)
> +{
> +       struct st33zp24_dev *tpm_dev;
> +       u8 data = TPM_ACCESS_ACTIVE_LOCALITY;
> +
> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
> +
> +       tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
> +} /* release_locality() */

drop that comment - please fix globally.

> +
> +/*
> + * check_locality if the locality is active
> + * @param: chip, the tpm chip description
> + * @return: the active locality or -EACCES.
> + */
> +static int check_locality(struct tpm_chip *chip)
> +{
> +       struct st33zp24_dev *tpm_dev;
> +       u8 data;
> +       u8 status;
> +
> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
> +
> +       status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
> +       if (status && (data &
> +               (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) ==
> +               (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID))
> +               return chip->vendor.locality;
> +
> +       return -EACCES;
> +} /* check_locality() */
> +
> +/*
> + * request_locality request the TPM locality
> + * @param: chip, the chip description
> + * @return: the active locality or negative value.
> + */
> +static int request_locality(struct tpm_chip *chip)
> +{
> +       unsigned long start, stop;
> +       long ret;
> +       struct st33zp24_dev *tpm_dev;
> +       u8 data;
> +
> +       if (check_locality(chip) == chip->vendor.locality)
> +               return chip->vendor.locality;
> +
> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
> +
> +       data = TPM_ACCESS_REQUEST_USE;
> +       ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* wait for locality activated */
> +       start = get_timer(0);
> +       stop = chip->vendor.timeout_a;
> +       do {
> +               if (check_locality(chip) >= 0)
> +                       return chip->vendor.locality;
> +               udelay(TPM_TIMEOUT * 1000);
> +       } while  (get_timer(start) < stop);
> +
> +       return -EACCES;
> +} /* request_locality() */
> +
> +/*
> + * st33zp24_status return the TPM_STS register
> + * @param: chip, the tpm chip description
> + * @return: the TPM_STS register value.
> + */
> +static u8 st33zp24_status(struct tpm_chip *chip)
> +{
> +       struct st33zp24_dev *tpm_dev;
> +       u8 data;
> +
> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
> +
> +       tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1);
> +       return data;
> +} /* st33zp24_status() */
> +
> +/*
> + * get_burstcount return the burstcount address 0x19 0x1A
> + * @param: chip, the chip description
> + * return: the burstcount or -TPM_DRIVER_ERR in case of error.
> + */
> +static int get_burstcount(struct tpm_chip *chip)
> +{
> +       unsigned long start, stop;
> +       int burstcnt, status;
> +       u8 tpm_reg, temp;
> +       struct st33zp24_dev *tpm_dev;
> +
> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
> +
> +       /* wait for burstcount */
> +       start = get_timer(0);
> +       stop = chip->vendor.timeout_d;
> +       do {
> +               tpm_reg = TPM_STS + 1;
> +               status = tpm_dev->ops->recv(tpm_dev->phy_id, tpm_reg, &temp, 1);
> +               if (status < 0)
> +                       return -EBUSY;
> +
> +               tpm_reg = TPM_STS + 2;
> +               burstcnt = temp;
> +               status = tpm_dev->ops->recv(tpm_dev->phy_id, tpm_reg, &temp, 1);
> +               if (status < 0)
> +                       return -EBUSY;
> +
> +               burstcnt |= temp << 8;
> +               if (burstcnt)
> +                       return burstcnt;
> +               udelay(TIS_SHORT_TIMEOUT * 1000);
> +       } while (get_timer(start) < stop);
> +       return -EBUSY;
> +} /* get_burstcount() */
> +
> +/*
> + * st33zp24_cancel, cancel the current command execution or
> + * set STS to COMMAND READY.
> + * @param: chip, tpm_chip description.
> + */
> +static void st33zp24_cancel(struct tpm_chip *chip)
> +{
> +       struct st33zp24_dev *tpm_dev;
> +       u8 data;
> +
> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
> +
> +       data = TPM_STS_COMMAND_READY;
> +       tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1);
> +} /* st33zp24_cancel() */
> +
> +/*
> + * 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 timeout
> + * @param: status,
> + * @return: the tpm status, 0 if success, -ETIME if timeout is reached.
> + */
> +static int wait_for_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
> +                        int *status)
> +{
> +       unsigned long start, stop;
> +
> +       /* Check current status */
> +       *status = st33zp24_status(chip);
> +       if ((*status & mask) == mask)
> +               return 0;
> +
> +       start = get_timer(0);
> +       stop = timeout;
> +       do {
> +               udelay(TPM_TIMEOUT * 1000);
> +               *status = st33zp24_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: the number of bytes read from TPM FIFO.
> + */
> +static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
> +{
> +       int size = 0, burstcnt, len, ret, status;
> +       struct st33zp24_dev *tpm_dev;
> +
> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
> +
> +       while (size < count &&
> +              wait_for_stat(chip,
> +                            TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> +                            chip->vendor.timeout_c, &status) == 0) {
> +               burstcnt = get_burstcount(chip);
> +               if (burstcnt < 0)
> +                       return burstcnt;
> +               len = min_t(int, burstcnt, count - size);
> +               ret = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_DATA_FIFO,
> +                                        buf + size, len);
> +               if (ret < 0)
> +                       return ret;
> +
> +               size += len;
> +       }
> +       return size;
> +} /* recv_data() */
> +
> +/*
> + * st33zp24_recv received TPM response through TPM phy.
> + * @param: chip, tpm_chip description.
> + * @param: buf,        the buffer to store data.
> + * @param: count, the number of bytes that can received (sizeof buf).
> + * @return: Returns zero in case of success else -EIO.
> + */
> +static int st33zp24_recv(struct tpm_chip *chip, unsigned char *buf,
> +                        size_t count)
> +{
> +       int size = 0;
> +       int expected;
> +
> +       if (!chip)
> +               return -ENODEV;
> +
> +       if (count < TPM_HEADER_SIZE) {
> +               size = -EIO;
> +               goto out;
> +       }
> +
> +       size = recv_data(chip, buf, TPM_HEADER_SIZE);
> +       if (size < TPM_HEADER_SIZE) {
> +               printf("TPM error, unable to read header\n");
> +               goto out;
> +       }
> +
> +       expected = get_unaligned_be32(buf + 2);
> +       if (expected > count) {
> +               size = -EIO;
> +               goto out;
> +       }
> +
> +       size += recv_data(chip, &buf[TPM_HEADER_SIZE],
> +                         expected - TPM_HEADER_SIZE);
> +       if (size < expected) {
> +               printf("TPM error, unable to read remaining bytes of result\n");
> +               size = -EIO;
> +               goto out;
> +       }
> +
> +out:
> +       st33zp24_cancel(chip);
> +       release_locality(chip);
> +       return size;
> +} /* st33zp24_recv() */
> +
> +/*
> + * st33zp24_send send TPM commands through TPM phy.
> + * @param: chip, tpm_chip description.
> + * @param: buf,        the buffer to send.
> + * @param: len, the number of bytes to send.
> + * @return: Returns zero in case of success else the negative error code.
> + */
> +static int st33zp24_send(struct tpm_chip *chip, u8 *buf,
> +                        size_t len)
> +{
> +       u32 i, size;
> +       int burstcnt, ret, status;
> +       u8 data, tpm_stat;
> +       struct st33zp24_dev *tpm_dev;
> +
> +       if (!chip)
> +               return -ENODEV;
> +       if (len < TPM_HEADER_SIZE)
> +               return -EIO;
> +
> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
> +
> +       ret = request_locality(chip);
> +       if (ret < 0)
> +               return ret;
> +
> +       tpm_stat = st33zp24_status(chip);
> +       if ((tpm_stat & TPM_STS_COMMAND_READY) == 0) {
> +               st33zp24_cancel(chip);
> +               if (wait_for_stat(chip, TPM_STS_COMMAND_READY,
> +                                 chip->vendor.timeout_b, &status) < 0) {
> +                       ret = -ETIME;
> +                       goto out_err;
> +               }
> +       }
> +
> +       for (i = 0; i < len - 1;) {
> +               burstcnt = get_burstcount(chip);
> +               if (burstcnt < 0)
> +                       return burstcnt;
> +               size = min_t(int, len - i - 1, burstcnt);
> +               ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_DATA_FIFO,
> +                                        buf + i, size);
> +               if (ret < 0)
> +                       goto out_err;
> +
> +               i += size;
> +       }
> +
> +       tpm_stat = st33zp24_status(chip);
> +       if ((tpm_stat & TPM_STS_DATA_EXPECT) == 0) {
> +               ret = -EIO;
> +               goto out_err;
> +       }
> +
> +       ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_DATA_FIFO,
> +                                buf + len - 1, 1);
> +       if (ret < 0)
> +               goto out_err;
> +
> +       tpm_stat = st33zp24_status(chip);
> +       if ((tpm_stat & TPM_STS_DATA_EXPECT) != 0) {
> +               ret = -EIO;
> +               goto out_err;
> +       }
> +
> +       data = TPM_STS_GO;
> +       ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1);
> +       if (ret < 0)
> +               goto out_err;
> +
> +       return len;
> +
> +out_err:
> +       st33zp24_cancel(chip);
> +       release_locality(chip);
> +       return ret;
> +} /* st33zp24_send() */
> +
> +static struct tpm_vendor_specific st33zp24_tpm = {
> +       .recv = st33zp24_recv,
> +       .send = st33zp24_send,
> +       .cancel = st33zp24_cancel,
> +       .status = st33zp24_status,

Methods should go in a new uclass if needed.

> +       .req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> +       .req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> +       .req_canceled = TPM_STS_COMMAND_READY,
> +};
> +
> +int st33zp24_init(struct udevice *dev, void *phy_id,
> +                 const struct st33zp24_phy_ops *ops)
> +{
> +       int ret;
> +       struct tpm_chip *chip;
> +       struct st33zp24_dev *tpm_dev;
> +
> +       chip = tpm_register_hardware(dev, &st33zp24_tpm);
> +       if (chip < 0) {
> +               ret = -ENODEV;
> +               goto out_err;
> +       }
> +
> +       tpm_dev = (struct st33zp24_dev *)malloc(sizeof(struct st33zp24_dev));
> +       if (!tpm_dev) {
> +               ret = -ENODEV;
> +               goto out_err;
> +       }
> +
> +       /* Disable interrupts (not supported) */
> +       chip->vendor.irq = 0;
> +
> +       /* Default timeouts */
> +       chip->vendor.timeout_a = TIS_SHORT_TIMEOUT;
> +       chip->vendor.timeout_b = TIS_LONG_TIMEOUT;
> +       chip->vendor.timeout_c = TIS_SHORT_TIMEOUT;
> +       chip->vendor.timeout_d = TIS_SHORT_TIMEOUT;
> +
> +       chip->vendor.locality = LOCALITY0;
> +       TPM_VPRIV(chip) = tpm_dev;
> +       tpm_dev->phy_id = phy_id;
> +       tpm_dev->ops = ops;
> +
> +       printf("ST33ZP24 TPM from STMicroelectronics found\n");
> +       return 0;
> +
> +out_err:
> +       return ret;
> +}
> +
> +int st33zp24_remove(struct udevice *dev)
> +{
> +       struct tpm_chip *chip = dev_get_uclass_priv(dev);
> +       struct st33zp24_dev *tpm_dev = TPM_VPRIV(chip);
> +
> +       release_locality(chip);
> +       free(tpm_dev);
> +       return 0;
> +}
> diff --git a/drivers/tpm/st33zp24/st33zp24.h b/drivers/tpm/st33zp24/st33zp24.h
> new file mode 100644
> index 0000000..4be06cb
> --- /dev/null
> +++ b/drivers/tpm/st33zp24/st33zp24.h
> @@ -0,0 +1,29 @@
> +/*
> + * STMicroelectronics TPM UBOOT driver for TPM ST33ZP24
> + *
> + * Copyright (C) 2015  STMicroelectronics
> + *
> + * Description: Device driver for ST33ZP24 I2C 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 Protocol Stack Specification version 1.2.0.
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#ifndef __LOCAL_ST33ZP24_H__
> +#define __LOCAL_ST33ZP24_H__
> +
> +#define TPM_WRITE_DIRECTION             0x80
> +#define TPM_HEADER_SIZE                        10
> +
> +struct st33zp24_phy_ops {
> +       int (*send)(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size);
> +       int (*recv)(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size);
> +};
> +
> +int st33zp24_init(struct udevice *dev, void *phy_id,
> +                 const struct st33zp24_phy_ops *ops);
> +int st33zp24_remove(struct udevice *dev);
> +#endif /* __LOCAL_ST33ZP24_H__ */
> diff --git a/include/dm/platform_data/st33zp24_i2c.h b/include/dm/platform_data/st33zp24_i2c.h
> new file mode 100644
> index 0000000..e71c9e3
> --- /dev/null
> +++ b/include/dm/platform_data/st33zp24_i2c.h
> @@ -0,0 +1,28 @@
> +/*
> + * STMicroelectronics TPM Linux driver for TPM 1.2 ST33ZP24
> + * Copyright (C) 2009 - 2015  STMicroelectronics

Can we use SPDX?

> + *
> + * 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, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __ST33ZP24_I2C_H__
> +#define __ST33ZP24_I2C_H__
> +
> +#define TPM_ST33ZP24_I2C               "st33zp24-i2c"
> +
> +struct st33zp24_i2c_platdata {
> +       int i2c_bus;
> +       uint8_t slave_addr;
> +} __packed;
> +
> +#endif /* __ST33ZP24_I2C_H__ */
> diff --git a/include/dm/platform_data/st33zp24_spi.h b/include/dm/platform_data/st33zp24_spi.h
> new file mode 100644
> index 0000000..82f560b
> --- /dev/null
> +++ b/include/dm/platform_data/st33zp24_spi.h
> @@ -0,0 +1,30 @@
> +/*
> + * STMicroelectronics TPM Linux driver for TPM 1.2 ST33ZP24
> + * Copyright (C) 2009 - 2015  STMicroelectronics
> + *
> + * 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, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __ST33ZP24_SPI_H__
> +#define __ST33ZP24_SPI_H__
> +
> +#define TPM_ST33ZP24_SPI               "st33zp24-spi"
> +
> +struct st33zp24_spi_platdata {
> +       int bus_num;
> +       int cs;
> +       int max_speed;
> +       int mode;

Hopefully not needed.

> +};
> +
> +#endif /* __ST33ZP24_SPI_H__ */
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index eac679e..9eb2b3d 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -182,7 +182,10 @@ enum fdt_compat_id {
>         COMPAT_INTEL_PCH,               /* Intel PCH */
>         COMPAT_INTEL_IRQ_ROUTER,        /* Intel Interrupt Router */
>         COMPAT_ALTERA_SOCFPGA_DWMAC,    /* SoCFPGA Ethernet controller */
> -
> +       COMPAT_STMICROELECTRONICS_ST33ZP24_I2C,
> +                                       /* STMicroelectronics ST33ZP24 I2C TPM */
> +       COMPAT_STMICROELECTRONICS_ST33ZP24_SPI,
> +                                       /* STMicroelectronics ST33ZP24 SPI TPM */

Shouldn't be needed.

>         COMPAT_COUNT,
>  };
>
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index b201787..55c64a0 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -76,6 +76,8 @@ static const char * const compat_names[COMPAT_COUNT] = {
>         COMPAT(COMPAT_INTEL_PCH, "intel,bd82x6x"),
>         COMPAT(COMPAT_INTEL_IRQ_ROUTER, "intel,irq-router"),
>         COMPAT(ALTERA_SOCFPGA_DWMAC, "altr,socfpga-stmmac"),
> +       COMPAT(STMICROELECTRONICS_ST33ZP24_I2C, "st,st33zp24-i2c"),
> +       COMPAT(STMICROELECTRONICS_ST33ZP24_SPI, "st,st33zp24-spi"),
>  };
>
>  const char *fdtdec_get_compatible(enum fdt_compat_id id)
> --
> 2.1.4
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/3] tpm: Initial work to introduce TPM driver model
  2015-08-13 15:55   ` Simon Glass
@ 2015-08-13 20:37     ` Christophe Ricard
  2015-08-13 22:53       ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: Christophe Ricard @ 2015-08-13 20:37 UTC (permalink / raw
  To: u-boot

Hi Simon,

On 13/08/2015 17:55, Simon Glass wrote:
> Hi Christophe,
>
> On 9 August 2015 at 07:19, Christophe Ricard
> <christophe.ricard@gmail.com> wrote:
>> drivers/tpm/tpm.c is a TPM core driver port from Linux.
>> So far in u-boot only infineon i2c driver is using it but it could fit
>> for others...
>>
>> Introduce a new tpm uclass so that every TPM driver can register against it and
>> and take benefit of common functions and data such as tpm_transmit,
>> tpm_register_hardware & tpm_remove_hardware.
>> Finally tis_init, tis_open, tis_close, tis_sendrecv are using ops allowing
>> to introduce proprietary instructions.
>> Also this patch convert tpm_i2c_infineon for using this tpm uclass.
>>
>> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
>> ---
>>
>>   README                                      |   8 +-
>>   drivers/tpm/Makefile                        |   2 +-
>>   drivers/tpm/tpm.c                           | 275 +++++++---------------------
>>   drivers/tpm/tpm_i2c_infineon.c              | 271 ++++++++++++++++-----------
>>   drivers/tpm/tpm_private.h                   |  23 ++-
>>   include/dm/platform_data/tpm_i2c_infineon.h |  23 +++
>>   include/dm/uclass-id.h                      |   1 +
>>   7 files changed, 270 insertions(+), 333 deletions(-)
>>   create mode 100644 include/dm/platform_data/tpm_i2c_infineon.h
> There is a lot going on in this patch - in general I think it is
> better to split things up so that each patch does one thing.
I understand. I have basically seen in your work more or less the same 
approach as far as TPM class.
I believe, according to your plan, i can wait for you to update you tree 
integrating my first comments on tpm-working branch or
if you prefer i can send a merge between your work and my comments.

What's your prefered option ?

Best Regards
Christophe
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/3] tpm: Add st33zp24 tpm with i2c and spi phy
  2015-08-13 15:55   ` Simon Glass
@ 2015-08-13 20:59     ` Christophe Ricard
  2015-08-13 22:53       ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: Christophe Ricard @ 2015-08-13 20:59 UTC (permalink / raw
  To: u-boot

Hi Simon,

On 13/08/2015 17:55, Simon Glass wrote:
> Hi Christophe,
>
> On 9 August 2015 at 07:19, Christophe Ricard
> <christophe.ricard@gmail.com> wrote:
>> Add TPM st33zp24 tpm with i2c and spi phy. This is a port from Linux.
>> This driver relies on tpm uclass.
>>
>> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
>> ---
>>
>>   README                                  |  11 +
>>   drivers/tpm/Makefile                    |   1 +
>>   drivers/tpm/st33zp24/Makefile           |   7 +
>>   drivers/tpm/st33zp24/i2c.c              | 226 ++++++++++++++++
>>   drivers/tpm/st33zp24/spi.c              | 286 ++++++++++++++++++++
>>   drivers/tpm/st33zp24/st33zp24.c         | 454 ++++++++++++++++++++++++++++++++
>>   drivers/tpm/st33zp24/st33zp24.h         |  29 ++
>>   include/dm/platform_data/st33zp24_i2c.h |  28 ++
>>   include/dm/platform_data/st33zp24_spi.h |  30 +++
>>   include/fdtdec.h                        |   5 +-
>>   lib/fdtdec.c                            |   2 +
>>   11 files changed, 1078 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/tpm/st33zp24/Makefile
>>   create mode 100644 drivers/tpm/st33zp24/i2c.c
>>   create mode 100644 drivers/tpm/st33zp24/spi.c
>>   create mode 100644 drivers/tpm/st33zp24/st33zp24.c
>>   create mode 100644 drivers/tpm/st33zp24/st33zp24.h
>>   create mode 100644 include/dm/platform_data/st33zp24_i2c.h
>>   create mode 100644 include/dm/platform_data/st33zp24_spi.h
>>
>> diff --git a/README b/README
>> index 506ff6c..d3f9590 100644
>> --- a/README
>> +++ b/README
>> @@ -1499,6 +1499,17 @@ The following options need to be configured:
>>                          CONFIG_TPM_TIS_I2C_BURST_LIMITATION
>>                          Define the burst count bytes upper limit
>>
>> +               CONFIG_TPM_ST33ZP24
>> +               Support for STMicroelectronics TPM devices. Requires DM_TPM support.
>> +
>> +                       CONFIG_TPM_ST33ZP24_I2C
>> +                       Support for STMicroelectronics ST33ZP24 I2C devices.
>> +                       Requires TPM_ST33ZP24 and I2C.
>> +
>> +                       CONFIG_TPM_ST33ZP24_SPI
>> +                       Support for STMicroelectronics ST33ZP24 SPI devices.
>> +                       Requires TPM_ST33ZP24 and SPI.
>> +
> These can go in Kconfig
Ok, your are correct, i will update this in a future v2.
>>                  CONFIG_TPM_ATMEL_TWI
>>                  Support for Atmel TWI TPM device. Requires I2C support.
>>
>> diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
>> index bd2cd6d..48bc5f3 100644
>> --- a/drivers/tpm/Makefile
>> +++ b/drivers/tpm/Makefile
>> @@ -9,3 +9,4 @@ obj-$(CONFIG_DM_TPM) += tpm.o
>>   obj-$(CONFIG_TPM_INFINEON_I2C) += tpm_i2c_infineon.o
>>   obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
>>   obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o
>> +obj-$(CONFIG_TPM_ST33ZP24) += st33zp24/
>> diff --git a/drivers/tpm/st33zp24/Makefile b/drivers/tpm/st33zp24/Makefile
>> new file mode 100644
>> index 0000000..ed28e8c
>> --- /dev/null
>> +++ b/drivers/tpm/st33zp24/Makefile
>> @@ -0,0 +1,7 @@
>> +#
>> +# Makefile for ST33ZP24 TPM 1.2 driver
>> +#
>> +
>> +obj-$(CONFIG_TPM_ST33ZP24) += st33zp24.o
>> +obj-$(CONFIG_TPM_ST33ZP24_I2C) += i2c.o
>> +obj-$(CONFIG_TPM_ST33ZP24_SPI) += spi.o
>> diff --git a/drivers/tpm/st33zp24/i2c.c b/drivers/tpm/st33zp24/i2c.c
>> new file mode 100644
>> index 0000000..204021a
>> --- /dev/null
>> +++ b/drivers/tpm/st33zp24/i2c.c
>> @@ -0,0 +1,226 @@
>> +/*
>> + * STMicroelectronics TPM ST33ZP24 I2C phy for UBOOT
>> + * Copyright (C) 2015  STMicroelectronics
>> + *
>> + * Description: Device driver for ST33ZP24 I2C 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 I2C Protocol Stack Specification version 1.2.0.
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <i2c.h>
>> +#include <dm.h>
>> +#include <linux/types.h>
> That may not be needed, but if so should go after the dm/platform_data...
>
>> +#include <malloc.h>
>> +#include <tpm.h>
>> +#include <errno.h>
> Should go up under common.h
ok.
>> +#include <asm/unaligned.h>
>> +#include <dm/platform_data/st33zp24_i2c.h>
>> +
>> +#include "../tpm_private.h"
>> +#include "st33zp24.h"
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#define TPM_DUMMY_BYTE 0xAA
>> +#define TPM_ST33ZP24_I2C_SLAVE_ADDR 0x13
>> +
>> +struct st33zp24_i2c_phy {
>> +       uint8_t slave_addr;
>> +       int i2c_bus;
>> +       int old_bus;
>> +       u8 buf[TPM_BUFSIZE + 1];
>> +} __packed;
> Should not need address and bus - just use a struct udevice. Also, why __packed?
What if my board (beagleboard xm) does not support DM_I2C ? Should i 
concider a new one ? (Any recommendation ?)
How do you attach a I2C device using platform_data approach ?
I was doing this in board/ti/beagle/beagle.c:

static const struct st33zp24_i2c_platdata beagle_tpm_i2c = {
         .i2c_bus = 1,
         .slave_addr = 0x13,
};

U_BOOT_DEVICE(beagle_tpm_i2c) = {
         .name = TPM_ST33ZP24_I2C,
         .platdata = &beagle_tpm_i2c,
};

>> +
>> +/*
>> + * write8_reg
>> + * Send byte to the TIS register according to the ST33ZP24 I2C protocol.
>> + * @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: Number of byte written successfully else an error code.
>> + */
>> +static int write8_reg(void *phy_id, u8 tpm_register, u8 *tpm_data, u16 tpm_size)
>> +{
>> +       struct st33zp24_i2c_phy *phy = phy_id;
> Why not pass struct st33zp24_i2c_phy to this function?
I think it is ok to use st33zp24_i2c_phy * in write8_reg.
However st33zp24_i2c_send needs to be void *phy_id to order to support 
multiple phys
> In general it's best to avoid things like u16 for parameters - just
> use uint or unsigned int. It creates unnecessary masking.
ok. I am not sure why i got to u16 and i am sure uint or unsigned int is 
fine.
>
>> +       int ret;
>> +       phy->buf[0] = tpm_register;
>> +       memcpy(phy->buf + 1, tpm_data, tpm_size);
>> +       ret = i2c_write(phy->slave_addr, 0, 0, phy->buf, tpm_size + 1);
>> +       if (!ret)
>> +               return tpm_size + 1;
>> +       return ret;
>> +} /* write8_reg() */
>> +
>> +/*
>> +* read8_reg
>> +* Recv byte from the TIS register according to the ST33ZP24 I2C protocol.
>> +* @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: Number of byte read successfully else an error code.
>> +*/
>> +static int read8_reg(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size)
>> +{
>> +       struct st33zp24_i2c_phy *phy = phy_id;
>> +       int status;
>> +       u8 data;
>> +
>> +       data = TPM_DUMMY_BYTE;
>> +       status = write8_reg(phy, tpm_register, &data, 1);
>> +       if (status == 2) {
> What is 2? How about jumping out when you get errors:
2 is the expected number of byte send over i2c. I can rework this to 
return 0 or an error.
>
> if (status < 0)
>    return status;
>
> status = i2c_read()...
> ...
> return 0;
>
>> +               status = i2c_read(phy->slave_addr, 0, 0, tpm_data, tpm_size);
>> +               if (!status)
>> +                       return tpm_size;
>> +       }
>> +       return status;
>> +} /* read8_reg() */
>> +
>> +static int tpm_select(void *phy_id)
> This isn't needed with driver model.
I can't test on Beagleboard DM_I2C. I need to find another one. Any 
recommendation ?
>> +{
>> +       int ret;
>> +       struct st33zp24_i2c_phy *phy = phy_id;
>> +
>> +       phy->old_bus = i2c_get_bus_num();
>> +       if (phy->old_bus != phy->i2c_bus) {
>> +               ret = i2c_set_bus_num(phy->i2c_bus);
>> +               if (ret) {
>> +                       debug("%s: Fail to set i2c bus %d\n", __func__,
>> +                             phy->i2c_bus);
>> +                       return -1;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int tpm_deselect(void *phy_id)
>> +{
>> +       int ret;
>> +       struct st33zp24_i2c_phy *phy = phy_id;
>> +
>> +       if (phy->old_bus != i2c_get_bus_num()) {
>> +               ret = i2c_set_bus_num(phy->old_bus);
>> +               if (ret) {
>> +                       debug("%s: Fail to restore i2c bus %d\n",
>> +                             __func__, phy->old_bus);
>> +                       return -1;
>> +               }
>> +       }
>> +       phy->old_bus = -1;
>> +
>> +       return 0;
>> +}
>> +
>> +/*
>> + * st33zp24_i2c_send
>> + * Send byte to the TIS register according to the ST33ZP24 I2C protocol.
>> + * @param: phy_id, the phy 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: number of byte written successfully: should be one if success.
>> + */
>> +static int st33zp24_i2c_send(void *phy_id, u8 tpm_register, u8 *tpm_data,
>> +                            int tpm_size)
>> +{
>> +       int rc;
>> +
>> +       tpm_select(phy_id);
>> +       rc = write8_reg(phy_id, tpm_register | TPM_WRITE_DIRECTION, tpm_data,
>> +                       tpm_size);
>> +       tpm_deselect(phy_id);
>> +       return rc;
>> +}
>> +
>> +/*
>> + * st33zp24_i2c_recv
>> + * Recv byte from the TIS register according to the ST33ZP24 I2C protocol.
>> + * @param: phy_id, the phy description
>> + * @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: number of byte read successfully: should be one if success.
>> + */
>> +static int st33zp24_i2c_recv(void *phy_id, u8 tpm_register, u8 *tpm_data,
>> +                            int tpm_size)
>> +{
>> +       int rc;
>> +
>> +       tpm_select(phy_id);
>> +       rc = read8_reg(phy_id, tpm_register, tpm_data, tpm_size);
>> +       tpm_deselect(phy_id);
>> +       return rc;
>> +}
>> +
>> +static const struct st33zp24_phy_ops i2c_phy_ops = {
>> +       .send = st33zp24_i2c_send,
>> +       .recv = st33zp24_i2c_recv,
> We should avoid creating new structures with function pointers, unless
> they are a uclass. What is the purpose of this? Is it for allowing
> either I2C or SPI access?
Yes it is for allowing I2C or SPI support. I hope my approach is 
acceptable for this purpose ?
>> +};
>> +
>> +static int st33zp24_i2c_probe(struct udevice *dev)
>> +{
>> +       struct st33zp24_i2c_platdata *platdata = dev_get_platdata(dev);
>> +       struct st33zp24_i2c_phy *i2c_phy = dev_get_priv(dev);
>> +
>> +       i2c_phy->slave_addr = platdata->slave_addr;
>> +       i2c_phy->i2c_bus = platdata->i2c_bus;
> Can you not just use the device? dm_i2c_read() doesn't need a bus / address.
I am doing this only because Beagleboard xM does not support DM_I2C :(.
>> +
>> +       return st33zp24_init(dev, i2c_phy, &i2c_phy_ops);
>> +}
>> +
>> +static int st33zp24_i2c_remove(struct udevice *dev)
>> +{
>> +       return st33zp24_remove(dev);
>> +}
>> +
>> +#ifdef CONFIG_OF_CONTROL
>> +static const struct udevice_id st33zp24_i2c_ids[] = {
>> +       { .compatible = "st,st33zp24-i2c" },
>> +       { }
>> +};
>> +
>> +static int st33zp24_i2c_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       int node, parent, i2c_bus;
>> +       const void *blob = gd->fdt_blob;
>> +       struct st33zp24_i2c_platdata *platdata = dev_get_platdata(dev);
>> +
>> +       node = fdtdec_next_compatible(blob, 0,
>> +                               COMPAT_STMICROELECTRONICS_ST33ZP24_I2C);
> You should be able to use dev->of_offset here.
>
>> +       if (node < 0) {
>> +               debug("%s: Node not found\n", __func__);
>> +               return -1;
>> +       }
>> +
>> +       parent = fdt_parent_offset(blob, node);
>> +       if (parent < 0) {
>> +               debug("%s: Cannot find node parent\n", __func__);
>> +               return -1;
>> +       }
>> +
>> +       i2c_bus = i2c_get_bus_num_fdt(parent);
>> +       if (i2c_bus < 0)
>> +               return -1;
>> +
>> +       platdata->i2c_bus = i2c_bus;
>> +       platdata->slave_addr = fdtdec_get_addr(blob, node, "reg");
> Really not needed. Just use dm_i2c_read()
Ditto.
>> +
>> +       return 0;
>> +}
>> +#endif
>> +
>> +U_BOOT_DRIVER(st33zp24_i2c) = {
>> +       .name   = "st33zp24-i2c",
>> +       .id     = UCLASS_TPM,
>> +       .of_match = of_match_ptr(st33zp24_i2c_ids),
>> +       .ofdata_to_platdata = of_match_ptr(st33zp24_i2c_ofdata_to_platdata),
>> +       .probe  = st33zp24_i2c_probe,
>> +       .remove = st33zp24_i2c_remove,
>> +       .priv_auto_alloc_size = sizeof(struct st33zp24_i2c_phy),
>> +       .platdata_auto_alloc_size = sizeof(struct st33zp24_i2c_platdata),
>> +};
>> diff --git a/drivers/tpm/st33zp24/spi.c b/drivers/tpm/st33zp24/spi.c
>> new file mode 100644
>> index 0000000..312ea07
>> --- /dev/null
>> +++ b/drivers/tpm/st33zp24/spi.c
>> @@ -0,0 +1,286 @@
>> +/*
>> + * STMicroelectronics TPM ST33ZP24 SPI phy for UBOOT
>> + * Copyright (C) 2015  STMicroelectronics
>> + *
>> + * 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+
>> + */
>> +
>> +#include <common.h>
>> +#include <spi.h>
>> +#include <dm.h>
>> +#include <linux/types.h>
>> +#include <tpm.h>
>> +#include <errno.h>
>> +#include <asm/unaligned.h>
>> +#include <dm/platform_data/st33zp24_spi.h>
>> +
>> +#include "../tpm_private.h"
>> +#include "st33zp24.h"
>> +
>> +#define TPM_DATA_FIFO                          0x24
>> +#define TPM_INTF_CAPABILITY                    0x14
>> +
>> +#define TPM_DUMMY_BYTE                         0x00
>> +#define TPM_WRITE_DIRECTION                    0x80
>> +
>> +#define MAX_SPI_LATENCY                                15
>> +#define LOCALITY0                              0
>> +
>> +#define ST33ZP24_OK                                    0x5A
>> +#define ST33ZP24_UNDEFINED_ERR                         0x80
>> +#define ST33ZP24_BADLOCALITY                           0x81
>> +#define ST33ZP24_TISREGISTER_UKNOWN                    0x82
>> +#define ST33ZP24_LOCALITY_NOT_ACTIVATED                        0x83
>> +#define ST33ZP24_HASH_END_BEFORE_HASH_START            0x84
>> +#define ST33ZP24_BAD_COMMAND_ORDER                     0x85
>> +#define ST33ZP24_INCORECT_RECEIVED_LENGTH              0x86
>> +#define ST33ZP24_TPM_FIFO_OVERFLOW                     0x89
>> +#define ST33ZP24_UNEXPECTED_READ_FIFO                  0x8A
>> +#define ST33ZP24_UNEXPECTED_WRITE_FIFO                 0x8B
>> +#define ST33ZP24_CMDRDY_SET_WHEN_PROCESSING_HASH_END   0x90
>> +#define ST33ZP24_DUMMY_BYTES                           0x00
>> +
>> +/*
>> + * TPM command can be up to 2048 byte, A TPM response can be up to
>> + * 1024 byte.
>> + * Between command and response, there are latency byte (up to 15
>> + * usually on st33zp24 2 are enough).
>> + *
>> + * Overall when sending a command and expecting an answer we need if
>> + * worst case:
>> + * 2048 (for the TPM command) + 1024 (for the TPM answer).  We need
>> + * some latency byte before the answer is available (max 15).
>> + * We have 2048 + 1024 + 15.
>> + */
>> +#define ST33ZP24_SPI_BUFFER_SIZE (TPM_BUFSIZE + (TPM_BUFSIZE / 2) +\
>> +                                 MAX_SPI_LATENCY)
>> +
>> +struct st33zp24_spi_phy {
>> +       struct spi_slave *spi_slave;
> Better to use struct udevice for new code.
>
>> +       int latency;
>> +
>> +       u8 tx_buf[ST33ZP24_SPI_BUFFER_SIZE];
>> +       u8 rx_buf[ST33ZP24_SPI_BUFFER_SIZE];
>> +};
>> +
>> +static int st33zp24_status_to_errno(u8 code)
>> +{
>> +       switch (code) {
>> +       case ST33ZP24_OK:
>> +               return 0;
>> +       case ST33ZP24_UNDEFINED_ERR:
>> +       case ST33ZP24_BADLOCALITY:
>> +       case ST33ZP24_TISREGISTER_UKNOWN:
>> +       case ST33ZP24_LOCALITY_NOT_ACTIVATED:
>> +       case ST33ZP24_HASH_END_BEFORE_HASH_START:
>> +       case ST33ZP24_BAD_COMMAND_ORDER:
>> +       case ST33ZP24_UNEXPECTED_READ_FIFO:
>> +       case ST33ZP24_UNEXPECTED_WRITE_FIFO:
>> +       case ST33ZP24_CMDRDY_SET_WHEN_PROCESSING_HASH_END:
>> +               return -EPROTO;
>> +       case ST33ZP24_INCORECT_RECEIVED_LENGTH:
>> +       case ST33ZP24_TPM_FIFO_OVERFLOW:
>> +               return -EMSGSIZE;
>> +       case ST33ZP24_DUMMY_BYTES:
>> +               return -ENOSYS;
>> +       }
>> +       return code;
>> +}
>> +
>> +/*
>> + * st33zp24_spi_send
>> + * Send byte to TPM register according to the ST33ZP24 SPI protocol.
>> + * @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 st33zp24_spi_send(void *phy_id, u8 tpm_register, u8 *tpm_data,
>> +                            int tpm_size)
>> +{
>> +       int total_length = 0, ret;
>> +       struct st33zp24_spi_phy *phy = phy_id;
>> +
>> +       struct spi_slave *dev = phy->spi_slave;
>> +       u8 *tx_buf = (u8 *)phy->tx_buf;
>> +       u8 *rx_buf = phy->rx_buf;
>> +
>> +       tx_buf[total_length++] = TPM_WRITE_DIRECTION | LOCALITY0;
>> +       tx_buf[total_length++] = tpm_register;
>> +
>> +       if (tpm_size > 0 && tpm_register == TPM_DATA_FIFO) {
>> +               tx_buf[total_length++] = tpm_size >> 8;
>> +               tx_buf[total_length++] = tpm_size;
>> +       }
>> +       memcpy(tx_buf + total_length, tpm_data, tpm_size);
>> +       total_length += tpm_size;
>> +
>> +       memset(tx_buf + total_length, TPM_DUMMY_BYTE, phy->latency);
>> +
>> +       total_length += phy->latency;
>> +
>> +       spi_claim_bus(dev);
>> +       ret = spi_xfer(dev, total_length * 8, tx_buf, rx_buf,
>> +                      SPI_XFER_BEGIN | SPI_XFER_END);
>> +       spi_release_bus(dev);
>> +
>> +       if (ret == 0)
>> +               ret = rx_buf[total_length - 1];
>> +
>> +       return st33zp24_status_to_errno(ret);
>> +} /* st33zp24_spi_send() */
>> +
>> +/*
>> + * 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 read8_reg(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size)
>> +{
>> +       int total_length = 0, nbr_dummy_bytes, ret;
>> +       struct st33zp24_spi_phy *phy = phy_id;
>> +       struct spi_slave *dev = phy->spi_slave;
>> +       u8 *tx_buf = (u8 *)phy->tx_buf;
>> +       u8 *rx_buf = phy->rx_buf;
>> +
>> +       /* Pre-Header */
>> +       tx_buf[total_length++] = LOCALITY0;
>> +       tx_buf[total_length++] = tpm_register;
>> +
>> +       nbr_dummy_bytes = phy->latency;
>> +       memset(&tx_buf[total_length], TPM_DUMMY_BYTE,
>> +              nbr_dummy_bytes + tpm_size);
>> +       total_length += nbr_dummy_bytes + tpm_size;
>> +
>> +       spi_claim_bus(dev);
> Error checking?
>
>> +       ret = spi_xfer(dev, total_length * 8, tx_buf, rx_buf,
>> +                      SPI_XFER_BEGIN | SPI_XFER_END);
>> +       spi_release_bus(dev);
>> +
>> +       if (tpm_size > 0 && ret == 0) {
>> +               ret = rx_buf[total_length - tpm_size - 1];
>> +               memcpy(tpm_data, rx_buf + total_length - tpm_size, tpm_size);
>> +       }
>> +       return ret;
>> +} /* read8_reg() */
>> +
>> +/*
>> + * st33zp24_spi_recv
>> + * Recv byte from the TIS register according to the ST33ZP24 SPI protocol.
>> + * @param: phy_id, the phy description
>> + * @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: number of byte read successfully: should be one if success.
>> + */
>> +static int st33zp24_spi_recv(void *phy_id, u8 tpm_register, u8 *tpm_data,
>> +                            int tpm_size)
>> +{
>> +       int ret;
>> +
>> +       ret = read8_reg(phy_id, tpm_register, tpm_data, tpm_size);
>> +       if (!st33zp24_status_to_errno(ret))
>> +               return tpm_size;
>> +       return ret;
>> +} /* st33zp24_spi_recv() */
>> +
>> +static int evaluate_latency(void *phy_id)
>> +{
>> +       struct st33zp24_spi_phy *phy = phy_id;
>> +       int latency = 1, status = 0;
>> +       u8 data = 0;
>> +
>> +       while (!status && latency < MAX_SPI_LATENCY) {
>> +               phy->latency = latency;
>> +               status = read8_reg(phy_id, TPM_INTF_CAPABILITY, &data, 1);
>> +               latency++;
>> +       }
>> +       return latency - 1;
>> +} /* evaluate_latency() */
>> +
>> +static const struct st33zp24_phy_ops spi_phy_ops = {
>> +       .send = st33zp24_spi_send,
>> +       .recv = st33zp24_spi_recv,
>> +};
> Again I'm not sure of the benefit of this indirection.
>
Is your meaning is to declare a new uclass for ST33ZP24 registering to 
uclass TPM.
phy i2c or spi will then have to register to uclass ST33ZP24 ?
>> +
>> +static int st33zp24_spi_probe(struct udevice *dev)
>> +{
>> +       struct st33zp24_spi_platdata *platdata = dev_get_platdata(dev);
>> +       struct st33zp24_spi_phy *spi_phy = dev_get_priv(dev);
>> +
>> +       #ifndef CONFIG_OF_CONTROL
> Do we need to support this case?
>
>> +       spi_phy->spi_slave = spi_setup_slave(platdata->bus_num,
>> +                                            platdata->cs,
>> +                                            platdata->max_speed,
>> +                                            platdata->mode);
>> +       #endif
>> +
>> +       spi_phy->latency = evaluate_latency(spi_phy);
>> +       if (spi_phy->latency <= 0)
>> +               return -ENODEV;
>> +
>> +       return st33zp24_init(dev, spi_phy, &spi_phy_ops);
>> +} /* st33zp24_spi_probe() */
>> +
>> +static int st33zp24_spi_remove(struct udevice *dev)
>> +{
>> +       return st33zp24_remove(dev);
>> +}
>> +
>> +#ifdef CONFIG_OF_CONTROL
>> +static const struct udevice_id st33zp24_spi_ids[] = {
>> +       { .compatible = "st,st33zp24-spi" },
>> +       { }
>> +};
>> +
>> +static int st33zp24_spi_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       int node, parent;
>> +       int i2c_bus;
>> +       const void *blob = gd->fdt_blob;
>> +       struct st33zp24_spi_platdata *platdata = dev_get_platdata(dev);
>> +       struct st33zp24_spi_phy *spi_phy = dev_get_priv(dev);
>> +
>> +       node = fdtdec_next_compatible(blob, 0,
>> +                               COMPAT_STMICROELECTRONICS_ST33ZP24_SPI);
> See comments for the I2C case.
>
>> +       if (node < 0) {
>> +               debug("%s: Node not found\n", __func__);
>> +               return -1;
>> +       }
>> +
>> +       parent = fdt_parent_offset(blob, node);
>> +       if (parent < 0) {
>> +               debug("%s: Cannot find node parent\n", __func__);
>> +               return -1;
>> +       }
>> +
>> +       spi_phy->spi_slave = spi_setup_slave_fdt(blob, node, parent);
>> +       if (!spi_phy->spi_slave)
>> +               return -1;
>> +
>> +       return 0;
>> +}
>> +#endif
>> +
>> +U_BOOT_DRIVER(st33zp24_spi) = {
>> +       .name   = "st33zp24-spi",
>> +       .id     = UCLASS_TPM,
>> +       .of_match = of_match_ptr(st33zp24_spi_ids),
>> +       .ofdata_to_platdata = of_match_ptr(st33zp24_spi_ofdata_to_platdata),
>> +       .probe  = st33zp24_spi_probe,
>> +       .remove = st33zp24_spi_remove,
>> +       .priv_auto_alloc_size = sizeof(struct st33zp24_spi_phy),
>> +       .platdata_auto_alloc_size = sizeof(struct st33zp24_spi_platdata),
>> +};
>> diff --git a/drivers/tpm/st33zp24/st33zp24.c b/drivers/tpm/st33zp24/st33zp24.c
>> new file mode 100644
>> index 0000000..862b98a
>> --- /dev/null
>> +++ b/drivers/tpm/st33zp24/st33zp24.c
>> @@ -0,0 +1,454 @@
>> +/*
>> + * STMicroelectronics TPM ST33ZP24 UBOOT driver
>> + *
>> + * Copyright (C) 2015  STMicroelectronics
>> + *
>> + * Description: Device driver for ST33ZP24 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 Protocol Stack Specification version 1.2.0.
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <linux/types.h>
>> +#include <linux/compat.h>
>> +#include <malloc.h>
>> +#include <tpm.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <asm/unaligned.h>
>> +
>> +#include "st33zp24.h"
>> +#include "../tpm_private.h"
>> +
>> +#define TPM_ACCESS                     0x0
>> +#define TPM_STS                                0x18
>> +#define TPM_DATA_FIFO                  0x24
>> +
>> +#define LOCALITY0                      0
>> +
>> +enum st33zp24_access {
>> +       TPM_ACCESS_VALID = 0x80,
>> +       TPM_ACCESS_ACTIVE_LOCALITY = 0x20,
>> +       TPM_ACCESS_REQUEST_PENDING = 0x04,
>> +       TPM_ACCESS_REQUEST_USE = 0x02,
>> +};
>> +
>> +enum st33zp24_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 st33zp24_int_flags {
>> +       TPM_GLOBAL_INT_ENABLE = 0x80,
>> +       TPM_INTF_CMD_READY_INT = 0x080,
>> +       TPM_INTF_FIFO_AVALAIBLE_INT = 0x040,
>> +       TPM_INTF_WAKE_UP_READY_INT = 0x020,
>> +       TPM_INTF_LOCTPM_BUFSIZE4SOFTRELEASE_INT = 0x008,
>> +       TPM_INTF_LOCALITY_CHANGE_INT = 0x004,
>> +       TPM_INTF_STS_VALID_INT = 0x002,
>> +       TPM_INTF_DATA_AVAIL_INT = 0x001,
>> +};
>> +
>> +enum tis_defaults {
>> +       TIS_SHORT_TIMEOUT = 750,        /* ms */
>> +       TIS_LONG_TIMEOUT = 2000,        /* 2 sec */
>> +};
>> +
>> +struct st33zp24_dev {
>> +       void *phy_id;
>> +       const struct st33zp24_phy_ops *ops;
>> +};
>> +
>> +/*
>> + * release_locality release the active locality
>> + * @param: chip, the tpm chip description.
>> + */
>> +static void release_locality(struct tpm_chip *chip)
>> +{
>> +       struct st33zp24_dev *tpm_dev;
>> +       u8 data = TPM_ACCESS_ACTIVE_LOCALITY;
>> +
>> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
>> +
>> +       tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
>> +} /* release_locality() */
> drop that comment - please fix globally.
I will.
>> +
>> +/*
>> + * check_locality if the locality is active
>> + * @param: chip, the tpm chip description
>> + * @return: the active locality or -EACCES.
>> + */
>> +static int check_locality(struct tpm_chip *chip)
>> +{
>> +       struct st33zp24_dev *tpm_dev;
>> +       u8 data;
>> +       u8 status;
>> +
>> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
>> +
>> +       status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
>> +       if (status && (data &
>> +               (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) ==
>> +               (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID))
>> +               return chip->vendor.locality;
>> +
>> +       return -EACCES;
>> +} /* check_locality() */
>> +
>> +/*
>> + * request_locality request the TPM locality
>> + * @param: chip, the chip description
>> + * @return: the active locality or negative value.
>> + */
>> +static int request_locality(struct tpm_chip *chip)
>> +{
>> +       unsigned long start, stop;
>> +       long ret;
>> +       struct st33zp24_dev *tpm_dev;
>> +       u8 data;
>> +
>> +       if (check_locality(chip) == chip->vendor.locality)
>> +               return chip->vendor.locality;
>> +
>> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
>> +
>> +       data = TPM_ACCESS_REQUEST_USE;
>> +       ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       /* wait for locality activated */
>> +       start = get_timer(0);
>> +       stop = chip->vendor.timeout_a;
>> +       do {
>> +               if (check_locality(chip) >= 0)
>> +                       return chip->vendor.locality;
>> +               udelay(TPM_TIMEOUT * 1000);
>> +       } while  (get_timer(start) < stop);
>> +
>> +       return -EACCES;
>> +} /* request_locality() */
>> +
>> +/*
>> + * st33zp24_status return the TPM_STS register
>> + * @param: chip, the tpm chip description
>> + * @return: the TPM_STS register value.
>> + */
>> +static u8 st33zp24_status(struct tpm_chip *chip)
>> +{
>> +       struct st33zp24_dev *tpm_dev;
>> +       u8 data;
>> +
>> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
>> +
>> +       tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1);
>> +       return data;
>> +} /* st33zp24_status() */
>> +
>> +/*
>> + * get_burstcount return the burstcount address 0x19 0x1A
>> + * @param: chip, the chip description
>> + * return: the burstcount or -TPM_DRIVER_ERR in case of error.
>> + */
>> +static int get_burstcount(struct tpm_chip *chip)
>> +{
>> +       unsigned long start, stop;
>> +       int burstcnt, status;
>> +       u8 tpm_reg, temp;
>> +       struct st33zp24_dev *tpm_dev;
>> +
>> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
>> +
>> +       /* wait for burstcount */
>> +       start = get_timer(0);
>> +       stop = chip->vendor.timeout_d;
>> +       do {
>> +               tpm_reg = TPM_STS + 1;
>> +               status = tpm_dev->ops->recv(tpm_dev->phy_id, tpm_reg, &temp, 1);
>> +               if (status < 0)
>> +                       return -EBUSY;
>> +
>> +               tpm_reg = TPM_STS + 2;
>> +               burstcnt = temp;
>> +               status = tpm_dev->ops->recv(tpm_dev->phy_id, tpm_reg, &temp, 1);
>> +               if (status < 0)
>> +                       return -EBUSY;
>> +
>> +               burstcnt |= temp << 8;
>> +               if (burstcnt)
>> +                       return burstcnt;
>> +               udelay(TIS_SHORT_TIMEOUT * 1000);
>> +       } while (get_timer(start) < stop);
>> +       return -EBUSY;
>> +} /* get_burstcount() */
>> +
>> +/*
>> + * st33zp24_cancel, cancel the current command execution or
>> + * set STS to COMMAND READY.
>> + * @param: chip, tpm_chip description.
>> + */
>> +static void st33zp24_cancel(struct tpm_chip *chip)
>> +{
>> +       struct st33zp24_dev *tpm_dev;
>> +       u8 data;
>> +
>> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
>> +
>> +       data = TPM_STS_COMMAND_READY;
>> +       tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1);
>> +} /* st33zp24_cancel() */
>> +
>> +/*
>> + * 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 timeout
>> + * @param: status,
>> + * @return: the tpm status, 0 if success, -ETIME if timeout is reached.
>> + */
>> +static int wait_for_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
>> +                        int *status)
>> +{
>> +       unsigned long start, stop;
>> +
>> +       /* Check current status */
>> +       *status = st33zp24_status(chip);
>> +       if ((*status & mask) == mask)
>> +               return 0;
>> +
>> +       start = get_timer(0);
>> +       stop = timeout;
>> +       do {
>> +               udelay(TPM_TIMEOUT * 1000);
>> +               *status = st33zp24_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: the number of bytes read from TPM FIFO.
>> + */
>> +static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
>> +{
>> +       int size = 0, burstcnt, len, ret, status;
>> +       struct st33zp24_dev *tpm_dev;
>> +
>> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
>> +
>> +       while (size < count &&
>> +              wait_for_stat(chip,
>> +                            TPM_STS_DATA_AVAIL | TPM_STS_VALID,
>> +                            chip->vendor.timeout_c, &status) == 0) {
>> +               burstcnt = get_burstcount(chip);
>> +               if (burstcnt < 0)
>> +                       return burstcnt;
>> +               len = min_t(int, burstcnt, count - size);
>> +               ret = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_DATA_FIFO,
>> +                                        buf + size, len);
>> +               if (ret < 0)
>> +                       return ret;
>> +
>> +               size += len;
>> +       }
>> +       return size;
>> +} /* recv_data() */
>> +
>> +/*
>> + * st33zp24_recv received TPM response through TPM phy.
>> + * @param: chip, tpm_chip description.
>> + * @param: buf,        the buffer to store data.
>> + * @param: count, the number of bytes that can received (sizeof buf).
>> + * @return: Returns zero in case of success else -EIO.
>> + */
>> +static int st33zp24_recv(struct tpm_chip *chip, unsigned char *buf,
>> +                        size_t count)
>> +{
>> +       int size = 0;
>> +       int expected;
>> +
>> +       if (!chip)
>> +               return -ENODEV;
>> +
>> +       if (count < TPM_HEADER_SIZE) {
>> +               size = -EIO;
>> +               goto out;
>> +       }
>> +
>> +       size = recv_data(chip, buf, TPM_HEADER_SIZE);
>> +       if (size < TPM_HEADER_SIZE) {
>> +               printf("TPM error, unable to read header\n");
>> +               goto out;
>> +       }
>> +
>> +       expected = get_unaligned_be32(buf + 2);
>> +       if (expected > count) {
>> +               size = -EIO;
>> +               goto out;
>> +       }
>> +
>> +       size += recv_data(chip, &buf[TPM_HEADER_SIZE],
>> +                         expected - TPM_HEADER_SIZE);
>> +       if (size < expected) {
>> +               printf("TPM error, unable to read remaining bytes of result\n");
>> +               size = -EIO;
>> +               goto out;
>> +       }
>> +
>> +out:
>> +       st33zp24_cancel(chip);
>> +       release_locality(chip);
>> +       return size;
>> +} /* st33zp24_recv() */
>> +
>> +/*
>> + * st33zp24_send send TPM commands through TPM phy.
>> + * @param: chip, tpm_chip description.
>> + * @param: buf,        the buffer to send.
>> + * @param: len, the number of bytes to send.
>> + * @return: Returns zero in case of success else the negative error code.
>> + */
>> +static int st33zp24_send(struct tpm_chip *chip, u8 *buf,
>> +                        size_t len)
>> +{
>> +       u32 i, size;
>> +       int burstcnt, ret, status;
>> +       u8 data, tpm_stat;
>> +       struct st33zp24_dev *tpm_dev;
>> +
>> +       if (!chip)
>> +               return -ENODEV;
>> +       if (len < TPM_HEADER_SIZE)
>> +               return -EIO;
>> +
>> +       tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip);
>> +
>> +       ret = request_locality(chip);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       tpm_stat = st33zp24_status(chip);
>> +       if ((tpm_stat & TPM_STS_COMMAND_READY) == 0) {
>> +               st33zp24_cancel(chip);
>> +               if (wait_for_stat(chip, TPM_STS_COMMAND_READY,
>> +                                 chip->vendor.timeout_b, &status) < 0) {
>> +                       ret = -ETIME;
>> +                       goto out_err;
>> +               }
>> +       }
>> +
>> +       for (i = 0; i < len - 1;) {
>> +               burstcnt = get_burstcount(chip);
>> +               if (burstcnt < 0)
>> +                       return burstcnt;
>> +               size = min_t(int, len - i - 1, burstcnt);
>> +               ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_DATA_FIFO,
>> +                                        buf + i, size);
>> +               if (ret < 0)
>> +                       goto out_err;
>> +
>> +               i += size;
>> +       }
>> +
>> +       tpm_stat = st33zp24_status(chip);
>> +       if ((tpm_stat & TPM_STS_DATA_EXPECT) == 0) {
>> +               ret = -EIO;
>> +               goto out_err;
>> +       }
>> +
>> +       ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_DATA_FIFO,
>> +                                buf + len - 1, 1);
>> +       if (ret < 0)
>> +               goto out_err;
>> +
>> +       tpm_stat = st33zp24_status(chip);
>> +       if ((tpm_stat & TPM_STS_DATA_EXPECT) != 0) {
>> +               ret = -EIO;
>> +               goto out_err;
>> +       }
>> +
>> +       data = TPM_STS_GO;
>> +       ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1);
>> +       if (ret < 0)
>> +               goto out_err;
>> +
>> +       return len;
>> +
>> +out_err:
>> +       st33zp24_cancel(chip);
>> +       release_locality(chip);
>> +       return ret;
>> +} /* st33zp24_send() */
>> +
>> +static struct tpm_vendor_specific st33zp24_tpm = {
>> +       .recv = st33zp24_recv,
>> +       .send = st33zp24_send,
>> +       .cancel = st33zp24_cancel,
>> +       .status = st33zp24_status,
> Methods should go in a new uclass if needed.
It am fine with adding this in a TPM uclass.
>> +       .req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
>> +       .req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
>> +       .req_canceled = TPM_STS_COMMAND_READY,
>> +};
>> +
>> +int st33zp24_init(struct udevice *dev, void *phy_id,
>> +                 const struct st33zp24_phy_ops *ops)
>> +{
>> +       int ret;
>> +       struct tpm_chip *chip;
>> +       struct st33zp24_dev *tpm_dev;
>> +
>> +       chip = tpm_register_hardware(dev, &st33zp24_tpm);
>> +       if (chip < 0) {
>> +               ret = -ENODEV;
>> +               goto out_err;
>> +       }
>> +
>> +       tpm_dev = (struct st33zp24_dev *)malloc(sizeof(struct st33zp24_dev));
>> +       if (!tpm_dev) {
>> +               ret = -ENODEV;
>> +               goto out_err;
>> +       }
>> +
>> +       /* Disable interrupts (not supported) */
>> +       chip->vendor.irq = 0;
>> +
>> +       /* Default timeouts */
>> +       chip->vendor.timeout_a = TIS_SHORT_TIMEOUT;
>> +       chip->vendor.timeout_b = TIS_LONG_TIMEOUT;
>> +       chip->vendor.timeout_c = TIS_SHORT_TIMEOUT;
>> +       chip->vendor.timeout_d = TIS_SHORT_TIMEOUT;
>> +
>> +       chip->vendor.locality = LOCALITY0;
>> +       TPM_VPRIV(chip) = tpm_dev;
>> +       tpm_dev->phy_id = phy_id;
>> +       tpm_dev->ops = ops;
>> +
>> +       printf("ST33ZP24 TPM from STMicroelectronics found\n");
>> +       return 0;
>> +
>> +out_err:
>> +       return ret;
>> +}
>> +
>> +int st33zp24_remove(struct udevice *dev)
>> +{
>> +       struct tpm_chip *chip = dev_get_uclass_priv(dev);
>> +       struct st33zp24_dev *tpm_dev = TPM_VPRIV(chip);
>> +
>> +       release_locality(chip);
>> +       free(tpm_dev);
>> +       return 0;
>> +}
>> diff --git a/drivers/tpm/st33zp24/st33zp24.h b/drivers/tpm/st33zp24/st33zp24.h
>> new file mode 100644
>> index 0000000..4be06cb
>> --- /dev/null
>> +++ b/drivers/tpm/st33zp24/st33zp24.h
>> @@ -0,0 +1,29 @@
>> +/*
>> + * STMicroelectronics TPM UBOOT driver for TPM ST33ZP24
>> + *
>> + * Copyright (C) 2015  STMicroelectronics
>> + *
>> + * Description: Device driver for ST33ZP24 I2C 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 Protocol Stack Specification version 1.2.0.
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#ifndef __LOCAL_ST33ZP24_H__
>> +#define __LOCAL_ST33ZP24_H__
>> +
>> +#define TPM_WRITE_DIRECTION             0x80
>> +#define TPM_HEADER_SIZE                        10
>> +
>> +struct st33zp24_phy_ops {
>> +       int (*send)(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size);
>> +       int (*recv)(void *phy_id, u8 tpm_register, u8 *tpm_data, int tpm_size);
>> +};
>> +
>> +int st33zp24_init(struct udevice *dev, void *phy_id,
>> +                 const struct st33zp24_phy_ops *ops);
>> +int st33zp24_remove(struct udevice *dev);
>> +#endif /* __LOCAL_ST33ZP24_H__ */
>> diff --git a/include/dm/platform_data/st33zp24_i2c.h b/include/dm/platform_data/st33zp24_i2c.h
>> new file mode 100644
>> index 0000000..e71c9e3
>> --- /dev/null
>> +++ b/include/dm/platform_data/st33zp24_i2c.h
>> @@ -0,0 +1,28 @@
>> +/*
>> + * STMicroelectronics TPM Linux driver for TPM 1.2 ST33ZP24
>> + * Copyright (C) 2009 - 2015  STMicroelectronics
> Can we use SPDX?
Sure i will update the headers.
>
>> + *
>> + * 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, see <http://www.gnu.org/licenses/>.
>> + */
>> +#ifndef __ST33ZP24_I2C_H__
>> +#define __ST33ZP24_I2C_H__
>> +
>> +#define TPM_ST33ZP24_I2C               "st33zp24-i2c"
>> +
>> +struct st33zp24_i2c_platdata {
>> +       int i2c_bus;
>> +       uint8_t slave_addr;
>> +} __packed;
>> +
>> +#endif /* __ST33ZP24_I2C_H__ */
>> diff --git a/include/dm/platform_data/st33zp24_spi.h b/include/dm/platform_data/st33zp24_spi.h
>> new file mode 100644
>> index 0000000..82f560b
>> --- /dev/null
>> +++ b/include/dm/platform_data/st33zp24_spi.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * STMicroelectronics TPM Linux driver for TPM 1.2 ST33ZP24
>> + * Copyright (C) 2009 - 2015  STMicroelectronics
>> + *
>> + * 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, see <http://www.gnu.org/licenses/>.
>> + */
>> +#ifndef __ST33ZP24_SPI_H__
>> +#define __ST33ZP24_SPI_H__
>> +
>> +#define TPM_ST33ZP24_SPI               "st33zp24-spi"
>> +
>> +struct st33zp24_spi_platdata {
>> +       int bus_num;
>> +       int cs;
>> +       int max_speed;
>> +       int mode;
> Hopefully not needed.
How to declare a SPI device using platform then ? On beagleboard i was 
doing this:

static const struct st33zp24_spi_platdata beagle_tpm_spi = {
         .bus_num = 3,
         .cs = 0,
         .max_speed = 10000000,
         .mode = 0,
};

U_BOOT_DEVICE(beagle_tpm_spi) = {
         .name = TPM_ST33ZP24_SPI,
         .platdata = &beagle_tpm_spi,
};

>
>> +};
>> +
>> +#endif /* __ST33ZP24_SPI_H__ */
>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>> index eac679e..9eb2b3d 100644
>> --- a/include/fdtdec.h
>> +++ b/include/fdtdec.h
>> @@ -182,7 +182,10 @@ enum fdt_compat_id {
>>          COMPAT_INTEL_PCH,               /* Intel PCH */
>>          COMPAT_INTEL_IRQ_ROUTER,        /* Intel Interrupt Router */
>>          COMPAT_ALTERA_SOCFPGA_DWMAC,    /* SoCFPGA Ethernet controller */
>> -
>> +       COMPAT_STMICROELECTRONICS_ST33ZP24_I2C,
>> +                                       /* STMicroelectronics ST33ZP24 I2C TPM */
>> +       COMPAT_STMICROELECTRONICS_ST33ZP24_SPI,
>> +                                       /* STMicroelectronics ST33ZP24 SPI TPM */
> Shouldn't be needed.
According to your previous comments, i think i understand why. Did you 
update Infineon id as well ?
>
>>          COMPAT_COUNT,
>>   };
>>
>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> index b201787..55c64a0 100644
>> --- a/lib/fdtdec.c
>> +++ b/lib/fdtdec.c
>> @@ -76,6 +76,8 @@ static const char * const compat_names[COMPAT_COUNT] = {
>>          COMPAT(COMPAT_INTEL_PCH, "intel,bd82x6x"),
>>          COMPAT(COMPAT_INTEL_IRQ_ROUTER, "intel,irq-router"),
>>          COMPAT(ALTERA_SOCFPGA_DWMAC, "altr,socfpga-stmmac"),
>> +       COMPAT(STMICROELECTRONICS_ST33ZP24_I2C, "st,st33zp24-i2c"),
>> +       COMPAT(STMICROELECTRONICS_ST33ZP24_SPI, "st,st33zp24-spi"),
>>   };
>>
>>   const char *fdtdec_get_compatible(enum fdt_compat_id id)
>> --
>> 2.1.4
>>
> Regards,
> Simon
Best Regards
Christophe

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/3] tpm: Initial work to introduce TPM driver model
  2015-08-13 20:37     ` Christophe Ricard
@ 2015-08-13 22:53       ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2015-08-13 22:53 UTC (permalink / raw
  To: u-boot

Hi Christophe,

On 13 August 2015 at 14:37, Christophe Ricard
<christophe.ricard@gmail.com> wrote:
> Hi Simon,
>
>
> On 13/08/2015 17:55, Simon Glass wrote:
>>
>> Hi Christophe,
>>
>> On 9 August 2015 at 07:19, Christophe Ricard
>> <christophe.ricard@gmail.com> wrote:
>>>
>>> drivers/tpm/tpm.c is a TPM core driver port from Linux.
>>> So far in u-boot only infineon i2c driver is using it but it could fit
>>> for others...
>>>
>>> Introduce a new tpm uclass so that every TPM driver can register against
>>> it and
>>> and take benefit of common functions and data such as tpm_transmit,
>>> tpm_register_hardware & tpm_remove_hardware.
>>> Finally tis_init, tis_open, tis_close, tis_sendrecv are using ops
>>> allowing
>>> to introduce proprietary instructions.
>>> Also this patch convert tpm_i2c_infineon for using this tpm uclass.
>>>
>>> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
>>> ---
>>>
>>>   README                                      |   8 +-
>>>   drivers/tpm/Makefile                        |   2 +-
>>>   drivers/tpm/tpm.c                           | 275
>>> +++++++---------------------
>>>   drivers/tpm/tpm_i2c_infineon.c              | 271
>>> ++++++++++++++++-----------
>>>   drivers/tpm/tpm_private.h                   |  23 ++-
>>>   include/dm/platform_data/tpm_i2c_infineon.h |  23 +++
>>>   include/dm/uclass-id.h                      |   1 +
>>>   7 files changed, 270 insertions(+), 333 deletions(-)
>>>   create mode 100644 include/dm/platform_data/tpm_i2c_infineon.h
>>
>> There is a lot going on in this patch - in general I think it is
>> better to split things up so that each patch does one thing.
>
> I understand. I have basically seen in your work more or less the same
> approach as far as TPM class.
> I believe, according to your plan, i can wait for you to update you tree
> integrating my first comments on tpm-working branch or
> if you prefer i can send a merge between your work and my comments.
>
> What's your prefered option ?

I'll update based on your comments and send out a series by mid next week.

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/3] tpm: Add st33zp24 tpm with i2c and spi phy
  2015-08-13 20:59     ` Christophe Ricard
@ 2015-08-13 22:53       ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2015-08-13 22:53 UTC (permalink / raw
  To: u-boot

Hi Christophe,

On 13 August 2015 at 14:59, Christophe Ricard
<christophe.ricard@gmail.com> wrote:
> Hi Simon,
>
>
> On 13/08/2015 17:55, Simon Glass wrote:
>>
>> Hi Christophe,
>>
>> On 9 August 2015 at 07:19, Christophe Ricard
>> <christophe.ricard@gmail.com> wrote:
>>>
>>> Add TPM st33zp24 tpm with i2c and spi phy. This is a port from Linux.
>>> This driver relies on tpm uclass.
>>>
>>> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
>>> ---
>>>
>>>   README                                  |  11 +
>>>   drivers/tpm/Makefile                    |   1 +
>>>   drivers/tpm/st33zp24/Makefile           |   7 +
>>>   drivers/tpm/st33zp24/i2c.c              | 226 ++++++++++++++++
>>>   drivers/tpm/st33zp24/spi.c              | 286 ++++++++++++++++++++
>>>   drivers/tpm/st33zp24/st33zp24.c         | 454
>>> ++++++++++++++++++++++++++++++++
>>>   drivers/tpm/st33zp24/st33zp24.h         |  29 ++
>>>   include/dm/platform_data/st33zp24_i2c.h |  28 ++
>>>   include/dm/platform_data/st33zp24_spi.h |  30 +++
>>>   include/fdtdec.h                        |   5 +-
>>>   lib/fdtdec.c                            |   2 +
>>>   11 files changed, 1078 insertions(+), 1 deletion(-)
>>>   create mode 100644 drivers/tpm/st33zp24/Makefile
>>>   create mode 100644 drivers/tpm/st33zp24/i2c.c
>>>   create mode 100644 drivers/tpm/st33zp24/spi.c
>>>   create mode 100644 drivers/tpm/st33zp24/st33zp24.c
>>>   create mode 100644 drivers/tpm/st33zp24/st33zp24.h
>>>   create mode 100644 include/dm/platform_data/st33zp24_i2c.h
>>>   create mode 100644 include/dm/platform_data/st33zp24_spi.h
>>>
>>> diff --git a/README b/README
>>> index 506ff6c..d3f9590 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -1499,6 +1499,17 @@ The following options need to be configured:
>>>                          CONFIG_TPM_TIS_I2C_BURST_LIMITATION
>>>                          Define the burst count bytes upper limit
>>>
>>> +               CONFIG_TPM_ST33ZP24
>>> +               Support for STMicroelectronics TPM devices. Requires
>>> DM_TPM support.
>>> +
>>> +                       CONFIG_TPM_ST33ZP24_I2C
>>> +                       Support for STMicroelectronics ST33ZP24 I2C
>>> devices.
>>> +                       Requires TPM_ST33ZP24 and I2C.
>>> +
>>> +                       CONFIG_TPM_ST33ZP24_SPI
>>> +                       Support for STMicroelectronics ST33ZP24 SPI
>>> devices.
>>> +                       Requires TPM_ST33ZP24 and SPI.
>>> +
>>
>> These can go in Kconfig
>
> Ok, your are correct, i will update this in a future v2.
>
>>>                  CONFIG_TPM_ATMEL_TWI
>>>                  Support for Atmel TWI TPM device. Requires I2C support.
>>>
>>> diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
>>> index bd2cd6d..48bc5f3 100644
>>> --- a/drivers/tpm/Makefile
>>> +++ b/drivers/tpm/Makefile
>>> @@ -9,3 +9,4 @@ obj-$(CONFIG_DM_TPM) += tpm.o
>>>   obj-$(CONFIG_TPM_INFINEON_I2C) += tpm_i2c_infineon.o
>>>   obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
>>>   obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o
>>> +obj-$(CONFIG_TPM_ST33ZP24) += st33zp24/
>>> diff --git a/drivers/tpm/st33zp24/Makefile
>>> b/drivers/tpm/st33zp24/Makefile
>>> new file mode 100644
>>> index 0000000..ed28e8c
>>> --- /dev/null
>>> +++ b/drivers/tpm/st33zp24/Makefile
>>> @@ -0,0 +1,7 @@
>>> +#
>>> +# Makefile for ST33ZP24 TPM 1.2 driver
>>> +#
>>> +
>>> +obj-$(CONFIG_TPM_ST33ZP24) += st33zp24.o
>>> +obj-$(CONFIG_TPM_ST33ZP24_I2C) += i2c.o
>>> +obj-$(CONFIG_TPM_ST33ZP24_SPI) += spi.o
>>> diff --git a/drivers/tpm/st33zp24/i2c.c b/drivers/tpm/st33zp24/i2c.c
>>> new file mode 100644
>>> index 0000000..204021a
>>> --- /dev/null
>>> +++ b/drivers/tpm/st33zp24/i2c.c
>>> @@ -0,0 +1,226 @@
>>> +/*
>>> + * STMicroelectronics TPM ST33ZP24 I2C phy for UBOOT
>>> + * Copyright (C) 2015  STMicroelectronics
>>> + *
>>> + * Description: Device driver for ST33ZP24 I2C 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 I2C Protocol Stack Specification version 1.2.0.
>>> + *
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <i2c.h>
>>> +#include <dm.h>
>>> +#include <linux/types.h>
>>
>> That may not be needed, but if so should go after the dm/platform_data...
>>
>>> +#include <malloc.h>
>>> +#include <tpm.h>
>>> +#include <errno.h>
>>
>> Should go up under common.h
>
> ok.
>>>
>>> +#include <asm/unaligned.h>
>>> +#include <dm/platform_data/st33zp24_i2c.h>
>>> +
>>> +#include "../tpm_private.h"
>>> +#include "st33zp24.h"
>>> +
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +#define TPM_DUMMY_BYTE 0xAA
>>> +#define TPM_ST33ZP24_I2C_SLAVE_ADDR 0x13
>>> +
>>> +struct st33zp24_i2c_phy {
>>> +       uint8_t slave_addr;
>>> +       int i2c_bus;
>>> +       int old_bus;
>>> +       u8 buf[TPM_BUFSIZE + 1];
>>> +} __packed;
>>
>> Should not need address and bus - just use a struct udevice. Also, why
>> __packed?
>
> What if my board (beagleboard xm) does not support DM_I2C ? Should i
> concider a new one ? (Any recommendation ?)
> How do you attach a I2C device using platform_data approach ?
> I was doing this in board/ti/beagle/beagle.c:
>
> static const struct st33zp24_i2c_platdata beagle_tpm_i2c = {
>         .i2c_bus = 1,
>         .slave_addr = 0x13,
> };
>
> U_BOOT_DEVICE(beagle_tpm_i2c) = {
>         .name = TPM_ST33ZP24_I2C,
>         .platdata = &beagle_tpm_i2c,
> };

We don't support I2C in driver model without device tree. We're trying
to encourage people to drop platform data and use device tree.

See am335x_boneblack_vboot. It should be fairly straightfoward to
convert over the omap_24xx driver if you have time.

[snip]
>>> +
>>> +#define TPM_ST33ZP24_SPI               "st33zp24-spi"
>>> +
>>> +struct st33zp24_spi_platdata {
>>> +       int bus_num;
>>> +       int cs;
>>> +       int max_speed;
>>> +       int mode;
>>
>> Hopefully not needed.
>
> How to declare a SPI device using platform then ? On beagleboard i was doing
> this:
>
> static const struct st33zp24_spi_platdata beagle_tpm_spi = {
>         .bus_num = 3,
>         .cs = 0,
>         .max_speed = 10000000,
>         .mode = 0,
> };
>
> U_BOOT_DEVICE(beagle_tpm_spi) = {
>         .name = TPM_ST33ZP24_SPI,
>         .platdata = &beagle_tpm_spi,
> };

Again, you can't. There is a guide to porting SPI drivers in doc/driver-model.

>
>>
>>> +};
>>> +
>>> +#endif /* __ST33ZP24_SPI_H__ */
>>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>>> index eac679e..9eb2b3d 100644
>>> --- a/include/fdtdec.h
>>> +++ b/include/fdtdec.h
>>> @@ -182,7 +182,10 @@ enum fdt_compat_id {
>>>          COMPAT_INTEL_PCH,               /* Intel PCH */
>>>          COMPAT_INTEL_IRQ_ROUTER,        /* Intel Interrupt Router */
>>>          COMPAT_ALTERA_SOCFPGA_DWMAC,    /* SoCFPGA Ethernet controller
>>> */
>>> -
>>> +       COMPAT_STMICROELECTRONICS_ST33ZP24_I2C,
>>> +                                       /* STMicroelectronics ST33ZP24
>>> I2C TPM */
>>> +       COMPAT_STMICROELECTRONICS_ST33ZP24_SPI,
>>> +                                       /* STMicroelectronics ST33ZP24
>>> SPI TPM */
>>
>> Shouldn't be needed.
>
> According to your previous comments, i think i understand why. Did you
> update Infineon id as well ?
>>
>>
>>>          COMPAT_COUNT,
>>>   };
>>>
>>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>>> index b201787..55c64a0 100644
>>> --- a/lib/fdtdec.c
>>> +++ b/lib/fdtdec.c
>>> @@ -76,6 +76,8 @@ static const char * const compat_names[COMPAT_COUNT] =
>>> {
>>>          COMPAT(COMPAT_INTEL_PCH, "intel,bd82x6x"),
>>>          COMPAT(COMPAT_INTEL_IRQ_ROUTER, "intel,irq-router"),
>>>          COMPAT(ALTERA_SOCFPGA_DWMAC, "altr,socfpga-stmmac"),
>>> +       COMPAT(STMICROELECTRONICS_ST33ZP24_I2C, "st,st33zp24-i2c"),
>>> +       COMPAT(STMICROELECTRONICS_ST33ZP24_SPI, "st,st33zp24-spi"),
>>>   };
>>>
>>>   const char *fdtdec_get_compatible(enum fdt_compat_id id)
>>> --
>>> 2.1.4
>>>
>> Regards,
>> Simon
>
> Best Regards
> Christophe

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 1/3] tpm: Move tpm_tis_i2c to tpm_i2c_infineon
  2015-08-13 15:55   ` Simon Glass
@ 2015-08-30 22:45     ` Simon Glass
  2015-09-02 17:54       ` Christophe Ricard
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2015-08-30 22:45 UTC (permalink / raw
  To: u-boot

Hi Chrisophe,

On 13 August 2015 at 09:55, Simon Glass <sjg@chromium.org> wrote:
> On 9 August 2015 at 07:19, Christophe Ricard
> <christophe.ricard@gmail.com> wrote:
>> As there is no TCG specification or recommendation for i2c TPM 1.2, move
>> tpm_tis_i2c driver to tpm_i2c_infineon. Other tpm vendors like atmel or
>> stmicroelectronics may have a different transport protocol for i2c.
>>
>> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
>> ---
>>
>>  README                                            | 4 ++--
>>  drivers/tpm/Makefile                              | 2 +-
>>  drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} | 0
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>  rename drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} (100%)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Can you please take another look at this patch?

You need to change existing users of this config to use your renamed
Kconfig. This means updating things in the configs/ directory.

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 1/3] tpm: Move tpm_tis_i2c to tpm_i2c_infineon
  2015-08-30 22:45     ` Simon Glass
@ 2015-09-02 17:54       ` Christophe Ricard
  2015-09-08 14:20         ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: Christophe Ricard @ 2015-09-02 17:54 UTC (permalink / raw
  To: u-boot

Hi Simon,

Apologies for the delay. I will try to rework this patch end of this 
week and send it back to you middle of next week.

Best Regards
Christophe

Le 31/08/2015 00:45, Simon Glass a ?crit :
> Hi Chrisophe,
>
> On 13 August 2015 at 09:55, Simon Glass <sjg@chromium.org> wrote:
>> On 9 August 2015 at 07:19, Christophe Ricard
>> <christophe.ricard@gmail.com> wrote:
>>> As there is no TCG specification or recommendation for i2c TPM 1.2, move
>>> tpm_tis_i2c driver to tpm_i2c_infineon. Other tpm vendors like atmel or
>>> stmicroelectronics may have a different transport protocol for i2c.
>>>
>>> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
>>> ---
>>>
>>>   README                                            | 4 ++--
>>>   drivers/tpm/Makefile                              | 2 +-
>>>   drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} | 0
>>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>>   rename drivers/tpm/{tpm_tis_i2c.c => tpm_i2c_infineon.c} (100%)
>> Reviewed-by: Simon Glass <sjg@chromium.org>
> Can you please take another look at this patch?
>
> You need to change existing users of this config to use your renamed
> Kconfig. This means updating things in the configs/ directory.
>
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 1/3] tpm: Move tpm_tis_i2c to tpm_i2c_infineon
  2015-09-02 17:54       ` Christophe Ricard
@ 2015-09-08 14:20         ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2015-09-08 14:20 UTC (permalink / raw
  To: u-boot

Hi Christophe,

On 2 September 2015 at 11:54, Christophe Ricard
<christophe.ricard@gmail.com> wrote:
> Hi Simon,
>
> Apologies for the delay. I will try to rework this patch end of this week
> and send it back to you middle of next week.

OK thanks, sounds good.

[snip]

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-09-08 14:20 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-09 13:19 [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs Christophe Ricard
2015-08-09 13:19 ` [U-Boot] [PATCH 1/3] tpm: Move tpm_tis_i2c to tpm_i2c_infineon Christophe Ricard
2015-08-13 15:55   ` Simon Glass
2015-08-30 22:45     ` Simon Glass
2015-09-02 17:54       ` Christophe Ricard
2015-09-08 14:20         ` Simon Glass
2015-08-09 13:19 ` [U-Boot] [PATCH 2/3] tpm: Initial work to introduce TPM driver model Christophe Ricard
2015-08-13 15:55   ` Simon Glass
2015-08-13 20:37     ` Christophe Ricard
2015-08-13 22:53       ` Simon Glass
2015-08-09 13:19 ` [U-Boot] [PATCH 3/3] tpm: Add st33zp24 tpm with i2c and spi phy Christophe Ricard
2015-08-13 15:55   ` Simon Glass
2015-08-13 20:59     ` Christophe Ricard
2015-08-13 22:53       ` Simon Glass
2015-08-09 13:28 ` [U-Boot] [PATCH 0/3] Introduce TPM driver model and STMicroelectronics ST33ZP24 TPMs Simon Glass
2015-08-09 14:19   ` Christophe Ricard
2015-08-09 15:12     ` Simon Glass

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).