[coreboot-gerrit] [L] Change in coreboot[main]: mb/asrock: Add Z87 Pro4 (Haswell)

[Jan Philipp Groß (Code Review)]

Attention is currently required from: Angel Pons, Máté Kukri, Nicholas Chin.

Jan Philipp Groß has posted comments on this change by Jan Philipp Groß. ( https://review.coreboot.org/c/coreboot/+/85756?usp=email )

Change subject: mb/asrock: Add Z87 Pro4 (Haswell)
......................................................................

Patch Set 3:

(8 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/85756/comment/1dcaa74d_99201ed2?usp... :
PS2, Line 22: - PCI Express 3.0 x16 slots (tested with AMD RX 550 dGPU)
> I only see one slot on this mainboard

Done

https://review.coreboot.org/c/coreboot/+/85756/comment/9f7ca4ec_e4ebdb1d?usp... :
PS2, Line 30: possible hardware defect)
> nit: add 2 spaces to align with previous line

Done

File src/mainboard/asrock/z87_pro4/Kconfig:

https://review.coreboot.org/c/coreboot/+/85756/comment/5e2600f4_0b4e61b5?usp... :
PS2, Line 25: config USBDEBUG_HCD_INDEX # Neither any of the rear ports nor the vertical port on the board. Can't test the other ones.
> How about: […]

Done

File src/mainboard/asrock/z87_pro4/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/85756/comment/417008bd_ca037450?usp... :
PS2, Line 41: 			device pci 16.1 on end	 # MEI 2
> Was this device enabled when using vendor firmware?

It wasn't enabled on the initial autoport-created devicetree.cb. I must have accidentally enabled it at one point. I've disabled it again.

File src/mainboard/asrock/z87_pro4/gma-mainboard.ads:

https://review.coreboot.org/c/coreboot/+/85756/comment/ef77ca2d_9f7a1007?usp... :
PS2, Line 11:    -- FIXME: check this
> There are no DP outputs, and only 2 HDMI outputs are used (1 for HDMI, 1 for DVI)

Done

File src/mainboard/asrock/z87_pro4/hda_verb.c:

https://review.coreboot.org/c/coreboot/+/85756/comment/9d0d0123_3ff9f65f?usp... :
PS2, Line 24:
> nit: remove blank line […]

Done

File src/mainboard/asrock/z87_pro4/romstage.c:

https://review.coreboot.org/c/coreboot/+/85756/comment/60d909aa_ab5590ca?usp... :
PS2, Line 3: #include <stdint.h>
> This is probably not needed (and autoport should be updated to not add it)

Done

https://review.coreboot.org/c/coreboot/+/85756/comment/038d2730_ab82c3b5?usp... :
PS2, Line 11: /* FIXME: called after romstage_common, remove it if not used */
            : void mb_late_romstage_setup(void)
            : {
            : }
> 🧹

Done