[coreboot-gerrit] Change in coreboot[master]: drivers/intel/gma: Include mainboard data.vbt

Nico Huber (Code Review) gerrit at coreboot.org
Thu May 31 12:36:38 CEST 2018


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`.



-- 
To view, visit https://review.coreboot.org/26653
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I730cb0737945631e2d5379a9e26b8c039ec6dc49
Gerrit-Change-Number: 26653
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Matt DeVillier <matt.devillier at gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Thu, 31 May 2018 10:36:38 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180531/c0a6cc80/attachment.html>


More information about the coreboot-gerrit mailing list