71 comments:
Patch Set #2, Line 25: edit irq_tables.c manualy and replace checksum.
'manualy' may be misspelled - perhaps 'manually'?
Patch Set #2, Line 23: printf("Checksum for IRQ Routing table is ok. You can use irq_tables.c in coreboot now.\n");
line over 96 characters
Patch Set #2, Line 12: return (sum);
return is not a function, parentheses are not required
Patch Set #2, 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.
Patch Set #2, 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.
Patch Set #2, Line 44: if ((fpir = fopen(filename, "w")) == NULL) {
do not use assignment in if condition
Patch Set #2, 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
Patch Set #2, Line 68: fprintf(fpir, "\t\t/* bus, dev | fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu */\n");
line over 96 characters
Patch Set #2, 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
Patch Set #2, Line 23: #if defined (__sun) && (defined(__i386) || defined(__amd64))
space prohibited between function name and open parenthesis '('
Patch Set #2, Line 37: static struct irq_routing_table *probe_table(char* ptr)
"foo* bar" should be "foo *bar"
Patch Set #2, Line 58: printf("Found PCI IRQ routing table signature at %p.\n", (void *) ((char *) rt - ptr + 0xf0000));
line over 96 characters
Patch Set #2, Line 59: printf("SIGNATURE = %s\n", (char*)&rt->signature);
"(foo*)" should be "(foo *)"
Patch Set #2, Line 64: printf("INT_ROUTER DEVICE = (0x%02x << 3) | 0x%01x\n", rt->rtr_devfn >> 3, rt->rtr_devfn & 7);
line over 96 characters
Patch Set #2, Line 70: printf("\tbus , dev | fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu\n");
line over 96 characters
Patch Set #2, Line 70: printf("\tbus , dev | fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu\n");
code indent should use tabs where possible
Patch Set #2, Line 70: printf("\tbus , dev | fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu\n");
please, no space before tabs
Patch Set #2, 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
Patch Set #2, 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
Patch Set #2, Line 73: (se_arr+i)->bus, (se_arr+i)->devfn >> 3,
line over 96 characters
Patch Set #2, Line 73: (se_arr+i)->bus, (se_arr+i)->devfn >> 3,
please, no space before tabs
Patch Set #2, Line 74: (se_arr+i)->devfn & 7, (se_arr+i)->irq[0].link,
please, no space before tabs
Patch Set #2, Line 75: (se_arr+i)->irq[0].bitmap, (se_arr+i)->irq[1].link,
please, no space before tabs
Patch Set #2, Line 76: (se_arr+i)->irq[1].bitmap, (se_arr+i)->irq[2].link,
please, no space before tabs
Patch Set #2, Line 77: (se_arr+i)->irq[2].bitmap, (se_arr+i)->irq[3].link,
please, no space before tabs
Patch Set #2, Line 78: (se_arr+i)->irq[3].bitmap, (se_arr+i)->slot,
please, no space before tabs
Patch Set #2, Line 82: if (rt->size > 0x400) {
braces {} are not necessary for single statement blocks
Patch Set #2, Line 89: printf("CHECKSUM = %#x\n", 0x100-((checksum_result - rt->checksum) & 0xFF));
line over 96 characters
Patch Set #2, Line 94: } else {
else is not generally useful after a break or return
Patch Set #2, Line 109: int main(int argc, char* argv[])
"foo* bar" should be "foo *bar"
Patch Set #2, Line 111: char* ptr;
"foo* bar" should be "foo *bar"
Patch Set #2, Line 113: struct irq_routing_table* rt = NULL;
"foo* bar" should be "foo *bar"
Patch Set #2, Line 114: void* bios_image = NULL;
"foo* bar" should be "foo *bar"
Patch Set #2, Line 115: if ( argc > 1 )
that open brace { should be on the previous line
Patch Set #2, Line 115: if ( argc > 1 )
space prohibited after that open parenthesis '('
Patch Set #2, Line 115: if ( argc > 1 )
space prohibited before that close parenthesis ')'
Patch Set #2, Line 117: /** there a paramater passed to the program, assume that it is a menory file */
'paramater' may be misspelled - perhaps 'parameter'?
Patch Set #2, Line 132: ptr = (char*)bios_image;
"(foo*)" should be "(foo *)"
Patch Set #2, Line 144: /** No paramaters means that the program will access the system memory */
'paramaters' may be misspelled - perhaps 'parameters'?
Patch Set #2, Line 163: return (1);
return is not a function, parentheses are not required
File util/getpir/pirq_routing.h:
Patch Set #2, Line 12: uint8_t link; /* IRQ line ID, chipset dependent, 0=not routed */
line over 96 characters
Patch Set #2, Line 46: #if CONFIG_HAVE_PIRQ_TABLE==1
spaces required around that '==' (ctx:VxV)
Patch Set #2, Line 53: #define ARRAY_SIZE(_x) (sizeof(_x) / sizeof(_x[0]))
Prefer ARRAY_SIZE(_x)
Patch Set #2, Line 149: {129, 8, "Bus Heirarchy"},
'Heirarchy' may be misspelled - perhaps 'Hierarchy'?
Patch Set #2, Line 251: static void apic_probe(vm_offset_t * paddr, int *where);
"foo * bar" should be "foo *bar"
Patch Set #2, Line 255: static void MPFloatingPointer(vm_offset_t paddr, int where, mpfps_t * mpfps);
"foo * bar" should be "foo *bar"
Patch Set #2, Line 274: static void pnstr(uint8_t * s, int c);
"foo * bar" should be "foo *bar"
Patch Set #2, Line 340: "usage: mptable [-dmesg] [-verbose] [-grope] [-help]\n");
Prefer using '"%s...", __func__' to using 'usage', this function's name, in a string
Patch Set #2, Line 373: if (strcmp(argv[0], "-dmesg") == 0) {
braces {} are not necessary for any arm of this statement
Patch Set #2, Line 388: if ((pfd = open("/dev/mem", O_RDONLY)) < 0)
do not use assignment in if condition
Patch Set #2, Line 413: if ((defaultConfig = mpfps.mpfb1))
do not use assignment in if condition
Patch Set #2, Line 437: static void apic_probe(vm_offset_t * paddr, int *where)
"foo * bar" should be "foo *bar"
Patch Set #2, Line 597: static void MPFloatingPointer(vm_offset_t paddr, int where, mpfps_t * mpfps)
"foo * bar" should be "foo *bar"
Patch Set #2, Line 788: /* initialze tables */
'initialze' may be misspelled - perhaps 'initialize'?
Patch Set #2, Line 789: for(x = 0; x < ARRAY_SIZE(busses); x++)
space required before the open parenthesis '('
Patch Set #2, Line 791: for(x = 0; x < ARRAY_SIZE(apics); x++)
space required before the open parenthesis '('
Patch Set #2, Line 827: printf("\t/* I/O Ints: Type Polarity Trigger Bus ID IRQ APIC ID PIN#*/ \n");
unnecessary whitespace before a quoted newline
Patch Set #2, Line 843: #if defined( EXTENDED_PROCESSING_READY )
space prohibited after that open parenthesis '('
Patch Set #2, Line 843: #if defined( EXTENDED_PROCESSING_READY )
space prohibited before that close parenthesis ')'
Patch Set #2, Line 845: if ((totalSize = cth.extended_table_length)) {
do not use assignment in if condition
Patch Set #2, Line 874: #if defined( OEM_PROCESSING_READY )
space prohibited after that open parenthesis '('
Patch Set #2, Line 874: #if defined( OEM_PROCESSING_READY )
space prohibited before that close parenthesis ')'
Patch Set #2, Line 880: if ((oemdata = (void *)malloc(cth.oem_table_size)) == NULL)
do not use assignment in if condition
Patch Set #2, Line 897: #if defined( RAW_DUMP )
space prohibited after that open parenthesis '('
Patch Set #2, Line 897: #if defined( RAW_DUMP )
space prohibited before that close parenthesis ')'
Patch Set #2, Line 961: cpuFlags & PROCENTRY_FLAG_EN) ? "usable" : "unusable");
Avoid multiple line dereference - prefer 'entry.cpuFlags'
Patch Set #2, Line 1005: for(i = 0; i < 6; i++) {
space required before the open parenthesis '('
Patch Set #2, Line 1006: switch(entry.busType[i]) {
space required before the open parenthesis '('
Patch Set #2, Line 1042: apicFlags & IOAPICENTRY_FLAG_EN) ? "usable" :
Avoid multiple line dereference - prefer 'entry.apicFlags'
Patch Set #2, 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
Patch Set #2, Line 1249: static void pnstr(uint8_t * s, int c)
"foo * bar" should be "foo *bar"
To view, visit change 48322. To unsubscribe, or for help writing mail filters, visit settings.