Dear Pierre Aubert, In message <1398182087-14913-4-git-send-email-p.aub...@staubli.com> you wrote: > This sub-command adds support for the RPMB partition of an eMMC: > * mmc rpmb key <address of the authentication key> > Programs the authentication key in the eMMC This key can not > be overwritten. > * mmc rpmb read <address> <block> <#count> [address of key] > Reads <#count> blocks of 256 bytes in the RPMB partition > beginning at block number <block>. If the optionnal > address of the authentication key is provided, the > Message Authentication Code (MAC) is verified on each > block. > * mmc rpmb write <address> <block> <#count> <address of key> > Writes <#count> blocks of 256 bytes in the RPMB partition > beginning at block number <block>. The datas are signed > with the key provided. > * mmc rpmb counter > Returns the 'Write counter' of the RPMB partition. > > The sub-command is conditional on compilation flag CONFIG_SUPPORT_EMMC_RPMB > > Signed-off-by: Pierre Aubert <p.aub...@staubli.com> > CC: Pantelis Antoniou <pa...@antoniou-consulting.com> > --- > README | 10 ++++ > common/cmd_mmc.c | 123 > +++++++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 132 insertions(+), 1 deletions(-)
Unfortunatley there is no changelog here. Please note that this is mandatory for updated patch versions, see [1] [1] http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions A changelog in the cover letter is useless, as this does not get recorded in patchwork. The changelog must always be in the patch itself. ... > + if (argc == 4 && strcmp(argv[2], "key") == 0) > + state = RPMB_KEY; > + if ((argc == 6 || argc == 7) && strcmp(argv[2], "read") == 0) > + state = RPMB_READ; > + else if (argc == 7 && strcmp(argv[2], "write") == 0) > + state = RPMB_WRITE; > + else if (argc == 3 && strcmp(argv[2], "counter") == 0) > + state = RPMB_COUNTER; As mentiuoned before, these repeated strcmp() should probably be replaced by a subcommand table. > + ret = CMD_RET_SUCCESS; > + if (state == RPMB_KEY) { > + key_addr = (void *)simple_strtoul(argv[3], NULL, 16); > + if (!confirm_key_prog()) > + return CMD_RET_FAILURE; > + if (mmc_rpmb_set_key(mmc, key_addr)) { > + printf("ERROR - Key already programmed ?\n"); > + ret = CMD_RET_FAILURE; Do a return here, or use some "goto errout" and place the label errout before the needed cleanup acrtions. > + } > + } else if (state == RPMB_COUNTER) { You can then get rid of this "else" level. > + unsigned long counter; > + > + if (mmc_rpmb_get_counter(mmc, &counter)) > + ret = CMD_RET_FAILURE; > + else Ditto. > + printf("Write counter= %lx\n", counter); Should this be a debug() ? > +#ifdef CONFIG_SUPPORT_EMMC_RPMB > + } else if (strcmp(argv[1], "rpmb") == 0) { > + return do_mmcrpmb(argc, argv); > +#endif /* CONFIG_SUPPORT_EMMC_RPMB */ > } Again, it would be better to switch the code to regular subcommand processing. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "I knew then (in 1970) that a 4-kbyte minicomputer would cost as much as a house. So I reasoned that after college, I'd have to live cheaply in an apartment and put all my money into owning a computer." - Apple co-founder Steve Wozniak, EE Times, June 6, 1988, pg 45 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
