Piotr Kleinschmidt has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42097 )
Change subject: mb/pcengines/apu2/mptable.c: fix invalid MP table and IRQ table ......................................................................
Patch Set 1:
(6 comments)
Patch Set 1:
(1 comment)
Can't this be autogenerated?
https://review.coreboot.org/c/coreboot/+/42097/1/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/mainboard.c:
https://review.coreboot.org/c/coreboot/+/42097/1/src/mainboard/pcengines/apu... PS1, Line 73: [0x30] = 0x1F,0x12,0x1F,0x12,0x1F,0x12,0x1F,0x00,
This is wrong, you are trying to set reserved values at offsets 0x31, 0x33 and 0x35, but the interru […]
Okay, I'll revert to 0x30, 0x32 and 0x34 configuration. I made this change because definitions in amd_oci_int_defs suggest that 0x30 is PIRQ_OHCI1, 0x32 is PIRQ_OHCI2... Whereas 0x31 is PIRQ_EHCI1 and so on. Therefore those names should be used in mptable.c file and MP Table configuration routine which may be misleading with OHCI names.
https://review.coreboot.org/c/coreboot/+/42097/1/src/mainboard/pcengines/apu... PS1, Line 95: {IOMMU_DEVFN, {PIRQ_A, PIRQ_NC, PIRQ_NC, PIRQ_NC}}, /* IOMMU: 00.2 */
As indicated in other comment, not sure about IOMMU interrupt.
I suggested myself by output from 'lspci -v' with acpi enabled. IOMMU has IRQ assigned there.
https://review.coreboot.org/c/coreboot/+/42097/1/src/mainboard/pcengines/apu... PS1, Line 107: {SMBUS_DEVFN, {PIRQ_NC, PIRQ_NC, PIRQ_NC, PIRQ_NC}}, /* SMBUS: 14.0 */
Why no interrupt for SMBus?
As in case of IOMMU I suggested myself by 'lspci -v' with acpi enabled and SMBus doesn't have any IRQ assigned.
https://review.coreboot.org/c/coreboot/+/42097/1/src/mainboard/pcengines/apu... File src/mainboard/pcengines/apu2/mptable.c:
https://review.coreboot.org/c/coreboot/+/42097/1/src/mainboard/pcengines/apu... PS1, Line 89: /* GPP0 */ : PCI_INT(0x0, 0x2, 0x0, 0x10); // Network 3 : /* GPP1 */ : PCI_INT(0x0, 0x2, 0x1, 0x11); // Network 2 : /* GPP2 */ : PCI_INT(0x0, 0x2, 0x2, 0x12); // Network 1 : /* GPP3 */ : PCI_INT(0x0, 0x2, 0x3, 0x13); // mPCI : /* GPP4 */ : PCI_INT(0x0, 0x2, 0x4, 0x14); // mPCI :
The bridges should also have their entries in MP table.
Ok, I'll revert this configuration
https://review.coreboot.org/c/coreboot/+/42097/1/src/mainboard/pcengines/apu... PS1, Line 75: PCI_INT(0x0, 0x14, 0x7, intr_data_ptr[PIRQ_SD]);
The third parameter does not indicate the PCI function number, but the interrupt lane, i.e. […]
Ack
https://review.coreboot.org/c/coreboot/+/42097/1/src/mainboard/pcengines/apu... PS1, Line 81: PCI_INT(0x0, 0x10, 0x0, intr_data_ptr[PIRQ_EHCI3]);
xHCI controller seems to be using PIRQ_C in mainboard_pirq_data. […]
If I understand correctly, XHCI uses PIRQC, but EHCIs controllers use PIRQC too. I have changed 'mainboard_pirq_data' structure according to that, so now every EHCI and XHCI controller uses PIRQC. Is it wrong?