I'm kind of new to u-boot and I'm not really able to review this code as well as I should.
But also I can't apply the patch. It seems white space damaged? The kernel has a good document on how to do this. I'm pretty sure u-boot does as well but I'm new. https://www.kernel.org/doc/Documentation/process/email-clients.rst Please run your patch through the scripts/checkpatch.pl script. Stuff like this triggers a warning: > +static int do_tpm_nv_write_value(struct cmd_tbl *cmdtp, int flag, > + int argc, char *const argv[]) //TODO: session handle > from auth session! > +{ > + struct udevice *dev; > + u32 nv_addr, nv_size, rc; > + void *session_addr = NULL; > + int ret; > + > + ret = get_tpm(&dev); > + if (ret) > + return ret; > + > + if (argc < 4) > + return CMD_RET_USAGE; WARNING: suspect code indent for conditional statements (0, 0) #250: FILE: cmd/tpm-v2.c:437: + if (ret) + return ret; WARNING: suspect code indent for conditional statements (0, 0) #253: FILE: cmd/tpm-v2.c:440: + if (argc < 4) + return CMD_RET_USAGE; Also the subject should have a subsystem prefix and the information from the email should be moved into the commit message. Currently the commit message is empty. > diff --git a/include/tpm-v2.h b/include/tpm-v2.h > index 33dd103767..5b60883777 100644 > --- a/include/tpm-v2.h > +++ b/include/tpm-v2.h > @@ -301,7 +301,8 @@ enum tpm2_startup_types { > */ > enum tpm2_handles { > TPM2_RH_OWNER = 0x40000001, > - TPM2_RS_PW = 0x40000009, > + TPM2_RH_NULL = 0x40000007, > + TPM2_RS_PW = 0x40000009, Changing TPM2_RS_PW is an unrelated whitespace change. Do that as a separate patch. But I don't get it at all because the TPM2_RS_PW enum has always been indented correctly as far as I can see. So it's a puzzle. I mean there are a lot of TODOs and I understand that you just wanted a high level review but I kept getting distracted and lost and I couldn't apply the patch so it was just really hard to figure out what was going on. :( regards, dan carpenter
