Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42440 )
Change subject: soc/intel/cannonlake: Add PchPmPwrCycDur to chip options ......................................................................
Patch Set 9:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42440/9/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/42440/9/src/soc/intel/cannonlake/fs... PS9, Line 51: asst to my eyes, at a glance this reads like "assistant"
https://review.coreboot.org/c/coreboot/+/42440/9/src/soc/intel/cannonlake/fs... PS9, Line 108: nit: extra space
https://review.coreboot.org/c/coreboot/+/42440/9/src/soc/intel/cannonlake/fs... PS9, Line 111: /* slp_s4_asst_dur_list : 1s, 1s, 2s, 3s, 4s(Default) */ : enum min_asst_dur slp_s4_asst_dur_list[] = { : MinAsstDur1s, MinAsstDur1s, MinAsstDur2s, MinAsstDur3s, MinAsstDur4s : }; : : /* slp_s3_asst_dur_list: 50ms, 60us, 50ms (Default), 2s */ : enum min_asst_dur slp_s3_asst_dur_list[] = { : MinAsstDur50ms, MinAsstDur60us, MinAsstDur50ms, MinAsstDur2s : }; : : /* slp_a_asst_dur_list: 2s, 0s, 4s, 98ms, 2s(Default) */ : enum min_asst_dur slp_a_asst_dur_list[] = { : MinAsstDur2s, MinAsstDur0s, MinAsstDur4s, MinAsstDur98ms, MinAsstDur2s : }; : : /* pm_pwr_cyc_dur_list: 4s(Default), 1s, 2s, 3s, 4s */ : enum min_asst_dur pm_pwr_cyc_dur_list[] = { : MinAsstDur4s, MinAsstDur1s, MinAsstDur2s, MinAsstDur3s, MinAsstDur4s : }; Kind of a bummer that the default often has to be repeated twice, but that would also require changes to all of the devictree entries too...
https://review.coreboot.org/c/coreboot/+/42440/9/src/soc/intel/cannonlake/fs... PS9, Line 536: if (config->PchPmPwrCycDur) So this is basically turning PchPmPwrCycDur into a boolean? So if selected, it will set the minimum possible assertion width, otherwise it won't program anything?