On Mon, Jul 18, 2016 at 08:27:18AM +0200, Thierry Reding wrote:
On Sun, Jul 17, 2016 at 02:27:25AM +0200, Andreas Färber wrote:
Hi,
When I boot my Jetson TK1, by default I get this from lspci:
00:02.0 PCI bridge: NVIDIA Corporation TegraK1 PCIe x1 Bridge (rev a1) 01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 0c)
If however I plug some mini PCIe card, I get this instead:
00:01.0 PCI bridge: NVIDIA Corporation TegraK1 PCIe x4 Bridge (rev a1) 00:02.0 PCI bridge: NVIDIA Corporation TegraK1 PCIe x1 Bridge (rev a1) 01:00.0 Network controller: MEDIATEK Corp. Device 7612 02:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 0c)
I.e., there is a new Tegra PCIe x4 bridge and the number of the previous x1 bridge changed.
That is ugly because it changes the ID of the on-board PCI NIC from 01:00.0 to 02:00.0, which on openSUSE renames the network interface from enp1s0 to enp2s0, so that my /etc/sysconfig/network/ifcfg-enp1s0 does not take effect and the network interface doesn't come up.
Tested with U-Boot v2016.05 and v2016.07 and kernel 4.6.2 and 4.7-rc6.
Shouldn't U-Boot or the kernel driver always configure the PCIe ports the same way (both bridges available) since the slot is always there on this board?
I don't think that's going to ensure stable naming of devices. Linux uses depth-first sorting when enumerating devices, so if you attach any kind of bridge device to the first port, anything downstream of the second port still won't get a stable B/D/F.
That said, I see how what you're proposing could help at least minimize the potential for instability in numbering. Could you try to uncomment the tegra_pcie_port_free() line in tegra_pcie_enable() of the Tegra PCI host controller driver (drivers/pci/host/pci-tegra.c) and see if that improves things in your use-case? It's slightly hackish because it does allow access to the root port even if it's disabled, so I'm not sure it will work (might give you an external abort or something like that) but it might be worth a quick try.
The below seems to work fine for me. Can you verify that this works for you, too?
Thanks, Thierry
--- >8 --- diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c index c388468c202a..0b8616e25d7b 100644 --- a/drivers/pci/host/pci-tegra.c +++ b/drivers/pci/host/pci-tegra.c @@ -587,17 +587,6 @@ static void tegra_pcie_port_disable(struct tegra_pcie_port *port) afi_writel(port->pcie, value, ctrl); }
-static void tegra_pcie_port_free(struct tegra_pcie_port *port) -{ - struct tegra_pcie *pcie = port->pcie; - - devm_iounmap(pcie->dev, port->base); - devm_release_mem_region(pcie->dev, port->regs.start, - resource_size(&port->regs)); - list_del(&port->list); - devm_kfree(pcie->dev, port); -} - /* Tegra PCIE root complex wrongly reports device class */ static void tegra_pcie_fixup_class(struct pci_dev *dev) { @@ -2053,7 +2042,6 @@ static int tegra_pcie_enable(struct tegra_pcie *pcie) dev_info(pcie->dev, "link %u down, ignoring\n", port->index);
tegra_pcie_port_disable(port); - tegra_pcie_port_free(port); }
memset(&hw, 0, sizeof(hw));