build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38359 )
Change subject: nb/intel/nehalem: Try to clean up code ......................................................................
Patch Set 2:
(48 comments)
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... File src/northbridge/intel/nehalem/raminit.c:
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 53: #define FOR_ALL_RANKS \ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 54: for (channel = 0; channel < NUM_CHANNELS; channel++) \ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 54: for (channel = 0; channel < NUM_CHANNELS; channel++) \ suspect code indent for conditional statements (2, 4)
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 55: for (slot = 0; slot < NUM_SLOTS; slot++) \ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 55: for (slot = 0; slot < NUM_SLOTS; slot++) \ suspect code indent for conditional statements (4, 6)
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 56: for (rank = 0; rank < NUM_RANKS; rank++) please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 58: #define FOR_ALL_RANKS_BACKWARDS \ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 59: for (channel = NUM_CHANNELS - 1; channel >= 0; channel--) \ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 59: for (channel = NUM_CHANNELS - 1; channel >= 0; channel--) \ suspect code indent for conditional statements (2, 4)
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 60: for (slot = 0; slot < NUM_SLOTS; slot++) \ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 60: for (slot = 0; slot < NUM_SLOTS; slot++) \ suspect code indent for conditional statements (4, 6)
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 63: #define FOR_CHAN_RANKS \ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 64: for (slot = 0; slot < NUM_SLOTS; slot++) \ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 64: for (slot = 0; slot < NUM_SLOTS; slot++) \ suspect code indent for conditional statements (2, 4)
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 65: for (rank = 0; rank < NUM_RANKS; rank++) please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 67: #define FOR_POPULATED_RANKS \ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 68: for (channel = 0; channel < NUM_CHANNELS; channel++) \ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 68: for (channel = 0; channel < NUM_CHANNELS; channel++) \ suspect code indent for conditional statements (2, 4)
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 69: for (slot = 0; slot < NUM_SLOTS; slot++) \ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 69: for (slot = 0; slot < NUM_SLOTS; slot++) \ suspect code indent for conditional statements (4, 6)
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 70: for (rank = 0; rank < NUM_RANKS; rank++) \ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 71: if (info->populated_ranks[channel][slot][rank]) code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 71: if (info->populated_ranks[channel][slot][rank]) please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 73: #define FOR_POPULATED_CHAN_RANKS \ Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 74: for (slot = 0; slot < NUM_SLOTS; slot++) \ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 74: for (slot = 0; slot < NUM_SLOTS; slot++) \ suspect code indent for conditional statements (2, 4)
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 75: for (rank = 0; rank < NUM_RANKS; rank++) \ please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 75: for (rank = 0; rank < NUM_RANKS; rank++) \ suspect code indent for conditional statements (4, 6)
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 76: if (info->populated_ranks[channel][slot][rank]) please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 82: if (info->populated_ranks[channel][slot][rank]) code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 82: if (info->populated_ranks[channel][slot][rank]) please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 416: training.lane_timings[2][channel][slot][rank][lane] + 0x20; Avoid multiple line dereference - prefer 'info->training.lane_timings[2][channel][slot][rank][lane]'
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 735: avg4044[channel] + u8_FFFD17E0[channel][extended_silicon_revision] Avoid multiple line dereference - prefer 'info->avg4044[channel]'
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 1521: for (tm = 0; tm < 4; tm++) FOR_ALL_RANKS for (lane = 0; lane < 9; lane++) { trailing statements should be on next line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 1564: FOR_POPULATED_RANKS for (lane = 0; lane < 9; lane++) for (i = 0; i < 4; i++) { trailing statements should be on next line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 1871: for (i = 0; i < 2; i++) FOR_CHAN_RANKS for (lane = 0; lane < 9; lane++) { trailing statements should be on next line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 1877: for (i = 1; i < 4; i++) FOR_ALL_RANKS for (lane = 0; lane < 9; lane++) { trailing statements should be on next line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 2303: if (timings[reg_178][channel][slot][rank] Too many leading tabs - consider code refactoring
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 2309: } else { Too many leading tabs - consider code refactoring
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 2383: if (timings[reg_178][channel][slot][rank] Too many leading tabs - consider code refactoring
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 2722: FOR_ALL_RANKS for (lane = 0; lane < 8 + info->use_ecc; lane++) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 3372: FOR_POPULATED_CHAN_RANKS for (lane = 0; lane < 8 + info->use_ecc; lane++) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 3836: pci_read_config32(PCI_DEV (0xff, 0, 0), 0xd0); // !!!! space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 3837: pci_write_config32(PCI_DEV (0xff, 0, 0), 0xd0, 0x00000180); space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 4113: pci_write_config32(PCI_DEV (0xff, 0, 0), 0x60, 0x00020220); space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 4208: for (tm = 0; tm < 4; tm++) FOR_ALL_RANKS for (lane = 0; lane < 9; lane++) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 4208: for (tm = 0; tm < 4; tm++) FOR_ALL_RANKS for (lane = 0; lane < 9; lane++) { trailing statements should be on next line
https://review.coreboot.org/c/coreboot/+/38359/2/src/northbridge/intel/nehal... PS2, Line 4395: if (info.populated_ranks[channel][slot][rank]) { braces {} are not necessary for single statement blocks