Lean Sheng Tan 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 20:
Patch Set 20:
Patch Set 20:
Patch Set 20:
Patch Set 20: Ok, But this code already creates a irq info data , I am using the same to generate the ACPI code too, I do not need to re-populate the irq matrix.
This argument sounds to me as if you don't want to use the common facilities we already have because you already wrote your own reimplementation of them. I hope I misunderstand you because that's not very convincing.
What is missing in acpi_pirq_gen that is present in your code (and can't be added there to avoid having all the common parts twice in our code base)?
The intent of this code was to populate irq info based on the device tree selection and pass it to FSP for IRQ configuration based on the irq assignment rules, this implementation is added new(does not exist in acpi_pirq_gen(neither should be scoped in ACPI module)), I think Arthur made a point to re-use APCI generation part from acpi_pirq_gen, to which my response was I did not want to re-populate the irq matrix again, since this code already creates that info.
You don't have to repopulate anything, the matrix is a local structure of that code. You just have to provide a callback function called 'intel_common_map_pirq' which returns a route for a given pin on a given PCI device. Given your current code that should be easy or even trivial to implement. Give the FSP what it needs using this code(and please document it first!) and use the common southbridge code create ACPI _PIR tables.
Your code potentially has multiple entries for the same PCI device and pin in the _PIR table. That needs to be avoided. The best way to avoid that is by populating a matrix[MAX_PCI_DEV][4] '4' being pin[A-D] otherwise you have to keep track of what device and what pin already have an entry. You end up needing a 'matrix' anyway as the data FSP needs does not fully match what ACPI _PIR expects.
Please consider that reusing common code usually means avoiding pitfalls someone else fell into in the past.
I believe Aamir has answered your questions?