Arthur Heymans 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 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34089/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34089/9//COMMIT_MSG@13 PS9, Line 13: This table can then be used for FSP interrupt config override or generate : acpi _PRT PIRx entry.
I can reuse the ACPI generation, but would like to stick to this implementation for irq map generation and allow FSP to configure the PIRx map. Also the older implementation does not seem to account for unique IRQ assignments for LPSS
The code should make it more clear that this is just to generate something the FSP needs.
Feel free to extend the older code with a hook for LPSS.
https://review.coreboot.org/c/coreboot/+/34089/9/src/soc/intel/common/block/... File src/soc/intel/common/block/itss/irq.c:
https://review.coreboot.org/c/coreboot/+/34089/9/src/soc/intel/common/block/... PS9, Line 78: uint8_t *irq_config
The support is restricted to CNL and ICL. This is with assumption to support 32 PCI IRQ routing information. Which I believe would suffice for existing designs(16- 20 PCI IRQ devices enabled). I have captured the assumption here.
https://review.coreboot.org/c/coreboot/+/34349/4/src/soc/intel/cannonlake/ir...
uint8_t is pointing to continuous memory chunk allocated from the SOC, that would hold the IRQ info for the PCI devices. I think Callee can process it as they want.
The place to document assumption is in the implementation of the function, not in the caller.
Why not simply pass a pointer to struct dev_irq instead? You don't need a pointer to uint8_t for a 'continuous memory chunk' or do you really want to hide that the actual DevIntConfigPtr is a pointer to struct dev_irq (please document that btw)?
You might also pass the size of the array and check for it. I'm not a fan of no boundary checks as this can result in problems that are hard to debug.