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

[Angel Pons (Code Review)]

Attention is currently required from: Jan Philipp Groß, Máté Kukri, Nicholas Chin.

Angel Pons 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 2: Code-Review+1

(9 comments)

Commit Message:

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

https://review.coreboot.org/c/coreboot/+/85756/comment/8c68b653_40d51fea?usp... :
PS2, Line 29: - Both USB 3.1 Gen1 headers (also not working on vendor firmware,
I think they're wired to an ASMedia USB 3.0 hub on the mainboard itself. This chip might be dead on your board.

I'm pretty sure the Z97 Extreme6 has the same ASMedia USB 3.0 hub (for the rear USB 3.0 ports), so it's likely it would just work anyway (on a board without hardware faults).

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

File src/mainboard/asrock/z87_pro4/Kconfig:

https://review.coreboot.org/c/coreboot/+/85756/comment/3047bf51_462a3752?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:

```suggestion
# FIXME: Figure out where the EHCI debug port is. It is not
#        one of the rear ports nor the vertical port on the
#        mainboard. Can't test the other ports.
config USBDEBUG_HCD_INDEX```

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

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

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

https://review.coreboot.org/c/coreboot/+/85756/comment/f730b851_eed28a47?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)

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

https://review.coreboot.org/c/coreboot/+/85756/comment/5521006f_276a1ff7?usp... :
PS2, Line 24: 
nit: remove blank line

(this should probably be fixed in autoport)

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

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

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