Hi Simon,
I think we are pretty inline for the uclass. Please find below some few remarks.
On 11/08/2015 16:48, Simon Glass wrote:
Add a new uclass for TPMs which uses almost the same TIS (TPM Interface Specification) as is currently implemented. Since init() is handled by the normal driver model probe() method, we don't need to implement that. Also rename the transfer method to xfer() which is a less clumbsy name.
Once all drivers and users are converted to driver model we can remove the old code.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/tpm/Kconfig | 9 +++++ drivers/tpm/Makefile | 2 + drivers/tpm/tpm-uclass.c | 57 ++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/tis.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 166 insertions(+) create mode 100644 drivers/tpm/tpm-uclass.c
diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig index 993d2d7..800239e 100644 --- a/drivers/tpm/Kconfig +++ b/drivers/tpm/Kconfig @@ -1,3 +1,12 @@ +config DM_TPM
- bool "Enable driver model for Trusted Platform Module drivers"
- depends on DM && TPM
- help
Enable driver model for TPMs. The TIS interface (tis_open(),tis_sendrecv(), etc.) is then implemented by the TPM uclass. Notethat even with driver model only a single TPM is currentlysupported, since the tpm library assumes this.- config TPM_TIS_SANDBOX bool "Enable sandbox TPM driver" depends on SANDBOX
diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile index 597966c..0d328f8 100644 --- a/drivers/tpm/Makefile +++ b/drivers/tpm/Makefile @@ -3,6 +3,8 @@ # SPDX-License-Identifier: GPL-2.0+ #
+obj-$(CONFIG_DM_TPM) += tpm-uclass.o
- obj-$(CONFIG_TPM_ATMEL_TWI) += tpm_atmel_twi.o obj-$(CONFIG_TPM_TIS_I2C) += tpm_tis_i2c.o obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o
diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c new file mode 100644 index 0000000..ccade5b --- /dev/null +++ b/drivers/tpm/tpm-uclass.c @@ -0,0 +1,57 @@ +/*
- Copyright (c) 2015 Google, Inc
- Written by Simon Glass sjg@chromium.org
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <dm.h> +#include <tis.h>
+int tis_open(struct udevice *dev) +{
- struct tpm_ops *ops = tpm_get_ops(dev);
- if (!ops->open)
return -ENOSYS;- return ops->open(dev);
+}
+int tis_close(struct udevice *dev) +{
- struct tpm_ops *ops = tpm_get_ops(dev);
- if (!ops->close)
return -ENOSYS;- return ops->close(dev);
+}
+int tis_get_desc(struct udevice *dev, char *buf, int size) +{
- struct tpm_ops *ops = tpm_get_ops(dev);
- if (!ops->get_desc)
return -ENOSYS;- return ops->get_desc(dev, buf, size);
+}
+int tis_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size,
- uint8_t *recvbuf, size_t *recv_size)
+{
- struct tpm_ops *ops = tpm_get_ops(dev);
- if (!ops->xfer)
return -ENOSYS;- return ops->xfer(dev, sendbuf, send_size, recvbuf, recv_size);
+}
tis_xfer could be more generic and rely on tpm_transmit from original tpm.c. The command duration could be calculated at probe time during driver initialisation running one single getcapability command.
+UCLASS_DRIVER(tpm) = {
- .id = UCLASS_TPM,
- .name = "tpm",
- .flags = DM_UC_FLAG_SEQ_ALIAS,
+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index c744044..3eff895 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -54,6 +54,7 @@ enum uclass_id { UCLASS_SPI_GENERIC, /* Generic SPI flash target */ UCLASS_SYSCON, /* System configuration device */ UCLASS_THERMAL, /* Thermal sensor */
- UCLASS_TPM, /* Trusted Platform Module TIS interface */ UCLASS_USB, /* USB bus */ UCLASS_USB_DEV_GENERIC, /* USB generic device */ UCLASS_USB_HUB, /* USB hub */
diff --git a/include/tis.h b/include/tis.h index 40a1f86..6620554 100644 --- a/include/tis.h +++ b/include/tis.h @@ -7,6 +7,102 @@ #ifndef __TIS_H #define __TIS_H
+#ifdef CONFIG_DM_TPM +struct tpm_ops {
As per a previous comment, an init handler could be usefull.
- /**
* open() - Request access to locality 0 for the caller** After all commands have been completed the caller should call* tis_close().** @dev: Device to close* @return 0 ok OK, -ve on error*/- int (*open)(struct udevice *dev);
- /**
* tis_close() - Close the current session** Releasing the locked locality. Returns 0 on success, -ve 1 on* failure (in case lock removal did not succeed).** @dev: Device to close* @return 0 ok OK, -ve on error*/- int (*close)(struct udevice *dev);
- /**
* get_desc() - Get a text description of the TPM** @dev: Device to check* @buf: Buffer to put the string* @size: Maximum size of buffer* @return length of string, or -ENOSPC it no space*/- int (*get_desc)(struct udevice *dev, char *buf, int size);
- /**
* xfer() - send data to the TPM and get response** @dev: Device to talk to* @sendbuf: Buffer of the data to send* @send_size: Size of the data to send* @recvbuf: Buffer to save the response to* @recv_size: Pointer to the size of the response buffer** Returns 0 on success (and places the number of response bytes at* recv_size) or -ve on failure.*/- int (*xfer)(struct udevice *dev, const uint8_t *sendbuf,
size_t send_size, uint8_t *recvbuf, size_t *recv_size);+};
+#define tpm_get_ops(dev) ((struct tpm_ops *)(dev)->driver->ops)
why not device_get_ops(dev) ?
+/*
- open() - Request access to locality 0 for the caller
- After all commands have been completed the caller is supposed to
- call tis_close().
- Returns 0 on success, -ve on failure.
- */
+int tis_open(struct udevice *dev);
+/*
- tis_close() - Close the current session
- Releasing the locked locality. Returns 0 on success, -ve 1 on
- failure (in case lock removal did not succeed).
- */
+int tis_close(struct udevice *dev);
+/**
- tis_get_desc() - Get a text description of the TPM
- @dev: Device to check
- @buf: Buffer to put the string
- @size: Maximum size of buffer
- @return length of string, or -ENOSPC it no space
- */
+int tis_get_desc(struct udevice *dev, char *buf, int size);
+/*
- tis_sendrecv() - send data to the TPM and get response
- @sendbuf - buffer of the data to send
- @send_size size of the data to send
- @recvbuf - memory to save the response to
- @recv_len - pointer to the size of the response buffer
- Returns 0 on success (and places the number of response bytes at
- recv_len) or -ve on failure.
- */
+int tis_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size,
uint8_t *recvbuf, size_t *recv_size);
As at the moment there is a 1 - 1 link with TPM and a platform, are you sure udevice should be a parameter ?
+#else
#include <common.h>
/* Low-level interface to access TPM */
@@ -53,5 +149,6 @@ int tis_close(void); */ int tis_sendrecv(const uint8_t *sendbuf, size_t send_size, uint8_t *recvbuf, size_t *recv_len); +#endif
#endif /* __TIS_H */
Best Regards Christophe