Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43791 )
Change subject: soc/intel/jasperlake: Add FSP UPD for minimum assertion widths ......................................................................
Patch Set 1:
(10 comments)
https://review.coreboot.org/c/coreboot/+/43791/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43791/1//COMMIT_MSG@11 PS1, Line 11: maiboard mainboard
https://review.coreboot.org/c/coreboot/+/43791/1//COMMIT_MSG@12 PS1, Line 12: * PchPmSlpS3MinAssert: SLP_S3 Minimum Assertion Width Policy. Please add a blank line above.
https://review.coreboot.org/c/coreboot/+/43791/1//COMMIT_MSG@13 PS1, Line 13: * PchPmSlpS4MinAssert: SLP_S4 Minimum Assertion Width Policy. : * PchPmSlpSusMinAssert: SLP_SUS Minimum Assertion Width Policy. : * PchPmSlpAMinAssert: SLP_A Minimum Assertion Width Policy. : * PchPmPwrCycDur: PCH PM Reset Power Cycle Duration. Please remove the dot/period at the end.
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/ch... File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/ch... PS1, Line 340: - Remove or use a colon:
… registers:
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/ch... PS1, Line 344: * - PM_CFG.SLP_LAN_MIN_ASST_WDTH Can you check this with asserts?
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/fs... PS1, Line 46: MinAssrtDur0s = 0, : MinAssrtDur60us = 60, : MinAssrtDur1ms = 1000, : MinAssrtDur50ms = 50000, : MinAssrtDur98ms = 98000, : MinAssrtDur500ms = 500000, : MinAssrtDur1s = 1000000, : MinAssrtDur2s = 2000000, : MinAssrtDur3s = 3000000, : MinAssrtDur4s = 4000000, Please use tabs for aligning the =, and please move them closer to the names.
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/fs... PS1, Line 74: #define PCH_PM_PWR_CYC_DUR 0 Please use a tab for aligning the 0.
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/fs... PS1, Line 170: asserton assertion
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/fs... PS1, Line 190: BIOS_DEBUG Please make that an error.
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/fs... PS1, Line 191: PmPwrCycDur(%d) Please add a space before the (.