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

build bot (Jenkins) (Code Review) gerrit at coreboot.org
Tue Jul 24 10:52:24 CEST 2018


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

Change subject: [NOTFORMERGE]mb/gigabyte/m57sli: Add mainboard
......................................................................


Patch Set 1:

(134 comments)

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

https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/acpi_tables.c@36
PS1, Line 36: 	extern unsigned apicid_mcp55;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/acpi_tables.c@38
PS1, Line 38: 	unsigned sbdn;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/acpi_tables.c@49
PS1, Line 49: 	dev = dev_find_slot(bus_mcp55[0], PCI_DEVFN(sbdn+ 0x1,0));
need consistent spacing around '+' (ctx:VxW)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/acpi_tables.c@49
PS1, Line 49: 	dev = dev_find_slot(bus_mcp55[0], PCI_DEVFN(sbdn+ 0x1,0));
space required after that ',' (ctx:VxV)


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


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/acpi_tables.c@59
PS1, Line 59: 	dev = dev_find_slot(bus_mcp55[0], PCI_DEVFN(sbdn+ 0x12,1));
need consistent spacing around '+' (ctx:VxW)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/acpi_tables.c@59
PS1, Line 59: 	dev = dev_find_slot(bus_mcp55[0], PCI_DEVFN(sbdn+ 0x12,1));
space required after that ',' (ctx:VxV)


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


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

https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/fanctl.c@13
PS1, Line 13: } sequence[]= {
spaces required around that '=' (ctx:VxW)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/fanctl.c@78
PS1, Line 78: 	for (i = 0; i < ARRAY_SIZE(sequence); i++) {
braces {} are not necessary for single statement blocks


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

https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@29
PS1, Line 29: // Global variables for MB layouts and these will be shared by irqtable mptable and acpi_tables
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@32
PS1, Line 32: unsigned apicid_mcp55;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@34
PS1, Line 34: unsigned pci1234x[] = {		//Here you only need to set value in pci1234 for HT-IO that could be installed or not
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@34
PS1, Line 34: unsigned pci1234x[] = {		//Here you only need to set value in pci1234 for HT-IO that could be installed or not
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@35
PS1, Line 35: 	//You may need to preset pci1234 for HTIO board, please refer to src/northbridge/amd/amdk8/get_sblk_pci1234.c for detail
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@46
PS1, Line 46: unsigned hcdnx[] = {		//HT Chain device num, actually it is unit id base of every ht device in chain, assume every chain only have 4 ht device at most
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@46
PS1, Line 46: unsigned hcdnx[] = {		//HT Chain device num, actually it is unit id base of every ht device in chain, assume every chain only have 4 ht device at most
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@59
PS1, Line 59: static unsigned get_bus_conf_done = 0;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@64
PS1, Line 64: 	unsigned apicid_base;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@65
PS1, Line 65: 	unsigned sbdn;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@83
PS1, Line 83: 	sysconf.sbdn = (sysconf.hcdn[0] & 0xff);	// first byte of first chain
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@86
PS1, Line 86: 	for (i = 0; i < 8; i++) {
braces {} are not necessary for single statement blocks


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/get_bus_conf.c@111
PS1, Line 111: 		if (dev) {
braces {} are not necessary for single statement blocks


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/irq_tables.c
File src/mainboard/gigabyte/m57sli/irq_tables.c:

https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/irq_tables.c@53
PS1, Line 53: 	unsigned slot_num;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/irq_tables.c@55
PS1, Line 55: 	unsigned sbdn;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/irq_tables.c@60
PS1, Line 60: 	get_bus_conf();		// it will find out all bus num and apic that share with mptable.c and mptable.c and acpi_tables.c
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/irq_tables.c@103
PS1, Line 103: 	if (sum != pirq->checksum) {
braces {} are not necessary for single statement blocks


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

https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@28
PS1, Line 28: extern unsigned apicid_mcp55;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@33
PS1, Line 33: 	unsigned sbdn;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@52
PS1, Line 52: 		dev = dev_find_slot(bus_mcp55[0], PCI_DEVFN(sbdn+ 0x1,0));
need consistent spacing around '+' (ctx:VxW)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@52
PS1, Line 52: 		dev = dev_find_slot(bus_mcp55[0], PCI_DEVFN(sbdn+ 0x1,0));
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@75
PS1, Line 75: 	PCI_INT(0,sbdn+1,1, 10); /* SMBus */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@75
PS1, Line 75: 	PCI_INT(0,sbdn+1,1, 10); /* SMBus */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@76
PS1, Line 76: 	PCI_INT(0,sbdn+2,0, 22); /* USB */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@76
PS1, Line 76: 	PCI_INT(0,sbdn+2,0, 22); /* USB */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@77
PS1, Line 77: 	PCI_INT(0,sbdn+2,1, 23); /* USB */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@77
PS1, Line 77: 	PCI_INT(0,sbdn+2,1, 23); /* USB */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@78
PS1, Line 78: 	PCI_INT(0,sbdn+4,0, 21); /* IDE */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@78
PS1, Line 78: 	PCI_INT(0,sbdn+4,0, 21); /* IDE */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@79
PS1, Line 79: 	PCI_INT(0,sbdn+5,0, 20); /* SATA */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@79
PS1, Line 79: 	PCI_INT(0,sbdn+5,0, 20); /* SATA */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@80
PS1, Line 80: 	PCI_INT(0,sbdn+5,1, 21); /* SATA */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@80
PS1, Line 80: 	PCI_INT(0,sbdn+5,1, 21); /* SATA */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@81
PS1, Line 81: 	PCI_INT(0,sbdn+5,2, 22); /* SATA */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@81
PS1, Line 81: 	PCI_INT(0,sbdn+5,2, 22); /* SATA */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@82
PS1, Line 82: 	PCI_INT(0,sbdn+6,1, 23); /* HD Audio */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@82
PS1, Line 82: 	PCI_INT(0,sbdn+6,1, 23); /* HD Audio */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@83
PS1, Line 83: 	PCI_INT(0,sbdn+8,0, 20); /* GBit Ethernet */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@83
PS1, Line 83: 	PCI_INT(0,sbdn+8,0, 20); /* GBit Ethernet */
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@87
PS1, Line 87: 	for(i = 0; i < 4; i++){
space required before the open brace '{'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@87
PS1, Line 87: 	for(i = 0; i < 4; i++){
space required before the open parenthesis '('


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@88
PS1, Line 88: 		for(j = 7; j > 1; j--){
space required before the open brace '{'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@88
PS1, Line 88: 		for(j = 7; j > 1; j--){
space required before the open parenthesis '('


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@89
PS1, Line 89: 			if(k > 3) k = 0;
space required before the open parenthesis '('


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@89
PS1, Line 89: 			if(k > 3) k = 0;
trailing statements should be on next line


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@90
PS1, Line 90: 			PCI_INT(j,0,i, 16+k);
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@90
PS1, Line 90: 			PCI_INT(j,0,i, 16+k);
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@101
PS1, Line 101: 	for(i = 0; i < 4; i++){
space required before the open brace '{'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@101
PS1, Line 101: 	for(i = 0; i < 4; i++){
space required before the open parenthesis '('


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@102
PS1, Line 102: 		for(j = 6; j < 11; j++){
space required before the open brace '{'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@102
PS1, Line 102: 		for(j = 6; j < 11; j++){
space required before the open parenthesis '('


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@103
PS1, Line 103: 			if(k > 3) k = 0;
space required before the open parenthesis '('


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@103
PS1, Line 103: 			if(k > 3) k = 0;
trailing statements should be on next line


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@104
PS1, Line 104: 			PCI_INT(1,j,i, 16+k);
space required after that ',' (ctx:VxV)


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/mptable.c@104
PS1, Line 104: 			PCI_INT(1,j,i, 16+k);
space required after that ',' (ctx:VxV)


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

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


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


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


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/resourcemap.c@48
PS1, 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/1/src/mainboard/gigabyte/m57sli/resourcemap.c@81
PS1, 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/1/src/mainboard/gigabyte/m57sli/resourcemap.c@84
PS1, Line 84: 		 *	   This field defines the upper address bits of a 40-bit address
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/resourcemap.c@87
PS1, 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/1/src/mainboard/gigabyte/m57sli/resourcemap.c@125
PS1, 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/1/src/mainboard/gigabyte/m57sli/resourcemap.c@125
PS1, 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/1/src/mainboard/gigabyte/m57sli/resourcemap.c@160
PS1, Line 160: 		 *	   This field defines the upper address bits of a 40bit address
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/resourcemap.c@216
PS1, 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/1/src/mainboard/gigabyte/m57sli/resourcemap.c@217
PS1, Line 217: 		 *	       range 3B0-3BB or 3C0-3DF independen of the base & limit registers
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/resourcemap.c@220
PS1, 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/1/src/mainboard/gigabyte/m57sli/resourcemap.c@264
PS1, Line 264: 		 *	   This field defines the lowest bus number in configuration region i
line over 80 characters


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


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/resourcemap.c@268
PS1, 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/1/src/mainboard/gigabyte/m57sli/romstage.c
File src/mainboard/gigabyte/m57sli/romstage.c:

https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@50
PS1, Line 50: unsigned get_sbdn(unsigned bus)
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@50
PS1, Line 50: unsigned get_sbdn(unsigned bus)
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@64
PS1, Line 64: int spd_read_byte(unsigned device, unsigned address);
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@64
PS1, Line 64: int spd_read_byte(unsigned device, unsigned address);
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@65
PS1, Line 65: int spd_read_byte(unsigned device, unsigned address)
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@65
PS1, Line 65: int spd_read_byte(unsigned device, unsigned address)
Prefer 'unsigned int' to bare use of 'unsigned'


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


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


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@71
PS1, Line 71:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@71
PS1, Line 71:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@72
PS1, Line 72:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0+38, 0x00, 0x68,/* GPIO39 PCI_GNT3 */ \
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@72
PS1, Line 72:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@72
PS1, Line 72:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@73
PS1, Line 73:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0+39, 0x00, 0x68,/* GPIO40 PCI_GNT2 */ \
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@73
PS1, Line 73:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@73
PS1, Line 73:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@74
PS1, Line 74:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0+40, 0x00, 0x68,/* GPIO41 PCI_REQ2 */ \
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@74
PS1, Line 74:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@74
PS1, Line 74:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@75
PS1, Line 75:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0+59, 0x00, 0x60,/* GPIP60 FANCTL0 */ \
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@75
PS1, Line 75:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@75
PS1, Line 75:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@76
PS1, Line 76:         RES_PORT_IO_8, SYSCTRL_IO_BASE + 0xc0+60, 0x00, 0x60,/* GPIO61 FANCTL1 */
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@76
PS1, Line 76:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@76
PS1, Line 76:         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/1/src/mainboard/gigabyte/m57sli/romstage.c@86
PS1, Line 86: 	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/1/src/mainboard/gigabyte/m57sli/romstage.c@88
PS1, Line 88: 	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/1/src/mainboard/gigabyte/m57sli/romstage.c@90
PS1, Line 90: 	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/1/src/mainboard/gigabyte/m57sli/romstage.c@92
PS1, Line 92: 	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/1/src/mainboard/gigabyte/m57sli/romstage.c@94
PS1, Line 94: 	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/1/src/mainboard/gigabyte/m57sli/romstage.c@96
PS1, Line 96: 	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/1/src/mainboard/gigabyte/m57sli/romstage.c@103
PS1, Line 103: 	static const u8 spd_addr[] = {RC00, DIMM0, DIMM2, 0, 0, DIMM1, DIMM3, 0, 0, };
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@107
PS1, Line 107: 	unsigned bsp_apicid = 0;
Prefer 'unsigned int' to bare use of 'unsigned'


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@133
PS1, Line 133: 		pnp_write_config(SERIAL_DEV, IT8716F_CONFIG_REG_SWSUSP, tmp | 0x10);
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@139
PS1, Line 139:  	it8716f_enable_dev(SERIAL_DEV, CONFIG_TTYS0_BASE);
code indent should use tabs where possible


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@139
PS1, Line 139:  	it8716f_enable_dev(SERIAL_DEV, CONFIG_TTYS0_BASE);
please, no space before tabs


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@139
PS1, Line 139:  	it8716f_enable_dev(SERIAL_DEV, CONFIG_TTYS0_BASE);
please, no spaces at the start of a line


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


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


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@156
PS1, Line 156: 	set_sysinfo_in_ram(0); // in BSP so could hold all ap until sysinfo is in ram
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@172
PS1, Line 172: 		/* Core0 on each node is configured. Now setup any additional cores. */
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@181
PS1, Line 181: 		printk(BIOS_DEBUG, "\nBegin FIDVID MSR 0xc0010071 0x%08x 0x%08x\n", msr.hi, msr.lo);
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@197
PS1, Line 197: 		printk(BIOS_DEBUG, "End FIDVIDMSR 0xc0010071 0x%08x 0x%08x\n", msr.hi, msr.lo);
line over 80 characters


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


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


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


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


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


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@249
PS1, Line 249: 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/1/src/mainboard/gigabyte/m57sli/romstage.c@251
PS1, Line 251: 	static const u8 swaplist[] = { 0xFF, CONFIG_HT_CHAIN_UNITID_BASE, CONFIG_HT_CHAIN_END_UNITID_BASE, 0xFF };
line over 80 characters


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


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@253
PS1, Line 253: 	if ((CONFIG_HT_CHAIN_UNITID_BASE != 0) && (CONFIG_HT_CHAIN_END_UNITID_BASE != 0)) {
line over 80 characters


https://review.coreboot.org/#/c/27618/1/src/mainboard/gigabyte/m57sli/romstage.c@254
PS1, Line 254: 		printk(BIOS_DEBUG, "AMD_CB_ManualBUIDSwapList()\n");
Prefer using '"%s...", __func__' to using 'AMD_CB_ManualBUIDSwapList', this function's name, in a string



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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20a437f6952d9f919ad186d4862ca00853d9ebca
Gerrit-Change-Number: 27618
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-CC: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Tue, 24 Jul 2018 08:52:24 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180724/b9cec95e/attachment.html>


More information about the coreboot-gerrit mailing list