Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47852 )
Change subject: lenovo/g505s: properly program the IRQ tables ......................................................................
Patch Set 2:
(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?
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
https://review.coreboot.org/c/coreboot/+/47852/2/src/mainboard/lenovo/g505s/... PS2, Line 57: picr_data_ptr = mainboard_picr_data; done twice.
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.