Attention is currently required from: Werner Zeh. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60711 )
Change subject: Add Elkhart Lake support ......................................................................
Patch Set 2: Code-Review+2
(5 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/60711/comment/ddf2ee0b_f024667c PS2, Line 15: TEST=Read and flash complete flash on Siemens MC EHL1 Some tests with --ifd and writing only specific regions would be nice to have. This can also happen with an external programmer.
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/60711/comment/26618e8d_2db36d6d PS1, Line 709: boot_straps = boot_straps_pch500;
In EDS there is just SPI defined (bbs=0), whereas bbs=1 is marked as reserved. […]
Hmmm, I was looking at revision 1.3 which says UFS. But I wouldn't mind to leave it as `reserved`, it feels unlikely to find it in the wild and we would return BUS_NONE, anyway.
https://review.coreboot.org/c/flashrom/+/60711/comment/46df6eb8_892a62a9 PS1, Line 995: dev
I guess this would be something for a follow-up patch, right?
Yep, at your convenience. Just something that could be done better for newer platforms in general. AIUI, at least Tiger Lake could be treated the same. Don't know actually since when it's not hidden anymore.
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/60711/comment/3ac0ee97_2d88b46f PS1, Line 1033: return CHIPSET_500_SERIES_TIGER_POINT;
Yes, I was starring at this code back than for a while. […]
Please also test if it detects the right platform. Should be easy with ich_descriptors_tool (in util/) run on one of your EHL images ;)
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/60711/comment/8d4e0af0_ef3f6085 PS1, Line 2027: ich_gen == CHIPSET_ELKHART_LAKE)) {
If moved below, something like this? […]
Sounds good.