Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
On Wed, Sep 21, 2016 at 06:51:55AM +, Po Liu wrote:
> Hi Bjorn,
>
> > -Original Message-
> > From: Bjorn Helgaas [mailto:helg...@kernel.org]
> > Sent: Wednesday, September 21, 2016 4:47 AM
> > To: Po Liu
> > Cc: Roy Zang; Arnd Bergmann; devicet...@vger.kernel.org; Marc Zyngier;
> > linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Stuart Yoder;
> > M.H. Lian; Murali Karicheri; Mingkai Hu; Bjorn Helgaas; Leo Li; Shawn
> > Guo; linux-arm-ker...@lists.infradead.org
> > Subject: Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
> >
> > On Mon, Aug 22, 2016 at 10:09:18AM +, Po Liu wrote:
> > > Hi Bjorn,
> > >
> > > Sorry for late reply.
> > >
> > > I checked the updated kernel with Dongdong mentioned ACPI patch which
> > was truly affected my quirk patch uploaded. So I suppose the quirk patch
> > is not qualify to fix the bug.
> >
> > I don't understand what you're saying here.
> >
> > The quirk worked on your machine. It apparently didn't work on
> > Dongdong's machine because of_irq_parse_and_map_pci() is run after the
> > quirk in this path:
> >
> >pci_device_probe
> > pcibios_alloc_irq # arm64
> >dev->irq = of_irq_parse_and_map_pci
> >
> > and of_irq_parse_and_map_pci() returned zero, probably because
> > of_irq_parse_pci() failed. My guess is that the reason it works on your
> > machine but not Dongdong's is that your DTs are different such that
> > of_irq_parse_pci() works for you but not for Dongdong.
> >
> > I think the idea of of_irq_parse_and_map_pci() is to set up a device's
> > INTx line. But that doesn't quite apply here because your device
> > doesn't actually *use* INTx. So I don't know why of_irq_parse_pci()
> > works for you. Maybe that's a symptom of a problem in your DT.
> >
> > Or maybe you're saying that the quirk *didn't* work on your machine when
> > you tested it in a kernel that included d8ed75d59332 ("ARM64:
> > PCI: ACPI support for legacy IRQs parsing and consolidation with DT
> > code").
>
> Yes, this point is what I mean. After this patch my quirk patch would not
> work.
> Since I discussed with Dongdong, the patches d8ed75d59332 ACPI related were
> not be merged yet.
>
>
> > But that doesn't make sense either, because prior to
> > d8ed75d59332, we *always* set
> >
> >dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> >
> > and after the patch we only do it if "acpi_disabled". I guess I just
> > don't understand what you're saying.
>
> Before the patch merged. pcibios_add_device()(which run the ->irq =
> of_irq_parse_and_map_pci(dev, 0, 0);) was loaded before the
> pci_fixup_device(pci_fixup_final). But after the patch
> d8ed75d59332("ARM64: PCI: ACPI support for legacy IRQs parsing and
> consolidation with DT code") merged, the
> pci_fixup_device(pci_fixup_final) run BEFORE the
> pcibios_alloc_irq()(which run the ->irq =
> of_irq_parse_and_map_pci(dev, 0, 0);). So the dev->irq were
> overwhelm by the pcibios_alloc_irq().
OK. Prior to d8ed75d59332, arm64 overrode the default empty
pcibios_add_device() implementation, and called
of_irq_parse_and_map_pci() there. d8ed75d59332 changed that function
to pcibios_alloc_irq(), which is called later, in the driver probe
path.
> When I test, the acpi_disabled is '1' although my kernel config
> default is CONFIG_ACPI=y. And no setting in the uboot with apci=xxx.
> But this is another issue, I didn't deep to check it.
Likely your platform just doesn't have ACPI or something's wrong in
the initial ACPI setup.
> > > I were keep thinking what your "explicitly checking for a root port
> > device" meaning. Do you mean I should upload again the first version
> > patch which fix it in the portdrv_core.c ? I would upload again if yes.
> >
> > No, I did not mean you should go back to the first version of the patch.
> > If we *can* do this in a quirk, I think that would be much better than
> > doing it in the PCIe port driver. I meant that Dongdong's suggestion of
> > adding this:
> >
> >if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
> > return;
> >
> > to your quirk made sense to me.
>
> If the quirk patch could make workaround. It should be the better way.
It doesn't sound like a quirk is going to work because all the quirks
run too early. I'll respond
RE: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
Hi Bjorn,
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: Wednesday, September 21, 2016 4:47 AM
> To: Po Liu
> Cc: Roy Zang; Arnd Bergmann; devicet...@vger.kernel.org; Marc Zyngier;
> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Stuart Yoder;
> M.H. Lian; Murali Karicheri; Mingkai Hu; Bjorn Helgaas; Leo Li; Shawn
> Guo; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
>
> On Mon, Aug 22, 2016 at 10:09:18AM +, Po Liu wrote:
> > Hi Bjorn,
> >
> > Sorry for late reply.
> >
> > I checked the updated kernel with Dongdong mentioned ACPI patch which
> was truly affected my quirk patch uploaded. So I suppose the quirk patch
> is not qualify to fix the bug.
>
> I don't understand what you're saying here.
>
> The quirk worked on your machine. It apparently didn't work on
> Dongdong's machine because of_irq_parse_and_map_pci() is run after the
> quirk in this path:
>
>pci_device_probe
> pcibios_alloc_irq # arm64
>dev->irq = of_irq_parse_and_map_pci
>
> and of_irq_parse_and_map_pci() returned zero, probably because
> of_irq_parse_pci() failed. My guess is that the reason it works on your
> machine but not Dongdong's is that your DTs are different such that
> of_irq_parse_pci() works for you but not for Dongdong.
>
> I think the idea of of_irq_parse_and_map_pci() is to set up a device's
> INTx line. But that doesn't quite apply here because your device
> doesn't actually *use* INTx. So I don't know why of_irq_parse_pci()
> works for you. Maybe that's a symptom of a problem in your DT.
>
> Or maybe you're saying that the quirk *didn't* work on your machine when
> you tested it in a kernel that included d8ed75d59332 ("ARM64:
> PCI: ACPI support for legacy IRQs parsing and consolidation with DT
> code").
Yes, this point is what I mean. After this patch my quirk patch would not work.
Since I discussed with Dongdong, the patches d8ed75d59332 ACPI related were not
be merged yet.
> But that doesn't make sense either, because prior to
> d8ed75d59332, we *always* set
>
>dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
>
> and after the patch we only do it if "acpi_disabled". I guess I just
> don't understand what you're saying.
Before the patch merged. pcibios_add_device()(which run the ->irq =
of_irq_parse_and_map_pci(dev, 0, 0);) was loaded before the
pci_fixup_device(pci_fixup_final).
But after the patch d8ed75d59332("ARM64: PCI: ACPI support for legacy IRQs
parsing and consolidation with DT code") merged, the
pci_fixup_device(pci_fixup_final) run BEFORE the pcibios_alloc_irq()(which run
the ->irq = of_irq_parse_and_map_pci(dev, 0, 0);). So the dev->irq were
overwhelm by the pcibios_alloc_irq().
When I test, the acpi_disabled is '1' although my kernel config default is
CONFIG_ACPI=y. And no setting in the uboot with apci=xxx. But this is another
issue, I didn't deep to check it.
>
> > I were keep thinking what your "explicitly checking for a root port
> device" meaning. Do you mean I should upload again the first version
> patch which fix it in the portdrv_core.c ? I would upload again if yes.
>
> No, I did not mean you should go back to the first version of the patch.
> If we *can* do this in a quirk, I think that would be much better than
> doing it in the PCIe port driver. I meant that Dongdong's suggestion of
> adding this:
>
>if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
> return;
>
> to your quirk made sense to me.
If the quirk patch could make workaround. It should be the better way.
Po Liu
>
> > > -Original Message-
> > > From: Bjorn Helgaas [mailto:helg...@kernel.org]
> > > Sent: Saturday, July 30, 2016 6:42 AM
> > > To: Po Liu
> > > Cc: linux-...@vger.kernel.org;
> > > linux-arm-ker...@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; devicet...@vger.kernel.org; Roy Zang;
> > > Arnd Bergmann; Marc Zyngier; Stuart Yoder; Yang-Leo Li; Minghuan
> > > Lian; Murali Karicheri; Bjorn Helgaas; Shawn Guo; Mingkai Hu
> > > Subject: Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
> > >
> > > On Tue, Jun 14, 2016 at 04:24:05PM +0800, Po Liu wrote:
> > > > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC
> mode.
> > > > When chip support the aer interrupt with none MSI/MSI-X/INTx
> >
Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
On Mon, Aug 22, 2016 at 10:09:18AM +, Po Liu wrote:
> Hi Bjorn,
>
> Sorry for late reply.
>
> I checked the updated kernel with Dongdong mentioned ACPI patch which was
> truly affected my quirk patch uploaded. So I suppose the quirk patch is not
> qualify to fix the bug.
I don't understand what you're saying here.
The quirk worked on your machine. It apparently didn't work on
Dongdong's machine because of_irq_parse_and_map_pci() is run after the
quirk in this path:
pci_device_probe
pcibios_alloc_irq # arm64
dev->irq = of_irq_parse_and_map_pci
and of_irq_parse_and_map_pci() returned zero, probably because
of_irq_parse_pci() failed. My guess is that the reason it works on
your machine but not Dongdong's is that your DTs are different such
that of_irq_parse_pci() works for you but not for Dongdong.
I think the idea of of_irq_parse_and_map_pci() is to set up a device's
INTx line. But that doesn't quite apply here because your device
doesn't actually *use* INTx. So I don't know why of_irq_parse_pci()
works for you. Maybe that's a symptom of a problem in your DT.
Or maybe you're saying that the quirk *didn't* work on your machine
when you tested it in a kernel that included d8ed75d59332 ("ARM64:
PCI: ACPI support for legacy IRQs parsing and consolidation with DT
code"). But that doesn't make sense either, because prior to
d8ed75d59332, we *always* set
dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
and after the patch we only do it if "acpi_disabled". I guess I just
don't understand what you're saying.
> I were keep thinking what your "explicitly checking for a root port device"
> meaning. Do you mean I should upload again the first version patch which fix
> it in the portdrv_core.c ? I would upload again if yes.
No, I did not mean you should go back to the first version of the
patch. If we *can* do this in a quirk, I think that would be much
better than doing it in the PCIe port driver. I meant that Dongdong's
suggestion of adding this:
if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
return;
to your quirk made sense to me.
> > -Original Message-
> > From: Bjorn Helgaas [mailto:helg...@kernel.org]
> > Sent: Saturday, July 30, 2016 6:42 AM
> > To: Po Liu
> > Cc: linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> > linux-kernel@vger.kernel.org; devicet...@vger.kernel.org; Roy Zang; Arnd
> > Bergmann; Marc Zyngier; Stuart Yoder; Yang-Leo Li; Minghuan Lian; Murali
> > Karicheri; Bjorn Helgaas; Shawn Guo; Mingkai Hu
> > Subject: Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
> >
> > On Tue, Jun 14, 2016 at 04:24:05PM +0800, Po Liu wrote:
> > > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
> > > When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
> > > maybe there is interrupt line for aer pme etc. Search the interrupt
> > > number in the fdt file. Then fixup the dev->irq with it.
> > >
> > > Signed-off-by: Po Liu
> >
> > I'm not sure where we're at with this. Dongdong had some issue
> > (possibly with a version of the quirk on a different platform?), and I
> > think the suggestion of explicitly checking for a root port device was a
> > good one.
> >
> > So please update and repost this for next cycle.
> >
> > > ---
> > > changes for V3:
> > > - Move to quirk;
> > > - Only correct the irq in RC mode;
> > >
> > > drivers/pci/quirks.c | 29 +
> > > 1 file changed, 29 insertions(+)
> > >
> > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > > ee72ebe..8b39cce 100644
> > > --- a/drivers/pci/quirks.c
> > > +++ b/drivers/pci/quirks.c
> > > @@ -25,6 +25,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include /* isa_dma_bridge_buggy */
> > > #include "pci.h"
> > >
> > > @@ -4419,3 +4420,31 @@ static void quirk_intel_qat_vf_cap(struct
> > pci_dev *pdev)
> > > }
> > > }
> > > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443,
> > > quirk_intel_qat_vf_cap);
> > > +
> > > +/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
> > > + * but use standalone irq. Read the device tree for the aer
> > > + * interrupt number.
> > > + */
> > > +static void quirk_aer_interrupt(struct pci_dev *dev) {
> &g
RE: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
Hi Bjorn,
Sorry for late reply.
I checked the updated kernel with Dongdong mentioned ACPI patch which was truly
affected my quirk patch uploaded. So I suppose the quirk patch is not qualify
to fix the bug.
I were keep thinking what your "explicitly checking for a root port device"
meaning. Do you mean I should upload again the first version patch which fix it
in the portdrv_core.c ? I would upload again if yes.
Thanks!
Best regards,
Liu Po
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: Saturday, July 30, 2016 6:42 AM
> To: Po Liu
> Cc: linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-kernel@vger.kernel.org; devicet...@vger.kernel.org; Roy Zang; Arnd
> Bergmann; Marc Zyngier; Stuart Yoder; Yang-Leo Li; Minghuan Lian; Murali
> Karicheri; Bjorn Helgaas; Shawn Guo; Mingkai Hu
> Subject: Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
>
> On Tue, Jun 14, 2016 at 04:24:05PM +0800, Po Liu wrote:
> > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
> > When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
> > maybe there is interrupt line for aer pme etc. Search the interrupt
> > number in the fdt file. Then fixup the dev->irq with it.
> >
> > Signed-off-by: Po Liu
>
> I'm not sure where we're at with this. Dongdong had some issue
> (possibly with a version of the quirk on a different platform?), and I
> think the suggestion of explicitly checking for a root port device was a
> good one.
>
> So please update and repost this for next cycle.
>
> > ---
> > changes for V3:
> >- Move to quirk;
> >- Only correct the irq in RC mode;
> >
> > drivers/pci/quirks.c | 29 +
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > ee72ebe..8b39cce 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -25,6 +25,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include /* isa_dma_bridge_buggy */
> > #include "pci.h"
> >
> > @@ -4419,3 +4420,31 @@ static void quirk_intel_qat_vf_cap(struct
> pci_dev *pdev)
> >}
> > }
> > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443,
> > quirk_intel_qat_vf_cap);
> > +
> > +/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
> > + * but use standalone irq. Read the device tree for the aer
> > + * interrupt number.
> > + */
> > +static void quirk_aer_interrupt(struct pci_dev *dev) {
> > + int ret;
> > + u8 header_type;
> > + struct device_node *np = NULL;
> > +
> > + /* Only for the RC mode device */
> > + pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type);
> > + if ((header_type & 0x7F) != PCI_HEADER_TYPE_BRIDGE)
> > + return;
> > +
> > + if (dev->bus->dev.of_node)
> > + np = dev->bus->dev.of_node;
> > +
> > + if (IS_ENABLED(CONFIG_OF_IRQ) && np) {
> > + ret = of_irq_get_byname(np, "aer");
> > + if (ret > 0) {
> > + dev->no_msi = 1;
> > + dev->irq = ret;
> > + }
> > + }
> > +}
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> > +quirk_aer_interrupt);
> > --
> > 2.1.0.27.g96db324
> >
> >
> > ___
> > linux-arm-kernel mailing list
> > linux-arm-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
On Tue, Jun 14, 2016 at 04:24:05PM +0800, Po Liu wrote:
> On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
> When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
> maybe there is interrupt line for aer pme etc. Search the interrupt
> number in the fdt file. Then fixup the dev->irq with it.
>
> Signed-off-by: Po Liu
I'm not sure where we're at with this. Dongdong had some issue
(possibly with a version of the quirk on a different platform?), and I
think the suggestion of explicitly checking for a root port device was
a good one.
So please update and repost this for next cycle.
> ---
> changes for V3:
> - Move to quirk;
> - Only correct the irq in RC mode;
>
> drivers/pci/quirks.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index ee72ebe..8b39cce 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
> #include /* isa_dma_bridge_buggy */
> #include "pci.h"
>
> @@ -4419,3 +4420,31 @@ static void quirk_intel_qat_vf_cap(struct pci_dev
> *pdev)
> }
> }
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap);
> +
> +/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
> + * but use standalone irq. Read the device tree for the aer
> + * interrupt number.
> + */
> +static void quirk_aer_interrupt(struct pci_dev *dev)
> +{
> + int ret;
> + u8 header_type;
> + struct device_node *np = NULL;
> +
> + /* Only for the RC mode device */
> + pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type);
> + if ((header_type & 0x7F) != PCI_HEADER_TYPE_BRIDGE)
> + return;
> +
> + if (dev->bus->dev.of_node)
> + np = dev->bus->dev.of_node;
> +
> + if (IS_ENABLED(CONFIG_OF_IRQ) && np) {
> + ret = of_irq_get_byname(np, "aer");
> + if (ret > 0) {
> + dev->no_msi = 1;
> + dev->irq = ret;
> + }
> + }
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> quirk_aer_interrupt);
> --
> 2.1.0.27.g96db324
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
Hi Po
在 2016/7/5 11:03, Po Liu 写道:
Hi Dongdong,
The patch were intend to fixup the NXP layerscape serial SOC and were tested ok.
I am not clear what platform are you trying to fix.
My platform is an ARM64 platform, PCIe host controller also use Synopsys
Designware.
The problem on your board may be as below comments:
-Original Message-
From: Dongdong Liu [mailto:liudongdo...@huawei.com]
Sent: Monday, July 04, 2016 4:44 PM
To: Po Liu; linux-...@vger.kernel.org; linux-arm-
ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
devicet...@vger.kernel.org
Cc: Bjorn Helgaas; Shawn Guo; Marc Zyngier; Rob Herring; Roy Zang;
Mingkai Hu; Stuart Yoder; Yang-Leo Li; Arnd Bergmann; Minghuan Lian;
Murali Karicheri; Linuxarm
Subject: Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
Hi Po
I found a problem with the similar patch. as the below log.
[4.287060] pci :80:00.0: quirk_aer_interrupt dev->irq 416
[4.293778] pcieport :80:00.0: pci_device_probe in
[4.299605] pcieport :80:00.0: of_irq_parse_pci() failed with
rc=-22
[4.307209] pcieport :80:00.0: init_service_irqs dev->irq 0
The fucntions are called as below sequence.
1. quirk_aer_interrupt, get the aer dev->irq 416.
This code quirk_aer_interrupt() should be run at
pci_fixup_device(pci_fixup_final) which is in the pci_bus_add_devices()
Yes, you are right.
2. pci_device_probe->of_irq_parse_pci, of_irq_parse_pci() failed, then
dev->irq changed to 0.
pci_device_probe->of_irq_parse_pci which in the pci_scan_child_bus() run before
pci_bus_add_devices(). See dw_pcie_host_init().
Apparently , your quirk_aer_interrupt() is running before the dev->irq
assignment in the of_irq_parse_pci().
So make sure your configure the quirk_aer_interrupt() run in the FINAL stage in
the quirk.c OR check your host driver which you are using.
Yes , It is FINAL stage in the quirk. I use DECLARE_PCI_FIXUP_FINAL.
I find it is the below patch affect this.
(https://patchwork.kernel.org/patch/9170333/),
but the patch will be applied to linux 4.8. So the problem will also be existed.
ARM64: PCI: ACPI support for legacy IRQs parsing and consolidation with DT code
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index d5d3d26..b3b8a2c 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -51,11 +51,16 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
}
/*
- * Try to assign the IRQ number from DT when adding a new device
+ * Try to assign the IRQ number when probing a new device
*/
-int pcibios_add_device(struct pci_dev *dev)
+int pcibios_alloc_irq(struct pci_dev *dev)
{
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
+ if (acpi_disabled)
+ dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
+#ifdef CONFIG_ACPI
+ else
+ return acpi_pci_irq_enable(dev);
+#endif
return 0;
}
Thanks
Dongdong
So this patch could not work with aer.
Thanks
Dongdong
在 2016/6/14 16:24, Po Liu 写道:
> On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
> When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
> maybe there is interrupt line for aer pme etc. Search the interrupt
> number in the fdt file. Then fixup the dev->irq with it.
>
> Signed-off-by: Po Liu
> ---
> changes for V3:
> - Move to quirk;
> - Only correct the irq in RC mode;
>
> drivers/pci/quirks.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> ee72ebe..8b39cce 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
> #include /* isa_dma_bridge_buggy */
> #include "pci.h"
>
> @@ -4419,3 +4420,31 @@ static void quirk_intel_qat_vf_cap(struct
pci_dev *pdev)
> }
> }
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443,
> quirk_intel_qat_vf_cap);
> +
> +/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
> + * but use standalone irq. Read the device tree for the aer
> + * interrupt number.
> + */
> +static void quirk_aer_interrupt(struct pci_dev *dev) {
> +int ret;
> +u8 header_type;
> +struct device_node *np = NULL;
> +
> +/* Only for the RC mode device */
> +pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type);
> +if ((header_type & 0x7F) != PCI_HEADER_TYPE_BRIDGE)
> +return;
> +
> +if (dev->bus->dev.of_node)
> +np = dev->bus->dev.of_node;
> +
> +i
RE: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
Hi Dongdong,
The patch were intend to fixup the NXP layerscape serial SOC and were tested ok.
I am not clear what platform are you trying to fix.
The problem on your board may be as below comments:
> -Original Message-
> From: Dongdong Liu [mailto:liudongdo...@huawei.com]
> Sent: Monday, July 04, 2016 4:44 PM
> To: Po Liu; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicet...@vger.kernel.org
> Cc: Bjorn Helgaas; Shawn Guo; Marc Zyngier; Rob Herring; Roy Zang;
> Mingkai Hu; Stuart Yoder; Yang-Leo Li; Arnd Bergmann; Minghuan Lian;
> Murali Karicheri; Linuxarm
> Subject: Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
>
> Hi Po
>
> I found a problem with the similar patch. as the below log.
>
> [4.287060] pci :80:00.0: quirk_aer_interrupt dev->irq 416
> [4.293778] pcieport :80:00.0: pci_device_probe in
> [4.299605] pcieport :80:00.0: of_irq_parse_pci() failed with
> rc=-22
> [4.307209] pcieport :80:00.0: init_service_irqs dev->irq 0
>
> The fucntions are called as below sequence.
> 1. quirk_aer_interrupt, get the aer dev->irq 416.
This code quirk_aer_interrupt() should be run at
pci_fixup_device(pci_fixup_final) which is in the pci_bus_add_devices()
> 2. pci_device_probe->of_irq_parse_pci, of_irq_parse_pci() failed, then
> dev->irq changed to 0.
pci_device_probe->of_irq_parse_pci which in the pci_scan_child_bus() run before
pci_bus_add_devices(). See dw_pcie_host_init().
Apparently , your quirk_aer_interrupt() is running before the dev->irq
assignment in the of_irq_parse_pci().
So make sure your configure the quirk_aer_interrupt() run in the FINAL stage in
the quirk.c OR check your host driver which you are using.
>
> So this patch could not work with aer.
>
> Thanks
> Dongdong
> 在 2016/6/14 16:24, Po Liu 写道:
> > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
> > When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
> > maybe there is interrupt line for aer pme etc. Search the interrupt
> > number in the fdt file. Then fixup the dev->irq with it.
> >
> > Signed-off-by: Po Liu
> > ---
> > changes for V3:
> >- Move to quirk;
> >- Only correct the irq in RC mode;
> >
> > drivers/pci/quirks.c | 29 +
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > ee72ebe..8b39cce 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -25,6 +25,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include /* isa_dma_bridge_buggy */
> > #include "pci.h"
> >
> > @@ -4419,3 +4420,31 @@ static void quirk_intel_qat_vf_cap(struct
> pci_dev *pdev)
> >}
> > }
> > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443,
> > quirk_intel_qat_vf_cap);
> > +
> > +/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
> > + * but use standalone irq. Read the device tree for the aer
> > + * interrupt number.
> > + */
> > +static void quirk_aer_interrupt(struct pci_dev *dev) {
> > + int ret;
> > + u8 header_type;
> > + struct device_node *np = NULL;
> > +
> > + /* Only for the RC mode device */
> > + pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type);
> > + if ((header_type & 0x7F) != PCI_HEADER_TYPE_BRIDGE)
> > + return;
> > +
> > + if (dev->bus->dev.of_node)
> > + np = dev->bus->dev.of_node;
> > +
> > + if (IS_ENABLED(CONFIG_OF_IRQ) && np) {
> > + ret = of_irq_get_byname(np, "aer");
> > + if (ret > 0) {
> > + dev->no_msi = 1;
> > + dev->irq = ret;
> > + }
> > + }
> > +}
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> > +quirk_aer_interrupt);
> >
>
Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
Hi Po
I found a problem with the similar patch. as the below log.
[4.287060] pci :80:00.0: quirk_aer_interrupt dev->irq 416
[4.293778] pcieport :80:00.0: pci_device_probe in
[4.299605] pcieport :80:00.0: of_irq_parse_pci() failed with rc=-22
[4.307209] pcieport :80:00.0: init_service_irqs dev->irq 0
The fucntions are called as below sequence.
1. quirk_aer_interrupt, get the aer dev->irq 416.
2. pci_device_probe->of_irq_parse_pci, of_irq_parse_pci() failed, then dev->irq
changed to 0.
So this patch could not work with aer.
Thanks
Dongdong
在 2016/6/14 16:24, Po Liu 写道:
On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
maybe there is interrupt line for aer pme etc. Search the interrupt
number in the fdt file. Then fixup the dev->irq with it.
Signed-off-by: Po Liu
---
changes for V3:
- Move to quirk;
- Only correct the irq in RC mode;
drivers/pci/quirks.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index ee72ebe..8b39cce 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include /* isa_dma_bridge_buggy */
#include "pci.h"
@@ -4419,3 +4420,31 @@ static void quirk_intel_qat_vf_cap(struct pci_dev *pdev)
}
}
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap);
+
+/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
+ * but use standalone irq. Read the device tree for the aer
+ * interrupt number.
+ */
+static void quirk_aer_interrupt(struct pci_dev *dev)
+{
+ int ret;
+ u8 header_type;
+ struct device_node *np = NULL;
+
+ /* Only for the RC mode device */
+ pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type);
+ if ((header_type & 0x7F) != PCI_HEADER_TYPE_BRIDGE)
+ return;
+
+ if (dev->bus->dev.of_node)
+ np = dev->bus->dev.of_node;
+
+ if (IS_ENABLED(CONFIG_OF_IRQ) && np) {
+ ret = of_irq_get_byname(np, "aer");
+ if (ret > 0) {
+ dev->no_msi = 1;
+ dev->irq = ret;
+ }
+ }
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
quirk_aer_interrupt);
RE: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
Hi Dongdong,
> -Original Message-
> From: Dongdong Liu [mailto:liudongdo...@huawei.com]
> Sent: Thursday, June 23, 2016 1:44 PM
> To: Po Liu; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicet...@vger.kernel.org
> Cc: Bjorn Helgaas; Shawn Guo; Marc Zyngier; Rob Herring; Roy Zang;
> Mingkai Hu; Stuart Yoder; Yang-Leo Li; Arnd Bergmann; Minghuan Lian;
> Murali Karicheri
> Subject: Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
>
>
>
> 在 2016/6/14 16:24, Po Liu 写道:
> > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
> > When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
> > maybe there is interrupt line for aer pme etc. Search the interrupt
> > number in the fdt file. Then fixup the dev->irq with it.
> >
> > Signed-off-by: Po Liu
> > ---
> > changes for V3:
> >- Move to quirk;
> >- Only correct the irq in RC mode;
> >
> > drivers/pci/quirks.c | 29 +
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > ee72ebe..8b39cce 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -25,6 +25,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include /* isa_dma_bridge_buggy */
> > #include "pci.h"
> >
> > @@ -4419,3 +4420,31 @@ static void quirk_intel_qat_vf_cap(struct
> pci_dev *pdev)
> >}
> > }
> > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443,
> > quirk_intel_qat_vf_cap);
> > +
> > +/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
> > + * but use standalone irq. Read the device tree for the aer
> > + * interrupt number.
> > + */
> > +static void quirk_aer_interrupt(struct pci_dev *dev) {
> > + int ret;
> > + u8 header_type;
> > + struct device_node *np = NULL;
> > +
> > + /* Only for the RC mode device */
> > + pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type);
> > + if ((header_type & 0x7F) != PCI_HEADER_TYPE_BRIDGE)
> > + return;
>
> How about that it is changed as below.
>
> /* Only for the RC mode device */
> if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
> return;
>
> Dongdong
> Thanks
Yes, it is also ok to read the capability register.
We see it is common used to read the header type register that is why we use in
this way.
> > +
> > + if (dev->bus->dev.of_node)
> > + np = dev->bus->dev.of_node;
> > +
> > + if (IS_ENABLED(CONFIG_OF_IRQ) && np) {
> > + ret = of_irq_get_byname(np, "aer");
> > + if (ret > 0) {
> > + dev->no_msi = 1;
> > + dev->irq = ret;
> > + }
> > + }
> > +}
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> > +quirk_aer_interrupt);
> >
Re: [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk
在 2016/6/14 16:24, Po Liu 写道:
On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
maybe there is interrupt line for aer pme etc. Search the interrupt
number in the fdt file. Then fixup the dev->irq with it.
Signed-off-by: Po Liu
---
changes for V3:
- Move to quirk;
- Only correct the irq in RC mode;
drivers/pci/quirks.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index ee72ebe..8b39cce 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include /* isa_dma_bridge_buggy */
#include "pci.h"
@@ -4419,3 +4420,31 @@ static void quirk_intel_qat_vf_cap(struct pci_dev *pdev)
}
}
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap);
+
+/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
+ * but use standalone irq. Read the device tree for the aer
+ * interrupt number.
+ */
+static void quirk_aer_interrupt(struct pci_dev *dev)
+{
+ int ret;
+ u8 header_type;
+ struct device_node *np = NULL;
+
+ /* Only for the RC mode device */
+ pci_read_config_byte(dev, PCI_HEADER_TYPE, &header_type);
+ if ((header_type & 0x7F) != PCI_HEADER_TYPE_BRIDGE)
+ return;
How about that it is changed as below.
/* Only for the RC mode device */
if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
return;
Dongdong
Thanks
+
+ if (dev->bus->dev.of_node)
+ np = dev->bus->dev.of_node;
+
+ if (IS_ENABLED(CONFIG_OF_IRQ) && np) {
+ ret = of_irq_get_byname(np, "aer");
+ if (ret > 0) {
+ dev->no_msi = 1;
+ dev->irq = ret;
+ }
+ }
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
quirk_aer_interrupt);
