From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Wed, 20 Aug 2014 13:04:22 -0600
Subject: [U-Boot] [PATCH 15/23] pci: tegra: Add Tegra PCIe driver
In-Reply-To: <1408346196-30419-16-git-send-email-thierry.reding@gmail.com>
References: <1408346196-30419-1-git-send-email-thierry.reding@gmail.com>
	<1408346196-30419-16-git-send-email-thierry.reding@gmail.com>
Message-ID: <53F4F136.7010708@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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?

Do you have Mike's S-o-b for the initial work?

> +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.

> +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.

> +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()?