Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47091 )
Change subject: chipset_enable.c: Add Geminilake support ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Uh, Gemini Lake has a different descriptor layout. This change as-is will make flashrom assume an invalid descriptor (that for WildcatPoint). While the APL layout is good enough, there's a few differences to account for.
I've got CB:43349 which handles this, but I went berserk and made a full descriptor dumper out of it...
So while everything you said is true the support here seems sufficiently good enough as I am taking it verbatim from the ChromiumOS Flashrom that is used in production. Not that I am saying that's a gold standard by any means! Just that it's better than not working at all.
I am not sure what is the best way forwards here; have the basic support just merged and iterate on that or maybe move forwards you series? In my view the former seems like a pragmatic path however, either way, we should get it across the line so ich is completely in-sync with CrOS and so we have Intel directly sending these sorts of patches to Flashrom moving forwards rather than adhoc fixes.
If you can at least adapt `guess_ich_chipset_from_content` to assume Apollo Lake compatibility, it would be good enough:
https://review.coreboot.org/c/flashrom/+/43349/4/ich_descriptors.c#1321