Nico Huber has posted comments on this change. ( https://review.coreboot.org/20922 )
Change subject: chispet_enable: Add support for C620-series Lewisburg PCH ......................................................................
Patch Set 4:
(4 comments)
The list in layout_from_ich_descriptors() should be updated too.
https://review.coreboot.org/#/c/20922/4/ich_descriptors.c File ich_descriptors.c:
https://review.coreboot.org/#/c/20922/4/ich_descriptors.c@320 PS4, Line 320: unknown Region 6 (counting from 0) is the Secondary BIOS region, according to some register's descriptions that control it.
https://review.coreboot.org/#/c/20922/4/ich_descriptors.c@321 PS4, Line 321: EC IIRC, this is "BMC" for Lewisburg. Maybe name it "EC/BMC"?
https://review.coreboot.org/#/c/20922/4/ich_descriptors.c@1133 PS4, Line 1133: }; I just remembered, here's a third list. Though, this one uses identi- fiers compatible to the ones in ifdtool from coreboot.
The -N and --ifd options could also be helpful to make flashrom work with unallocated space in the chip, like the gaps Sandy has (as far as I followed the discussion).
https://review.coreboot.org/#/c/20922/4/util/ich_descriptors_tool/ich_descri... File util/ich_descriptors_tool/ich_descriptors_tool.c:
https://review.coreboot.org/#/c/20922/4/util/ich_descriptors_tool/ich_descri... PS4, Line 48: }; That's a lot of maintenance, maybe we should move the list from pprint_freg() into the header file and use it instead?
-
Nico Huber (Code Review)