| Message ID | 1394025951-32438-3-git-send-email-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/05/2014 06:25 AM, Lucas Stach wrote: > This is the recommended method of doing the IRQ > mapping. For old devicetrees we fall back to the > previous practice. Tested-by: Stephen Warren <swarren@nvidia.com> I tested both with and without patch 1/6, and the PCIe-based NIC on Beaver worked fine either way. Without patch 1/6, I do see: pci 0000:00:01.0: of_irq_parse_pci() failed with rc=-22 ... but that seems reasonable given that the DT that of_irq_parse_pci() parses is missing, and did correctly trigger the fallback path, so everything still worked.
Am Donnerstag, den 06.03.2014, 10:36 -0700 schrieb Stephen Warren: > On 03/05/2014 06:25 AM, Lucas Stach wrote: > > This is the recommended method of doing the IRQ > > mapping. For old devicetrees we fall back to the > > previous practice. > > Tested-by: Stephen Warren <swarren@nvidia.com> > > I tested both with and without patch 1/6, and the PCIe-based NIC on > Beaver worked fine either way. Without patch 1/6, I do see: > > pci 0000:00:01.0: of_irq_parse_pci() failed with rc=-22 > > ... but that seems reasonable given that the DT that of_irq_parse_pci() > parses is missing, and did correctly trigger the fallback path, so > everything still worked. Yes, this should be normal. It spits this error for old DTs, but keeps doing the right thing. I'm not sure if we should downgrade this to info or dbg. Regards, Lucas
On Thursday 06 March 2014, Lucas Stach wrote: > Am Donnerstag, den 06.03.2014, 10:36 -0700 schrieb Stephen Warren: > > On 03/05/2014 06:25 AM, Lucas Stach wrote: > > > This is the recommended method of doing the IRQ > > > mapping. For old devicetrees we fall back to the > > > previous practice. > > > > Tested-by: Stephen Warren <swarren@nvidia.com> > > > > I tested both with and without patch 1/6, and the PCIe-based NIC on > > Beaver worked fine either way. Without patch 1/6, I do see: > > > > pci 0000:00:01.0: of_irq_parse_pci() failed with rc=-22 > > > > ... but that seems reasonable given that the DT that of_irq_parse_pci() > > parses is missing, and did correctly trigger the fallback path, so > > everything still worked. > > Yes, this should be normal. It spits this error for old DTs, but keeps > doing the right thing. I'm not sure if we should downgrade this to info > or dbg. No, I think printing an error like this is appropriate: it is an incentive to update the dts files, but doesn't look too urgent as long as everything still works. Arnd
On Friday, March 07, 2014 9:25 AM, Arnd Bergmann wrote: > On Thursday 06 March 2014, Lucas Stach wrote: > > Am Donnerstag, den 06.03.2014, 10:36 -0700 schrieb Stephen Warren: > > > On 03/05/2014 06:25 AM, Lucas Stach wrote: > > > > This is the recommended method of doing the IRQ > > > > mapping. For old devicetrees we fall back to the > > > > previous practice. > > > > > > Tested-by: Stephen Warren <swarren@nvidia.com> > > > > > > I tested both with and without patch 1/6, and the PCIe-based NIC on > > > Beaver worked fine either way. Without patch 1/6, I do see: > > > > > > pci 0000:00:01.0: of_irq_parse_pci() failed with rc=-22 > > > > > > ... but that seems reasonable given that the DT that of_irq_parse_pci() > > > parses is missing, and did correctly trigger the fallback path, so > > > everything still worked. > > > > Yes, this should be normal. It spits this error for old DTs, but keeps > > doing the right thing. I'm not sure if we should downgrade this to info > > or dbg. > > No, I think printing an error like this is appropriate: it is an incentive > to update the dts files, but doesn't look too urgent as long as everything > still works. +1 I agree with Arnd's opinion. I think that all dts files should be updated properly. This error message can be the good way to do it. Thank you. Best regards, Jingoo Han
On Wed, Mar 05, 2014 at 02:25:47PM +0100, Lucas Stach wrote: > This is the recommended method of doing the IRQ > mapping. For old devicetrees we fall back to the > previous practice. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > Acked-by: Arnd Bergmann <arnd@arndb.de> Applied with Stephen's Tested-by to my pending/host-tegra branch. I'll rebase and rename it after v3.15-rc1, and I think we can squeeze it into v3.15 shortly after that. Thanks. > --- > drivers/pci/host/pci-tegra.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c > index 330f7e3a32dd..083cf37ca047 100644 > --- a/drivers/pci/host/pci-tegra.c > +++ b/drivers/pci/host/pci-tegra.c > @@ -639,10 +639,15 @@ static int tegra_pcie_setup(int nr, struct pci_sys_data *sys) > static int tegra_pcie_map_irq(const struct pci_dev *pdev, u8 slot, u8 pin) > { > struct tegra_pcie *pcie = sys_to_pcie(pdev->bus->sysdata); > + int irq; > > tegra_cpuidle_pcie_irqs_in_use(); > > - return pcie->irq; > + irq = of_irq_parse_and_map_pci(pdev, slot, pin); > + if (!irq) > + irq = pcie->irq; > + > + return irq; > } > > static void tegra_pcie_add_bus(struct pci_bus *bus) > -- > 1.9.0 > > -- > 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 Lucas/Tanmay, On Thu, Mar 6, 2014 at 11:09 PM, Lucas Stach <l.stach@pengutronix.de> wrote: > Am Donnerstag, den 06.03.2014, 10:36 -0700 schrieb Stephen Warren: >> On 03/05/2014 06:25 AM, Lucas Stach wrote: >> > This is the recommended method of doing the IRQ >> > mapping. For old devicetrees we fall back to the >> > previous practice. >> >> Tested-by: Stephen Warren <swarren@nvidia.com> >> >> I tested both with and without patch 1/6, and the PCIe-based NIC on >> Beaver worked fine either way. Without patch 1/6, I do see: >> >> pci 0000:00:01.0: of_irq_parse_pci() failed with rc=-22 >> >> ... but that seems reasonable given that the DT that of_irq_parse_pci() >> parses is missing, and did correctly trigger the fallback path, so >> everything still worked. > > Yes, this should be normal. It spits this error for old DTs, but keeps > doing the right thing. I'm not sure if we should downgrade this to info > or dbg. I see this error too on my setup (Xilinx PCIe Root Complex Driver), https://lkml.org/lkml/2014/3/3/183 After digging into it, I see I need to override the API pcibios_get_phb_of_node() to pass my device node. Is this the only possible way? From the discussion in this thread, I feel am missing something in the dt node. Below is my dt node, please let me know if you see any issues. +++++++++++++++++++++++ pcie{ reg = <XXXX>; ranges = <YYYY>; interrupts = <0 52 4>; pcie_intc: interrupt-controller { interrupt-controller; #address-cells = <0>; #interrupt-cells = <1>; } interrupt-map-mask = <0 0 0 7>; interrupt-map = <0 0 0 1 &pcie_intc 1>, <0 0 0 2 &pcie_intc 2>, <0 0 0 3 &pcie_intc 3>, <0 0 0 4 &pcie_intc 4>; }; Thanks in advance, Srikanth. > > Regards, > Lucas > -- > Pengutronix e.K. | Lucas Stach | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > -- > 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
On Fri, Apr 11, 2014 at 11:10:59PM +0530, Srikanth Thokala wrote: > I see this error too on my setup (Xilinx PCIe Root Complex Driver), > https://lkml.org/lkml/2014/3/3/183 > After digging into it, I see I need to override the API > pcibios_get_phb_of_node() No, as I pointed out before, the DT node comes in through pci_scan_root_bus: +static struct pci_bus __init *xilinx_pcie_scan_bus(int nr, + struct pci_sys_data *sys) +{ + struct xilinx_pcie_port *port = sys_to_pcie(sys); + struct pci_bus *bus; + + if (port) { + port->root_busno = sys->busnr; + bus = pci_scan_root_bus(NULL, sys->busnr, &xilinx_pcie_ops, ^^^^^^ You can't pass NULL here and have DT work properly. See http://www.spinics.net/lists/arm-kernel/msg312392.html Jason
On Sat, Apr 12, 2014 at 2:11 AM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Fri, Apr 11, 2014 at 11:10:59PM +0530, Srikanth Thokala wrote: > >> I see this error too on my setup (Xilinx PCIe Root Complex Driver), >> https://lkml.org/lkml/2014/3/3/183 > >> After digging into it, I see I need to override the API >> pcibios_get_phb_of_node() > > No, as I pointed out before, the DT node comes in through > pci_scan_root_bus: Thanks Jason for the advice, it is working. Srikanth > > +static struct pci_bus __init *xilinx_pcie_scan_bus(int nr, > + struct pci_sys_data *sys) > +{ > + struct xilinx_pcie_port *port = sys_to_pcie(sys); > + struct pci_bus *bus; > + > + if (port) { > + port->root_busno = sys->busnr; > + bus = pci_scan_root_bus(NULL, sys->busnr, &xilinx_pcie_ops, > ^^^^^^ > > You can't pass NULL here and have DT work properly. > > See http://www.spinics.net/lists/arm-kernel/msg312392.html > > Jason
Hi Bjorn, Am Freitag, den 04.04.2014, 10:55 -0600 schrieb Bjorn Helgaas: > On Wed, Mar 05, 2014 at 02:25:47PM +0100, Lucas Stach wrote: > > This is the recommended method of doing the IRQ > > mapping. For old devicetrees we fall back to the > > previous practice. > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > Applied with Stephen's Tested-by to my pending/host-tegra branch. I'll > rebase and rename it after v3.15-rc1, and I think we can squeeze it into > v3.15 shortly after that. Thanks. > Are you still planning to push this into 3.15, or has this slipped to 3.16? Regards, Lucas
On Tue, Apr 15, 2014 at 12:07:34PM +0200, Lucas Stach wrote: > Hi Bjorn, > > Am Freitag, den 04.04.2014, 10:55 -0600 schrieb Bjorn Helgaas: > > On Wed, Mar 05, 2014 at 02:25:47PM +0100, Lucas Stach wrote: > > > This is the recommended method of doing the IRQ > > > mapping. For old devicetrees we fall back to the > > > previous practice. > > > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > Applied with Stephen's Tested-by to my pending/host-tegra branch. I'll > > rebase and rename it after v3.15-rc1, and I think we can squeeze it into > > v3.15 shortly after that. Thanks. > > > > Are you still planning to push this into 3.15, or has this slipped to > 3.16? Yes, I'm hoping to put them in v3.15. I assume these actually fix something, e.g., we need these changes to boot with new devicetrees, or something? The changelogs don't make it clear that these are fixes, and I want to heed Linus' guidance: "Anyway, because -rc1 is already pretty darn big, I do *not* want to hear about 'sorry this missed the window, can I still sneak in'. Fixes only." I should have applied these sooner to make the merge window; I apologize for that. Anyway, if you outline what these fix, I'll update the changelogs in my tree. Bjorn
Am Dienstag, den 15.04.2014, 12:30 -0600 schrieb Bjorn Helgaas: > On Tue, Apr 15, 2014 at 12:07:34PM +0200, Lucas Stach wrote: > > Hi Bjorn, > > > > Am Freitag, den 04.04.2014, 10:55 -0600 schrieb Bjorn Helgaas: > > > On Wed, Mar 05, 2014 at 02:25:47PM +0100, Lucas Stach wrote: > > > > This is the recommended method of doing the IRQ > > > > mapping. For old devicetrees we fall back to the > > > > previous practice. > > > > > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > > > Applied with Stephen's Tested-by to my pending/host-tegra branch. I'll > > > rebase and rename it after v3.15-rc1, and I think we can squeeze it into > > > v3.15 shortly after that. Thanks. > > > > > > > Are you still planning to push this into 3.15, or has this slipped to > > 3.16? > > Yes, I'm hoping to put them in v3.15. I assume these actually > fix something, e.g., we need these changes to boot with new devicetrees, or > something? > > The changelogs don't make it clear that these are fixes, and I want to heed > Linus' guidance: "Anyway, because -rc1 is already pretty darn big, I do > *not* want to hear about 'sorry this missed the window, can I still sneak > in'. Fixes only." > > I should have applied these sooner to make the merge window; I apologize > for that. Anyway, if you outline what these fix, I'll update the > changelogs in my tree. > Actually they are a bit on the fence. The i.MX and thus the designware patch actually fixes wrong behavior, where all PCI legacy interrupts would be mapped to a single GIC interrupt, which would leave INT B,C,D nonfunctional on i.MX. The others only make DT interrupt mapping functional for all drivers, so they would be useful if you need to remap interrupts across bridges or something. But apparently nobody had the need to to this on platforms other than i.MX until now, so those patches only fix a theoretical issue. Regards, Lucas
On Wed, Apr 16, 2014 at 10:20:45AM +0200, Lucas Stach wrote: > Am Dienstag, den 15.04.2014, 12:30 -0600 schrieb Bjorn Helgaas: > > On Tue, Apr 15, 2014 at 12:07:34PM +0200, Lucas Stach wrote: > > > Hi Bjorn, > > > > > > Am Freitag, den 04.04.2014, 10:55 -0600 schrieb Bjorn Helgaas: > > > > On Wed, Mar 05, 2014 at 02:25:47PM +0100, Lucas Stach wrote: > > > > > This is the recommended method of doing the IRQ > > > > > mapping. For old devicetrees we fall back to the > > > > > previous practice. > > > > > > > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > > > > > Applied with Stephen's Tested-by to my pending/host-tegra branch. I'll > > > > rebase and rename it after v3.15-rc1, and I think we can squeeze it into > > > > v3.15 shortly after that. Thanks. > > > > > > > > > > Are you still planning to push this into 3.15, or has this slipped to > > > 3.16? > > > > Yes, I'm hoping to put them in v3.15. I assume these actually > > fix something, e.g., we need these changes to boot with new devicetrees, or > > something? > > > > The changelogs don't make it clear that these are fixes, and I want to heed > > Linus' guidance: "Anyway, because -rc1 is already pretty darn big, I do > > *not* want to hear about 'sorry this missed the window, can I still sneak > > in'. Fixes only." > > > > I should have applied these sooner to make the merge window; I apologize > > for that. Anyway, if you outline what these fix, I'll update the > > changelogs in my tree. > > > Actually they are a bit on the fence. > > The i.MX and thus the designware patch actually fixes wrong behavior, > where all PCI legacy interrupts would be mapped to a single GIC > interrupt, which would leave INT B,C,D nonfunctional on i.MX. > > The others only make DT interrupt mapping functional for all drivers, so > they would be useful if you need to remap interrupts across bridges or > something. But apparently nobody had the need to to this on platforms > other than i.MX until now, so those patches only fix a theoretical > issue. It sounds like the others should fix real problems; it's just that nobody has actually tested relevant configurations yet. I think that's fair game, so I updated the changelogs and put them in my for-linus branch for v3.15. This includes the designware, rcar, and tegra patches. Bjorn
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c index 330f7e3a32dd..083cf37ca047 100644 --- a/drivers/pci/host/pci-tegra.c +++ b/drivers/pci/host/pci-tegra.c @@ -639,10 +639,15 @@ static int tegra_pcie_setup(int nr, struct pci_sys_data *sys) static int tegra_pcie_map_irq(const struct pci_dev *pdev, u8 slot, u8 pin) { struct tegra_pcie *pcie = sys_to_pcie(pdev->bus->sysdata); + int irq; tegra_cpuidle_pcie_irqs_in_use(); - return pcie->irq; + irq = of_irq_parse_and_map_pci(pdev, slot, pin); + if (!irq) + irq = pcie->irq; + + return irq; } static void tegra_pcie_add_bus(struct pci_bus *bus)