Mike Banon has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47852 )
Change subject: lenovo/g505s: properly program the IRQ tables ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/47852/2/src/mainboard/lenovo/g505s/... File src/mainboard/lenovo/g505s/mainboard.c:
https://review.coreboot.org/c/coreboot/+/47852/2/src/mainboard/lenovo/g505s/... PS2, Line 48: const u8 *picr_data = mainboard_picr_data; : const u8 *intr_data = mainboard_intr_data;
are those used?
Done (removed the first occurence).
https://review.coreboot.org/c/coreboot/+/47852/2/src/mainboard/lenovo/g505s/... PS2, Line 55: sizeof(mainboard_pirq_data) / sizeof(struct pirq_struct);
ARRAY_SIZE
Done
https://review.coreboot.org/c/coreboot/+/47852/2/src/mainboard/lenovo/g505s/... PS2, Line 57: picr_data_ptr = mainboard_picr_data;
done twice.
Done
https://review.coreboot.org/c/coreboot/+/47852/2/src/mainboard/lenovo/g505s/... File src/mainboard/lenovo/g505s/mptable.h:
https://review.coreboot.org/c/coreboot/+/47852/2/src/mainboard/lenovo/g505s/... PS2, Line 17: static const u8 mainboard_picr_data[FCH_INT_TABLE_SIZE] = { : /* INTA# - INTH# */ : [0x00] = 0x03, 0x04, 0x05, 0x07, 0x1F, 0x1F, 0x1F, 0x1F, : /* Misc-nil,0,1,2, INTA-INTD from Serial irq */ : [0x08] = 0xAA, 0xF1, 0x00, 0x00, 0x1F, 0x1F, 0x1F, 0x1F, : /* SCI, SMBUS0, ASF, HDA, SD, GEC, PerMon */ : [0x10] = 0x09, 0x1F, 0x1F, 0x03, 0x1F, 0x1F, 0x1F, : /* IMC INT0-INT5 */ : [0x20] = 0x1F, 0x1F, 0x1F, 0x1F, 0x1F, 0x1F, : /* USB Devs: 18 INTA#,B#; 19 INTA#,B#; 22 INTA#,B#; 20 INTC# */ : [0x30] = 0x05, 0x04, 0x05, 0x04, 0x05, 0x04, 0x05, : /* IDE, SATA */ : [0x40] = 0x1F, 0x07, : /* GPP Int0-Int3 */ : [0x50] = 0x1F, 0x1F, 0x1F, 0x1F : }; : : static const u8 mainboard_intr_data[FCH_INT_TABLE_SIZE] = { : /* INTA# - INTH# */ : [0x00] = 0x10, 0x11, 0x12, 0x13, 0x1F, 0x1F, 0x1F, 0x1F, : /* Misc-nil,0,1,2, INTA-INTD from Serial irq */ : [0x08] = 0x00, 0x00, 0x00, 0x00, 0x1F, 0x1F, 0x1F, 0x1F, : /* SCI, SMBUS0, ASF, HDA, SD, GEC, PerMon */ : [0x10] = 0x09, 0x1F, 0x1F, 0x10, 0x1F, 0x1F, 0x1F, : /* IMC INT0-INT5 */ : [0x20] = 0x1F, 0x1F, 0x1F, 0x1F, 0x1F, 0x1F, : /* USB Devs: 18 INTA#,B#; 19 INTA#,B#; 22 INTA#,B#; 20 INTC# */ : [0x30] = 0x12, 0x11, 0x12, 0x11, 0x12, 0x11, 0x12, : /* IDE, SATA */ : [0x40] = 0x1F, 0x13, : /* GPP Int0-Int3 */ : [0x50] = 0x1F, 0x1F, 0x1F, 0x1F : };
Having static variables in a header file seems like a bad idea.
Although it could've been transformed into a bunch of defines, I kept this old format for the easiness of comparison (for when we'd be fixing the other boards). And it's in a header because two .c files are using it