On Mon, Mar 12, 2018 at 07:18:30PM -0500, Bjorn Helgaas wrote:
> [+cc Rafael, linux-acpi]
>
> On Sat, Mar 10, 2018 at 10:58:12AM +0800, Dongdong Liu wrote:
> > The init value of dev->irq is from PCI_INTERRUPT_LINE register.The default
> > value of dev->irq usually is 255 before calling acpi_pci_irq_enable().
> > This will cause a problem When the platform does not support INTx well.
> > For example, we do not config _PRT on our HIP07 platform, we met irq 255
> > confilict.
>
> s/confilict/conflict/
>
> arm64 calls acpi_pci_irq_enable() in the probe() path instead of in
> the pci_enable_device() path. I can't remember the reason, and
> unfortunately we didn't add a comment explaining why arm64 has to be
> different.
>
> PCI_INTERRUPT_LINE is used only by software and has no effect on the
> hardware. If it contains 255 when Linux boots, that means firmware
> must have set it to that value.
>
> We need some comment here about why we should always ignore what
> firmware put there on arm64. It needs to explain why this is
> different on arm64 than it is on x86 and ia64. The changelog above
> basically says "we need this to make it work", but that's not very
> informative or convincing.
>
> It sounds like you're saying HIP07 doesn't provide _PRT? Per the PCI
> Firmware Spec, r3.0, sec 4.4, _PRT is required under all PCI host
> bridges. So I think you're saying HIP07 doesn't comply with the spec,
> but the workaround here applies to *all* arm64 platforms. That
> doesn't seem right.
>
> On x86, I think if the _PRT doesn't tell us where an interrupt pin is
> routed, we fall back to looking at PCI_INTERRUPT_LINE. But here
> you seem to be saying "there's no _PRT" *and* "there's no useful
> information in PCI_INTERRUPT_LINE".
I don't think arm64 is doing anything different from x86/ia64 except
that we route the IRQ at device probe time (it was done to route IRQ at
the same boot stage in DT and ACPI but I then removed the DT path
with the pci_fixup_irqs() removal series - moving the IRQ routing at
pcibios_enable_device() time may well break platforms, x86 has even
a boot parameter to prevent that :) - ie pci=routeirq).
Regardless, we are not going to patch the kernel to work around
a specific platform firmware bug - a _PRT object should be added.
Lorenzo
> Maybe I'm not understanding what you're saying.
>
> > The error message is as below.
> > snd_hda_intel 000d:33:00.1: PCI INT B: no GSI
> > snd_hda_intel 000d:33:00.1: enabling device (0000 -> 0002)
> > snd_hda_intel 000d:33:00.1: Force to snoop mode by module option
> > snd_hda_intel 000d:33:00.1: Device has broken 64-bit MSI but arch tried to
> > assign one above 4G
> > genirq: Flags mismatch irq 255. 00000001 (enahisic2i0-tx1) vs. 00000081
> > (snd_hda_intel:card0)
> > hns-nic HISI00C2:00 enahisic2i0: request irq(255) fail
> >
> > enahisic2i0-tx1: this device is a platform device.
> > snd_hda_intel:card0: this device is a PCI device.
> >
> > Signed-off-by: Dongdong Liu <liudongdong3@xxxxxxxxxx>
> > ---
> > arch/arm64/kernel/pci.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > index 0e2ea1c..6a77bef 100644
> > --- a/arch/arm64/kernel/pci.c
> > +++ b/arch/arm64/kernel/pci.c
> > @@ -28,8 +28,11 @@
> > */
> > int pcibios_alloc_irq(struct pci_dev *dev)
> > {
> > - if (!acpi_disabled)
> > + if (!acpi_disabled) {
> > + dev->irq = 0;
> > acpi_pci_irq_enable(dev);
> > + }
> > +
> >
> > return 0;
> > }
> > --
> > 1.9.1
> >
