Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38988 )
Change subject: mainboard: New ivybridge board ASUS P8Z77-M ......................................................................
Patch Set 4:
(15 comments)
Patch Set 4:
I think your port is another variant compared to this one: https://review.coreboot.org/c/coreboot/+/25662
It could be added as a variant, but I would wait a bit. I want to see if merging all the Asus sandy/ivy boards would be feasible. There's only two different SuperIOs.
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... File src/mainboard/asus/p8z77-m/Kconfig:
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 21: select INTEL_INT15 This is used with the VBIOS, but I don't think anyone should be using it
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... File src/mainboard/asus/p8z77-m/board_info.txt:
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 6: Flashrom support: ? Does `sudo flashrom -p internal --ifd -i bios -w build/coreboot.rom -N` work?
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... File src/mainboard/asus/p8z77-m/cmos.default:
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 21: else otherwise
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... File src/mainboard/asus/p8z77-m/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 14: # FIXME: check gfx.ndid and gfx.did : register "gfx.did" = "{ 0x80000100, 0x80000240, 0x80000410 }" : register "gfx.ndid" = "3" Just remove them. They cause ACPI errors.
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 17: 0x0 0
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 25: 0x0 0
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 29: 0x0 0
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 37: register "gen4_dec" = "0x00000000" It's zero, you can remove it
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 53: Audio Audio It's stereo audio!
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 64: LPC bridge PCI-LPC bridge Some obnoxious redundancy here
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 95: # Deep sleep This comment is not aligned
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... File src/mainboard/asus/p8z77-m/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 17: #define ACPI_VIDEO_DEVICE _SB.PCI0.GFX0 Not needed.
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... File src/mainboard/asus/p8z77-m/early_init.c:
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 88: void mainboard_fill_pei_data(struct pei_data *pei_data) I think you need to guard these
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... File src/mainboard/asus/p8z77-m/gma-mainboard.ads:
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 32: Internal At least "Internal" is not there. Replace it with:
Others => Disabled
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... File src/mainboard/asus/p8z77-m/mainboard.c:
https://review.coreboot.org/c/coreboot/+/38988/4/src/mainboard/asus/p8z77-m/... PS4, Line 10: fix those values Or remove the entire file.