Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31363 )
Change subject: mb/gigabyte: add GA-P67A-UD3R ......................................................................
Patch Set 3:
(14 comments)
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... File src/mainboard/gigabyte/ga-p67a-ud3r/acpi/mainboard.asl:
PS3: Is this file needed?
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... File src/mainboard/gigabyte/ga-p67a-ud3r/acpi_tables.c:
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... PS3, Line 21: : /* Disable USB ports in S3 by default */ : gnvs->s3u0 = 0; : gnvs->s3u1 = 0; : : /* Disable USB ports in S5 by default */ : gnvs->s5u0 = 0; : gnvs->s5u1 = 0; Please remove, their value is already zero.
https://review.coreboot.org/c/coreboot/+/31363/2/src/mainboard/gigabyte/ga-p... File src/mainboard/gigabyte/ga-p67a-ud3r/cmos.layout:
https://review.coreboot.org/c/coreboot/+/31363/2/src/mainboard/gigabyte/ga-p... PS2, Line 62: # coreboot config options: northbridge : #432 3 r 0 unused
This can be removed
Ack
https://review.coreboot.org/c/coreboot/+/31363/2/src/mainboard/gigabyte/ga-p... PS2, Line 65: # SandyBridge MRC Scrambler Seed values : 896 32 r 0 mrc_scrambler_seed : 928 32 r 0 mrc_scrambler_seed_s3 : 960 16 r 0 mrc_scrambler_seed_chk
These shouldn't be needed for native raminit.
Ack
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... File src/mainboard/gigabyte/ga-p67a-ud3r/cmos.layout:
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... PS3, Line 81: 2 0 Enable : 2 1 Disable This isn't used
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... File src/mainboard/gigabyte/ga-p67a-ud3r/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... PS3, Line 17: 0x0 just 0
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... PS3, Line 19: 0x0 just 0
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... PS3, Line 34: 0x0 just 0
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... PS3, Line 50: register "pcie_port_coalesce" = "0" This is zero by default already, so it can be removed.
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... PS3, Line 56: off Shouldn't this be on?
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... PS3, Line 84: (PCIEX1_1) huh? How are the PCIe ports distributed?
https://review.coreboot.org/c/coreboot/+/31363/3/src/mainboard/gigabyte/ga-p... PS3, Line 99: rounted routed
https://review.coreboot.org/c/coreboot/+/31363/1/src/mainboard/gigabyte/ga-p... File src/mainboard/gigabyte/ga-p67a-ud3r/romstage.c:
https://review.coreboot.org/c/coreboot/+/31363/1/src/mainboard/gigabyte/ga-p... PS1, Line 65: /* Disable SIO WDT which kicks in DualBIOS */
I'll add you to the copyright.
Ack
https://review.coreboot.org/c/coreboot/+/31363/2/src/mainboard/gigabyte/ga-p... File src/mainboard/gigabyte/ga-p67a-ud3r/romstage.c:
https://review.coreboot.org/c/coreboot/+/31363/2/src/mainboard/gigabyte/ga-p... PS2, Line 33: CNF2_LPC_EN
This is for decoding I/O locations 4Eh and 4Fh to the LPC interface. […]
Ack