Re: [U-Boot] [PATCH 25/25] tegra: nyan: Enable TPM command and driver

2015-08-11 Thread christophe.ricard
Hi Simon, Acked-by: Christophe Ricard Best Regards Christophe On 11/08/2015 16:48, Simon Glass wrote: The TPM is listed in the device tree. Enable the driver and 'tpm' command so that it can be used. Signed-off-by: Simon Glass --- configs/nyan-big_defconfig | 4 1 file changed, 4 i

Re: [U-Boot] [PATCH 24/25] tpm: Add a 'tpm info' command

2015-08-11 Thread christophe.ricard
Hi Simon, Acked-by: Christophe Ricard Best Regards Christophe On 11/08/2015 16:48, Simon Glass wrote: Add a command to display basic information about a TPM such as the model and open/close state. This can be useful for debugging. Signed-off-by: Simon Glass --- common/cmd_tpm.c | 26

Re: [U-Boot] [PATCH 22/25] dm: tpm: Convert I2C driver to driver model

2015-08-11 Thread christophe.ricard
Hi Simon, I would be ok if we can get to a common agreement on my previous comments. Best Regards Christophe On 11/08/2015 16:48, Simon Glass wrote: Convert the tpm_tis_i2c driver to use driver model and update boards which use it. Signed-off-by: Simon Glass --- configs/peach-pi_defconfig

Re: [U-Boot] [PATCH 01/25] tpm: Remove old pre-driver-model I2C code

2015-08-11 Thread christophe.ricard
Hi Simon, In here you are removing usage of i2c_read/i2c_write for dm_i2c_read/dm_i2c_write. This require boards to support DM_I2C which is not the case with example beagleboard or boards using omap. Will you drop all drivers not supporting DM in the future ? Other than that i have proposed

Re: [U-Boot] [PATCH 23/25] dm: tpm: Convert LPC driver to driver model

2015-08-11 Thread christophe.ricard
Hi Simon, Ok for now, except with the xfer handler. Best Regards Christophe On 11/08/2015 16:48, Simon Glass wrote: Convert the tpm_tis_lpc driver to use driver model and update boards which use it. Signed-off-by: Simon Glass --- configs/chromebook_link_defconfig | 2 + configs/chrom

Re: [U-Boot] [PATCH 05/25] tpm: Convert drivers to use SPDX

2015-08-11 Thread christophe.ricard
Hi Simon, Just to mention i have seen this one as well. Acked-by: Christophe Ricard Best Regards Christophe On 11/08/2015 16:47, Simon Glass wrote: Add an SPDX header to two drivers that don't have it. Signed-off-by: Simon Glass --- drivers/tpm/tpm_atmel_twi.c | 15 +++ driv

Re: [U-Boot] [PATCH 06/25] tpm: Move the I2C TPM code into one file

2015-08-11 Thread christophe.ricard
Hi Simon, I would basically disagree with this one. The code from tpm.c you are merging into tpm_tis_i2c may not only be used by tpm_tis_i2c as it is using data from TPM standards (e.g: Trusted computing group) that should be used by other TPM drivers. You can find in chapter 17 how the table t

Re: [U-Boot] [PATCH 20/25] tpm: Check that parse_byte_string() has data to parse

2015-08-11 Thread christophe.ricard
Hi Simon, Acked-by: Christophe Ricard Best Regards Christophe On 11/08/2015 16:48, Simon Glass wrote: Rather then crashing when there is no data, print an error. Signed-off-by: Simon Glass --- common/cmd_tpm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/cmd_tpm.c b/comm

Re: [U-Boot] [PATCH 19/25] dm: tpm: sandbox: Convert TPM driver to driver model

2015-08-11 Thread christophe.ricard
Acked-by: Christophe Ricard On 11/08/2015 16:48, Simon Glass wrote: Convert the sandbox TPM driver to use driver model. Add it to the device tree so that it can be found on start-up. Signed-off-by: Simon Glass --- arch/sandbox/dts/sandbox.dts | 4 configs/sandbox_defconfig | 1

Re: [U-Boot] [PATCH 18/25] tpm: Report tpm errors on the command line

2015-08-11 Thread christophe.ricard
Hi Simon, Good one ! Acked-by: Christophe Ricard Best Regards Christophe On 11/08/2015 16:48, Simon Glass wrote: When a 'tpm' command fails, we set the return code but give no indication of failure. This can be confusing. Add an error message when any tpm command fails. Signed-off-by: Simon

Re: [U-Boot] [PATCH 16/25] dm: tpm: Convert the TPM command and library to driver model

2015-08-11 Thread christophe.ricard
Hi Simon On 11/08/2015 16:48, Simon Glass wrote: Add driver model support to the TPM command and the TPM library. Both support only a single TPM at present. Signed-off-by: Simon Glass --- common/cmd_tpm.c | 26 ++ include/tpm.h| 2 +- lib/tpm.c| 29 +++

Re: [U-Boot] [PATCH 15/25] dm: tpm: Add a uclass for Trusted Platform Modules

2015-08-11 Thread christophe.ricard
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 mod

Re: [U-Boot] [PATCH 14/25] tpm: tpm_tis_i2c: Tidy up delays

2015-08-11 Thread christophe.ricard
Hi Simon, Acked-by: Christophe Ricard Best Regards Christophe On 11/08/2015 16:48, Simon Glass wrote: Use a _US suffix for microseconds and a _MS suffic for milliseconds. Move all timeouts and delays into one place. Use mdelay() instead of udelay() where appropriate. Signed-off-by: Simon Gla

Re: [U-Boot] [PATCH 13/25] tpm: tpm_tis_i2c: Use a consistent tpm_tis_i2c_ prefix

2015-08-11 Thread christophe.ricard
Hi Simon, Acked-by: Christophe Ricard Best Regards Christophe On 11/08/2015 16:48, Simon Glass wrote: Use the same prefix on each function for consistency. Signed-off-by: Simon Glass --- drivers/tpm/tpm_tis_i2c.c | 113 -- 1 file changed, 58 i

Re: [U-Boot] [PATCH 02/25] tpm: Drop two unused options

2015-08-11 Thread christophe.ricard
Hi Simon, I am ok with this one. I proposed about the same changed in here: http://lists.denx.de/pipermail/u-boot/2015-August/222598.html Acked-by: Christophe Ricard Best Regards Christophe On 11/08/2015 16:47, Simon Glass wrote: The address of the I2C TPM is now defined in the device tree so

Re: [U-Boot] [PATCH 04/25] tpm: Convert board config TPM options to Kconfig

2015-08-11 Thread christophe.ricard
Hi Simon, Acked-by: Christophe Ricard Best Regards Christophe On 11/08/2015 16:47, Simon Glass wrote: Convert all TPM options to Kconfig and tidy up. Signed-off-by: Simon Glass --- configs/chromebook_link_defconfig | 2 ++ configs/chromebox_panther_defconfig

Re: [U-Boot] [PATCH 12/25] tpm: tpm_tis_i2c: Simplify init code

2015-08-11 Thread christophe.ricard
Hi Simon, Nothing much to say on tpm_tis_i2c, however this could be even more generic if providing init ops to be specified by TPM driver developers. For example: +struct dm_tpm_ops { + int (*init)(struct udevice *); + int (*open)(struct udevice *); + int (*close)(struct udev

Re: [U-Boot] [PATCH 03/25] tpm: Add Kconfig options for TPMs

2015-08-11 Thread christophe.ricard
Hi Simon, On 11/08/2015 16:47, Simon Glass wrote: Add new Kconfig options for TPMs in preparation for moving boards to use Kconfig for TPM configuration. Signed-off-by: Simon Glass --- common/Kconfig | 12 drivers/tpm/Kconfig | 52 +

Re: [U-Boot] [PATCH 11/25] tpm: tpm_tis_i2c: Move definitions into the header file

2015-08-11 Thread christophe.ricard
Hi Simon, As per my previous comment, this file may be kept common to may other drivers (lpc or from ST...). I would renamed tpm_tis_i2c.h into something more generic like tpm.h. Below please find data specific to Infineon (tpm_tis_i2c) driver. On 11/08/2015 16:48, Simon Glass wrote: Some de

Re: [U-Boot] [PATCH 10/25] tpm: tpm_tis_i2c: Merge struct tpm into tpm_chip

2015-08-11 Thread christophe.ricard
Hi Simon, I almost agree with this one. Except that here tis_init, tis_open... are already merged into tpm_tis_i2c... Best Regards Christophe On 11/08/2015 16:48, Simon Glass wrote: There are too many structures storing the same sort of information. Move the fields from struct tpm into struct

Re: [U-Boot] [PATCH 09/25] tpm: tpm_tis_i2c: Merge struct tpm_dev into tpm_chip

2015-08-11 Thread christophe.ricard
Hi Simon, As per my delivery tentative, don't you think driver specific data could be stored in a void *priv field in struct tpm_vendor_specific ? This how we manage such information in Linux drivers. Please have a look in tpm_private.h changes: http://lists.denx.de/pipermail/u-boot/2015-Augus

Re: [U-Boot] [PATCH 07/25] tpm: tpm_tis_i2c: Drop unnecessary methods

2015-08-11 Thread christophe.ricard
Hi Simon, As per my comment on patch 6, i would disagree as well on this one. It tpm_vendor_specific structure is convenient for ST33ZP24 for example. Best Regards Christophe On 11/08/2015 16:47, Simon Glass wrote: The function methods in struct tpm_vendor_specific just call local functions. C

Re: [U-Boot] [PATCH 08/25] tpm: tpm_tis_i2c: Drop struct tpm_vendor_specific

2015-08-11 Thread christophe.ricard
Hi Simon, Locality concept are valid almost on any chip assuming if no locality are supported the default one is locality 0. I would leave this change open for discussion. However, as per patch 06 & 07, i would keep req_complete_mask, req_complete_val, req_canceled, timeout_a, timeout_b, time

Re: [U-Boot] [PATCH 00/25] dm: Convert TPM drivers to driver model

2015-08-11 Thread christophe.ricard
Hi Simon, I pretty much like the move to driver model for TPM. However, i have some few remarks: The current i2c driver stick to Infineon TPMs and will not support any other vendors like ST(in my case). The main reason for this is that there is no transport protocol over I2C specification defi