On Wed, Feb 27, 2019 at 8:40 AM Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote:
>
> Am Mittwoch, den 27.02.2019, 08:29 -0800 schrieb Tim Harvey:
> > > On Wed, Feb 27, 2019 at 2:01 AM Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote:
> > >
> > > Hi Tim,
> > >
> > > Am Dienstag, den 26.02.2019, 13:13 -0800 schrieb Tim Harvey:
> > > > Greetings,
> > > >
> > > > I've got a miniPCIe card with a TW6869 8x frame grabber that stopped
> > > > working on an IMX6 based board with a PLX switch with Linux 4.17 as
> > > > the driver errors out with 'tw686x 0000:07:00.0: unable to request
> > > > interrupt' (request_irq() fails). I've found this only happens on
> > > > boards that have a switch. Note I'm booting with pci=nomsi as well as
> > > > the IMX6 has a limitation where legacy IRQ's wont fire if MSI irq's
> > > > are enabled. Strangely I don't see this issue with other cards such
> > > > as
> > > > the ath9k with msi disabled.
> > > >
> > > > I bisected the issue to 7c5925afbc58c6d6b384e1dc051bb992969bf787
> > > > 'PCI:
> > > > dwc: Move MSI IRQs allocation to IRQ domains hierarchical API' which
> > > > due to continued changes in drivers/pci/dwc can no longer be
> > > > reverted.
> > > >
> > > > Any ideas what happened here? IMX6 PCIe through a bridge always seems
> > > > not so well tested and very fragile over the past couple of years.
> > >
> > > Thanks for the report. Both the DWC PCIe core in general and the i.MX6
> > > integration have some rough edges, which makes things fall from time to
> > > time. Having a PCIe bridge in the mix does extend the test space quite
> > > a bit.
> > >
> > > Does the following patch help? I didn't test it yet, but it's based on
> > > my assumption of what is going wrong with the referenced commit.
> > >
> > > Regards,
> > > Lucas
> > >
> > > ----------------------------->8--------------------------------------
> > >
> > > From 40b4fc03878a935fa81af226a8e759d4c72a3603 Mon Sep 17 00:00:00 2001
> > > From: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> > > Date: Wed, 27 Feb 2019 10:50:20 +0100
> > > Subject: [PATCH] PCI: dwc: skip MSI init if MSIs have been explicitly disabled
> > >
> > > Since 7c5925afbc58 (PCI: dwc: Move MSI IRQs allocation to IRQ domains
> > > hierarchical API) the MSi init claims one of the controller IRQs as a
> > > chained IRQ line for the MSI controller. On some designs, like the i.MX6,
> > > this line is shared with a PCIe legacy IRQ. When the line is claimed for
> > > the MSI domain, any device trying to use this legacy IRQs will fail to
> > > request this IRQ line.
> > >
> > > As MSI and legacy IRQs are already mutually exclusive on the DWC core,
> > > as the core won't forward any legacy IRQs once any MSI has been enabled,
> > > users wishing to use legacy IRQs already need to explictly disable MSI
> > > support (usually via the pci=nomsi kernel commandline option). To avoid
> > > any issues with MSI conflicting with legacy IRQs, just skip all of the
> > > DWC MSI initalization, inclusing the IRQ line claim, when MSI is disabled.
> > >
> > > Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> > > ---
> > > drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index 29a05759a294..f4a8494f616b 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -433,7 +433,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
> > > if (ret)
> > > pci->num_viewport = 2;
> > >
> > > - if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > > + if (IS_ENABLED(CONFIG_PCI_MSI) && pci_msi_enabled()) {
> > > /*
> > > * If a specific SoC driver needs to change the
> > > * default number of vectors, it needs to implement
> > > --
> > > 2.20.1
> >
> > Lucas,
> >
> > Yes, this patch resolves the issue. Where did this come from? It looks
> > like it came from back in September but never made it upstream?
>
> Nope, I wrote it just after your report, as seen in the "Date" line.
>
> > I'm a bit baffled why I see this issue pre-patch with 'and' without
> > pci=nomsi based on this patch? I do see other checks in
> > imx6_pcie_host_init() and imx6_add_pcie_port() for
> > IS_ENABLED(CONFIG_PCI_MSI) that do not have an accompanying check of
> > pci_msi_enabled() to see if its enabled at runtime... are these
> > problematic as well?
>
> I'll look over them, but I think with this added all the relevant
> places are covered.
>
Ok, thanks for the patch!
Regards,
Tim
