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 4:
(25 comments)
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... File src/northbridge/intel/nehalem/raminit.c:
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 72: #define FOR_CHAN_RANKS FOR_EACH_SLOT FOR_EACH_RANK Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 73: #define FOR_CHAN_LANES(ecc) FOR_CHAN_RANKS FOR_EACH_LANE(ecc) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 74: #define FOR_ALL_RANKS FOR_EACH_CHANNEL_UP FOR_CHAN_RANKS Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 75: #define FOR_ALL_RANKS_BACKWARDS FOR_EACH_CHANNEL_DN FOR_CHAN_RANKS Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 76: #define FOR_ALL_SLOTS FOR_EACH_CHANNEL_UP FOR_EACH_SLOT Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 77: #define FOR_ALL_LANES(ecc) FOR_ALL_RANKS FOR_EACH_LANE(ecc) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 79: #define IF_POPULATED if (info->populated_ranks[channel][slot][rank]) Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 80: #define FOR_POPULATED_RANKS FOR_ALL_RANKS IF_POPULATED Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 81: #define FOR_POPULATED_CHAN_RANKS FOR_CHAN_RANKS IF_POPULATED Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 82: #define FOR_POPULATED_RANKS_BACKWARDS FOR_ALL_RANKS_BACKWARDS IF_POPULATED Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 83: #define FOR_POPULATED_RANKS_LANES(ecc) FOR_POPULATED_RANKS FOR_EACH_LANE(ecc) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 2333: if (timings[reg_178][channel][slot][rank] Too many leading tabs - consider code refactoring
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 2339: } else { Too many leading tabs - consider code refactoring
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 2414: if (timings[reg_178][channel][slot][rank] Too many leading tabs - consider code refactoring
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 2480: FOR_POPULATED_RANKS_BACKWARDS FOR_EACH_LANE(1) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 2701: FOR_POPULATED_RANKS_BACKWARDS FOR_EACH_LANE(0) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 2988: FOR_POPULATED_RANKS_BACKWARDS FOR_EACH_LANE(0) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 3064: FOR_POPULATED_RANKS_BACKWARDS FOR_EACH_LANE(0) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 3410: FOR_POPULATED_CHAN_RANKS FOR_EACH_LANE(info->use_ecc) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 3868: pci_read_config32(PCI_DEV (0xff, 0, 0), 0xd0); // !!!! space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 3869: 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/4/src/northbridge/intel/nehal... PS4, Line 4149: 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/4/src/northbridge/intel/nehal... PS4, Line 4223: FOR_EACH_CHANNEL_UP FOR_EACH_LANE(1) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 4245: FOR_ALL_RANKS FOR_EACH_LANE(1) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/38359/4/src/northbridge/intel/nehal... PS4, Line 4434: if (info.populated_ranks[channel][slot][rank]) { braces {} are not necessary for single statement blocks