From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Mon, 16 Nov 2015 16:27:30 -0700
Subject: [U-Boot] [PATCH v4 8/8] dm: tegra: pci: Convert tegra boards to
 driver model for PCI
In-Reply-To: <CAPnjgZ20s2QkvVz2SAwJtyeWbqQVJz=LngBWUAddWoz8BQtjkw@mail.gmail.com>
References: <1447474781-26929-1-git-send-email-sjg@chromium.org>
 <1447474781-26929-9-git-send-email-sjg@chromium.org>
 <564A6335.5060601@wwwdotorg.org>
 <CAPnjgZ20s2QkvVz2SAwJtyeWbqQVJz=LngBWUAddWoz8BQtjkw@mail.gmail.com>
Message-ID: <564A6662.6040606@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 11/16/2015 04:20 PM, Simon Glass wrote:
> On 16 November 2015 at 16:13, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> 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.
>
> Thanks for looking at this.
>
>> To make this series work on Jetson TX1, two things need to happen.
...
>>> 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?
>
> Does gd->pci_ram_top in decode_regions() do what you want?

Ah of course; the following does solve the problem:

> diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c
> index 8ba143d996ca..d2c957a2a426 100644
> --- a/arch/arm/mach-tegra/board2.c
> +++ b/arch/arm/mach-tegra/board2.c
> @@ -377,6 +377,8 @@ void dram_init_banksize(void)
>  	gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
>  	gd->bd->bi_dram[0].size = usable_ram_size_below_4g();
>
> +	gd->pci_ram_top = gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size;
> +
>  #ifdef CONFIG_PHYS_64BIT
>  	if (gd->ram_size > SZ_2G) {
>  		gd->bd->bi_dram[1].start = 0x100000000;