[coreboot-gerrit] Change in ...coreboot[master]: mb/gigabyte/m57sli: Add mainboard

Fri Dec 14 22:20:12 CET 2018

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27618 )

Change subject: mb/gigabyte/m57sli: Add mainboard
......................................................................

Patch Set 3:

(71 comments)

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/acpi_tables.c 
File src/mainboard/gigabyte/m57sli/acpi_tables.c:

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/acpi_tables.c@55 
PS3, Line 55: 			current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *) current,
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/acpi_tables.c@65 
PS3, Line 65: 			current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *) current,
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/fanctl.c 
File src/mainboard/gigabyte/m57sli/fanctl.c:

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/fanctl.c@14 
PS3, Line 14: {
that open brace { should be on the previous line

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/get_bus_conf.c 
File src/mainboard/gigabyte/m57sli/get_bus_conf.c:

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/get_bus_conf.c@72 
PS3, Line 72: 	printk(BIOS_SPEW, "get_bus_conf()\n");
Prefer using '"%s...", __func__' to using 'get_bus_conf', this function's name, in a string

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/get_bus_conf.c@109 
PS3, Line 109: 			m->bus_mcp55[i] = pci_read_config8(dev, PCI_SECONDARY_BUS);
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/get_bus_conf.c@111 
PS3, Line 111: 		else {
else should follow close brace '}'

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c 
File src/mainboard/gigabyte/m57sli/mptable.c:

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@72 
PS3, Line 72: 			 m->bus_mcp55[0], ((sbdn+1)<<2)|1, m->apicid_mcp55, 0xa);
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@75 
PS3, Line 75: 			 m->bus_mcp55[0], ((sbdn+2)<<2)|0, m->apicid_mcp55, 0x16); // 22
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@78 
PS3, Line 78: 			 m->bus_mcp55[0], ((sbdn+2)<<2)|1, m->apicid_mcp55, 0x17); // 23
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@81 
PS3, Line 81: 			 m->bus_mcp55[0], ((sbdn+6)<<2)|1, m->apicid_mcp55, 0x17); // 23
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@84 
PS3, Line 84: 			 m->bus_mcp55[0], ((sbdn+5)<<2)|0, m->apicid_mcp55, 0x14); // 20
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@86 
PS3, Line 86: 			 m->bus_mcp55[0], ((sbdn+5)<<2)|1, m->apicid_mcp55, 0x17); // 23
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@88 
PS3, Line 88: 			 m->bus_mcp55[0], ((sbdn+5)<<2)|2, m->apicid_mcp55, 0x15); // 21
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@91 
PS3, Line 91: 			 m->bus_mcp55[0], ((sbdn+8)<<2)|0, m->apicid_mcp55, 0x16); // 22
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@93 
PS3, Line 93: 			 m->bus_mcp55[0], ((sbdn+9)<<2)|0, m->apicid_mcp55, 0x15); // 21
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@99 
PS3, Line 99: 			smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW,
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@101 
PS3, Line 101: 					 m->apicid_mcp55, 0x10 + (2 + j + i + 4 - sbdn % 4) %4);
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@101 
PS3, Line 101: 					 m->apicid_mcp55, 0x10 + (2 + j + i + 4 - sbdn % 4) %4);
need consistent spacing around '%' (ctx:WxV)

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@106 
PS3, Line 106: 			smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW,
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/mptable.c@108 
PS3, Line 108: 					 m->apicid_mcp55, 0x10 + (2 + i + j) % 4);
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c 
File src/mainboard/gigabyte/m57sli/resourcemap.c:

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@21 
PS3, Line 21: 		/* Careful set limit registers before base registers which contain the enables */
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@42 
PS3, Line 42: 		 *	   specifies the values of A[14:12] to use with interleave enable.
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@45 
PS3, Line 45: 		 *	   This field defines the upper address bits of a 40 bit  address
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@48 
PS3, Line 48: //		PCI_ADDR(0, 0x18, 1, 0x44), 0x0000f8f8, 0x00000000, Need for CAR with FAM10
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@81 
PS3, Line 81: 		 *	   111 = Interleve on A[12] and A[13] and A[14] (8 nodes)
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@84 
PS3, Line 84: 		 *	   This field defines the upper address bits of a 40-bit address
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@87 
PS3, Line 87: //		PCI_ADDR(0, 0x18, 1, 0x40), 0x0000f8fc, 0x00000000, need for CAR with FAM10
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@125 
PS3, Line 125: 		 *	   This field defines the upp adddress bits of a 40-bit address that
'adddress' may be misspelled - perhaps 'address'?

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@125 
PS3, Line 125: 		 *	   This field defines the upp adddress bits of a 40-bit address that
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@160 
PS3, Line 160: 		 *	   This field defines the upper address bits of a 40bit address
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@216 
PS3, Line 216: 		 *	   1 = matches all address < 64K and where A[9:0] is in the
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@217 
PS3, Line 217: 		 *	       range 3B0-3BB or 3C0-3DF independen of the base & limit registers
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@220 
PS3, Line 220: 		 *	   1 = Blocks address < 64K and in the last 768 bytes of eack 1K block
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@264 
PS3, Line 264: 		 *	   This field defines the lowest bus number in configuration region i
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@266 
PS3, Line 266: 		 *	   This field defines the highest bus number in configuration region i
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/resourcemap.c@268 
PS3, Line 268: //		PCI_ADDR(0, 0x18, 1, 0xE0), 0x0000FC88, 0xff000003, /* link 0 of CPU 0 --> Nvidia MCP55 */
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c 
File src/mainboard/gigabyte/m57sli/romstage.c:

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@71 
PS3, Line 71: #define MCP55_MB_SETUP \
Macros with complex values should be enclosed in parentheses

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@72 
PS3, Line 72:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 37, 0x00, 0x68,/* GPIO38 PCI_REQ3 */ \
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@72 
PS3, Line 72:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 37, 0x00, 0x68,/* GPIO38 PCI_REQ3 */ \
code indent should use tabs where possible

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@72 
PS3, Line 72:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 37, 0x00, 0x68,/* GPIO38 PCI_REQ3 */ \
please, no spaces at the start of a line

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@73 
PS3, Line 73:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 38, 0x00, 0x68,/* GPIO39 PCI_GNT3 */ \
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@73 
PS3, Line 73:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 38, 0x00, 0x68,/* GPIO39 PCI_GNT3 */ \
code indent should use tabs where possible

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@73 
PS3, Line 73:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 38, 0x00, 0x68,/* GPIO39 PCI_GNT3 */ \
please, no spaces at the start of a line

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@74 
PS3, Line 74:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 39, 0x00, 0x68,/* GPIO40 PCI_GNT2 */ \
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@74 
PS3, Line 74:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 39, 0x00, 0x68,/* GPIO40 PCI_GNT2 */ \
code indent should use tabs where possible

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@74 
PS3, Line 74:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 39, 0x00, 0x68,/* GPIO40 PCI_GNT2 */ \
please, no spaces at the start of a line

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@75 
PS3, Line 75:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 40, 0x00, 0x68,/* GPIO41 PCI_REQ2 */ \
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@75 
PS3, Line 75:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 40, 0x00, 0x68,/* GPIO41 PCI_REQ2 */ \
code indent should use tabs where possible

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@75 
PS3, Line 75:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 40, 0x00, 0x68,/* GPIO41 PCI_REQ2 */ \
please, no spaces at the start of a line

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@76 
PS3, Line 76:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 59, 0x00, 0x60,/* GPIP60 FANCTL0 */ \
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@76 
PS3, Line 76:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 59, 0x00, 0x60,/* GPIP60 FANCTL0 */ \
code indent should use tabs where possible

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@76 
PS3, Line 76:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 59, 0x00, 0x60,/* GPIP60 FANCTL0 */ \
please, no spaces at the start of a line

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@77 
PS3, Line 77:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 60, 0x00, 0x60,/* GPIO61 FANCTL1 */
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@77 
PS3, Line 77:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 60, 0x00, 0x60,/* GPIO61 FANCTL1 */
code indent should use tabs where possible

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@77 
PS3, Line 77:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0 + 60, 0x00, 0x60,/* GPIO61 FANCTL1 */
please, no spaces at the start of a line

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@87 
PS3, Line 87: 	byte = pci_read_config8(PCI_DEV(0, MCP55_DEVN_BASE+1 , 0), 0x7b);
space prohibited before that ',' (ctx:WxW)

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@89 
PS3, Line 89: 	pci_write_config8(PCI_DEV(0, MCP55_DEVN_BASE+1 , 0), 0x7b, byte);
space prohibited before that ',' (ctx:WxW)

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@91 
PS3, Line 91: 	dword = pci_read_config32(PCI_DEV(0, MCP55_DEVN_BASE+1 , 0), 0xa0);
space prohibited before that ',' (ctx:WxW)

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@93 
PS3, Line 93: 	pci_write_config32(PCI_DEV(0, MCP55_DEVN_BASE+1 , 0), 0xa0, dword);
space prohibited before that ',' (ctx:WxW)

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@95 
PS3, Line 95: 	dword = pci_read_config32(PCI_DEV(0, MCP55_DEVN_BASE+1 , 0), 0xa4);
space prohibited before that ',' (ctx:WxW)

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@97 
PS3, Line 97: 	pci_write_config32(PCI_DEV(0, MCP55_DEVN_BASE+1 , 0), 0xa4, dword);
space prohibited before that ',' (ctx:WxW)

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@104 
PS3, Line 104: 	static const u8 spd_addr[] = {RC00, DIMM0, DIMM2, 0, 0, DIMM1, DIMM3, 0, 0, };
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@136 
PS3, Line 136: 	printk(BIOS_DEBUG, "*sysinfo range: [%p,%p]\n",sysinfo,sysinfo+1);
space required after that ',' (ctx:VxV)

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@136 
PS3, Line 136: 	printk(BIOS_DEBUG, "*sysinfo range: [%p,%p]\n",sysinfo,sysinfo+1);
space required after that ',' (ctx:VxV)

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@225 
PS3, Line 225:  *	BUID assignment may be controlled explicitly on a non-coherent chain. Provide a
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@226 
PS3, Line 226:  *	swap list. The first part of the list controls the BUID assignment and the
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@227 
PS3, Line 227:  *	second part of the list provides the device to device linking.  Device orientation
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@228 
PS3, Line 228:  *	can be detected automatically, or explicitly.  See documentation for more details.
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@230 
PS3, Line 230:  *	Automatic non-coherent init assigns BUIDs starting at 1 and incrementing sequentially
line over 80 characters

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@238 
PS3, Line 238: BOOL AMD_CB_ManualBUIDSwapList (u8 node, u8 link, const u8 **List)
space prohibited between function name and open parenthesis '('

https://review.coreboot.org/#/c/27618/3/src/mainboard/gigabyte/m57sli/romstage.c@241 
PS3, Line 241: 	/* If the BUID was adjusted in early_ht we need to do the manual override */
line over 80 characters

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/27618
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I20a437f6952d9f919ad186d4862ca00853d9ebca
Gerrit-Change-Number: 27618
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Fri, 14 Dec 2018 21:20:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20181214/818f8c21/attachment.html>