Attention is currently required from: Anastasia Klimchuk, Peter Marheine.

View Change

8 comments:

• Patchset:

  • Patch Set #4:

    Oh one more thing I forgot to mention sorry... […]

    1) I think that makes sense I have created a new change at ID 84934 for the first commit and left this one as the documenation and cli_client change.

    2) Done.

    3) Yes I would like to become a maintainer for the rpmc feature :)

• Commit Message:

  • Patch Set #4, Line 21: usefull

    usefull -> useful (typo) […]

    Done

• File cli_classic.c:

  • Patch Set #4, Line 147:

    #if CONFIG_RPMC_ENABLED == 1
    	       "RPMC COMMANDS\n"
    	       "      --get-rpmc-status             read the extended status\n"
    	       "      --write-root-key              write the root key register\n"
    	       "      --update-hmac-key             update the hmac key register\n"
    	       "      --increment-counter <previous>\n"
    	       "                                    increment rpmc counter\n"
    	       "      --get-counter                 get rpmc counter\n"
    	       "RPMC OPTIONS\n"
    	       "      --counter-address <number>    counter address (default: 0)\n"
    	       "      --rpmc-root-key <keyfile>     rpmc root key file\n"
    	       "      --key-data <keydata>          hex number to use as key data (default: 0)\n"
    #endif // CONFIG_RPMC_ENABLED
    

    These new commands need to also be documented on the manpage. […]

    Done

  • Patch Set #4, Line 1323:

    	if (options.rpmc_read_data)
    		ret = rpmc_read_data(fill_flash);
    

    I think having code in a separate file rpmc.c is better, you don't need to move it. Keep rpmc. […]

    Ok then

• File include/flash.h:

  • Patch Set #4, Line 553: implement

    I know it's a small thing, but if you start with uppercase it would look better.

    This comment doesn't really make sense in the final commit, since it was more of an todo for me. I have removed it.

  • Patch Set #4, Line 575:

    		/*
    		 * all times in microsecond (us)
    		 */
    

    Also small comment, but this can be 1-line comment, and starting with uppercase […]

    Done

  • Patch Set #4, Line 581: rpmc_ctx

    I understand this goes into a flashchip definition? and you mentioned you tested on one chip? It wou […]

    This is sadly not as easily possible, since the chip reports itself with the same id as the W25Q128.V. So adding this feature would require changing the polling function to basically just be the same as the one for sfdp-capable chips.

• File meson.build:

  • Patch Set #4, Line 177:

    if libcrypto.found()
        add_project_arguments('-DCONFIG_RPMC_ENABLED=1', language : 'c')
    

    This ignores the meson option you added. […]

    Done

To view, visit change 84840. To unsubscribe, or for help writing mail filters, visit settings.