[coreboot-gerrit] [L] Change in coreboot[main]: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H

[Angel Pons (Code Review)]

Attention is currently required from: Fabian Groffen, Felix Singer, Patrick Rudolph.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77046?usp=email )

Change subject: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H
......................................................................

Patch Set 7: Code-Review+1

(12 comments)

File src/mainboard/gigabyte/ga-h77m-d3h/Kconfig:

https://review.coreboot.org/c/coreboot/+/77046/comment/ae20006b_7adec9ed :
PS7, Line 21: 	default 25
As per schematics, this board doesn't seem to use a DRAM reset gate GPIO at all (the DRAMRST# signal is wired directly between the CPU and the DIMM slots).

But turns out that GPIO25 is mobile-only, so nothing should happen if coreboot tries to change its state. Which is good, because toggling a random GPIO that is wired elsewhere would be worse.

TL;DR would be nice to add a comment:
> Use GPIO25 because it does not exist on desktop PCHs and
> because this board does not use a DRAM reset gate GPIO.

File src/mainboard/gigabyte/ga-h77m-d3h/acpi/pci.asl:

https://review.coreboot.org/c/coreboot/+/77046/comment/8f849b45_1581f0be :
PS7, Line 7: 	Name (_ADR, 0x001E0000)
This can't be correct because the board uses a dedicated PCIe-to-PCI bridge chip instead of the southbridge's conventional PCI port (which IIRC is only available on "corporate" PCH SKUs, e.g. Q-series)

File src/mainboard/gigabyte/ga-h77m-d3h/acpi/thermal.asl:

PS7: 
Has this been tested?

File src/mainboard/gigabyte/ga-h77m-d3h/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/77046/comment/de1ce0fc_ed2e5f3e :
PS7, Line 5: 			subsystemid 0x1458 0x5000
Same ID already gets inherited, you can omit this line

https://review.coreboot.org/c/coreboot/+/77046/comment/393efae2_e1fe2439 :
PS7, Line 26: 			device ref mei2 off end	# Management Engine Interface 2
I think the chipset devicetree already provides default on/off states for named devices, so you should omit entries for disabled devices (as long as they're disabled in the chipset devicetree).

https://review.coreboot.org/c/coreboot/+/77046/comment/48137fd5_b05f9a13 :
PS7, Line 36: 		device ref pcie_rp1 on  end	# PCIe Port #1
            : 			device ref pcie_rp2 off end	# PCIe Port #2
            : 			device ref pcie_rp3 off end	# PCIe Port #3
            : 			device ref pcie_rp4 off end	# PCIe Port #4
            : 			device ref pcie_rp5 on	# PCIe Port #5
            : 				device pci 00.0 on	# AR8161 GbE
            : 				end
            : 			end
            : 			device ref pcie_rp6 off end	# PCIe Port #6
            : 			device ref pcie_rp7 off end	# PCIe Port #7
            : 			device ref pcie_rp8 off end	# PCIe Port #8
Based on what I can see in board pictures [1], this is not accurate. There should be at least *four* root ports enabled:

- second PCIe x16 slot (electrical)
- onboard Ethernet (AR8161 GbE)
- PCIe x1 slot
- ITE PCIe-to-PCI bridge (likely same chip as in `mb/gigabyte/ga-h61m-series`)

According to the schematics for the GA-Z77M-D3H (which is likely identical, save for minor differences between board revisions), looks like the correct mapping should be:

```
    device ref pcie_rp1 on  end	# RP #1: PCIEX4 slot
    device ref pcie_rp2 off end	# RP #2
    device ref pcie_rp3 off end	# RP #3
    device ref pcie_rp4 off end	# RP #4
    device ref pcie_rp5 on		# RP #5: AR8161 GbE
    	device pci 00.0 on end
    end
    device ref pcie_rp6 on  end	# RP #6: PCIEX1 slot
    device ref pcie_rp7 on  end	# RP #7: IT8892E PCIe-to-PCI
    device ref pcie_rp8 off end	# RP #8
```

[1]: https://imgur.com/a/sq6MNFd

https://review.coreboot.org/c/coreboot/+/77046/comment/5115f435_a7d935fa :
PS7, Line 50: device ref pci_bridge on end # PCI bridge
The PCI bridge in the southbridge doesn't seem to be used. You should disable it.

https://review.coreboot.org/c/coreboot/+/77046/comment/d32e0017_90600dce :
PS7, Line 107: 			device pci 1f.4 off end
I don't think this device is supposed to exist

File src/mainboard/gigabyte/ga-h77m-d3h/dsdt.asl:

https://review.coreboot.org/c/coreboot/+/77046/comment/3b137883_a3f40dfa :
PS7, Line 10: // OEM revision
I think autoport hardcodes this value, as well as the comment. I'd drop the comment, since it's not true.

File src/mainboard/gigabyte/ga-h77m-d3h/gma-mainboard.ads:

https://review.coreboot.org/c/coreboot/+/77046/comment/3a833b81_50cd860b :
PS7, Line 12: HDMI1
DVI-D connector

https://review.coreboot.org/c/coreboot/+/77046/comment/c50effd0_578b0910 :
PS7, Line 13: HDMI3
HDMI connector

https://review.coreboot.org/c/coreboot/+/77046/comment/1fbaaf74_b3782df0 :
PS7, Line 14: Analog
VGA connector