On 2022/7/16 上午12:37, Bjorn Helgaas wrote:
On Fri, Jul 15, 2022 at 04:05:12PM +0800, Jianmin Lv wrote:
On 2022/7/15 上午11:44, Bjorn Helgaas wrote:
On Thu, Jul 14, 2022 at 08:42:16PM +0800, Huacai Chen wrote:
From: Jianmin Lv <lvjianmin@xxxxxxxxxxx>
In LS7A, multifunction device use same PCI PIN (because the PIN register
report the same INTx value to each function) but we need different IRQ
for different functions, so add a quirk to fix it for standard PCI PIN
usage.
This patch only affect ACPI based systems (and only needed by ACPI based
systems, too). For DT based systems, the irq mappings is defined in .dts
files and be handled by of_irq_parse_pci().
I'm sorry, I know you've explained this before, but I don't understand
yet, so let's try again. I *think* you're saying that:
- These devices integrated into LS7A all report 0 in their Interrupt
Pin registers. Per spec, this means they do not use INTx (PCIe
r6.0, sec 7.5.1.1.13).
- However, these devices actually *do* use INTx. Function 0 uses
INTA, function 1 uses INTB, ..., function 4 uses INTA, ...
- The quirk overrides the incorrect values read from the Interrupt
Pin registers.
Yes, right.
Sorry, I didn't see the first item here carefully, so I have to correct
it: all the integrated devices in 7A report 1 in PIN reg instead of 0.
That much makes sense to me.
And I even see that in of_irq_parse_pci(), if there's a DT node for
the device, of_irq_parse_one() gets the interrupt info from DT and
returns the IRQ all the way back up to (I think) loongson_map_irq().
Agree, I think so for DT.
But I'm still confused about how loongson_map_irq() gets called. The
only likely path I see is here:
pci_device_probe # pci_bus_type.probe
pci_assign_irq
pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin)
if (pin)
bridge->swizzle_irq(dev, &pin)
irq = bridge->map_irq(dev, slot, pin)
where bridge->map_irq points to loongson_map_irq(). But
pci_assign_irq() should read 0 from PCI_INTERRUPT_PIN [1], so it
wouldn't call bridge->map_irq(). Obviously I'm missing something.
Same thing, PCI_INTERRUPT_PIN reports 1, so bridge->map_irq() will be
called.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/setup-irq.c?id=v5.18#n37
For ACPI, bridge->map_irq is NULL, so in above path,
the pci_assign_irq will return because of !(hbrg->map_irq) as following:
if (!(hbrg->map_irq)) {
pci_dbg(dev, "runtime IRQ mapping not provided by arch\n");
return;
}
And again as I explained in previous version patch, dev->irq is set in
acpi_pci_irq_enable() in the following path for ACPI:
pci_device_probe
->pcibios_alloc_irq
->acpi_pci_irq_enable
->acpi_pci_irq_lookup
And the reason that we fixed the pin is to get an correct entry in prt
table when calling acpi_pci_irq_lookup. With out the fix, we can't find
out a entry.
After found an entry, we get gsi, and map irq as following:
rc = acpi_register_gsi(&dev->dev, gsi, triggering, polarity);
if (rc < 0) {
dev_warn(&dev->dev, "PCI INT %c: failed to register GSI\n",
pin_name(pin));
kfree(entry);
return rc;
}
dev->irq = rc;
Here, dev->irq is set like in pci_assign_irq for DT.
Yes. The above explains how things work for ACPI, but I'm not asking
about that.
I'm asking how this works in the *DT* case. I see that
pci_assign_irq() is called for both ACPI and DT, and I see that it
does nothing in the ACPI path because bridge->map_irq hasn't been set.
What I *don't* see is how pci_assign_irq() works in the DT case
because it reads PCI_INTERRUPT_PIN, which should return 0 for these
broken devices, and if "pin == 0", it never calls ->map_irq().
Is ->map_irq() called via some other path?
Same as above.
Bjorn
