Attention is currently required from: Alexander Couzens, Maciej Pijanowski, Michał Kopeć, Paul Menzel.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80609?usp=email )
Change subject: mb/lenovo: Add ThinkCentre M920q (Cannon Lake) ......................................................................
Patch Set 7: Code-Review+1
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80609/comment/26175a6d_f891a535 : PS7, Line 7: Cannon Lake That's Coffee Lake, even if the coreboot code is named `cannonlake`
https://review.coreboot.org/c/coreboot/+/80609/comment/7f5bf373_df06c2c6 : PS7, Line 28: Display Port nit: If you mean the VESA standard, it's "DisplayPort" without a space
File src/mainboard/lenovo/m920q/cmos.layout:
https://review.coreboot.org/c/coreboot/+/80609/comment/6a5c2591_61a1eb1f : PS7, Line 24: 408 1 e 1 nmi Is this option actually implemented?
File src/mainboard/lenovo/m920q/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/80609/comment/68b2dc1c_5276c9c3 : PS7, Line 7: register "RMT" = "1" Shouldn't this be a Kconfig option?
File src/mainboard/lenovo/m920q/romstage.c:
https://review.coreboot.org/c/coreboot/+/80609/comment/feada1ee_c85b65f5 : PS6, Line 33: /* Rcomp target values for CFL-S, DDR4 and 2 DIMMs per channel */
Updated the comment. […]
I barely remember, but I think the only difference (if any) is the value of the first target (50 or 60)
-
Angel Pons (Code Review)