Re: Possible Bug on Xgene PCIe Driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux