Attention is currently required from: Damien Zammit. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62759 )
Change subject: hp/z220_cmt_workstation: Add variant of z220_sff_workstation ......................................................................
Patch Set 1: Code-Review+1
(5 comments)
File src/mainboard/hp/z220_sff_workstation/Kconfig:
https://review.coreboot.org/c/coreboot/+/62759/comment/c9c16218_f98e225e PS1, Line 1: BOARD_HP_Z220_VARIANT As this symbol is for a group of boards, I'd rename it to `BOARD_HP_Z220_SERIES`
https://review.coreboot.org/c/coreboot/+/62759/comment/4ef7308b_31fa64c6 PS1, Line 52: Please don't make a copy of data.vbt, just keep it where it is and add this here:
# Override the default variant behavior, since the data.vbt is the same config INTEL_GMA_VBT_FILE default "src/mainboard/$(MAINBOARDDIR)/data.vbt"
https://review.coreboot.org/c/coreboot/+/62759/comment/723cc542_8c56df18 PS1, Line 53: config VGA_BIOS_FILE : string : default "pci8086,0102.rom" : : config VGA_BIOS_ID : string : default "8086,0102" Where did this come from? Please remove, it doesn't make sense for a board with a socketed CPU.
File src/mainboard/hp/z220_sff_workstation/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/62759/comment/5a01c65b_06fd00a6 PS1, Line 3: nit: spaces not needed
File src/mainboard/hp/z220_sff_workstation/variants/z220_cmt_workstation/gpio.c:
PS1: Are there many differences between the GPIO settings?