Nico Huber has posted comments on this change. ( https://review.coreboot.org/26653 )
Change subject: drivers/intel/gma: Include mainboard data.vbt ......................................................................
Patch Set 3:
(2 comments)
Looks good, but I don't like to scatter related Kconfig options around.
https://review.coreboot.org/#/c/26653/3/src/device/Kconfig File src/device/Kconfig:
https://review.coreboot.org/#/c/26653/3/src/device/Kconfig@614 PS3, Line 614: if INTEL_GMA_VBT_HAVE_SOURCE_FILE && VARIANT_DIR != "" Please indent further by another tab.
https://review.coreboot.org/#/c/26653/3/src/drivers/intel/gma/Kconfig File src/drivers/intel/gma/Kconfig:
https://review.coreboot.org/#/c/26653/3/src/drivers/intel/gma/Kconfig@16 PS3, Line 16: config INTEL_GMA_VBT_HAVE_SOURCE_FILE Why not place this in device/Kconfig together with the other VBT options? Or move the other options here (would move them into the drivers menu, though)?
Also, why name it `SOURCE_FILE` when it's `DATA_FILE` in the other option? I would prefer to have neither suffix, just `INTEL_GMA_HAVE_VBT` / `INTEL_GMA_ADD_VBT`.
-
Nico Huber (Code Review)