[U-Boot] [PATCH v4 8/8] dm: tegra: pci: Convert tegra boards to driver model for PCI

Stephen Warren
2015-11-16T23:13:57Z

On 11/13/2015 09:19 PM, Simon Glass wrote:
> Adjust the Tegra PCI driver to support driver model and move all boards over
> at the same time. This can make use of some generic driver model code, such
> as the range-decoding logic.

To make this series work on Jetson TX1, two things need to happen.

First, this patch reverts a change I made earlier to move the call to
tegra_pcie_board_init() to an earlier point in the initialization
process, i.e. before /any/ part of the PCIe or pad HW is accessed. To
solve that, apply the following:

> diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c
> index ed363e4220c4..8f089b3f6bbe 100644
> --- a/drivers/pci/pci_tegra.c
> +++ b/drivers/pci/pci_tegra.c
> @@ -433,6 +435,11 @@ static int tegra_pcie_parse_port_info(const void *fdt, int node,
> return 0;
> }
>
> +int __weak tegra_pcie_board_init(void)
> +{
> + return 0;
> +}
> +
> static int tegra_pcie_parse_dt(const void *fdt, int node, enum tegra_pci_id id,
> struct tegra_pcie *pcie)
> {
> @@ -460,6 +467,8 @@ static int tegra_pcie_parse_dt(const void *fdt, int node, enum tegra_pci_id id,
> return err;
> }
>
> + tegra_pcie_board_init();
> +
> pcie->phy = tegra_xusb_phy_get(TEGRA_XUSB_PADCTL_PCIE);
> if (pcie->phy) {
> err = tegra_xusb_phy_prepare(pcie->phy);
> @@ -512,11 +521,6 @@ static int tegra_pcie_parse_dt(const void *fdt, int node, enum tegra_pci_id id,
> return 0;
> }
>
> -int __weak tegra_pcie_board_init(void)
> -{
> - return 0;
> -}
> -
> static int tegra_pcie_power_on(struct tegra_pcie *pcie)
> {
> const struct tegra_pcie_soc *soc = pcie->soc;
> @@ -534,8 +538,6 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
> return err;
> }
>
> - tegra_pcie_board_init();
> -
> err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_PCIE,
> PERIPH_ID_PCIE);
> if (err < 0) {

Second, the removal of the following piece of code:

> diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c

> -static int process_nodes(const void *fdt, int nodes[], unsigned int count)
> {
> - unsigned int i;
> - uint64_t dram_end;
> - uint32_t pci_dram_size;
> -
> - /* Clip PCI-accessible DRAM to 32-bits */
> - dram_end = ((uint64_t)NV_PA_SDRAM_BASE) + gd->ram_size;
> - if (dram_end > 0x100000000)
> - dram_end = 0x100000000;
> - pci_dram_size = dram_end - NV_PA_SDRAM_BASE;

... means that the PCI resource structure that represents system memory
ends up with size=0 rather than the expected size=0x80000000. That's
because gd->ram_size is 4GB and sizeof(res->size)==4.

Perhaps I could solve this by defining CONFIG_SYS_PCI_64BIT, which
changes the size of res->size to 8 bytes. However:

a) I'm a bit worried that changing the size of that type will lead to
other unexpected behavioural changes, e.g. interactions with DT parsing
due to cell count differences.

b) This only solves the integer overflow issue. In fact,
res->base+res->size should fit inside 32-bits since IIRC the PCIe
controller can only access 32-bit addresses, and hence we need to clip
the RAM size to acknowledge that, which the code above was doing. Doing
such a clipping operation in the generic/shared decode_regions() doesn't
feel like a good idea, at least not unconditionally.

In practice, (a) doesn't seem to be a problem, and perhaps we can ignore
(b), since board_get_usable_ram_top() (in arch/arm/mach-tegra/board2.c)
limits usable RAM size to fit within 32 bits for similar reasons. So, I
know that no allocated memory buffer will ever be above 32 bits, so it
doesn't matter in practice whether a PCI region's base+size fits into 32
bits or not.

What are your thoughts on this?

BTW, your V4 plus these fixes is available at:

git://github.com/swarren/u-boot.git test-pci-dm-v4