Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32045 )
Change subject: soc/intel/skylake: Add devicetree options for PEG ......................................................................
Patch Set 1:
(3 comments)
Thanks for taking care of this.
https://review.coreboot.org/#/c/32045/1/src/soc/intel/skylake/Kconfig File src/soc/intel/skylake/Kconfig:
https://review.coreboot.org/#/c/32045/1/src/soc/intel/skylake/Kconfig@263 PS1, Line 263: default 3 Would this ever be something else? If not it could be a constant in the code.
https://review.coreboot.org/#/c/32045/1/src/soc/intel/skylake/romstage/romst... File src/soc/intel/skylake/romstage/romstage_fsp20.c:
https://review.coreboot.org/#/c/32045/1/src/soc/intel/skylake/romstage/romst... PS1, Line 253: m_t_cfg->Peg2Gen3EqPh2Enable = config->PegGen3EqPh2Enable[2]; What about `PegGen3EqPh3Method`? I wouldn't mind hard- coding it to `0` here. That's still better than not knowing what it's set to.
https://review.coreboot.org/#/c/32045/1/src/soc/intel/skylake/romstage/romst... PS1, Line 254: } Until somebody tells me something else, I consider every setting that differs from "Auto" as a workaround for a bug. So I wonder, why would we ever want the same overrides on all three ports?
Please elaborate what settings you need and what the defaults in the binary are. I would prefer as many reasonable hard-coded defaults as possible. Only when we really need a non-default, I'd add settings to `chip.h`.