Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37579 )
Change subject: vendorcode/intel: Remove Ice Lake FSP Bindings ......................................................................
Patch Set 9: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/37579/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37579/9//COMMIT_MSG@15 PS9, Line 15: has been : removed by Intel That's speculation. We cannot know if the headers pushed to vendorcode/ were unaltered. It's more likely that they added it to the vendorcode/ version, because it didn't compile, and never added it to the official FSP headers. Suggesting:
that are missing in the official headers.
https://review.coreboot.org/c/coreboot/+/37579/9/src/drivers/intel/fsp2_0/in... File src/drivers/intel/fsp2_0/include/fsp/soc_binding.h:
https://review.coreboot.org/c/coreboot/+/37579/9/src/drivers/intel/fsp2_0/in... PS9, Line 20: /* : * This file is a implementation specific header. i.e. different : * FSP implementations for different chipsets. : */ Does somebody understand this comment???
Was it maybe referring to one of the includes? Because this file (soc_binding.h) is obviously not implementation specific, it draws in implementation specific headers, though.
Please keep the comment where it was (directly above Base.h).
https://review.coreboot.org/c/coreboot/+/37579/9/src/drivers/intel/fsp2_0/in... PS9, Line 24: Please add a comment that the below block needs to be here to fix missing includes in the FSP headers files.
https://review.coreboot.org/c/coreboot/+/37579/6/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/37579/6/src/soc/intel/tigerlake/rom... PS6, Line 43:
I reduced this commit to icelake where I can verify things
What I meant is that at the time you pushed this, the whole soc/intel/tigerlake dir wasn't build tested by Jenkins. So you didn't notice that this change actually broke it.