V Sowmya 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 4:
(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
Done
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.
Done
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.
Done
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: […]
Done
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?
Could you elaborate on this question?
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.
Done
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.
Done
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/fs... PS1, Line 170: asserton
assertion
Done
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/fs... PS1, Line 190: BIOS_DEBUG
Please make that an error.
Done
https://review.coreboot.org/c/coreboot/+/43791/1/src/soc/intel/jasperlake/fs... PS1, Line 191: PmPwrCycDur(%d)
Please add a space before the (.
Done