Attention is currently required from: Arthur Heymans, Nico Huber, Jonathan Zhang, David Hendricks, Edward O'Callaghan, Angel Pons. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57633 )
Change subject: ich_descriptors: Explicitly check for Ibex Peak ISL value ......................................................................
Patch Set 2: -Code-Review
(2 comments)
Patchset:
PS2:
This looks like it suffers from the same problem as another patch that I gave a -2 for a reason. You indirectly change paths (look at what cases hit GEMINILAKE for instance) without noticing.
I'm pretty sure the APL/GLK descriptors never have ISL <= 16, so it shouldn't be an issue. But I could be wrong, so let's try to preserve the current behavior.
The current patch also changes when the warning is printed (not anymore for ISL < 16) while the commit message states otherwise.
Indeed, this is an issue.
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/57633/comment/5a7a029e_d8c6e5af PS2, Line 936: if (content->ISL <= 16) : return CHIPSET_5_SERIES_IBEX_PEAK;
How about: […]
Based on Nico's comment, looks like this should be placed after the `if (content->FLMAP2 == 0) { ... }` block.
-
Angel Pons (Code Review)