[PATCH 0/6] Fix Atmel/Microchip TPMv1.2 issues

[PATCH 0/6] Fix Atmel/Microchip TPMv1.2 issues

[Mathew McBride]

While doing bringup/rebase for the Ten64 I did some troubleshooting
for the tpm (v1.2, NOT tpm2) command which did not appear to function,
despite the Linux driver and tools (tcsd) working on the same board.

Evidently the Atmel TPM driver hasn't kept up with various step
changes in the I2C and TPM stacks, and while TPMv1.2 is quite
dated to TPMv2 it would be nice to make some use of the hardware
that is there.
(Admittedly I would love to replace our hardware TPM with an fTPM
but that is a project for another day)

There are also subcommands in tpm-v1 which also have been
missed in changes to the TPMv1 API and are fixed in this patchset.

I have checked that this set isn't impacted by Ilias' TPM cleanup
series[1] which only touches TPMv2.

[1] - http://patchwork.ozlabs.org/project/uboot/cover/20211103150910.69732-1-ilias.apalodimas@linaro.org/

Mathew McBride (6):
  cmd: tpm-v1: fix compile error in TPMv1 list resources command
  cmd: tpm-v1: fix load_key_by_sha1 compile errors
  drivers: tpm: atmel_twi: drop non-DM_I2C compatibility
  drivers: tpm: atmel_twi: do not use an offset byte
  drivers: tpm: atmel_twi: implement get_desc operation
  drivers: tpm: atmel_twi: fix printf specifier compile warning

 cmd/tpm-v1.c                | 17 +++++++++++------
 drivers/tpm/tpm_atmel_twi.c | 22 +++++++---------------
 lib/tpm-v1.c                |  4 ++--
 3 files changed, 20 insertions(+), 23 deletions(-)

-- 
2.30.1

[PATCH 1/6] cmd: tpm-v1: fix compile error in TPMv1 list resources command

[Mathew McBride]

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

[PATCH 2/6] cmd: tpm-v1: fix load_key_by_sha1 compile errors

[Mathew McBride]

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

[PATCH 3/6] drivers: tpm: atmel_twi: drop non-DM_I2C compatibility

[Mathew McBride]

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

[PATCH 4/6] drivers: tpm: atmel_twi: do not use an offset byte

[Mathew McBride]

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

[PATCH 5/6] drivers: tpm: atmel_twi: implement get_desc operation

[Mathew McBride]

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);
 }
 
 /*
-- 
2.30.1

[PATCH 6/6] drivers: tpm: atmel_twi: fix printf specifier compile warning

[Mathew McBride]

%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

Re: [PATCH 1/6] cmd: tpm-v1: fix compile error in TPMv1 list resources command

[Ilias Apalodimas]

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>

Re: [PATCH 2/6] cmd: tpm-v1: fix load_key_by_sha1 compile errors

[Ilias Apalodimas]

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>

Re: [PATCH 3/6] drivers: tpm: atmel_twi: drop non-DM_I2C compatibility

[Ilias Apalodimas]

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>

Re: [PATCH 4/6] drivers: tpm: atmel_twi: do not use an offset byte

[Ilias Apalodimas]

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>

Re: [PATCH 5/6] drivers: tpm: atmel_twi: implement get_desc operation

[Ilias Apalodimas]

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
>

Re: [PATCH 6/6] drivers: tpm: atmel_twi: fix printf specifier compile warning

[Ilias Apalodimas]

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>

Re: [PATCH 0/6] Fix Atmel/Microchip TPMv1.2 issues

[Eugen.Hristev]

On 11/4/21 3:12 AM, Mathew McBride wrote:
> While doing bringup/rebase for the Ten64 I did some troubleshooting
> for the tpm (v1.2, NOT tpm2) command which did not appear to function,
> despite the Linux driver and tools (tcsd) working on the same board.
> 
> Evidently the Atmel TPM driver hasn't kept up with various step
> changes in the I2C and TPM stacks, and while TPMv1.2 is quite
> dated to TPMv2 it would be nice to make some use of the hardware
> that is there.
> (Admittedly I would love to replace our hardware TPM with an fTPM
> but that is a project for another day)
> 
> There are also subcommands in tpm-v1 which also have been
> missed in changes to the TPMv1 API and are fixed in this patchset.
> 
> I have checked that this set isn't impacted by Ilias' TPM cleanup
> series[1] which only touches TPMv2.
> 
> [1] - http://patchwork.ozlabs.org/project/uboot/cover/20211103150910.69732-1-ilias.apalodimas@linaro.org/
> 
> Mathew McBride (6):
>    cmd: tpm-v1: fix compile error in TPMv1 list resources command
>    cmd: tpm-v1: fix load_key_by_sha1 compile errors
>    drivers: tpm: atmel_twi: drop non-DM_I2C compatibility
>    drivers: tpm: atmel_twi: do not use an offset byte
>    drivers: tpm: atmel_twi: implement get_desc operation
>    drivers: tpm: atmel_twi: fix printf specifier compile warning
> 
>   cmd/tpm-v1.c                | 17 +++++++++++------
>   drivers/tpm/tpm_atmel_twi.c | 22 +++++++---------------
>   lib/tpm-v1.c                |  4 ++--
>   3 files changed, 20 insertions(+), 23 deletions(-)
> 
> --
> 2.30.1
> 

Hello Mathew, Ilias,

Even if this series touches a Microchip driver/part that is not part of 
the at91 architecture, the patches are in my queue.
I can take them through at91 tree if this is the way to go.
Is there another custodian tree that is dedicated for such kind of 
drivers? or more specific ?

Simon, your opinion on this ?

P.S. some of the patches look to be fixes most likely, so I guess it 
would be more likely to have them as fixes for 2022.01 release ?

Thanks,
Eugen

Re: [PATCH 0/6] Fix Atmel/Microchip TPMv1.2 issues

[Ilias Apalodimas]

On Tue, 9 Nov 2021 at 10:42, <Eugen.Hristev@microchip.com> wrote:
>
> On 11/4/21 3:12 AM, Mathew McBride wrote:
> > While doing bringup/rebase for the Ten64 I did some troubleshooting
> > for the tpm (v1.2, NOT tpm2) command which did not appear to function,
> > despite the Linux driver and tools (tcsd) working on the same board.
> >
> > Evidently the Atmel TPM driver hasn't kept up with various step
> > changes in the I2C and TPM stacks, and while TPMv1.2 is quite
> > dated to TPMv2 it would be nice to make some use of the hardware
> > that is there.
> > (Admittedly I would love to replace our hardware TPM with an fTPM
> > but that is a project for another day)
> >
> > There are also subcommands in tpm-v1 which also have been
> > missed in changes to the TPMv1 API and are fixed in this patchset.
> >
> > I have checked that this set isn't impacted by Ilias' TPM cleanup
> > series[1] which only touches TPMv2.
> >
> > [1] - http://patchwork.ozlabs.org/project/uboot/cover/20211103150910.69732-1-ilias.apalodimas@linaro.org/
> >
> > Mathew McBride (6):
> >    cmd: tpm-v1: fix compile error in TPMv1 list resources command
> >    cmd: tpm-v1: fix load_key_by_sha1 compile errors
> >    drivers: tpm: atmel_twi: drop non-DM_I2C compatibility
> >    drivers: tpm: atmel_twi: do not use an offset byte
> >    drivers: tpm: atmel_twi: implement get_desc operation
> >    drivers: tpm: atmel_twi: fix printf specifier compile warning
> >
> >   cmd/tpm-v1.c                | 17 +++++++++++------
> >   drivers/tpm/tpm_atmel_twi.c | 22 +++++++---------------
> >   lib/tpm-v1.c                |  4 ++--
> >   3 files changed, 20 insertions(+), 23 deletions(-)
> >
> > --
> > 2.30.1
> >
>
> Hello Mathew, Ilias,
>
> Even if this series touches a Microchip driver/part that is not part of
> the at91 architecture, the patches are in my queue.
> I can take them through at91 tree if this is the way to go.

I am fine with that

> Is there another custodian tree that is dedicated for such kind of
> drivers? or more specific ?

As far as I know there isn't.

>
> Simon, your opinion on this ?
>
> P.S. some of the patches look to be fixes most likely, so I guess it
> would be more likely to have them as fixes for 2022.01 release ?

Yes all of those look good.  I had a minor comment on one of those,
but we can always add more info on the TPM later.

Thanks
/Ilias
>
> Thanks,
> Eugen

Re: [PATCH 5/6] drivers: tpm: atmel_twi: implement get_desc operation

[Mathew McBride]

On Thu, Nov 4, 2021, at 6:39 PM, Ilias Apalodimas wrote:
> Hi Matthew,
> 
> On Thu, 4 Nov 2021 at 03:14, Mathew McBride <matt@traverse.com.au> wrote:
> 
> >  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?
Perhaps the DT compatible would be useful?

=> tpm device
device 0: atmel,at97sc3204t TPM 1.2 (tpm@29)

tpm_version under Linux provides this, but it requires talking to the TPM over TIS:
$ tpm_version
TPM 1.2 Version Info:
  Chip Version:        1.2.66.16
  Spec Level:          2
  Errata Revision:     3
  TPM Vendor ID:       ATML
  TPM Version:         01010000
  Manufacturer Info:   41544d4c

> 
> Regards
> /Ilias
> >  }
> >
> >  /*
> > --
> > 2.30.1
> >
> 

Re: [PATCH 5/6] drivers: tpm: atmel_twi: implement get_desc operation

[Ilias Apalodimas]

Hi Matthew,

On Wed, 10 Nov 2021 at 04:43, Mathew McBride <matt@traverse.com.au> wrote:
>
>
>
> On Thu, Nov 4, 2021, at 6:39 PM, Ilias Apalodimas wrote:
>
> Hi Matthew,
>
> On Thu, 4 Nov 2021 at 03:14, Mathew McBride <matt@traverse.com.au> wrote:
>
> >  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?
>
> Perhaps the DT compatible would be useful?
>
> => tpm device
> device 0: atmel,at97sc3204t TPM 1.2 (tpm@29)

No strong preference here, up to you.

>
>
> tpm_version under Linux provides this, but it requires talking to the TPM over TIS:
> $ tpm_version
> TPM 1.2 Version Info:
>   Chip Version:        1.2.66.16
>   Spec Level:          2
>   Errata Revision:     3
>   TPM Vendor ID:       ATML
>   TPM Version:         01010000
>   Manufacturer Info:   41544d4c
>

Ok fair enough.  I am fine keeping it as is.

Thanks
/Ilias

Re: [PATCH 0/6] Fix Atmel/Microchip TPMv1.2 issues

[Ilias Apalodimas]

Eugen,

Will this go through your tree?  I recently picked up maintainership
of the TPM drivers, so I could send them through that.  Up to you.

Cheers
/Ilias

On Tue, 9 Nov 2021 at 10:57, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Tue, 9 Nov 2021 at 10:42, <Eugen.Hristev@microchip.com> wrote:
> >
> > On 11/4/21 3:12 AM, Mathew McBride wrote:
> > > While doing bringup/rebase for the Ten64 I did some troubleshooting
> > > for the tpm (v1.2, NOT tpm2) command which did not appear to function,
> > > despite the Linux driver and tools (tcsd) working on the same board.
> > >
> > > Evidently the Atmel TPM driver hasn't kept up with various step
> > > changes in the I2C and TPM stacks, and while TPMv1.2 is quite
> > > dated to TPMv2 it would be nice to make some use of the hardware
> > > that is there.
> > > (Admittedly I would love to replace our hardware TPM with an fTPM
> > > but that is a project for another day)
> > >
> > > There are also subcommands in tpm-v1 which also have been
> > > missed in changes to the TPMv1 API and are fixed in this patchset.
> > >
> > > I have checked that this set isn't impacted by Ilias' TPM cleanup
> > > series[1] which only touches TPMv2.
> > >
> > > [1] - http://patchwork.ozlabs.org/project/uboot/cover/20211103150910.69732-1-ilias.apalodimas@linaro.org/
> > >
> > > Mathew McBride (6):
> > >    cmd: tpm-v1: fix compile error in TPMv1 list resources command
> > >    cmd: tpm-v1: fix load_key_by_sha1 compile errors
> > >    drivers: tpm: atmel_twi: drop non-DM_I2C compatibility
> > >    drivers: tpm: atmel_twi: do not use an offset byte
> > >    drivers: tpm: atmel_twi: implement get_desc operation
> > >    drivers: tpm: atmel_twi: fix printf specifier compile warning
> > >
> > >   cmd/tpm-v1.c                | 17 +++++++++++------
> > >   drivers/tpm/tpm_atmel_twi.c | 22 +++++++---------------
> > >   lib/tpm-v1.c                |  4 ++--
> > >   3 files changed, 20 insertions(+), 23 deletions(-)
> > >
> > > --
> > > 2.30.1
> > >
> >
> > Hello Mathew, Ilias,
> >
> > Even if this series touches a Microchip driver/part that is not part of
> > the at91 architecture, the patches are in my queue.
> > I can take them through at91 tree if this is the way to go.
>
> I am fine with that
>
> > Is there another custodian tree that is dedicated for such kind of
> > drivers? or more specific ?
>
> As far as I know there isn't.
>
> >
> > Simon, your opinion on this ?
> >
> > P.S. some of the patches look to be fixes most likely, so I guess it
> > would be more likely to have them as fixes for 2022.01 release ?
>
> Yes all of those look good.  I had a minor comment on one of those,
> but we can always add more info on the TPM later.
>
> Thanks
> /Ilias
> >
> > Thanks,
> > Eugen

Re: [PATCH 0/6] Fix Atmel/Microchip TPMv1.2 issues

[Eugen.Hristev]

On 11/17/21 12:35 PM, Ilias Apalodimas wrote:
> Eugen,
> 
> Will this go through your tree?  I recently picked up maintainership
> of the TPM drivers, so I could send them through that.  Up to you.
> 

Hi ,

The patch series by Mathew is now assigned to Tom Rini. You picked 
maintainership, that's fine, but you have a dedicated custodian tree ?

Tom, do you wish to handle these patches or you wish to reassign them to 
me and take them through at91 tree ?

Thanks,
Eugen

> Cheers
> /Ilias
> 
> On Tue, 9 Nov 2021 at 10:57, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
>>
>> On Tue, 9 Nov 2021 at 10:42, <Eugen.Hristev@microchip.com> wrote:
>>>
>>> On 11/4/21 3:12 AM, Mathew McBride wrote:
>>>> While doing bringup/rebase for the Ten64 I did some troubleshooting
>>>> for the tpm (v1.2, NOT tpm2) command which did not appear to function,
>>>> despite the Linux driver and tools (tcsd) working on the same board.
>>>>
>>>> Evidently the Atmel TPM driver hasn't kept up with various step
>>>> changes in the I2C and TPM stacks, and while TPMv1.2 is quite
>>>> dated to TPMv2 it would be nice to make some use of the hardware
>>>> that is there.
>>>> (Admittedly I would love to replace our hardware TPM with an fTPM
>>>> but that is a project for another day)
>>>>
>>>> There are also subcommands in tpm-v1 which also have been
>>>> missed in changes to the TPMv1 API and are fixed in this patchset.
>>>>
>>>> I have checked that this set isn't impacted by Ilias' TPM cleanup
>>>> series[1] which only touches TPMv2.
>>>>
>>>> [1] - http://patchwork.ozlabs.org/project/uboot/cover/20211103150910.69732-1-ilias.apalodimas@linaro.org/
>>>>
>>>> Mathew McBride (6):
>>>>     cmd: tpm-v1: fix compile error in TPMv1 list resources command
>>>>     cmd: tpm-v1: fix load_key_by_sha1 compile errors
>>>>     drivers: tpm: atmel_twi: drop non-DM_I2C compatibility
>>>>     drivers: tpm: atmel_twi: do not use an offset byte
>>>>     drivers: tpm: atmel_twi: implement get_desc operation
>>>>     drivers: tpm: atmel_twi: fix printf specifier compile warning
>>>>
>>>>    cmd/tpm-v1.c                | 17 +++++++++++------
>>>>    drivers/tpm/tpm_atmel_twi.c | 22 +++++++---------------
>>>>    lib/tpm-v1.c                |  4 ++--
>>>>    3 files changed, 20 insertions(+), 23 deletions(-)
>>>>
>>>> --
>>>> 2.30.1
>>>>
>>>
>>> Hello Mathew, Ilias,
>>>
>>> Even if this series touches a Microchip driver/part that is not part of
>>> the at91 architecture, the patches are in my queue.
>>> I can take them through at91 tree if this is the way to go.
>>
>> I am fine with that
>>
>>> Is there another custodian tree that is dedicated for such kind of
>>> drivers? or more specific ?
>>
>> As far as I know there isn't.
>>
>>>
>>> Simon, your opinion on this ?
>>>
>>> P.S. some of the patches look to be fixes most likely, so I guess it
>>> would be more likely to have them as fixes for 2022.01 release ?
>>
>> Yes all of those look good.  I had a minor comment on one of those,
>> but we can always add more info on the TPM later.
>>
>> Thanks
>> /Ilias
>>>
>>> Thanks,
>>> Eugen

Re: [PATCH 0/6] Fix Atmel/Microchip TPMv1.2 issues

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On Wed, Nov 17, 2021 at 11:09:05AM +0000, Eugen.Hristev@microchip.com wrote:
> On 11/17/21 12:35 PM, Ilias Apalodimas wrote:
> > Eugen,
> > 
> > Will this go through your tree?  I recently picked up maintainership
> > of the TPM drivers, so I could send them through that.  Up to you.
> > 
> 
> Hi ,
> 
> The patch series by Mathew is now assigned to Tom Rini. You picked 
> maintainership, that's fine, but you have a dedicated custodian tree ?
> 
> Tom, do you wish to handle these patches or you wish to reassign them to 
> me and take them through at91 tree ?

So, Ilias is getting set up with a tpm custodian tree now.  The question
I had was if you wanted these via the atmel tree, or would be OK with
them coming via the TPM tree.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/6] Fix Atmel/Microchip TPMv1.2 issues

[Eugen.Hristev]

 > So, Ilias is getting set up with a tpm custodian tree now.  The question
 >I had was if you wanted these via the atmel tree, or would be OK with
 >them coming via the TPM tree.


It's fine for me to take them through TPM tree. They are not in fact 
related to at91 architecture, it's another Atmel division that was 
creating these products.
I was willing to take them because there wasn't a better place for them, 
but if it's a TPM tree now, please go ahead with it.

Eugen


(P.S. when replying to your e-mails , thunderbird somehow deletes 
everything because it sees your e-mail as an attachment)