Attention is currently required from: Sam McNally, Rizwan Qureshi, Edward O'Callaghan, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 )
Change subject: ichspi: Add Alder Lake support ......................................................................
Patch Set 12: Code-Review+1
(5 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62251/comment/064d449f_450b512d PS12, Line 12: TEST=```localhost ~ # flashrom --flash-name
This tests the changes to `guess_ich_chipset_from_content()`, please check the flashrom log to verify the correct chipset is detected.
This is only visible in a very verbose log. For `--ifd` the chipset is detected separately so it also works with external programmers. The line would start with "Assuming chipset ", `ich_descriptors_tool` can also be used to test.
Patchset:
PS12:
Same question as for Jasper Lake, does there anything differ from the […]
So far I've seen no difference beside the number of straps in the descriptor.
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/62251/comment/0fa2d6e6_180adf91 PS12, Line 307: case CHIPSET_600_SERIES_ALDER_POINT: Looking at the ADL-P SPI Guide Rev. 1.24, it shows the same numbers as for TGL. What document/version was used to write this? (please also mention in the commit message)
If your version of the documentation differs, we could comare the output to the settings in FIT tool (it usually has the last word at Intel).
https://review.coreboot.org/c/flashrom/+/62251/comment/6521362b_8069828f PS12, Line 1041: return CHIPSET_600_SERIES_ALDER_POINT; Please test the detection (easy to do with ich_descriptors_tool). For EHL the SPI guide needed to be fixed.
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/62251/comment/2e5c09a7_137548ce PS12, Line 1818: case CHIPSET_300_SERIES_CANNON_POINT: : case CHIPSET_400_SERIES_COMET_POINT: : case CHIPSET_500_SERIES_TIGER_POINT: : case CHIPSET_600_SERIES_ALDER_POINT: I'm not sure if all of this was guess work, I can't find more than 6 regs in datasheets. Can somebody confirm? It probably doesn't matter much, as there are no conflicting registers documented and we don't write to them anyway.
If the datasheet turns out to be incomplete, please mention which was used in the commit message and what is missing.
-
Nico Huber (Code Review)