Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35921 )
Change subject: mb/google/glados: port to FSP 2.0 ......................................................................
Uploaded patch set 4.
(3 comments)
https://review.coreboot.org/c/coreboot/+/35921/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35921/3//COMMIT_MSG@18 PS3, Line 18: - set FSP board type to "mobile"
this is being set to BOARD_TYPE_ULT_ULX at SoC level, why do we want to override? what are consequen […]
overlooked that, thanks!
https://review.coreboot.org/c/coreboot/+/35921/3//COMMIT_MSG@19 PS3, Line 19: - add FSP parameter DmiVc1 (see TODO)
this doesn't seem necessary (at least on chell), so def need better understanding on its addition gi […]
hmm, interesting. I found that vendor BIOS on supermicro boards has a option for the user to enable/disable this. enabling channel 1 (Vc1) for dmi shall improve performance of the PCH-MCH communication.... it is not required, but I wonder if we should set this as default on SKL/KBL/...?
https://review.coreboot.org/c/coreboot/+/35921/3//COMMIT_MSG@23 PS3, Line 23: - loadline values in devicetree maybe can dropped again, as Maxim is : working on adding missing loadline values in CB:35167
I would recommend dropping them and simply rebasing your patchset on top of that change, no reason t […]
Ack
-
Michael Niewöhner (Code Review)