Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/28791 )
Change subject: soc/intel/skylake: Ensure FSP don't override ITSS IPCx registers ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/28791/3/src/soc/intel/skylake/chip_... File src/soc/intel/skylake/chip_fsp20.c:
https://review.coreboot.org/c/coreboot/+/28791/3/src/soc/intel/skylake/chip_... PS3, Line 177: fsp_silicon_init(romstage_handoff_is_resume());
This includes calling back to the soc and then the mainboard code (mainboard_silicon_init_params()) where many boards configure the pads which includes configuring IRQ polarities. So it may supersede the config that was backed up above.
You mean when pads are configured? Why would pads be configured during mainboard_silicon_init_params() ? I see it's happening, but it doesn't seem correct.
We should be doing pad configuration either prior to this fsp call through mainboard chip->init(). I think big core's flow is messed up as the pad configuration is in the flow of the callback you mentioned as opposed to prior to this call. i.e. there is not an init() in mainboard_ops for these mainboards.
Yes, the way GPIOs are configured on big cores is messed up. We should be using mainboard chip->init() for configuring the GPIOs and not use the mainboard_silicon_init_params() callback. Basically follow what is done on APL/GLK.