Hi Jean-Luc,
On 8 July 2014 16:05, Jean-Luc BLANC <stmicroelectronics....@gmail.com> wrote:
> This driver add support for STMicroelectronics ST33ZP24 SPI TPM.
s/add/adds/
> Driver support 2 SPI TPMs.
> Driver support also hash in Locality 4 feature (the only way to
On both lines s/support/supports/
> update PCR17).
> ---
> README | 29 ++
> common/cmd_tpm.c | 63 +++-
> drivers/tpm/Makefile | 1 +
> drivers/tpm/tpm_spi_stm_st33.c | 724
> ++++++++++++++++++++++++++++++++++++++++
> include/tis.h | 11 +-
> include/tpm.h | 22 ++
> lib/tpm.c | 26 ++
> 7 files changed, 874 insertions(+), 2 deletions(-)
> create mode 100644 drivers/tpm/tpm_spi_stm_st33.c
>
> diff --git a/README b/README
> index a248ab5..a4aa28a 100644
> --- a/README
> +++ b/README
> @@ -1397,6 +1397,35 @@ The following options need to be configured:
> Define this to enable authorized functions in the TPM library.
> Requires CONFIG_TPM and CONFIG_SHA1.
>
> + CONFIG_TPM_ST
> + Support additional hash in locality 4 command for
> + STMicroelectronics TPMs (SPI or I2C). Require CONFIG_CMD_TPM.
> +
> + CONFIG_TPM_ST_SPI
> + Support SPI STMicroelectronics TPM. Require SPI support
s/Require/Requires/
> +
> + TPM0_SPI_MAX_SPEED
> + Define SPI frequency for TPM, 10000000 Hz max
> +
> + TPM0_SPI_BUS_NUM
> + Define SPI Bus ID connected to TPM
> +
> + TPM0_SPI_CS
> + Define SPI Chip Select ID connected to TPM
> +
> + CONFIG_TPM_ST_2TPM
> + Support additional STMicoelectronics SPI TPM.
> + Require CONFIG_TPM_ST_SPI
> +
> + TPM1_SPI_MAX_SPEED
> + Define SPI frequency for TPM, 10000000 Hz max
> +
> + TPM1_SPI_BUS_NUM
> + Define SPI Bus ID connected to TPM
> +
> + TPM1_SPI_CS
> + Define SPI Chip Select ID connected to TPM
Not essential, but it would be nice to support CONFIG_OF_CONTROL and
get these parameters from the device tree, as other TPM drivers do.
> +
> - USB Support:
> At the moment only the UHCI host controller is
> supported (PIP405, MIP405, MPC5200); define
> diff --git a/common/cmd_tpm.c b/common/cmd_tpm.c
> index 0294952..63f52e4 100644
> --- a/common/cmd_tpm.c
> +++ b/common/cmd_tpm.c
> @@ -334,6 +334,29 @@ static int do_tpm_extend(cmd_tbl_t *cmdtp, int flag,
> return convert_return_code(rc);
> }
>
> +#ifdef CONFIG_TPM_ST
Is this a feature only supported by ST, in which case this code is
correct, or is it a standard feature that other chips may support, in
which case you should add a new CONFIG like CONFIG_TPM_HASH_LOC4?
> +static int do_tpm_hash_loc4(cmd_tbl_t *cmdtp, int flag,
> + int argc, char * const argv[])
> +{
> + uint32_t rc;
> + size_t count;
> + void *data;
> +
> + if (argc != 2)
> + return CMD_RET_USAGE;
> +
> + data = parse_byte_string(argv[1], NULL, &count);
> + if (!data) {
> + printf("Couldn't parse byte string %s\n", argv[1]);
Can we move this error into the parse_byte_string() to avoid repeating it?
> + return CMD_RET_FAILURE;
> + }
> +
> + rc = tpm_hash_loc4(data, count);
> + free(data);
> + return convert_return_code(rc);
> +}
> +#endif /* CONFIG_TPM_ST */
> +
> static int do_tpm_pcr_read(cmd_tbl_t *cmdtp, int flag,
> int argc, char * const argv[])
> {
> @@ -355,6 +378,25 @@ static int do_tpm_pcr_read(cmd_tbl_t *cmdtp, int flag,
> return convert_return_code(rc);
> }
>
> +#ifdef CONFIG_TPM_ST_2TPM
> +static int do_tpm_spi_select(cmd_tbl_t *cmdtp, int flag,
> + int argc, char * const argv[])
> +{
> + uint32_t rc, spi_number;
> +
> + if (argc != 2)
> + return CMD_RET_USAGE;
> + spi_number = simple_strtoul(argv[1], NULL, 0);
> + if (spi_number < CONFIG_TPM_ST_2TPM) {
> + rc = tpm_spi_select(spi_number);
> + } else {
> + printf("Couldn't parse argument %s\n", argv[1]);
> + return CMD_RET_FAILURE;
> + }
> + return convert_return_code(rc);
> +}
> +#endif /* CONFIG_TPM_ST_2TPM */
This seems quite specific to SPI. Why not have a 'tpm number' and have
it wholly kept in this file? It looks like the tpm.h file should be
changed to add a TPM number to each call? If so, that should be a
separate patch. This is what the 'i2c dev' command does. See how SPI
FLASH works for another example (sf probe command).
> +
> static int do_tpm_tsc_physical_presence(cmd_tbl_t *cmdtp, int flag,
> int argc, char * const argv[])
> {
> @@ -629,8 +671,16 @@ static cmd_tbl_t tpm_commands[] = {
> do_tpm_nv_write_value, "", ""),
> U_BOOT_CMD_MKENT(extend, 0, 1,
> do_tpm_extend, "", ""),
> +#ifdef CONFIG_TPM_ST
> + U_BOOT_CMD_MKENT(hash_loc4, 0, 1,
> + do_tpm_hash_loc4, "", ""),
> +#endif /* CONFIG_TPM_ST */
> U_BOOT_CMD_MKENT(pcr_read, 0, 1,
> - do_tpm_pcr_read, "", ""),
> + do_tpm_pcr_read, "", ""),
> +#ifdef CONFIG_TPM_ST_2TPM
> + U_BOOT_CMD_MKENT(spi_select, 0, 1,
> + do_tpm_spi_select, "", ""),
> +#endif /* CONFIG_TPM_ST_2TPM */
> U_BOOT_CMD_MKENT(tsc_physical_presence, 0, 1,
> do_tpm_tsc_physical_presence, "", ""),
> U_BOOT_CMD_MKENT(read_pubek, 0, 1,
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
