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 2:
(3 comments)
https://review.coreboot.org/#/c/20922/2/chipset_enable.c File chipset_enable.c:
https://review.coreboot.org/#/c/20922/2/chipset_enable.c@836 PS2, Line 836: and s/and/or/? (ich you read the identifier as a statement...)
https://review.coreboot.org/#/c/20922/2/ich_descriptors.c File ich_descriptors.c:
https://review.coreboot.org/#/c/20922/2/ich_descriptors.c@76 PS2, Line 76: else : return -1; You could remove both else paths and just return -1 at the end.
https://review.coreboot.org/#/c/20922/2/ichspi.c File ichspi.c:
https://review.coreboot.org/#/c/20922/2/ichspi.c@1703 PS2, Line 1703: num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */
Depending on what the regular PCH returns for the register addresses,
10 is correct, the registers read with 0x00007fff just what is set in the descriptor.
12 would work too for the regular PCH (registers read 0 and are ignored by the current code).
-
Nico Huber (Code Review)