build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48322 )
Change subject: Revert "util: Remove 'getpir' and 'mptable' tools" ......................................................................
Patch Set 2:
(71 comments)
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/README File util/getpir/README:
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/README@25 PS2, Line 25: edit irq_tables.c manualy and replace checksum. 'manualy' may be misspelled - perhaps 'manually'?
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/checkpir.c File util/getpir/checkpir.c:
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/checkpir.c@23 PS2, Line 23: printf("Checksum for IRQ Routing table is ok. You can use irq_tables.c in coreboot now.\n"); line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/checksum.c File util/getpir/checksum.c:
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/checksum.c@12 PS2, Line 12: return (sum); return is not a function, parentheses are not required
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/code_gen.c File util/getpir/code_gen.c:
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/code_gen.c@23 PS2, Line 23: " * along with this program; if not, write to the Free Software\n", Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/code_gen.c@24 PS2, Line 24: " * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA\n", Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/code_gen.c@44 PS2, Line 44: if ((fpir = fopen(filename, "w")) == NULL) { do not use assignment in if condition
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/code_gen.c@65 PS2, Line 65: fprintf(fpir, "\t%#x, /* Checksum (has to be set to some value that\n * would give 0 after the sum of all bytes\n * for this structure (including checksum).\n */\n", line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/code_gen.c@68 PS2, Line 68: fprintf(fpir, "\t\t/* bus, dev | fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu */\n"); line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/code_gen.c@70 PS2, Line 70: fprintf(fpir, "\t\t{0x%02x, (0x%02x << 3) | 0x%01x, {{0x%02x, 0x%04x}, {0x%02x, 0x%04x}, {0x%02x, 0x%04x}, {0x%02x, 0x%04x}}, 0x%x, 0x%x},\n", line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c File util/getpir/getpir.c:
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@23 PS2, Line 23: #if defined (__sun) && (defined(__i386) || defined(__amd64)) space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@37 PS2, Line 37: static struct irq_routing_table *probe_table(char* ptr) "foo* bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@58 PS2, Line 58: printf("Found PCI IRQ routing table signature at %p.\n", (void *) ((char *) rt - ptr + 0xf0000)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@59 PS2, Line 59: printf("SIGNATURE = %s\n", (char*)&rt->signature); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@64 PS2, Line 64: printf("INT_ROUTER DEVICE = (0x%02x << 3) | 0x%01x\n", rt->rtr_devfn >> 3, rt->rtr_devfn & 7); line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@70 PS2, Line 70: printf("\tbus , dev | fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu\n"); line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@70 PS2, Line 70: printf("\tbus , dev | fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu\n"); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@70 PS2, Line 70: printf("\tbus , dev | fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu\n"); please, no space before tabs
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@70 PS2, Line 70: printf("\tbus , dev | fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu\n"); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@72 PS2, Line 72: printf("\t0x%02x, (0x%02x << 3) | 0x%01x, {{0x%02x, 0x%04x}, {0x%02x, 0x%04x}, {0x%02x, 0x%04x}, {0x%02x, 0x%04x}}, 0x%x, 0x%x},\n", line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@73 PS2, Line 73: (se_arr+i)->bus, (se_arr+i)->devfn >> 3, line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@73 PS2, Line 73: (se_arr+i)->bus, (se_arr+i)->devfn >> 3, please, no space before tabs
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@74 PS2, Line 74: (se_arr+i)->devfn & 7, (se_arr+i)->irq[0].link, please, no space before tabs
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@75 PS2, Line 75: (se_arr+i)->irq[0].bitmap, (se_arr+i)->irq[1].link, please, no space before tabs
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@76 PS2, Line 76: (se_arr+i)->irq[1].bitmap, (se_arr+i)->irq[2].link, please, no space before tabs
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@77 PS2, Line 77: (se_arr+i)->irq[2].bitmap, (se_arr+i)->irq[3].link, please, no space before tabs
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@78 PS2, Line 78: (se_arr+i)->irq[3].bitmap, (se_arr+i)->slot, please, no space before tabs
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@82 PS2, Line 82: if (rt->size > 0x400) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@89 PS2, Line 89: printf("CHECKSUM = %#x\n", 0x100-((checksum_result - rt->checksum) & 0xFF)); line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@94 PS2, Line 94: } else { else is not generally useful after a break or return
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@109 PS2, Line 109: int main(int argc, char* argv[]) "foo* bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@111 PS2, Line 111: char* ptr; "foo* bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@113 PS2, Line 113: struct irq_routing_table* rt = NULL; "foo* bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@114 PS2, Line 114: void* bios_image = NULL; "foo* bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@115 PS2, Line 115: if ( argc > 1 ) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@115 PS2, Line 115: if ( argc > 1 ) space prohibited after that open parenthesis '('
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@115 PS2, Line 115: if ( argc > 1 ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@117 PS2, Line 117: /** there a paramater passed to the program, assume that it is a menory file */ 'paramater' may be misspelled - perhaps 'parameter'?
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@132 PS2, Line 132: ptr = (char*)bios_image; "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@144 PS2, Line 144: /** No paramaters means that the program will access the system memory */ 'paramaters' may be misspelled - perhaps 'parameters'?
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/getpir.c@163 PS2, Line 163: return (1); return is not a function, parentheses are not required
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/pirq_routing.h File util/getpir/pirq_routing.h:
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/pirq_routing.h@... PS2, Line 12: uint8_t link; /* IRQ line ID, chipset dependent, 0=not routed */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/getpir/pirq_routing.h@... PS2, Line 46: #if CONFIG_HAVE_PIRQ_TABLE==1 spaces required around that '==' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c File util/mptable/mptable.c:
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@53 PS2, Line 53: #define ARRAY_SIZE(_x) (sizeof(_x) / sizeof(_x[0])) Prefer ARRAY_SIZE(_x)
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@149 PS2, Line 149: {129, 8, "Bus Heirarchy"}, 'Heirarchy' may be misspelled - perhaps 'Hierarchy'?
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@251 PS2, Line 251: static void apic_probe(vm_offset_t * paddr, int *where); "foo * bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@255 PS2, Line 255: static void MPFloatingPointer(vm_offset_t paddr, int where, mpfps_t * mpfps); "foo * bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@274 PS2, Line 274: static void pnstr(uint8_t * s, int c); "foo * bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@340 PS2, Line 340: "usage: mptable [-dmesg] [-verbose] [-grope] [-help]\n"); Prefer using '"%s...", __func__' to using 'usage', this function's name, in a string
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@373 PS2, Line 373: if (strcmp(argv[0], "-dmesg") == 0) { braces {} are not necessary for any arm of this statement
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@388 PS2, Line 388: if ((pfd = open("/dev/mem", O_RDONLY)) < 0) do not use assignment in if condition
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@413 PS2, Line 413: if ((defaultConfig = mpfps.mpfb1)) do not use assignment in if condition
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@437 PS2, Line 437: static void apic_probe(vm_offset_t * paddr, int *where) "foo * bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@597 PS2, Line 597: static void MPFloatingPointer(vm_offset_t paddr, int where, mpfps_t * mpfps) "foo * bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@788 PS2, Line 788: /* initialze tables */ 'initialze' may be misspelled - perhaps 'initialize'?
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@789 PS2, Line 789: for(x = 0; x < ARRAY_SIZE(busses); x++) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@791 PS2, Line 791: for(x = 0; x < ARRAY_SIZE(apics); x++) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@827 PS2, Line 827: printf("\t/* I/O Ints: Type Polarity Trigger Bus ID IRQ APIC ID PIN#*/ \n"); unnecessary whitespace before a quoted newline
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@843 PS2, Line 843: #if defined( EXTENDED_PROCESSING_READY ) space prohibited after that open parenthesis '('
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@843 PS2, Line 843: #if defined( EXTENDED_PROCESSING_READY ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@845 PS2, Line 845: if ((totalSize = cth.extended_table_length)) { do not use assignment in if condition
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@874 PS2, Line 874: #if defined( OEM_PROCESSING_READY ) space prohibited after that open parenthesis '('
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@874 PS2, Line 874: #if defined( OEM_PROCESSING_READY ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@880 PS2, Line 880: if ((oemdata = (void *)malloc(cth.oem_table_size)) == NULL) do not use assignment in if condition
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@897 PS2, Line 897: #if defined( RAW_DUMP ) space prohibited after that open parenthesis '('
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@897 PS2, Line 897: #if defined( RAW_DUMP ) space prohibited before that close parenthesis ')'
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@961 PS2, Line 961: cpuFlags & PROCENTRY_FLAG_EN) ? "usable" : "unusable"); Avoid multiple line dereference - prefer 'entry.cpuFlags'
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@1005 PS2, Line 1005: for(i = 0; i < 6; i++) { space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@1006 PS2, Line 1006: switch(entry.busType[i]) { space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@1042 PS2, Line 1042: apicFlags & IOAPICENTRY_FLAG_EN) ? "usable" : Avoid multiple line dereference - prefer 'entry.apicFlags'
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@1099 PS2, Line 1099: printf("\tsmp_write_intsrc(mc, %s, %s|%s, 0x%x, (0x%02x << 2) | INT%c, 0x%x, 0x%x);\n", line over 96 characters
https://review.coreboot.org/c/coreboot/+/48322/2/util/mptable/mptable.c@1249 PS2, Line 1249: static void pnstr(uint8_t * s, int c) "foo * bar" should be "foo *bar"