Re: [PATCH V16 7/7] PCI: Add quirk for multifunction devices of LS7A

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2022/7/16 下午2:12, Jianmin Lv wrote:


On 2022/7/16 上午11:23, Bjorn Helgaas wrote:
On Sat, Jul 16, 2022 at 10:27:00AM +0800, Jianmin Lv wrote:
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.

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.

OK, that makes a lot more sense, thank you!

But it does leave another question: the quirk applies to
DEV_PCIE_PORT_0 (0x7a09), DEV_PCIE_PORT_1 (0x7a19), and
DEV_PCIE_PORT_2 (0x7a29).

According to the .dtsi [1], all those root ports are at function 0,
and if they report INTA, the quirk will also compute INTA.  So why do
you need to apply the quirk for them?


Oh, yes, I don't think they are required either. The fix is only required for multi-func devices of 7A.

Huacai, we should remove PCIE ports from the patch.

The same would apply to any Device ID that only appears at function 0,
which looks like it also includes DEV_LS7A_OHCI (0x7a24), and
DEV_LS7A_GPU (0x7a15).


Same thing, they are also not required.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/boot/dts/loongson/ls7a-pch.dtsi?id=v5.18#n231







[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux