Tim Wawrzynczak 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 18:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34089/8/src/soc/intel/common/block/... File src/soc/intel/common/block/itss/irq.c:
https://review.coreboot.org/c/coreboot/+/34089/8/src/soc/intel/common/block/... PS8, Line 32: create_device_node
nit: This doesn't have anything to do with a device, rather with a list of IRQ information; maybe cr […]
Ack
https://review.coreboot.org/c/coreboot/+/34089/8/src/soc/intel/common/block/... PS8, Line 71: static struct irq_node *info; : static int slot; : /* PIRx mapped from IRQ# 16:23 starting from PIRQA */ : static int int_line = PIRQA_APIC_IRQ; : static int int_lpss = PIRQA_APIC_IRQ;
This is more to keep track on previous assignment and use it abide by following IRQ allocation rules […]
Ack
https://review.coreboot.org/c/coreboot/+/34089/8/src/soc/intel/common/block/... PS8, Line 93: * only interrupt 16-23 can be shared */ : if (int_line > 23)
I think initializing it with static int int_line = PIRQA_APIC_IRQ would ensure it. […]
Ack
https://review.coreboot.org/c/coreboot/+/34089/8/src/soc/intel/common/block/... PS8, Line 102: static bool is_irq_dev(struct device *dev) : { : static size_t size; : static const int *devlist; : : if (!devlist) : devlist = soc_irq_devices_list(&size); : for (int i = 0; i < size; i++) { : if (dev->path.pci.devfn == devlist[i]) : return true; : } : return false; : } :
yeah, for each pci device in the tree , wanted to avoid soc call backs
Ack
https://review.coreboot.org/c/coreboot/+/34089/17/src/soc/intel/common/block... File src/soc/intel/common/block/itss/irq.c:
https://review.coreboot.org/c/coreboot/+/34089/17/src/soc/intel/common/block... PS17, Line 31: * 3. LPSS controllers need to be assigned unique IRQs
Done. […]
Ack
https://review.coreboot.org/c/coreboot/+/34089/18/src/soc/intel/common/block... File src/soc/intel/common/block/itss/irq.c:
https://review.coreboot.org/c/coreboot/+/34089/18/src/soc/intel/common/block... PS18, Line 41: Is there a concern there could be more than 8 LPSS devices requiring IRQ assignment? Maybe we need an error here if int_lpss > 23 ?
https://review.coreboot.org/c/coreboot/+/34089/18/src/soc/intel/common/block... PS18, Line 99: index To me, exiting early if index becomes >= num_entries shows the error a little more clearly. What do you think?