build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39414 )
Change subject: [WIP] nb/intel/pineview: Clean up ......................................................................
Patch Set 1:
(74 comments)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... File src/northbridge/intel/pineview/pineview.h:
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 123: #define EPBAR8(x) *((volatile u8 *)(DEFAULT_EPBAR + x)) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 131: #define DMIBAR8(x) *((volatile u8 *)(DEFAULT_DMIBAR + x)) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... File src/northbridge/intel/pineview/raminit.c:
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 591: { // FSB = 667, DDR = 667 please, no space before tabs
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1007: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1015: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1025: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1033: 1,1,1,1,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1043: 0,0,0,0,0,0,0,0,0,0,0,0, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1051: 1,1,1,1,1,1,1,1,1,1,1,1, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1224: #define TABLE static const Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1227: #define FOR_EACH_RCOMP_GROUP for (i = 0; i < 7; i++) if (i != 1) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1677: while ((MCHBAR8(COMPCTRL1) & 1) != 0); trailing statements should be on next line
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 1689: while ((MCHBAR8(COMPCTRL1) & 1) != 0); trailing statements should be on next line
https://review.coreboot.org/c/coreboot/+/39414/1/src/northbridge/intel/pinev... PS1, Line 2581: const char *boot_str[] = {"Normal", "Reset", "Resume"}; char * array declaration might be better as static const