[coreboot-gerrit] Change in coreboot[master]: mb/asus/p5qpl-am: Add support for ASUS P5QPL-AM

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/25659 )

Change subject: mb/asus/p5qpl-am: Add support for ASUS P5QPL-AM
......................................................................


Patch Set 1:

(8 comments)

Looks ok but there are a few thing to check that can be mainboard specific.

https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/acpi/ich7_pci_irqs.asl
File src/mainboard/asus/p5qpl-am/acpi/ich7_pci_irqs.asl:

https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/acpi/ich7_pci_irqs.asl@20
PS1, Line 20: 
This is mainboard dependent. Please check this in vendor DSDT or schematics if you're lucky enough to find those.


https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/devicetree.cb
File src/mainboard/asus/p5qpl-am/devicetree.cb:

https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/devicetree.cb@59
PS1, Line 59: 			device pci 1c.3 off end		# PCIe 4
Better be complete and add 1c.4, 1c.5 off too. (does not matter with this code though)


https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/devicetree.cb@75
PS1, Line 75: 			device pci 1e.0 on end		# PCI bridge
add device pci 1e.2 off end and device pci 1e.3 off end please (Those are disabled in romstage but better be complete)


https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/devicetree.cb@127
PS1, Line 127: # W83115RG-965
you sure your device has this?


https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/hda_verb.c
File src/mainboard/asus/p5qpl-am/hda_verb.c:

https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/hda_verb.c@16
PS1, Line 16: 
could be different on this mainboard: please check /proc/asound/card0/codec#


https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/romstage.c
File src/mainboard/asus/p5qpl-am/romstage.c:

https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/romstage.c@24
PS1, Line 24: #include <superio/nuvoton/nct6776/nct6776.h>
remove


https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/romstage.c@27
PS1, Line 27: #include <superio/nuvoton/common/nuvoton.h>
remove


https://review.coreboot.org/#/c/25659/1/src/mainboard/asus/p5qpl-am/romstage.c@113
PS1, Line 113: 
remove newline



-- 
To view, visit https://review.coreboot.org/25659
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: I160f9201e812f44ef9842caef851750fb7690c9e
Gerrit-Change-Number: 25659
Gerrit-PatchSet: 1
Gerrit-Owner: AndiR <andir at memeware.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-CC: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Comment-Date: Fri, 13 Apr 2018 19:42:50 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180413/ccbae6ff/attachment.html>