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 14:
(5 comments)
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
The LPSS devices cannot share the same IRQ# and needs to be assigned different IRQs. […]
What I was referring to is:
Non-LPSS device has IRQ# x
Can LPSS device have IRQ# x?
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 33: *
Add as rule? or did you mean if this implementation would take care of it. […]
Can you please add a debug dump function that prints out the configuration for all devices? And attach the output here. That would be very helpful.
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 47: slot != PCI_SLOT(dev->path.pci.devfn)
Is there even any guarantee that the devicetree provides the devices "in order" ? I could arbitrarily go in and swap two entries and there should be no side-effects because of that.
Exactly. It depends on the way devicetree entries are added and that could silently fail.
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 71: soc_irq_devices_list
I can add an additional parameter and eliminate the soc call from common. […]
Humm the sorting requirement seems very brittle.
https://review.coreboot.org/c/coreboot/+/34089/13/src/soc/intel/common/block... PS13, Line 99: index++;
Did not think of that. […]
Yes, you have SoC maintain the buffer. But, then it means that every SoC implementation needs to do that. If you maintain the irq buffer here, then you can avoid the duplicate code in SoC.