| Message ID | 1465892645-32381-2-git-send-email-po.liu@nxp.com |
|---|---|
| State | Changes Requested |
| Headers | show |
在 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 <po.liu@nxp.com> > --- > 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 <linux/sched.h> > #include <linux/ktime.h> > #include <linux/mm.h> > +#include <linux/of_irq.h> > #include <asm/dma.h> /* 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); > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Dongdong, > -----Original Message----- > From: Dongdong Liu [mailto:liudongdong3@huawei.com] > Sent: Thursday, June 23, 2016 1:44 PM > To: Po Liu; linux-pci@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > devicetree@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 <po.liu@nxp.com> > > --- > > 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 <linux/sched.h> > > #include <linux/ktime.h> > > #include <linux/mm.h> > > +#include <linux/of_irq.h> > > #include <asm/dma.h> /* 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); > >
Hi Po I found a problem with the similar patch. as the below log. [ 4.287060] pci 0000:80:00.0: quirk_aer_interrupt dev->irq 416 [ 4.293778] pcieport 0000:80:00.0: pci_device_probe in [ 4.299605] pcieport 0000:80:00.0: of_irq_parse_pci() failed with rc=-22 [ 4.307209] pcieport 0000: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 <po.liu@nxp.com> > --- > 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 <linux/sched.h> > #include <linux/ktime.h> > #include <linux/mm.h> > +#include <linux/of_irq.h> > #include <asm/dma.h> /* 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); > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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:liudongdong3@huawei.com] > Sent: Monday, July 04, 2016 4:44 PM > To: Po Liu; linux-pci@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > devicetree@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 0000:80:00.0: quirk_aer_interrupt dev->irq 416 > [ 4.293778] pcieport 0000:80:00.0: pci_device_probe in > [ 4.299605] pcieport 0000:80:00.0: of_irq_parse_pci() failed with > rc=-22 > [ 4.307209] pcieport 0000: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 <po.liu@nxp.com> > > --- > > 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 <linux/sched.h> > > #include <linux/ktime.h> > > #include <linux/mm.h> > > +#include <linux/of_irq.h> > > #include <asm/dma.h> /* 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); > > >
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 <po.liu@nxp.com> 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 <linux/sched.h> > #include <linux/ktime.h> > #include <linux/mm.h> > +#include <linux/of_irq.h> > #include <asm/dma.h> /* 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-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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:helgaas@kernel.org] > Sent: Saturday, July 30, 2016 6:42 AM > To: Po Liu > Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; devicetree@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 <po.liu@nxp.com> > > 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 <linux/sched.h> > > #include <linux/ktime.h> > > #include <linux/mm.h> > > +#include <linux/of_irq.h> > > #include <asm/dma.h> /* 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-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Aug 22, 2016 at 10:09:18AM +0000, 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:helgaas@kernel.org] > > Sent: Saturday, July 30, 2016 6:42 AM > > To: Po Liu > > Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > > linux-kernel@vger.kernel.org; devicetree@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 <po.liu@nxp.com> > > > > 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 <linux/sched.h> > > > #include <linux/ktime.h> > > > #include <linux/mm.h> > > > +#include <linux/of_irq.h> > > > #include <asm/dma.h> /* 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-kernel@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn, > -----Original Message----- > From: Bjorn Helgaas [mailto:helgaas@kernel.org] > Sent: Wednesday, September 21, 2016 4:47 AM > To: Po Liu > Cc: Roy Zang; Arnd Bergmann; devicetree@vger.kernel.org; Marc Zyngier; > linux-pci@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-kernel@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 +0000, 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:helgaas@kernel.org] > > > Sent: Saturday, July 30, 2016 6:42 AM > > > To: Po Liu > > > Cc: linux-pci@vger.kernel.org; > > > linux-arm-kernel@lists.infradead.org; > > > linux-kernel@vger.kernel.org; devicetree@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 <po.liu@nxp.com> > > > > > > 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 > > > <linux/sched.h> > #include <linux/ktime.h> > #include > > > <linux/mm.h> > +#include <linux/of_irq.h> > > > > #include <asm/dma.h> /* 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-kernel@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Sep 21, 2016 at 06:51:55AM +0000, Po Liu wrote: > Hi Bjorn, > > > -----Original Message----- > > From: Bjorn Helgaas [mailto:helgaas@kernel.org] > > Sent: Wednesday, September 21, 2016 4:47 AM > > To: Po Liu > > Cc: Roy Zang; Arnd Bergmann; devicetree@vger.kernel.org; Marc Zyngier; > > linux-pci@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-kernel@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 +0000, 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 to the patch itself with more ideas. > > > > -----Original Message----- > > > > From: Bjorn Helgaas [mailto:helgaas@kernel.org] > > > > Sent: Saturday, July 30, 2016 6:42 AM > > > > To: Po Liu > > > > Cc: linux-pci@vger.kernel.org; > > > > linux-arm-kernel@lists.infradead.org; > > > > linux-kernel@vger.kernel.org; devicetree@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 <po.liu@nxp.com> > > > > > > > > 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 > > > > <linux/sched.h> > #include <linux/ktime.h> > #include > > > > <linux/mm.h> > +#include <linux/of_irq.h> > > > > > #include <asm/dma.h> /* 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-kernel@lists.infradead.org > > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <linux/sched.h> #include <linux/ktime.h> #include <linux/mm.h> +#include <linux/of_irq.h> #include <asm/dma.h> /* 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);
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 <po.liu@nxp.com> --- changes for V3: - Move to quirk; - Only correct the irq in RC mode; drivers/pci/quirks.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)