Hi Mathew,

On Thu, 4 Nov 2021 at 03:14, Mathew McBride matt@traverse.com.au wrote:

This command is not compiled by default and was not updated to pass the udevice to tpm_get_capability.

Signed-off-by: Mathew McBride matt@traverse.com.au

cmd/tpm-v1.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/cmd/tpm-v1.c b/cmd/tpm-v1.c index 3a7e35d525..55f2aeff46 100644 --- a/cmd/tpm-v1.c +++ b/cmd/tpm-v1.c @@ -582,6 +582,7 @@ static int do_tpm_flush(struct cmd_tbl *cmdtp, int flag, int argc, static int do_tpm_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) {

•   struct udevice *dev;
    int type = 0;
    u16 res_count;
    u8 buf[288];
  

@@ -589,6 +590,10 @@ static int do_tpm_list(struct cmd_tbl *cmdtp, int flag, int argc, int err; uint i;

•   err = get_tpm(&dev);
  

•   if (err)
  

•           return err;
  

•   if (argc != 2)
            return CMD_RET_USAGE;
  
  

@@ -619,7 +624,7 @@ static int do_tpm_list(struct cmd_tbl *cmdtp, int flag, int argc, }

    /* fetch list of already loaded resources in the TPM */

•   err = tpm_get_capability(TPM_CAP_HANDLE, type, buf,
  

•   err = tpm_get_capability(dev, TPM_CAP_HANDLE, type, buf,
                             sizeof(buf));
    if (err) {
            printf("tpm_get_capability returned error %d.\n", err);
  

-- 2.30.1

Thanks for fixing this, indeed I missed the tpm1.2 changes :(

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

On Thu, 4 Nov 2021 at 03:14, Mathew McBride matt@traverse.com.au wrote:

This command is not compiled by default and has not been updated alongside changes to the tpmv1 API, such as passing the TPM udevice to the relevant functions.

Signed-off-by: Mathew McBride matt@traverse.com.au

cmd/tpm-v1.c | 10 +++++----- lib/tpm-v1.c | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/cmd/tpm-v1.c b/cmd/tpm-v1.c index 55f2aeff46..bf238a9f2e 100644 --- a/cmd/tpm-v1.c +++ b/cmd/tpm-v1.c @@ -406,9 +406,9 @@ static int do_tpm_load_key_by_sha1(struct cmd_tbl *cmdtp, int flag, int argc, void *key; struct udevice *dev;

•   rc = get_tpm(&dev);
  

•   if (rc)
  

•           return rc;
  

•   err = get_tpm(&dev);
  

•   if (err)
  

•           return err;
  
    if (argc < 5)
            return CMD_RET_USAGE;
  

@@ -420,7 +420,7 @@ static int do_tpm_load_key_by_sha1(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; parse_byte_string(argv[4], usage_auth, NULL);

•   err = tpm_find_key_sha1(usage_auth, parent_hash, &parent_handle);
  

•   err = tpm1_find_key_sha1(dev, usage_auth, parent_hash, &parent_handle);
    if (err) {
            printf("Could not find matching parent key (err = %d)\n", err);
            return CMD_RET_FAILURE;
  

@@ -428,7 +428,7 @@ static int do_tpm_load_key_by_sha1(struct cmd_tbl *cmdtp, int flag, int argc,

    printf("Found parent key %08x\n", parent_handle);

•   err = tpm_load_key2_oiap(parent_handle, key, key_len, usage_auth,
  

•   err = tpm1_load_key2_oiap(dev, parent_handle, key, key_len, usage_auth,
                             &key_handle);
    if (!err) {
            printf("Key handle is 0x%x\n", key_handle);
  

diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c index 8dc144080c..22a769c587 100644 --- a/lib/tpm-v1.c +++ b/lib/tpm-v1.c @@ -840,7 +840,7 @@ u32 tpm1_find_key_sha1(struct udevice *dev, const u8 auth[20], unsigned int i;

    /* fetch list of already loaded keys in the TPM */

•   err = tpm_get_capability(dev, TPM_CAP_HANDLE, TPM_RT_KEY, buf,
  

•   err = tpm1_get_capability(dev, TPM_CAP_HANDLE, TPM_RT_KEY, buf,
                             sizeof(buf));
    if (err)
            return -1;
  

@@ -852,7 +852,7 @@ u32 tpm1_find_key_sha1(struct udevice *dev, const u8 auth[20], /* now search a(/ the) key which we can access with the given auth */ for (i = 0; i < key_count; ++i) { buf_len = sizeof(buf);

•           err = tpm_get_pub_key_oiap(key_handles[i], auth, buf, &buf_len);
  

•           err = tpm1_get_pub_key_oiap(dev, key_handles[i], auth, buf, &buf_len);
            if (err && err != TPM_AUTHFAIL)
                    return -1;
            if (err)
  

-- 2.30.1

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

On Thu, 4 Nov 2021 at 03:14, Mathew McBride matt@traverse.com.au wrote:

There are no users of this driver without DM_I2C

Signed-off-by: Mathew McBride matt@traverse.com.au

drivers/tpm/tpm_atmel_twi.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/tpm/tpm_atmel_twi.c b/drivers/tpm/tpm_atmel_twi.c index 2dcc2af67f..4ff4cf4cd4 100644 --- a/drivers/tpm/tpm_atmel_twi.c +++ b/drivers/tpm/tpm_atmel_twi.c @@ -81,22 +81,15 @@ static int tpm_atmel_twi_xfer(struct udevice *dev, print_buffer(0, (void *)sendbuf, 1, send_size, 0); #endif

-#if !CONFIG_IS_ENABLED(DM_I2C)

•   res = i2c_write(0x29, 0, 0, (uchar *)sendbuf, send_size);
  

-#else res = dm_i2c_write(dev, 0, sendbuf, send_size); -#endif if (res) { printf("i2c_write returned %d\n", res); return -1; }

    start = get_timer(0);

-#if !CONFIG_IS_ENABLED(DM_I2C)

•   while ((res = i2c_read(0x29, 0, 0, recvbuf, 10)))
  

-#else

•   while ((res = dm_i2c_read(dev, 0, recvbuf, 10)))
  

-#endif { /* TODO Use TIS_TIMEOUT from tpm_tis_infineon.h */ if (get_timer(start) > ATMEL_TPM_TIMEOUT_MS) { @@ -116,12 +109,7 @@ static int tpm_atmel_twi_xfer(struct udevice *dev, return -1; } else { *recv_len = hdr_recv_len; -#if !CONFIG_IS_ENABLED(DM_I2C)

•                   res = i2c_read(0x29, 0, 0, recvbuf, *recv_len);
  

-#else res = dm_i2c_read(dev, 0, recvbuf, *recv_len); -#endif

•           }
    }
    if (res) {
  

-- 2.30.1

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

On Thu, 4 Nov 2021 at 03:14, Mathew McBride matt@traverse.com.au wrote:

This driver was broken due to an empty offset byte being prepended at the start of every transmission.

The hardware does not mimic an EEPROM device with registers so an offset byte is not required.

Signed-off-by: Mathew McBride matt@traverse.com.au

drivers/tpm/tpm_atmel_twi.c | 1 + 1 file changed, 1 insertion(+)

diff --git a/drivers/tpm/tpm_atmel_twi.c b/drivers/tpm/tpm_atmel_twi.c index 4ff4cf4cd4..71b101406d 100644 --- a/drivers/tpm/tpm_atmel_twi.c +++ b/drivers/tpm/tpm_atmel_twi.c @@ -131,6 +131,7 @@ static int tpm_atmel_twi_xfer(struct udevice *dev,

static int tpm_atmel_twi_probe(struct udevice *dev) {

•   i2c_set_chip_offset_len(dev, 0);
    return 0;
  

}

-- 2.30.1

Acked-by: Ilias Apalodimas ilias.apalodimas@linaro.org

Hi Matthew,

On Thu, 4 Nov 2021 at 03:14, Mathew McBride matt@traverse.com.au wrote:

Without get_desc, the tpm command will not provide a description of the device in 'tpm device' or 'tpm info'.

Due to the characteristics of the Atmel TPM it isn't possible to determine certain attributes (e.g open/close status) without using the TPM stack (compare Infineon and ST TPM drivers), so just print out the udevice name as a unique identifier.

Signed-off-by: Mathew McBride matt@traverse.com.au

drivers/tpm/tpm_atmel_twi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tpm/tpm_atmel_twi.c b/drivers/tpm/tpm_atmel_twi.c index 71b101406d..9ca33e4334 100644 --- a/drivers/tpm/tpm_atmel_twi.c +++ b/drivers/tpm/tpm_atmel_twi.c @@ -52,7 +52,10 @@ static int tpm_atmel_twi_close(struct udevice *dev) */ static int tpm_atmel_twi_get_desc(struct udevice *dev, char *buf, int size) {

•   return 0;
  

•   if (size < 50)
  

•           return -ENOSPC;
  

•   return snprintf(buf, size, "Atmel 1.2 TPM (%s)", dev->name);
  

Is there anything more helpful we can print on that? Device ID, manufacturer ID maybe?

Regards /Ilias

}

/*

2.30.1

On Thu, 4 Nov 2021 at 03:14, Mathew McBride matt@traverse.com.au wrote:

%d was being used as the specifier for size_t, leading to a compiler warning

Signed-off-by: Mathew McBride matt@traverse.com.au

drivers/tpm/tpm_atmel_twi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tpm/tpm_atmel_twi.c b/drivers/tpm/tpm_atmel_twi.c index 9ca33e4334..e84f9183d1 100644 --- a/drivers/tpm/tpm_atmel_twi.c +++ b/drivers/tpm/tpm_atmel_twi.c @@ -116,7 +116,7 @@ static int tpm_atmel_twi_xfer(struct udevice *dev, } } if (res) {

•           printf("i2c_read returned %d (rlen=%d)\n", res, *recv_len);
  

•           printf("i2c_read returned %d (rlen=%zu)\n", res, *recv_len);
  

#ifdef DEBUG print_buffer(0, recvbuf, 1, *recv_len, 0);

#endif

2.30.1

At some point we might want to change all these prints to log debugging /error messages. However the current patch is a straight up fix so

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org