Change in coreboot[master]: src/soc/intel/common/itss: Add support to get IRQ configuration for P...
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34089 )
Change subject: src/soc/intel/common/itss: Add support to get IRQ configuration for PCI devices ......................................................................
Patch Set 13:
(9 comments)
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/irq.h:
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 25: P nit: space after *
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 31: PCI devices Is this list of PCI devfns?
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 32: int pci_devfn_t?
https://review.coreboot.org/c/coreboot/+/34089/11/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/irq.h:
https://review.coreboot.org/c/coreboot/+/34089/11/src/soc/intel/common/block... PS11, Line 25: * space after *
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... File src/soc/intel/common/block/itss/irq.c:
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 32: LPSS controllers need to be assigned unique IRQs Does this mean that IRQ# for LPSS device cannot be shared with another LPSS device or any other device? I am guessing it is the latter?
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 33: * What about PCI swizzling for PCIe root ports?
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 47: slot != PCI_SLOT(dev->path.pci.devfn) It looks like this is setting int_line to PIRQA_APIC_IRQ if slot for current device does not match what's stored in slot. Is the assumption here that the devices are sorted in such a way that all the functions of the same slot would be accessed one after the other?
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 71: soc_irq_devices_list Instead of having another call go back to SoC, can it provide the device list as a parameter to fill_pci_irq_config? Actually, can fill_pci_irq_config() loop only over the device list provided by SoC instead of all devices?
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 99: index++;
The reason I Kept it out was for the buffer overrun check and return error. […]
Curious why SoC has to pass in the address of table that needs to be filled in? Instead can this file maintain a table of MAX_INDEX and return the index count that is actually used? That way this function does not have to run the loop if it gets called multiple times.
-
Furquan Shaikh (Code Review)