Yes that makes sense. I sort just focus on the diff lines without looking further. Thanks. -- Xuheng Li On Thu, Mar 4, 2021 at 1:36 PM Stefan Mätje <Stefan.Maetje@xxxxxx> wrote: > > Am Dienstag, den 02.03.2021, 22:51 -0500 schrieb Xuheng Li: > > Hi, > > > > We are using Linux 5.10 on a HPE Proliant m400 machine with an XGene > > PCIe bridge. The machine works well on some earlier versions like 5.4 > > but fails to set up the PCIe bridge on 5.10. > > > > Running `lscpi` on 5.4 shows: > > 00:00.0 PCI bridge: Applied Micro Circuits Corp. X-Gene PCIe bridge (rev 04) > > 01:00.0 Ethernet controller: Mellanox Technologies MT27520 Family > > [ConnectX-3 Pro] > > > > while on 5.10 it shows nothing. > > > > The earliest commit we found that causes the bug is > > > https://lore.kernel.org/linux-pci/20200602171601.17630-1-zhengdejin5@xxxxxxxxx/ > > which changes the file drivers/pci/controller/pci-xgene.c by wrapping > > the call of devm_platform_ioremap_resource_byname into > > platform_get_resource_byname. > > > > By reverting the change, the PCIe bridge works now. We are curious why > > this patch can cause the issue. > > > > Additionally, this bug still exists on 5.10.19 and reverting the above > > patch also fixed the issue. > > > > Any help would be appreciated! > > > > Being curious I've been looking at the diff from the mentioned patch for the > pci-xgene.c source: > > diff --git a/drivers/pci/controller/pci-xgene.c b/drivers/pci/controller/pci- > xgene.c > index d1efa8ffbae1..1431a18eb02c 100644 > --- a/drivers/pci/controller/pci-xgene.c > +++ b/drivers/pci/controller/pci-xgene.c > @@ -355,8 +355,7 @@ static int xgene_pcie_map_reg(struct xgene_pcie_port *port, > if (IS_ERR(port->csr_base)) > return PTR_ERR(port->csr_base); > > - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg"); > - port->cfg_base = devm_ioremap_resource(dev, res); > + port->cfg_base = devm_platform_ioremap_resource_byname(pdev, "cfg"); > if (IS_ERR(port->cfg_base)) > return PTR_ERR(port->cfg_base); > port->cfg_addr = res->start; /* <==== */ > > I think you can see the error even from this short snippet. In the working case > with the lines marked with "-" present the res variable is set by > platform_get_resource_byname() correctly. Then it can be used to set up > port->cfg_addr in the line marked by /* <==== */. > > In the failure case with the line marked with "+" only active the res variable > is not updated by devm_platform_ioremap_resource_byname() and carries data from > an earlier resource search or even is uninitialized. The value set to > port->cfg_addr will be crap with a high probability I think. > > Best regards, > Stefan >
