Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30890 )
Change subject: autoport: Add support for Haswell-LynxPoint platform ......................................................................
Patch Set 2:
(10 comments)
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go File util/autoport/lynxpoint.go:
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go@161 PS1, Line 161: if dmi.Vendor == "LENOVO" { : KconfigInt["DRAM_RESET_GATE_GPIO"] = 10 : } else { : KconfigInt["DRAM_RESET_GATE_GPIO"] = 60 : } : KconfigComment["DRAM_RESET_GATE_GPIO"] = "FIXME: check this"
Lynx Point doesn't currently implement handling of DRAM_RESET_GATE_GPIO.
Done
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go@189 PS1, Line 189: : /* SPI init */ : MainboardIncludes = append(MainboardIncludes, "southbridge/intel/lynxpoint/pch.h")
This isn't needed, I think.
Done
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go@211 PS1, Line 211: "sata_port_map": fmt.Sprintf("0x%x", PCIMap[PCIAddr{Bus: 0, Dev: 0x1f, Func: 2}].ConfigDump[0x92]&0x3f),
I think this should go next to the other SATA register.
Done
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go@214 PS1, Line 214: USB 3.0
Better to say xHCI controller.
Done
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go@231 PS1, Line 231: PCISlot{PCIAddr: PCIAddr{Dev: 0x1e, Func: 0}, writeEmpty: true, additionalComment: "PCI bridge"},
Doesn't exist on Lynx Point.
Done
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go@272 PS1, Line 272: RestoreDIRRoute(sb, "D31IR", uint16(inteltool.RCBA[0x3140])) : RestoreDIRRoute(sb, "D29IR", uint16(inteltool.RCBA[0x3144])) : RestoreDIRRoute(sb, "D28IR", uint16(inteltool.RCBA[0x3146])) : RestoreDIRRoute(sb, "D27IR", uint16(inteltool.RCBA[0x3148])) : RestoreDIRRoute(sb, "D26IR", uint16(inteltool.RCBA[0x314c])) : RestoreDIRRoute(sb, "D25IR", uint16(inteltool.RCBA[0x3150])) : RestoreDIRRoute(sb, "D22IR", uint16(inteltool.RCBA[0x315c])) : RestoreDIRRoute(sb, "D20IR", uint16(inteltool.RCBA[0x3160]))
I typically leave this to reset default, or use the defaults on my platform. […]
I see google/slippy/variants/peppy also writes DxxIP registers. I still don't know which are needed.
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go@356 PS1, Line 356: 0x0040
Sometimes the USB port might need a greater value here, if the trace […]
I saw in mrc.bin that the position and length are computed to another value, then written to IOBP. But currently there isn't a tool to read IOBP registers.
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go@392 PS1, Line 392: &rcba_config[0]
Just `rcba_config` should be fine.
Done
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go@461 PS1, Line 461: RegisterPCI(0x8086, 0x1c22, GenericPCI{MissingParent: "smbus"}) : RegisterPCI(0x8086, 0x1e22, GenericPCI{MissingParent: "smbus"})
These are for Cougar/Panther Point, not Lynx Point.
Oh, I just copied these IDs from lynxpoint/smbus.c, and didn't check the document.
https://review.coreboot.org/#/c/30890/1/util/autoport/lynxpoint.go@488 PS1, Line 488: 0x8c3a, 0x8c3b,
Also 0x8c3c, 0x8c3d, 0x9c3a, 0x9c3b, 0x9c3c, 0x9c3d.
Done