diff mbox series

legacy PCI device behind a bridge not getting a valid IRQ on imx host controller

Message ID CAJ+vNU2YVpQ=csr-O65L_pcNFWbFMvHK4XO44cbfUfPKwuw6vg@mail.gmail.com (mailing list archive)
State Not Applicable
Delegated to: Krzysztof Wilczyński
Headers show
Series legacy PCI device behind a bridge not getting a valid IRQ on imx host controller | expand

Commit Message

Tim Harvey Aug. 28, 2024, 9:40 p.m. UTC
Greetings,

I have a user that is using an IMX8MM SoC (dwc controller) with a
miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
device and the device is not getting a valid interrupt.

The PCI bus looks like this:
00.00.0: 16c3:abcd (rev 01)
01:00.0: 10b5:8112
^^^ PEX8112 x1 Lane PCI bridge
02:00.0: 4ddc:1a00
02:01.0: 4ddc:1a00
^^^ PCI devices

lspci -vvv -s 02:00.0:
02:00.0 Communication controller: ILC Data Device Corp Device 1a00 (rev 10)
        Subsystem: ILC Data Device Corp Device 1a00
        Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap- 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium
>TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Interrupt: pin A routed to IRQ 0
        Region 0: Memory at 18100000 (32-bit, non-prefetchable)
[disabled] [size=256K]
        Region 1: Memory at 18180000 (32-bit, non-prefetchable)
[disabled] [size=4K]
^^^ 'Interrupt: pin A routed to IRQ 0' is wrong

I found an old thread from 2019 on an NVidia forum [1] where the same
thing occurred and Nvidia's solution was a patch to the dwc driver to
call pci_fixup_irqs():
Since that time the pci/dwc drivers have changed quite a bit;
pci_fixup_irqs() was changed to pci_assign_irq() called now from
pcie_device_probe() and dw_pcie_host_init() calls commit init
functions.

While I don't have the particular card in hand described above yet to
test with, I did manage to reproduce this on an imx6dl soc (same dwc
controller and driver) connected to a TI XIO2001 with an Intel I210
behind it and see the exact same issue.

Does anyone understand why legacy PCI interrupt mapping behind a
bridge isn't working here?

Best regards,

Tim
[1] https://forums.developer.nvidia.com/t/xavier-not-routing-pci-interrupts-across-pex8112-bridge/78556

Comments

Bjorn Helgaas Aug. 29, 2024, 9:22 p.m. UTC | #1
[+cc Richard, Lucas, maintainers of IMX6 PCI]

On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> Greetings,
> 
> I have a user that is using an IMX8MM SoC (dwc controller) with a
> miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> device and the device is not getting a valid interrupt.

Does pci-imx6.c support INTx at all?

I see that drivers/pci/controller/dwc/pci-imx6.c supports both host
and endpoint modes, but the only mention of "intx" is for an IMX
device in endpoint mode to raise an INTx interrupt.

A few DWC-based drivers look like they support INTx:

  dra7xx_pcie_init_irq_domain
  ks_pcie_config_intx_irq
  rockchip_pcie_init_irq_domain (the dwc/pcie-dw-rockchip.c one)
  uniphier_pcie_config_intx_irq

but most (including pci-imx6.c) don't have anything that looks like
those.

> The PCI bus looks like this:
> 00.00.0: 16c3:abcd (rev 01)
> 01:00.0: 10b5:8112
> ^^^ PEX8112 x1 Lane PCI bridge
> 02:00.0: 4ddc:1a00
> 02:01.0: 4ddc:1a00
> ^^^ PCI devices
> 
> lspci -vvv -s 02:00.0:
> 02:00.0 Communication controller: ILC Data Device Corp Device 1a00 (rev 10)
>         Subsystem: ILC Data Device Corp Device 1a00
>         Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop-
> ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap- 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium
> >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Interrupt: pin A routed to IRQ 0
>         Region 0: Memory at 18100000 (32-bit, non-prefetchable)
> [disabled] [size=256K]
>         Region 1: Memory at 18180000 (32-bit, non-prefetchable)
> [disabled] [size=4K]
> ^^^ 'Interrupt: pin A routed to IRQ 0' is wrong
> 
> I found an old thread from 2019 on an NVidia forum [1] where the same
> thing occurred and Nvidia's solution was a patch to the dwc driver to
> call pci_fixup_irqs():
> diff --git a/drivers/pci/dwc/pcie-designware-host.c
> b/drivers/pci/dwc/pcie-designware-host.c
> index ec2e4a61aa4e..a72ba177a5fd 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -477,6 +477,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
>         if (pp->ops->scan_bus)
>                 pp->ops->scan_bus(pp);
> 
> +       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> +
>         pci_bus_size_bridges(bus);
>         pci_bus_assign_resources(bus);
> 
> Since that time the pci/dwc drivers have changed quite a bit;
> pci_fixup_irqs() was changed to pci_assign_irq() called now from
> pcie_device_probe() and dw_pcie_host_init() calls commit init
> functions.
> 
> While I don't have the particular card in hand described above yet to
> test with, I did manage to reproduce this on an imx6dl soc (same dwc
> controller and driver) connected to a TI XIO2001 with an Intel I210
> behind it and see the exact same issue.
> 
> Does anyone understand why legacy PCI interrupt mapping behind a
> bridge isn't working here?
> 
> Best regards,
> 
> Tim
> [1] https://forums.developer.nvidia.com/t/xavier-not-routing-pci-interrupts-across-pex8112-bridge/78556
Frank Li Aug. 29, 2024, 9:43 p.m. UTC | #2
On Thu, Aug 29, 2024 at 04:22:35PM -0500, Bjorn Helgaas wrote:
> [+cc Richard, Lucas, maintainers of IMX6 PCI]
>
> On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > Greetings,
> >
> > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > device and the device is not getting a valid interrupt.
>
> Does pci-imx6.c support INTx at all?

Yes, dwc controller map INTx message to 4 irq lines, which connect to GIC.
we tested it by add nomsi in kernel command line.

>
> I see that drivers/pci/controller/dwc/pci-imx6.c supports both host
> and endpoint modes, but the only mention of "intx" is for an IMX
> device in endpoint mode to raise an INTx interrupt.
>
> A few DWC-based drivers look like they support INTx:
>
>   dra7xx_pcie_init_irq_domain
>   ks_pcie_config_intx_irq
>   rockchip_pcie_init_irq_domain (the dwc/pcie-dw-rockchip.c one)
>   uniphier_pcie_config_intx_irq
>
> but most (including pci-imx6.c) don't have anything that looks like
> those.
>
> > The PCI bus looks like this:
> > 00.00.0: 16c3:abcd (rev 01)
> > 01:00.0: 10b5:8112
> > ^^^ PEX8112 x1 Lane PCI bridge
> > 02:00.0: 4ddc:1a00
> > 02:01.0: 4ddc:1a00
> > ^^^ PCI devices
> >
> > lspci -vvv -s 02:00.0:
> > 02:00.0 Communication controller: ILC Data Device Corp Device 1a00 (rev 10)
> >         Subsystem: ILC Data Device Corp Device 1a00
> >         Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop-
> > ParErr- Stepping- SERR- FastB2B- DisINTx-
> >         Status: Cap- 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium
> > >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> >         Interrupt: pin A routed to IRQ 0
> >         Region 0: Memory at 18100000 (32-bit, non-prefetchable)
> > [disabled] [size=256K]
> >         Region 1: Memory at 18180000 (32-bit, non-prefetchable)
> > [disabled] [size=4K]
> > ^^^ 'Interrupt: pin A routed to IRQ 0' is wrong

look like bridge route PCI bus INTA to msi. I remember msi irq0 is reserved.
Do you have more information about bridge's MSI informaiton by lspci?

Frank

> >
> > I found an old thread from 2019 on an NVidia forum [1] where the same
> > thing occurred and Nvidia's solution was a patch to the dwc driver to
> > call pci_fixup_irqs():
> > diff --git a/drivers/pci/dwc/pcie-designware-host.c
> > b/drivers/pci/dwc/pcie-designware-host.c
> > index ec2e4a61aa4e..a72ba177a5fd 100644
> > --- a/drivers/pci/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/dwc/pcie-designware-host.c
> > @@ -477,6 +477,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
> >         if (pp->ops->scan_bus)
> >                 pp->ops->scan_bus(pp);
> >
> > +       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> > +
> >         pci_bus_size_bridges(bus);
> >         pci_bus_assign_resources(bus);
> >
> > Since that time the pci/dwc drivers have changed quite a bit;
> > pci_fixup_irqs() was changed to pci_assign_irq() called now from
> > pcie_device_probe() and dw_pcie_host_init() calls commit init
> > functions.
> >
> > While I don't have the particular card in hand described above yet to
> > test with, I did manage to reproduce this on an imx6dl soc (same dwc
> > controller and driver) connected to a TI XIO2001 with an Intel I210
> > behind it and see the exact same issue.
> >
> > Does anyone understand why legacy PCI interrupt mapping behind a
> > bridge isn't working here?
> >
> > Best regards,
> >
> > Tim
> > [1] https://forums.developer.nvidia.com/t/xavier-not-routing-pci-interrupts-across-pex8112-bridge/78556
Bjorn Helgaas Aug. 29, 2024, 10 p.m. UTC | #3
On Thu, Aug 29, 2024 at 05:43:42PM -0400, Frank Li wrote:
> On Thu, Aug 29, 2024 at 04:22:35PM -0500, Bjorn Helgaas wrote:
> > On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > > Greetings,
> > >
> > > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > > device and the device is not getting a valid interrupt.
> >
> > Does pci-imx6.c support INTx at all?
> 
> Yes, dwc controller map INTx message to 4 irq lines, which connect to GIC.
> we tested it by add nomsi in kernel command line.

Thanks, Frank.  Can you point me to the dwc code where this happens?
Maybe I can remember this for next time or add a comment to help
people find it.

Bjorn
Tim Harvey Aug. 29, 2024, 10:21 p.m. UTC | #4
On Thu, Aug 29, 2024 at 3:00 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Thu, Aug 29, 2024 at 05:43:42PM -0400, Frank Li wrote:
> > On Thu, Aug 29, 2024 at 04:22:35PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > > > Greetings,
> > > >
> > > > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > > > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > > > device and the device is not getting a valid interrupt.
> > >
> > > Does pci-imx6.c support INTx at all?
> >
> > Yes, dwc controller map INTx message to 4 irq lines, which connect to GIC.
> > we tested it by add nomsi in kernel command line.
>
> Thanks, Frank.  Can you point me to the dwc code where this happens?
> Maybe I can remember this for next time or add a comment to help
> people find it.
>
> Bjorn

Bjorn and Frank,

I provided some misinformation regarding IMX6 - my original testing
was flawed. When testing the IMX6DL linked to a XIO2001 with a legacy
PCI device behind it the device driver is given an appropriate legacy
IRQ.

Regarding IMX8MM linked to a PEX8112 bridge with a legacy PCI device
behind it; the user gets a -28 (ENOSPC) when requesting an IRQ from
the driver but I don't have the hardware or the driver for that in
hand so I need to wait for more details. I don't see any reason why
this case would not work as it works on an IMX6DL.

Best Regards,

Tim
Richard Zhu Aug. 30, 2024, 1:50 a.m. UTC | #5
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2024年8月30日 5:23
> To: tharvey@gateworks.com; Hongxing Zhu <hongxing.zhu@nxp.com>; Lucas
> Stach <l.stach@pengutronix.de>
> Cc: linux-pci@vger.kernel.org
> Subject: Re: legacy PCI device behind a bridge not getting a valid IRQ on imx
> host controller
> 
> [+cc Richard, Lucas, maintainers of IMX6 PCI]
> 
> On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > Greetings,
> >
> > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > device and the device is not getting a valid interrupt.
> 
> Does pci-imx6.c support INTx at all?
> 
i.MX PCIe RC supports INTx.
Add pci=nomsi into kernel command line, can verify it when one endpoint
 device is connected.
Based i.MX8MM EVK board and on NVME, MSI or INTx are enabled.
logs of MSI:
root@imx8_all:~# lspci
00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01)
01:00.0 Non-Volatile memory controller: Device 1e49:0021 (rev 01)
root@imx8_all:~# cat /proc/interrupts | grep MSI
221:          0          0          0          0   PCI-MSI   0 Edge      PCIe PME
222:         14          0          0          0   PCI-MSI 524288 Edge      nvme0q0
223:        382          0          0          0   PCI-MSI 524289 Edge      nvme0q1
224:        115          0          0          0   PCI-MSI 524290 Edge      nvme0q2
225:        521          0          0          0   PCI-MSI 524291 Edge      nvme0q3
226:         53          0          0          0   PCI-MSI 524292 Edge      nvme0q4

Logs of INTx after pci=nomsi is added into kernel command line:
root@imx8_all:~# lspci
00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01)
01:00.0 Non-Volatile memory controller: Device 1e49:0021 (rev 01)
root@imx8_all:~#  cat /proc/interrupts | grep nvme
219:       1225          0          0          0     GICv3 157 Level     PCIe PME, nvme0q0, nvme0q1

Best Regards
Richard Zhu
> I see that drivers/pci/controller/dwc/pci-imx6.c supports both host and
> endpoint modes, but the only mention of "intx" is for an IMX device in
> endpoint mode to raise an INTx interrupt.
> 
> A few DWC-based drivers look like they support INTx:
> 
>   dra7xx_pcie_init_irq_domain
>   ks_pcie_config_intx_irq
>   rockchip_pcie_init_irq_domain (the dwc/pcie-dw-rockchip.c one)
>   uniphier_pcie_config_intx_irq
> 
> but most (including pci-imx6.c) don't have anything that looks like those.
> 
> > The PCI bus looks like this:
> > 00.00.0: 16c3:abcd (rev 01)
> > 01:00.0: 10b5:8112
> > ^^^ PEX8112 x1 Lane PCI bridge
> > 02:00.0: 4ddc:1a00
> > 02:01.0: 4ddc:1a00
> > ^^^ PCI devices
> >
> > lspci -vvv -s 02:00.0:
> > 02:00.0 Communication controller: ILC Data Device Corp Device 1a00 (rev
> 10)
> >         Subsystem: ILC Data Device Corp Device 1a00
> >         Control: I/O- Mem- BusMaster- SpecCycle- MemWINV-
> VGASnoop-
> > ParErr- Stepping- SERR- FastB2B- DisINTx-
> >         Status: Cap- 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium
> > >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> >         Interrupt: pin A routed to IRQ 0
> >         Region 0: Memory at 18100000 (32-bit, non-prefetchable)
> > [disabled] [size=256K]
> >         Region 1: Memory at 18180000 (32-bit, non-prefetchable)
> > [disabled] [size=4K] ^^^ 'Interrupt: pin A routed to IRQ 0' is wrong
> >
> > I found an old thread from 2019 on an NVidia forum [1] where the same
> > thing occurred and Nvidia's solution was a patch to the dwc driver to
> > call pci_fixup_irqs():
> > diff --git a/drivers/pci/dwc/pcie-designware-host.c
> > b/drivers/pci/dwc/pcie-designware-host.c
> > index ec2e4a61aa4e..a72ba177a5fd 100644
> > --- a/drivers/pci/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/dwc/pcie-designware-host.c
> > @@ -477,6 +477,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
> >         if (pp->ops->scan_bus)
> >                 pp->ops->scan_bus(pp);
> >
> > +       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> > +
> >         pci_bus_size_bridges(bus);
> >         pci_bus_assign_resources(bus);
> >
> > Since that time the pci/dwc drivers have changed quite a bit;
> > pci_fixup_irqs() was changed to pci_assign_irq() called now from
> > pcie_device_probe() and dw_pcie_host_init() calls commit init
> > functions.
> >
> > While I don't have the particular card in hand described above yet to
> > test with, I did manage to reproduce this on an imx6dl soc (same dwc
> > controller and driver) connected to a TI XIO2001 with an Intel I210
> > behind it and see the exact same issue.
> >
> > Does anyone understand why legacy PCI interrupt mapping behind a
> > bridge isn't working here?
> >
> > Best regards,
> >
> > Tim
> > [1]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fforu
> > ms.developer.nvidia.com%2Ft%2Fxavier-not-routing-pci-interrupts-across
> >
> -pex8112-bridge%2F78556&data=05%7C02%7Chongxing.zhu%40nxp.com%7
> Cfa1d98
> >
> fbdea045fb72f108dcc870b5b8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%
> >
> 7C638605633626084761%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQI
> >
> joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=ik
> 3qvBuPw
> > lj29q1BUUte%2F42SwSpxYnKGW9PvmFLFuaE%3D&reserved=0
Frank Li Aug. 30, 2024, 3:49 p.m. UTC | #6
On Thu, Aug 29, 2024 at 05:00:05PM -0500, Bjorn Helgaas wrote:
> On Thu, Aug 29, 2024 at 05:43:42PM -0400, Frank Li wrote:
> > On Thu, Aug 29, 2024 at 04:22:35PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > > > Greetings,
> > > >
> > > > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > > > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > > > device and the device is not getting a valid interrupt.
> > >
> > > Does pci-imx6.c support INTx at all?
> >
> > Yes, dwc controller map INTx message to 4 irq lines, which connect to GIC.
> > we tested it by add nomsi in kernel command line.
>
> Thanks, Frank.  Can you point me to the dwc code where this happens?
> Maybe I can remember this for next time or add a comment to help
> people find it.

I think it needn't special code to handle this. in dts

 interrupt-map = <0 0 0 1 &gic GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
                 <0 0 0 2 &gic GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
                 <0 0 0 3 &gic GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
                 <0 0 0 4 &gic GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;

It map INT(A,B,C,D) to 4 GIC irq.

Frank

>
> Bjorn
Frank Li Aug. 30, 2024, 3:52 p.m. UTC | #7
On Thu, Aug 29, 2024 at 03:21:13PM -0700, Tim Harvey wrote:
> On Thu, Aug 29, 2024 at 3:00 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Thu, Aug 29, 2024 at 05:43:42PM -0400, Frank Li wrote:
> > > On Thu, Aug 29, 2024 at 04:22:35PM -0500, Bjorn Helgaas wrote:
> > > > On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > > > > Greetings,
> > > > >
> > > > > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > > > > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > > > > device and the device is not getting a valid interrupt.
> > > >
> > > > Does pci-imx6.c support INTx at all?
> > >
> > > Yes, dwc controller map INTx message to 4 irq lines, which connect to GIC.
> > > we tested it by add nomsi in kernel command line.
> >
> > Thanks, Frank.  Can you point me to the dwc code where this happens?
> > Maybe I can remember this for next time or add a comment to help
> > people find it.
> >
> > Bjorn
>
> Bjorn and Frank,
>
> I provided some misinformation regarding IMX6 - my original testing
> was flawed. When testing the IMX6DL linked to a XIO2001 with a legacy
> PCI device behind it the device driver is given an appropriate legacy
> IRQ.
>
> Regarding IMX8MM linked to a PEX8112 bridge with a legacy PCI device
> behind it; the user gets a -28 (ENOSPC) when requesting an IRQ from
> the driver but I don't have the hardware or the driver for that in
> hand so I need to wait for more details. I don't see any reason why
> this case would not work as it works on an IMX6DL.

Do you have chance to try other arm64 platform? Maybe defconfig cause this
difference. CONFIG_PCIEPORTBUS=y is enabled default in arm64, but not
at arm32.

Frank

>
> Best Regards,
>
> Tim
Tim Harvey Aug. 30, 2024, 4:03 p.m. UTC | #8
On Thu, Aug 29, 2024 at 6:50 PM Hongxing Zhu <hongxing.zhu@nxp.com> wrote:
>
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: 2024年8月30日 5:23
> > To: tharvey@gateworks.com; Hongxing Zhu <hongxing.zhu@nxp.com>; Lucas
> > Stach <l.stach@pengutronix.de>
> > Cc: linux-pci@vger.kernel.org
> > Subject: Re: legacy PCI device behind a bridge not getting a valid IRQ on imx
> > host controller
> >
> > [+cc Richard, Lucas, maintainers of IMX6 PCI]
> >
> > On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > > Greetings,
> > >
> > > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > > device and the device is not getting a valid interrupt.
> >
> > Does pci-imx6.c support INTx at all?
> >
> i.MX PCIe RC supports INTx.
> Add pci=nomsi into kernel command line, can verify it when one endpoint
>  device is connected.
> Based i.MX8MM EVK board and on NVME, MSI or INTx are enabled.
> logs of MSI:
> root@imx8_all:~# lspci
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01)
> 01:00.0 Non-Volatile memory controller: Device 1e49:0021 (rev 01)
> root@imx8_all:~# cat /proc/interrupts | grep MSI
> 221:          0          0          0          0   PCI-MSI   0 Edge      PCIe PME
> 222:         14          0          0          0   PCI-MSI 524288 Edge      nvme0q0
> 223:        382          0          0          0   PCI-MSI 524289 Edge      nvme0q1
> 224:        115          0          0          0   PCI-MSI 524290 Edge      nvme0q2
> 225:        521          0          0          0   PCI-MSI 524291 Edge      nvme0q3
> 226:         53          0          0          0   PCI-MSI 524292 Edge      nvme0q4
>

Richard,

off topic but I've seen in the IMX8MMRM a claim that it supports MSI-X
but I have not seen this to be the case as you are showing above with
an nvme that would clearly use MSI-X if available.

Does the IMX8MM hardware support MSI-X?

Best Regards,

Tim
Manivannan Sadhasivam Aug. 30, 2024, 4:06 p.m. UTC | #9
On Fri, Aug 30, 2024 at 11:49:39AM -0400, Frank Li wrote:
> On Thu, Aug 29, 2024 at 05:00:05PM -0500, Bjorn Helgaas wrote:
> > On Thu, Aug 29, 2024 at 05:43:42PM -0400, Frank Li wrote:
> > > On Thu, Aug 29, 2024 at 04:22:35PM -0500, Bjorn Helgaas wrote:
> > > > On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > > > > Greetings,
> > > > >
> > > > > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > > > > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > > > > device and the device is not getting a valid interrupt.
> > > >
> > > > Does pci-imx6.c support INTx at all?
> > >
> > > Yes, dwc controller map INTx message to 4 irq lines, which connect to GIC.
> > > we tested it by add nomsi in kernel command line.
> >
> > Thanks, Frank.  Can you point me to the dwc code where this happens?
> > Maybe I can remember this for next time or add a comment to help
> > people find it.
> 
> I think it needn't special code to handle this. in dts
> 
>  interrupt-map = <0 0 0 1 &gic GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>                  <0 0 0 2 &gic GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
>                  <0 0 0 3 &gic GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>                  <0 0 0 4 &gic GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;
> 
> It map INT(A,B,C,D) to 4 GIC irq.
> 

Yeah, the mapping is handled in pci_host_bridge::map_irq() callback, which has a
default assignment, of_irq_parse_and_map_pci() parsing the 'interrupt-map'
property in DT and resolving the interrupt. So we do not need any special code
to handle INTx for RC.

- Mani
Tim Harvey Aug. 30, 2024, 4:11 p.m. UTC | #10
On Fri, Aug 30, 2024 at 8:52 AM Frank Li <Frank.li@nxp.com> wrote:
>
> On Thu, Aug 29, 2024 at 03:21:13PM -0700, Tim Harvey wrote:
> > On Thu, Aug 29, 2024 at 3:00 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > >
> > > On Thu, Aug 29, 2024 at 05:43:42PM -0400, Frank Li wrote:
> > > > On Thu, Aug 29, 2024 at 04:22:35PM -0500, Bjorn Helgaas wrote:
> > > > > On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > > > > > Greetings,
> > > > > >
> > > > > > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > > > > > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > > > > > device and the device is not getting a valid interrupt.
> > > > >
> > > > > Does pci-imx6.c support INTx at all?
> > > >
> > > > Yes, dwc controller map INTx message to 4 irq lines, which connect to GIC.
> > > > we tested it by add nomsi in kernel command line.
> > >
> > > Thanks, Frank.  Can you point me to the dwc code where this happens?
> > > Maybe I can remember this for next time or add a comment to help
> > > people find it.
> > >
> > > Bjorn
> >
> > Bjorn and Frank,
> >
> > I provided some misinformation regarding IMX6 - my original testing
> > was flawed. When testing the IMX6DL linked to a XIO2001 with a legacy
> > PCI device behind it the device driver is given an appropriate legacy
> > IRQ.
> >
> > Regarding IMX8MM linked to a PEX8112 bridge with a legacy PCI device
> > behind it; the user gets a -28 (ENOSPC) when requesting an IRQ from
> > the driver but I don't have the hardware or the driver for that in
> > hand so I need to wait for more details. I don't see any reason why
> > this case would not work as it works on an IMX6DL.
>
> Do you have chance to try other arm64 platform? Maybe defconfig cause this
> difference. CONFIG_PCIEPORTBUS=y is enabled default in arm64, but not
> at arm32.
>

Frank,

It's unclear what other platforms they have been able to run this on.
I'll have to wait for the hardware in the form of a miniPCIe card. The
card showing the issue on imx8mm is a DDC BU-67114 [1].

Best Regards,

Tim
[1]
Frank Li Aug. 30, 2024, 4:14 p.m. UTC | #11
On Fri, Aug 30, 2024 at 09:03:09AM -0700, Tim Harvey wrote:
> On Thu, Aug 29, 2024 at 6:50 PM Hongxing Zhu <hongxing.zhu@nxp.com> wrote:
> >
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > Sent: 2024年8月30日 5:23
> > > To: tharvey@gateworks.com; Hongxing Zhu <hongxing.zhu@nxp.com>; Lucas
> > > Stach <l.stach@pengutronix.de>
> > > Cc: linux-pci@vger.kernel.org
> > > Subject: Re: legacy PCI device behind a bridge not getting a valid IRQ on imx
> > > host controller
> > >
> > > [+cc Richard, Lucas, maintainers of IMX6 PCI]
> > >
> > > On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > > > Greetings,
> > > >
> > > > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > > > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > > > device and the device is not getting a valid interrupt.
> > >
> > > Does pci-imx6.c support INTx at all?
> > >
> > i.MX PCIe RC supports INTx.
> > Add pci=nomsi into kernel command line, can verify it when one endpoint
> >  device is connected.
> > Based i.MX8MM EVK board and on NVME, MSI or INTx are enabled.
> > logs of MSI:
> > root@imx8_all:~# lspci
> > 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01)
> > 01:00.0 Non-Volatile memory controller: Device 1e49:0021 (rev 01)
> > root@imx8_all:~# cat /proc/interrupts | grep MSI
> > 221:          0          0          0          0   PCI-MSI   0 Edge      PCIe PME
> > 222:         14          0          0          0   PCI-MSI 524288 Edge      nvme0q0
> > 223:        382          0          0          0   PCI-MSI 524289 Edge      nvme0q1
> > 224:        115          0          0          0   PCI-MSI 524290 Edge      nvme0q2
> > 225:        521          0          0          0   PCI-MSI 524291 Edge      nvme0q3
> > 226:         53          0          0          0   PCI-MSI 524292 Edge      nvme0q4
> >
>
> Richard,
>
> off topic but I've seen in the IMX8MMRM a claim that it supports MSI-X
> but I have not seen this to be the case as you are showing above with
> an nvme that would clearly use MSI-X if available.

IMX8MM should support MSI only.

Frank
>
> Does the IMX8MM hardware support MSI-X?
>
> Best Regards,
>
> Tim
Bjorn Helgaas Aug. 30, 2024, 4:15 p.m. UTC | #12
On Fri, Aug 30, 2024 at 11:49:39AM -0400, Frank Li wrote:
> On Thu, Aug 29, 2024 at 05:00:05PM -0500, Bjorn Helgaas wrote:
> > On Thu, Aug 29, 2024 at 05:43:42PM -0400, Frank Li wrote:
> > > On Thu, Aug 29, 2024 at 04:22:35PM -0500, Bjorn Helgaas wrote:
> > > > On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > > > > Greetings,
> > > > >
> > > > > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > > > > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > > > > device and the device is not getting a valid interrupt.
> > > >
> > > > Does pci-imx6.c support INTx at all?
> > >
> > > Yes, dwc controller map INTx message to 4 irq lines, which connect to GIC.
> > > we tested it by add nomsi in kernel command line.
> >
> > Thanks, Frank.  Can you point me to the dwc code where this happens?
> > Maybe I can remember this for next time or add a comment to help
> > people find it.
> 
> I think it needn't special code to handle this. in dts
> 
>  interrupt-map = <0 0 0 1 &gic GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>                  <0 0 0 2 &gic GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
>                  <0 0 0 3 &gic GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>                  <0 0 0 4 &gic GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;
> 
> It map INT(A,B,C,D) to 4 GIC irq.

OK, thanks!  I guess this happens in the of_irq_parse_pci() path, e.g.:

  imx6_pcie_probe
    dw_pcie_host_init
      devm_pci_alloc_host_bridge
        devm_of_pci_bridge_init
          bridge->map_irq = of_irq_parse_and_map_pci

  pci_device_probe                    # pci_bus_type.probe
    pci_assign_irq
      bridge->map_irq()
        of_irq_parse_and_map_pci
          of_irq_parse_pci
            of_irq_parse_one
              of_irq_parse_raw
                of_get_property(ipar, "interrupt-map", &imaplen)

So there really isn't any mention of INTx directly in imx6 or the dwc
core.

I suppose something like:

  - Set CONFIG_DYNAMIC_DEBUG=y

  - Boot with kernel parameters:

      ignore_loglevel dyndbg="file drivers/pci/* +p; file drivers/of/* +p"

should enable some debug output related to this.

Bjorn
Tim Harvey Sept. 6, 2024, 7:37 p.m. UTC | #13
On Fri, Aug 30, 2024 at 9:14 AM Frank Li <Frank.li@nxp.com> wrote:
>
> On Fri, Aug 30, 2024 at 09:03:09AM -0700, Tim Harvey wrote:
> > On Thu, Aug 29, 2024 at 6:50 PM Hongxing Zhu <hongxing.zhu@nxp.com> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@kernel.org>
> > > > Sent: 2024年8月30日 5:23
> > > > To: tharvey@gateworks.com; Hongxing Zhu <hongxing.zhu@nxp.com>; Lucas
> > > > Stach <l.stach@pengutronix.de>
> > > > Cc: linux-pci@vger.kernel.org
> > > > Subject: Re: legacy PCI device behind a bridge not getting a valid IRQ on imx
> > > > host controller
> > > >
> > > > [+cc Richard, Lucas, maintainers of IMX6 PCI]
> > > >
> > > > On Wed, Aug 28, 2024 at 02:40:33PM -0700, Tim Harvey wrote:
> > > > > Greetings,
> > > > >
> > > > > I have a user that is using an IMX8MM SoC (dwc controller) with a
> > > > > miniPCIe card that has a PEX8112 PCI-to-PCIe bridge to a legacy PCI
> > > > > device and the device is not getting a valid interrupt.
> > > >
> > > > Does pci-imx6.c support INTx at all?
> > > >
> > > i.MX PCIe RC supports INTx.
> > > Add pci=nomsi into kernel command line, can verify it when one endpoint
> > >  device is connected.
> > > Based i.MX8MM EVK board and on NVME, MSI or INTx are enabled.
> > > logs of MSI:
> > > root@imx8_all:~# lspci
> > > 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01)
> > > 01:00.0 Non-Volatile memory controller: Device 1e49:0021 (rev 01)
> > > root@imx8_all:~# cat /proc/interrupts | grep MSI
> > > 221:          0          0          0          0   PCI-MSI   0 Edge      PCIe PME
> > > 222:         14          0          0          0   PCI-MSI 524288 Edge      nvme0q0
> > > 223:        382          0          0          0   PCI-MSI 524289 Edge      nvme0q1
> > > 224:        115          0          0          0   PCI-MSI 524290 Edge      nvme0q2
> > > 225:        521          0          0          0   PCI-MSI 524291 Edge      nvme0q3
> > > 226:         53          0          0          0   PCI-MSI 524292 Edge      nvme0q4
> > >
> >
> > Richard,
> >
> > off topic but I've seen in the IMX8MMRM a claim that it supports MSI-X
> > but I have not seen this to be the case as you are showing above with
> > an nvme that would clearly use MSI-X if available.
>
> IMX8MM should support MSI only.
>

Hi Frank,

Thanks for clarifying this; I don't know why the IMX8MMRM and IMX8MPRM
indicate they support MSI-X.

I have the hardware in hand now as well as the out-of-tree driver
that's being used. I can say there is nothing wrong here with legacy
PCI interrupt mapping, if I write a skeleton driver that uses
pci_resister_driver(struct pci_driver) its probe is called with an
interrupt and request_irq on that interrupt succeeds just fine.

The issue here is with the vendor's out-of-tree driver which instead
is using pci_get_device() to scan the bus which returns a struct
pci_dev * that doesn't have an irq assigned (like what is described in
https://www.kernel.org/doc/html/v5.5/PCI/pci.html#how-to-find-pci-devices-manually).
When using pci_get_device() when/how does pci_assign_irq() get called
to assign the irq to the device?

Best Regards,

Tim
Bjorn Helgaas Sept. 6, 2024, 7:58 p.m. UTC | #14
On Fri, Sep 06, 2024 at 12:37:42PM -0700, Tim Harvey wrote:
> ...

> I have the hardware in hand now as well as the out-of-tree driver
> that's being used. I can say there is nothing wrong here with legacy
> PCI interrupt mapping, if I write a skeleton driver that uses
> pci_resister_driver(struct pci_driver) its probe is called with an
> interrupt and request_irq on that interrupt succeeds just fine.
> 
> The issue here is with the vendor's out-of-tree driver which instead
> is using pci_get_device() to scan the bus which returns a struct
> pci_dev * that doesn't have an irq assigned (like what is described
> in
> https://www.kernel.org/doc/html/v5.5/PCI/pci.html#how-to-find-pci-devices-manually).
> When using pci_get_device() when/how does pci_assign_irq() get
> called to assign the irq to the device?

Hmmm.  pci_get_device() is strongly discouraged because it subverts
the driver model (it skips the usual driver probe/claim model so it
allows multiple drivers to operate the device simultaneously which can
obviously cause conflicts), and it doesn't play well with hotplug
(hotplug events automatically cause driver .probe() methods to be
called, but users of pci_get_device() have to roll their own way of
doing this).

So I'm not aware of a documented/supported way to set up the INTx
interrupts in the pci_get_device() case.  

Bjorn
Tim Harvey Sept. 10, 2024, 4:50 p.m. UTC | #15
On Fri, Sep 6, 2024 at 12:58 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Fri, Sep 06, 2024 at 12:37:42PM -0700, Tim Harvey wrote:
> > ...
>
> > I have the hardware in hand now as well as the out-of-tree driver
> > that's being used. I can say there is nothing wrong here with legacy
> > PCI interrupt mapping, if I write a skeleton driver that uses
> > pci_resister_driver(struct pci_driver) its probe is called with an
> > interrupt and request_irq on that interrupt succeeds just fine.
> >
> > The issue here is with the vendor's out-of-tree driver which instead
> > is using pci_get_device() to scan the bus which returns a struct
> > pci_dev * that doesn't have an irq assigned (like what is described
> > in
> > https://www.kernel.org/doc/html/v5.5/PCI/pci.html#how-to-find-pci-devices-manually).
> > When using pci_get_device() when/how does pci_assign_irq() get
> > called to assign the irq to the device?
>
> Hmmm.  pci_get_device() is strongly discouraged because it subverts
> the driver model (it skips the usual driver probe/claim model so it
> allows multiple drivers to operate the device simultaneously which can
> obviously cause conflicts), and it doesn't play well with hotplug
> (hotplug events automatically cause driver .probe() methods to be
> called, but users of pci_get_device() have to roll their own way of
> doing this).
>
> So I'm not aware of a documented/supported way to set up the INTx
> interrupts in the pci_get_device() case.
>

Hi Bjorn,

Makes sense to me. Perhaps some changes to Documentation/PCI/pci.rst
could explain this.

Thanks for the help here, glad to find there is nothing broken here. I
think there could have been some confusion by the user here because
they were used to x86 assigning irq's without using
pci_resister_driver() but they were also using a kernel param of
pci=routeirq which looks like its an x86 only temporary workaround
that may have made this work on that architecture.

Best Regards,

Tim
Bjorn Helgaas Sept. 10, 2024, 11:04 p.m. UTC | #16
On Tue, Sep 10, 2024 at 09:50:02AM -0700, Tim Harvey wrote:
> On Fri, Sep 6, 2024 at 12:58 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Fri, Sep 06, 2024 at 12:37:42PM -0700, Tim Harvey wrote:
> > > ...
> >
> > > I have the hardware in hand now as well as the out-of-tree driver
> > > that's being used. I can say there is nothing wrong here with legacy
> > > PCI interrupt mapping, if I write a skeleton driver that uses
> > > pci_resister_driver(struct pci_driver) its probe is called with an
> > > interrupt and request_irq on that interrupt succeeds just fine.
> > >
> > > The issue here is with the vendor's out-of-tree driver which instead
> > > is using pci_get_device() to scan the bus which returns a struct
> > > pci_dev * that doesn't have an irq assigned (like what is described
> > > in
> > > https://www.kernel.org/doc/html/v5.5/PCI/pci.html#how-to-find-pci-devices-manually).
> > > When using pci_get_device() when/how does pci_assign_irq() get
> > > called to assign the irq to the device?
> >
> > Hmmm.  pci_get_device() is strongly discouraged because it subverts
> > the driver model (it skips the usual driver probe/claim model so it
> > allows multiple drivers to operate the device simultaneously which can
> > obviously cause conflicts), and it doesn't play well with hotplug
> > (hotplug events automatically cause driver .probe() methods to be
> > called, but users of pci_get_device() have to roll their own way of
> > doing this).
> >
> > So I'm not aware of a documented/supported way to set up the INTx
> > interrupts in the pci_get_device() case.
> 
> Makes sense to me. Perhaps some changes to Documentation/PCI/pci.rst
> could explain this.

Yeah, that would be a good idea.

> Thanks for the help here, glad to find there is nothing broken here. I
> think there could have been some confusion by the user here because
> they were used to x86 assigning irq's without using
> pci_resister_driver() but they were also using a kernel param of
> pci=routeirq which looks like its an x86 only temporary workaround
> that may have made this work on that architecture.

I wondered about "pci=routirq", but I lost the trail and couldn't
figure out how that would lead to pci_assign_irq() or something
functionally equivalent.

Bjorn
Tim Harvey Sept. 12, 2024, 11:28 p.m. UTC | #17
On Tue, Sep 10, 2024 at 4:04 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, Sep 10, 2024 at 09:50:02AM -0700, Tim Harvey wrote:
> > On Fri, Sep 6, 2024 at 12:58 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Fri, Sep 06, 2024 at 12:37:42PM -0700, Tim Harvey wrote:
> > > > ...
> > >
> > > > I have the hardware in hand now as well as the out-of-tree driver
> > > > that's being used. I can say there is nothing wrong here with legacy
> > > > PCI interrupt mapping, if I write a skeleton driver that uses
> > > > pci_resister_driver(struct pci_driver) its probe is called with an
> > > > interrupt and request_irq on that interrupt succeeds just fine.
> > > >
> > > > The issue here is with the vendor's out-of-tree driver which instead
> > > > is using pci_get_device() to scan the bus which returns a struct
> > > > pci_dev * that doesn't have an irq assigned (like what is described
> > > > in
> > > > https://www.kernel.org/doc/html/v5.5/PCI/pci.html#how-to-find-pci-devices-manually).
> > > > When using pci_get_device() when/how does pci_assign_irq() get
> > > > called to assign the irq to the device?
> > >
> > > Hmmm.  pci_get_device() is strongly discouraged because it subverts
> > > the driver model (it skips the usual driver probe/claim model so it
> > > allows multiple drivers to operate the device simultaneously which can
> > > obviously cause conflicts), and it doesn't play well with hotplug
> > > (hotplug events automatically cause driver .probe() methods to be
> > > called, but users of pci_get_device() have to roll their own way of
> > > doing this).
> > >
> > > So I'm not aware of a documented/supported way to set up the INTx
> > > interrupts in the pci_get_device() case.
> >
> > Makes sense to me. Perhaps some changes to Documentation/PCI/pci.rst
> > could explain this.
>
> Yeah, that would be a good idea.
>
> > Thanks for the help here, glad to find there is nothing broken here. I
> > think there could have been some confusion by the user here because
> > they were used to x86 assigning irq's without using
> > pci_resister_driver() but they were also using a kernel param of
> > pci=routeirq which looks like its an x86 only temporary workaround
> > that may have made this work on that architecture.
>
> I wondered about "pci=routirq", but I lost the trail and couldn't
> figure out how that would lead to pci_assign_irq() or something
> functionally equivalent.
>

Hi Bjorn and Frank,

While switching to pci_resister_driver() resolves the request_irq
issue I find that legacy IRQ's fire on the imx8mm/imx8mp only when the
device is not behind a bridge:

Again this specific device is a DDK BU-69092S1 which has a TI XIO2001
PCIe-to-PCI bridge with a PCI device behind it.

Here is an example of a GW72xx (which has a PI7C9X2G404E 4-port PCIe
switch) with an imx8mm:
root@noble-venice:~# uname -r
6.6.8-gc6ef8b5e47a0
root@noble-venice:~# lspci -n
00:00.0 0604: 16c3:abcd (rev 01)
01:00.0 0604: 12d8:b404 (rev 01)
02:01.0 0604: 12d8:b404 (rev 01)
02:02.0 0604: 12d8:b404 (rev 01)
02:03.0 0604: 12d8:b404 (rev 01)
04:00.0 0604: 104c:8240
05:00.0 0780: 4ddc:1a00 (rev 10)
05:01.0 0780: 4ddc:1a00 (rev 10)
06:00.0 0200: 1055:7430 (rev 11)
root@noble-venice:~# lspci -s 05:00 -vvv | grep Interrupt
        Interrupt: pin A routed to IRQ 206
root@noble-venice:~# grep 206 /proc/interrupts
206:          0          0          0          0     GICv3 157 Level
  PCIe PME
^^^ makes sense... driver has probed and requested the IRQ but nothing
has made it fire yet
root@noble-venice:~# ./tester
Testing.....Registers Passed test.
Testing.....Ram Passed 1234 test.
Testing.....Ram Passed aaaa test.
Testing.....Ram Passed aa55 test.
Testing.....Ram Passed 55aa test.
Testing.....Ram Passed 5555 test.
Testing.....Ram Passed ffff test.
Testing.....Ram Passed 1111 test.
Testing.....Ram Passed 8888 test.
Testing.....Ram Passed 0000 test.
Testing.....Interrupt Test Failure,  NO IRQ!!!
Testing.....Protocol Test RTL Function Failure-> Function not supported.
Testing.....Vector Test RTL Function Failure-> Function not supported.
EXIT: tester
^^^ this app causes the device to fire an IRQ and verifies its been
caught; fails due to no irq firing
root@noble-venice:~# grep 206 /proc/interrupts
206:          0          0          0          0     GICv3 157 Level
  PCIe PME
^^^ 0 interrupts

Here is an example of the same device on a GW71xx (which has no
switch) with an imx8mm:
root@noble-venice:~# uname -r
6.6.8-gc6ef8b5e47a0
root@noble-venice:~# lspci -n
00:00.0 0604: 16c3:abcd (rev 01)
01:00.0 0604: 104c:8240
02:00.0 0780: 4ddc:1a00 (rev 10)
02:01.0 0780: 4ddc:1a00 (rev 10)
root@noble-venice:~# lspci -s 02:00 -vvv | grep Interrupt
        Interrupt: pin A routed to IRQ 204
root@noble-venice:~# grep 204 /proc/interrupts
204:          0          0          0          0     GICv3 157 Level
  PCIe PME
root@noble-venice:~# ./tester
Testing.....Registers Passed test.
Testing.....Ram Passed 1234 test.
Testing.....Ram Passed aaaa test.
Testing.....Ram Passed aa55 test.
Testing.....Ram Passed 55aa test.
Testing.....Ram Passed 5555 test.
Testing.....Ram Passed ffff test.
Testing.....Ram Passed 1111 test.
Testing.....Ram Passed 8888 test.
Testing.....Ram Passed 0000 test.
Testing.....Interrupt Occurred, Passed test.
Testing.....Protocol Test RTL Function Failure-> Function not supported.
Testing.....Vector Test RTL Function Failure-> Function not supported.
EXIT: tester
root@noble-venice:~# grep 204 /proc/interrupts
204:          1          0          0          0     GICv3 157 Level
  PCIe PME

I believe this issue is likely the same thing I enquired about over a
year ago [1] showing with an ath9k device which uses legacy IRQ's
which I've also never resolved.

Any ideas here?

Best Regards,

Tim
Tim Harvey Sept. 23, 2024, 5:56 p.m. UTC | #18
On Thu, Sep 12, 2024 at 4:28 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Tue, Sep 10, 2024 at 4:04 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Tue, Sep 10, 2024 at 09:50:02AM -0700, Tim Harvey wrote:
> > > On Fri, Sep 6, 2024 at 12:58 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > On Fri, Sep 06, 2024 at 12:37:42PM -0700, Tim Harvey wrote:
> > > > > ...
> > > >
> > > > > I have the hardware in hand now as well as the out-of-tree driver
> > > > > that's being used. I can say there is nothing wrong here with legacy
> > > > > PCI interrupt mapping, if I write a skeleton driver that uses
> > > > > pci_resister_driver(struct pci_driver) its probe is called with an
> > > > > interrupt and request_irq on that interrupt succeeds just fine.
> > > > >
> > > > > The issue here is with the vendor's out-of-tree driver which instead
> > > > > is using pci_get_device() to scan the bus which returns a struct
> > > > > pci_dev * that doesn't have an irq assigned (like what is described
> > > > > in
> > > > > https://www.kernel.org/doc/html/v5.5/PCI/pci.html#how-to-find-pci-devices-manually).
> > > > > When using pci_get_device() when/how does pci_assign_irq() get
> > > > > called to assign the irq to the device?
> > > >
> > > > Hmmm.  pci_get_device() is strongly discouraged because it subverts
> > > > the driver model (it skips the usual driver probe/claim model so it
> > > > allows multiple drivers to operate the device simultaneously which can
> > > > obviously cause conflicts), and it doesn't play well with hotplug
> > > > (hotplug events automatically cause driver .probe() methods to be
> > > > called, but users of pci_get_device() have to roll their own way of
> > > > doing this).
> > > >
> > > > So I'm not aware of a documented/supported way to set up the INTx
> > > > interrupts in the pci_get_device() case.
> > >
> > > Makes sense to me. Perhaps some changes to Documentation/PCI/pci.rst
> > > could explain this.
> >
> > Yeah, that would be a good idea.
> >
> > > Thanks for the help here, glad to find there is nothing broken here. I
> > > think there could have been some confusion by the user here because
> > > they were used to x86 assigning irq's without using
> > > pci_resister_driver() but they were also using a kernel param of
> > > pci=routeirq which looks like its an x86 only temporary workaround
> > > that may have made this work on that architecture.
> >
> > I wondered about "pci=routirq", but I lost the trail and couldn't
> > figure out how that would lead to pci_assign_irq() or something
> > functionally equivalent.
> >
>
> Hi Bjorn and Frank,
>
> While switching to pci_resister_driver() resolves the request_irq
> issue I find that legacy IRQ's fire on the imx8mm/imx8mp only when the
> device is not behind a bridge:
>
> Again this specific device is a DDK BU-69092S1 which has a TI XIO2001
> PCIe-to-PCI bridge with a PCI device behind it.
>
> Here is an example of a GW72xx (which has a PI7C9X2G404E 4-port PCIe
> switch) with an imx8mm:
> root@noble-venice:~# uname -r
> 6.6.8-gc6ef8b5e47a0
> root@noble-venice:~# lspci -n
> 00:00.0 0604: 16c3:abcd (rev 01)
> 01:00.0 0604: 12d8:b404 (rev 01)
> 02:01.0 0604: 12d8:b404 (rev 01)
> 02:02.0 0604: 12d8:b404 (rev 01)
> 02:03.0 0604: 12d8:b404 (rev 01)
> 04:00.0 0604: 104c:8240
> 05:00.0 0780: 4ddc:1a00 (rev 10)
> 05:01.0 0780: 4ddc:1a00 (rev 10)
> 06:00.0 0200: 1055:7430 (rev 11)
> root@noble-venice:~# lspci -s 05:00 -vvv | grep Interrupt
>         Interrupt: pin A routed to IRQ 206
> root@noble-venice:~# grep 206 /proc/interrupts
> 206:          0          0          0          0     GICv3 157 Level
>   PCIe PME
> ^^^ makes sense... driver has probed and requested the IRQ but nothing
> has made it fire yet
> root@noble-venice:~# ./tester
> Testing.....Registers Passed test.
> Testing.....Ram Passed 1234 test.
> Testing.....Ram Passed aaaa test.
> Testing.....Ram Passed aa55 test.
> Testing.....Ram Passed 55aa test.
> Testing.....Ram Passed 5555 test.
> Testing.....Ram Passed ffff test.
> Testing.....Ram Passed 1111 test.
> Testing.....Ram Passed 8888 test.
> Testing.....Ram Passed 0000 test.
> Testing.....Interrupt Test Failure,  NO IRQ!!!
> Testing.....Protocol Test RTL Function Failure-> Function not supported.
> Testing.....Vector Test RTL Function Failure-> Function not supported.
> EXIT: tester
> ^^^ this app causes the device to fire an IRQ and verifies its been
> caught; fails due to no irq firing
> root@noble-venice:~# grep 206 /proc/interrupts
> 206:          0          0          0          0     GICv3 157 Level
>   PCIe PME
> ^^^ 0 interrupts
>
> Here is an example of the same device on a GW71xx (which has no
> switch) with an imx8mm:
> root@noble-venice:~# uname -r
> 6.6.8-gc6ef8b5e47a0
> root@noble-venice:~# lspci -n
> 00:00.0 0604: 16c3:abcd (rev 01)
> 01:00.0 0604: 104c:8240
> 02:00.0 0780: 4ddc:1a00 (rev 10)
> 02:01.0 0780: 4ddc:1a00 (rev 10)
> root@noble-venice:~# lspci -s 02:00 -vvv | grep Interrupt
>         Interrupt: pin A routed to IRQ 204
> root@noble-venice:~# grep 204 /proc/interrupts
> 204:          0          0          0          0     GICv3 157 Level
>   PCIe PME
> root@noble-venice:~# ./tester
> Testing.....Registers Passed test.
> Testing.....Ram Passed 1234 test.
> Testing.....Ram Passed aaaa test.
> Testing.....Ram Passed aa55 test.
> Testing.....Ram Passed 55aa test.
> Testing.....Ram Passed 5555 test.
> Testing.....Ram Passed ffff test.
> Testing.....Ram Passed 1111 test.
> Testing.....Ram Passed 8888 test.
> Testing.....Ram Passed 0000 test.
> Testing.....Interrupt Occurred, Passed test.
> Testing.....Protocol Test RTL Function Failure-> Function not supported.
> Testing.....Vector Test RTL Function Failure-> Function not supported.
> EXIT: tester
> root@noble-venice:~# grep 204 /proc/interrupts
> 204:          1          0          0          0     GICv3 157 Level
>   PCIe PME
>
> I believe this issue is likely the same thing I enquired about over a
> year ago [1] showing with an ath9k device which uses legacy IRQ's
> which I've also never resolved.
>
> Any ideas here?
>

Bjorn and Frank, any ideas here?

I neglected to point to the other email which referenced what I
believe to be the same issue:
https://www.spinics.net/lists/linux-wireless/msg233763.html

I believe that legacy interrupts do not work on imx8m{m,p} when behind
a bridge. I believe the interrupts are getting swizzled when they
should not as it's a PCIe bridge (Int A/B/C/D should be mapped based
on dt and thus should not change depending on downstream slot).

Best Regards,

Tim
Bjorn Helgaas Sept. 26, 2024, 10:30 p.m. UTC | #19
[+cc Rob since I'm not a DT expert]

On Mon, Sep 23, 2024 at 10:56:18AM -0700, Tim Harvey wrote:
> On Thu, Sep 12, 2024 at 4:28 PM Tim Harvey <tharvey@gateworks.com> wrote:
> > On Tue, Sep 10, 2024 at 4:04 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Tue, Sep 10, 2024 at 09:50:02AM -0700, Tim Harvey wrote:
> > > > On Fri, Sep 6, 2024 at 12:58 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > > On Fri, Sep 06, 2024 at 12:37:42PM -0700, Tim Harvey wrote:
> > > > > > ...
> > > > >
> > > > > > I have the hardware in hand now as well as the out-of-tree driver
> > > > > > that's being used. I can say there is nothing wrong here with legacy
> > > > > > PCI interrupt mapping, if I write a skeleton driver that uses
> > > > > > pci_register_driver(struct pci_driver) its probe is called with an
> > > > > > interrupt and request_irq on that interrupt succeeds just fine.
> > > > > >
> > > > > > The issue here is with the vendor's out-of-tree driver which instead
> > > > > > is using pci_get_device() to scan the bus which returns a struct
> > > > > > pci_dev * that doesn't have an irq assigned (like what is described
> > > > > > in
> > > > > > https://www.kernel.org/doc/html/v5.5/PCI/pci.html#how-to-find-pci-devices-manually).
> > > > > > When using pci_get_device() when/how does pci_assign_irq() get
> > > > > > called to assign the irq to the device?
> > > > >
> > > > > Hmmm.  pci_get_device() is strongly discouraged because it subverts
> > > > > the driver model (it skips the usual driver probe/claim model so it
> > > > > allows multiple drivers to operate the device simultaneously which can
> > > > > obviously cause conflicts), and it doesn't play well with hotplug
> > > > > (hotplug events automatically cause driver .probe() methods to be
> > > > > called, but users of pci_get_device() have to roll their own way of
> > > > > doing this).
> > > > >
> > > > > So I'm not aware of a documented/supported way to set up the INTx
> > > > > interrupts in the pci_get_device() case.
> > > >
> > > > Makes sense to me. Perhaps some changes to Documentation/PCI/pci.rst
> > > > could explain this.
> > >
> > > Yeah, that would be a good idea.
> > >
> > > > Thanks for the help here, glad to find there is nothing broken here. I
> > > > think there could have been some confusion by the user here because
> > > > they were used to x86 assigning irq's without using
> > > > pci_resister_driver() but they were also using a kernel param of
> > > > pci=routeirq which looks like its an x86 only temporary workaround
> > > > that may have made this work on that architecture.
> > >
> > > I wondered about "pci=routirq", but I lost the trail and couldn't
> > > figure out how that would lead to pci_assign_irq() or something
> > > functionally equivalent.
> >
> > Hi Bjorn and Frank,
> >
> > While switching to pci_register_driver() resolves the request_irq
> > issue I find that legacy IRQ's fire on the imx8mm/imx8mp only when the
> > device is not behind a bridge:
> >
> > Again this specific device is a DDK BU-69092S1 which has a TI XIO2001
> > PCIe-to-PCI bridge with a PCI device behind it.
> >
> > Here is an example of a GW72xx (which has a PI7C9X2G404E 4-port PCIe
> > switch) with an imx8mm:
> > root@noble-venice:~# uname -r
> > 6.6.8-gc6ef8b5e47a0
> > root@noble-venice:~# lspci -n
> > 00:00.0 0604: 16c3:abcd (rev 01)
> > 01:00.0 0604: 12d8:b404 (rev 01)
> > 02:01.0 0604: 12d8:b404 (rev 01)
> > 02:02.0 0604: 12d8:b404 (rev 01)
> > 02:03.0 0604: 12d8:b404 (rev 01)
> > 04:00.0 0604: 104c:8240
> > 05:00.0 0780: 4ddc:1a00 (rev 10)
> > 05:01.0 0780: 4ddc:1a00 (rev 10)
> > 06:00.0 0200: 1055:7430 (rev 11)
> > root@noble-venice:~# lspci -s 05:00 -vvv | grep Interrupt
> >         Interrupt: pin A routed to IRQ 206
> > root@noble-venice:~# grep 206 /proc/interrupts
> > 206:          0          0          0          0     GICv3 157 Level
> >   PCIe PME
> > ^^^ makes sense... driver has probed and requested the IRQ but nothing
> > has made it fire yet
> > root@noble-venice:~# ./tester
> > Testing.....Registers Passed test.
> > Testing.....Ram Passed 1234 test.
> > Testing.....Ram Passed aaaa test.
> > Testing.....Ram Passed aa55 test.
> > Testing.....Ram Passed 55aa test.
> > Testing.....Ram Passed 5555 test.
> > Testing.....Ram Passed ffff test.
> > Testing.....Ram Passed 1111 test.
> > Testing.....Ram Passed 8888 test.
> > Testing.....Ram Passed 0000 test.
> > Testing.....Interrupt Test Failure,  NO IRQ!!!
> > Testing.....Protocol Test RTL Function Failure-> Function not supported.
> > Testing.....Vector Test RTL Function Failure-> Function not supported.
> > EXIT: tester
> > ^^^ this app causes the device to fire an IRQ and verifies its been
> > caught; fails due to no irq firing
> > root@noble-venice:~# grep 206 /proc/interrupts
> > 206:          0          0          0          0     GICv3 157 Level
> >   PCIe PME
> > ^^^ 0 interrupts
> >
> > Here is an example of the same device on a GW71xx (which has no
> > switch) with an imx8mm:
> > root@noble-venice:~# uname -r
> > 6.6.8-gc6ef8b5e47a0
> > root@noble-venice:~# lspci -n
> > 00:00.0 0604: 16c3:abcd (rev 01)
> > 01:00.0 0604: 104c:8240
> > 02:00.0 0780: 4ddc:1a00 (rev 10)
> > 02:01.0 0780: 4ddc:1a00 (rev 10)
> > root@noble-venice:~# lspci -s 02:00 -vvv | grep Interrupt
> >         Interrupt: pin A routed to IRQ 204
> > root@noble-venice:~# grep 204 /proc/interrupts
> > 204:          0          0          0          0     GICv3 157 Level
> >   PCIe PME
> > root@noble-venice:~# ./tester
> > Testing.....Registers Passed test.
> > Testing.....Ram Passed 1234 test.
> > Testing.....Ram Passed aaaa test.
> > Testing.....Ram Passed aa55 test.
> > Testing.....Ram Passed 55aa test.
> > Testing.....Ram Passed 5555 test.
> > Testing.....Ram Passed ffff test.
> > Testing.....Ram Passed 1111 test.
> > Testing.....Ram Passed 8888 test.
> > Testing.....Ram Passed 0000 test.
> > Testing.....Interrupt Occurred, Passed test.
> > Testing.....Protocol Test RTL Function Failure-> Function not supported.
> > Testing.....Vector Test RTL Function Failure-> Function not supported.
> > EXIT: tester
> > root@noble-venice:~# grep 204 /proc/interrupts
> > 204:          1          0          0          0     GICv3 157 Level
> >   PCIe PME
> >
> > I believe this issue is likely the same thing I enquired about over a
> > year ago [1] showing with an ath9k device which uses legacy IRQ's
> > which I've also never resolved.
> >
> > Any ideas here?
> 
> Bjorn and Frank, any ideas here?
> 
> I neglected to point to the other email which referenced what I
> believe to be the same issue:
> https://www.spinics.net/lists/linux-wireless/msg233763.html

[ad-free permalink: https://lore.kernel.org/all/CAJ+vNU0TuNA26F+hFwTRGc2pVvW34-7Sc7oQ9EW8V+2cVFgcag@mail.gmail.com/]

> I believe that legacy interrupts do not work on imx8m{m,p} when behind
> a bridge. I believe the interrupts are getting swizzled when they
> should not as it's a PCIe bridge (Int A/B/C/D should be mapped based
> on dt and thus should not change depending on downstream slot).

I assume that INTx swizzling for everything below the host bridge is
specified by the PCI spec and the only place DT is relevant is at the
host bridge itself?

What's the specific DT in question here?

I don't know enough about DT to have any ideas yet, maybe Rob does?

If I had to try to figure this out in my ignorance, I would start with
debug like below (probably overkill for people who know how this
works), and look at the DT and the driver and try to figure out what's
going wrong.  Wish I knew enough to suggest something smart ;)

diff --git a/drivers/pci/irq.c b/drivers/pci/irq.c
index 4555630be9ec..47cf77d29211 100644
--- a/drivers/pci/irq.c
+++ b/drivers/pci/irq.c
@@ -79,6 +79,11 @@ void pci_free_irq(struct pci_dev *dev, unsigned int nr, void *dev_id)
 }
 EXPORT_SYMBOL(pci_free_irq);
 
+static inline char pin_name(u8 pin)
+{
+	return 'A' + pin - 1;
+}
+
 /**
  * pci_swizzle_interrupt_pin - swizzle INTx for device behind bridge
  * @dev: the PCI device
@@ -93,13 +98,17 @@ EXPORT_SYMBOL(pci_free_irq);
 u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin)
 {
 	int slot;
+	u8 swizzled_pin;
 
 	if (pci_ari_enabled(dev->bus))
 		slot = 0;
 	else
 		slot = PCI_SLOT(dev->devfn);
 
-	return (((pin - 1) + slot) % 4) + 1;
+	swizzled_pin = (((pin - 1) + slot) % 4) + 1;
+	pci_dbg(dev, "%s: swizzle INT%c -> INT%c (slot %d)\n",
+		__func__, pin_name(pin), pin_name(swizzled_pin), slot);
+	return swizzled_pin;
 }
 
 int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge)
@@ -115,6 +124,7 @@ int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge)
 		dev = dev->bus->self;
 	}
 	*bridge = dev;
+	pci_dbg(dev, "%s: INT%c\n", __func__, pin_name(pin));
 	return pin;
 }
 
@@ -135,6 +145,7 @@ u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp)
 		dev = dev->bus->self;
 	}
 	*pinp = pin;
+	pci_dbg(dev, "%s: INT%c\n", __func__, pin_name(pin));
 	return PCI_SLOT(dev->devfn);
 }
 EXPORT_SYMBOL_GPL(pci_common_swizzle);
@@ -168,6 +179,9 @@ void pci_assign_irq(struct pci_dev *dev)
 		if (hbrg->swizzle_irq)
 			slot = (*(hbrg->swizzle_irq))(dev, &pin);
 
+		pci_dbg(dev, "%s: swizzled INT%c slot %d\n",
+			__func__, pin_name(pin), slot);
+
 		/*
 		 * If a swizzling function is not used, map_irq() must
 		 * ignore slot.
diff mbox series

Patch

diff --git a/drivers/pci/dwc/pcie-designware-host.c
b/drivers/pci/dwc/pcie-designware-host.c
index ec2e4a61aa4e..a72ba177a5fd 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -477,6 +477,8 @@  int dw_pcie_host_init(struct pcie_port *pp)
        if (pp->ops->scan_bus)
                pp->ops->scan_bus(pp);

+       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+
        pci_bus_size_bridges(bus);
        pci_bus_assign_resources(bus);