Please add a more elaborate commit message for such a huge diff stat. What is SPM and why is it needed? Why does it have to be set up by the firmware, and it’s not enough that the OS sets it up? From what datasheet did you take the macro definitions?

Also, please add a follow-up commit with additional time-stamps (or stopwatch messages) so the time of this can be measured.

What is the boot time penalty of this commit?

View Change

5 comments:

• File src/soc/mediatek/mt8183/spm.c:

  • Patch Set #2, Line 225: CBFS_TYPE_RAW);

    Please use the full text width.

  • Patch Set #2, Line 243: BIOS_INFO

    Probably more debug level?

  • Patch Set #2, Line 244: (unsigned int)(unsigned long)

    Why the double cast?

  • Patch Set #2, Line 305: spm_load_firmware(index);

    Did you add time-stamps for this? I thinks these thinks should be measured for how long the take.

  • Patch Set #2, Line 310: printk(BIOS_ERR, "SPM: firmware is not ready!!!\n");

    As this is a user visible message, please add more information, what the user should do.

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