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

[Fabian Groffen (Code Review)]

Attention is currently required from: Angel Pons, Felix Singer, Patrick Rudolph.

Fabian Groffen has posted comments on this change by Fabian Groffen. ( 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 8:

(14 comments)

Patchset:

PS8: 
rebased, addressed comments

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

PS7:
> Missing license header

Done

https://review.coreboot.org/c/coreboot/+/77046/comment/5698282c_812352e3?usp... :
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 […]

Done

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

PS7:
> Missing license header

Done

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

PS7:
> Missing license header

Done

https://review.coreboot.org/c/coreboot/+/77046/comment/297d8356_7092cc46?usp... :
PS7, Line 5: 			subsystemid 0x1458 0x5000
> See this very file, two lines above this one.

Done

https://review.coreboot.org/c/coreboot/+/77046/comment/9afc9200_122a3274?usp... :
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 shou […]

Done

https://review.coreboot.org/c/coreboot/+/77046/comment/cdffa7cb_1fdf11bd?usp... :
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. […]

Done

https://review.coreboot.org/c/coreboot/+/77046/comment/93cb29c4_eb601f4e?usp... :
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.

Done

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

Done

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

https://review.coreboot.org/c/coreboot/+/77046/comment/acd7c99c_c3af5d05?usp... :
PS7, Line 10: // OEM revision
> Still, it's most likely autoport copy-paste. I'd drop the comment anyway.

Done

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

https://review.coreboot.org/c/coreboot/+/77046/comment/162b1eae_2c30fad5?usp... :
PS7, Line 1: GPL-2.0-only
> GPL-2. […]

Done

File src/mainboard/gigabyte/ga-h77m-d3h/gpio.c:

PS7:
> Was this file copied as-is from the other board? I hope not, because bad GPIO configuration can brea […]

Done

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

https://review.coreboot.org/c/coreboot/+/77046/comment/3379d9a6_d5fb1713?usp... :
PS7, Line 9: 	/* Temperature which OS will throttle CPU */
> We usually don't indent defines. Please remove.

Done