[coreboot-gerrit] Change in ...coreboot[master]: mb/gigabyte/m57sli: Add mainboard
build bot (Jenkins) (Code Review)
gerrit at coreboot.org
Fri Dec 14 22:00:14 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 2:
(106 comments)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/acpi_tables.c
File src/mainboard/gigabyte/m57sli/acpi_tables.c:
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/acpi_tables.c@39
PS2, Line 39: unsigned sbdn;
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/acpi_tables.c@51
PS2, Line 51: dev = dev_find_slot(m->bus_mcp55[0], PCI_DEVFN(sbdn+ 0x1,0));
need consistent spacing around '+' (ctx:VxW)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/acpi_tables.c@51
PS2, Line 51: dev = dev_find_slot(m->bus_mcp55[0], PCI_DEVFN(sbdn+ 0x1,0));
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/acpi_tables.c@55
PS2, Line 55: current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *) current,
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/acpi_tables.c@61
PS2, Line 61: dev = dev_find_slot(m->bus_mcp55[0], PCI_DEVFN(sbdn+ 0x12,1));
need consistent spacing around '+' (ctx:VxW)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/acpi_tables.c@61
PS2, Line 61: dev = dev_find_slot(m->bus_mcp55[0], PCI_DEVFN(sbdn+ 0x12,1));
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/acpi_tables.c@65
PS2, Line 65: current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *) current,
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/fanctl.c
File src/mainboard/gigabyte/m57sli/fanctl.c:
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/fanctl.c@13
PS2, Line 13: } sequence[]= {
spaces required around that '=' (ctx:VxW)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/fanctl.c@78
PS2, Line 78: for (i = 0; i < ARRAY_SIZE(sequence); i++) {
braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c
File src/mainboard/gigabyte/m57sli/get_bus_conf.c:
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c@30
PS2, Line 30: // 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/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c@49
PS2, Line 49: static unsigned hcdnx[] = {
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c@61
PS2, Line 61: static unsigned get_bus_conf_done = 0;
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c@65
PS2, Line 65: unsigned apicid_base;
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c@71
PS2, Line 71: 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/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c@95
PS2, Line 95: dev = dev_find_slot(m->bus_mcp55[0], PCI_DEVFN(sysconf.sbdn + 0x06,0));
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c@108
PS2, Line 108: m->bus_mcp55[i] = pci_read_config8(dev, PCI_SECONDARY_BUS);
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c@110
PS2, Line 110: else {
else should follow close brace '}'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c@120
PS2, Line 120: printk(BIOS_SPEW, "CONFIG_LOGICAL_CPUS == 1: apicid_base: %08x\n", apicid_base);
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/get_bus_conf.c@123
PS2, Line 123: printk(BIOS_SPEW, "CONFIG_LOGICAL_CPUS == 0: apicid_base: %08x\n", apicid_base);
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/irq_tables.c
File src/mainboard/gigabyte/m57sli/irq_tables.c:
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/irq_tables.c@52
PS2, Line 52: unsigned slot_num;
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/irq_tables.c@55
PS2, Line 55: unsigned sbdn;
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/irq_tables.c@60
PS2, 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/2/src/mainboard/gigabyte/m57sli/irq_tables.c@100
PS2, Line 100: unsigned busn = (sysconf.pci1234[i] >> 12) & 0xff;
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/irq_tables.c@101
PS2, Line 101: unsigned devn = sysconf.hcdn[i] & 0xff;
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/irq_tables.c@130
PS2, Line 130: if (sum != pirq->checksum) {
braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mb_sysconf.h
File src/mainboard/gigabyte/m57sli/mb_sysconf.h:
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mb_sysconf.h@23
PS2, Line 23: unsigned apicid_mcp55;
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c
File src/mainboard/gigabyte/m57sli/mptable.c:
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@29
PS2, Line 29: unsigned sbdn;
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@51
PS2, Line 51: dev = dev_find_slot(m->bus_mcp55[0], PCI_DEVFN(sbdn+ 0x1,0));
need consistent spacing around '+' (ctx:VxW)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@51
PS2, Line 51: dev = dev_find_slot(m->bus_mcp55[0], PCI_DEVFN(sbdn+ 0x1,0));
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@75
PS2, Line 75: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[0], ((sbdn+1)<<2)|1, m->apicid_mcp55, 0xa);
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@77
PS2, Line 77: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[0], ((sbdn+2)<<2)|0, m->apicid_mcp55, 0x16); // 22
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@79
PS2, Line 79: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[0], ((sbdn+2)<<2)|1, m->apicid_mcp55, 0x17); // 23
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@81
PS2, Line 81: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[0], ((sbdn+6)<<2)|1, m->apicid_mcp55, 0x17); // 23
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@83
PS2, Line 83: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[0], ((sbdn+5)<<2)|0, m->apicid_mcp55, 0x14); // 20
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@84
PS2, Line 84: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[0], ((sbdn+5)<<2)|1, m->apicid_mcp55, 0x17); // 23
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@85
PS2, Line 85: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[0], ((sbdn+5)<<2)|2, m->apicid_mcp55, 0x15); // 21
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@87
PS2, Line 87: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[0], ((sbdn+8)<<2)|0, m->apicid_mcp55, 0x16); // 22
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@88
PS2, Line 88: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[0], ((sbdn+9)<<2)|0, m->apicid_mcp55, 0x15); // 21
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@94
PS2, Line 94: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[j], (0x00 << 2)|i, m->apicid_mcp55, 0x10 + (2+j+i+4-sbdn%4)%4);
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/mptable.c@99
PS2, Line 99: smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, m->bus_mcp55[1], ((0x04+j)<<2)|i, m->apicid_mcp55, 0x10 + (2+i+j)%4);
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/resourcemap.c
File src/mainboard/gigabyte/m57sli/resourcemap.c:
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/resourcemap.c@21
PS2, Line 21: /* Careful set limit registers before base registers which contain the enables */
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/resourcemap.c@42
PS2, Line 42: * specifies the values of A[14:12] to use with interleave enable.
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/resourcemap.c@45
PS2, Line 45: * This field defines the upper address bits of a 40 bit address
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/resourcemap.c@48
PS2, 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/2/src/mainboard/gigabyte/m57sli/resourcemap.c@81
PS2, 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/2/src/mainboard/gigabyte/m57sli/resourcemap.c@84
PS2, Line 84: * This field defines the upper address bits of a 40-bit address
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/resourcemap.c@87
PS2, 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/2/src/mainboard/gigabyte/m57sli/resourcemap.c@125
PS2, 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/2/src/mainboard/gigabyte/m57sli/resourcemap.c@125
PS2, 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/2/src/mainboard/gigabyte/m57sli/resourcemap.c@160
PS2, Line 160: * This field defines the upper address bits of a 40bit address
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/resourcemap.c@216
PS2, 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/2/src/mainboard/gigabyte/m57sli/resourcemap.c@217
PS2, Line 217: * range 3B0-3BB or 3C0-3DF independen of the base & limit registers
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/resourcemap.c@220
PS2, 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/2/src/mainboard/gigabyte/m57sli/resourcemap.c@264
PS2, Line 264: * This field defines the lowest bus number in configuration region i
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/resourcemap.c@266
PS2, Line 266: * This field defines the highest bus number in configuration region i
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/resourcemap.c@268
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c
File src/mainboard/gigabyte/m57sli/romstage.c:
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@51
PS2, Line 51: unsigned get_sbdn(unsigned bus)
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@51
PS2, Line 51: unsigned get_sbdn(unsigned bus)
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@65
PS2, Line 65: int spd_read_byte(unsigned device, unsigned address);
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@65
PS2, Line 65: int spd_read_byte(unsigned device, unsigned address);
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@66
PS2, Line 66: int spd_read_byte(unsigned device, unsigned address)
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@66
PS2, Line 66: int spd_read_byte(unsigned device, unsigned address)
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@71
PS2, Line 71: #define MCP55_MB_SETUP \
Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@72
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@72
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@72
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@73
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@73
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@73
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@74
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@74
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@74
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@75
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@75
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@75
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@76
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@76
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@76
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@77
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@77
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@77
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@87
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@89
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@91
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@93
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@95
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@97
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@104
PS2, 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/2/src/mainboard/gigabyte/m57sli/romstage.c@108
PS2, Line 108: unsigned bsp_apicid = 0;
Prefer 'unsigned int' to bare use of 'unsigned'
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@134
PS2, Line 134: pnp_write_config(SERIAL_DEV, IT8716F_CONFIG_REG_SWSUSP, tmp | 0x10);
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@140
PS2, Line 140: it8716f_enable_dev(SERIAL_DEV, CONFIG_TTYS0_BASE);
code indent should use tabs where possible
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@140
PS2, Line 140: it8716f_enable_dev(SERIAL_DEV, CONFIG_TTYS0_BASE);
please, no space before tabs
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@140
PS2, Line 140: it8716f_enable_dev(SERIAL_DEV, CONFIG_TTYS0_BASE);
please, no spaces at the start of a line
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@153
PS2, Line 153: printk(BIOS_DEBUG, "*sysinfo range: [%p,%p]\n",sysinfo,sysinfo+1);
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@153
PS2, Line 153: printk(BIOS_DEBUG, "*sysinfo range: [%p,%p]\n",sysinfo,sysinfo+1);
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@157
PS2, Line 157: 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/2/src/mainboard/gigabyte/m57sli/romstage.c@173
PS2, Line 173: /* Core0 on each node is configured. Now setup any additional cores. */
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@182
PS2, Line 182: 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/2/src/mainboard/gigabyte/m57sli/romstage.c@198
PS2, Line 198: 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/2/src/mainboard/gigabyte/m57sli/romstage.c@237
PS2, Line 237: * BUID assignment may be controlled explicitly on a non-coherent chain. Provide a
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@238
PS2, Line 238: * swap list. The first part of the list controls the BUID assignment and the
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@239
PS2, Line 239: * second part of the list provides the device to device linking. Device orientation
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@240
PS2, Line 240: * can be detected automatically, or explicitly. See documentation for more details.
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@242
PS2, Line 242: * Automatic non-coherent init assigns BUIDs starting at 1 and incrementing sequentially
line over 80 characters
https://review.coreboot.org/#/c/27618/2/src/mainboard/gigabyte/m57sli/romstage.c@250
PS2, Line 250: 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/2/src/mainboard/gigabyte/m57sli/romstage.c@253
PS2, Line 253: /* 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: 2
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:00:14 +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/04f42448/attachment.html>
More information about the coreboot-gerrit
mailing list