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

[Simon Glass]

Hi Stephen,

On 16 November 2015 at 16:13, Stephen Warren <swarren at 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.
>
> 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) {
>

OK

>
> 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?

Does gd->pci_ram_top in decode_regions() do what you want?

>
> BTW, your V4 plus these fixes is available at:
>
> git://github.com/swarren/u-boot.git test-pci-dm-v4

Thanks!

Regards,
Simon