Attention is currently required from: Subrata Banik, Edward O'Callaghan, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 )
Change subject: ichspi: Add Jasper Lake support ......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS4:
You are right `NM` is confusing, can we comment it better? Possibly copy what you just said there near verbatim into the `ich_number_of_masters()` function?
Yeah, maybe a comment would help. If you want to avoid confusion in the future, it's probably more efficient to fix it instead.
OK, I picked to follow pch300 in `prettyprint_ich_descriptor_master()` and fixed that up. `ich_number_of_masters()` was left untouched.
Have you tested it? IIRC, for the descriptor FIT gave me for JSL it would then only print two masters (even if GbE is enabled).
I wonder how much of this declarative information about the differences could be encoded into just a big table instead of scattered over heaps of switch statements in the control flow itself? A lot of it doesn't seem dynamic just XXX has 3 and YYY has 4 of "thing".
Probably worth a shot. I sometimes experiment with such things but often fail to find an obviously better solution.
Resolved?
If you can show, e.g. by testing, that there is a reason to add another enum entry, yes.
-
Nico Huber (Code Review)