Attention is currently required from: Nico Huber, Furquan Shaikh, Martin Roth, Duncan Laurie, Aamir Bohra, Srinidhi N Kaushik, Patrick Rudolph.
9 comments:
File src/soc/intel/common/block/irq/irq.c:
Patch Set #3, Line 69: return entries[i].irq;
Put the for body in braces for consistency and balance?
PCI_INT_A
Patch Set #3, Line 87: return (enum pci_pin)(i + 1);
Put the for body in braces for consistency and balance?
Patch Set #3, Line 87: i + 1
PCI_INT_A + i
no need for cast
Patch Set #3, Line 102: while (next != curr_idx) {
Assumes `current <= MAX_IRQ` to terminate.
dev = pcidev_path_on_root(devfn);
if (!dev) {
dummy.bus = &bus;
dummy.path.type = DEVICE_PATH_PCI;
dummy.path.pci.devfn = devfn;
dev = &dummy;
}
const uint8_t pin = pci_read_config8(dev, PCI_INTERRUPT_PIN);
Looks like calling pci_s_read_config8() directly would be easier here.
Patch Set #3, Line 155: line = MIN_IRQ;
Why?
Patch Set #3, Line 186: int pin = get_pci_irq_pin(devfn);
This seems a bit fragile, `UNIQUE_IRQ` would have the requirement that no other fn
has the same pin set. Which is harder to verify. next_pin_for_slot() also does
not take potential future get_pci_irq_pin() results into account and relies on
the already set `entries` alone. It seems everything would be easier if we say
`UNIQUE_IRQ` requires all pins of a `slot` configurable and all fn's should be
set to `UNIQUE_IRQ`. That seems true for the TGL use-case at least.
PIRQA
To view, visit change 49408. To unsubscribe, or for help writing mail filters, visit settings.