Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41719 )
Change subject: amd/picasso: Load x86 microcode from CBFS modules ......................................................................
Patch Set 8:
(3 comments)
Three unresolved comments.
Please review to see if it is OK to click solved?
https://review.coreboot.org/c/coreboot/+/41719/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41719/5//COMMIT_MSG@11 PS5, Line 11:
Why is this Picasso specific? Can’t this be put into common code?
In theory, it can. But it needs too much work to extract generality and individuality. Can we do it later?
https://review.coreboot.org/c/coreboot/+/41719/1/src/soc/amd/picasso/microco... File src/soc/amd/picasso/microcode_picasso.c:
https://review.coreboot.org/c/coreboot/+/41719/1/src/soc/amd/picasso/microco... PS1, Line 106: uint32_t equivalent_processor_rev_id
It might be good to explain this equivalent_processor_rev_id in the commit message?
It is hard to explain. It is explained in Revision Guide.
https://review.coreboot.org/c/coreboot/+/41719/1/src/soc/amd/picasso/update_... File src/soc/amd/picasso/update_microcode.c:
https://review.coreboot.org/c/coreboot/+/41719/1/src/soc/amd/picasso/update_... PS1, Line 23: /* Array terminator */ : { 0xffffff, 0x0000 },
No reason. […]
Solved?