Hi Simon,

On 13/08/2015 17:55, Simon Glass wrote:
Hi Christophe,

On 9 August 2015 at 07:19, Christophe Ricard
<[email protected]> 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 <[email protected]>
---

  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

_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to