Hi, Sorry, I forgot to report the Reviewed-by of Miquel Raynal.
> Objet: [PATCH V3 2/2] cmd: tpm: add a subcommand device > The command tpm (and tpm2) search the tpm and use it. > On sandbox, there are two tpm (tpm 1.x and tpm 2.0). > So the command tpm and tpm2 are always executed with > the first tpm (tpm 1.x), and the command tpm2 always > fails. > > This add a subcommand device to command tpm and > command tpm2. Then the command tpm and tpm2 use > the device selected with the subcommand device. > > To be compatible with previous behaviour, if the > subcommand device is not used before a tpm (or tpm2) > command, the device 0 is selected. > > Signed-off-by: Philippe Reynes <philippe.rey...@softathome.com> Reviewed-by: Miquel Raynal <miquel.ray...@bootlin.com> > --- > cmd/tpm-common.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++--- > cmd/tpm-user-utils.h | 1 + > cmd/tpm-v1.c | 3 ++ > cmd/tpm-v2.c | 3 ++ > 4 files changed, 80 insertions(+), 4 deletions(-) > > Changelog: > v3: > - no change > v2: > - use helper for tpm device (idea from Miquel) > - don't set tpm in the look (feedback from Miquel) > - add a comment to explain the backward code (feedback from Miquel) > > diff --git a/cmd/tpm-common.c b/cmd/tpm-common.c > index 38900fb..42882b1 100644 > --- a/cmd/tpm-common.c > +++ b/cmd/tpm-common.c > @@ -12,6 +12,8 @@ > #include <tpm-common.h> > #include "tpm-user-utils.h" > > +static struct udevice *tpm_dev; > + > /** > * Print a byte string in hexdecimal format, 16-bytes per line. > * > @@ -231,19 +233,86 @@ int type_string_write_vars(const char *type_str, u8 > *data, > return 0; > } > > +static int tpm_show_device(void) > +{ > + struct udevice *dev; > + char buf[80]; > + int n = 0, rc; > + > + for_each_tpm_device(dev) { > + rc = tpm_get_desc(dev, buf, sizeof(buf)); > + if (rc < 0) > + printf("device %d: can't get info\n", n); > + else > + printf("device %d: %s\n", n, buf); > + > + n++; > + }; > + > + return 0; > +} > + > +static int tpm_set_device(unsigned long num) > +{ > + struct udevice *dev; > + unsigned long n = 0; > + int rc = CMD_RET_FAILURE; > + > + for_each_tpm_device(dev) { > + if (n == num) { > + rc = 0; > + break; > + } > + > + n++; > + } > + > + if (!rc) > + tpm_dev = dev; > + > + return rc; > +} > + > int get_tpm(struct udevice **devp) > { > int rc; > > - rc = uclass_first_device_err(UCLASS_TPM, devp); > - if (rc) { > - printf("Could not find TPM (ret=%d)\n", rc); > - return CMD_RET_FAILURE; > + /* > + * To keep a backward compatibility with previous code, > + * if a tpm device is not explicitly set, we set the first one. > + */ > + if (!tpm_dev) { > + rc = tpm_set_device(0); > + if (rc) { > + printf("Couldn't set TPM 0 (rc = %d)\n", rc); > + return CMD_RET_FAILURE; > + } > } > > + if (devp) > + *devp = tpm_dev; > + > return 0; > } > > +int do_tpm_device(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) > +{ > + unsigned long num; > + int rc; > + > + if (argc == 2) { > + num = simple_strtoul(argv[1], NULL, 10); > + > + rc = tpm_set_device(num); > + if (rc) > + printf("Couldn't set TPM %lu (rc = %d)\n", num, rc); > + } else { > + rc = tpm_show_device(); > + } > + > + return rc; > +} > + > int do_tpm_info(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) > { > struct udevice *dev; > diff --git a/cmd/tpm-user-utils.h b/cmd/tpm-user-utils.h > index 8ce9861..a851d9c 100644 > --- a/cmd/tpm-user-utils.h > +++ b/cmd/tpm-user-utils.h > @@ -17,6 +17,7 @@ int type_string_pack(const char *type_str, char * const > values[], u8 *data); > int type_string_write_vars(const char *type_str, u8 *data, char * const > vars[]); > int get_tpm(struct udevice **devp); > > +int do_tpm_device(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]); > int do_tpm_init(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]); > int do_tpm_info(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[]); > diff --git a/cmd/tpm-v1.c b/cmd/tpm-v1.c > index 2807331..bc34e06 100644 > --- a/cmd/tpm-v1.c > +++ b/cmd/tpm-v1.c > @@ -645,6 +645,7 @@ TPM_COMMAND_NO_ARG(tpm_physical_enable) > TPM_COMMAND_NO_ARG(tpm_physical_disable) > > static cmd_tbl_t tpm1_commands[] = { > + U_BOOT_CMD_MKENT(device, 0, 1, do_tpm_device, "", ""), > U_BOOT_CMD_MKENT(info, 0, 1, do_tpm_info, "", ""), > U_BOOT_CMD_MKENT(init, 0, 1, do_tpm_init, "", ""), > U_BOOT_CMD_MKENT(startup, 0, 1, > @@ -721,6 +722,8 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm, > "cmd args...\n" > " - Issue TPM command <cmd> with arguments <args...>.\n" > "Admin Startup and State Commands:\n" > +" device [num device]\n" > +" - Show all devices or set the specified device\n" > " info - Show information about the TPM\n" > " init\n" > " - Put TPM into a state where it waits for 'startup' command.\n" > diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c > index 459a955..0cd3982 100644 > --- a/cmd/tpm-v2.c > +++ b/cmd/tpm-v2.c > @@ -354,6 +354,7 @@ static int do_tpm_pcr_setauthvalue(cmd_tbl_t *cmdtp, int > flag, > } > > static cmd_tbl_t tpm2_commands[] = { > + U_BOOT_CMD_MKENT(device, 0, 1, do_tpm_device, "", ""), > U_BOOT_CMD_MKENT(info, 0, 1, do_tpm_info, "", ""), > U_BOOT_CMD_MKENT(init, 0, 1, do_tpm_init, "", ""), > U_BOOT_CMD_MKENT(startup, 0, 1, do_tpm2_startup, "", ""), > @@ -381,6 +382,8 @@ cmd_tbl_t *get_tpm2_commands(unsigned int *size) > U_BOOT_CMD(tpm2, CONFIG_SYS_MAXARGS, 1, do_tpm, "Issue a TPMv2.x command", > "<command> [<arguments>]\n" > "\n" > +"device [num device]\n" > +" Show all devices or set the specified device\n" > "info\n" > " Show information about the TPM.\n" > "init\n" > -- > 2.7.4 Regards, Philippe