Change in coreboot[master]: src/soc/intel/commmon/itss: Add support to get interrupt table for PC...
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34089 )
Change subject: src/soc/intel/commmon/itss: Add support to get interrupt table for PCI devices ......................................................................
Patch Set 6:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34089/5/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/itss.h:
https://review.coreboot.org/c/coreboot/+/34089/5/src/soc/intel/common/block/... PS5, Line 56: intconfigptr
int_config_ptr?
Done
https://review.coreboot.org/c/coreboot/+/34089/5/src/soc/intel/common/block/... PS5, Line 56: itss_configure_pirx
It would be a good idea to add a comment indicating what the function really does and what params it […]
Done
https://review.coreboot.org/c/coreboot/+/34089/5/src/soc/intel/common/block/... PS5, Line 56: void
Why void?
Ok, changed the return type to accommodate for error checking
https://review.coreboot.org/c/coreboot/+/34089/5/src/soc/intel/common/block/... File src/soc/intel/common/block/itss/itss.c:
https://review.coreboot.org/c/coreboot/+/34089/5/src/soc/intel/common/block/... PS5, Line 192: PIRx mapped from IRQ# 16:23 starting from PIRQA
Shouldn't this info be provided by SoC specific code?
This is same across SOCs. The implementation is planned for CNL onwards and all would follow the same mappings.
https://review.coreboot.org/c/coreboot/+/34089/5/src/soc/intel/common/block/... PS5, Line 198: if (!islpss) {
It would be good to add comments indicating the rules you are following for assigning IRQ#.
Yes, Missed that, Added in revised Patch set
https://review.coreboot.org/c/coreboot/+/34089/5/src/soc/intel/common/block/... PS5, Line 225: if (!is_lpss(dev)) : /* Devices that have shared PIRx routing */ : irqlist = add_dev_irq(dev, false); : else : /* LPSS controllers needs unique IRQ assignments */ : irqlist = add_dev_irq(dev, true);
This can just be: […]
Done
https://review.coreboot.org/c/coreboot/+/34089/5/src/soc/intel/common/block/... PS5, Line 236: itss_configure_pirx
Can't this common block driver maintain a pointer to the interrupt config list and return that point […]
Ack
-
Aamir Bohra (Code Review)