Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45336 )
Change subject: soc/intel/common/pch: Add Intel common reset code ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/45336/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45336/2//COMMIT_MSG@13 PS2, Line 13: TEST=Able to boot CML and TGL platform.
Are you able to reset them too? 😊
😎
https://review.coreboot.org/c/coreboot/+/45336/2/src/soc/intel/common/pch/re... File src/soc/intel/common/pch/reset/reset.c:
https://review.coreboot.org/c/coreboot/+/45336/2/src/soc/intel/common/pch/re... PS2, Line 14: * BIOS should ensure it does a global reset : * to reset both host and Intel ME by setting : * PCH PMC [B0:D31:F2 register offset 0xAC bit 20] : */
For running text, I'd prefer shorter lines. […]
Ack
https://review.coreboot.org/c/coreboot/+/45336/2/src/soc/intel/common/pch/re... PS2, Line 20: /* Now BIOS can write 0x06 or 0x0E to 0xCF9 port : * to global reset platform */
Should fit in one line
Ack
https://review.coreboot.org/c/coreboot/+/45336/2/src/soc/intel/common/pch/re... PS2, Line 28: /* If ME unable to reset platform then : * force global reset using PMC CF9GR register*/
I'd place the comment outside of the if-block, then it might fit in a single line. […]
Ack
https://review.coreboot.org/c/coreboot/+/45336/2/src/soc/intel/common/pch/re... PS2, Line 37: FSP_STATUS_RESET_REQUIRED_3
The only exceptions are Apollo Lake and Denverton-NS. […]
Ack