Attention is currently required from: Bora Guvendik, Cliff Huang, Wonkyu Kim.
Maximilian Brune has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/87247?usp=email )
Change subject: ec/intel: read board ID one time from EC per stage ......................................................................
Patch Set 10:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/87247/comment/71e4f86f_b76545c0?usp... : PS10, Line 9: optimizes boot time
Please provide numbers
I personally don't think that this should be required in this case. It is fairly obvious that this patch will cause a performance improvement (even if by just a little) and it is a common optimization technique in our coreboot codebase. I agree that boot times should usually be added, but I think in this case I would not require it from the author.
https://review.coreboot.org/c/coreboot/+/87247/comment/02f95d57_20c735a9?usp... : PS10, Line 11: This approach reduces redundant hardware access, ensuring efficient : and consistent retrieval of the board ID throughout the boot process. : Static variables help streamline operations, minimize overhead, and : improve performance by maintaining the board ID in a fixed memory : location, enhancing the efficiency of each boot stage. I think this part is redundant. It sounds like an AI generated (not that anything is wrong with that) formal justification for this patch. But that is already captured perfectly fine by the previous sentence.
-
Maximilian Brune (Code Review)