[U-Boot] [PATCH 0/6] Allow both TPM stacks to be compiled at the same time

[U-Boot] [PATCH 0/6] Allow both TPM stacks to be compiled at the same time

[Miquel Raynal]

It was first decided that the user would choose the TPM version (1.x
or 2.x) that he wants to use at compile time. After some time the
decision has been made to change this and allow both stacks (and
drivers) to be compiled at the same time. The primary benefit would be
to be able to compile the maximum of code with only one Sandbox
configuration.

This series removes this limitation by creating a 'tpm2' command
(instead of two commands named 'tpm'). While test suites must be
updated to use 'tpm2' instead of 'tpm' (in case both stacks are
compiled), the user will not have to change anything as U-Boot prompt
will find the closest matching command by default.

In the TPM private data a version entry is added, updated by TPMv2
drivers in the ->set_version() hook added in the uclass.

This has been tested with the tpm2.py test suite on Sandbox with
either only the V1, only the V2 and with both versions selected.

Thanks,
Miquèl

Miquel Raynal (6):
  tpm: fix typo in kernel doc
  tpm: compile Sandbox driver by default
  tpm: allow TPM v1 and v2 to be compiled at the same time
  test/py: tpm2: switch from 'tpm' to 'tpm2' command
  tpm: make TPM_V2 be compiled by default
  sandbox: compile both TPM stack versions and drivers

 cmd/tpm-common.c               | 24 +++++++++++++++++++++++-
 cmd/tpm-v1.c                   |  2 +-
 cmd/tpm-v2.c                   |  4 ++--
 configs/sandbox_defconfig      |  3 +++
 drivers/tpm/Kconfig            | 10 +++++-----
 drivers/tpm/tpm-uclass.c       | 13 ++++++++++---
 drivers/tpm/tpm2_tis_sandbox.c | 11 +++++++++++
 drivers/tpm/tpm2_tis_spi.c     | 15 +++++++++++++++
 include/tpm-common.h           | 38 +++++++++++++++++++++++++++++++-------
 lib/tpm-common.c               |  4 ++++
 lib/tpm-utils.h                |  9 +++++++++
 test/py/tests/test_tpm2.py     | 40 ++++++++++++++++++++--------------------
 12 files changed, 134 insertions(+), 39 deletions(-)

-- 
2.14.1

[U-Boot] [PATCH 1/6] tpm: fix typo in kernel doc

[Miquel Raynal]

The udevice given to the open() function of course must be opened,
not closed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/tpm-common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/tpm-common.h b/include/tpm-common.h
index 734c2c9d53..68bf8fd627 100644
--- a/include/tpm-common.h
+++ b/include/tpm-common.h
@@ -71,7 +71,7 @@ struct tpm_ops {
 	 * After all commands have been completed the caller should call
 	 * close().
 	 *
-	 * @dev:	Device to close
+	 * @dev:	Device to open
 	 * @return 0 ok OK, -ve on error
 	 */
 	int (*open)(struct udevice *dev);
-- 
2.14.1

[U-Boot] [PATCH 2/6] tpm: compile Sandbox driver by default

[Miquel Raynal]

When Sandbox and the TPM stack are both selected, compile Sandbox TPM
driver by default.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/tpm/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
index 93264ddd34..5e3fb3267f 100644
--- a/drivers/tpm/Kconfig
+++ b/drivers/tpm/Kconfig
@@ -20,6 +20,7 @@ if TPM_V1 && !TPM_V2
 config TPM_TIS_SANDBOX
 	bool "Enable sandbox TPM driver"
 	depends on TPM_V1 && SANDBOX
+	default y
 	help
 	  This driver emulates a TPMv1.x, providing access to base functions
 	  such as reading and writing TPM private data. This is enough to
@@ -132,6 +133,7 @@ if TPM_V2 && !TPM_V1
 config TPM2_TIS_SANDBOX
 	bool "Enable sandbox TPMv2.x driver"
 	depends on TPM_V2 && SANDBOX
+	default y
 	select TPM_DRIVER_SELECTED
 	help
 	  This driver emulates a TPMv2.x, providing access to base functions
-- 
2.14.1

[U-Boot] [PATCH 3/6] tpm: allow TPM v1 and v2 to be compiled at the same time

[Miquel Raynal]

While there is probably no reason to do so in a real life situation, it
will allow to compile test both stacks with the same sandbox defconfig.

As we cannot define two 'tpm' commands at the same time, the command for
TPM v1 is still called 'tpm' and the one for TPM v2 'tpm2'. While this
is the exact command name that must be written into eg. test files, any
user already using the TPM v2 stack can continue using just 'tpm' as
command as long as TPM v1 support is not compiled (because U-Boot prompt
will search for the closest command named after 'tpm'.b)

The command set can also be changed at runtime (not supported yet, but
ready to be), but as one can compile only either one stack or the other,
there is still one spot in the code where conditionals are used: to
retrieve the v1 or v2 command set.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/tpm-common.c               | 24 +++++++++++++++++++++++-
 cmd/tpm-v1.c                   |  2 +-
 cmd/tpm-v2.c                   |  4 ++--
 drivers/tpm/Kconfig            |  7 ++-----
 drivers/tpm/tpm-uclass.c       | 13 ++++++++++---
 drivers/tpm/tpm2_tis_sandbox.c | 11 +++++++++++
 drivers/tpm/tpm2_tis_spi.c     | 15 +++++++++++++++
 include/tpm-common.h           | 36 ++++++++++++++++++++++++++++++------
 lib/tpm-common.c               |  4 ++++
 lib/tpm-utils.h                |  9 +++++++++
 10 files changed, 107 insertions(+), 18 deletions(-)

diff --git a/cmd/tpm-common.c b/cmd/tpm-common.c
index 6cf9fcc9ac..69cc02b599 100644
--- a/cmd/tpm-common.c
+++ b/cmd/tpm-common.c
@@ -273,12 +273,34 @@ int do_tpm_init(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 int do_tpm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	cmd_tbl_t *tpm_commands, *cmd;
+	struct tpm_chip_priv *priv;
+	struct udevice *dev;
 	unsigned int size;
+	int rc;
 
 	if (argc < 2)
 		return CMD_RET_USAGE;
 
-	tpm_commands = get_tpm_commands(&size);
+	rc = get_tpm(&dev);
+	if (rc)
+		return rc;
+
+	priv = dev_get_uclass_priv(dev);
+
+	switch (priv->version) {
+#if defined(CONFIG_TPM_V1)
+	case TPM_V1:
+		tpm_commands = get_tpm1_commands(&size);
+		break;
+#endif
+#if defined(CONFIG_TPM_V2)
+	case TPM_V2:
+		tpm_commands = get_tpm2_commands(&size);
+		break;
+#endif
+	default:
+		return CMD_RET_USAGE;
+	}
 
 	cmd = find_cmd_tbl(argv[1], tpm_commands, size);
 	if (!cmd)
diff --git a/cmd/tpm-v1.c b/cmd/tpm-v1.c
index 0874c4d7ba..69870002d4 100644
--- a/cmd/tpm-v1.c
+++ b/cmd/tpm-v1.c
@@ -608,7 +608,7 @@ static cmd_tbl_t tpm1_commands[] = {
 #endif /* CONFIG_TPM_LIST_RESOURCES */
 };
 
-cmd_tbl_t *get_tpm_commands(unsigned int *size)
+cmd_tbl_t *get_tpm1_commands(unsigned int *size)
 {
 	*size = ARRAY_SIZE(tpm1_commands);
 
diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
index 38add4f462..ffbf35a75c 100644
--- a/cmd/tpm-v2.c
+++ b/cmd/tpm-v2.c
@@ -319,14 +319,14 @@ static cmd_tbl_t tpm2_commands[] = {
 			 do_tpm_pcr_setauthvalue, "", ""),
 };
 
-cmd_tbl_t *get_tpm_commands(unsigned int *size)
+cmd_tbl_t *get_tpm2_commands(unsigned int *size)
 {
 	*size = ARRAY_SIZE(tpm2_commands);
 
 	return tpm2_commands;
 }
 
-U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm, "Issue a TPMv2.x command",
+U_BOOT_CMD(tpm2, CONFIG_SYS_MAXARGS, 1, do_tpm, "Issue a TPMv2.x command",
 "<command> [<arguments>]\n"
 "\n"
 "info\n"
diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
index 5e3fb3267f..704ab81e2d 100644
--- a/drivers/tpm/Kconfig
+++ b/drivers/tpm/Kconfig
@@ -4,9 +4,6 @@
 
 menu "TPM support"
 
-comment "Please select only one TPM revision"
-	depends on TPM_V1 && TPM_V2
-
 config TPM_V1
 	bool "TPMv1.x support"
 	depends on TPM
@@ -15,7 +12,7 @@ config TPM_V1
 	  Major TPM versions are not compatible at all, choose either
 	  one or the other. This option enables TPMv1.x drivers/commands.
 
-if TPM_V1 && !TPM_V2
+if TPM_V1
 
 config TPM_TIS_SANDBOX
 	bool "Enable sandbox TPM driver"
@@ -128,7 +125,7 @@ config TPM_V2
 	  Major TPM versions are not compatible at all, choose either
 	  one or the other. This option enables TPMv2.x drivers/commands.
 
-if TPM_V2 && !TPM_V1
+if TPM_V2
 
 config TPM2_TIS_SANDBOX
 	bool "Enable sandbox TPMv2.x driver"
diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c
index 412697eedc..a80f3df78b 100644
--- a/drivers/tpm/tpm-uclass.c
+++ b/drivers/tpm/tpm-uclass.c
@@ -7,13 +7,20 @@
 #include <common.h>
 #include <dm.h>
 #include <linux/unaligned/be_byteshift.h>
-#if defined(CONFIG_TPM_V1)
 #include <tpm-v1.h>
-#elif defined(CONFIG_TPM_V2)
 #include <tpm-v2.h>
-#endif
 #include "tpm_internal.h"
 
+int tpm_set_version(struct udevice *dev)
+{
+	struct tpm_ops *ops = tpm_get_ops(dev);
+
+	if (ops->set_version)
+		return ops->set_version(dev);
+
+	return 0;
+}
+
 int tpm_open(struct udevice *dev)
 {
 	struct tpm_ops *ops = tpm_get_ops(dev);
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c
index 3240cc5dba..0b8baad056 100644
--- a/drivers/tpm/tpm2_tis_sandbox.c
+++ b/drivers/tpm/tpm2_tis_sandbox.c
@@ -573,6 +573,16 @@ static int sandbox_tpm2_get_desc(struct udevice *dev, char *buf, int size)
 	return snprintf(buf, size, "Sandbox TPM2.x");
 }
 
+static int sandbox_tpm2_set_version(struct udevice *dev)
+{
+	struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
+
+	/* Use the TPM v2 stack */
+	priv->version = TPM_V2;
+
+	return 0;
+}
+
 static int sandbox_tpm2_open(struct udevice *dev)
 {
 	struct sandbox_tpm2 *tpm = dev_get_priv(dev);
@@ -604,6 +614,7 @@ static int sandbox_tpm2_close(struct udevice *dev)
 }
 
 static const struct tpm_ops sandbox_tpm2_ops = {
+	.set_version	= sandbox_tpm2_set_version,
 	.open		= sandbox_tpm2_open,
 	.close		= sandbox_tpm2_close,
 	.get_desc	= sandbox_tpm2_get_desc,
diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c
index c5d17a679d..3577f3501c 100644
--- a/drivers/tpm/tpm2_tis_spi.c
+++ b/drivers/tpm/tpm2_tis_spi.c
@@ -507,9 +507,23 @@ static int tpm_tis_spi_cleanup(struct udevice *dev)
 	return 0;
 }
 
+static int tpm_tis_spi_set_version(struct udevice *dev)
+{
+	struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
+
+	/* Use the TPM v2 stack */
+	priv->version = TPM_V2;
+
+	return 0;
+}
+
 static int tpm_tis_spi_open(struct udevice *dev)
 {
 	struct tpm_chip *chip = dev_get_priv(dev);
+	struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
+
+	/* Use the TPM v2 stack */
+	priv->version = TPM_V2;
 
 	if (chip->is_open)
 		return -EBUSY;
@@ -646,6 +660,7 @@ static int tpm_tis_spi_remove(struct udevice *dev)
 }
 
 static const struct tpm_ops tpm_tis_spi_ops = {
+	.set_version	= tpm_tis_spi_set_version,
 	.open		= tpm_tis_spi_open,
 	.close		= tpm_tis_spi_close,
 	.get_desc	= tpm_tis_get_desc,
diff --git a/include/tpm-common.h b/include/tpm-common.h
index 68bf8fd627..f39b58cd6c 100644
--- a/include/tpm-common.h
+++ b/include/tpm-common.h
@@ -26,6 +26,16 @@ enum tpm_duration {
 /* Max buffer size supported by our tpm */
 #define TPM_DEV_BUFSIZE		1260
 
+/**
+ * enum tpm_version - The version of the TPM stack to be used
+ * @TPM_V1:		Use TPM v1.x stack
+ * @TPM_V2:		Use TPM v2.x stack
+ */
+enum tpm_version {
+	TPM_V1 = 0,
+	TPM_V2,
+};
+
 /**
  * struct tpm_chip_priv - Information about a TPM, stored by the uclass
  *
@@ -33,20 +43,23 @@ enum tpm_duration {
  * communcation is attempted. If the device has an xfer() method, this is
  * not needed. There is no need to set up @buf.
  *
+ * @version:		TPM stack to be used
  * @duration_ms:	Length of each duration type in milliseconds
  * @retry_time_ms:	Time to wait before retrying receive
+ * @buf:		Buffer used during the exchanges with the chip
  * @pcr_count:		Number of PCR per bank
  * @pcr_select_min:	Minimum size in bytes of the pcrSelect array
- * @buf:		Buffer used during the exchanges with the chip
  */
 struct tpm_chip_priv {
+	enum tpm_version version;
+
 	uint duration_ms[TPM_DURATION_COUNT];
 	uint retry_time_ms;
-#if defined(CONFIG_TPM_V2)
+	u8 buf[TPM_DEV_BUFSIZE + sizeof(u8)];  /* Max buffer size + addr */
+
+	/* TPM v2 specific data */
 	uint pcr_count;
 	uint pcr_select_min;
-#endif
-	u8 buf[TPM_DEV_BUFSIZE + sizeof(u8)];  /* Max buffer size + addr */
 };
 
 /**
@@ -65,6 +78,16 @@ struct tpm_chip_priv {
  * to bytes which are sent and received.
  */
 struct tpm_ops {
+	/**
+	 * set_version() - Set the right TPM stack version to use
+	 *
+	 * Default is TPM_V1. TPM of newer versions can implement this
+	 * optional hook to set another value.
+	 *
+	 * @dev:	TPM device
+	 */
+	int (*set_version)(struct udevice *dev);
+
 	/**
 	 * open() - Request access to locality 0 for the caller
 	 *
@@ -208,10 +231,11 @@ int tpm_xfer(struct udevice *dev, const u8 *sendbuf, size_t send_size,
 int tpm_init(void);
 
 /**
- * Retrieve the array containing all the commands.
+ * Retrieve the array containing all the v1 (resp. v2) commands.
  *
  * @return a cmd_tbl_t array.
  */
-cmd_tbl_t *get_tpm_commands(unsigned int *size);
+cmd_tbl_t *get_tpm1_commands(unsigned int *size);
+cmd_tbl_t *get_tpm2_commands(unsigned int *size);
 
 #endif /* __TPM_COMMON_H */
diff --git a/lib/tpm-common.c b/lib/tpm-common.c
index 43b530865a..b228aad0aa 100644
--- a/lib/tpm-common.c
+++ b/lib/tpm-common.c
@@ -193,5 +193,9 @@ int tpm_init(void)
 	if (err)
 		return err;
 
+	err = tpm_set_version(dev);
+	if (err)
+		return err;
+
 	return tpm_open(dev);
 }
diff --git a/lib/tpm-utils.h b/lib/tpm-utils.h
index a9cb7dc7ee..10daffb423 100644
--- a/lib/tpm-utils.h
+++ b/lib/tpm-utils.h
@@ -18,6 +18,15 @@
 #define tpm_u16(x) __MSB(x), __LSB(x)
 #define tpm_u32(x) tpm_u16((x) >> 16), tpm_u16((x) & 0xFFFF)
 
+/**
+ * tpm_open() - Request the driver to set the TPM version it uses
+ *
+ * This must be done first. By default, TPM v1 stack is used.
+ *
+ * Returns 0 on success, a negative error otherwise.
+ */
+int tpm_set_version(struct udevice *dev);
+
 /**
  * tpm_open() - Request access to locality 0 for the caller
  *
-- 
2.14.1

[U-Boot] [PATCH 4/6] test/py: tpm2: switch from 'tpm' to 'tpm2' command

[Miquel Raynal]

While using the 'tpm' command should work on most cases, this test suite
only works with TPMv2 and since the work to make both versions build at
the same time, we might end up having both 'tpm' (TPMv1) and 'tpm2'
(TPMv2) commands available at the same time. Ensure this test suite
always use the right one.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 test/py/tests/test_tpm2.py | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py
index 01ffb3178d..3e1f42d83e 100644
--- a/test/py/tests/test_tpm2.py
+++ b/test/py/tests/test_tpm2.py
@@ -29,22 +29,22 @@ def force_init(u_boot_console, force=False):
     twice will spawn an error used to detect that the TPM was not reset and no
     initialization code should be run.
     """
-    output = u_boot_console.run_command('tpm init')
+    output = u_boot_console.run_command('tpm2 init')
     if force or not 'Error' in output:
         u_boot_console.run_command('echo --- start of init ---')
-        u_boot_console.run_command('tpm startup TPM2_SU_CLEAR')
-        u_boot_console.run_command('tpm self_test full')
-        u_boot_console.run_command('tpm clear TPM2_RH_LOCKOUT')
+        u_boot_console.run_command('tpm2 startup TPM2_SU_CLEAR')
+        u_boot_console.run_command('tpm2 self_test full')
+        u_boot_console.run_command('tpm2 clear TPM2_RH_LOCKOUT')
         output = u_boot_console.run_command('echo $?')
         if not output.endswith('0'):
-            u_boot_console.run_command('tpm clear TPM2_RH_PLATFORM')
+            u_boot_console.run_command('tpm2 clear TPM2_RH_PLATFORM')
         u_boot_console.run_command('echo --- end of init ---')
 
 @pytest.mark.buildconfigspec('cmd_tpm_v2')
 def test_tpm2_init(u_boot_console):
     """Init the software stack to use TPMv2 commands."""
 
-    u_boot_console.run_command('tpm init')
+    u_boot_console.run_command('tpm2 init')
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
 
@@ -55,7 +55,7 @@ def test_tpm2_startup(u_boot_console):
     Initiate the TPM internal state machine.
     """
 
-    u_boot_console.run_command('tpm startup TPM2_SU_CLEAR')
+    u_boot_console.run_command('tpm2 startup TPM2_SU_CLEAR')
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
 
@@ -66,7 +66,7 @@ def test_tpm2_self_test_full(u_boot_console):
     Ask the TPM to perform all self tests to also enable full capabilities.
     """
 
-    u_boot_console.run_command('tpm self_test full')
+    u_boot_console.run_command('tpm2 self_test full')
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
 
@@ -78,7 +78,7 @@ def test_tpm2_continue_self_test(u_boot_console):
     to enter a fully operational state.
     """
 
-    u_boot_console.run_command('tpm self_test continue')
+    u_boot_console.run_command('tpm2 self_test continue')
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
 
@@ -95,11 +95,11 @@ def test_tpm2_clear(u_boot_console):
     PLATFORM hierarchies are also available.
     """
 
-    u_boot_console.run_command('tpm clear TPM2_RH_LOCKOUT')
+    u_boot_console.run_command('tpm2 clear TPM2_RH_LOCKOUT')
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
 
-    u_boot_console.run_command('tpm clear TPM2_RH_PLATFORM')
+    u_boot_console.run_command('tpm2 clear TPM2_RH_PLATFORM')
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
 
@@ -115,13 +115,13 @@ def test_tpm2_change_auth(u_boot_console):
 
     force_init(u_boot_console)
 
-    u_boot_console.run_command('tpm change_auth TPM2_RH_LOCKOUT unicorn')
+    u_boot_console.run_command('tpm2 change_auth TPM2_RH_LOCKOUT unicorn')
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
 
-    u_boot_console.run_command('tpm clear TPM2_RH_LOCKOUT unicorn')
+    u_boot_console.run_command('tpm2 clear TPM2_RH_LOCKOUT unicorn')
     output = u_boot_console.run_command('echo $?')
-    u_boot_console.run_command('tpm clear TPM2_RH_PLATFORM')
+    u_boot_console.run_command('tpm2 clear TPM2_RH_PLATFORM')
     assert output.endswith('0')
 
 @pytest.mark.buildconfigspec('cmd_tpm_v2')
@@ -140,7 +140,7 @@ def test_tpm2_get_capability(u_boot_console):
     force_init(u_boot_console)
     ram = u_boot_utils.find_ram_base(u_boot_console)
 
-    read_cap = u_boot_console.run_command('tpm get_capability 0x6 0x20e 0x200 1') #0x%x 1' % ram)
+    read_cap = u_boot_console.run_command('tpm2 get_capability 0x6 0x20e 0x200 1') #0x%x 1' % ram)
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
     assert 'Property 0x0000020e: 0x00000000' in read_cap
@@ -163,12 +163,12 @@ def test_tpm2_dam_parameters(u_boot_console):
     ram = u_boot_utils.find_ram_base(u_boot_console)
 
     # Set the DAM parameters to known values
-    u_boot_console.run_command('tpm dam_parameters 3 10 0')
+    u_boot_console.run_command('tpm2 dam_parameters 3 10 0')
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
 
     # Check the values have been saved
-    read_cap = u_boot_console.run_command('tpm get_capability 0x6 0x20f 0x%x 3' % ram)
+    read_cap = u_boot_console.run_command('tpm2 get_capability 0x6 0x20f 0x%x 3' % ram)
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
     assert 'Property 0x0000020f: 0x00000003' in read_cap
@@ -185,7 +185,7 @@ def test_tpm2_pcr_read(u_boot_console):
     force_init(u_boot_console)
     ram = u_boot_utils.find_ram_base(u_boot_console) + 1024
 
-    read_pcr = u_boot_console.run_command('tpm pcr_read 0 0x%x' % ram)
+    read_pcr = u_boot_console.run_command('tpm2 pcr_read 0 0x%x' % ram)
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
 
@@ -212,11 +212,11 @@ def test_tpm2_pcr_extend(u_boot_console):
     force_init(u_boot_console)
     ram = u_boot_utils.find_ram_base(u_boot_console) + 1024
 
-    u_boot_console.run_command('tpm pcr_extend 0 0x%x' % ram)
+    u_boot_console.run_command('tpm2 pcr_extend 0 0x%x' % ram)
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
 
-    read_pcr = u_boot_console.run_command('tpm pcr_read 0 0x%x' % ram)
+    read_pcr = u_boot_console.run_command('tpm2 pcr_read 0 0x%x' % ram)
     output = u_boot_console.run_command('echo $?')
     assert output.endswith('0')
     assert 'f5 a5 fd 42 d1 6a 20 30 27 98 ef 6e d3 09 97 9b' in read_pcr
-- 
2.14.1

[U-Boot] [PATCH 5/6] tpm: make TPM_V2 be compiled by default

[Miquel Raynal]

TPM_V1 was already compiled by default. Now that both can be compiled
at the same time, compiled them both by default.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/tpm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
index 704ab81e2d..e20768fa5e 100644
--- a/drivers/tpm/Kconfig
+++ b/drivers/tpm/Kconfig
@@ -121,6 +121,7 @@ endif # TPM_V1
 config TPM_V2
 	bool "TPMv2.x support"
 	depends on TPM
+	default y
 	help
 	  Major TPM versions are not compatible at all, choose either
 	  one or the other. This option enables TPMv2.x drivers/commands.
-- 
2.14.1

[U-Boot] [PATCH 6/6] sandbox: compile both TPM stack versions and drivers

[Miquel Raynal]

Now that TPMv1 and TPMv2 can be compiled at the same time, let's compile
them both with Sandbox as well as both drivers (and, it is already
implied in Kconfig: both commands).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 configs/sandbox_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 2fc84a16c9..d5756d1173 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -176,6 +176,7 @@ CONFIG_TIMER=y
 CONFIG_TIMER_EARLY=y
 CONFIG_SANDBOX_TIMER=y
 CONFIG_TPM_TIS_SANDBOX=y
+CONFIG_TPM2_TIS_SANDBOX=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_EMUL=y
@@ -192,6 +193,8 @@ CONFIG_FS_CBFS=y
 CONFIG_FS_CRAMFS=y
 CONFIG_CMD_DHRYSTONE=y
 CONFIG_TPM=y
+CONFIG_TPM_V1=y
+CONFIG_TPM_V2=y
 CONFIG_LZ4=y
 CONFIG_ERRNO_STR=y
 CONFIG_OF_LIBFDT_OVERLAY=y
-- 
2.14.1

[U-Boot] [PATCH 1/6] tpm: fix typo in kernel doc

[Simon Glass]

On 14 July 2018 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> The udevice given to the open() function of course must be opened,
> not closed.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  include/tpm-common.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

[U-Boot] [PATCH 2/6] tpm: compile Sandbox driver by default

[Simon Glass]

On 14 July 2018 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> When Sandbox and the TPM stack are both selected, compile Sandbox TPM
> driver by default.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/tpm/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)

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

[U-Boot] [PATCH 3/6] tpm: allow TPM v1 and v2 to be compiled at the same time

[Simon Glass]

Hi Miquel,

On 14 July 2018 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> While there is probably no reason to do so in a real life situation, it
> will allow to compile test both stacks with the same sandbox defconfig.
>
> As we cannot define two 'tpm' commands at the same time, the command for
> TPM v1 is still called 'tpm' and the one for TPM v2 'tpm2'. While this
> is the exact command name that must be written into eg. test files, any
> user already using the TPM v2 stack can continue using just 'tpm' as
> command as long as TPM v1 support is not compiled (because U-Boot prompt
> will search for the closest command named after 'tpm'.b)
>
> The command set can also be changed at runtime (not supported yet, but
> ready to be), but as one can compile only either one stack or the other,
> there is still one spot in the code where conditionals are used: to
> retrieve the v1 or v2 command set.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  cmd/tpm-common.c               | 24 +++++++++++++++++++++++-
>  cmd/tpm-v1.c                   |  2 +-
>  cmd/tpm-v2.c                   |  4 ++--
>  drivers/tpm/Kconfig            |  7 ++-----
>  drivers/tpm/tpm-uclass.c       | 13 ++++++++++---
>  drivers/tpm/tpm2_tis_sandbox.c | 11 +++++++++++
>  drivers/tpm/tpm2_tis_spi.c     | 15 +++++++++++++++
>  include/tpm-common.h           | 36 ++++++++++++++++++++++++++++++------
>  lib/tpm-common.c               |  4 ++++
>  lib/tpm-utils.h                |  9 +++++++++
>  10 files changed, 107 insertions(+), 18 deletions(-)
>

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

Some comments below.

> diff --git a/cmd/tpm-common.c b/cmd/tpm-common.c
> index 6cf9fcc9ac..69cc02b599 100644
> --- a/cmd/tpm-common.c
> +++ b/cmd/tpm-common.c
> @@ -273,12 +273,34 @@ int do_tpm_init(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  int do_tpm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  {
>         cmd_tbl_t *tpm_commands, *cmd;
> +       struct tpm_chip_priv *priv;
> +       struct udevice *dev;
>         unsigned int size;
> +       int rc;

Can we use 'ret'? That is more common.

>
>         if (argc < 2)
>                 return CMD_RET_USAGE;
>
> -       tpm_commands = get_tpm_commands(&size);
> +       rc = get_tpm(&dev);
> +       if (rc)
> +               return rc;
> +
> +       priv = dev_get_uclass_priv(dev);
> +
> +       switch (priv->version) {
> +#if defined(CONFIG_TPM_V1)
> +       case TPM_V1:

if (IS_ENABLED(CONFIG_TPM_V1))

(I think #ifdef is worse)

> +               tpm_commands = get_tpm1_commands(&size);
> +               break;
> +#endif
> +#if defined(CONFIG_TPM_V2)
> +       case TPM_V2:
> +               tpm_commands = get_tpm2_commands(&size);
> +               break;
> +#endif
> +       default:
> +               return CMD_RET_USAGE;
> +       }
>
>         cmd = find_cmd_tbl(argv[1], tpm_commands, size);
>         if (!cmd)
> diff --git a/cmd/tpm-v1.c b/cmd/tpm-v1.c
> index 0874c4d7ba..69870002d4 100644
> --- a/cmd/tpm-v1.c
> +++ b/cmd/tpm-v1.c
> @@ -608,7 +608,7 @@ static cmd_tbl_t tpm1_commands[] = {
>  #endif /* CONFIG_TPM_LIST_RESOURCES */
>  };
>
> -cmd_tbl_t *get_tpm_commands(unsigned int *size)
> +cmd_tbl_t *get_tpm1_commands(unsigned int *size)
>  {
>         *size = ARRAY_SIZE(tpm1_commands);
>
> diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
> index 38add4f462..ffbf35a75c 100644
> --- a/cmd/tpm-v2.c
> +++ b/cmd/tpm-v2.c
> @@ -319,14 +319,14 @@ static cmd_tbl_t tpm2_commands[] = {
>                          do_tpm_pcr_setauthvalue, "", ""),
>  };
>
> -cmd_tbl_t *get_tpm_commands(unsigned int *size)
> +cmd_tbl_t *get_tpm2_commands(unsigned int *size)
>  {
>         *size = ARRAY_SIZE(tpm2_commands);
>
>         return tpm2_commands;
>  }
>
> -U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm, "Issue a TPMv2.x command",
> +U_BOOT_CMD(tpm2, CONFIG_SYS_MAXARGS, 1, do_tpm, "Issue a TPMv2.x command",
>  "<command> [<arguments>]\n"
>  "\n"
>  "info\n"
> diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
> index 5e3fb3267f..704ab81e2d 100644
> --- a/drivers/tpm/Kconfig
> +++ b/drivers/tpm/Kconfig
> @@ -4,9 +4,6 @@
>
>  menu "TPM support"
>
> -comment "Please select only one TPM revision"
> -       depends on TPM_V1 && TPM_V2
> -
>  config TPM_V1
>         bool "TPMv1.x support"
>         depends on TPM
> @@ -15,7 +12,7 @@ config TPM_V1
>           Major TPM versions are not compatible at all, choose either
>           one or the other. This option enables TPMv1.x drivers/commands.
>
> -if TPM_V1 && !TPM_V2
> +if TPM_V1
>
>  config TPM_TIS_SANDBOX
>         bool "Enable sandbox TPM driver"
> @@ -128,7 +125,7 @@ config TPM_V2
>           Major TPM versions are not compatible at all, choose either
>           one or the other. This option enables TPMv2.x drivers/commands.
>
> -if TPM_V2 && !TPM_V1
> +if TPM_V2
>
>  config TPM2_TIS_SANDBOX
>         bool "Enable sandbox TPMv2.x driver"
> diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c
> index 412697eedc..a80f3df78b 100644
> --- a/drivers/tpm/tpm-uclass.c
> +++ b/drivers/tpm/tpm-uclass.c
> @@ -7,13 +7,20 @@
>  #include <common.h>
>  #include <dm.h>
>  #include <linux/unaligned/be_byteshift.h>
> -#if defined(CONFIG_TPM_V1)
>  #include <tpm-v1.h>
> -#elif defined(CONFIG_TPM_V2)
>  #include <tpm-v2.h>
> -#endif
>  #include "tpm_internal.h"
>
> +int tpm_set_version(struct udevice *dev)
> +{
> +       struct tpm_ops *ops = tpm_get_ops(dev);
> +
> +       if (ops->set_version)
> +               return ops->set_version(dev);
> +
> +       return 0;
> +}
> +
>  int tpm_open(struct udevice *dev)
>  {
>         struct tpm_ops *ops = tpm_get_ops(dev);
> diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c
> index 3240cc5dba..0b8baad056 100644
> --- a/drivers/tpm/tpm2_tis_sandbox.c
> +++ b/drivers/tpm/tpm2_tis_sandbox.c
> @@ -573,6 +573,16 @@ static int sandbox_tpm2_get_desc(struct udevice *dev, char *buf, int size)
>         return snprintf(buf, size, "Sandbox TPM2.x");
>  }
>
> +static int sandbox_tpm2_set_version(struct udevice *dev)
> +{
> +       struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
> +
> +       /* Use the TPM v2 stack */
> +       priv->version = TPM_V2;

Don't you think this could be done in the probe() method?

> +
> +       return 0;
> +}
> +
>  static int sandbox_tpm2_open(struct udevice *dev)
>  {
>         struct sandbox_tpm2 *tpm = dev_get_priv(dev);
> @@ -604,6 +614,7 @@ static int sandbox_tpm2_close(struct udevice *dev)
>  }
>
>  static const struct tpm_ops sandbox_tpm2_ops = {
> +       .set_version    = sandbox_tpm2_set_version,
>         .open           = sandbox_tpm2_open,
>         .close          = sandbox_tpm2_close,
>         .get_desc       = sandbox_tpm2_get_desc,
> diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c
> index c5d17a679d..3577f3501c 100644
> --- a/drivers/tpm/tpm2_tis_spi.c
> +++ b/drivers/tpm/tpm2_tis_spi.c
> @@ -507,9 +507,23 @@ static int tpm_tis_spi_cleanup(struct udevice *dev)
>         return 0;
>  }
>
> +static int tpm_tis_spi_set_version(struct udevice *dev)
> +{
> +       struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
> +
> +       /* Use the TPM v2 stack */
> +       priv->version = TPM_V2;
> +
> +       return 0;
> +}
> +
>  static int tpm_tis_spi_open(struct udevice *dev)
>  {
>         struct tpm_chip *chip = dev_get_priv(dev);
> +       struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
> +
> +       /* Use the TPM v2 stack */
> +       priv->version = TPM_V2;
>
>         if (chip->is_open)
>                 return -EBUSY;
> @@ -646,6 +660,7 @@ static int tpm_tis_spi_remove(struct udevice *dev)
>  }
>
>  static const struct tpm_ops tpm_tis_spi_ops = {
> +       .set_version    = tpm_tis_spi_set_version,
>         .open           = tpm_tis_spi_open,
>         .close          = tpm_tis_spi_close,
>         .get_desc       = tpm_tis_get_desc,
> diff --git a/include/tpm-common.h b/include/tpm-common.h
> index 68bf8fd627..f39b58cd6c 100644
> --- a/include/tpm-common.h
> +++ b/include/tpm-common.h
> @@ -26,6 +26,16 @@ enum tpm_duration {
>  /* Max buffer size supported by our tpm */
>  #define TPM_DEV_BUFSIZE                1260
>
> +/**
> + * enum tpm_version - The version of the TPM stack to be used
> + * @TPM_V1:            Use TPM v1.x stack
> + * @TPM_V2:            Use TPM v2.x stack
> + */
> +enum tpm_version {
> +       TPM_V1 = 0,
> +       TPM_V2,
> +};
> +
>  /**
>   * struct tpm_chip_priv - Information about a TPM, stored by the uclass
>   *
> @@ -33,20 +43,23 @@ enum tpm_duration {
>   * communcation is attempted. If the device has an xfer() method, this is
>   * not needed. There is no need to set up @buf.
>   *
> + * @version:           TPM stack to be used
>   * @duration_ms:       Length of each duration type in milliseconds
>   * @retry_time_ms:     Time to wait before retrying receive
> + * @buf:               Buffer used during the exchanges with the chip
>   * @pcr_count:         Number of PCR per bank
>   * @pcr_select_min:    Minimum size in bytes of the pcrSelect array
> - * @buf:               Buffer used during the exchanges with the chip
>   */
>  struct tpm_chip_priv {
> +       enum tpm_version version;
> +
>         uint duration_ms[TPM_DURATION_COUNT];
>         uint retry_time_ms;
> -#if defined(CONFIG_TPM_V2)
> +       u8 buf[TPM_DEV_BUFSIZE + sizeof(u8)];  /* Max buffer size + addr */
> +
> +       /* TPM v2 specific data */
>         uint pcr_count;
>         uint pcr_select_min;
> -#endif
> -       u8 buf[TPM_DEV_BUFSIZE + sizeof(u8)];  /* Max buffer size + addr */
>  };
>
>  /**
> @@ -65,6 +78,16 @@ struct tpm_chip_priv {
>   * to bytes which are sent and received.
>   */
>  struct tpm_ops {
> +       /**
> +        * set_version() - Set the right TPM stack version to use
> +        *
> +        * Default is TPM_V1. TPM of newer versions can implement this
> +        * optional hook to set another value.
> +        *
> +        * @dev:        TPM device
> +        */
> +       int (*set_version)(struct udevice *dev);

I think this could just be a helper function provided by the uclass,
rather than a device method.

> +
>         /**
>          * open() - Request access to locality 0 for the caller
>          *
> @@ -208,10 +231,11 @@ int tpm_xfer(struct udevice *dev, const u8 *sendbuf, size_t send_size,
>  int tpm_init(void);
>
>  /**
> - * Retrieve the array containing all the commands.
> + * Retrieve the array containing all the v1 (resp. v2) commands.
>   *
>   * @return a cmd_tbl_t array.
>   */
> -cmd_tbl_t *get_tpm_commands(unsigned int *size);
> +cmd_tbl_t *get_tpm1_commands(unsigned int *size);
> +cmd_tbl_t *get_tpm2_commands(unsigned int *size);
>
>  #endif /* __TPM_COMMON_H */
> diff --git a/lib/tpm-common.c b/lib/tpm-common.c
> index 43b530865a..b228aad0aa 100644
> --- a/lib/tpm-common.c
> +++ b/lib/tpm-common.c
> @@ -193,5 +193,9 @@ int tpm_init(void)
>         if (err)
>                 return err;
>
> +       err = tpm_set_version(dev);
> +       if (err)
> +               return err;
> +
>         return tpm_open(dev);
>  }
> diff --git a/lib/tpm-utils.h b/lib/tpm-utils.h
> index a9cb7dc7ee..10daffb423 100644
> --- a/lib/tpm-utils.h
> +++ b/lib/tpm-utils.h
> @@ -18,6 +18,15 @@
>  #define tpm_u16(x) __MSB(x), __LSB(x)
>  #define tpm_u32(x) tpm_u16((x) >> 16), tpm_u16((x) & 0xFFFF)
>
> +/**
> + * tpm_open() - Request the driver to set the TPM version it uses
> + *
> + * This must be done first. By default, TPM v1 stack is used.
> + *
> + * Returns 0 on success, a negative error otherwise.
> + */
> +int tpm_set_version(struct udevice *dev);
> +
>  /**
>   * tpm_open() - Request access to locality 0 for the caller
>   *
> --
> 2.14.1
>

Regards,
Simon

[U-Boot] [PATCH 4/6] test/py: tpm2: switch from 'tpm' to 'tpm2' command

[Simon Glass]

On 14 July 2018 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> While using the 'tpm' command should work on most cases, this test suite
> only works with TPMv2 and since the work to make both versions build at
> the same time, we might end up having both 'tpm' (TPMv1) and 'tpm2'
> (TPMv2) commands available at the same time. Ensure this test suite
> always use the right one.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  test/py/tests/test_tpm2.py | 40 ++++++++++++++++++++--------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)

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

[U-Boot] [PATCH 5/6] tpm: make TPM_V2 be compiled by default

[Simon Glass]

On 14 July 2018 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> TPM_V1 was already compiled by default. Now that both can be compiled
> at the same time, compiled them both by default.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/tpm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

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

[U-Boot] [PATCH 6/6] sandbox: compile both TPM stack versions and drivers

[Simon Glass]

On 14 July 2018 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Now that TPMv1 and TPMv2 can be compiled at the same time, let's compile
> them both with Sandbox as well as both drivers (and, it is already
> implied in Kconfig: both commands).
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  configs/sandbox_defconfig | 3 +++
>  1 file changed, 3 insertions(+)

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

[U-Boot] [PATCH 3/6] tpm: allow TPM v1 and v2 to be compiled at the same time

[Miquel Raynal]

Hi Simon,

Simon Glass <sjg@chromium.org> wrote on Wed, 18 Jul 2018 19:31:41 -0600:

> Hi Miquel,
> 
> On 14 July 2018 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > While there is probably no reason to do so in a real life situation, it
> > will allow to compile test both stacks with the same sandbox defconfig.
> >
> > As we cannot define two 'tpm' commands at the same time, the command for
> > TPM v1 is still called 'tpm' and the one for TPM v2 'tpm2'. While this
> > is the exact command name that must be written into eg. test files, any
> > user already using the TPM v2 stack can continue using just 'tpm' as
> > command as long as TPM v1 support is not compiled (because U-Boot prompt
> > will search for the closest command named after 'tpm'.b)
> >
> > The command set can also be changed at runtime (not supported yet, but
> > ready to be), but as one can compile only either one stack or the other,
> > there is still one spot in the code where conditionals are used: to
> > retrieve the v1 or v2 command set.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  cmd/tpm-common.c               | 24 +++++++++++++++++++++++-
> >  cmd/tpm-v1.c                   |  2 +-
> >  cmd/tpm-v2.c                   |  4 ++--
> >  drivers/tpm/Kconfig            |  7 ++-----
> >  drivers/tpm/tpm-uclass.c       | 13 ++++++++++---
> >  drivers/tpm/tpm2_tis_sandbox.c | 11 +++++++++++
> >  drivers/tpm/tpm2_tis_spi.c     | 15 +++++++++++++++
> >  include/tpm-common.h           | 36 ++++++++++++++++++++++++++++++------
> >  lib/tpm-common.c               |  4 ++++
> >  lib/tpm-utils.h                |  9 +++++++++
> >  10 files changed, 107 insertions(+), 18 deletions(-)
> >  
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Some comments below.
> 
> > diff --git a/cmd/tpm-common.c b/cmd/tpm-common.c
> > index 6cf9fcc9ac..69cc02b599 100644
> > --- a/cmd/tpm-common.c
> > +++ b/cmd/tpm-common.c
> > @@ -273,12 +273,34 @@ int do_tpm_init(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >  int do_tpm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >  {
> >         cmd_tbl_t *tpm_commands, *cmd;
> > +       struct tpm_chip_priv *priv;
> > +       struct udevice *dev;
> >         unsigned int size;
> > +       int rc;  
> 
> Can we use 'ret'? That is more common.

Of course.

> 
> >
> >         if (argc < 2)
> >                 return CMD_RET_USAGE;
> >
> > -       tpm_commands = get_tpm_commands(&size);
> > +       rc = get_tpm(&dev);
> > +       if (rc)
> > +               return rc;
> > +
> > +       priv = dev_get_uclass_priv(dev);
> > +
> > +       switch (priv->version) {
> > +#if defined(CONFIG_TPM_V1)
> > +       case TPM_V1:  
> 
> if (IS_ENABLED(CONFIG_TPM_V1))
> 
> (I think #ifdef is worse)

Actually, in the present state, get_tpmx_commands symbol is only
available if the file if TPM_Vx is selected in Kconfig.

I created two dummy functions in tpm-common.h which return NULL for
each symbol if their respective TPM version is not compiled. This way it
removes any conditionals in the code.

> 
> > +               tpm_commands = get_tpm1_commands(&size);
> > +               break;
> > +#endif
> > +#if defined(CONFIG_TPM_V2)
> > +       case TPM_V2:
> > +               tpm_commands = get_tpm2_commands(&size);
> > +               break;
> > +#endif
> > +       default:
> > +               return CMD_RET_USAGE;
> > +       }

[...]

> >
> > +static int sandbox_tpm2_set_version(struct udevice *dev)
> > +{
> > +       struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
> > +
> > +       /* Use the TPM v2 stack */
> > +       priv->version = TPM_V2;  
> 
> Don't you think this could be done in the probe() method?

When I first wrote this I faced some issues. As I did not remembered
why it would fail I tried again and indeed, moving this
selection in the probe works.

> 
> > +
> > +       return 0;
> > +}
> > +

[...]

> > @@ -65,6 +78,16 @@ struct tpm_chip_priv {
> >   * to bytes which are sent and received.
> >   */
> >  struct tpm_ops {
> > +       /**
> > +        * set_version() - Set the right TPM stack version to use
> > +        *
> > +        * Default is TPM_V1. TPM of newer versions can implement this
> > +        * optional hook to set another value.
> > +        *
> > +        * @dev:        TPM device
> > +        */
> > +       int (*set_version)(struct udevice *dev);  
> 
> I think this could just be a helper function provided by the uclass,
> rather than a device method.

Actually there is no need for this hook or any helper anymore, I
removed them all as the version selection is done in the driver
->probe().

I'll let other people that would need to change the version at runtime
adding support for that if they want, it should not be too tricky
anyway.

Thanks,
Miquèl