On Wed, Aug 20, 2014 at 01:04:22PM -0600, Stephen Warren wrote:
On 08/18/2014 01:16 AM, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
Add support for the PCIe controller found on some generations of Tegra. Tegra20 has 2 root ports with a total of 4 lanes, Tegra30 has 3 root ports with a total of 6 lanes and Tegra124 has 2 root ports with a total of 5 lanes.
diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c
- Copyright (c) 2010, CompuLab, Ltd.
- Author: Mike Rapoport mike@compulab.co.il
- Based on NVIDIA PCIe driver
- Copyright (c) 2008-2009, NVIDIA Corporation.
Presumably also at least (c) 2013-2014 NVIDIA too?
Yes.
Do you have Mike's S-o-b for the initial work?
Yes, I can take it from the kernel's git log.
+static int tegra_pcie_read_conf(struct pci_controller *hose, pci_dev_t bdf,
int where, u32 *value)+{
- struct tegra_pcie *pcie = to_tegra_pcie(hose);
- unsigned long address;
- int err;
- err = tegra_pcie_conf_address(pcie, bdf, where, &address);
- if (err < 0) {
*value = 0xffffffff;return 1;- }
In the kernel, don't we have to play games to dynamically map/unmap memory regions and/or map pages in a non-linear fashion since the config space is so large, and bus/device ID -> address mapping is odd? I don't see that here.
We do that in the kernel to avoid having to I/O remap a full 256 MiB of memory even if only a single PCI bus is needed. Since memory is mapped 1:1 in U-Boot and we access physical memory "directly", we don't have to worry about being wasteful. The tegra_pcie_conf_offset() function computes the proper offset:
static unsigned long tegra_pcie_conf_offset(pci_dev_t bdf, int where) { return ((where & 0xf00) << 16) | (PCI_BUS(bdf) << 16) | (PCI_DEV(bdf) << 11) | (PCI_FUNC(bdf) << 8) | (where & 0xfc); }
+static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port) +{
- unsigned int retries = 3;
- unsigned long value;
- value = rp_readl(port, RP_PRIV_MISC);
- value &= ~RP_PRIV_MISC_PRSNT_MAP_EP_ABSNT;
- value |= RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT;
- rp_writel(port, value, RP_PRIV_MISC);
- do {
unsigned int timeout = 200;do {value = rp_readl(port, RP_VEND_XP);if (value & RP_VEND_XP_DL_UP)break;On my board, this almost never succeeds for Jetson TK1's built-in NIC, although it works fine for the mini-PCIe slot. On the other two boards I borrowed, this succeeds almost always. I'd prefer if we could get to the bottom of this before we actually apply this, although the bug is proving quite perplexing. The mainline kernel PCIe patches have the same issue.
That's indeed odd. It's possible that the powergate fix that I've applied (to wait for the partition to become powered) may improve things. I'll be pushing out my latest work shortly (I don't think I'll manage to send out the whole series for review, though), so in case you get the chance you may want to test it.
+static int process_nodes(const void *fdt, int nodes[], unsigned int count)
+#ifdef CONFIG_PCI_SCAN_SHOW
printf("PCI: Enumerating devices...\n");printf("---------------------------------------\n");printf(" Device ID Description\n");printf(" ------ -- -----------\n");+#endif
pcie->hose.last_busno = pci_hose_scan(&pcie->hose);Shouldn't that ifdef'd block be part of pci_hose_scan()?
Every driver does it this way, but I agree that it should presumably go in the core somewhere. I'll see if I can find a good place to do it.
Thierry