Re: [PATCH v2 00/40] PCI: dwc: Driver clean-ups

[Rob Herring <robh@kernel.org>]

On Tue, Sep 15, 2020 at 3:12 AM Michael Walle <michael@walle.cc> wrote:
>
> Hi Rob,
>
> > This is a series of clean-ups for the Designware PCI driver. The series
> > initially reworks the config space accessors to use the existing pci_ops
> > struct. Then there's removal of various private data that's also present
> > in the pci_host_bridge struct. There's also some duplicated common (PCI
> > and DWC) register defines which I converted to use the common defines.
> > Finally, the initialization for speed/gen, number of lanes, and N_FTS
> > are all moved to the common DWC code.
>
> > This is compile tested only as I don't have any DWC based h/w, so any
> > testing would be helpful. A branch is here[1].
>
> I've noticed that with the latest linux-next, my board doesn't boot
> anymore. I've traced it back to this series. There is a similar
> board in kernelci [1,2] where you can have a look at the backtrace.
>
> I've bisected this to the following patch:
>   PCI: dwc: Use generic config accessors

That's helpful.

> I'm pretty much lost here. It seems that the kernel tries to read from
> an invalid/unmapped memory address.
>
> [1] https://kernelci.org/test/plan/id/5f5f4992d1c53777a0a6092d/
> [2] https://storage.kernelci.org/next/master/next-20200914/arm64/defconfig/gcc-8/lab-nxp/baseline-fsl-ls1028a-rdb.txt

Thanks for the pointers. I was wondering if kernelci had any boards with DWC.

Can you try this? The link up check seemed unnecessary as it is racy.
What happens if the link goes down right after checking? That's the
only thing in the change that sticks out.

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
b/drivers/pci/controller/dwc/pcie-designware-host.c
index 317ff512f8df..afee1a0e8883 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -441,6 +441,9 @@ static void __iomem
*dw_pcie_other_conf_map_bus(struct pci_bus *bus,
        struct pcie_port *pp = bus->sysdata;
        struct dw_pcie *pci = to_dw_pcie_from_pp(pp);

+       if (!dw_pcie_link_up(pci))
+               return NULL;
+
        busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) |
                 PCIE_ATU_FUNC(PCI_FUNC(devfn));