Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36385/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36385/3//COMMIT_MSG@18 PS3, Line 18: between h8_mainboard_init_dock() and h8_enable() should be considered.
Mainboard ops precede all others, so that won't work. […]
Refactored.
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/Kconfig File src/ec/lenovo/h8/Kconfig:
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/Kconfig@47 PS3, Line 47: config H8_DOCK_INIT
This name seems too generic. Not selecting it makes it look like there is […]
It may be better to retire the confusing H8_DOCK_EARLY_INIT and apply Patrick's idea, if Arther's statement is true.
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/Kconfig@50 PS3, Line 50: config H8_DOCK_EARLY_INIT
This is not used anymore.
Done
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c:
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c@357 PS2, Line 357: #if CONFIG(H8_DOCK_INIT)
Looking at boards that need dock init here (x200, x201, t410) I don't think it matters if it's done […]
Okay. I am going to implement the idea of Patrick Rudolph directly if the timing between h8_mainboard_init_dock() and h8_enable() does not matter.
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c:
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/h8.c@357 PS3, Line 357: #if CONFIG(H8_DOCK_INIT)
Please use a C `if (CONFIG(H8_DOCK_INIT))`. […]
Refactored